canonical-ubuntu-qa team mailing list archive

[Tim Andersson <mp+467026@xxxxxxxxxxxxxxxxxx>]

Tim Andersson has proposed merging ~andersson123/autopkgtest-cloud:restart-worker-on-charm-update into autopkgtest-cloud:master.

Commit message:
Commit #1:

fix: worker: restart autopkgtest.target when worker code has changed

We've, up until this point, had the finnicky issue of requiring the
autopkgtest admins to manually restart the autopkgtest.target systemd
target upon a charm update.

This is problematic as it's something we could easily miss/forget, and
also requires us to discuss whether a restart of the services is
necessary when we're deciding whether or not to update the charms.

This commit utilises the `any_file_changed` function from
`charms.reactive.helpers`.

The reactive part of the cloud worker charm now checks to see if there's
been any changes to the worker code, and if so, sets the
`autopkgtest.target-restart-needed` flag, restarting autopkgtest.target.

Commit #2:
fix: worker: restart autopkgtest.target on worker conf file change

I noticed this whilst working on the previous commit.

The autopkgtest-reload isn't appropriate after a worker.conf file
change.

The config file is loaded by the worker executable via the command line,
and reloading a systemd unit (see `reload` under [1]) only reloads the
environmental variables for the unit, and any other service-specific
configuration.

If the worker script is already running, which in most cases it would
be, the worker conf file is already loaded into the executable. Only by
restarting the unit will the changes to the worker conf file be taken
into effect.

It's important to reiterate that the worker conf is not considered part
of the service config for systemd, since it's simply passed to the
worker via the command line.

In this instance, a restart is necessary. With this change, when a
worker config file is modified by the reactive charm, the
autopkgtest.target will be restarted.

I considered also adding the list of worker files to the
`any_file_changed` check, like for the worker code, but I thought we
may, at some point, cowboy the worker conf files, and we don't want to
restart the whole autopkgtest.target in this case. We will only want to
restart the services for that specific region/arch combo.

[1] https://manpages.debian.org/stretch/systemd/systemctl.1.en.html#Unit_Commands

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

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

Having to manually restart the autopkgtest.target on charm update is a pain, and something that can easily be missed, and is easily automated.

This MP fulfills this, and also restarts the autopkgtest.target in the event that we modify the worker conf files (via juju config).
-- 
Your team Canonical's Ubuntu QA is requested to review the proposed merge of ~andersson123/autopkgtest-cloud:restart-worker-on-charm-update into autopkgtest-cloud:master.

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 ef46a0d..dc53ad2 100644
--- a/charms/focal/autopkgtest-cloud-worker/reactive/autopkgtest_cloud_worker.py
+++ b/charms/focal/autopkgtest-cloud-worker/reactive/autopkgtest_cloud_worker.py
@@ -28,6 +28,7 @@ from charms.reactive import (
     when_not,
     when_not_all,
 )
+from charms.reactive.helpers import any_file_changed
 from utils import UnixUser, install_autodep8
 
 AUTOPKGTEST_LOCATION = os.path.expanduser("~ubuntu/autopkgtest")
@@ -176,6 +177,22 @@ def set_up_systemd_units():
     set_flag("autopkgtest.systemd_units_linked_and_enabled")
 
 
+@when_all(
+    "autopkgtest.autopkgtest_cloud_symlinked",
+    "autopkgtest.rabbitmq-configured",
+    "autopkgtest.influx-creds-written",
+)
+def check_if_worker_code_has_changed():
+    status.maintenance("Checking for changes to the worker code...")
+    if any_file_changed([AUTOPKGTEST_CLOUD_LOCATION + "/worker/worker"]):
+        status.maintenance(
+            "worker code has changed, restarting autopkgtest.target"
+        )
+        set_flag("autopkgtest.target-restart-needed")
+    else:
+        status.maintenance("worker code has not changed.")
+
+
 @when("autopkgtest.systemd_units_linked_and_enabled")
 @when_not("autopkgtest.target_running")
 def start():