vpc: fix ips on wrong interfaces after rebooting vpc vrs by ustcweizhou · Pull Request #4467 · apache/cloudstack

ustcweizhou · 2020-11-12T10:17:30Z

Description

When vpc tiers are added/removed and public ips in different range is added/removed, the nics in VPC VRs are added/removed, it leads to inconsecutive device in VPC VRs.
When we reboot the VR, (or VR is rebooted by some other reasons like HA), the ips in VPC VRs might be assigned to wrong interfaces.

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?

sureshanaparti · 2020-11-12T10:57:59Z

+ nic.setDeviceId(deviceId);
+ _nicDao.update(nic.getId(), nic);
+ return nic;
+ }


method name is not clear, Either define & use separate methods (for "updating nic device id" and "getting nic with device id"), or change the name appropriately.

getNicAndUpdtaeDeviceId() might have been more accurate but this is clearly stating what it does on the tin (to me) , Can you be more explicit about what you want @sureshanaparti ?

getNicAndUpdtaeDeviceId() might have been more accurate but this is clearly stating what it does on the tin (to me) , Can you be more explicit about what you want @sureshanaparti ?
simply "updateNicWithDeviceId()" returning nic would be ok.

@sureshanapartiupdateNicWithDeviceId() looks good to me. @DaanHoogland what's your opinion ?

yes, fine. the only problem i have is with my own spelling; getNicAndUpdtaeDeviceId() misses a 'k' ;)

@sureshanaparti @DaanHoogland thanks guys. code is changed.

DaanHoogland · 2020-11-13T13:19:41Z

@blueorangutan package

blueorangutan · 2020-11-13T13:20:32Z

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

blueorangutan · 2020-11-13T13:47:40Z

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

DaanHoogland · 2020-11-13T13:48:27Z

@blueorangutan test

blueorangutan · 2020-11-13T13:49:30Z

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

blueorangutan · 2020-11-13T23:08:44Z

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

Test	Result	Time (s)	Test File

weizhouapache · 2020-11-15T16:16:52Z

@DaanHoogland please do not merge this pr. I found an issue which might be caused by this pr.

DaanHoogland · 2020-11-17T18:51:22Z

@weizhouapache converted to draft so it is clear 4.15 is not waiting for it. please change back if you disagree

…unning or configuration has changed

weizhouapache · 2020-11-17T20:32:26Z

@DaanHoogland fixed a bug by commit f603816

in description of commit 7988f51, it says that "Force a restart of keepalived if conntrackd is not running or configuration has changed". however, in code, it is "and" not "or"

DaanHoogland · 2020-11-18T07:14:56Z

@weizhouapache can you add soem test data/results?

yadvr · 2020-11-18T08:51:03Z

@blueorangutan package

blueorangutan · 2020-11-18T08:51:56Z

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

sureshanaparti

changes LGTM, haven't tested.

blueorangutan · 2020-11-18T09:31:32Z

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

DaanHoogland · 2020-11-18T11:37:15Z

@blueorangutan package

blueorangutan · 2020-11-18T11:37:51Z

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

blueorangutan · 2020-11-18T12:37:49Z

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

DaanHoogland · 2020-11-18T12:59:16Z

@blueorangutan test

blueorangutan · 2020-11-18T12:59:57Z

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

blueorangutan · 2020-11-18T21:44:00Z

Trillian test result (tid-3197)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 29550 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4467-t3197-kvm-centos7.zip
Smoke tests completed. 83 look OK, 0 have error(s)
Only failed tests results shown below:

Test	Result	Time (s)	Test File

yadvr · 2020-11-18T23:25:10Z

Needs a round of test against vmware
@blueorangutan test centos7 vmware-67u3

blueorangutan · 2020-11-18T23:25:54Z

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

blueorangutan · 2020-11-19T20:41:13Z

Trillian test result (tid-3207)
Environment: vmware-67u3 (x2), Advanced Networking with Mgmt server 7
Total time taken: 37926 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4467-t3207-vmware-67u3.zip
Intermittent failure detected: /marvin/tests/smoke/test_privategw_acl.py
Intermittent failure detected: /marvin/tests/smoke/test_templates.py
Smoke tests completed. 82 look OK, 1 have error(s)
Only failed tests results shown below:

Test	Result	Time (s)	Test File
test_02_create_template_with_checksum_sha1	`Error`	5.22	test_templates.py
test_03_create_template_with_checksum_sha256	`Error`	5.22	test_templates.py
test_04_create_template_with_checksum_md5	`Error`	5.23	test_templates.py

DaanHoogland · 2020-11-20T10:44:11Z

I do not see how these template tests (all pertaining to checksums are related. We may have uncovered an unrelated issue thought it does not happen in other vmware tests. @rhtyd , you want to make the call?

yadvr · 2020-11-20T15:32:47Z

Agree, seems like env related

vpc: fix ips on wrong interfaces after rebooting vpc vrs
522f4eb

sureshanaparti reviewed Nov 12, 2020
View reviewed changes

DaanHoogland added type:bug Severity:Major labels Nov 12, 2020

DaanHoogland added this to the 4.14.1.0 milestone Nov 12, 2020

apache#4467: Rename to updateNicWithDeviceId
eba9c86

DaanHoogland approved these changes Nov 14, 2020
View reviewed changes

DaanHoogland self-requested a review November 16, 2020 07:48

DaanHoogland marked this pull request as draft November 17, 2020 18:49

CLSTACK-8923 vr: Force a restart of keepalived if conntrackd is not r…
f603816
…unning or configuration has changed

weizhouapache marked this pull request as ready for review November 17, 2020 20:30

DaanHoogland approved these changes Nov 18, 2020
View reviewed changes

DaanHoogland requested a review from sureshanaparti November 18, 2020 07:13

sureshanaparti approved these changes Nov 18, 2020
View reviewed changes

yadvr approved these changes Nov 18, 2020
View reviewed changes

ustcweizhou mentioned this pull request Nov 19, 2020
VPC: fix some issues related to multiple public IP ranges and private gateway #4484
Merged
10 tasks

yadvr merged commit 75fdb07 into apache:4.14Nov 20, 2020

Uh oh!

Conversation

ustcweizhou commented Nov 12, 2020

Description

Types of changes

Screenshots (if appropriate):

How Has This Been Tested?

Uh oh!

sureshanapartiNov 12, 2020

Choose a reason for hiding this comment

Uh oh!

DaanHooglandNov 12, 2020

Choose a reason for hiding this comment

Uh oh!

sureshanapartiNov 13, 2020

Choose a reason for hiding this comment

Uh oh!

weizhouapacheNov 13, 2020

Choose a reason for hiding this comment

Uh oh!

DaanHooglandNov 13, 2020

Choose a reason for hiding this comment

Uh oh!

weizhouapacheNov 13, 2020

Choose a reason for hiding this comment

Uh oh!

DaanHoogland commented Nov 13, 2020

Uh oh!

blueorangutan commented Nov 13, 2020

Uh oh!

blueorangutan commented Nov 13, 2020

Uh oh!

DaanHoogland commented Nov 13, 2020

Uh oh!

blueorangutan commented Nov 13, 2020

Uh oh!

blueorangutan commented Nov 13, 2020

Uh oh!

weizhouapache commented Nov 15, 2020

Uh oh!

DaanHoogland commented Nov 17, 2020

Uh oh!

weizhouapache commented Nov 17, 2020

Uh oh!

DaanHoogland commented Nov 18, 2020

Uh oh!

yadvr commented Nov 18, 2020

Uh oh!

blueorangutan commented Nov 18, 2020

Uh oh!

sureshanaparti left a comment

Choose a reason for hiding this comment

Uh oh!

blueorangutan commented Nov 18, 2020

Uh oh!

DaanHoogland commented Nov 18, 2020

Uh oh!

blueorangutan commented Nov 18, 2020

Uh oh!

blueorangutan commented Nov 18, 2020

Uh oh!

DaanHoogland commented Nov 18, 2020

Uh oh!

blueorangutan commented Nov 18, 2020

Uh oh!

blueorangutan commented Nov 18, 2020

Uh oh!

yadvr commented Nov 18, 2020

Uh oh!

blueorangutan commented Nov 18, 2020

Uh oh!

blueorangutan commented Nov 19, 2020

Uh oh!

DaanHoogland commented Nov 20, 2020

Uh oh!

yadvr commented Nov 20, 2020

Uh oh!

Reviewers

Assignees

Labels