launchpad-reviewers team mailing list archive

[Colin Watson <mp+445186@xxxxxxxxxxxxxxxxxx>]

Colin Watson has proposed merging ~cjwatson/launchpad:charm-assets-fixes into launchpad:master.

Commit message:
charm/launchpad-assets: Fix various issues affecting staging

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/+git/launchpad/+merge/445186

Staging is a bit weird because of the use of a separate stream of deployment artifacts for it (see https://code.launchpad.net/~cjwatson/launchpad/+git/launchpad/+merge/439659), and some of that weirdness affects handling of `+combo` and `+icing` URLs in the assets charm.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad:charm-assets-fixes into launchpad:master.

diff --git a/charm/launchpad-assets/reactive/launchpad-assets.py b/charm/launchpad-assets/reactive/launchpad-assets.py
index 2612611..331cb58 100644
--- a/charm/launchpad-assets/reactive/launchpad-assets.py
+++ b/charm/launchpad-assets/reactive/launchpad-assets.py
@@ -31,9 +31,17 @@ def configure_convoy(config):
     build_label = config["build_label"]
     convoy_path = Path(base.base_dir()) / "convoy"
     convoy_path.mkdir(parents=True, exist_ok=True)
-    link_path = convoy_path / f"rev{build_label}"
+    # The build label may include a suffix such as "-db".  These identify
+    # different streams of deployment artifacts that are pruned on a
+    # different schedule (see utilities/publish-to-swift), and entries in
+    # the payloads directory include the suffix.  However, the +combo URLs
+    # generated by the appserver don't include the suffix, so the names of
+    # symlinks in the convoy path shouldn't either.
+    link_path = convoy_path / f"rev{build_label.split('-', 1)[0]}"
     if not link_path.is_symlink():
-        link_path.symlink_to(Path(base.code_dir()) / "build" / "js")
+        link_path.symlink_to(
+            Path(base.payloads_dir()) / build_label / "build" / "js"
+        )
     for link_path in convoy_path.iterdir():
         if link_path.name.startswith("rev") and link_path.is_symlink():
             if not link_path.exists():
@@ -50,9 +58,18 @@ def configure_convoy(config):
 
 def get_service_config():
     config = hookenv.config()
+    # If the build label has a suffix (see comment in configure_convoy
+    # above), then we'll need to append it when mapping +icing URLs to
+    # payload directories on the file system.
+    build_label = config["build_label"]
+    if "-" in build_label:
+        build_label_suffix = f"-{build_label.split('-', 1)[1]}"
+    else:
+        build_label_suffix = ""
     config.update(
         {
             "base_dir": base.base_dir(),
+            "build_label_suffix": build_label_suffix,
             "code_dir": base.code_dir(),
             "logs_dir": base.logs_dir(),
             "payloads_dir": base.payloads_dir(),
diff --git a/charm/launchpad-assets/templates/vhost.conf.j2 b/charm/launchpad-assets/templates/vhost.conf.j2
index 75c6dfc..1066898 100644
--- a/charm/launchpad-assets/templates/vhost.conf.j2
+++ b/charm/launchpad-assets/templates/vhost.conf.j2
@@ -20,7 +20,7 @@
         Header set Cache-Control "public,max-age=5184000"
         Require all granted
     </LocationMatch>
-    AliasMatch "^/\+icing/rev([0-9a-f]+)/(.*)" "{{ payloads_dir }}/$1/lib/canonical/launchpad/icing/$2"
+    AliasMatch "^/\+icing/rev([0-9a-f]+)/(.*)" "{{ payloads_dir }}/$1{{ build_label_suffix }}/lib/canonical/launchpad/icing/$2"
     <Location "/+icing/">
         Header set Cache-Control "public,max-age=5184000"
         Require all granted
@@ -38,7 +38,7 @@
         Header set Cache-Control "public,max-age=5184000"
         Require all granted
     </LocationMatch>
-    AliasMatch "^/favicon\.(?:ico|gif|png)$" "/srv/launchpad/code/lib/canonical/launchpad/images/launchpad.png"
+    AliasMatch "^/favicon\.(?:ico|gif|png)$" "{{ code_dir }}/lib/canonical/launchpad/images/launchpad.png"
 
     <Location "/_status/check">
         Require all granted