LXD storage backend integration tests

[holmanb]

Test lxd config functionality

[aciba90]

LGTM! Thanks.

[blackboxsw]

All tests pass, it would be nice to confirm apt install log of known storage pkg. but otherwise +1

[blackboxsw]

LGTM. We will have intermittent errors on this type of test due to occasional exit 100 from apt. I just hit this locally

]: Unexpected error while running command.
Command: ['eatmydata', 'apt-get', '--option=Dpkg::Options::=--force-confold', '--option=Dpkg::options::=--force-unsafe-io', '--assume-yes', '--quiet', 'install', 'lvm2']
Exit code: 100                                                                  
Reason: -        ```

We may want to solve this separately with a  retry on that package install

[blackboxsw]

@holmanb the test_storage_lvm test is broken (maybe due to a postinst script failure of installing lvm2). This may be a bug in the lvm2 postinst packaging or in our specific test environments as apt install lvm2 fails every time with exit 100

root@cloudinit-0721-232304wnvzml11:~# apt install lvm2
Reading package lists... Done
Building dependency tree       
Reading state information... Done
lvm2 is already the newest version (2.03.07-1ubuntu1).
0 upgraded, 0 newly installed, 0 to remove and 3 not upgraded.
1 not fully installed or removed.
After this operation, 0 B of additional disk space will be used.
Do you want to continue? [Y/n] y
Setting up lvm2 (2.03.07-1ubuntu1) ...
update-initramfs: deferring update (trigger activated)
Failed to restart lvm2-lvmpolld.service: Unit lvm2-lvmpolld.socket is masked.
invoke-rc.d: initscript lvm2-lvmpolld, action "restart" failed.
● lvm2-lvmpolld.service - LVM2 poll daemon
     Loaded: loaded (/lib/systemd/system/lvm2-lvmpolld.service; static; vendor p
reset: enabled)
     Active: inactive (dead)
       Docs: man:lvmpolld(8)
dpkg: error processing package lvm2 (--configure):
 installed lvm2 package post-installation script subprocess returned error exit 
status 1
Processing triggers for initramfs-tools (0.136ubuntu6.7) ...
update-initramfs: Generating /boot/initrd.img-5.4.0-1071-kvm
W: mkconf: MD subsystem is not loaded, thus I cannot scan for arrays.
W: mdadm: failed to auto-generate temporary mdadm.conf file.
Errors were encountered while processing:
 lvm2
E: Sub-process /usr/bin/dpkg returned an error code (1)
root@cloudinit-0721-232304wnvzml11:~# echo $?
100

Not sure if we need to file a bug here due to postrm vs postinst logic in lvm2 package. But in the meantime we may need to workaround this issue to allow you to add this test by supplementing your bootcmd with
systemctl unmask lvm2-lvmpolld.socket on the lvm case

[holmanb]

Not sure if we need to file a bug

I just added a "me too" to the open bug here: https://bugs.launchpad.net/ubuntu/+source/lvm2/+bug/1871176

[holmanb]

Thanks for the initial reviews on this @aciba90 and @blackboxsw. Reset and requesting another round.

Changes:

• work around lvm2 issue @blackboxsw pointed out previously (nothing new to report, this issue has been reported multiple times on LP and SO has some workarounds)
• found and worked around a kernel bug in Ubuntu's -kvm kernels causing LXD to fail with default lvm backend settings.

[aciba90]

LGTM!

[aciba90]

Nice one!

[blackboxsw]

@holmanb thanks for the updates. looks pretty good but I think we need to tweak the conditions underwhich we determine to setup lvm.use_thinpool=false

[blackboxsw]

This issue isn't limited to ubuntu, it's possible that this is a problem on any given OS/kernel if the right kernel modules are not included.

I think we instead want to perform a check to see if the dm-thin-pool module is present for the active kernel.

We can either do that through:

1. file test on required dm-thin-pools module

kernel = util.system_info()["uname"][2])
if not os.path.exists(f"/lib/modules/{kernel}/kernel/drivers/md/dm-thin-pool.ko):
   ...log.warn and setup w/ lvm.use_thinpool=False....

2 subp.subp(['modinfo', 'dm-thin-pool']) and check if we hit ProcesseExecutionError "modinfo: ERROR: Module dm-thin-pools not found.".

I'd lean toward solution #1 because solution #2 requires either a strict package dependency on kmod or optional runtime package dependency on kmow which I don't think we should grow broadly in cloud-init for this one off use-case when all we need it for is a file check.

[holmanb]

This issue isn't limited to ubuntu

Since this config module is limited to ubuntu, I think it is ubuntu-specific.

I think we instead want to perform a check to see if the dm-thin-pool module is present for the active kernel.

This approach causes a behavioral change that would be unpleasant to debug: kmod package update will change cloud-init behavior under this scheme. However, since we're logging this when the kmod is missing, I guess there is a breadcrumb.

I'll grab the first option in case we want to extend cc_lxd support to other linux distros.

Thanks!

[holmanb]

kernel = util.system_info()["uname"][2])

Note to self: If we get rid of the cast-to-list in system_info(), we get a named tuple for free (and more readable code).

[blackboxsw]

+1 I was thinking the same, though we'd maybe have to look at how that is represented in /run/cloud-init/instance-data.json to be sure we aren't doing some funky object-style annotations in the JSON output as I believe this data is surfaced there too.

[blackboxsw]

Let's switch the order here and only call os.path.exists if we are trying to 'lvm' also we didn't enclose the os.path.exists around the filepath it also included the "and" clause

Here's the consolidated diff and a unittest to cover this

diff --git a/cloudinit/config/cc_lxd.py b/cloudinit/config/cc_lxd.py
index 07b29b5ad..76edebfdb 100644
--- a/cloudinit/config/cc_lxd.py
+++ b/cloudinit/config/cc_lxd.py
@@ -136,9 +136,8 @@ def handle(name, cfg, cloud: Cloud, log: Logger, args):
 
         # Bug https://bugs.launchpad.net/ubuntu/+source/linux-kvm/+bug/1982780
         kernel = util.system_info()["uname"][2]
-        if not os.path.exists(
+        if init_cfg["storage_backend"] == "lvm" and not os.path.exists(
             f"/lib/modules/{kernel}/kernel/drivers/md/dm-thin-pool.ko"
-            and init_cfg["storage_backend"] == "lvm"
         ):
             log.warning(
                 "cloud-init doesn't use thinpool by default on Ubuntu due to "
diff --git a/tests/unittests/config/test_cc_lxd.py b/tests/unittests/config/test_cc_lxd.py
index 574af12ce..e79604ab0 100644
--- a/tests/unittests/config/test_cc_lxd.py
+++ b/tests/unittests/config/test_cc_lxd.py
@@ -35,30 +35,34 @@ class TestLxd(t_help.CiTestCase):
         ("dir", None, None),
     )
 
+    @mock.patch("cloudinit.config.cc_lxd.util.system_info")
+    @mock.patch("cloudinit.config.cc_lxd.os.path.exists", return_value=True)
     @mock.patch("cloudinit.config.cc_lxd.subp.subp", return_value=True)
     @mock.patch("cloudinit.config.cc_lxd.subp.which", return_value=False)
     @mock.patch(
         "cloudinit.config.cc_lxd.maybe_cleanup_default", return_value=None
     )
-    def test_lxd_init(self, m_maybe_clean, m_which, m_subp):
+    def test_lxd_init(self, maybe_clean, which, subp, exists, system_info):
+        system_info.return_value = {"uname": [0, 1, "mykernel"]}
         cc = get_cloud(mocked_distro=True)
-        m_install = cc.distro.install_packages
+        install = cc.distro.install_packages
 
         for backend, cmd, package in self.backend_def:
             lxd_cfg = deepcopy(self.lxd_cfg)
             lxd_cfg["lxd"]["init"]["storage_backend"] = backend
-            m_subp.call_args_list = []
-            m_install.call_args_list = []
+            subp.call_args_list = []
+            install.call_args_list = []
+            exists.call_args_list = []
             cc_lxd.handle("cc_lxd", lxd_cfg, cc, self.logger, [])
             if cmd:
-                m_which.assert_called_with(cmd)
+                which.assert_called_with(cmd)
             # no bridge config, so maybe_cleanup should not be called.
-            self.assertFalse(m_maybe_clean.called)
+            self.assertFalse(maybe_clean.called)
             self.assertEqual(
                 [
                     mock.call(list(filter(None, ["lxd", package]))),
                 ],
-                m_install.call_args_list,
+                install.call_args_list,
             )
             self.assertEqual(
                 [
@@ -74,9 +78,21 @@ class TestLxd(t_help.CiTestCase):
                         ]
                     ),
                 ],
-                m_subp.call_args_list,
+                subp.call_args_list,
             )
 
+            if backend == "lvm":
+                self.assertEqual(
+                    [
+                        mock.call(
+                            "/lib/modules/mykernel/kernel/drivers/md/dm-thin-pool.ko"
+                        )
+                    ],
+                    exists.call_args_list,
+                )
+            else:
+                self.assertEqual([], exists.call_args_list)
+
     @mock.patch("cloudinit.config.cc_lxd.subp.which", return_value=False)
     def test_lxd_package_install(self, m_which):
         for backend, _, package in self.backend_def:

[blackboxsw]

LGTM! awaiting CI