You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudstack.apache.org by GitBox <gi...@apache.org> on 2020/02/20 13:17:24 UTC
[GitHub] [cloudstack] ravening opened a new pull request #3903: Send VM
password to MASTER VR in case its the second vr in network
ravening opened a new pull request #3903: Send VM password to MASTER VR in case its the second vr in network
URL: https://github.com/apache/cloudstack/pull/3903
## Description
<!--- Describe your changes in detail -->
Currently, the cloudstack sends VM password only to the first
router in the network even if its the backup and return the result.
In some cases the first router will be back up and the second will be master.
Since password server is not running in backup, when the user resets the password,
it is sent to the first router which can be backup.
In that case, the new password is not stored in the password server and users cant log in with a new password.
This change ensures that we send the password to both the routers instead
of the first router so that a new password is stored in the master router.
<!-- For new features, provide link to FS, dev ML discussion etc. -->
<!-- In case of bug fix, the expected and actual behaviours, steps to reproduce. -->
<!-- When "Fixes: #<id>" is specified, the issue/PR will automatically be closed when this PR gets merged -->
<!-- For addressing multiple issues/PRs, use multiple "Fixes: #<id>" -->
<!-- Fixes: # -->
## Types of changes
<!--- What types of changes does your code introduce? Put an `x` in all the boxes that apply: -->
- [ ] Breaking change (fix or feature that would cause existing functionality to change)
- [ ] New feature (non-breaking change which adds functionality)
- [X] Bug fix (non-breaking change which fixes an issue)
- [ ] Enhancement (improves an existing feature and functionality)
- [ ] Cleanup (Code refactoring and cleanup, that may add test cases)
## Screenshots (if appropriate):
## How Has This Been Tested?
<!-- Please describe in detail how you tested your changes. -->
<!-- Include details of your testing environment, and the tests you ran to -->
<!-- see how your change affects other areas of the code, etc. -->
1 . Create and isolated network with redundant VR and a VM in it.
2 . Note down the password of the newly created and try to ssh to it from VR
3 . Ssh works with the existing password.
4 . Now reboot the master VR so that it will become BACKUP and the BACKUP will become MASTER
5 . Now stop the VM and reset it's password.
6 . Try to login to VM using the new password
Expected Result:
Ssh should work with new password
Actual result:
Ssh wont work
<!-- Please read the [CONTRIBUTING](https://github.com/apache/cloudstack/blob/master/CONTRIBUTING.md) document -->
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] DaanHoogland commented on a change in pull request
#3903: VR: Send VM password to all Running VRs in network/vpc
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on a change in pull request #3903: VR: Send VM password to all Running VRs in network/vpc
URL: https://github.com/apache/cloudstack/pull/3903#discussion_r382254459
##########
File path: server/src/main/java/com/cloud/network/element/VirtualRouterElement.java
##########
@@ -702,18 +702,27 @@ public boolean savePassword(final Network network, final NicProfile nic, final V
// If any router is running then send save password command otherwise
// save the password in DB
+ boolean savePasswordResult = true;
+ boolean isVrRunning = false;
for (final VirtualRouter router : routers) {
if (router.getState() == State.Running) {
final boolean result = networkTopology.savePasswordToRouter(network, nic, uservm, router);
- if (result) {
- // Explicit password reset, while VM hasn't generated a password yet.
- final UserVmVO userVmVO = _userVmDao.findById(vm.getId());
- userVmVO.setUpdateParameters(false);
- _userVmDao.update(userVmVO.getId(), userVmVO);
- }
- return result;
+ isVrRunning = true;
+ savePasswordResult = savePasswordResult & result;
Review comment:
is ok, but a former colleague scorned me for this once; why the bitwise and (single &), accidental?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] ravening commented on a change in pull request #3903:
VR: Send VM password to all Running VRs in network/vpc
Posted by GitBox <gi...@apache.org>.
ravening commented on a change in pull request #3903: VR: Send VM password to all Running VRs in network/vpc
URL: https://github.com/apache/cloudstack/pull/3903#discussion_r382497563
##########
File path: server/src/main/java/com/cloud/network/element/VirtualRouterElement.java
##########
@@ -702,18 +702,27 @@ public boolean savePassword(final Network network, final NicProfile nic, final V
// If any router is running then send save password command otherwise
// save the password in DB
+ boolean savePasswordResult = true;
+ boolean isVrRunning = false;
for (final VirtualRouter router : routers) {
if (router.getState() == State.Running) {
final boolean result = networkTopology.savePasswordToRouter(network, nic, uservm, router);
- if (result) {
- // Explicit password reset, while VM hasn't generated a password yet.
- final UserVmVO userVmVO = _userVmDao.findById(vm.getId());
- userVmVO.setUpdateParameters(false);
- _userVmDao.update(userVmVO.getId(), userVmVO);
- }
- return result;
+ isVrRunning = true;
+ savePasswordResult = savePasswordResult & result;
Review comment:
will change it
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] blueorangutan commented on issue #3903: VR: Send VM
password to all Running VRs in network/vpc
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on issue #3903: VR: Send VM password to all Running VRs in network/vpc
URL: https://github.com/apache/cloudstack/pull/3903#issuecomment-591467773
@DaanHoogland a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] DaanHoogland commented on issue #3903: VR: Send VM
password to all Running VRs in network/vpc
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on issue #3903: VR: Send VM password to all Running VRs in network/vpc
URL: https://github.com/apache/cloudstack/pull/3903#issuecomment-591467366
@blueorangutan test
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] ravening edited a comment on issue #3903: VR: Send VM
password to all Running VRs in network/vpc
Posted by GitBox <gi...@apache.org>.
ravening edited a comment on issue #3903: VR: Send VM password to all Running VRs in network/vpc
URL: https://github.com/apache/cloudstack/pull/3903#issuecomment-591318066
> > Is it OK to have one of the routers returning false? Would it make sense do break de loop and log an error/warn/debug message?
>
> @GabrielBrascher good point. if result is false, we can simply log a error message and return false.
> @ravening what do you think ?
@GabrielBrascher Looked at the code and the function `applyRules` is throwing exception if anything bad happens. So yeah returning false makes sense if it fails for the first router and no need to continue
I have made the necessary changes. Please review it again
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] blueorangutan commented on issue #3903: VR: Send VM
password to all Running VRs in network/vpc
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on issue #3903: VR: Send VM password to all Running VRs in network/vpc
URL: https://github.com/apache/cloudstack/pull/3903#issuecomment-591371357
Packaging result: ✖centos6 ✔centos7 ✔debian. JID-952
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] DaanHoogland commented on issue #3903: VR: Send VM
password to all Running VRs in network/vpc
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on issue #3903: VR: Send VM password to all Running VRs in network/vpc
URL: https://github.com/apache/cloudstack/pull/3903#issuecomment-591363485
@blueorangutan package
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] DaanHoogland commented on a change in pull request
#3903: VR: Send VM password to all Running VRs in network/vpc
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on a change in pull request #3903: VR: Send VM password to all Running VRs in network/vpc
URL: https://github.com/apache/cloudstack/pull/3903#discussion_r382452030
##########
File path: server/src/main/java/com/cloud/network/element/VirtualRouterElement.java
##########
@@ -702,18 +702,27 @@ public boolean savePassword(final Network network, final NicProfile nic, final V
// If any router is running then send save password command otherwise
// save the password in DB
+ boolean savePasswordResult = true;
+ boolean isVrRunning = false;
Review comment:
yes @weizhouapache @GabrielBrascher , and i don't even think the keyword is valid in that place. so unless you are asking to move them to fields...?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] GabrielBrascher commented on a change in pull request
#3903: VR: Send VM password to all Running VRs in network/vpc
Posted by GitBox <gi...@apache.org>.
GabrielBrascher commented on a change in pull request #3903: VR: Send VM password to all Running VRs in network/vpc
URL: https://github.com/apache/cloudstack/pull/3903#discussion_r382784999
##########
File path: server/src/main/java/com/cloud/network/element/VirtualRouterElement.java
##########
@@ -702,18 +702,27 @@ public boolean savePassword(final Network network, final NicProfile nic, final V
// If any router is running then send save password command otherwise
// save the password in DB
+ boolean savePasswordResult = true;
+ boolean isVrRunning = false;
Review comment:
You are right, I did not expand correctly the diff and saw it as on the class instead of method.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] weizhouapache commented on a change in pull request
#3903: VR: Send VM password to all Running VRs in network/vpc
Posted by GitBox <gi...@apache.org>.
weizhouapache commented on a change in pull request #3903: VR: Send VM password to all Running VRs in network/vpc
URL: https://github.com/apache/cloudstack/pull/3903#discussion_r382450625
##########
File path: server/src/main/java/com/cloud/network/element/VirtualRouterElement.java
##########
@@ -702,18 +702,27 @@ public boolean savePassword(final Network network, final NicProfile nic, final V
// If any router is running then send save password command otherwise
// save the password in DB
+ boolean savePasswordResult = true;
+ boolean isVrRunning = false;
Review comment:
@GabrielBrascher isnot variables private by default ?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] blueorangutan commented on issue #3903: VR: Send VM
password to all Running VRs in network/vpc
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on issue #3903: VR: Send VM password to all Running VRs in network/vpc
URL: https://github.com/apache/cloudstack/pull/3903#issuecomment-589965211
Packaging result: ✖centos6 ✔centos7 ✔debian. JID-935
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] rhtyd merged pull request #3903: VR: Send VM password
to all Running VRs in network/vpc
Posted by GitBox <gi...@apache.org>.
rhtyd merged pull request #3903: VR: Send VM password to all Running VRs in network/vpc
URL: https://github.com/apache/cloudstack/pull/3903
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] weizhouapache commented on issue #3903: VR: Send VM
password to all Running VRs in network/vpc
Posted by GitBox <gi...@apache.org>.
weizhouapache commented on issue #3903: VR: Send VM password to all Running VRs in network/vpc
URL: https://github.com/apache/cloudstack/pull/3903#issuecomment-590740421
> Is it OK to have one of the routers returning false? Would it make sense do break de loop and log an error/warn/debug message?
@GabrielBrascher good point. if result is false, we can simply log a error message and return false.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] DaanHoogland commented on issue #3903: VR: Send VM
password to all Running VRs in network/vpc
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on issue #3903: VR: Send VM password to all Running VRs in network/vpc
URL: https://github.com/apache/cloudstack/pull/3903#issuecomment-589963307
@blueorangutan package
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] blueorangutan commented on issue #3903: VR: Send VM
password to all Running VRs in network/vpc
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on issue #3903: VR: Send VM password to all Running VRs in network/vpc
URL: https://github.com/apache/cloudstack/pull/3903#issuecomment-590116294
@DaanHoogland a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] blueorangutan commented on issue #3903: VR: Send VM
password to all Running VRs in network/vpc
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on issue #3903: VR: Send VM password to all Running VRs in network/vpc
URL: https://github.com/apache/cloudstack/pull/3903#issuecomment-591363855
@DaanHoogland a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] blueorangutan commented on issue #3903: VR: Send VM
password to all Running VRs in network/vpc
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on issue #3903: VR: Send VM password to all Running VRs in network/vpc
URL: https://github.com/apache/cloudstack/pull/3903#issuecomment-589963332
@DaanHoogland a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] weizhouapache commented on a change in pull request
#3903: VR: Send VM password to all Running VRs in network/vpc
Posted by GitBox <gi...@apache.org>.
weizhouapache commented on a change in pull request #3903: VR: Send VM password to all Running VRs in network/vpc
URL: https://github.com/apache/cloudstack/pull/3903#discussion_r382450608
##########
File path: server/src/main/java/com/cloud/network/element/VirtualRouterElement.java
##########
@@ -702,18 +702,27 @@ public boolean savePassword(final Network network, final NicProfile nic, final V
// If any router is running then send save password command otherwise
// save the password in DB
+ boolean savePasswordResult = true;
+ boolean isVrRunning = false;
for (final VirtualRouter router : routers) {
if (router.getState() == State.Running) {
final boolean result = networkTopology.savePasswordToRouter(network, nic, uservm, router);
- if (result) {
- // Explicit password reset, while VM hasn't generated a password yet.
- final UserVmVO userVmVO = _userVmDao.findById(vm.getId());
- userVmVO.setUpdateParameters(false);
- _userVmDao.update(userVmVO.getId(), userVmVO);
- }
- return result;
+ isVrRunning = true;
+ savePasswordResult = savePasswordResult & result;
Review comment:
@DaanHoogland 👍
maybe it is a typo, we should use && instead of &.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] GabrielBrascher commented on a change in pull request
#3903: VR: Send VM password to all Running VRs in network/vpc
Posted by GitBox <gi...@apache.org>.
GabrielBrascher commented on a change in pull request #3903: VR: Send VM password to all Running VRs in network/vpc
URL: https://github.com/apache/cloudstack/pull/3903#discussion_r382205814
##########
File path: server/src/main/java/com/cloud/network/element/VirtualRouterElement.java
##########
@@ -702,18 +702,27 @@ public boolean savePassword(final Network network, final NicProfile nic, final V
// If any router is running then send save password command otherwise
// save the password in DB
+ boolean savePasswordResult = true;
+ boolean isVrRunning = false;
Review comment:
@ravening can you please make both variables private?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] GabrielBrascher commented on issue #3903: VR: Send VM
password to all Running VRs in network/vpc
Posted by GitBox <gi...@apache.org>.
GabrielBrascher commented on issue #3903: VR: Send VM password to all Running VRs in network/vpc
URL: https://github.com/apache/cloudstack/pull/3903#issuecomment-590293123
Additionally to what I mentioned on the previous comment:
`BasicNetworkTopology.applyRules(Network, VirtualRouter, String, boolean, Long, boolean, RuleApplierWrapper<RuleApplier>)` is the method that will return true or false on `savePasswordToRouter`. Most of the issues that can happen there will either raise an exception or log it so I don't see much problem; however, I got interested in the fact that it might make sense to break the loop (and maybe add a log to that explaining).
Again, overall it looks OK :)
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] ravening commented on issue #3903: VR: Send VM
password to all Running VRs in network/vpc
Posted by GitBox <gi...@apache.org>.
ravening commented on issue #3903: VR: Send VM password to all Running VRs in network/vpc
URL: https://github.com/apache/cloudstack/pull/3903#issuecomment-591318066
> > Is it OK to have one of the routers returning false? Would it make sense do break de loop and log an error/warn/debug message?
>
> @GabrielBrascher good point. if result is false, we can simply log a error message and return false.
> @ravening what do you think ?
@GabrielBrascher Looked at the code and the function `applyRules` is throwing exception if anything bad happens. So yeah returning false makes sense if it fails for the first router and no need to continue
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] DaanHoogland commented on issue #3903: VR: Send VM
password to all Running VRs in network/vpc
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on issue #3903: VR: Send VM password to all Running VRs in network/vpc
URL: https://github.com/apache/cloudstack/pull/3903#issuecomment-590116206
@blueorangutan test
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] weizhouapache commented on issue #3903: Send VM
password to MASTER VR in case its the second vr in network
Posted by GitBox <gi...@apache.org>.
weizhouapache commented on issue #3903: Send VM password to MASTER VR in case its the second vr in network
URL: https://github.com/apache/cloudstack/pull/3903#issuecomment-589102494
@ravening it looks the title is not correct. vm password is sent to all Running VRs , not MASTER VR.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] weizhouapache edited a comment on issue #3903: VR:
Send VM password to all Running VRs in network/vpc
Posted by GitBox <gi...@apache.org>.
weizhouapache edited a comment on issue #3903: VR: Send VM password to all Running VRs in network/vpc
URL: https://github.com/apache/cloudstack/pull/3903#issuecomment-590740421
> Is it OK to have one of the routers returning false? Would it make sense do break de loop and log an error/warn/debug message?
@GabrielBrascher good point. if result is false, we can simply log a error message and return false.
@ravening what do you think ?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] ravening commented on a change in pull request #3903:
VR: Send VM password to all Running VRs in network/vpc
Posted by GitBox <gi...@apache.org>.
ravening commented on a change in pull request #3903: VR: Send VM password to all Running VRs in network/vpc
URL: https://github.com/apache/cloudstack/pull/3903#discussion_r382495380
##########
File path: server/src/main/java/com/cloud/network/element/VirtualRouterElement.java
##########
@@ -702,18 +702,27 @@ public boolean savePassword(final Network network, final NicProfile nic, final V
// If any router is running then send save password command otherwise
// save the password in DB
+ boolean savePasswordResult = true;
+ boolean isVrRunning = false;
Review comment:
@GabrielBrascher @DaanHoogland do you still want me to add private keyword?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] weizhouapache commented on a change in pull request
#3903: VR: Send VM password to all Running VRs in network/vpc
Posted by GitBox <gi...@apache.org>.
weizhouapache commented on a change in pull request #3903: VR: Send VM password to all Running VRs in network/vpc
URL: https://github.com/apache/cloudstack/pull/3903#discussion_r384973413
##########
File path: server/src/main/java/com/cloud/network/element/VirtualRouterElement.java
##########
@@ -702,18 +702,32 @@ public boolean savePassword(final Network network, final NicProfile nic, final V
// If any router is running then send save password command otherwise
// save the password in DB
+ boolean savePasswordResult = true;
+ boolean isVrRunning = false;
for (final VirtualRouter router : routers) {
if (router.getState() == State.Running) {
final boolean result = networkTopology.savePasswordToRouter(network, nic, uservm, router);
- if (result) {
- // Explicit password reset, while VM hasn't generated a password yet.
- final UserVmVO userVmVO = _userVmDao.findById(vm.getId());
- userVmVO.setUpdateParameters(false);
- _userVmDao.update(userVmVO.getId(), userVmVO);
+ if (!result) {
+ s_logger.error("Unable to save password for VM " + vm.getInstanceName() +
+ " on router " + router.getInstanceName());
+ return false;
}
- return result;
+ isVrRunning = true;
+ savePasswordResult = savePasswordResult && result;
Review comment:
@ravening it seems savePasswordResult is not needed any more, because it is always true.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [cloudstack] blueorangutan commented on issue #3903: VR: Send VM
password to all Running VRs in network/vpc
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on issue #3903: VR: Send VM password to all Running VRs in network/vpc
URL: https://github.com/apache/cloudstack/pull/3903#issuecomment-591691301
<b>Trillian test result (tid-1133)</b>
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 27970 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr3903-t1133-kvm-centos7.zip
Intermittent failure detected: /marvin/tests/smoke/test_privategw_acl.py
Smoke tests completed. 76 look OK, 1 have error(s)
Only failed tests results shown below:
Test | Result | Time (s) | Test File
--- | --- | --- | ---
test_02_vpc_privategw_static_routes | `Failure` | 179.14 | test_privategw_acl.py
test_03_vpc_privategw_restart_vpc_cleanup | `Failure` | 182.24 | test_privategw_acl.py
test_04_rvpc_privategw_static_routes | `Failure` | 236.95 | test_privategw_acl.py
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services