launchpad-reviewers team mailing list archive

[Tushar Gupta <mp+481408@xxxxxxxxxxxxxxxxxx>]

Tushar Gupta has proposed merging ~tushar5526/launchpad:refactor-excluded-ppas-and-add-support-parallel-runs into launchpad:master.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~tushar5526/launchpad/+git/launchpad/+merge/481408
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~tushar5526/launchpad:refactor-excluded-ppas-and-add-support-parallel-runs into launchpad:master.

diff --git a/charm/launchpad-ppa-publisher/config.yaml b/charm/launchpad-ppa-publisher/config.yaml
index 411ffce..ef4e81b 100644
--- a/charm/launchpad-ppa-publisher/config.yaml
+++ b/charm/launchpad-ppa-publisher/config.yaml
@@ -78,11 +78,34 @@ options:
     type: string
     default:
     description: Endpoint for the signing service.
-  excluded_ppas:
+  parallel_publisher_config:
     type: string
     description: |
-      YAML-encoded list of PPA references that should be excluded from the
-      main publisher run. Example:
-      - ~foo/bar/ppa1
-      - ~foo/ubuntu/ppa2
+      YAML-encoded config that lists the excluded PPAs and configurations 
+      for parallel publisher runs. Example:
+        parallel_publisher_config:
+          excluded_ppas:
+            - foo/ubuntu/ppa1
+            - foo/ubuntu/ppa2
+            - foo/ubuntu/ppa5
+            - foo/debian/ppa3
+            - foo/soss/ppa4
+          runs:
+            - distro: ubuntu
+              id: parallel-run-1
+              ppas:
+                - foo/ubuntu/ppa1
+            - distro: ubuntu
+              id: parallel-run-2
+              ppas:
+                - foo/ubuntu/ppa2
+                - foo/ubuntu/ppa5
+            - distro: debian
+              id: parallel-run-3
+              ppas:
+                - foo/debian/ppa3
+            - distro: all_derived
+              id: parallel-run-4
+              ppas:
+                - foo/soss/ppa4
     default: ""
diff --git a/charm/launchpad-ppa-publisher/reactive/launchpad-ppa-publisher.py b/charm/launchpad-ppa-publisher/reactive/launchpad-ppa-publisher.py
index 0fc235a..58f8836 100644
--- a/charm/launchpad-ppa-publisher/reactive/launchpad-ppa-publisher.py
+++ b/charm/launchpad-ppa-publisher/reactive/launchpad-ppa-publisher.py
@@ -53,6 +53,102 @@ def generate_exclude_ppas_options(excluded_ppas):
     return f" --exclude {' --exclude '.join(excluded_ppas)}"
 
 
+def validate_parallel_publisher_config(parallel_publisher_config):
+    """
+    Validates whether the parallel publisher config follows the following
+    rules:
+    1. parallel run IDs are unique.
+    2. only excluded ppas are used in parallel runs
+    3. all required config values are present
+    """
+    runs = parallel_publisher_config.get("runs")
+    if not runs:
+        return True
+    excluded_ppas = set(parallel_publisher_config.get("excluded_ppas"))
+
+    # runs are configured but there are no excluded PPAs
+    if not excluded_ppas:
+        hookenv.log(
+            "Parallel publisher runs are configured but there are no PPAs"
+            " excluded."
+        )
+        return False
+
+    run_ids = set()
+    for run in runs:
+        distro = run.get("distro")
+        run_id = run.get("id")
+        ppas = run.get("ppas")
+
+        if not ppas:
+            hookenv.log("No PPAs specified for the run.")
+            return False
+
+        if not distro:
+            hookenv.log("No distribution specified for the run.")
+            return False
+
+        if not run_id:
+            hookenv.log("No run ID specified for the run.")
+            return False
+
+        if run_id in run_ids:
+            hookenv.log(f"Duplicate run ID found: {run_id}")
+            return False
+
+        run_ids.add(run_id)
+
+        for ppa in ppas:
+            if ppa not in excluded_ppas:
+                hookenv.log(f"PPA {ppa} is not in the excluded PPAs list.")
+                return False
+    return True
+
+
+def generate_parallel_publisher_crons(parallel_publisher_config):
+    if not parallel_publisher_config:
+        return []
+    runs = parallel_publisher_config.get("runs")
+
+    if not runs:
+        return []
+
+    if not validate_parallel_publisher_config(parallel_publisher_config):
+        return []
+
+    crons = []
+    for run in runs:
+        distro = run.get("distro")
+        run_id = run.get("id")
+        ppas = run.get("ppas")
+
+        # ensure we have all the required config
+        if not distro or not run_id or not ppas:
+            hookenv.log(
+                "Please specify each of distro, id, and ppas for parallel run."
+                f" Got distro: {distro}, id: {run_id}, ppas: {ppas}"
+            )
+            continue
+
+        distro_option = (
+            "--all-derived" if distro == "all_derived" else f"-d {distro}"
+        )
+        archive_options = [f"--archive {ppa}" for ppa in ppas]
+        lockfilename_option = f"--lockfilename {run_id}.lock"
+        logfilename = f"cron.ppa.{run_id}.log"
+
+        cron = (
+            "* * * * * umask 022; nice -n 5 ionice -c 2 -n 7 "
+            "{{ code_dir }}/cronscripts/publishing/cron.parallel-publish-ppa "
+            f'"{distro_option} {' '.join(archive_options)} '
+            f'{lockfilename_option}" '
+            f">> {{ logs_dir }}/{logfilename} 2>&1"
+        )
+        crons.append(cron)
+
+    return crons
+
+
 @when(
     "launchpad.db.configured",
     "memcache.available",
@@ -72,9 +168,15 @@ def configure():
     config["ppa_signing_keys_root"] = ppa_keys_root
     config["oval_data_root"] = oval_data_root
 
-    excluded_ppas = yaml.safe_load(config["excluded_ppas"])
+    parallel_publisher_config = yaml.safe_load(
+        config.get("parallel_publisher_config")
+    )
     config["excluded_ppas_options"] = generate_exclude_ppas_options(
-        excluded_ppas
+        parallel_publisher_config.get("excluded_ppas")
+    )
+
+    config["parallel_publisher_crons"] = generate_parallel_publisher_crons(
+        parallel_publisher_config
     )
 
     host.mkdir(data_dir, owner=base.user(), group=base.user(), perms=0o775)
diff --git a/charm/launchpad-ppa-publisher/templates/crontab.j2 b/charm/launchpad-ppa-publisher/templates/crontab.j2
index 5dd8234..1920e10 100644
--- a/charm/launchpad-ppa-publisher/templates/crontab.j2
+++ b/charm/launchpad-ppa-publisher/templates/crontab.j2
@@ -11,6 +11,10 @@ P3AROOT={{ ppa_archive_private_root }}
 {% if active -%}
 * * * * * umask 022; nice -n 5 ionice -c 2 -n 7 {{ code_dir }}/cronscripts/publishing/cron.publish-ppa "-d ubuntu{{ excluded_ppas_options }}" "--all-derived{{ excluded_ppas_options }}" "-d debian{{ excluded_ppas_options }}" >> {{ logs_dir }}/cron.ppa.log 2>&1
 
+{% for parallel_publisher_cron in parallel_publisher_crons %}
+{{ parallel_publisher_cron }}
+{% endfor %}
+
 17,47 * * * * nice -n 15 {{ code_dir }}/cronscripts/parse-ppa-apache-access-logs.py -q --log-file=INFO:{{ logs_dir }}/parse-ppa-apache-access-logs.log
 
 59 05  * * 0 {{ code_dir }}/cronscripts/publishing/cron.daily-ppa >> {{ logs_dir }}/cron.ppa.log 2>&1
diff --git a/cronscripts/publishing/cron.publish-parallel-ppa b/cronscripts/publishing/cron.publish-parallel-ppa
new file mode 100755
index 0000000..3e7d9cb
--- /dev/null
+++ b/cronscripts/publishing/cron.publish-parallel-ppa
@@ -0,0 +1,39 @@
+#!/bin/bash
+#
+# Copyright 2009 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+set -x
+
+RUN_LOCKFILENAME=$(echo "$*" | awk '{print $NF}')
+ 
+# we take another lock here to make sure other invocations
+# of this script from the crontab do not race with each other. 
+LOCKFILENAME="master-$RUN_LOCKFILENAME"
+LOCKFILE="/var/lock/$LOCKFILENAME"
+
+if [ "$LOCKFILEOPTIONS" == "" ]; then
+   LOCKFILEOPTIONS="-r1"
+fi
+
+# Claim the lockfile.  ($LOCKFILEOPTIONS is deliberately unquoted, since it
+# may expand to either zero words or one word.)
+# shellcheck disable=SC2086
+if ! lockfile $LOCKFILEOPTIONS "$LOCKFILE"; then
+  echo "Could not claim lock file: $LOCKFILE"
+  exit 1
+fi
+
+# Cleanup the lockfile on exit.
+cleanup () {
+  echo "Cleaning up lockfile: $LOCKFILE"
+  rm -f "$LOCKFILE"
+}
+
+trap cleanup EXIT
+
+LPCURRENT="$(dirname "$0")/../.."
+# shellcheck disable=SC2086 disable=2068
+"$LPCURRENT/scripts/process-accepted.py" -v $@
+# shellcheck disable=SC2086 disable=2068
+"$LPCURRENT/scripts/publish-distro.py" -v $@