Fix failure in validating IP address in case of multiple Management Servers

[Pearl1594]

Description

When there are multiple Management servers, their IPs are listed as comma separated string in /var/cache/cloud/cmdline file against the "host" parameter, and when setting up the SSVM, SSVM fails to validate the IP of the management servers:

WARN  [storage.resource.NfsSecondaryStorageResource] (main:null) destIp is not a valid ip address or cidr destIp=x.x.x.x,y.y.y.y

In order to fix this, the IP addresses are split on the delimiter - ',' comma and then the further operations are performed.

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)

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

• Major
• Minor

Bug Severity

• BLOCKER
• Critical
• Major
• Minor
• Trivial

How Has This Been Tested?

Deployed a setup with 2 management servers, and validated the fix. No longer fails validation of IP address

[Pearl1594]

@blueorangutan package

[blueorangutan]

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

[blueorangutan]

Packaging result: ✖centos7 ✖centos8 ✖debian. JID-2446

[Pearl1594]

@blueorangutan package

[blueorangutan]

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

[blueorangutan]

Packaging result: ✔centos7 ✖centos8 ✔debian. JID-2447

[Pearl1594]

@blueorangutan test

[blueorangutan]

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

[DaanHoogland]

Though I see the need of this fix, i am a bit wary of it. It must be implemented somewhere for the kvm agent as well, so why is that code not called here. It seems like we are doubling/overlooking something.

[weizhouapache]

@Pearl1594 why not change line 2592 to below ?

                String mgmtHosts = (String)params.get("host");
                for (final String mgmtHost : mgmtHosts.split(",")) {
                    addRouteToInternalIpOrCidr(_localgw, _eth1ip, _eth1mask, mgmtHost);
                }

[blueorangutan]

Trillian test result (tid-3288)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 32996 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4507-t3288-kvm-centos7.zip
Intermittent failure detected: /marvin/tests/smoke/test_kubernetes_clusters.py
Intermittent failure detected: /marvin/tests/smoke/test_kubernetes_supported_versions.py
Smoke tests completed. 83 look OK, 0 have error(s)
Only failed tests results shown below:

Test	Result	Time (s)	Test File

[weizhouapache]

code LGTM

[sureshanaparti]

code changes LGTM, same behavior exists in ConsoleProxyResource.

[Pearl1594]

Though I see the need of this fix, i am a bit wary of it. It must be implemented somewhere for the kvm agent as well, so why is that code not called here. It seems like we are doubling/overlooking something.

This situation does not arise in case of KVM agent @DaanHoogland

[Pearl1594]

@blueorangutan package

[blueorangutan]

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

[blueorangutan]

Packaging result: ✔centos7 ✖centos8 ✔debian. JID-2452

[Pearl1594]

@blueorangutan test

[blueorangutan]

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

[DaanHoogland]

Though I see the need of this fix, i am a bit wary of it. It must be implemented somewhere for the kvm agent as well, so why is that code not called here. It seems like we are doubling/overlooking something.

This situation does not arise in case of KVM agent @DaanHoogland

I think I wasn't clear , @Pearl1594 . The kvm agent must have code to handle the case of multiple ip addresses for the MS, why isn't the same code used for both? The ip selection process during connecting is the same for all agent so the code should be the same bit as well.

[blueorangutan]

Trillian test result (tid-3299)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 33607 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4507-t3299-kvm-centos7.zip
Intermittent failure detected: /marvin/tests/smoke/test_internal_lb.py
Intermittent failure detected: /marvin/tests/smoke/test_ssvm.py
Smoke tests completed. 83 look OK, 0 have error(s)
Only failed tests results shown below:

Test	Result	Time (s)	Test File

[Pearl1594]

@DaanHoogland wrt KVM agent, the workflow followed is a bit different:
https://github.com/apache/cloudstack/blob/master/agent/src/main/java/com/cloud/agent/Agent.java#L194
https://github.com/apache/cloudstack/blob/master/agent/src/main/java/com/cloud/agent/AgentShell.java#L152
And here, we correctly consider the possibility of having multiple MS and split them accordingly.

This piece of code aligns to how ConsoleProxyResource handles multiple MSs.

[DaanHoogland]

ok, I'll shut my loud mouth, @Pearl1594
@PaulAngus this is minor but meet merge criteria. At your discretion.

[shwstppr]

LGTM