Merge ~andersson123/autopkgtest-cloud:worker-dont-remove-queue-item-systemctl-restart into autopkgtest-cloud:master

[Tim Andersson]

[Tim Andersson (andersson123)]

Needs a delineation for systemctl stop and systemctl restart

moved to WIP

[Tim Andersson (andersson123)]

with restart:
May 21 10:46:50 juju-113c58-stg-proposed-migration-environment-94 /home/ubuntu/autopkgtest-cloud/worker/worker[427475]: INFO: Caught SIGTERM, requesting exit
May 21 10:46:51 juju-113c58-stg-proposed-migration-environment-94 /home/ubuntu/autopkgtest-cloud/worker/worker[427475]: WARNING: autopkgtest exited with code: -15
                                                                                                                        ack-ing message: b'hello\n{"triggers": ["hello/2.10-3build1"], "submit-time": "2024-05-21 10:46:27", "uuid": "f23dfb9c-4bd0-4dda-93a6-819ee104545f"}'
May 21 10:46:51 juju-113c58-stg-proposed-migration-environment-94 /home/ubuntu/autopkgtest-cloud/worker/worker[427475]: INFO: autopkgtest has been killed with USR1, indicating a restart, not removing message hello
                                                                                                                        {"triggers": ["hello/2.10-3build1"], "submit-time": "2024-05-21 10:46:27", "uuid": "f23dfb9c-4bd0-4dda-93a6-819ee104545f"} from queue
May 21 10:46:51 juju-113c58-stg-proposed-migration-environment-94 /home/ubuntu/autopkgtest-cloud/worker/worker[427475]: INFO: Killing openstack server with uuid f23dfb9c-4bd0-4dda-93a6-819ee104545f
May 21 10:46:53 juju-113c58-stg-proposed-migration-environment-94 /home/ubuntu/autopkgtest-cloud/worker/worker[427475]: INFO: Deleted test server: adt-noble-amd64-hello-20240521-104627-juju-113c58-stg-proposed-migration-environment-94-f23dfb9c-4bd0-4dda-93a6-819ee104545f

[Tim Andersson (andersson123)]

with stop:

May 21 10:48:14 juju-113c58-stg-proposed-migration-environment-94 systemd[1]: Stopping autopkgtest worker lcy02-1...
May 21 10:48:14 juju-113c58-stg-proposed-migration-environment-94 systemd[1]: <email address hidden>: Main process exited, code=killed, status=9/KILL

That's not great...

[Tim Andersson (andersson123)]

Well, actually, once started again, it picks up the same message, so having SIGKILL might be good... Although too brute force actually I think

[Tim Andersson (andersson123)]

Hm, same with USR1

May 21 10:53:39 juju-113c58-stg-proposed-migration-environment-94 systemd[1]: Stopping autopkgtest worker lcy02-1...
May 21 10:53:39 juju-113c58-stg-proposed-migration-environment-94 systemd[1]: <email address hidden>: Main process exited, code=killed, status=10/USR1

[Tim Andersson (andersson123)]

Still goes back into queue however.

[Tim Andersson (andersson123)]

Okay. Ready for review. All tested in staging.

[Tim Andersson (andersson123)]

Note to reviewer - I know the logic of the block delineating the exit codes could be simplified a bit, but I think the explicit logging will be helpful and it's most readable in its current state.

[Paride Legovini (paride)]

review: Needs Information

[Paride Legovini (paride)]

One comment, for the rest lgtm.

[Tim Andersson (andersson123)]

Hey, amended! The only thing different here, apart from removing those systemd unit configs, is that I also changed it so that any edge cases are also requeued. I think this is a good thing to do by default. This is NOT related to permanent looping, however. This is different entirely. If something kills the process we're unaware of, or in an odd way, we don't want to lose the test request.

Ready for re-review!

[Paride Legovini (paride)]

review: Approve

diff --git a/charms/focal/autopkgtest-cloud-worker/autopkgtest-cloud/worker/worker b/charms/focal/autopkgtest-cloud-worker/autopkgtest-cloud/worker/worker
index 809f7ad..0173fe5 100755
--- a/charms/focal/autopkgtest-cloud-worker/autopkgtest-cloud/worker/worker
+++ b/charms/focal/autopkgtest-cloud-worker/autopkgtest-cloud/worker/worker
@@ -1332,13 +1332,33 @@ def request(msg):
                     # tests can be requested to be killed with:
                     # kill -15 $pid
                     logging.warning(
-                        "autopkgtest exited with code: %i\nack-ing message: %s",
+                        "autopkgtest exited with code: %i",
                         code,
-                        body.encode(),
                     )
                     running_test = False
                     # we ack the message so it doesn't go back in the queue
-                    msg.channel.basic_ack(msg.delivery_tag)
+                    if code == -15:
+                        logging.info(
+                            "autopkgtest has been killed with SIGTERM, indicating a restart or stop, requeue-ing message %s"
+                            % body
+                        )
+                        msg.channel.basic_reject(
+                            msg.delivery_tag, requeue=True
+                        )
+                    elif code == -9:
+                        logging.info(
+                            "autopkgtest has been killed intentionally, removing message %s from the queue"
+                            % body
+                        )
+                        msg.channel.basic_ack(msg.delivery_tag)
+                    else:
+                        logging.info(
+                            "autopkgtest has hit an obscure failure mode, removing message %s from the queue"
+                            % body
+                        )
+                        msg.channel.basic_reject(
+                            msg.delivery_tag, requeue=True
+                        )
                     logging.info(
                         "Killing openstack server with uuid %s",
                         test_uuid,
diff --git a/docs/administration.rst b/docs/administration.rst
index e899b03..302fcc7 100644
--- a/docs/administration.rst
+++ b/docs/administration.rst
@@ -485,6 +485,9 @@ In order to kill a currently running test, grab the test uuid. This can be seen 
 
   ps aux | grep runner | grep $uuid
   # grab the PID from the process
+  # If you want to kill the test and remove the test request from the queue:
+  kill -9 $pid
+  # If you want to kill the test and preserve the test request in the queue:
   kill -15 $pid
 
 This will kill the autopkgtest process, and then the worker will `ack` the test request