Skip to content

CLOUDSTACK-10220: Configure IPv4 alias on VR regardless of IPv6 #2396

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
yadvr merged 2 commits into from
Jan 12, 2018
Merged

CLOUDSTACK-10220: Configure IPv4 alias on VR regardless of IPv6 #2396

yadvr merged 2 commits into from
Jan 12, 2018

Conversation

@wido
Copy link
Contributor

@wido wido commented Jan 9, 2018

For some reason when a IPv6 address is present for a NIC IPv4 subnet aliases are not configured on the VR.

This seems to be very old code (>5 yr) and written at a time when IPv6 wasn't implemented in CloudStack

Removing this additional evaluation in the if-statement works for us in Basic Networking

wido added 2 commits January 9, 2018 14:35
@wido
CLOUDSTACK-10220: Configure IPv4 NIC alias on VR regardless of IPv6
a4e976f 
IPv4 and IPv6 are two different protocols and the presence of IPv6
in a network does not mean that IPv4 aliases/multiple subnets should
not be configured or supported by the VR.

This if-statement was written almost 5 years ago in a attempt to
add IPv6 support to CloudStack but was never fully implemented.

Signed-off-by: Wido den Hollander <wido@widodh.nl>
@wido
CLOUDSTACK-10220: Simplify if-statement by using already present method
717e40e 
The function isDhcpAccrossMultipleSubnetsSupported() in the same file
already does what this if-statement does.

This removes duplicate code.

Signed-off-by: Wido den Hollander <wido@widodh.nl>
@yadvr
Copy link
Member

yadvr commented Jan 10, 2018

@blueorangutan package

@blueorangutan
Copy link

blueorangutan commented Jan 10, 2018

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

@blueorangutan
Copy link

blueorangutan commented Jan 10, 2018

Packaging result: ✔centos6 ✔centos7 ✔debian. JID-1642

@yadvr
Copy link
Member

yadvr commented Jan 10, 2018

@blueorangutan test

@blueorangutan
Copy link

blueorangutan commented Jan 10, 2018

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

@blueorangutan
Copy link

blueorangutan commented Jan 11, 2018

Trillian test result (tid-2109)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 35734 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr2396-t2109-kvm-centos7.zip
Intermitten failure detected: /marvin/tests/smoke/test_privategw_acl.py
Intermitten failure detected: /marvin/tests/smoke/test_templates.py
Intermitten failure detected: /marvin/tests/smoke/test_volumes.py
Intermitten failure detected: /marvin/tests/smoke/test_host_maintenance.py
Smoke tests completed. 64 look OK, 3 have error(s)
Only failed tests results shown below:

Test Result Time (s) Test File
test_03_deploy_vm_wrong_checksum Error 59.40 test_templates.py
ContextSuite context=TestCreateTemplateWithDirectDownload>:teardown Error 73.08 test_templates.py
test_07_resize_fail Failure 4.52 test_volumes.py
test_02_cancel_host_maintenace_with_migration_jobs Error 2.56 test_host_maintenance.py

@yadvr
Copy link
Member

yadvr commented Jan 12, 2018

Tests LGTM. Failures not related to this PR.
Given @wido has flagged this as a blocker due to issue in 4.10, we can accept this PR towards 4.11 milestone as a blocker fix if others agree in the community - @DaanHoogland @rafaelweingartner @marcaurele @borisstoyanov @bwsw @resmo and others.

rafaelweingartner
rafaelweingartner approved these changes Jan 12, 2018
View reviewed changes
Copy link
Member

@rafaelweingartner rafaelweingartner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Everything seems to be ok. I only think that you should squash commits for this one.

@yadvr yadvr added this to the 4.11 milestone Jan 12, 2018
@yadvr
Copy link
Member

yadvr commented Jan 12, 2018

Thanks @rafaelweingartner, I'll squas merge this now as we're two code review lgtms and test results.

@yadvr yadvr merged commit 823a789 into apache:master Jan 12, 2018
julien-vaz pushed a commit to scclouds/cloudstack that referenced this pull request Feb 18, 2025
Merge branch 'backport-incremental-snap' into '4.20.0.0-scclouds'
b67ce5f 
Port 4.20 - Incremental volume snapshots

Closes apache#2400, apache#2296, and apache#2396

See merge request scclouds/scclouds!976
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@yadvr yadvr yadvr approved these changes

@rafaelweingartner rafaelweingartner rafaelweingartner approved these changes

Assignees

No one assigned

Labels

component:basic-network component:ipv6 component:virtual-router type:bug

Projects

None yet

Milestone

4.11.0.0

Development

Successfully merging this pull request may close these issues.

4 participants

@wido @yadvr @blueorangutan @rafaelweingartner