launchpad-reviewers team mailing list archive

[Tushar Gupta <mp+478388@xxxxxxxxxxxxxxxxxx>]

Tushar Gupta has proposed merging ~tushar5526/lpbuildbot-worker:add-support-for-flavors-in-image-cleanup-logic into lpbuildbot-worker:main.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~tushar5526/lpbuildbot-worker/+git/lpbuildbot-worker/+merge/478388
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~tushar5526/lpbuildbot-worker:add-support-for-flavors-in-image-cleanup-logic into lpbuildbot-worker:main.

diff --git a/charm/src/charm.py b/charm/src/charm.py
index e3dc5de..dd74bc1 100755
--- a/charm/src/charm.py
+++ b/charm/src/charm.py
@@ -273,20 +273,33 @@ class LPBuildbotWorkerCharm(CharmBase):
             )
             subprocess.run(["service", service_name, "stop"], check=True)
 
+        self._cleanup_images(ubuntu_series, current_images)
+
+        self._stored.ubuntu_series = ubuntu_series
+
+    def _cleanup_images(self, ubuntu_series, current_images):
+        """
+        Delete vanilla and flavor images for disabled series
+        """
         for image in current_images:
             if not image.startswith("lptests-"):
                 continue
-            series = image[len("lptests-") :]
-            if series not in ubuntu_series:
+
+            # tests images are named as
+            # lptests-{series}, lptests-{series}-{flavor}
+            image_series = image.split("-", 2)[1]
+
+            if image_series not in ubuntu_series:
                 self._set_maintenance_step(
-                    "Deleting obsolete worker for {}".format(series)
+                    "Deleting obsolete worker image: {} as {} \
+is disabled".format(
+                        image, image_series
+                    )
                 )
                 self._run_as_buildbot(
                     ["lxc", "image", "delete", image], check=True
                 )
 
-        self._stored.ubuntu_series = ubuntu_series
-
     def _setup_launchpad_working_dir(self, series, workers_path, flavor=""):
         work_dir_name = "{}{}".format(series, "-" + flavor if flavor else "")
         self._set_maintenance_step(
diff --git a/charm/tests/test_charm.py b/charm/tests/test_charm.py
index 8ed21be..5937bc8 100644
--- a/charm/tests/test_charm.py
+++ b/charm/tests/test_charm.py
@@ -469,7 +469,8 @@ def test_make_workers_deletes_obsolete_workers(harness, fs, fp, fake_user):
     harness.charm._make_workers()
 
     assert harness.model.unit.status == MaintenanceStatus(
-        "Deleting obsolete worker for precise"
+        "Deleting obsolete worker image: lptests-precise \
+as precise is disabled"
     )
     assert [
         "sudo",
@@ -526,3 +527,101 @@ def test_config_changed(harness, fs, fp, fake_user):
         Path("/var/lib/buildbot/slaves/buildbot.tac").read_text().splitlines()
     )
     assert "password = 'another-secret'" in buildbot_tac
+
+
+def test_only_worker_images_for_inactive_series_are_deleted(
+    harness, fs, fp, fake_user
+):
+    fs.add_real_directory(harness.charm.charm_dir / "templates")
+    fp.keep_last_process(True)
+    fp.register(
+        ["sudo", "-Hu", "buildbot", "lxc", "image", "list", "-f", "json"],
+        stdout=json.dumps(
+            [
+                {"aliases": [{"name": "lptests-focal-postgresql16"}]},
+                {"aliases": [{"name": "lptests-focal"}]},
+                {"aliases": [{"name": "lptests-random-other-image"}]},
+                {"aliases": [{"name": "lptests-xenial-postgresql16"}]},
+                {"aliases": [{"name": "lptests-xenial"}]},
+            ]
+        ),
+    )
+    fp.register([fp.any()])
+    harness._update_config(
+        {
+            "manager-host": "manager.example.com",
+            "buildbot-password": "secret",
+            "ubuntu-series": "focal",
+            # focal-postgresql14 should be allowed as focal
+            # series is enabled. xenial-postgresql14 should be
+            # skipped
+            "extra-series-flavors": "focal-postgresql14 xenial-postgresql14",
+        }
+    )
+
+    harness.charm._make_workers()
+
+    assert harness.model.unit.status == MaintenanceStatus(
+        "Deleting obsolete worker image: lptests-xenial as \
+xenial is disabled"
+    )
+
+    assert all(
+        [
+            [
+                "sudo",
+                "-Hu",
+                "buildbot",
+                "lxc",
+                "image",
+                "delete",
+                "lptests-random-other-image",
+            ]
+            in fp.calls,
+            [
+                "sudo",
+                "-Hu",
+                "buildbot",
+                "lxc",
+                "image",
+                "delete",
+                "lptests-xenial-postgresql16",
+            ]
+            in fp.calls,
+            [
+                "sudo",
+                "-Hu",
+                "buildbot",
+                "lxc",
+                "image",
+                "delete",
+                "lptests-xenial",
+            ]
+            in fp.calls,
+        ]
+    )
+
+    assert all(
+        [
+            [
+                "sudo",
+                "-Hu",
+                "buildbot",
+                "lxc",
+                "image",
+                "delete",
+                "lptests-focal-postgresql16",
+            ]
+            not in fp.calls,
+            [
+                "sudo",
+                "-Hu",
+                "buildbot",
+                "lxc",
+                "image",
+                "delete",
+                "lptests-focal",
+            ]
+            not in fp.calls,
+        ]
+    )