FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

server: Cleanup dhcp and dns entries only on expunging VM by yadvr · Pull Request #3608 · apache/cloudstack · GitHub

  1. apache
  2. cloudstack

Repository navigation

server: Cleanup dhcp and dns entries only on expunging VM - #3608

Merged
yadvr merged 1 commit into
apache:4.13from
shapeblue:vr-cleanup-vm-expunge
Sep 26, 2019
ConversationCommits (1)ChecksFiles changed
Merged

server: Cleanup dhcp and dns entries only on expunging VM#3608
yadvr merged 1 commit into
apache:4.13from
shapeblue:vr-cleanup-vm-expunge

Conversation

yadvr commented Sep 25, 2019

Copy link
Copy Markdown
Member

This fixes a behaviour to not cleanup DHCP and DNS rules for NICs of a
VM in the VR when it is stopped, but instead when VM is expunged because
stopped VMs in CloudStack still retain the IPs and records.

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)

server: Cleanup dhcp and dns entries only on expunging VM
4eca1e4 
This fixes a behaviour to not cleanup DHCP and DNS rules for NICs of a
VM in the VR when it is stopped, but instead when VM is expunged because
stopped VMs in CloudStack still retain the IPs and records.

Signed-off-by: Rohit Yadav <rohit.yadav@shapeblue.com>
yadvr added this to the 4.13.1.0 milestone Sep 25, 2019

yadvr commented Sep 25, 2019

Copy link
Copy Markdown
Member Author

@blueorangutan package

blueorangutan commented Sep 25, 2019

Copy link
Copy Markdown

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

blueorangutan commented Sep 25, 2019

Copy link
Copy Markdown

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

yadvr commented Sep 25, 2019

Copy link
Copy Markdown
Member Author

@blueorangutan test

blueorangutan commented Sep 25, 2019

Copy link
Copy Markdown

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

anuragaw approved these changes Sep 26, 2019
View reviewed changes

anuragaw left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

LGTM from code.

shwstppr approved these changes Sep 26, 2019
View reviewed changes

shwstppr left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

LGTM on changes!
Although dhcp cleanup on VM stop seems more logical to me.

yadvr requested review from nvazquez and removed request for nvazquez September 26, 2019 11:33
nvazquez approved these changes Sep 26, 2019
View reviewed changes

nvazquez left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

LGTM, this makes cleanup introduced in this PR: #3351 to be performed only on VM expunge

wido approved these changes Sep 26, 2019
View reviewed changes

wido left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

LGTM based on code

yadvr merged commit b853ebd into apache:4.13 Sep 26, 2019

yadvr commented Oct 2, 2019

Copy link
Copy Markdown
Member Author

FYI - this partially reverts and refactors changes introduced in #3351 to cleanup VM dhcp/dns rules in the VR only when a VM is expunged. A VM hostname and IP address is used as constraint and checks even of a VM is stopped, i.e. you cannot deploy a VM with the same hostname and IP in a network with an existing VM with those attributes. This was tested against a local monkeybox KVM env and regression tested agains Trillian/kvm.

ustcweizhou added a commit to ustcweizhou/cloudstack that referenced this pull request Oct 8, 2019
server: Do NOT cleanup dhcp and dns when stop a vm
cecb713 
According comment in PR apache#3608, dhcp and dns entries are cleaned up only when a VM is expunged.
Revert part of commit 8fb388e.
ustcweizhou mentioned this pull request Oct 8, 2019
Merged
1 of 5 tasks
nvazquez added a commit to shapeblue/cloudstack that referenced this pull request Oct 10, 2019
Backport apache#3608: server: Cleanup dhcp and dns entries only on ex…
cae3d23 
…punging VM
yadvr pushed a commit that referenced this pull request Oct 17, 2019
server: Do NOT cleanup dhcp and dns when stop a vm (#3627)
cc27095 
* server: Do NOT cleanup dhcp and dns when stop a vm

According comment in PR #3608, dhcp and dns entries are cleaned up only when a VM is expunged.
Revert part of commit 8fb388e.

* server: cleanup dns/dhcp entries in removeNic instead of finalizeExpunge
This was referenced Oct 29, 2019
Closed
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

shwstppr shwstppr approved these changes
wido wido approved these changes
nvazquez nvazquez approved these changes
borisstoyanov Awaiting requested review from borisstoyanov
GabrielBrascher Awaiting requested review from GabrielBrascher
+1 more reviewer
anuragaw anuragaw approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

component:compute component:virtual-router

Projects

None yet

Milestone

4.13.1.0

Development

Successfully merging this pull request may close these issues.

6 participants

Back | FazBrowse Home | New Git URL