Skip to content

fixed inconsistency of IP on VR when VR is destroyed and recrea…#3825

Merged
DaanHoogland merged 3 commits into
apache:4.13from
shapeblue:CS-1342
Jan 23, 2020
Merged

fixed inconsistency of IP on VR when VR is destroyed and recrea…#3825
DaanHoogland merged 3 commits into
apache:4.13from
shapeblue:CS-1342

Conversation

@Pearl1594
Copy link
Copy Markdown
Contributor

@Pearl1594 Pearl1594 commented Jan 21, 2020

Description

This address the issue mentioned in #3787 - Guest IP address of Shared Network VR is not persistent when VR is destroyed/recreated

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)

How Has This Been Tested?

  • Created a shared network
  • Created a VM in the shared network
  • destroyed the Virtual router
  • stopped and restarted the instance
  • the VR created will have the same Guest IP as it had previously

@yadvr yadvr added this to the 4.13.1.0 milestone Jan 21, 2020
@yadvr
Copy link
Copy Markdown
Member

yadvr commented Jan 21, 2020

Regression testing and manual QA against VPC, shared and isolated network with/without redundant router. Also we need to check if L2 is affected (likely not, as there are no routers).

DaanHoogland
DaanHoogland reviewed Jan 21, 2020
View reviewed changes
Copy link
Copy Markdown
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.

one remark (code itself looks good)

Comment thread server/src/main/java/com/cloud/network/NetworkModelImpl.java
Comment on lines 2301 to +2304
//return nic only when its ip address belong to the pod range (for the Basic zone case)
if (vlans.isEmpty()) {
return nic;
}
Copy link
Copy Markdown
Contributor

@DaanHoogland DaanHoogland Jan 21, 2020

Choose a reason for hiding this comment

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

I don't think this comment still describves the line below. Can you clarify or remove it?

Copy link
Copy Markdown
Contributor Author

@Pearl1594 Pearl1594 Jan 21, 2020

Choose a reason for hiding this comment

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

@DaanHoogland I've moved the comment

@apache apache deleted a comment from blueorangutan Jan 21, 2020
@apache apache deleted a comment from blueorangutan Jan 21, 2020
@apache apache deleted a comment from blueorangutan Jan 22, 2020
@DaanHoogland
Copy link
Copy Markdown
Contributor

DaanHoogland commented Jan 22, 2020

@blueorangutan package

@apache apache deleted a comment from blueorangutan Jan 22, 2020
@blueorangutan
Copy link
Copy Markdown

blueorangutan commented Jan 22, 2020

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

@blueorangutan
Copy link
Copy Markdown

blueorangutan commented Jan 22, 2020

Packaging result: ✖centos6 ✖centos7 ✖debian. JID-653

@DaanHoogland
Copy link
Copy Markdown
Contributor

DaanHoogland commented Jan 22, 2020

@Pearl1594 there is a checkstyle error. Can you check?

@Pearl1594
Copy link
Copy Markdown
Contributor Author

Pearl1594 commented Jan 22, 2020

@DaanHoogland Handled it

@DaanHoogland
Copy link
Copy Markdown
Contributor

DaanHoogland commented Jan 23, 2020

@blueorangutan package

@blueorangutan
Copy link
Copy Markdown

blueorangutan commented Jan 23, 2020

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

@blueorangutan
Copy link
Copy Markdown

blueorangutan commented Jan 23, 2020

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

@DaanHoogland
Copy link
Copy Markdown
Contributor

DaanHoogland commented Jan 23, 2020

@blueorangutan test

@blueorangutan
Copy link
Copy Markdown

blueorangutan commented Jan 23, 2020

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

weizhouapache
weizhouapache approved these changes Jan 23, 2020
View reviewed changes
Copy link
Copy Markdown
Member

@weizhouapache weizhouapache 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

@DaanHoogland DaanHoogland changed the title fixed inconsistency of IP on VR when VR is destroyed and recreated (shared network) fixed inconsistency of IP on VR when VR is destroyed and recrea… Jan 23, 2020
@DaanHoogland DaanHoogland merged commit b581f85 into apache:4.13 Jan 23, 2020
@DaanHoogland DaanHoogland deleted the CS-1342 branch January 23, 2020 15:59
DaanHoogland added a commit that referenced this pull request Jan 23, 2020
@DaanHoogland
Merge release branch 4.13 to master
f276485 
* 4.13:
  fixed inconsistency of IP on VR when VR is destroyed and recrea… (#3825)
  server: fix resource count error when upgrade a vm (#3759)
@blueorangutan
Copy link
Copy Markdown

blueorangutan commented Jan 23, 2020

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

Test Result Time (s) Test File

ustcweizhou pushed a commit to ustcweizhou/cloudstack that referenced this pull request Feb 28, 2020
@Pearl1594 @ustcweizhou
fixed inconsistency of IP on VR when VR is destroyed and recrea… (apa…
522a249 
…che#3825)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@DaanHoogland DaanHoogland DaanHoogland left review comments

@yadvr yadvr yadvr approved these changes

@weizhouapache weizhouapache weizhouapache approved these changes

@andrijapanicsb andrijapanicsb Awaiting requested review from andrijapanicsb

Assignees

No one assigned

Labels

component:virtual-router type:bug

Projects

None yet

Milestone

4.13.1.0

Development

Successfully merging this pull request may close these issues.

6 participants

@Pearl1594 @yadvr @DaanHoogland @blueorangutan @weizhouapache @svenvogel