Uh oh!
There was an error while loading. Please reload this page.
server: Do NOT cleanup dhcp and dns when stop a vm - #3627
Conversation
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
commented
Oct 8, 2019
@rhtyd |
yadvr
commented
Oct 8, 2019
@blueorangutan package |
blueorangutan
commented
Oct 8, 2019
@rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress. |
blueorangutan
commented
Oct 8, 2019
Packaging result: ✔centos6 ✔centos7 ✔debian. JID-311 |
yadvr
commented
Oct 8, 2019
@blueorangutan test |
blueorangutan
commented
Oct 8, 2019
@rhtyd a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests |
yadvr
commented
Oct 10, 2019
@blueorangutan test |
blueorangutan
commented
Oct 10, 2019
@rhtyd a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests |
anuragaw
left a comment
There was a problem hiding this comment.
Haven't tested manually but change seems in line with PR description and added fix for removing entries only on VM expunge.
andrijapanicsb
commented
Oct 10, 2019
This absolutely requires manual testing (that also means #3608). No merging until tested thoroughly, please! |
yadvr
commented
Oct 10, 2019
We'll wait for your test confirmation @andrijapanicsb |
andrijapanicsb
commented
Oct 10, 2019
In short - NOTHING IS CLEANED at all, whatsoever... Test: Created Isolated network (R21 in examples below) Created a "vm1" (and some more VMs, but irrelevant) and joined vm1 to:
While VM running, inside R21 (Isolated) vm1 expunged, inside R21 (Isolated) Identical results are for R22 (Shared network) - zero difference in grep results (not sharing, for brevity) |
andrijapanicsb
commented
Oct 10, 2019
Additional thing I saw - when you restart network with cleanup - dhcp.leases file is completely empty (probably some more issues in other dns and dhcp related files) |
andrijapanicsb
commented
Oct 10, 2019
Tested with a VM attached just to Isolated network - same results. |
blueorangutan
commented
Oct 10, 2019
Trillian test result (tid-406)
|
ustcweizhou
commented
Oct 15, 2019
@andrijapanicsb thanks a lot for testing. I will test it and update this PR if it is tested ok. |
ustcweizhou
commented
Oct 15, 2019
@andrijapanicsb I have updated this PR and tested ok. could you please test it ?
|
yadvr
commented
Oct 15, 2019
@andrijapanicsb I did not review your test, the test should be to check the dnsmasq related config file and even maybe the leases file. |
andrijapanicsb
commented
Oct 15, 2019
Thx @ustcweizhou @rhtyd - will test is as soon as possible. |
blueorangutan
commented
Oct 15, 2019
Packaging result: ✔centos6 ✔centos7 ✔debian. JID-315 |
There was a problem hiding this comment.
All right - this seems to be good - I can confirm what @ustcweizhou shared.
I can see in logs proper the request to remove DHCP entries from the VR, later removing NICs of the VM and finally wiping the VM.
(another issue which I noticed, seems to be solved now with this PR)
Just a notice, on Network Restart with Clean UP:
- /var/lib/misc/dnsmasq.leases file is wiped (same in 4.11.2 - i.e. before issues in 4.11.3)
This means that DHCP client will get DHCPNAK on its DHCPREQUEST to extend the lease - and will later fail back to the "initialization process" i.e. start from scratch with DHCPDISCOVER - so will again get it's IP address from the /etc/dhcphosts.txt file.
Once all tests pass, I'm OK for this to be merged.
LGTM
ustcweizhou
commented
Oct 16, 2019
@andrijapanicsb that's good, thanks for testing. |
andrijapanicsb
commented
Oct 17, 2019
@blueorangutan package |
blueorangutan
commented
Oct 17, 2019
@andrijapanicsb a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress. |
blueorangutan
commented
Oct 17, 2019
Packaging result: ✔centos6 ✔centos7 ✔debian. JID-317 |
yadvr
commented
Oct 17, 2019
@ustcweizhou@andrijapanicsb - have you tested the case that when a VM is stopped, its rules are not removed from the VR? I'll kick the test |
blueorangutan
commented
Oct 17, 2019
@rhtyd a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests |
andrijapanicsb
commented
Oct 17, 2019
yes @rhtyd , tested pretty thoroughly. Also tested when VR is stopped (and VM expunge automatically kicks in, etc). Looks good to me. Was even restarting networks with and without cleanup - all good. |
blueorangutan
commented
Oct 17, 2019
Trillian test result (tid-413)
|
andrijapanicsb
commented
Nov 21, 2019
@weizhouapache @rhtyd I think I see a bit of a problem with this one (somewhat edge case). VM1 has default network NET1 Can you think of any other consequence here? |
andrijapanicsb
commented
Nov 21, 2019
So... when an additional network is detached from the VM, and we later stop/delete/expunge that VM - nothing happens, since we only clean DHCP/DNS from the VRs/networks to which the VM is still attached. Possibly a correct fix would be to ALSO do the same cleanup (that we otherwise do only when VM is expunging) when DETACHING from an ADDITIONAL network. Well, when detaching from ANY network - since after VM is detached, its reference(s) to its previous network(s) is lost and will never be later cleaned up (dhcp/dns config) when VM is destroyed Makes sense @ustcweizhou @rhtyd /CC @PaulAngus ? |
yadvr
commented
Nov 22, 2019
This PR is closed, we might lose the discussion @andrijapanicsb - can you create a ticket with your concerns? (i.e. to cleanup on vm nic remove/detach?) |
weizhouapache
commented
Nov 22, 2019
@andrijapanicsb make sense. we need to remove dhcp/dns entries in VR when remove a nic from vm. |
andrijapanicsb
commented
Nov 22, 2019
To be continued via: #3706 thx both! |
Description
According comment in PR #3608, dhcp and dns entries are cleaned up only when a VM is expunged.
Revert part of commit 8fb388e.
Types of changes
Screenshots (if appropriate):
How Has This Been Tested?