lxd: Delete default network and detach device if lxd-init created them.

Newer versions (3.0.1+) of lxd create the 'lxdbr0' network when
'lxd init --auto' is invoked.

When cloud-init is given a network configuration to pass on to
lxc and that config had no name specified or 'lxdbr0', then cloud-init
would fail to create the network as it already exists.

Similarly, we need to remove the device from the default profile
so that the attach code can work.

Also, add a _lxc method and use it to make sure we're getting the
--force-local flag everywhere.

LP: #1776958

1 files changed, 64 insertions, 16 deletions

diff --git a/tests/unittests/test_handler/test_handler_lxd.py b/tests/unittests/test_handler/test_handler_lxd.py
index a2054980..4dd7e09f 100644
--- a/tests/unittests/test_handler/test_handler_lxd.py
+++ b/tests/unittests/test_handler/test_handler_lxd.py
@@ -33,12 +33,16 @@ class TestLxd(t_help.CiTestCase):
         cc = cloud.Cloud(ds, paths, {}, d, None)
         return cc
 
+    @mock.patch("cloudinit.config.cc_lxd.maybe_cleanup_default")
     @mock.patch("cloudinit.config.cc_lxd.util")
-    def test_lxd_init(self, mock_util):
+    def test_lxd_init(self, mock_util, m_maybe_clean):
         cc = self._get_cloud('ubuntu')
         mock_util.which.return_value = True
+        m_maybe_clean.return_value = None
         cc_lxd.handle('cc_lxd', self.lxd_cfg, cc, self.logger, [])
         self.assertTrue(mock_util.which.called)
+        # no bridge config, so maybe_cleanup should not be called.
+        self.assertFalse(m_maybe_clean.called)
         init_call = mock_util.subp.call_args_list[0][0][0]
         self.assertEqual(init_call,
                          ['lxd', 'init', '--auto',
@@ -46,32 +50,39 @@ class TestLxd(t_help.CiTestCase):
                           '--storage-backend=zfs',
                           '--storage-pool=poolname'])
 
+    @mock.patch("cloudinit.config.cc_lxd.maybe_cleanup_default")
     @mock.patch("cloudinit.config.cc_lxd.util")
-    def test_lxd_install(self, mock_util):
+    def test_lxd_install(self, mock_util, m_maybe_clean):
         cc = self._get_cloud('ubuntu')
         cc.distro = mock.MagicMock()
         mock_util.which.return_value = None
         cc_lxd.handle('cc_lxd', self.lxd_cfg, cc, self.logger, [])
         self.assertNotIn('WARN', self.logs.getvalue())
         self.assertTrue(cc.distro.install_packages.called)
+        cc_lxd.handle('cc_lxd', self.lxd_cfg, cc, self.logger, [])
+        self.assertFalse(m_maybe_clean.called)
         install_pkg = cc.distro.install_packages.call_args_list[0][0][0]
         self.assertEqual(sorted(install_pkg), ['lxd', 'zfs'])
 
+    @mock.patch("cloudinit.config.cc_lxd.maybe_cleanup_default")
     @mock.patch("cloudinit.config.cc_lxd.util")
-    def test_no_init_does_nothing(self, mock_util):
+    def test_no_init_does_nothing(self, mock_util, m_maybe_clean):
         cc = self._get_cloud('ubuntu')
         cc.distro = mock.MagicMock()
         cc_lxd.handle('cc_lxd', {'lxd': {}}, cc, self.logger, [])
         self.assertFalse(cc.distro.install_packages.called)
         self.assertFalse(mock_util.subp.called)
+        self.assertFalse(m_maybe_clean.called)
 
+    @mock.patch("cloudinit.config.cc_lxd.maybe_cleanup_default")
     @mock.patch("cloudinit.config.cc_lxd.util")
-    def test_no_lxd_does_nothing(self, mock_util):
+    def test_no_lxd_does_nothing(self, mock_util, m_maybe_clean):
         cc = self._get_cloud('ubuntu')
         cc.distro = mock.MagicMock()
         cc_lxd.handle('cc_lxd', {'package_update': True}, cc, self.logger, [])
         self.assertFalse(cc.distro.install_packages.called)
         self.assertFalse(mock_util.subp.called)
+        self.assertFalse(m_maybe_clean.called)
 
     def test_lxd_debconf_new_full(self):
         data = {"mode": "new",
@@ -147,14 +158,13 @@ class TestLxd(t_help.CiTestCase):
                 "domain": "lxd"}
         self.assertEqual(
             cc_lxd.bridge_to_cmd(data),
-            (["lxc", "network", "create", "testbr0",
+            (["network", "create", "testbr0",
               "ipv4.address=10.0.8.1/24", "ipv4.nat=true",
               "ipv4.dhcp.ranges=10.0.8.2-10.0.8.254",
               "ipv6.address=fd98:9e0:3744::1/64",
-              "ipv6.nat=true", "dns.domain=lxd",
-              "--force-local"],
-             ["lxc", "network", "attach-profile",
-              "testbr0", "default", "eth0", "--force-local"]))
+              "ipv6.nat=true", "dns.domain=lxd"],
+             ["network", "attach-profile",
+              "testbr0", "default", "eth0"]))
 
     def test_lxd_cmd_new_partial(self):
         data = {"mode": "new",
@@ -163,19 +173,18 @@ class TestLxd(t_help.CiTestCase):
                 "ipv6_nat": "true"}
         self.assertEqual(
             cc_lxd.bridge_to_cmd(data),
-            (["lxc", "network", "create", "lxdbr0", "ipv4.address=none",
-              "ipv6.address=fd98:9e0:3744::1/64", "ipv6.nat=true",
-              "--force-local"],
-             ["lxc", "network", "attach-profile",
-              "lxdbr0", "default", "eth0", "--force-local"]))
+            (["network", "create", "lxdbr0", "ipv4.address=none",
+              "ipv6.address=fd98:9e0:3744::1/64", "ipv6.nat=true"],
+             ["network", "attach-profile",
+              "lxdbr0", "default", "eth0"]))
 
     def test_lxd_cmd_existing(self):
         data = {"mode": "existing",
                 "name": "testbr0"}
         self.assertEqual(
             cc_lxd.bridge_to_cmd(data),
-            (None, ["lxc", "network", "attach-profile",
-                    "testbr0", "default", "eth0", "--force-local"]))
+            (None, ["network", "attach-profile",
+                    "testbr0", "default", "eth0"]))
 
     def test_lxd_cmd_none(self):
         data = {"mode": "none"}
@@ -183,4 +192,43 @@ class TestLxd(t_help.CiTestCase):
             cc_lxd.bridge_to_cmd(data),
             (None, None))
 
+
+class TestLxdMaybeCleanupDefault(t_help.CiTestCase):
+    """Test the implementation of maybe_cleanup_default."""
+
+    defnet = cc_lxd._DEFAULT_NETWORK_NAME
+
+    @mock.patch("cloudinit.config.cc_lxd._lxc")
+    def test_network_other_than_default_not_deleted(self, m_lxc):
+        """deletion or removal should only occur if bridge is default."""
+        cc_lxd.maybe_cleanup_default(
+            net_name="lxdbr1", did_init=True, create=True, attach=True)
+        m_lxc.assert_not_called()
+
+    @mock.patch("cloudinit.config.cc_lxd._lxc")
+    def test_did_init_false_does_not_delete(self, m_lxc):
+        """deletion or removal should only occur if did_init is True."""
+        cc_lxd.maybe_cleanup_default(
+            net_name=self.defnet, did_init=False, create=True, attach=True)
+        m_lxc.assert_not_called()
+
+    @mock.patch("cloudinit.config.cc_lxd._lxc")
+    def test_network_deleted_if_create_true(self, m_lxc):
+        """deletion of network should occur if create is True."""
+        cc_lxd.maybe_cleanup_default(
+            net_name=self.defnet, did_init=True, create=True, attach=False)
+        m_lxc.assert_called_once_with(["network", "delete", self.defnet])
+
+    @mock.patch("cloudinit.config.cc_lxd._lxc")
+    def test_device_removed_if_attach_true(self, m_lxc):
+        """deletion of network should occur if create is True."""
+        nic_name = "my_nic"
+        profile = "my_profile"
+        cc_lxd.maybe_cleanup_default(
+            net_name=self.defnet, did_init=True, create=False, attach=True,
+            profile=profile, nic_name=nic_name)
+        m_lxc.assert_called_once_with(
+            ["profile", "device", "remove", profile, nic_name])
+
+
 # vi: ts=4 expandtab