[lxc-devel] [lxd/master] Fixes dnsmasq host file cleanup

[tomponline on Github]

A non-text attachment was scrubbed...
Name: not available
Type: text/x-mailbox
Size: 629 bytes
Desc: not available
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20190628/f5ba504f/attachment.bin>
-------------- next part --------------
From d28816474c956560bd8c389854b4261bed70d037 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Fri, 28 Jun 2019 10:01:46 +0100
Subject: [PATCH 1/3] Revert "container/lxc: Moves networkUpdateStatic during
 Stop with the other lease related code"

This reverts commit 3955d9f38b765165a6a86251a16b22ecafa6282e.

Fixes #5889
---
 lxd/container_lxc.go | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/lxd/container_lxc.go b/lxd/container_lxc.go
index c9ed8937e8..558ceb0000 100644
--- a/lxd/container_lxc.go
+++ b/lxd/container_lxc.go
@@ -4360,9 +4360,6 @@ func (c *containerLXC) Delete() error {
 			return err
 		}
 
-		// Remove any static lease file
-		networkUpdateStatic(c.state, "")
-
 		// Update network files
 		for k, m := range c.expandedDevices {
 			if m["type"] != "nic" || m["nictype"] != "bridged" {
@@ -4401,6 +4398,11 @@ func (c *containerLXC) Delete() error {
 		}
 	}
 
+	if !c.IsSnapshot() {
+		// Remove any static lease file
+		networkUpdateStatic(c.state, "")
+	}
+
 	logger.Info("Deleted container", ctxMap)
 
 	if c.IsSnapshot() {

From e0c00b15a6896453e43654eff4d065dbf7da860e Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Fri, 28 Jun 2019 10:04:37 +0100
Subject: [PATCH 2/3] container/lxc: Improves comment on DHCP host config
 removal

The call to networkUpdateStatic() during container deletion is dependent on a specific ordering that is non-obvious, adds a comment for future readers to avoid introducing this bug again.

Fixes #5889

Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
 lxd/container_lxc.go | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/lxd/container_lxc.go b/lxd/container_lxc.go
index 558ceb0000..bb67eb4877 100644
--- a/lxd/container_lxc.go
+++ b/lxd/container_lxc.go
@@ -4399,7 +4399,9 @@ func (c *containerLXC) Delete() error {
 	}
 
 	if !c.IsSnapshot() {
-		// Remove any static lease file
+		// Remove any static lease file *after* container config has been removed from cluster.
+		// This is ordering is important, as if it is done earlier than c.state.Cluster.ContainerRemove
+		// then the static host config is re-created and left after the container is deleted.
 		networkUpdateStatic(c.state, "")
 	}
 

From a53f912435970d04f4c07460bdaa96c438c3be04 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Fri, 28 Jun 2019 10:10:14 +0100
Subject: [PATCH 3/3] test: Adds check for dnsmasq host config file removal on
 container delete

Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
 test/suites/container_devices_nic_bridged.sh | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/test/suites/container_devices_nic_bridged.sh b/test/suites/container_devices_nic_bridged.sh
index c29ff68c36..925d3052fa 100644
--- a/test/suites/container_devices_nic_bridged.sh
+++ b/test/suites/container_devices_nic_bridged.sh
@@ -282,6 +282,12 @@ test_container_devices_nic_bridged() {
     false
   fi
 
+  # Check dnsmasq host config file is removed.
+  if [ -f "${LXD_DIR}/networks/${brName}/dnsmasq.hosts/${ctName}" ] ; then
+    echo "dnsmasq host config file not removed"
+    false
+  fi
+
   # Cleanup.
   lxc network delete "${brName}"
   lxc profile delete "${ctName}"