Have persistent DHCP leases file on VRs and cleanup /etc/hosts on VM deletion #3351

nvazquez · 2019-05-24T00:44:56Z

Description

Since the CloudStack virtual router was redesigned on version 4.6 it has been observed that the DHCP leases file is not persistent across network operations. This causes conflicts on guest VMs static IPs, causing these static IPs to not be renewed by the DHCP server running on isolated and VPC networks' virtual routers (dnsmasq). On stopping or destroying a VM, its dhcp/dns records are not removed from the virtual router causing ghost effects.

Fixes #3272
Fixes #3354

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?

Tested on local environment, 1x KVM host, Isolated network

Create VMs on isolated network
Restart network

rohityadavcloud · 2019-05-24T07:00:53Z

@blueorangutan package

blueorangutan · 2019-05-24T07:02:05Z

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

blueorangutan · 2019-05-24T08:02:16Z

Packaging result: ✖centos6 ✔centos7 ✔debian. JID-2782

DaanHoogland · 2019-05-24T11:49:34Z

@blueorangutan package

blueorangutan · 2019-05-24T11:50:03Z

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

blueorangutan · 2019-05-24T12:10:36Z

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

DaanHoogland · 2019-05-24T12:49:36Z

@blueorangutan test

blueorangutan · 2019-05-24T12:50:03Z

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

blueorangutan · 2019-05-24T21:49:03Z

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

Test	Result	Time (s)	Test File

rohityadavcloud · 2019-05-27T12:13:13Z

@blueorangutan test matrix

blueorangutan · 2019-05-27T12:13:16Z

@rhtyd a Trillian-Jenkins matrix job (centos6 mgmt + xs71, centos7 mgmt + vmware65, centos7 mgmt + kvmcentos7) has been kicked to run smoke tests

blueorangutan · 2019-05-27T22:16:15Z

Trillian test result (tid-3587)
Environment: xenserver-71 (x2), Advanced Networking with Mgmt server 7
Total time taken: 34342 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr3351-t3587-xenserver-71.zip
Intermittent failure detected: /marvin/tests/smoke/test_scale_vm.py
Intermittent failure detected: /marvin/tests/smoke/test_volumes.py
Intermittent failure detected: /marvin/tests/smoke/test_vpc_redundant.py
Intermittent failure detected: /marvin/tests/smoke/test_vpc_router_nics.py
Smoke tests completed. 66 look OK, 2 have error(s)
Only failed tests results shown below:

Test	Result	Time (s)	Test File
test_01_scale_vm	`Error`	17.43	test_scale_vm.py
test_01_create_redundant_VPC_2tiers_4VMs_4IPs_4PF_ACL	`Failure`	562.10	test_vpc_redundant.py
test_05_rvpc_multi_tiers	`Failure`	528.05	test_vpc_redundant.py
test_05_rvpc_multi_tiers	`Error`	567.24	test_vpc_redundant.py

blueorangutan · 2019-05-27T23:50:30Z

Trillian test result (tid-3589)
Environment: vmware-65u2 (x2), Advanced Networking with Mgmt server 7
Total time taken: 40124 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr3351-t3589-vmware-65u2.zip
Intermittent failure detected: /marvin/tests/smoke/test_public_ip_range.py
Intermittent failure detected: /marvin/tests/smoke/test_templates.py
Intermittent failure detected: /marvin/tests/smoke/test_usage.py
Intermittent failure detected: /marvin/tests/smoke/test_volumes.py
Smoke tests completed. 65 look OK, 3 have error(s)
Only failed tests results shown below:

Test	Result	Time (s)	Test File
test_04_extract_template	`Failure`	136.54	test_templates.py
ContextSuite context=TestISOUsage>:setup	`Error`	0.00	test_usage.py
test_06_download_detached_volume	`Failure`	167.45	test_volumes.py

onitake · 2019-05-28T16:18:20Z

Is the fix implemented by this PR ready to be tested?
The WIP tag is slightly confusing...
I'd like to test it on one of our affected networks, if possible.

nvazquez · 2019-05-28T16:40:15Z

Hi @onitake the WIP tag stands for 'Work in progress' which means this is still not ready. Having said that, you can test it now but you may expect new changes

gildegoma · 2019-05-29T08:43:25Z

systemvm/debian/opt/cloud/bin/cs/CsDhcp.py

+                mac = lease[1]
+                ip = lease[2]
+                if mac not in macs_dhcphosts:
+                    cmd = "dhcp_release %s %s" % (ip, mac)


@nvazquez the dhcp_release <interface> parameter is missing here

See https://manpages.debian.org/stretch/dnsmasq-utils/dhcp_release.1.en.html

Thanks I'll fix this

Fixed @gildegoma @nvazquez

rohityadavcloud · 2019-05-30T06:36:26Z

@nvazquez can you address review comments and also advise if this PR is ready for review and testing?

rohityadavcloud · 2019-05-30T06:38:22Z

@nvazquez while you're on it, can you also look at another similar issue related to VRs - #3273?

rohityadavcloud · 2019-05-30T09:09:15Z

@nvazquez can you also fix pylint error:

************* Module cleanuphosts
E: 75,78: Instance of 'Exception' has no 'strerror' member (no-member)
-----------------------------------
Your code has been rated at 9.99/10
+'[' 2 -gt 0 ']'
+echo 'Pylint failed, please check your code'
Pylint failed, please check your code

Signed-off-by: Rohit Yadav <rohit.yadav@shapeblue.com>

rohityadavcloud · 2019-05-31T10:47:16Z

@nvazquez fixes for #3272 and #3354 LGTM, but I found a weird issue:

Deploy two small VMs
Destroy one of the small VMs, verify that their entries are not seen in /etc/hosts and /etc/dhcphosts.txt
Deploy another new small VM
Verify that the new small VM's entry exists, but not the delete VM's entry suddenly show up as well (possibly something to do with CsDhcp.py and databags, we probably need to either fix in /etc/cloudstack/dhcpentry.json in the VR via the cleanup script, or see DhcpEntryCommand, command setup helper:: createDhcpEntryCommand)

Please continue with the investigation and fix for the above edge-case and also fix for #3273

nvazquez · 2019-05-31T21:16:05Z

@rhtyd changed the implementation, no need for a new script. Marking entries as 'remove=True' on dhcpentry.json solves the issue. This solves #3272 and #3354.
Fix for #3273 is the only remaining

rohityadavcloud · 2019-06-01T00:43:34Z

@blueorangutan package

blueorangutan · 2019-06-01T00:44:34Z

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

blueorangutan · 2019-06-01T01:09:59Z

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

rohityadavcloud · 2019-06-01T01:16:40Z

@blueorangutan test

blueorangutan · 2019-06-01T01:17:40Z

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

blueorangutan · 2019-06-01T10:49:47Z

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

Test	Result	Time (s)	Test File
test_05_rvpc_multi_tiers	`Failure`	407.69	test_vpc_redundant.py
test_05_rvpc_multi_tiers	`Error`	433.89	test_vpc_redundant.py

rohityadavcloud

Tested Nicolas's latest changes, LGTM for both the bugs. The third one is not started yet.

rohityadavcloud · 2019-06-03T11:05:38Z

Let's do #3273 in a separate PR.

anuragaw

LGTM - Based on code read and test results

server24 · 2019-07-17T15:48:03Z

@nvazquez @rhtyid this breaks Basic Networking (#3497) whenever a VM is stopped or expunged.

rohityadavcloud · 2019-07-17T16:05:52Z

Can you check and possibly send a PR @nvazquez ? Thanks.

nvazquez · 2019-07-17T21:31:11Z

Sure I'll take a look tonight

GabrielBrascher · 2019-10-28T15:02:32Z

@nvazquez has this PR been tested on a shared guest network within a Basic Network Zone? I am facing a null pointer when trying to remove the DHCP entry rule from a VM that is being stopped.

ACS version: 4.11.3

java.lang.NullPointerException
        at org.apache.cloudstack.network.topology.BasicNetworkVisitor.visit(BasicNetworkVisitor.java:201)
        at com.cloud.network.rules.DhcpEntryRules.accept(DhcpEntryRules.java:64)
        at org.apache.cloudstack.network.topology.BasicNetworkTopology.applyRules(BasicNetworkTopology.java:390)
        at org.apache.cloudstack.network.topology.BasicNetworkTopology.removeDhcpEntry(BasicNetworkTopology.java:464)
        at com.cloud.network.element.VirtualRouterElement.removeDhcpEntry(VirtualRouterElement.java:972)
        at org.apache.cloudstack.engine.orchestration.NetworkOrchestrator.cleanupNicDhcpDnsEntry(NetworkOrchestrator.java:2933)

GabrielBrascher · 2019-10-28T15:22:21Z

Please ignore the message above, just saw the PR #3501 fixing it.

Thanks for the PR @nvazquez.

nvazquez changed the title ~~Fix not persistent DHCP leases file on VRs~~ [WIP: DO NOT MERGE] Fix not persistent DHCP leases file on VRs May 24, 2019

svenvogel added type:bug component:dhcp component:virtual-router labels May 26, 2019

rohityadavcloud approved these changes May 27, 2019

View reviewed changes

rohityadavcloud added this to the 4.11.3.0 milestone May 27, 2019

gildegoma reviewed May 29, 2019

View reviewed changes

rohityadavcloud assigned nvazquez May 30, 2019

rohityadavcloud changed the title ~~[WIP: DO NOT MERGE] Fix not persistent DHCP leases file on VRs~~ [WIP: DO NOT MERGE] Have persistent DHCP leases file on VRs and cleanup /etc/hosts on VM deletion May 30, 2019

nvazquez added 3 commits May 31, 2019 13:10

Fix not persistent DHCP leases file on VRs

323fbc5

Delete entry on dhcphosts on VM stop

63bd5b6

Remove DNS host entry

4feb000

rohityadavcloud added 3 commits May 31, 2019 15:24

simplify cleanup implementation

ce7d0a7

Signed-off-by: Rohit Yadav <rohit.yadav@shapeblue.com>

implement fcntl/flock based locking on cleanup

88dd7fe

Signed-off-by: Rohit Yadav <rohit.yadav@shapeblue.com>

fix bug to write lines using the list object

e3569be

Signed-off-by: Rohit Yadav <rohit.yadav@shapeblue.com>

Optimize and fix entries being added after deletion

ef08154

rohityadavcloud approved these changes Jun 3, 2019

View reviewed changes

rohityadavcloud changed the title ~~[WIP: DO NOT MERGE] Have persistent DHCP leases file on VRs and cleanup /etc/hosts on VM deletion~~ Have persistent DHCP leases file on VRs and cleanup /etc/hosts on VM deletion Jun 3, 2019

fix error logging

9d8cf1e

anuragaw approved these changes Jun 3, 2019

View reviewed changes

shwstppr approved these changes Jun 3, 2019

View reviewed changes

rohityadavcloud merged commit c9ce3e2 into apache:4.11 Jun 3, 2019

richardlawley mentioned this pull request Jun 19, 2019

RvR Services should be provided on VIP instead of VR IP #3409

Closed

div8cn mentioned this pull request Aug 12, 2019

VR Loses Instance IP Address #3550

Closed

rohityadavcloud mentioned this pull request Oct 2, 2019

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

Merged

5 tasks

nvazquez deleted the dhcp411 branch April 6, 2020 14:41

weizhouapache mentioned this pull request Aug 19, 2021

vr: reload dnsmasq when start vms #5319

Merged

12 tasks

Have persistent DHCP leases file on VRs and cleanup /etc/hosts on VM deletion #3351

Have persistent DHCP leases file on VRs and cleanup /etc/hosts on VM deletion #3351

Conversation

nvazquez commented May 24, 2019 • edited by rohityadavcloud

Description

Types of changes

Screenshots (if appropriate):

How Has This Been Tested?

rohityadavcloud commented May 24, 2019

blueorangutan commented May 24, 2019

blueorangutan commented May 24, 2019

DaanHoogland commented May 24, 2019

blueorangutan commented May 24, 2019

blueorangutan commented May 24, 2019

DaanHoogland commented May 24, 2019

blueorangutan commented May 24, 2019

blueorangutan commented May 24, 2019

rohityadavcloud commented May 27, 2019

blueorangutan commented May 27, 2019

blueorangutan commented May 27, 2019

blueorangutan commented May 27, 2019

onitake commented May 28, 2019

nvazquez commented May 28, 2019

gildegoma May 29, 2019

Choose a reason for hiding this comment

nvazquez May 31, 2019

Choose a reason for hiding this comment

rohityadavcloud May 31, 2019

Choose a reason for hiding this comment

rohityadavcloud commented May 30, 2019

rohityadavcloud commented May 30, 2019

rohityadavcloud commented May 30, 2019

rohityadavcloud commented May 31, 2019 • edited

nvazquez commented May 31, 2019 • edited

rohityadavcloud commented Jun 1, 2019

blueorangutan commented Jun 1, 2019

blueorangutan commented Jun 1, 2019

rohityadavcloud commented Jun 1, 2019

blueorangutan commented Jun 1, 2019

blueorangutan commented Jun 1, 2019

rohityadavcloud left a comment

Choose a reason for hiding this comment

rohityadavcloud commented Jun 3, 2019

anuragaw left a comment

Choose a reason for hiding this comment

server24 commented Jul 17, 2019

rohityadavcloud commented Jul 17, 2019

nvazquez commented Jul 17, 2019

GabrielBrascher commented Oct 28, 2019

GabrielBrascher commented Oct 28, 2019

nvazquez commented May 24, 2019 •

edited by rohityadavcloud

rohityadavcloud commented May 31, 2019 •

edited

nvazquez commented May 31, 2019 •

edited