sts-sponsors team mailing list archive

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

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

Commit message:
Introduce performance improvements in the ansible systemtests

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

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

diff --git a/systemtests/ansible.py b/systemtests/ansible.py
index 7c671c8..8d43c66 100644
--- a/systemtests/ansible.py
+++ b/systemtests/ansible.py
@@ -21,6 +21,9 @@ if TYPE_CHECKING:
 NAME = "systemtests.ansible"
 LOG = getLogger(NAME)
 
+# match a string "[test_text]" without including the brackets
+MATCH_HEADERS = r"(?<=\[)[a-zA-Z]+(?=\])"
+
 
 class MissingRoleOnHost(Exception):
     """Raised when a host is missing a role they were expected to have."""
@@ -140,11 +143,11 @@ class AnsibleHost:
     def __repr__(self) -> str:
         return f"<AnsibleHost in {self.instance}>"
 
-    @property
+    @cached_property
     def name(self) -> str:
         return self.instance.name
 
-    @property
+    @cached_property
     def ip(self) -> str:
         return self.instance.get_ip_address()
 
@@ -515,28 +518,57 @@ class AnsibleMain:
 
     def create_config_file(self) -> None:
         cfg_dict = {
-            "host_key_checking": "False",
-            "remote_user": "ubuntu",
-            "deprecation_warnings": "False",
-            "callback_result_format": "yaml",
-            "timeout": "60",
-            "display_skipped_hosts": "False",
-            "display_ok_hosts": "False",
+            "defaults": {
+                "callback_result_format": "yaml",
+                "deprecation_warnings": "False",
+                "display_ok_hosts": "False",
+                "display_skipped_hosts": "False",
+                "host_key_checking": "False",
+                "pipelining": "True",
+                "remote_user": "ubuntu",
+                "timeout": "60",
+            },
+            "ssh_connection": {
+                "ssh_args": "-o ControlMaster=auto -o ControlPersist=60s"
+            },
         }
+        if self.use_timeout:
+            cfg_dict["defaults"][
+                "task_timeout"
+            ] = f"{int(self.timeout.total_seconds())}"
 
         path = f"{self.ansible_repo_path}/ansible.cfg"
         ansible_cfg = self.instance.files[path]
-        if not ansible_cfg.exists():
-            ansible_cfg.write("[defaults]\ninventory = hosts\n")
-        ansible_cfg_content = ansible_cfg.read().split("\n")
-
-        if self.use_timeout:
-            cfg_dict["task_timeout"] = f"{int(self.timeout.total_seconds())}"
-        for k, v in cfg_dict.items():
-            if k not in ansible_cfg_content:
-                ansible_cfg_content.append(f"{k} = {v}")
-
-        ansible_cfg.write("\n".join(ansible_cfg_content))
+        _found_cfg_dict = {"defaults": {"inventory": "hosts"}}
+        if ansible_cfg.exists():
+            ansible_cfg_content = ansible_cfg.read()
+            # find all sections denoted by "[<title>]"
+            if cfg_heads := re.findall(MATCH_HEADERS, ansible_cfg_content):
+                head_pos = [ansible_cfg_content.find(heading) for heading in cfg_heads]
+                head_pos.append(-1)
+                # fetch all of the key, values under each column
+                for key, start, end in zip(cfg_heads, head_pos[:-1], head_pos[1:]):
+                    if key not in _found_cfg_dict.keys():
+                        _found_cfg_dict[key] = {}
+                    for entries in ansible_cfg_content[start:end].split("\n")[1:]:
+                        self.logger.info(entries)
+                        k, v = entries.split("=")
+                        _found_cfg_dict[key.strip("[]")][k.strip()] = v.strip()
+        for key in cfg_dict.keys():
+            if key in _found_cfg_dict:
+                _found_cfg_dict[key].update(cfg_dict[key])
+            else:
+                _found_cfg_dict[key] = cfg_dict[key]
+
+        # reconstruct the file format from the dictionary
+        new_cfg_content = []
+        for new_head, new_entries in _found_cfg_dict.items():
+            new_cfg_content.append(f"[{new_head}]")
+            for new_key, new_value in new_entries.items():
+                new_cfg_content.append(f"{new_key} = {new_value}")
+
+        # and write back to the cfg file
+        ansible_cfg.write("\n".join(new_cfg_content))
         self.instance.execute(["mkdir", "-p", "/etc/ansible"])
         etc_ansible_cfg = self.instance.files["/etc/ansible/ansible.cfg"]
         etc_ansible_cfg.write(ansible_cfg.read())