sts-sponsors team mailing list archive

[Igor Brovtsin <mp+439621@xxxxxxxxxxxxxxxxxx>]

Igor Brovtsin has proposed merging ~igor-brovtsin/maas:lp-1743648 into maas:master.

Commit message:
Improved listener (un)registration for images sync

Fixes LP#1743648

Requested reviews:
  MAAS Maintainers (maas-maintainers)

For more details, see:
https://code.launchpad.net/~igor-brovtsin/maas/+git/maas/+merge/439621

This MP fixes the issue described in LP#1743648 that prevents image sync after failed attempt to call `download_boot_resources`. Any exception risen by `download_boot_resources` will prevent `unregister` call from ever happening, so further attempts to call `download_all_boot_resources` will fail due to channel handler being already registered. 

To address the issue, I introduced `@contextmanager`-decorated function `PostgresListenerService.listen` that registers and unregisters handlers for developer convenience.
-- 
Your team MAAS Committers is subscribed to branch maas:master.

diff --git a/src/maasserver/bootresources.py b/src/maasserver/bootresources.py
index 90044ce..8573fad 100644
--- a/src/maasserver/bootresources.py
+++ b/src/maasserver/bootresources.py
@@ -1227,34 +1227,33 @@ def download_all_boot_resources(
         if needs_cancel:
             store.cancel_finalize()
 
-    listener.register("sys_stop_import", stop_import)
-
-    # Download all of the metadata first.
-    for source in sources:
-        msg = "Importing images from source: %s" % source["url"]
-        Event.objects.create_region_event(EVENT_TYPES.REGION_IMPORT_INFO, msg)
-        maaslog.info(msg)
-        download_boot_resources(
-            source["url"],
-            store,
-            product_mapping,
-            keyring_file=source.get("keyring"),
-        )
+    with listener.listen("sys_stop_import", stop_import):
+        for source in sources:
+            msg = "Importing images from source: %s" % source["url"]
+            Event.objects.create_region_event(
+                EVENT_TYPES.REGION_IMPORT_INFO, msg
+            )
+            maaslog.info(msg)
+            download_boot_resources(
+                source["url"],
+                store,
+                product_mapping,
+                keyring_file=source.get("keyring"),
+            )
 
-    # Start finalizing or cancel finalizing.
-    with lock:
-        stopped = stop
-    if stopped:
-        log.debug(
-            "Finalizing BootResourceStore was cancelled before starting."
-        )
-        store.cancel_finalize(notify=notify)
-    else:
-        log.debug("Finalizing BootResourceStore.")
+        # Start finalizing or cancel finalizing.
         with lock:
-            finalizing = True
-        store.finalize(notify=notify)
-    listener.unregister("sys_stop_import", stop_import)
+            stopped = stop
+        if stopped:
+            log.debug(
+                "Finalizing BootResourceStore was cancelled before starting."
+            )
+            store.cancel_finalize(notify=notify)
+        else:
+            log.debug("Finalizing BootResourceStore.")
+            with lock:
+                finalizing = True
+            store.finalize(notify=notify)
     return not stop
 
 
diff --git a/src/maasserver/listener.py b/src/maasserver/listener.py
index 673f4d1..167818a 100644
--- a/src/maasserver/listener.py
+++ b/src/maasserver/listener.py
@@ -5,6 +5,7 @@
 
 
 from collections import defaultdict, deque
+from contextlib import contextmanager
 from errno import ENOENT
 import json
 import threading
@@ -536,6 +537,17 @@ class PostgresListenerService(Service):
         # that we don't process them a second time.
         del notifies[:]
 
+    @contextmanager
+    def listen(self, channel, handler):
+        """
+        Helper for processes that register their channels temporarily
+        """
+        self.register(channel, handler)
+        try:
+            yield
+        finally:
+            self.unregister(channel, handler)
+
 
 def notify_action(target: str, action: str, identifier: Any):
     """Send a notification for an action on a target."""