[lxc-devel] [lxd/master] Allow bridged device removal when DHCP disabled on parent network
tomponline on Github
lxc-bot at linuxcontainers.org
Fri Aug 23 11:05:34 UTC 2019
A non-text attachment was scrubbed...
Name: not available
Type: text/x-mailbox
Size: 472 bytes
Desc: not available
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20190823/9527fac6/attachment.bin>
-------------- next part --------------
From bc8ea0f11929533fab63ac0340f2f03eaf2d263e Mon Sep 17 00:00:00 2001
From: Thomas Parrott <tomp at tomp.uk>
Date: Fri, 23 Aug 2019 11:57:15 +0100
Subject: [PATCH 1/4] Adds manifest and manifest.uuid to gitignore
Signed-off-by: Thomas Parrott <tomp at tomp.uk>
---
.gitignore | 2 ++
1 file changed, 2 insertions(+)
diff --git a/.gitignore b/.gitignore
index 136c388ba3..8924d46658 100644
--- a/.gitignore
+++ b/.gitignore
@@ -7,6 +7,8 @@ test/deps/devlxd-client
test/macaroon-identity/macaroon-identity
*~
tags
+manifest
+manifest.uuid
# For Atom ctags
.tags
From 8b573eebee0a7cf0e86ed07d7bc6cbfbff52b95d Mon Sep 17 00:00:00 2001
From: Thomas Parrott <tomp at tomp.uk>
Date: Fri, 23 Aug 2019 11:59:08 +0100
Subject: [PATCH 2/4] device/nic/bridged: DHCP release fixes
- Always tries to remove the static dnsmasq host config file on remove, even if dnsmasq isn't running any more.
- No longer errors on inability to send DHCP release packet on remove if parent network has no IP. Instead logs warning and continues to next DHCP lease line.
This allows removing a bridged device if the parent has had DHCP disabled after the instance has aquired a lease.
Fixes #6102
Signed-off-by: Thomas Parrott <tomp at tomp.uk>
---
lxd/device/nic_bridged.go | 21 ++++++++++++---------
1 file changed, 12 insertions(+), 9 deletions(-)
diff --git a/lxd/device/nic_bridged.go b/lxd/device/nic_bridged.go
index d0fac97524..c0c43911b0 100644
--- a/lxd/device/nic_bridged.go
+++ b/lxd/device/nic_bridged.go
@@ -24,6 +24,7 @@ import (
"github.com/lxc/lxd/lxd/instance"
"github.com/lxc/lxd/lxd/iptables"
"github.com/lxc/lxd/shared"
+ "github.com/lxc/lxd/shared/logger"
)
// dhcpAllocation represents an IP allocation from dnsmasq used for IP filtering.
@@ -274,20 +275,22 @@ func (d *nicBridged) Remove() error {
return err
}
- // If device was on managed parent, remove old config file.
- if d.config["parent"] != "" && shared.PathExists(shared.VarPath("networks", d.config["parent"], "dnsmasq.pid")) {
+ if d.config["parent"] != "" {
dnsmasq.ConfigMutex.Lock()
defer dnsmasq.ConfigMutex.Unlock()
+ // Remove dnsmasq config if it exists (doesn't return error if file is missing).
err := dnsmasq.RemoveStaticEntry(d.config["parent"], d.instance.Project(), d.instance.Name())
if err != nil {
return err
}
- // Reload dnsmasq to apply new settings.
- err = dnsmasq.Kill(d.config["parent"], true)
- if err != nil {
- return err
+ // Reload dnsmasq to apply new settings if dnsmasq is running.
+ if shared.PathExists(shared.VarPath("networks", d.config["parent"], "dnsmasq.pid")) {
+ err = dnsmasq.Kill(d.config["parent"], true)
+ if err != nil {
+ return err
+ }
}
}
@@ -1181,8 +1184,8 @@ func (d *nicBridged) networkClearLease(name string, network string, hwaddr strin
srcIP := net.ParseIP(fields[2])
if dstIPv4 == nil {
- errs = append(errs, fmt.Errorf("Failed to release DHCPv4 lease for instance \"%s\", IP \"%s\", MAC \"%s\", %v", name, srcIP, srcMAC, "No server address found"))
- continue
+ logger.Warnf("Failed to release DHCPv4 lease for instance \"%s\", IP \"%s\", MAC \"%s\", %v", name, srcIP, srcMAC, "No server address found")
+ continue // Cant send release packet if no dstIP found.
}
err = d.networkDHCPv4Release(srcMAC, srcIP, dstIPv4)
@@ -1200,7 +1203,7 @@ func (d *nicBridged) networkClearLease(name string, network string, hwaddr strin
}
if dstIPv6 == nil {
- errs = append(errs, fmt.Errorf("Failed to release DHCPv6 lease for instance \"%s\", IP \"%s\", DUID \"%s\", IAID \"%s\": %s", name, srcIP, DUID, IAID, "No server address found"))
+ logger.Warn("Failed to release DHCPv6 lease for instance \"%s\", IP \"%s\", DUID \"%s\", IAID \"%s\": %s", name, srcIP, DUID, IAID, "No server address found")
continue // Cant send release packet if no dstIP found.
}
From dad32cff302d61bad5d2fc08e32dc80f30ab4317 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <tomp at tomp.uk>
Date: Fri, 23 Aug 2019 12:01:40 +0100
Subject: [PATCH 3/4] networks: Removes old dnsmasq.leases file on network
start
If dnsmasq is not going to be started (i.e DHCP is disabled) and an old dnsmasq.leases file exists then it is removed.
Fixes #6102
Signed-off-by: Thomas Parrott <tomp at tomp.uk>
---
lxd/networks.go | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/lxd/networks.go b/lxd/networks.go
index c8cd30d736..2875232cbe 100644
--- a/lxd/networks.go
+++ b/lxd/networks.go
@@ -1913,6 +1913,15 @@ func (n *network) Start() error {
return err
}
}
+ } else {
+ // Clean up old dnsmasq config if exists and we are not starting dnsmasq.
+ leasesPath := shared.VarPath("networks", n.name, "dnsmasq.leases")
+ if shared.PathExists(leasesPath) {
+ err := os.Remove(leasesPath)
+ if err != nil {
+ return errors.Wrapf(err, "Failed to remove old dnsmasq leases file '%s'", leasesPath)
+ }
+ }
}
return nil
From d25ab8af361603736a8654a658ad6e23b73d8e85 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <tomp at tomp.uk>
Date: Fri, 23 Aug 2019 12:02:37 +0100
Subject: [PATCH 4/4] test: Adds more bridged DHCP release tests
- Test to ensure bridged device can be removed after DHCP being disabled on parent network after instance has aquired a lease.
- Test that dnsmasq.leases file is removed if DHCP is disabled on a network.
Signed-off-by: Thomas Parrott <tomp at tomp.uk>
---
test/suites/container_devices_nic_bridged.sh | 29 ++++++++++++++++++++
1 file changed, 29 insertions(+)
diff --git a/test/suites/container_devices_nic_bridged.sh b/test/suites/container_devices_nic_bridged.sh
index 898cd710f3..63724a50b8 100644
--- a/test/suites/container_devices_nic_bridged.sh
+++ b/test/suites/container_devices_nic_bridged.sh
@@ -379,6 +379,35 @@ test_container_devices_nic_bridged() {
false
fi
+ # Check dnsmasq leases file removed if DHCP disabled and that device can be removed.
+ lxc config device add "${ctName}" eth0 nic nictype=bridged parent="${brName}" name=eth0
+ lxc start "${ctName}"
+ lxc exec "${ctName}" -- udhcpc -i eth0
+ lxc network unset "${brName}" ipv4.address
+ lxc network unset "${brName}" ipv6.address
+
+ if [ -f "${LXD_DIR}/networks/${brName}/dnsmasq.leases" ] ; then
+ echo "dnsmasq.leases file still present after disabling DHCP"
+ false
+ fi
+
+ if [ -f "${LXD_DIR}/networks/${brName}/dnsmasq.pid" ] ; then
+ echo "dnsmasq.pid file still present after disabling DHCP"
+ false
+ fi
+
+ lxc config device remove "${ctName}" eth0
+ lxc stop -f "${ctName}"
+ if [ -f "${LXD_DIR}/networks/${brName}/dnsmasq.hosts/${ctName}" ] ; then
+ echo "dnsmasq host config file not removed from network"
+ false
+ fi
+
+ # Re-enable DHCP on network.
+ lxc network set "${brName}" ipv4.address 192.0.2.1/24
+ lxc network set "${brName}" ipv6.address 2001:db8::1/64
+
+ # Check dnsmasq host file is created on add.
lxc config device add "${ctName}" eth0 nic nictype=bridged parent="${brName}" name=eth0
if [ ! -f "${LXD_DIR}/networks/${brName}/dnsmasq.hosts/${ctName}" ] ; then
echo "dnsmasq host config file not created"
More information about the lxc-devel
mailing list