VR: Send VM password to all Running VRs in network/vpc

[ravening]

Description

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.

Types of changes

• Breaking change (fix or feature that would cause existing functionality to change)
• New feature (non-breaking change which adds functionality)
• 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?

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

[weizhouapache]

@ravening it looks the title is not correct. vm password is sent to all Running VRs , not MASTER VR.

[weizhouapache]

tested ok with 4.13

[DaanHoogland]

looks ok, just one question to author (and one to reviewer)

[DaanHoogland]

@blueorangutan package

[blueorangutan]

@DaanHoogland a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.

[blueorangutan]

Packaging result: ✖centos6 ✔centos7 ✔debian. JID-935

[DaanHoogland]

@blueorangutan test

[blueorangutan]

@DaanHoogland a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests

[GabrielBrascher]

Thanks for the PR @ravening.

Just one point that I got concerned. Please correct me if I am wrong, but in case the savePasswordToRouter method fails (and therefore returns false), savePasswordResult will be set to false and will stay false during the whole loop (due to &&).

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]

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 :)

[weizhouapache]

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 ?

[ravening]

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

[DaanHoogland]

@blueorangutan package

[blueorangutan]

@DaanHoogland a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.

[blueorangutan]

Packaging result: ✖centos6 ✔centos7 ✔debian. JID-952

[DaanHoogland]

@blueorangutan test

[blueorangutan]

@DaanHoogland a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests

[blueorangutan]

Trillian test result (tid-1133)
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

[weizhouapache]

@ravening it seems savePasswordResult is not needed any more, because it is always true.