sts-sponsors team mailing list archive

[Jack Lloyd-Walters <mp+439087@xxxxxxxxxxxxxxxxxx>]

Jack Lloyd-Walters has proposed merging ~maas-committers/maas-ci/+git/system-tests:more-ansible-tests into ~maas-committers/maas-ci/+git/system-tests:master.

Commit message:
Introduce new test to update a running MAAS with ansible.
Change default debugging behaviour for ansible-playbook-test

Requested reviews:
  MAAS Lander (maas-lander)
  MAAS Committers (maas-committers)

For more details, see:
https://code.launchpad.net/~maas-committers/maas-ci/+git/system-tests/+merge/439087
-- 
Your team MAAS Committers is requested to review the proposed merge of ~maas-committers/maas-ci/+git/system-tests:more-ansible-tests into ~maas-committers/maas-ci/+git/system-tests:master.

diff --git a/config.yaml.sample b/config.yaml.sample
index e656a27..1167ada 100644
--- a/config.yaml.sample
+++ b/config.yaml.sample
@@ -121,3 +121,4 @@ o11y:
 ansible-playbooks:
     git-repo: https://github.com/maas/maas-ansible-playbook.git
     git-branch: main
+    verbosity:
diff --git a/systemtests/ansible.py b/systemtests/ansible.py
index 8db10f9..c55b922 100644
--- a/systemtests/ansible.py
+++ b/systemtests/ansible.py
@@ -18,6 +18,7 @@ if TYPE_CHECKING:
     from logging import Logger
 
     from .lxd import CLILXD, _FileWrapper
+
 NAME = "systemtests.ansible"
 LOG = getLogger(NAME)
 
@@ -134,6 +135,7 @@ class AnsibleHost:
         self._image = image
         self.config: dict[str, str] = {"ansible_user": "ubuntu"}
         self.roles: dict[str, dict[str, str]] = {}
+        self.keepalive = False
 
         LOG.info(f"Created {self}")
 
@@ -148,8 +150,8 @@ class AnsibleHost:
     def ip(self) -> str:
         return self.instance.get_ip_address()
 
-    def get_or_create_instance(self, public_ssh_key: str) -> None:
-        self.instance.create_container(
+    def get_or_create_instance(self, public_ssh_key: str) -> Instance:
+        return self.instance.create_container(
             self._image,
             f"ssh_authorized_keys:\n- {public_ssh_key}",
         )
@@ -272,6 +274,40 @@ class AnsibleHost:
         self._remove_role("maas_proxy")
 
 
+class MAASRegionAsAnsibleHost(AnsibleHost):
+    def __init__(self, maas_api_client: AuthenticatedAPIClient) -> None:
+        self.client = maas_api_client
+        self.keepalive = True
+
+        self.api = self.client.api_client
+        self.instance = self.api.maas_container
+        self.config: dict[str, str] = {"ansible_user": "ubuntu"}
+        self.roles: dict[str, dict[str, str]] = {}
+
+        self.add_region_rack()
+
+        self.update_config({"maas_url": self.api.url, "maas_installation_type": "snap"})
+
+        LOG.info(f"Enrolled {self}")
+
+    @property
+    def version(self) -> str:
+        return str(self.client.read_version_information()["version"])
+
+    def get_or_create_instance(self, public_ssh_key: str) -> Instance:
+        return self.instance
+
+    def refresh(self) -> None:
+        self.client.execute(["refresh"])
+
+    def restart(self) -> None:
+        if self.fetch_config("maas_installation_type") == "snap":
+            cmd = ["snap", "restart", "maas"]
+        else:
+            cmd = ["systemctl", "restart", "maas-regiond"]
+        self.client.api_client.execute(cmd, ["sudo", "-u", "ubuntu"])
+
+
 class AnsibleMain:
     use_timeout = True
     timeout = timedelta(minutes=10)
@@ -284,6 +320,7 @@ class AnsibleMain:
         playbooks_repo: str,
         playbooks_branch: str,
         proxy_env: Optional[dict[str, str]],
+        debug: str = "",
     ) -> None:
         self._lxd = lxd
         self.instance = instance
@@ -297,6 +334,7 @@ class AnsibleMain:
         self._inventory_: set[AnsibleHost] = set()
 
         self.ansible_repo_path = "/home/ubuntu/ansible_repo"
+        self.default_debug = debug
 
     def setup(self) -> None:
         self.logger.info("Installing python3-pip")
@@ -425,6 +463,14 @@ class AnsibleMain:
             return f"{name_scheme}-{min(set(range(1, max(nums)+2)) - nums)}"
         return f"{name_scheme}-1"
 
+    def add_running_host(
+        self, maas_api_client: AuthenticatedAPIClient, keep_alive: bool = True
+    ) -> MAASRegionAsAnsibleHost:
+        host = MAASRegionAsAnsibleHost(maas_api_client)
+        host.keepalive = keep_alive
+        self._inventory_.add(host)
+        return host
+
     def add_host(self, image: str = "ubuntu:20.04") -> AnsibleHost:
         name = self._make_host_name_()
         instance = Instance(self._lxd, name)
@@ -438,7 +484,8 @@ class AnsibleMain:
         self._inventory_.difference_update(hosts)
 
     def _remove_host(self, host: AnsibleHost) -> None:
-        host.instance.delete()
+        if not host.keepalive:
+            host.instance.delete()
 
     def update_config(self, config: dict[str, str]) -> dict[str, str]:
         self.config |= config
@@ -456,6 +503,10 @@ class AnsibleMain:
     def fetch_region(
         self, host: AnsibleHost, user: str = "admin"
     ) -> AuthenticatedAPIClient:
+        if isinstance(host, MAASRegionAsAnsibleHost):
+            host.refresh()
+            host.restart()
+            return host.client
         if host.has_region:
             url = self.config.get(
                 "maas_url", host.fetch_config("maas_url", "maas_region_controller")
@@ -504,7 +555,10 @@ class AnsibleMain:
                 ]
             )
         file_content = "\n".join(inv)
-        LOG.info(f"Ansible hosts file generated:\n{file_content}")
+        if self.default_debug:
+            self.logger.info("Ansible hosts file generated:")
+            for line in file_content:
+                self.logger.info(line)
         self._hosts_file.write(file_content)
 
     def create_config_file(self) -> None:
@@ -522,6 +576,11 @@ class AnsibleMain:
         append_if_not_found("host_key_checking = False", ansible_cfg)
         append_if_not_found("remote_user = ubuntu", ansible_cfg)
         append_if_not_found("deprecation_warnings = False", ansible_cfg)
+        append_if_not_found("callback_result_format = yaml", ansible_cfg)
+        append_if_not_found("timeout = 60", ansible_cfg)
+        # don't show taks that weren't executed, or succeeded without changing anything
+        append_if_not_found("display_skipped_hosts = False", ansible_cfg)
+        append_if_not_found("display_ok_hosts = False", ansible_cfg)
         if self.use_timeout:
             append_if_not_found(
                 f"task_timeout = {int(self.timeout.total_seconds())}", ansible_cfg
@@ -530,7 +589,7 @@ class AnsibleMain:
         etc_ansible_cfg = self.instance.files["/etc/ansible/ansible.cfg"]
         etc_ansible_cfg.write(ansible_cfg.read())
 
-    def run_playbook(self, playbook: str = "site.yaml", debug: str = "-v") -> None:
+    def run_playbook(self, playbook: str = "site.yaml", debug: str = "") -> None:
         self.create_hosts_file()
         cmd: list[str] = [
             "eatmydata",
@@ -541,6 +600,6 @@ class AnsibleMain:
             "--private-key",
             self.ssh_key_file[:-4],
         ]
-        if _debug := re.match(r"-(v)+", debug):
+        if _debug := re.match(r"-(v)+", debug or self.default_debug):
             cmd.append(str(_debug.group()))
         self.instance.execute(cmd, environment=self._proxy_env)
diff --git a/systemtests/ansible_tests/test_ansible.py b/systemtests/ansible_tests/test_ansible.py
index 0b89249..4360f82 100644
--- a/systemtests/ansible_tests/test_ansible.py
+++ b/systemtests/ansible_tests/test_ansible.py
@@ -9,6 +9,10 @@ from systemtests.ansible import AnsibleMain, pip_package_exists
 if TYPE_CHECKING:
     from logging import Logger
 
+    from systemtests.api import AuthenticatedAPIClient
+
+DEFAULT_MAAS_VERSION = "3.3"
+
 
 @pytest.mark.skip_if_ansible_playbooks_unconfigured(
     "Needs Ansible playbook configuration"
@@ -62,10 +66,10 @@ class TestAnsibleMAAS:
                     "maas_installation_type": installation_type,
                     "maas_postgres_password": "sekret",
                     "maas_url": f"http://{regionrack_host.ip}:5240/MAAS";,
-                    "maas_version": "3.3",
+                    "maas_version": DEFAULT_MAAS_VERSION,
                 }
             )
-            ansible_main.run_playbook("site.yaml", "-vv")
+            ansible_main.run_playbook("site.yaml")
             region = ansible_main.fetch_region(regionrack_host)
             assert region.read_version_information()["version"][:3] == "3.3"
         assert not ansible_main._inventory_
@@ -77,9 +81,9 @@ class TestAnsibleMAAS:
     ) -> None:
         ansible_main.logger = testlog
         start_version = "3.1"
-        upgrade_version = "3.3"
+        upgrade_version = DEFAULT_MAAS_VERSION
         database = ansible_main.add_host().add_postgres_primary()
-        host = ansible_main.add_host().add_region_rack()
+        host = ansible_main.add_host(image="ubuntu:22.04").add_region_rack()
 
         with ansible_main.collect_inventory():
             ansible_main.update_config(
@@ -90,14 +94,43 @@ class TestAnsibleMAAS:
                     "maas_version": start_version,
                 }
             )
-            ansible_main.run_playbook("site.yaml", "-vvv")
+            ansible_main.run_playbook("site.yaml")
             region = ansible_main.fetch_region(host)
             assert region.read_version_information()["version"][:3] == start_version
 
             ansible_main.update_config({"maas_version": upgrade_version})
-            ansible_main.run_playbook("site.yaml", "-vvv")
+            ansible_main.run_playbook("site.yaml")
+            region = ansible_main.fetch_region(host)
             region.refresh()
             assert region.read_version_information()["version"][:3] == upgrade_version
         assert not ansible_main._inventory_
         assert not host.instance.exists()
         assert not database.instance.exists()
+
+@pytest.mark.usefixtures("configured_maas")
+class TestAnsibleWithRunningMAAS:
+    def test_pre_running_maas_region_is_updated(
+        self,
+        testlog: Logger,
+        ansible_main: AnsibleMain,
+        maas_api_client: AuthenticatedAPIClient,
+    ) -> None:
+        ansible_main.logger = testlog
+        # We don't want to persist this MAAS after the playbooks are complete.
+        host = ansible_main.add_running_host(maas_api_client, keep_alive=False)
+        upgrade_version = "3.2"
+        with ansible_main.collect_inventory():
+            ansible_main.update_config(
+                {
+                    "maas_url": host.fetch_config("maas_url"),
+                    "maas_version": upgrade_version,
+                    "maas_postgres_password": "sekret",
+                }
+            )
+
+            ansible_main.run_playbook("site.yaml")
+            assert ansible_main.fetch_region(host) is maas_api_client
+            region = ansible_main.fetch_region(host)
+            assert region.read_version_information()["version"][:3] == upgrade_version
+        assert not ansible_main._inventory_
+        assert not host.instance.exists()
diff --git a/systemtests/fixtures.py b/systemtests/fixtures.py
index f035b4e..ce4c5cd 100644
--- a/systemtests/fixtures.py
+++ b/systemtests/fixtures.py
@@ -61,6 +61,7 @@ def ansible_main(config: dict[str, Any]) -> Optional[Iterator[AnsibleMain]]:
         playbooks_repo=playbooks_repo,
         playbooks_branch=playbooks_branch,
         proxy_env=proxy_env,
+        debug=playbooks_config.get("verbosity", ""),
     )
     main.setup()
     yield main
@@ -79,7 +80,7 @@ def maas_from_ansible(ansible_main: AnsibleMain) -> Iterator[AuthenticatedAPICli
                 "maas_url": f"http://{host.ip}:5240/MAAS";,
             }
         )
-        ansible_main.run_playbook("site.yaml", "-vv")
+        ansible_main.run_playbook("site.yaml")
         yield ansible_main.fetch_region(host)
 
 
diff --git a/systemtests/region.py b/systemtests/region.py
index 4b979b6..15a18e2 100644
--- a/systemtests/region.py
+++ b/systemtests/region.py
@@ -42,6 +42,10 @@ class MAASRegion:
         result = self.execute(["maas", "apikey", "--username", user])
         return result.stdout.rstrip("\n")
 
+    def refresh(self) -> str:
+        result = self.execute(["maas", "refresh"])
+        return result.stdout.rstrip("\n")
+
     def user_exists(self, user: str) -> bool:
         try:
             self.execute(["maas", "apikey", "--username", user])