CLOUDSTACK-9853: Add support for Secondary IPv6 Addresses and Subnets#2028
CLOUDSTACK-9853: Add support for Secondary IPv6 Addresses and Subnets#2028yadvr merged 1 commit intoapache:masterfrom
Conversation
| private static final String CIDR_LENGTH_SEPARATOR = "/"; | ||
| private static final char RULE_TARGET_SEPARATOR = ','; | ||
| private static final char RULE_COMMAND_SEPARATOR = ':'; | ||
| private static final char RULE_COMMAND_SEPARATOR = ';'; |
There was a problem hiding this comment.
@wido can changing the separator cause issues (upgrade issues) for existing environments?
There was a problem hiding this comment.
Not that I'm sure of. I wouldn't know why as this is all run locally on the HV in KVM's case
|
LGTM, @wido any chance we can refactor RULE_COMMAND_SEPARATOR and use it throughout java code at least? |
|
@blueorangutan package |
|
@rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress. |
|
Packaging result: ✖centos6 ✔centos7 ✖debian. JID-851 |
|
@blueorangutan package |
|
@rhtyd We could do that maybe, but I don't know where RULE_COMMAND_SEPARATOR would need to be defined globally? |
|
@wido just a suggestion, we can move it possibly in some utils? (in the cloud-utils module?) |
|
Packaging result: ✖centos6 ✔centos7 ✖debian. JID-853 |
|
Packaging result: ✖centos6 ✔centos7 ✖debian. JID-854 |
|
@blueorangutan package |
|
@rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress. |
|
Packaging result: ✖centos6 ✔centos7 ✖debian. JID-862 |
|
@wido this one is facing build failures, please see. |
|
I see, looking at this one: Tests run: 3, Failures: 1, Errors: 0, Skipped: 0, Time elapsed: 0.013 sec <<< FAILURE! - in com.cloud.agent.api.SecurityGroupRulesCmdTest testCompressStringifiedRules(com.cloud.agent.api.SecurityGroupRulesCmdTest) Time elapsed: 0.01 sec <<< FAILURE! java.lang.AssertionError: null at org.junit.Assert.fail(Assert.java:86) at org.junit.Assert.assertTrue(Assert.java:41) at org.junit.Assert.assertTrue(Assert.java:52) at com.cloud.agent.api.SecurityGroupRulesCmdTest.testCompressStringifiedRules(SecurityGroupRulesCmdTest.java:91) |
This commit adds support for passing IPv6 Addresses and/or Subnets as Secondary IPs. This is groundwork for CLOUDSTACK-9853 where IPv6 Subnets have to be allowed in the Security Groups of Instances to we can add DHCPv6 Prefix Delegation. Use ; instead of : for separating addresses, otherwise it would cause problems with IPv6 Addresses. Signed-off-by: Wido den Hollander <wido@widodh.nl>
|
Test is fixed! Odd that it came through. I swear I fixed this earlier. Anyway, it's fixed now. |
|
@blueorangutan package |
|
@wido Travis has some failures, can you check? |
|
I checked @rhtyd, might be that I forgot a local force push. Did that again. All tests go through locally. Travis now fails on a OOB/IPMI test: The command ./tools/travis/script.sh smoke/test_outofbandmanagement smoke/test_over_provisioning smoke/test_password_server smoke/test_portable_publicip smoke/test_primary_storage smoke/test_privategw_acl smoke/test_public_ip_range smoke/test_pvlan smoke/test_regions smoke/test_reset_vm_on_reboot smoke/test_resource_detail smoke/test_router_dhcphosts smoke/test_router_dns smoke/test_routers smoke/test_routers_iptables_default_policy smoke/test_routers_network_ops smoke/test_scale_vm smoke/test_secondary_storage smoke/test_service_offerings smoke/test_snapshots smoke/test_ssvm smoke/test_staticroles smoke/test_templates smoke/test_usage_events smoke/test_vm_life_cycle smoke/test_vm_snapshots smoke/test_volumes smoke/test_vpc_redundant smoke/test_vpc_router_nics smoke/test_vpc_vpn smoke/misc/test_deploy_vm smoke/misc/test_escalations_templates smoke/misc/test_vm_ha smoke/misc/test_vm_sync exited with 1. |
|
@wido okay re-ran failed travis run. You can go to a travis job and re-kick it, might be one of the intermittent/env failures. |
|
All checks have now passed @rhtyd |
|
@wido thanks, will re-run tests |
|
@rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress. |
|
Packaging result: ✔centos6 ✔centos7 ✔debian. JID-880 |
|
@blueorangutan package |
|
@rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress. |
|
Packaging result: ✔centos6 ✔centos7 ✔debian. JID-988 |
|
Are there any further comments on this one or can I get the LGTM? |
|
@wido several test failures, we'll need another round and more extensive testing |
|
@rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress. |
|
Packaging result: ✖centos6 ✔centos7 ✔debian. JID-1078 |
|
@blueorangutan package |
|
@rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress. |
|
Packaging result: ✔centos6 ✔centos7 ✔debian. JID-1086 |
|
@blueorangutan test |
|
@rhtyd a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests |
|
Trillian test result (tid-1514)
|
|
It seems this PR is passing all the tests and has two LGTMs. Can we merge it? |
|
@wido let me kick another test round since results are old |
|
@rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress. |
|
Packaging result: ✔centos6 ✔centos7 ✔debian. JID-1179 |
|
@blueorangutan test centos7 vmware-55u3 |
|
@rhtyd a Trillian-Jenkins test job (centos7 mgmt + vmware-55u3) has been kicked to run smoke tests |
|
Trillian test result (tid-1599)
|
|
LGTM, the failing tests are known intermittent issues. I'll merge this, thanks @wido |
With merge of PR apache#2028 the separator for lines to the Security Group Python script changed from : to ; to support IPv6 addresses. This broke certain situations where rules were parsed improperly. This commit fixes that Signed-off-by: Wido den Hollander <wido@widodh.nl>
With merge of PR apache#2028 the separator for lines to the Security Group Python script changed from : to ; to support IPv6 addresses. This broke certain situations where rules were parsed improperly. This commit fixes the issue. Signed-off-by: Wido den Hollander <wido@widodh.nl> Signed-off-by: Rohit Yadav <rohit.yadav@shapeblue.com>
With merge of PR #2028 the separator for lines to the Security Group Python script changed from : to ; to support IPv6 addresses. This broke certain situations where rules were parsed improperly. This commit fixes the issue. Signed-off-by: Wido den Hollander <wido@widodh.nl> Signed-off-by: Rohit Yadav <rohit.yadav@shapeblue.com>
5 participants
This commit adds support for passing IPv6 Addresses and/or Subnets as
Secondary IPs.
This is groundwork for CLOUDSTACK-9853 where IPv6 Subnets have to be
allowed in the Security Groups of Instances to we can add DHCPv6
Prefix Delegation.
Use ; instead of : for separating addresses, otherwise it would cause
problems with IPv6 Addresses.
Signed-off-by: Wido den Hollander wido@widodh.nl