Skip to content

CLOUDSTACK-10212: Update Netmask/Gateway when Changing IPv4 address#2388

Merged
yadvr merged 1 commit intoapache:masterfrom
wido:CLOUDSTACK-10212
Jan 9, 2018
Merged

CLOUDSTACK-10212: Update Netmask/Gateway when Changing IPv4 address#2388
yadvr merged 1 commit intoapache:masterfrom
wido:CLOUDSTACK-10212

Conversation

@wido
Copy link
Contributor

@wido wido commented Jan 5, 2018

This can otherwise cause problems in Basic Networking where multiple
IPv4 ranges are configured in a POD.

Signed-off-by: Wido den Hollander wido@widodh.nl

Copy link
Contributor

@DaanHoogland DaanHoogland left a comment

Choose a reason for hiding this comment

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

code lgtm

@yadvr
Copy link
Member

yadvr commented Jan 5, 2018

@blueorangutan package

@yadvr
Copy link
Member

yadvr commented Jan 5, 2018

@wido can you check unit test/build failures from travis:

Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.015 sec - in com.cloud.keystore.KeystoreTest
Results :
Tests in error: 
  UserVmManagerTest.testUpdateVmNicIpSuccess2:854 » NullPointer

@yadvr yadvr removed this from the 4.11 milestone Jan 6, 2018
This can otherwise cause problems in Basic Networking where multiple
IPv4 ranges are configured in a POD.

Signed-off-by: Wido den Hollander <wido@widodh.nl>
@wido wido force-pushed the CLOUDSTACK-10212 branch from fe710ed to 3f2b237 Compare January 8, 2018 08:39
@wido
Copy link
Contributor Author

wido commented Jan 8, 2018

Fixed @rhtyd ! Tests are now all passing.

@yadvr
Copy link
Member

yadvr commented Jan 8, 2018

@blueorangutan package

@blueorangutan
Copy link

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

@blueorangutan
Copy link

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

@wido
Copy link
Contributor Author

wido commented Jan 8, 2018

@rhtyd I thought I added the milestone 4.11 to this PR, but I see you removed it:

wido added this to the 4.11 milestone 3 days ago
rhtyd removed this from the 4.11 milestone 2 days ago

Was this due to the NPE during the tests?

As this is a bug, we can include it in 4.11, right?

@yadvr
Copy link
Member

yadvr commented Jan 8, 2018

@wido Yes, due to NPE in tests I could run further tests/reviews. I've removed PRs from 4.11 milestone where authors were not replying or were busy elsewhere. This was done in past 3 days, as today was the "freeze" date. I'll re-include this on your request but now no more PRs please :)

@blueorangutan test

@yadvr yadvr added this to the 4.11 milestone Jan 8, 2018
@blueorangutan
Copy link

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

@wido
Copy link
Contributor Author

wido commented Jan 8, 2018

Ok, get it @rhtyd :) It was the weekend in between which made me not reply to the PR.

I don't have anything else to submit :)

@blueorangutan
Copy link

Trillian test result (tid-2076)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 31309 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr2388-t2076-kvm-centos7.zip
Intermitten failure detected: /marvin/tests/smoke/test_vpc_redundant.py
Intermitten failure detected: /marvin/tests/smoke/test_hostha_kvm.py
Smoke tests completed. 66 look OK, 1 have error(s)
Only failed tests results shown below:

Test Result Time (s) Test File
test_01_create_redundant_VPC_2tiers_4VMs_4IPs_4PF_ACL Failure 394.32 test_vpc_redundant.py

Copy link
Member

@yadvr yadvr left a comment

Choose a reason for hiding this comment

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

LGTM

@yadvr
Copy link
Member

yadvr commented Jan 9, 2018

Tests LGTM, the one failure is env related.
Merging this based on code reviews and test results.

@yadvr yadvr merged commit 35b4339 into apache:master Jan 9, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants