launchpad-reviewers team mailing list archive

[Colin Watson <mp+454165@xxxxxxxxxxxxxxxxxx>]

Colin Watson has proposed merging ~cjwatson/launchpad-layers:fix-publisher-parts-directory into launchpad-layers:main.

Commit message:
launchpad-publisher-parts: Fix parts directory

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

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

`publisher_parts_dir()` was set as `config.archivepublisher.run_parts_location`, and is `/srv/launchpad/publisher-parts`.  However, the code that uses this looks like this:

    def find_run_parts_dir(distribution_name, parts):
        """Find the requested run-parts directory, if it exists."""
        run_parts_location = config.archivepublisher.run_parts_location
        if not run_parts_location:
            return None

        parts_dir = os.path.join(run_parts_location, distribution_name, parts)
        if file_exists(parts_dir):
            return parts_dir
        else:
            return None

This means that it's actually trying to look in `/srv/launchpad/publisher-parts/ubuntu`, which didn't exist.

The layer's configuration is really a bit wrong for this, since strictly we ought to support multiple distributions with independent configuration for each.  However, for now, hardcoding Ubuntu will do well enough.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad-layers:fix-publisher-parts-directory into launchpad-layers:main.

diff --git a/launchpad-publisher-parts/lib/charms/launchpad/publisher_parts.py b/launchpad-publisher-parts/lib/charms/launchpad/publisher_parts.py
index 848ed74..d3ab445 100644
--- a/launchpad-publisher-parts/lib/charms/launchpad/publisher_parts.py
+++ b/launchpad-publisher-parts/lib/charms/launchpad/publisher_parts.py
@@ -21,6 +21,13 @@ def publisher_parts_dir():
     return os.path.join(base.base_dir(), parts)
 
 
+def publisher_parts_subdir():
+    # XXX cjwatson 2023-10-20: Need to unhardcode Ubuntu here, but that
+    # probably requires reshaping the configuration to take a dict of
+    # distribution name -> (repository, revision).
+    return os.path.join(publisher_parts_dir(), "ubuntu")
+
+
 def safe_payload_dir(repository, revision):
     dir = quote(urlparse(repository).path, safe="")
     return os.path.join(payloads_dir(), f"{dir}-{revision}")
@@ -33,8 +40,8 @@ def ensure_payloads_dir():
 
 
 def get_current_revision():
-    current_revision_path = os.path.realpath(publisher_parts_dir())
-    if current_revision_path == publisher_parts_dir():
+    current_revision_path = os.path.realpath(publisher_parts_subdir())
+    if current_revision_path == publisher_parts_subdir():
         return ""
     return current_revision_path
 
@@ -64,15 +71,23 @@ def link_run_parts(repository, revision):
 
     dir_path = safe_payload_dir(repository, revision)
     run_parts_dir = publisher_parts_dir()
+    run_parts_subdir = publisher_parts_subdir()
+
+    hookenv.log(f"Creating symlink {dir_path} {run_parts_subdir}")
 
-    hookenv.log(f"Creating symlink {dir_path} {run_parts_dir}")
+    # Fix up mistaken layout in earlier versions of this layer.
+    if os.path.islink(run_parts_dir):
+        os.unlink(run_parts_dir)
 
+    host.mkdir(
+        run_parts_dir, owner=base.user(), group=base.user(), perms=0o755
+    )
     try:
-        os.unlink(f"{run_parts_dir}.new")
+        os.unlink(f"{run_parts_subdir}.new")
     except FileNotFoundError:
         pass
-    os.symlink(dir_path, f"{run_parts_dir}.new")
-    os.rename(f"{run_parts_dir}.new", run_parts_dir)
+    os.symlink(dir_path, f"{run_parts_subdir}.new")
+    os.rename(f"{run_parts_subdir}.new", run_parts_subdir)
 
 
 def prune_payloads(keep_revisions):