vr: Ensuring dnsmasq.leases file is populated by davidjumani · Pull Request #4529 · apache/cloudstack · GitHub

Description

When a VR is deleted and a new one brought up it populates the dhcphosts.txt with all hosts in the network, but the dnsmasq.leases file is blank which can cause issues if VMs try to renew their lease.
This PR also adds an entry in the dnsmasq.leases file for each host when the dhcphosts.txt file is populated
An existing lease when a VM tries to acquire a new one doesn't seem to cause any issues

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?

Create a network with a couple of VMs on them
Restart the network with cleanup
Wait for the new router to come up
Cat /var/log/dnsmasq.log to verify that no DHCP request has been made
Check out /var/lib/misc/dnsmasq.leases

/var/log/dnsmasq.log - No DHCP requests received

root@r-8-VM:~# cat /var/log/dnsmasq.log
Dec 10 13:01:44 dnsmasq[928]: started, version 2.80 cachesize 150
Dec 10 13:01:44 dnsmasq[928]: compile time options: IPv6 GNU-getopt DBus i18n IDN DHCP DHCPv6 no-Lua TFTP conntrack ipset auth DNSSEC loop-detect inotify dumpfile
Dec 10 13:01:44 dnsmasq-dhcp[928]: DHCP, static leases only on 10.1.1.1, lease time 1h
Dec 10 13:01:44 dnsmasq-tftp[928]: TFTP root is /opt/tftpboot 
Dec 10 13:01:44 dnsmasq[928]: using local addresses only for domain cs2cloud.internal
Dec 10 13:01:44 dnsmasq[928]: reading /etc/dnsmasq-resolv.conf
Dec 10 13:01:44 dnsmasq[928]: using local addresses only for domain cs2cloud.internal
Dec 10 13:01:44 dnsmasq[928]: using nameserver 10.2.0.50#53
Dec 10 13:01:44 dnsmasq[928]: using nameserver 8.8.8.8#53
Dec 10 13:01:44 dnsmasq[928]: read /etc/hosts - 6 addresses
Dec 10 13:01:44 dnsmasq[928]: cannot read /etc/dhcphosts.txt: No such file or directory
Dec 10 13:01:44 dnsmasq-dhcp[928]: read /etc/dhcpopts.txt
Dec 10 13:01:44 dnsmasq[928]: read /etc/hosts - 6 addresses
Dec 10 13:01:44 dnsmasq[928]: cannot read /etc/dhcphosts.txt: No such file or directory
Dec 10 13:01:44 dnsmasq-dhcp[928]: read /etc/dhcpopts.txt
Dec 10 13:01:50 dnsmasq[928]: exiting on receipt of SIGTERM
Dec 10 13:01:50 dnsmasq[1841]: started, version 2.80 cachesize 150
Dec 10 13:01:50 dnsmasq[1841]: compile time options: IPv6 GNU-getopt DBus i18n IDN DHCP DHCPv6 no-Lua TFTP conntrack ipset auth DNSSEC loop-detect inotify dumpfile
Dec 10 13:01:50 dnsmasq-dhcp[1841]: DHCP, static leases only on 10.1.1.1, lease time 1h
Dec 10 13:01:50 dnsmasq-dhcp[1841]: DHCP, static leases only on 10.1.1.1, lease time 1h
Dec 10 13:01:50 dnsmasq-tftp[1841]: TFTP root is /opt/tftpboot 
Dec 10 13:01:50 dnsmasq[1841]: using local addresses only for domain cs2cloud.internal
Dec 10 13:01:50 dnsmasq[1841]: reading /etc/dnsmasq-resolv.conf
Dec 10 13:01:50 dnsmasq[1841]: using local addresses only for domain cs2cloud.internal
Dec 10 13:01:50 dnsmasq[1841]: using nameserver 10.2.0.50#53
Dec 10 13:01:50 dnsmasq[1841]: using nameserver 8.8.8.8#53
Dec 10 13:01:50 dnsmasq[1841]: read /etc/hosts - 8 addresses
Dec 10 13:01:50 dnsmasq-dhcp[1841]: read /etc/dhcphosts.txt
Dec 10 13:01:50 dnsmasq-dhcp[1841]: read /etc/dhcpopts.txt
Dec 10 13:01:56 dnsmasq[1841]: read /etc/hosts - 8 addresses
Dec 10 13:01:56 dnsmasq-dhcp[1841]: read /etc/dhcphosts.txt
Dec 10 13:01:56 dnsmasq-dhcp[1841]: read /etc/dhcpopts.txt

/var/lib/misc/dnsmasq.leases - They exist ✨

root@r-8-VM:~# cat /var/lib/misc/dnsmasq.leases 
0 02:00:62:58:00:04 10.1.1.52 vm-2 *
0 02:00:50:85:00:01 10.1.1.224 VM-1 *

BEFORE :

Logs when rebooting a VM after network restart with cleanup

Dec 10 13:12:45 dnsmasq[2240]: read /etc/hosts - 9 addresses
Dec 10 13:12:45 dnsmasq-dhcp[2240]: read /etc/dhcphosts.txt
Dec 10 13:12:45 dnsmasq-dhcp[2240]: read /etc/dhcpopts.txt

Dec 10 13:13:48 dnsmasq-dhcp[2240]: DHCPDISCOVER(eth0) 10.1.1.24 02:00:24:5c:00:03 
Dec 10 13:13:48 dnsmasq-dhcp[2240]: DHCPOFFER(eth0) 10.1.1.24 02:00:24:5c:00:03 
Dec 10 13:13:48 dnsmasq-dhcp[2240]: DHCPREQUEST(eth0) 10.1.1.24 02:00:24:5c:00:03 
Dec 10 13:13:48 dnsmasq-dhcp[2240]: DHCPACK(eth0) 10.1.1.24 02:00:24:5c:00:03 VM-2

AFTER :

Logs when rebooting a VM after network restart with cleanup

Dec 10 13:06:19 dnsmasq[1841]: read /etc/hosts - 8 addresses
Dec 10 13:06:19 dnsmasq-dhcp[1841]: read /etc/dhcphosts.txt
Dec 10 13:06:19 dnsmasq-dhcp[1841]: read /etc/dhcpopts.txt

Dec 10 13:07:03 dnsmasq-dhcp[1841]: DHCPREQUEST(eth0) 10.1.1.52 02:00:62:58:00:04 
Dec 10 13:07:03 dnsmasq-dhcp[1841]: DHCPACK(eth0) 10.1.1.52 02:00:62:58:00:04 vm-2

@blueorangutan package

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

Packaging result: ✔centos7 ✔centos8 ✔debian. JID-2476

@blueorangutan test

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

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

Test	Result	Time (s)	Test File

LGTM on high level but did not test it.

same as above - can you check if the lease should have a high expiry (80yrs or inf?) instead of *, I'm not sure what 0 and * mean here

0 is infinite lease time (generally the epoch when the lease expires), * is the client id (which is generally the mac, but can change)
Taken this format from existing leases file
https://dnsmasq-discuss.thekelleys.org.narkive.com/ia9YNLmE/dnsmasq-leases-file-format-specification

@davidjumani can you change base branch to 4.14 ?

reproduced the issue in 4.14
verified this pr fixes the issue.

@blueorangutan package

@davidjumani you may need to create a new fork from 4.14, instead of master.

tested ok
deploy vm, stop vm, start vm, restart network with cleanup, expunge vm.

@weizhouapache Rebased against 4.14

@rhtyd @davidjumani
the only issue I see is, dnsmasq will be restarted each time when conf/host/dhcp lease is changed.
if there are many vms in the network, might it be a problem ?
it is not caused by this pr , but the line above

        if self.cloud.commit():
            restart_dnsmasq = True

might it be better to restart dnsmasq when all are processed ?

From what I could see at https://github.com/apache/cloudstack/blob/master/systemvm/debian/opt/cloud/bin/cs/CsFile.py#L58, a commit returns a boolean value of whether the file has changed, so if it tries to add a host which already exists in the file, it shouldn't restart dnsmasq

@rhtyd @davidjumani no worries, it looks good.

2020-12-11 09:39:48,228 INFO     Processing JSON file cmd_line.json
2020-12-11 09:39:49,165 INFO     Wrote edited file /etc/dnsmasq.d/cloud.conf
2020-12-11 09:39:49,166 INFO     Nothing to commit. The /var/lib/misc/dnsmasq.leases file did not change
2020-12-11 09:39:49,166 INFO     Attempting to delete entries from dnsmasq.leases file for VMs which are not on dhcphosts file
2020-12-11 09:39:49,167 ERROR    Caught error while trying to delete entries from dnsmasq.leases file: [Errno 2] No such file or directory: '/etc/dhcphosts.txt'
2020-12-11 09:39:49,168 INFO     Executing: systemctl restart dnsmasq
2020-12-11 09:39:49,704 INFO     Service dnsmasq restart
2020-12-11 09:39:49,795 INFO     Nothing to commit. The /etc/dnsmasq.d/cloud.conf file did not change
2020-12-11 09:39:49,795 INFO     Nothing to commit. The /var/lib/misc/dnsmasq.leases file did not change
2020-12-11 09:39:49,795 INFO     Executing: systemctl is-active dnsmasq
2020-12-11 09:39:49,820 INFO     Executing: systemctl reload dnsmasq
2020-12-11 09:39:49,878 INFO     Service dnsmasq reload
2020-12-11 09:40:11,302 INFO     Processing JSON file ip_associations.json.a72a28ec-caed-4f3a-908f-e8d8df726d57
2020-12-11 09:40:11,910 INFO     Processing JSON file monitor_service.json.335af30e-fd9e-44b1-a6c0-248a98c480db
2020-12-11 09:40:12,587 INFO     Processing JSON file vm_dhcp_entry.json.afaa3985-b0e0-4f2f-bd73-f61282006e0e
2020-12-11 09:40:12,794 INFO     Processing JSON file vm_dhcp_entry.json.fbba4e25-3569-4513-99ab-39d9f38a80d2
2020-12-11 09:40:13,129 INFO     Processing JSON file vm_dhcp_entry.json.b6a1f368-180b-4a48-b664-22aa8e89cfba
2020-12-11 09:40:13,530 INFO     Processing JSON file vm_metadata.json.ab86c2c2-acfc-4d01-8e4b-bd11ae7101ed
2020-12-11 09:40:13,993 INFO     Processing JSON file vm_metadata.json.8750a124-ba49-4fb5-8484-50ff65971f9a
2020-12-11 09:40:14,256 INFO     Processing JSON file vm_metadata.json.72e3d268-5e11-4bcd-a693-ab8fc599718b
2020-12-11 09:40:14,713 INFO     Wrote edited file /etc/dnsmasq.d/cloud.conf
2020-12-11 09:40:14,714 INFO     Wrote edited file /var/lib/misc/dnsmasq.leases
2020-12-11 09:40:14,715 INFO     Attempting to delete entries from dnsmasq.leases file for VMs which are not on dhcphosts file
2020-12-11 09:40:14,724 INFO     Deleted 0 entries from dnsmasq.leases file
2020-12-11 09:40:14,725 INFO     Executing: systemctl restart dnsmasq
2020-12-11 09:40:14,866 INFO     Service dnsmasq restart
2020-12-11 09:40:23,530 INFO     Processing JSON file firewall_rules.json.0b59db02-5eb7-44b3-a6d7-2ab3f1ad03b5
2020-12-11 09:40:24,646 INFO     Nothing to commit. The /etc/dnsmasq.d/cloud.conf file did not change
2020-12-11 09:40:24,646 INFO     Nothing to commit. The /var/lib/misc/dnsmasq.leases file did not change
2020-12-11 09:40:24,648 INFO     Executing: systemctl is-active dnsmasq
2020-12-11 09:40:24,660 INFO     Executing: systemctl reload dnsmasq
2020-12-11 09:40:24,688 INFO     Service dnsmasq reload

From what I could see at https://github.com/apache/cloudstack/blob/master/systemvm/debian/opt/cloud/bin/cs/CsFile.py#L58, a commit returns a boolean value of whether the file has changed, so if it tries to add a host which already exists in the file, it shouldn't restart dnsmasq

@davidjumani from my testing, dnsmasq will be restarted each time when start a vm.
it's better to reload it. not a big problem.

2020-12-11 09:44:56,927 INFO     Processing JSON file vm_dhcp_entry.json.c0a11309-5ec0-4166-8c17-ae2c56a44b8e
2020-12-11 09:44:57,244 INFO     Nothing to commit. The /etc/dnsmasq.d/cloud.conf file did not change
2020-12-11 09:44:57,244 INFO     Wrote edited file /var/lib/misc/dnsmasq.leases
2020-12-11 09:44:57,245 INFO     Attempting to delete entries from dnsmasq.leases file for VMs which are not on dhcphosts file
2020-12-11 09:44:57,245 INFO     Deleted 0 entries from dnsmasq.leases file
2020-12-11 09:44:57,245 INFO     Executing: systemctl restart dnsmasq
2020-12-11 09:44:57,366 INFO     Service dnsmasq restart
2020-12-11 09:44:58,253 INFO     Processing JSON file vm_metadata.json.250b5ba8-fd86-4dcb-86ef-2bc6c634d224

That's linked to #3613
I can create a separate PR to change it globally, but that will require extensive testing

That's linked to #3613
I can create a separate PR to change it globally, but that will require extensive testing

@davidjumani yeah.
@rhtyd this is ready for merge.

We probably want to wait until a manual test confirmation cc @borisstoyanov @vladimirpetrov

cc me as well. Andrija Panic Cloud Architect ShapeBlue

…

________________________________ From: Rohit Yadav <notifications@github.com> Sent: Friday, December 11, 2020 12:41:38 PM To: apache/cloudstack <cloudstack@noreply.github.com> Cc: Andrija Panic <andrija.panic@shapeblue.com>; Review requested <review_requested@noreply.github.com> Subject: Re: [apache/cloudstack] vr: Ensuring dnsmasq.leases file is populated (#4529) We probably want to wait until a manual test confirmation cc @borisstoyanov<https://github.com/borisstoyanov> @vladimirpetrov<https://github.com/vladimirpetrov> — You are receiving this because your review was requested. Reply to this email directly, view it on GitHub<#4529 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AK5EN3LJADBBK3NYZFOC4BDSUIAPFANCNFSM4UU4HHAA>. Andrija Panic Cloud Architect s: +44 20 3603 0540 | m: +381 69 272 3690 e: andrija.panic@shapeblue.com | w: www.shapeblue.com | t: @shapeblue a: 3 London Bridge Street, 3rd floor, News Building, London SE1 9SGUK Shape Blue Ltd is a company incorporated in England & Wales. ShapeBlue Services India LLP is a company incorporated in India and is operated under license from Shape Blue Ltd. Shape Blue Brasil Consultoria Ltda is a company incorporated in Brasil and is operated under license from Shape Blue Ltd. ShapeBlue SA Pty Ltd is a company registered by The Republic of South Africa and is traded under license from Shape Blue Ltd. ShapeBlue is a registered trademark.This email and any attachments to it may be confidential and are intended solely for the use of the individual to whom it is addressed. Any views or opinions expressed are solely those of the author and do not necessarily represent those of Shape Blue Ltd or related companies. If you are not the intended recipient of this email, you must neither take any action based upon its contents, nor copy or show it to anyone. Please contact the sender if you believe you have received this email in error. Find out more about ShapeBlue and our range of CloudStack related services: IaaS Cloud Design & Build | CSForge - rapid IaaS deployment framework CloudStack Consulting | CloudStack Software Engineering CloudStack Infrastructure Support | CloudStack Bootcamp Training Courses

@blueorangutan package

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

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

@blueorangutan package

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

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

@blueorangutan test

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

Trillian test result (tid-3341)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 30186 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4529-t3341-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

This looks like a good fix, Wei has provided manual tests. @DaanHoogland should we consider this for RC2 if one of @andrijapanicsb @borisstoyanov @vladimirpetrov or yourself can do a quick manual test? Thanks.

* master: server: add conditions for custom offerings (apache#4540) vr: Ensuring dnsmasq.leases file is populated (apache#4529) template: Ensuring template is cross zone if type changed to system (apache#4522) storage: Fix hypervisor type cast to string (apache#4516) db upgrade: fix sql exception: Access denied; you need (at least one of) the SUPER privilege(s) for this operation (apache#4533) CLOUDSTACK-10423:Potential sensitive information disclosure (apache#4536) jobs: The patch remove the password from resultObject and make it be humanreadable (apache#4538) listphysicalnetworks: Honouring keyword parameter (apache#4511) Fix NPE when Volume exists on secondary store but doesn't have a download URL (apache#4530) apidoc issue (apache#4532) db: Fix description of volume.stats.interval which is in milliseconds not seconds (apache#4526) kvm: set cpu topology only if cpucore per socket is positive value (apache#4527) xenserver: check and eject patch vbd for systemvms (apache#4525) Fix warning when setup cloudstack-common (apache#4523) kvm: FIX cpucorespersocket is not working on KVM (apache#4497) change debug to warn for unknown exceptions (apache#4521) Fix failure in validating IP address in case of multiple Management Servers (apache#4507) Update log output for FirstFitPlanner (apache#4515) ui: deprecate old UI and move to legacy to be served at /client/legacy (apache#4518)

davidjumani added component:virtual-router Severity:Major type:bug labels Dec 10, 2020

davidjumani requested review from DaanHoogland, PaulAngus, andrijapanicsb and yadvr December 10, 2020 12:58

davidjumani marked this pull request as ready for review December 10, 2020 12:58

yadvr requested review from GabrielBrascher and weizhouapache December 10, 2020 19:12

davidjumani force-pushed the fix-dnsmasq-leases branch from 4f61604 to 1ac4c78 Compare December 11, 2020 04:03

yadvr requested review from harikrishna-patnala and sureshanaparti December 11, 2020 08:44

yadvr added this to the 4.15.1.0 milestone Dec 11, 2020

yadvr approved these changes Dec 11, 2020

View reviewed changes

yadvr reviewed Dec 11, 2020

View reviewed changes

weizhouapache approved these changes Dec 11, 2020

View reviewed changes

davidjumani changed the base branch from master to 4.14 December 11, 2020 09:27

davidjumani added 2 commits December 11, 2020 14:59

vr: Ensuring dnsmasq.leases file is populated

a19faf9

Fix pylint issues

3f1a444

davidjumani force-pushed the fix-dnsmasq-leases branch from 1ac4c78 to 3f1a444 Compare December 11, 2020 09:29

weizhouapache reviewed Dec 11, 2020

View reviewed changes

yadvr modified the milestones: 4.15.1.0, 4.14.1.0 Dec 11, 2020

yadvr requested review from borisstoyanov and vladimirpetrov December 11, 2020 12:42

DaanHoogland merged commit 4d33e15 into apache:4.14 Dec 14, 2020

DaanHoogland deleted the fix-dnsmasq-leases branch December 14, 2020 09:06

andrijapanicsb mentioned this pull request Dec 16, 2020

dnsmasq.leases file is missing on the new VR after network restart #3788

Closed

                           self.dhcp_opts.add("%s,%s" % (tag, 3))
                           self.dhcp_opts.add("%s,%s" % (tag, 6))
                           self.dhcp_opts.add("%s,%s" % (tag, 15))
+                          self.dhcp_leases.search(entry['mac_address'], "0 %s %s %s *" % (entry['mac_address'],

vr: Ensuring dnsmasq.leases file is populated by davidjumani · Pull Request #4529 · apache/cloudstack · GitHub

Repository navigation

vr: Ensuring dnsmasq.leases file is populated - #4529

vr: Ensuring dnsmasq.leases file is populated#4529DaanHoogland merged 2 commits intoapache:4.14apache/cloudstack:4.14from shapeblue:fix-dnsmasq-leasesshapeblue/cloudstack:fix-dnsmasq-leasesCopy head branch name to clipboard

Conversation

davidjumani commented Dec 10, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Types of changes

How Has This Been Tested?

/var/log/dnsmasq.log - No DHCP requests received

/var/lib/misc/dnsmasq.leases - They exist ✨

BEFORE :

Logs when rebooting a VM after network restart with cleanup

AFTER :

Logs when rebooting a VM after network restart with cleanup

Uh oh!

davidjumani commented Dec 10, 2020

Uh oh!

blueorangutan commented Dec 10, 2020

Uh oh!

blueorangutan commented Dec 10, 2020

Uh oh!

davidjumani commented Dec 10, 2020

Uh oh!

blueorangutan commented Dec 10, 2020

Uh oh!

blueorangutan commented Dec 10, 2020

Uh oh!

yadvr left a comment

Choose a reason for hiding this comment

Uh oh!

yadvr Dec 11, 2020

Choose a reason for hiding this comment

Uh oh!

davidjumani Dec 11, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

weizhouapache commented Dec 11, 2020

Uh oh!

weizhouapache left a comment

Choose a reason for hiding this comment

Uh oh!

davidjumani commented Dec 11, 2020

Uh oh!

weizhouapache commented Dec 11, 2020

Uh oh!

davidjumani commented Dec 11, 2020

Uh oh!

weizhouapache Dec 11, 2020

Choose a reason for hiding this comment

Uh oh!

davidjumani Dec 11, 2020

Choose a reason for hiding this comment

Uh oh!

weizhouapache Dec 11, 2020

Choose a reason for hiding this comment

Uh oh!

weizhouapache Dec 11, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

davidjumani Dec 11, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

weizhouapache Dec 11, 2020

Choose a reason for hiding this comment

Uh oh!

yadvr commented Dec 11, 2020

Uh oh!

andrijapanicsb commented Dec 11, 2020 via email

Uh oh!

davidjumani commented Dec 11, 2020

Uh oh!

blueorangutan commented Dec 11, 2020

Uh oh!

blueorangutan commented Dec 11, 2020

Uh oh!

vr: Ensuring dnsmasq.leases file is populated#4529
DaanHoogland merged 2 commits into
apache:4.14from
shapeblue:fix-dnsmasq-leases

davidjumani commented Dec 10, 2020 •
edited
Loading

davidjumani Dec 11, 2020 •
edited
Loading

weizhouapache Dec 11, 2020 •
edited
Loading

davidjumani Dec 11, 2020 •
edited
Loading