Merge ~andersson123/autopkgtest-cloud:improve-handling-cache-amqp-failures into autopkgtest-cloud:master

[Tim Andersson]

[Tim Andersson (andersson123)]

Mmmm, I think I can write this a bit better

[Tim Andersson (andersson123)]

Okay, amended, please review

[Tim Andersson (andersson123)]

ah, nah, actually that's my bad, one sec I amend

[Brian Murray (brian-murray)]

We (you!) previously did some development around raising appropriate exit codes when submit.py fails. What will the exit code of this TimeoutError be?

review: Needs Information

[Paride Legovini (paride)]

It is not completely clear to me how this improves the UX.

As of today: if rabbitmq is broken, users experience a timeout error after a while, because the request can't be enqueued. Isn't this "right" already?

Is it actually useful to users that the "behind the scenes" error is surfaced?

(Is this useful for other reasons I'm not getting?)

[Tim Andersson (andersson123)]

I amended this as requested :)

I just had a thought though, I think I'll move the timeout class to helpers/utils.py

[Tim Andersson (andersson123)]

Okay, I reworked this mp a little bit, please re-review

[Brian Murray (brian-murray)]

> It is not completely clear to me how this improves the UX.
>
> As of today: if rabbitmq is broken, users experience a timeout error after a
> while, because the request can't be enqueued. Isn't this "right" already?
>
> Is it actually useful to users that the "behind the scenes" error is surfaced?
>
> (Is this useful for other reasons I'm not getting?)

I asked for this feature because it wasn't obvious that rabbimq was the culprit or something else and because it took an extended period of time to debug. So by surfacing where the error actually is we'll know what part of the service to look at.

[Tim Andersson (andersson123)]

I think what you stated is actually done in the latest version of the diff. It surfaces the error to /var/log/apache2/error.log, but not to the user via the webpage. Let me know if this is appropriate.

[Skia (hyask)]

See inline comment. I'm aligned with Paride's view of surfacing the error up to user, and not hide it in some obscure log file that is not monitored.

Moreover, I think our all_exception handler is not behaving correctly in the sense that the error is not logged anymore in our files. That would be another much needed improvement.

review: Needs Fixing

[Skia (hyask)]

Thanks!

review: Approve

[Tim Andersson (andersson123)]

I had to fix a merge conflict, and this also requires testing somehow. I'll wait till staging is fully functional again and test this, then merge it.

Unmerged commits

f679675... by Tim Andersson on 2024-01-17

fix: web: make send_amqp_request function have a timeout

We were recently having some issues with production, where
requesting a test via the webpage would result in the page
eventually telling the user the server has timed out. This
was because the send_amqp_request function was failing, but
the library used to send the request has no internal timeout.

send_amqp_request was failing because the rabbitmq-server ran
out of memory.

This commit amends the above issue by adding a timeout for the
send_amqp_request function is the form of an signal handler,
via the `signal` python module.

This commit also introduces a new exception parent class,
InternalException. And a child class of this class, QueueDead.

This commit also introduces a new child class of WebControlException,
QueueDead.

This exception is raised when the rabbitmq connection times out, as
described above. Now, when this happens, the exception is raised and the
error message defined in the QueueDead declaration is displayed to the
user.

	pre_commit:0 (build)
	unit_tests:0 (build)
	build_charms:0 (build)

1 → 3 of 3 results

First • Previous • Next • Last

diff --git a/charms/focal/autopkgtest-web/webcontrol/helpers/exceptions.py b/charms/focal/autopkgtest-web/webcontrol/helpers/exceptions.py
index e94164e..96a206a 100644
--- a/charms/focal/autopkgtest-web/webcontrol/helpers/exceptions.py
+++ b/charms/focal/autopkgtest-web/webcontrol/helpers/exceptions.py
@@ -103,3 +103,14 @@ class InvalidArgs(WebControlException):
             ),
             400,
         )
+
+
+class QueueDead(WebControlException):
+    def __init__(self):
+        super().__init__(
+            (
+                "Rabbitmq unresponsive. Perhaps it needs restarting. "
+                "Please contact the QA team on #ubuntu-quality (highlight qa-help)."
+            ),
+            500,
+        )
diff --git a/charms/focal/autopkgtest-web/webcontrol/helpers/utils.py b/charms/focal/autopkgtest-web/webcontrol/helpers/utils.py
index 9b95cc8..67eb3c2 100644
--- a/charms/focal/autopkgtest-web/webcontrol/helpers/utils.py
+++ b/charms/focal/autopkgtest-web/webcontrol/helpers/utils.py
@@ -8,6 +8,7 @@ import logging
 import os
 import pathlib
 import random
+import signal
 import sqlite3
 import time
 import typing
@@ -84,6 +85,22 @@ def get_supported_releases():
     )
 
 
+class timeout:
+    def __init__(self, seconds=1, error_message="Timeout"):
+        self.seconds = seconds
+        self.error_message = error_message
+
+    def handle_timeout(self, signum, frame):
+        raise TimeoutError(self.error_message)
+
+    def __enter__(self):
+        signal.signal(signal.SIGALRM, self.handle_timeout)
+        signal.alarm(self.seconds)
+
+    def __exit__(self, type, value, traceback):
+        signal.alarm(0)
+
+
 def setup_key(app, path):
     """Create or load app.secret_key for cookie encryption."""
     try:
diff --git a/charms/focal/autopkgtest-web/webcontrol/request/submit.py b/charms/focal/autopkgtest-web/webcontrol/request/submit.py
index e7f1613..7e4943f 100644
--- a/charms/focal/autopkgtest-web/webcontrol/request/submit.py
+++ b/charms/focal/autopkgtest-web/webcontrol/request/submit.py
@@ -23,10 +23,14 @@ from helpers.exceptions import (
     ForbiddenRequest,
     InvalidArgs,
     NotFound,
+    QueueDead,
     RequestInQueue,
     RequestRunning,
 )
-from helpers.utils import get_autopkgtest_cloud_conf
+from helpers.utils import (
+    get_autopkgtest_cloud_conf,
+    timeout,
+)
 
 # Launchpad REST API base
 LP = "https://api.launchpad.net/1.0/"
@@ -348,10 +352,14 @@ class Submit:
             password=self.amqp_creds.password,
         ) as amqp_con:
             with amqp_con.channel() as ch:
-                ch.basic_publish(
-                    amqp.Message(body, delivery_mode=2),  # persistent
-                    routing_key=queue,
-                )
+                try:
+                    with timeout(seconds=60):
+                        ch.basic_publish(
+                            amqp.Message(body, delivery_mode=2),  # persistent
+                            routing_key=queue,
+                        )
+                except TimeoutError as e:
+                    raise QueueDead from e
         return params["uuid"]
 
     @classmethod