From 9a4c49a3410ea9fe9d47b50ce8fc03533256affc Mon Sep 17 00:00:00 2001 From: Brett Holman Date: Mon, 18 Jul 2022 09:54:18 -0600 Subject: [PATCH 01/24] rename file more generic --- .../integration_tests/modules/{test_lxd_bridge.py => test_lxd.py} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename tests/integration_tests/modules/{test_lxd_bridge.py => test_lxd.py} (100%) diff --git a/tests/integration_tests/modules/test_lxd_bridge.py b/tests/integration_tests/modules/test_lxd.py similarity index 100% rename from tests/integration_tests/modules/test_lxd_bridge.py rename to tests/integration_tests/modules/test_lxd.py From df734ef15972816f3b5dfa8f5aa450abf31be751 Mon Sep 17 00:00:00 2001 From: Brett Holman Date: Mon, 18 Jul 2022 12:09:16 -0600 Subject: [PATCH 02/24] Add tests for lxd backends --- tests/integration_tests/modules/test_lxd.py | 41 +++++++++++++++++++-- 1 file changed, 38 insertions(+), 3 deletions(-) diff --git a/tests/integration_tests/modules/test_lxd.py b/tests/integration_tests/modules/test_lxd.py index 002b16456f9..5a10e7a0ad8 100644 --- a/tests/integration_tests/modules/test_lxd.py +++ b/tests/integration_tests/modules/test_lxd.py @@ -8,9 +8,8 @@ from tests.integration_tests.util import verify_clean_log -USER_DATA = """\ +BRIDGE_USER_DATA = """\ #cloud-config -bootcmd: [ "apt-get --yes remove btrfs-progs" ] lxd: init: storage_backend: btrfs @@ -26,9 +25,18 @@ mtu: 9000 """ +STORAGE_USER_DATA = """\ +#cloud-config +bootcmd: [ "apt-get --yes remove {}" ] +{} +lxd: + init: + storage_backend: {} +""" + @pytest.mark.no_container -@pytest.mark.user_data(USER_DATA) +@pytest.mark.user_data(BRIDGE_USER_DATA) class TestLxdBridge: @pytest.mark.parametrize("binary_name", ["lxc", "lxd"]) def test_binaries_installed(self, class_client, binary_name): @@ -46,3 +54,30 @@ def test_bridge(self, class_client): raw_network_config = class_client.execute("lxc network show lxdbr0") network_config = yaml.safe_load(raw_network_config) assert "10.100.100.1/24" == network_config["config"]["ipv4.address"] + + +def validate_storage(validate_client): + cloud_init_log = validate_client.read_from_file("/var/log/cloud-init.log") + assert not any( + problem in cloud_init_log for problem in ("WARN", "ERR", "Traceback") + ) + + +@pytest.mark.no_container +@pytest.mark.user_data(STORAGE_USER_DATA.format("btrfs-progs", "", "btrfs")) +def test_storage_btrfs(client): + validate_storage(client) + + +@pytest.mark.no_container +@pytest.mark.user_data( + STORAGE_USER_DATA.format( + "lvm2", "packages:\n [ thin-provisioning-tools ] ", "lvm")) +def test_storage_lvm(client): + validate_storage(client) + + +@pytest.mark.no_container +@pytest.mark.user_data(STORAGE_USER_DATA.format("zfsutils-linux", "", "zfs")) +def test_storage_zfs(client): + validate_storage(client) From 6ff4e8d996d6988e109268fa19b730a590480c9a Mon Sep 17 00:00:00 2001 From: Brett Holman Date: Mon, 18 Jul 2022 13:51:11 -0600 Subject: [PATCH 03/24] use verification func --- tests/integration_tests/modules/test_lxd.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/integration_tests/modules/test_lxd.py b/tests/integration_tests/modules/test_lxd.py index 5a10e7a0ad8..7a0640637e0 100644 --- a/tests/integration_tests/modules/test_lxd.py +++ b/tests/integration_tests/modules/test_lxd.py @@ -57,9 +57,9 @@ def test_bridge(self, class_client): def validate_storage(validate_client): - cloud_init_log = validate_client.read_from_file("/var/log/cloud-init.log") - assert not any( - problem in cloud_init_log for problem in ("WARN", "ERR", "Traceback") + verify_clean_log( + validate_client.read_from_file("/var/log/cloud-init.log"), + ignore_deprecations=False ) From 3ee7ba54d6dca1153b5a5312652481a3cc64c266 Mon Sep 17 00:00:00 2001 From: Brett Holman Date: Mon, 18 Jul 2022 14:03:09 -0600 Subject: [PATCH 04/24] drop lvm workaround (not needed? --- tests/integration_tests/modules/test_lxd.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/tests/integration_tests/modules/test_lxd.py b/tests/integration_tests/modules/test_lxd.py index 7a0640637e0..ca2f3ec3207 100644 --- a/tests/integration_tests/modules/test_lxd.py +++ b/tests/integration_tests/modules/test_lxd.py @@ -28,7 +28,6 @@ STORAGE_USER_DATA = """\ #cloud-config bootcmd: [ "apt-get --yes remove {}" ] -{} lxd: init: storage_backend: {} @@ -64,7 +63,7 @@ def validate_storage(validate_client): @pytest.mark.no_container -@pytest.mark.user_data(STORAGE_USER_DATA.format("btrfs-progs", "", "btrfs")) +@pytest.mark.user_data(STORAGE_USER_DATA.format("btrfs-progs", "btrfs")) def test_storage_btrfs(client): validate_storage(client) @@ -72,12 +71,12 @@ def test_storage_btrfs(client): @pytest.mark.no_container @pytest.mark.user_data( STORAGE_USER_DATA.format( - "lvm2", "packages:\n [ thin-provisioning-tools ] ", "lvm")) + "lvm2", "lvm")) def test_storage_lvm(client): validate_storage(client) @pytest.mark.no_container -@pytest.mark.user_data(STORAGE_USER_DATA.format("zfsutils-linux", "", "zfs")) +@pytest.mark.user_data(STORAGE_USER_DATA.format("zfsutils-linux", "zfs")) def test_storage_zfs(client): validate_storage(client) From 34f8a5b02cc11024cf8a49d6ab2277b72f7eefa4 Mon Sep 17 00:00:00 2001 From: Brett Holman Date: Mon, 18 Jul 2022 14:26:31 -0600 Subject: [PATCH 05/24] fmt --- tests/integration_tests/modules/test_lxd.py | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/tests/integration_tests/modules/test_lxd.py b/tests/integration_tests/modules/test_lxd.py index ca2f3ec3207..3faeb62f01d 100644 --- a/tests/integration_tests/modules/test_lxd.py +++ b/tests/integration_tests/modules/test_lxd.py @@ -58,7 +58,7 @@ def test_bridge(self, class_client): def validate_storage(validate_client): verify_clean_log( validate_client.read_from_file("/var/log/cloud-init.log"), - ignore_deprecations=False + ignore_deprecations=False, ) @@ -69,9 +69,7 @@ def test_storage_btrfs(client): @pytest.mark.no_container -@pytest.mark.user_data( - STORAGE_USER_DATA.format( - "lvm2", "lvm")) +@pytest.mark.user_data(STORAGE_USER_DATA.format("lvm2", "lvm")) def test_storage_lvm(client): validate_storage(client) From ec83781beba1a4f0f365cdc9ed5c2c51e4d61b69 Mon Sep 17 00:00:00 2001 From: Brett Holman Date: Thu, 21 Jul 2022 06:42:48 -0600 Subject: [PATCH 06/24] check package install --- tests/integration_tests/modules/test_lxd.py | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/tests/integration_tests/modules/test_lxd.py b/tests/integration_tests/modules/test_lxd.py index 3faeb62f01d..ed3ab5d00ef 100644 --- a/tests/integration_tests/modules/test_lxd.py +++ b/tests/integration_tests/modules/test_lxd.py @@ -55,26 +55,25 @@ def test_bridge(self, class_client): assert "10.100.100.1/24" == network_config["config"]["ipv4.address"] -def validate_storage(validate_client): - verify_clean_log( - validate_client.read_from_file("/var/log/cloud-init.log"), - ignore_deprecations=False, - ) +def validate_storage(validate_client, pkg_name): + log = validate_client.read_from_file("/var/log/cloud-init.log") + verify_clean_log(log, ignore_deprecations=False) + assert f"install', '{pkg_name}'" in log @pytest.mark.no_container @pytest.mark.user_data(STORAGE_USER_DATA.format("btrfs-progs", "btrfs")) def test_storage_btrfs(client): - validate_storage(client) + validate_storage(client, "btrfs-progs") @pytest.mark.no_container @pytest.mark.user_data(STORAGE_USER_DATA.format("lvm2", "lvm")) def test_storage_lvm(client): - validate_storage(client) + validate_storage(client, "lvm2") @pytest.mark.no_container @pytest.mark.user_data(STORAGE_USER_DATA.format("zfsutils-linux", "zfs")) def test_storage_zfs(client): - validate_storage(client) + validate_storage(client, "zfsutils-linux") From d2e1c5f2f71dc822d6eca6f8cc60fda150812bc5 Mon Sep 17 00:00:00 2001 From: Brett Holman Date: Thu, 21 Jul 2022 20:13:49 -0600 Subject: [PATCH 07/24] test --- tests/integration_tests/modules/test_lxd.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/integration_tests/modules/test_lxd.py b/tests/integration_tests/modules/test_lxd.py index ed3ab5d00ef..ad80ac902bc 100644 --- a/tests/integration_tests/modules/test_lxd.py +++ b/tests/integration_tests/modules/test_lxd.py @@ -27,10 +27,10 @@ STORAGE_USER_DATA = """\ #cloud-config -bootcmd: [ "apt-get --yes remove {}" ] +bootcmd: [ "apt-get --yes remove {0}", "apt-get purge {0}" ] lxd: init: - storage_backend: {} + storage_backend: {1} """ From 18d8ea0b1c7b5fcfc5734806d6393c5057fe4905 Mon Sep 17 00:00:00 2001 From: Brett Holman Date: Thu, 21 Jul 2022 21:39:40 -0600 Subject: [PATCH 08/24] unmask --- tests/integration_tests/modules/test_lxd.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/tests/integration_tests/modules/test_lxd.py b/tests/integration_tests/modules/test_lxd.py index ad80ac902bc..26c0d4ec943 100644 --- a/tests/integration_tests/modules/test_lxd.py +++ b/tests/integration_tests/modules/test_lxd.py @@ -27,7 +27,7 @@ STORAGE_USER_DATA = """\ #cloud-config -bootcmd: [ "apt-get --yes remove {0}", "apt-get purge {0}" ] +bootcmd: [ "apt-get --yes remove {0}", "{2}" ] lxd: init: storage_backend: {1} @@ -62,18 +62,19 @@ def validate_storage(validate_client, pkg_name): @pytest.mark.no_container -@pytest.mark.user_data(STORAGE_USER_DATA.format("btrfs-progs", "btrfs")) +@pytest.mark.user_data(STORAGE_USER_DATA.format("btrfs-progs", "btrfs", "true")) def test_storage_btrfs(client): validate_storage(client, "btrfs-progs") @pytest.mark.no_container -@pytest.mark.user_data(STORAGE_USER_DATA.format("lvm2", "lvm")) +@pytest.mark.user_data(STORAGE_USER_DATA.format( + "lvm2", "lvm", "systemctl unmask lvm2-lvmpolld.socket")) def test_storage_lvm(client): validate_storage(client, "lvm2") @pytest.mark.no_container -@pytest.mark.user_data(STORAGE_USER_DATA.format("zfsutils-linux", "zfs")) +@pytest.mark.user_data(STORAGE_USER_DATA.format("zfsutils-linux", "zfs", "true")) def test_storage_zfs(client): validate_storage(client, "zfsutils-linux") From 34c1da676a7a7c073c828d1d76a9011d43ddfecb Mon Sep 17 00:00:00 2001 From: Brett Holman Date: Mon, 25 Jul 2022 09:10:39 -0600 Subject: [PATCH 09/24] ignore test for a sec --- tests/integration_tests/modules/test_lxd.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/integration_tests/modules/test_lxd.py b/tests/integration_tests/modules/test_lxd.py index 26c0d4ec943..7c1564f8fbf 100644 --- a/tests/integration_tests/modules/test_lxd.py +++ b/tests/integration_tests/modules/test_lxd.py @@ -27,7 +27,7 @@ STORAGE_USER_DATA = """\ #cloud-config -bootcmd: [ "apt-get --yes remove {0}", "{2}" ] +bootcmd: [ "apt-get --yes remove {0}", "true" ] lxd: init: storage_backend: {1} From 224354056c509b17517d3b9ded1b9632d691b09f Mon Sep 17 00:00:00 2001 From: Brett Holman Date: Mon, 25 Jul 2022 12:59:24 -0600 Subject: [PATCH 10/24] Workaround kernel bug --- cloudinit/config/cc_lxd.py | 26 ++++++++++++++++- tests/integration_tests/modules/test_lxd.py | 31 ++++++++++++++------- 2 files changed, 46 insertions(+), 11 deletions(-) diff --git a/cloudinit/config/cc_lxd.py b/cloudinit/config/cc_lxd.py index 167bb75faab..cb50e99161d 100644 --- a/cloudinit/config/cc_lxd.py +++ b/cloudinit/config/cc_lxd.py @@ -7,11 +7,13 @@ """LXD: configure lxd with ``lxd init`` and optionally lxd-bridge""" import os +from logging import Logger from textwrap import dedent from typing import List from cloudinit import log as logging from cloudinit import subp, util +from cloudinit.cloud import Cloud from cloudinit.config.schema import MetaSchema, get_meta_doc from cloudinit.settings import PER_INSTANCE @@ -78,7 +80,7 @@ __doc__ = get_meta_doc(meta) -def handle(name, cfg, cloud, log, args): +def handle(name, cfg, cloud: Cloud, log: Logger, args): # Get config lxd_cfg = cfg.get("lxd") if not lxd_cfg: @@ -127,7 +129,29 @@ def handle(name, cfg, cloud, log, args): "storage_pool", "trust_password", ) + subp.subp(["lxd", "waitready", "--timeout=300"]) + + # Bug https://bugs.launchpad.net/ubuntu/+source/linux-kvm/+bug/1982780 + if ( + cloud.distro.name == "ubuntu" + and init_cfg["storage_backend"] == "lvm" + ): + log.warning( + "cloud-init doesn't use thinpool by default on Ubuntu due to " + "LP 1982780. This behavior will change in the future.", + ) + subp.subp( + [ + "lxc", + "storage", + "create", + "default", + "lvm", + "lvm.use_thinpool=false", + ] + ) + cmd = ["lxd", "init", "--auto"] for k in init_keys: if init_cfg.get(k): diff --git a/tests/integration_tests/modules/test_lxd.py b/tests/integration_tests/modules/test_lxd.py index 7c1564f8fbf..3e8c15c6449 100644 --- a/tests/integration_tests/modules/test_lxd.py +++ b/tests/integration_tests/modules/test_lxd.py @@ -27,7 +27,7 @@ STORAGE_USER_DATA = """\ #cloud-config -bootcmd: [ "apt-get --yes remove {0}", "true" ] +bootcmd: [ "apt-get --yes remove {0}", "! command -v {2}", "{3}" ] lxd: init: storage_backend: {1} @@ -55,26 +55,37 @@ def test_bridge(self, class_client): assert "10.100.100.1/24" == network_config["config"]["ipv4.address"] -def validate_storage(validate_client, pkg_name): +def validate_storage(validate_client, pkg_name, command): log = validate_client.read_from_file("/var/log/cloud-init.log") verify_clean_log(log, ignore_deprecations=False) - assert f"install', '{pkg_name}'" in log + assert validate_client.execute(f"which {command}") @pytest.mark.no_container -@pytest.mark.user_data(STORAGE_USER_DATA.format("btrfs-progs", "btrfs", "true")) +@pytest.mark.user_data( + STORAGE_USER_DATA.format("btrfs-progs", "btrfs", "mkfs.btrfs", "true") +) def test_storage_btrfs(client): - validate_storage(client, "btrfs-progs") + validate_storage(client, "btrfs-progs", "mkfs.btrfs") @pytest.mark.no_container -@pytest.mark.user_data(STORAGE_USER_DATA.format( - "lvm2", "lvm", "systemctl unmask lvm2-lvmpolld.socket")) +@pytest.mark.user_data( + STORAGE_USER_DATA.format( + "lvm2", + "lvm", + "lvcreate", + "apt-get install " + "thin-provisioning-tools && systemctl unmask lvm2-lvmpolld.socket", + ) +) def test_storage_lvm(client): - validate_storage(client, "lvm2") + validate_storage(client, "lvm2", "lvcreate") @pytest.mark.no_container -@pytest.mark.user_data(STORAGE_USER_DATA.format("zfsutils-linux", "zfs", "true")) +@pytest.mark.user_data( + STORAGE_USER_DATA.format("zfsutils-linux", "zfs", "zpool", "true") +) def test_storage_zfs(client): - validate_storage(client, "zfsutils-linux") + validate_storage(client, "zfsutils-linux", "zpool") From 4a075bea30c626827f0ba25ab1465d0ab87ca6f5 Mon Sep 17 00:00:00 2001 From: Brett Holman Date: Mon, 25 Jul 2022 14:30:37 -0600 Subject: [PATCH 11/24] tests --- tests/integration_tests/modules/test_lxd.py | 8 ++++++-- tests/integration_tests/util.py | 4 +++- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/tests/integration_tests/modules/test_lxd.py b/tests/integration_tests/modules/test_lxd.py index 3e8c15c6449..a1b816432e6 100644 --- a/tests/integration_tests/modules/test_lxd.py +++ b/tests/integration_tests/modules/test_lxd.py @@ -5,6 +5,7 @@ """ import pytest import yaml +import warnings from tests.integration_tests.util import verify_clean_log @@ -58,7 +59,8 @@ def test_bridge(self, class_client): def validate_storage(validate_client, pkg_name, command): log = validate_client.read_from_file("/var/log/cloud-init.log") verify_clean_log(log, ignore_deprecations=False) - assert validate_client.execute(f"which {command}") + assert 0 == validate_client.execute(f"which {command}").return_code + return log @pytest.mark.no_container @@ -80,7 +82,9 @@ def test_storage_btrfs(client): ) ) def test_storage_lvm(client): - validate_storage(client, "lvm2", "lvcreate") + log = validate_storage(client, "lvm2", "lvcreate") + if "doesn't use thinpool by default on Ubuntu due to LP" not in log: + warnings.warn("LP 1982780 has been fixed, update to allow thinpools") @pytest.mark.no_container diff --git a/tests/integration_tests/util.py b/tests/integration_tests/util.py index bffda59f3f4..034d882834c 100644 --- a/tests/integration_tests/util.py +++ b/tests/integration_tests/util.py @@ -53,7 +53,9 @@ def verify_clean_log(log: str, ignore_deprecations: bool = True): warning_texts = [ # Consistently on all Azure launches: # azure.py[WARNING]: No lease found; using default endpoint - "No lease found; using default endpoint" + "No lease found; using default endpoint", + # Ubuntu lxd storage + "cloud-init doesn't use thinpool by default on Ubuntu due to LP" ] traceback_texts = [] if "oracle" in log: From baf555355214993c4433926ed485e7ea35d81a3e Mon Sep 17 00:00:00 2001 From: Brett Holman Date: Mon, 25 Jul 2022 14:41:53 -0600 Subject: [PATCH 12/24] tests --- tests/integration_tests/modules/test_lxd.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/integration_tests/modules/test_lxd.py b/tests/integration_tests/modules/test_lxd.py index a1b816432e6..af68c3fd886 100644 --- a/tests/integration_tests/modules/test_lxd.py +++ b/tests/integration_tests/modules/test_lxd.py @@ -59,7 +59,6 @@ def test_bridge(self, class_client): def validate_storage(validate_client, pkg_name, command): log = validate_client.read_from_file("/var/log/cloud-init.log") verify_clean_log(log, ignore_deprecations=False) - assert 0 == validate_client.execute(f"which {command}").return_code return log From b21ec5c7814b18d8702520145c0f516df8a7b656 Mon Sep 17 00:00:00 2001 From: Brett Holman Date: Mon, 25 Jul 2022 22:16:33 -0600 Subject: [PATCH 13/24] don't configure storage twice --- cloudinit/config/cc_lxd.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/cloudinit/config/cc_lxd.py b/cloudinit/config/cc_lxd.py index cb50e99161d..7417192ea34 100644 --- a/cloudinit/config/cc_lxd.py +++ b/cloudinit/config/cc_lxd.py @@ -152,6 +152,11 @@ def handle(name, cfg, cloud: Cloud, log: Logger, args): ] ) + # Since we're manually setting use_thinpool=false + # filter it from the lxd init commands, don't configure + # storage twice + init_cfg.pop("storage_backend") + cmd = ["lxd", "init", "--auto"] for k in init_keys: if init_cfg.get(k): From 2bd230b81cb46e15309ce97b7aeb03fe51a10df3 Mon Sep 17 00:00:00 2001 From: Brett Holman Date: Mon, 25 Jul 2022 22:34:17 -0600 Subject: [PATCH 14/24] pop key --- cloudinit/config/cc_lxd.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cloudinit/config/cc_lxd.py b/cloudinit/config/cc_lxd.py index 7417192ea34..b511356f72a 100644 --- a/cloudinit/config/cc_lxd.py +++ b/cloudinit/config/cc_lxd.py @@ -155,7 +155,7 @@ def handle(name, cfg, cloud: Cloud, log: Logger, args): # Since we're manually setting use_thinpool=false # filter it from the lxd init commands, don't configure # storage twice - init_cfg.pop("storage_backend") + init_keys = tuple(key for key in init_keys if key != "storage_backend") cmd = ["lxd", "init", "--auto"] for k in init_keys: From a6d76a80a73b8866e2c9cf5698106941a179d9d8 Mon Sep 17 00:00:00 2001 From: Brett Holman Date: Mon, 25 Jul 2022 23:12:47 -0600 Subject: [PATCH 15/24] tests --- tests/integration_tests/modules/test_lxd.py | 6 +++++- tests/integration_tests/util.py | 2 +- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/tests/integration_tests/modules/test_lxd.py b/tests/integration_tests/modules/test_lxd.py index af68c3fd886..fa1721236bf 100644 --- a/tests/integration_tests/modules/test_lxd.py +++ b/tests/integration_tests/modules/test_lxd.py @@ -81,10 +81,14 @@ def test_storage_btrfs(client): ) ) def test_storage_lvm(client): - log = validate_storage(client, "lvm2", "lvcreate") + log = client.read_from_file("/var/log/cloud-init.log") + + # Note to self if "doesn't use thinpool by default on Ubuntu due to LP" not in log: warnings.warn("LP 1982780 has been fixed, update to allow thinpools") + validate_storage(client, "btrfs-progs", "mkfs.btrfs") + @pytest.mark.no_container @pytest.mark.user_data( diff --git a/tests/integration_tests/util.py b/tests/integration_tests/util.py index 034d882834c..3c54122943f 100644 --- a/tests/integration_tests/util.py +++ b/tests/integration_tests/util.py @@ -55,7 +55,7 @@ def verify_clean_log(log: str, ignore_deprecations: bool = True): # azure.py[WARNING]: No lease found; using default endpoint "No lease found; using default endpoint", # Ubuntu lxd storage - "cloud-init doesn't use thinpool by default on Ubuntu due to LP" + "thinpool by default on Ubuntu due to LP 1982780", ] traceback_texts = [] if "oracle" in log: From 0aef404df98efdca5bb0640c79ed8ac81d04e809 Mon Sep 17 00:00:00 2001 From: Brett Holman Date: Tue, 26 Jul 2022 07:59:49 -0600 Subject: [PATCH 16/24] typing --- cloudinit/config/cc_lxd.py | 9 +++++++-- tests/integration_tests/modules/test_lxd.py | 3 ++- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/cloudinit/config/cc_lxd.py b/cloudinit/config/cc_lxd.py index b511356f72a..4ed5e8ea2dd 100644 --- a/cloudinit/config/cc_lxd.py +++ b/cloudinit/config/cc_lxd.py @@ -120,7 +120,10 @@ def handle(name, cfg, cloud: Cloud, log: Logger, args): # Set up lxd if init config is given if init_cfg: - init_keys = ( + + # type is known, number of elements is not + # in the case of the ubuntu+lvm backend workaround + init_keys: tuple[str, ...] = ( "network_address", "network_port", "storage_backend", @@ -155,7 +158,9 @@ def handle(name, cfg, cloud: Cloud, log: Logger, args): # Since we're manually setting use_thinpool=false # filter it from the lxd init commands, don't configure # storage twice - init_keys = tuple(key for key in init_keys if key != "storage_backend") + init_keys = tuple( + key for key in init_keys if key != "storage_backend" + ) cmd = ["lxd", "init", "--auto"] for k in init_keys: diff --git a/tests/integration_tests/modules/test_lxd.py b/tests/integration_tests/modules/test_lxd.py index fa1721236bf..baafb28a7a9 100644 --- a/tests/integration_tests/modules/test_lxd.py +++ b/tests/integration_tests/modules/test_lxd.py @@ -3,9 +3,10 @@ (This is ported from ``tests/cloud_tests/testcases/modules/lxd_bridge.yaml``.) """ +import warnings + import pytest import yaml -import warnings from tests.integration_tests.util import verify_clean_log From 7b24279cce17944f917ca550d8a54434cd7fa836 Mon Sep 17 00:00:00 2001 From: Brett Holman Date: Tue, 26 Jul 2022 08:05:16 -0600 Subject: [PATCH 17/24] typing --- cloudinit/config/cc_lxd.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cloudinit/config/cc_lxd.py b/cloudinit/config/cc_lxd.py index 4ed5e8ea2dd..5a82a43cdb3 100644 --- a/cloudinit/config/cc_lxd.py +++ b/cloudinit/config/cc_lxd.py @@ -9,7 +9,7 @@ import os from logging import Logger from textwrap import dedent -from typing import List +from typing import List, Tuple from cloudinit import log as logging from cloudinit import subp, util @@ -123,7 +123,7 @@ def handle(name, cfg, cloud: Cloud, log: Logger, args): # type is known, number of elements is not # in the case of the ubuntu+lvm backend workaround - init_keys: tuple[str, ...] = ( + init_keys: Tuple[str, ...] = ( "network_address", "network_port", "storage_backend", From 866115ccfb4c6c14cf21751811eec10eedc5fed0 Mon Sep 17 00:00:00 2001 From: Brett Holman Date: Wed, 27 Jul 2022 13:37:08 -0600 Subject: [PATCH 18/24] suggestions Co-authored-by: Chad Smith --- cloudinit/config/cc_lxd.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cloudinit/config/cc_lxd.py b/cloudinit/config/cc_lxd.py index 5a82a43cdb3..ce412daaaba 100644 --- a/cloudinit/config/cc_lxd.py +++ b/cloudinit/config/cc_lxd.py @@ -142,7 +142,7 @@ def handle(name, cfg, cloud: Cloud, log: Logger, args): ): log.warning( "cloud-init doesn't use thinpool by default on Ubuntu due to " - "LP 1982780. This behavior will change in the future.", + "LP #1982780. This behavior will change in the future.", ) subp.subp( [ From 62522d7aad573cbbd754b375ad1954d92b6749f1 Mon Sep 17 00:00:00 2001 From: Brett Holman Date: Wed, 27 Jul 2022 14:54:25 -0600 Subject: [PATCH 19/24] change behavior based on the existance of a kmod --- cloudinit/config/cc_lxd.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/cloudinit/config/cc_lxd.py b/cloudinit/config/cc_lxd.py index ce412daaaba..da73386557a 100644 --- a/cloudinit/config/cc_lxd.py +++ b/cloudinit/config/cc_lxd.py @@ -136,8 +136,9 @@ def handle(name, cfg, cloud: Cloud, log: Logger, args): subp.subp(["lxd", "waitready", "--timeout=300"]) # Bug https://bugs.launchpad.net/ubuntu/+source/linux-kvm/+bug/1982780 - if ( - cloud.distro.name == "ubuntu" + kernel = util.system_info()["uname"][2] + if not os.path.exists( + f"/lib/modules/{kernel}/kernel/drivers/md/dm-thin-pool.ko" and init_cfg["storage_backend"] == "lvm" ): log.warning( From e76c87784ddc93568c9186e375408ae61c55e8d7 Mon Sep 17 00:00:00 2001 From: Brett Holman Date: Wed, 27 Jul 2022 16:53:38 -0600 Subject: [PATCH 20/24] less metaphysical, more kmod testing --- cloudinit/config/cc_lxd.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/cloudinit/config/cc_lxd.py b/cloudinit/config/cc_lxd.py index da73386557a..d2edef4b903 100644 --- a/cloudinit/config/cc_lxd.py +++ b/cloudinit/config/cc_lxd.py @@ -137,9 +137,11 @@ 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( - f"/lib/modules/{kernel}/kernel/drivers/md/dm-thin-pool.ko" - and init_cfg["storage_backend"] == "lvm" + if ( + init_cfg["storage_backend"] == "lvm" + and not os.path.exists( + f"/lib/modules/{kernel}/kernel/drivers/md/dm-thin-pool.ko" + ) ): log.warning( "cloud-init doesn't use thinpool by default on Ubuntu due to " From dfca33cbe734a72b3fdb5dfc6faec4f49aa4df8a Mon Sep 17 00:00:00 2001 From: Brett Holman Date: Wed, 27 Jul 2022 16:56:45 -0600 Subject: [PATCH 21/24] fmt --- cloudinit/config/cc_lxd.py | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/cloudinit/config/cc_lxd.py b/cloudinit/config/cc_lxd.py index d2edef4b903..490533c03ac 100644 --- a/cloudinit/config/cc_lxd.py +++ b/cloudinit/config/cc_lxd.py @@ -137,11 +137,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 ( - init_cfg["storage_backend"] == "lvm" - and not os.path.exists( - f"/lib/modules/{kernel}/kernel/drivers/md/dm-thin-pool.ko" - ) + if init_cfg["storage_backend"] == "lvm" and not os.path.exists( + f"/lib/modules/{kernel}/kernel/drivers/md/dm-thin-pool.ko" ): log.warning( "cloud-init doesn't use thinpool by default on Ubuntu due to " From 711905706a88d8fb107cb4fecbe7abb351fe4860 Mon Sep 17 00:00:00 2001 From: Brett Holman Date: Wed, 27 Jul 2022 16:57:45 -0600 Subject: [PATCH 22/24] thanks @blackboxsw --- tests/unittests/config/test_cc_lxd.py | 32 ++++++++++++++++++++------- 1 file changed, 24 insertions(+), 8 deletions(-) diff --git a/tests/unittests/config/test_cc_lxd.py b/tests/unittests/config/test_cc_lxd.py index 20cd1e3f49a..c7b31e88784 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 @@ def test_lxd_init(self, m_maybe_clean, m_which, m_subp): ] ), ], - 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: From 8e7000f99c611e61edb3b15ee152cb8b149216f0 Mon Sep 17 00:00:00 2001 From: Brett Holman Date: Wed, 27 Jul 2022 17:07:41 -0600 Subject: [PATCH 23/24] fmt --- tests/unittests/config/test_cc_lxd.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/unittests/config/test_cc_lxd.py b/tests/unittests/config/test_cc_lxd.py index c7b31e88784..8b75a1f7aa9 100644 --- a/tests/unittests/config/test_cc_lxd.py +++ b/tests/unittests/config/test_cc_lxd.py @@ -85,7 +85,8 @@ def test_lxd_init(self, maybe_clean, which, subp, exists, system_info): self.assertEqual( [ mock.call( - "/lib/modules/mykernel/kernel/drivers/md/dm-thin-pool.ko" + "/lib/modules/mykernel/" + "kernel/drivers/md/dm-thin-pool.ko" ) ], exists.call_args_list, From d28e87e7bc78d53498e1210d0d45e4198972b00f Mon Sep 17 00:00:00 2001 From: Chad Smith Date: Wed, 27 Jul 2022 21:11:47 -0600 Subject: [PATCH 24/24] tests: update verify_clean_log warning text add check for apt install --- tests/integration_tests/modules/test_lxd.py | 4 +++- tests/integration_tests/util.py | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/tests/integration_tests/modules/test_lxd.py b/tests/integration_tests/modules/test_lxd.py index baafb28a7a9..f404542583b 100644 --- a/tests/integration_tests/modules/test_lxd.py +++ b/tests/integration_tests/modules/test_lxd.py @@ -3,6 +3,7 @@ (This is ported from ``tests/cloud_tests/testcases/modules/lxd_bridge.yaml``.) """ +import re import warnings import pytest @@ -59,6 +60,7 @@ def test_bridge(self, class_client): def validate_storage(validate_client, pkg_name, command): log = validate_client.read_from_file("/var/log/cloud-init.log") + assert re.search(f"apt-get.*install.*{pkg_name}", log) is not None verify_clean_log(log, ignore_deprecations=False) return log @@ -88,7 +90,7 @@ def test_storage_lvm(client): if "doesn't use thinpool by default on Ubuntu due to LP" not in log: warnings.warn("LP 1982780 has been fixed, update to allow thinpools") - validate_storage(client, "btrfs-progs", "mkfs.btrfs") + validate_storage(client, "lvm2", "lvcreate") @pytest.mark.no_container diff --git a/tests/integration_tests/util.py b/tests/integration_tests/util.py index 3c54122943f..4eb348f7c5d 100644 --- a/tests/integration_tests/util.py +++ b/tests/integration_tests/util.py @@ -55,7 +55,7 @@ def verify_clean_log(log: str, ignore_deprecations: bool = True): # azure.py[WARNING]: No lease found; using default endpoint "No lease found; using default endpoint", # Ubuntu lxd storage - "thinpool by default on Ubuntu due to LP 1982780", + "thinpool by default on Ubuntu due to LP #1982780", ] traceback_texts = [] if "oracle" in log: