launchpad-reviewers team mailing list archive

[Colin Watson <mp+416672@xxxxxxxxxxxxxxxxxx>]

Colin Watson has proposed merging ~cjwatson/launchpad-buildd:drop-charm-resources into launchpad-buildd:master.

Commit message:
charm: Drop support for installing packages using resources

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

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

It's pretty simple to tell the charm to use some other installation source (e.g. a different PPA) using the `install_sources` configuration option, and so the amount of code required to support installing `launchpad-buildd` and `python3-lpbuildd` using Juju resources doesn't really pay for itself.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad-buildd:drop-charm-resources into launchpad-buildd:master.

diff --git a/charm/Makefile b/charm/Makefile
index 2c39dca..73fc2bd 100644
--- a/charm/Makefile
+++ b/charm/Makefile
@@ -8,7 +8,6 @@ CHARM_SRC ?= $(CURDIR)
 JUJU_REPOSITORY = $(BUILDDIR)
 CHARMDIR = $(BUILDDIR)/$(CHARM_SERIES)/$(NAME)
 CHARMREPODIR = $(BUILDDIR)/build
-RESOURCES = $(BUILDDIR)/resources/$(NAME)
 CHARM = $(CHARMDIR)/.done
 LAYER_PATH = $(TMPDIR)/layer
 INTERFACE_PATH = $(TMPDIR)/interface
@@ -23,33 +22,17 @@ export JUJU_REPOSITORY
 
 build: $(CHARM)
 
-packages: $(RESOURCES)
-
-build-with-packages: build packages
-
 $(BUILDDIR):
 	mkdir -p $@
 
 $(CHARM): $(CHARM_SRC) | $(BUILDDIR)
 	rm -rf $(CHARMDIR)
 	charm build -o $(BUILDDIR) -s $(CHARM_SERIES) -n $(NAME) $(EXTRA_CHARM_BUILD_ARGS)
-	touch $(CHARMDIR)/dummy-launchpad-buildd.deb
-	touch $(CHARMDIR)/dummy-python3-lpbuildd.deb
 	touch $@
 
 clean:
 	rm -rf $(BUILDDIR) $(TMPDIR)
 
-$(RESOURCES):
-	rm -rf $(TMPDIR)
-	mkdir -p $(TMPDIR)
-	rsync -a --exclude /charm $(TOPDIR)/ $(TMPDIR)/$(NAME)/
-	cd $(TMPDIR)/$(NAME)/ && debuild -b -uc -us
-	mkdir -p $(RESOURCES)
-	cp -a $(TMPDIR)/launchpad-buildd_*.deb \
-	      $(RESOURCES)/launchpad-buildd.deb
-	cp -a $(TMPDIR)/python3-lpbuildd_*.deb $(RESOURCES)/python3-lpbuildd.deb
-
 create-privileged-model:
 	juju add-model privileged localhost
 	lxc profile set juju-privileged security.privileged true
@@ -57,21 +40,7 @@ create-privileged-model:
 deploy:
 	juju deploy $(CHARMDIR)
 
-deploy-with-packages:
-	juju deploy \
-		--resource=launchpad-buildd=$(RESOURCES)/launchpad-buildd.deb \
-		--resource=python3-lpbuildd=$(RESOURCES)/python3-lpbuildd.deb \
-		$(CHARMDIR)
-
 push:
 	charm push $(CHARMDIR) $(CHARM_STORE_URL)
 
-push-with-packages:
-	charm push \
-		--resource=launchpad-buildd=$(CHARMDIR)/dummy-launchpad-buildd.deb \
-		--resource=python3-lpbuildd=$(CHARMDIR)/dummy-python3-lpbuildd.deb \
-		$(CHARMDIR) $(CHARM_STORE_URL)
-
-.PHONY: build packages build-with-packages clean
-.PHONY: create-privileged-model deploy deploy-with-packages
-.PHONY: push push-with-packages
+.PHONY: build clean create-privileged-model deploy push
diff --git a/charm/README.md b/charm/README.md
index 32e9259..6e268de 100644
--- a/charm/README.md
+++ b/charm/README.md
@@ -23,24 +23,15 @@ lxc profile set juju-privileged security.privileged true
 
 # Deployment
 
-You can either deploy the stock launchpad-buildd package from a PPA, or
-build your own.
-
-Installing from a PPA is the default; just deploy this charm.
-
-If you're building your own package, then you already have the
-launchpad-buildd code checked out.  In the `charm/` subdirectory, build the
-charm and packages together:
-
 ```
-make build-with-packages
+make deploy
 ```
 
-Then deploy the charm, attaching the packages as resources:
-
-```
-make deploy-with-packages
-```
+This charm will deploy the launchpad-buildd package from a PPA.  If you want
+to deploy a modified version of launchpad-buildd, you can either build it
+locally and install the resulting packages manually after initial
+deployment, or you can upload a modified source package to your own PPA and
+set `install_sources` to refer to that PPA.
 
 Either way, this should eventually give you a running builder.  Find out its
 host name (e.g. `juju-XXXXXX-0.lxd`) and [add it to your local Launchpad
diff --git a/charm/layer.yaml b/charm/layer.yaml
index 4ec9f2e..9867966 100644
--- a/charm/layer.yaml
+++ b/charm/layer.yaml
@@ -7,6 +7,7 @@ options:
         packages:
             - bzr-builder
             - git-build-recipe
+            - launchpad-buildd
             - quilt
 ignore:
     - dist
diff --git a/charm/metadata.yaml b/charm/metadata.yaml
index 1817d1d..6a063b7 100644
--- a/charm/metadata.yaml
+++ b/charm/metadata.yaml
@@ -9,13 +9,4 @@ maintainer: Colin Watson <cjwatson@xxxxxxxxxxxxx>
 subordinate: false
 series:
     - bionic
-resources:
-    launchpad-buildd:
-        type: file
-        filename: launchpad-buildd.deb
-        description: The main Launchpad builder package.
-    python3-lpbuildd:
-        type: file
-        filename: python3-lpbuildd.deb
-        description: Python libraries supporting the Launchpad builder.
 min-juju-version: 2.0.0
diff --git a/charm/reactive/launchpad-buildd.py b/charm/reactive/launchpad-buildd.py
index 5b0ac3b..4e88d3e 100644
--- a/charm/reactive/launchpad-buildd.py
+++ b/charm/reactive/launchpad-buildd.py
@@ -1,16 +1,9 @@
-# Copyright 2016 Canonical Ltd.  This software is licensed under the
+# Copyright 2016-2022 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 import os.path
 import re
-import shutil
-import subprocess
 
-from charmhelpers import fetch
-from charmhelpers.core import (
-    hookenv,
-    host,
-    )
 from charms.apt import status_set
 from charms.reactive import (
     hook,
@@ -26,63 +19,28 @@ from charms.reactive import (
 def install():
     with open("/etc/default/launchpad-buildd", "w") as default_file:
         print("RUN_NETWORK_REQUESTS_AS_ROOT=yes", file=default_file)
-    set_state("launchpad-buildd.needs_install")
+    remove_state("launchpad-buildd.installed")
 
 
 @hook("upgrade-charm", "config-changed")
 def mark_needs_install():
-    set_state("launchpad-buildd.needs_install")
-
-
-@when_not("apt.needs_update")
-@when("launchpad-buildd.needs_install")
-def install_packages():
-    cache_dir = os.path.join(hookenv.charm_dir(), "cache")
-    host.mkdir(cache_dir, perms=0o755)
-    to_install = []
-    packages = ["launchpad-buildd", "python3-lpbuildd"]
-    options = ["--option=Dpkg::Options::=--force-confold"]
-    resource_paths = [hookenv.resource_get(package) for package in packages]
-    if all(path and os.path.getsize(path) for path in resource_paths):
-        # Install from resources.
-        changed = False
-        for package, resource_path in zip(packages, resource_paths):
-            local_path = os.path.join(cache_dir, f"{package}.deb")
-            to_install.append((local_path, resource_path))
-            if host.file_hash(local_path) != host.file_hash(resource_path):
-                changed = True
-        if not changed:
-            return
-        options.append("--reinstall")
-    else:
-        # We don't have resource-provided packages, so just install from the
-        # PPA.
-        to_install.extend([(None, package) for package in packages])
-    new_paths = [new_path for _, new_path in to_install]
-    try:
-        status_set(None, f"Installing {','.join(packages)}")
-        fetch.apt_unhold(packages)
-        fetch.apt_install(new_paths, options=options)
-        fetch.apt_hold(packages)
-    except subprocess.CalledProcessError:
-        status_set(
-            "blocked", f"Unable to install packages {','.join(packages)}")
-    else:
-        for local_path, resource_path in to_install:
-            if local_path is not None:
-                shutil.copy2(resource_path, local_path)
-        # ntp.buildd isn't likely to work outside of the Canonical
-        # datacentre, and LXD containers can't set the system time.  Let's
-        # just not worry about NTP.
-        config_path = "/etc/launchpad-buildd/default"
-        with open(config_path) as config_file:
-            config = config_file.read()
-        config = re.sub(r"^ntphost = .*", "ntphost = ", config, flags=re.M)
-        with open(config_path + ".new", "w") as new_config_file:
-            new_config_file.write(config)
-        os.rename(config_path + ".new", config_path)
-        remove_state("launchpad-buildd.needs_install")
-        set_state("launchpad-buildd.installed")
+    remove_state("launchpad-buildd.installed")
+
+
+@when("apt.installed.launchpad-buildd")
+@when_not("launchpad-buildd.installed")
+def configure_launchpad_buildd():
+    # ntp.buildd isn't likely to work outside of the Canonical datacentre,
+    # and LXD containers can't set the system time.  Let's just not worry
+    # about NTP.
+    config_path = "/etc/launchpad-buildd/default"
+    with open(config_path) as config_file:
+        config = config_file.read()
+    config = re.sub(r"^ntphost = .*", "ntphost = ", config, flags=re.M)
+    with open(config_path + ".new", "w") as new_config_file:
+        new_config_file.write(config)
+    os.rename(config_path + ".new", config_path)
+    set_state("launchpad-buildd.installed")
 
 
 @when("apt.installed.bzr-builder")