CLOUDSTACK-10252: Delete dnsmasq leases file on restart#2427
CLOUDSTACK-10252: Delete dnsmasq leases file on restart#2427yadvr merged 1 commit intoapache:4.11apache/cloudstack:4.11from shapeblue:dnsmasq-leases-remove-fixshapeblue/cloudstack:dnsmasq-leases-remove-fixCopy head branch name to clipboard
Conversation
Delete dnsmasq's leases file when dnsmasq is restarted to avoid it use old ip-mac-address-vm mapping leases. Signed-off-by: Rohit Yadav <rohit.yadav@shapeblue.com>
|
@rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress. |
|
Packaging result: ✔centos6 ✔centos7 ✔debian. JID-1674 |
|
@blueorangutan test |
|
@rhtyd a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests |
|
Trillian test result (tid-2186)
|
| if self.conf.commit(): | ||
| restart_dnsmasq = True | ||
|
|
||
| if self.cloud.commit(): |
There was a problem hiding this comment.
@resmo The commit is called on two different files, it's the same old code refactored differently.
There was a problem hiding this comment.
I see conf.commit and cloud.commit. Those are two different objects, @resmo . How is this suspect?
|
I am skeptical, at the first view, are the "old" valid entries rewritten? |
|
@resmo the values from json/dictionary are written on commit. |
DaanHoogland
left a comment
There was a problem hiding this comment.
I don't like the idea of deleting the leases file. As we have seen at a client's, people can have static routes that get lost if renewal results in a nok and a temporary absence of an ip on their interface. We should call release on specific addresses when these are changed or deleted.
that said, this fixes the regression, so it is an improvement by the looks of it.
|
@DaanHoogland that's right, the aim of the PR was scoped only to fix the blocker. Perhaps for 4.11.1+ we can aim to improve changing the leases file instead of removing it, how it used to be done before it was removed: |
|
Yes, well that bit of code does dnsmasq's job. I think we should merge this and then find a way to call a release on a single lease by mac or ip. LGTM ^^ |
|
sounds like a plan. |
|
Thanks, merging this based on code reviews and test results. |
…os' into '4.20.0.0-scclouds' Remoção do bloqueio na UI para acessar a API `changeOfferingForVolume` Closes apache#2427 and apache#1363 See merge request scclouds/scclouds!1077
Delete dnsmasq's leases file when dnsmasq is restarted to avoid it
use old ip-mac-address-vm mapping leases.
Pinging for review - @DaanHoogland @borisstoyanov @nvazquez @wido @rafaelweingartner and others.
@blueorangutan package