canonical-ubuntu-qa team mailing list archive

[Tim Andersson <mp+468066@xxxxxxxxxxxxxxxxxx>]

Tim Andersson has proposed merging ~andersson123/autopkgtest-cloud:flexible-net-names into autopkgtest-cloud:master.

Commit message:
feat: cloud: make network names flexible for dc/arch combinations

Much like the recent change to the flavor config, this commit introduces
a mechanism to have specific network names for each datacentre/arch
combination.

It introduces a new config variable to the autopkgtest-cloud-worker
charm, worker-net-names, which is a string with yaml inside of it, the
same as the flavor config.

The values in the yaml are inserted into the
worker-$datacentre-$arch.conf file, as the net-name juju config option
used to be.

Having a specific datacentre/arch network name isn't required as a
default is specified, which is
net_$instance-proposed-migration, where instance is either "prod" or
"stg".

This commit also refactors part of the `write_net_names` function to use
pathlib instead of writing to a file using `with open`.

Requested reviews:
  Canonical's Ubuntu QA (canonical-ubuntu-qa)

For more details, see:
https://code.launchpad.net/~andersson123/autopkgtest-cloud/+git/autopkgtest-cloud/+merge/468066

This MP makes the network names flexible much like the flavors. In PS6 IS don't want to rename the networks, as to not make it confusing for them (understandably), so this workaround means we can have different network names.
-- 
Your team Canonical's Ubuntu QA is requested to review the proposed merge of ~andersson123/autopkgtest-cloud:flexible-net-names into autopkgtest-cloud:master.

diff --git a/charms/focal/autopkgtest-cloud-worker/config.yaml b/charms/focal/autopkgtest-cloud-worker/config.yaml
index 4cc5333..17dc182 100644
--- a/charms/focal/autopkgtest-cloud-worker/config.yaml
+++ b/charms/focal/autopkgtest-cloud-worker/config.yaml
@@ -61,10 +61,10 @@ options:
     type: string
     description: "the archive mirror to use for tests"
     default: "http://archive.ubuntu.com/ubuntu/";
-  net-name:
+  worker-net-names:
     type: string
-    description: "the neutron network name to use for the SSH/nova \
-                  runner (must have port 22 inbound open)"
+    description: "Network names for datacentre/arch combinations with a \
+                  default (must have port 22 inbound open)"
   worker-setup-command:
     type: string
     description: "autopkgtest's --setup-command argument"
diff --git a/charms/focal/autopkgtest-cloud-worker/reactive/autopkgtest_cloud_worker.py b/charms/focal/autopkgtest-cloud-worker/reactive/autopkgtest_cloud_worker.py
index 2a411d9..4e73fd2 100644
--- a/charms/focal/autopkgtest-cloud-worker/reactive/autopkgtest_cloud_worker.py
+++ b/charms/focal/autopkgtest-cloud-worker/reactive/autopkgtest_cloud_worker.py
@@ -511,7 +511,7 @@ def write_swift_config():
     "config.changed.n-workers",
     "config.changed.lxd-remotes",
     "config.changed.mirror",
-    "config.changed.net-name",
+    "config.changed.worker-net-names",
     "config.changed.worker-upstream-percentage",
     "config.changed.stable-release-percentage",
 )
@@ -525,6 +525,7 @@ def write_worker_config():
     nworkers = config().get("n-workers") or ""
     lxdremotes = config().get("lxd-remotes") or ""
     flavorconfig = config().get("worker-flavor-config") or ""
+    net_names = config().get("worker-net-names") or ""
 
     if not nworkers and not lxdremotes:
         return
@@ -532,6 +533,7 @@ def write_worker_config():
     nworkers_yaml = yaml.load(nworkers, Loader=yaml.CSafeLoader) or {}
     lxdremotes_yaml = yaml.load(lxdremotes, Loader=yaml.CSafeLoader) or {}
     flavorconfig_yaml = yaml.load(flavorconfig, Loader=yaml.CSafeLoader) or {}
+    net_names_yaml = yaml.load(net_names, Loader=yaml.CSafeLoader) or {}
 
     conf = {
         "autopkgtest": {
@@ -552,13 +554,15 @@ def write_worker_config():
         },
     }
 
-    def subst(s):
+    def subst(s, region, arch):
         replacements = {
             "/CHECKOUTDIR/": AUTOPKGTEST_LOCATION,
             "/HOSTNAME/": socket.gethostname(),
             "/AUTOPKGTEST_CLOUD_DIR/": AUTOPKGTEST_CLOUD_LOCATION,
             "/MIRROR/": config().get("mirror"),
-            "/NET_NAME/": config().get("net-name"),
+            "/NET_NAME/": net_names_yaml.get(region, {}).get(
+                arch, net_names_yaml.get("default", "")
+            ),
         }
 
         for k in replacements:  # pylint: disable=consider-using-dict-items
@@ -568,16 +572,10 @@ def write_worker_config():
 
         return s
 
-    # prune anything which wasn't set (nested dict)
-    conf = {
-        k: {l: subst(w) for l, w in v.items() if w is not None}
-        for k, v in conf.items()
-    }
-
     def write(conf_file):
         with open(conf_file, "w") as cf:
             cp = configparser.ConfigParser()
-            cp.read_dict(conf)
+            cp.read_dict(region_arch_conf)
             cp.write(cf)
 
         # FIXME: Hotfix for bos01
@@ -594,29 +592,38 @@ def write_worker_config():
 
     for region in nworkers_yaml:
         for arch in nworkers_yaml[region]:
+            # prune anything which wasn't set (nested dict)
+            region_arch_conf = {
+                k: {
+                    l: subst(w, region, arch)
+                    for l, w in v.items()
+                    if w is not None
+                }
+                for k, v in conf.items()
+            }
             try:
                 flavor = flavorconfig_yaml[region][arch]["default"]
             except KeyError:
                 # No explicit flavor set for this region+arch combination.
                 flavor = flavorconfig_yaml["default"]
-            conf["virt"]["package_size_default"] = flavor
+            region_arch_conf["virt"]["package_size_default"] = flavor
             try:
                 flavor = flavorconfig_yaml[region][arch]["big"]
             except KeyError:
                 flavor = flavorconfig_yaml["big"]
-            conf["virt"]["package_size_big"] = flavor
+            region_arch_conf["virt"]["package_size_big"] = flavor
             if arch == "amd64":
                 conf_file = os.path.join(
                     os.path.expanduser("~ubuntu"),
                     "worker-{}.conf".format(region),
                 )
-                conf["autopkgtest"]["architectures"] = "amd64 i386"
+                region_arch_conf["autopkgtest"]["architectures"] = "amd64 i386"
             else:
                 conf_file = os.path.join(
                     os.path.expanduser("~ubuntu"),
                     "worker-{}-{}.conf".format(region, arch),
                 )
-                conf["autopkgtest"]["architectures"] = arch
+                region_arch_conf["autopkgtest"]["architectures"] = arch
             write(conf_file)
 
     for arch in lxdremotes_yaml:
@@ -631,12 +638,24 @@ def write_worker_config():
     set_flag("autopkgtest.reload-needed")
 
 
-@when("config.changed.net-name")
-def write_net_name():
+@when("config.changed.worker-net-names")
+def write_net_names():
     status.maintenance("Writing net-name configuration")
+    net_names = config().get("worker-net-names") or ""
+    net_names_yaml = yaml.load(net_names, Loader=yaml.CSafeLoader) or {}
     clear_flag("autopkgtest.net-name-written")
-    with open(os.path.expanduser("~ubuntu/net-name.rc"), "w") as f:
-        f.write('NET_NAME="{}"\n'.format(config().get("net-name")))
+    net_name_file = Path("/home/ubuntu/net-name.rc")
+    net_name_file.write_text(
+        'NET_NAME="{}"\n'.format(net_names_yaml.get("default"))
+    )
+    for region in net_names_yaml:
+        if region == "default":
+            continue
+        for arch in net_names_yaml[region]:
+            net_name_file = Path(f"/home/ubuntu/net-name-{region}-{arch}.rc")
+            net_name_file.write_text(
+                'NET_NAME="{}"\n'.format(net_names_yaml[region][arch])
+            )
     status.maintenance("Done writing net-name configuration")
     set_flag("autopkgtest.net-name-written")
     set_flag("autopkgtest.reload-needed")
diff --git a/mojo/service-bundle b/mojo/service-bundle
index ae507f8..3a7f609 100644
--- a/mojo/service-bundle
+++ b/mojo/service-bundle
@@ -78,7 +78,6 @@ applications:
             worker-upstream-percentage: 33
             stable-release-percentage: 100
 {%- if stage_name == "production" %}
-            net-name: net_prod-proposed-migration
             worker-flavor-config: |-
               default: autopkgtest
               big: autopkgtest-big
@@ -89,6 +88,10 @@ applications:
                 s390x:
                   default: autopkgtest-s390x
                   big: autopkgtest-big-s390x
+            worker-net-names: |-
+              default: net_prod-proposed-migration
+              bos03:
+                ppc64el: net_prod-proposed-migration-ppc64el
 {%- elif stage_name == "staging" %}
             net-name: net_stg-proposed-migration-environment
             worker-flavor-config: |-
@@ -98,6 +101,10 @@ applications:
                 ppc64el:
                   default: builder-ppc64el-cpu2-ram4-disk20
                   big: builder-ppc64el-cpu2-ram4-disk100
+            worker-net-names: |-
+              default: net_stg-proposed-migration
+              bos03:
+                ppc64el: net_stg-proposed-migration-ppc64el
 {%- elif stage_name == "devel" %}
             net-name: net_instances
             worker-flavor-config: |-