Colin Watson has proposed merging ~cjwatson/launchpad-buildd:snap-lxd-simplify
into launchpad-buildd:master.
Commit message:
Don't require the lxd snap to be installed at postinst time
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~cjwatson/launchpad-buildd/+git/launchpad-buildd/+merge/436285
In commit 679f3234d4c21d12ca5469cbda9242d9fb27c647, I switched from depending
on the lxd .deb to requiring the snap to be installed, with the aim of getting
launchpad-buildd working on jammy. Unfortunately, this turned out to break
upgrades of the riscv64 builders, where we currently handle upgrading the base
VM images using a chroot.
However, the only reason the postinst needed to care about this was so that it
could add the `buildd` user to the `lxd` group, and now that we run
launchpad-buildd from a systemd service we can safely just rely on
`SupplementaryGroups=lxd` giving the service that group membership. Not
actually adding that user to that group avoids problems with the group
membership leaking through `sbuild`, allowing us to drop a chunk of quite
obscure code.
--
Your team Launchpad code reviewers is requested to review the proposed merge of
~cjwatson/launchpad-buildd:snap-lxd-simplify into launchpad-buildd:master.
diff --git a/bin/sbuild-package b/bin/sbuild-package
index 5e3e9f0..b5fc862 100755
--- a/bin/sbuild-package
+++ b/bin/sbuild-package
@@ -51,25 +51,6 @@ getent group sbuild | sudo tee -a chroot-autobuild/etc/group > /dev/null || exit
getent passwd sbuild | sudo tee -a chroot-autobuild/etc/passwd > /dev/null || exit 2
sudo chown sbuild:sbuild chroot-autobuild/build || exit 2
-# schroot calls initgroups(3) when entering a session, which sets
-# supplementary groups to the target user's groups in the host system's
-# /etc/group. As a result, additional group memberships of the buildd user
-# (currently just lxd) outside the chroot are also visible inside the
-# chroot, and must exist in /etc/group there as well or else a few package
-# builds will fail, so we temporarily remove the lxd group membership for
-# the duration of this build. This is all very unfortunate and not very
-# robust; perhaps eventually we should fix this by doing package builds in
-# LXD containers, although that would have its own problems, particularly
-# when bootstrapping new architectures.
-cleanup () {
- sudo adduser --quiet buildd lxd
-}
-trap cleanup EXIT
-# According to deluser(8):
-# 6 The user does not belong to the specified group. No action was
-# performed.
-sudo deluser --quiet buildd lxd || [ $? = 6 ]
-
hostarch=$(dpkg --print-architecture)
UNAME26=""
diff --git a/debian/changelog b/debian/changelog
index 867c9f5..8eb2462 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -2,6 +2,8 @@ launchpad-buildd (227) UNRELEASED; urgency=medium
* Tolerate receiving "builder_constraints": None.
* Check the appropriate server.key path for the LXD snap.
+ * Restructure lxd group membership handling to avoid requiring the lxd
+ snap to be installed at postinst time.
-- Colin Watson <cjwat...@ubuntu.com> Tue, 24 Jan 2023 13:13:27 +0000
diff --git a/debian/launchpad-buildd@.service b/debian/launchpad-buildd@.service
index 9d4b4b2..cdc7a99 100644
--- a/debian/launchpad-buildd@.service
+++ b/debian/launchpad-buildd@.service
@@ -13,10 +13,6 @@ Type=simple
RuntimeDirectory=launchpad-buildd
LogsDirectory=launchpad-buildd
User=buildd
-# The buildd user should normally already be a member of this group, but due
-# to the deluser hacks in sbuild-package it's possible for the group
-# membership to be missing if a non-virtualized builder crashes in the
-# middle of an sbuild job. Make sure of it here.
SupplementaryGroups=lxd
EnvironmentFile=-/etc/default/launchpad-buildd
Environment=BUILDD_CONFIG=/etc/launchpad-buildd/%i
diff --git a/debian/postinst b/debian/postinst
index 84f9a40..7eb86cb 100644
--- a/debian/postinst
+++ b/debian/postinst
@@ -21,11 +21,6 @@ make_buildd()
case "$1" in
configure)
- if [ ! -d /var/snap/lxd ]; then
- echo "LXD is not installed. Run 'snap install lxd' and retry." >&2
- exit 1
- fi
-
getent group buildd >/dev/null 2>&1 ||
addgroup --gid $BUILDDGID buildd
@@ -33,9 +28,26 @@ case "$1" in
adduser --ingroup buildd --disabled-login --gecos 'Buildd user' \
--uid $BUILDDUID ${USER}
adduser --quiet buildd sbuild
- # Note that any additional group memberships here must currently be
- # reflected in the deluser hacks in sbuild-package.
- adduser --quiet buildd lxd
+
+ if dpkg --compare-versions "$2" lt-nl 227~; then
+ # We used to add the buildd user to the lxd group. This had
+ # problems with leaking through sbuild, and it required lxd to
+ # be installed at postinst time, which is problematic now that
+ # lxd is typically installed as a snap, so we now rely entirely
+ # on SupplementaryGroups=lxd in the systemd service. Clean up
+ # the old group membership.
+ code=0
+ sudo deluser --quiet buildd lxd || code=$?
+ # According to deluser(8):
+ # 0 Success: The action was successfully executed.
+ # 3 There is no such group. No action was performed.
+ # 6 The user does not belong to the specified group. No
+ # action was performed.
+ case $code in
+ 0|3|6) ;;
+ *) exit "$code" ;;
+ esac
+ fi
SUDO_VERSION=$(sudo -V | sed -n '/^Sudo version/s/.* //p')
if dpkg --compare-versions $SUDO_VERSION lt 1.7 ||
_______________________________________________
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to : launchpad-reviewers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help : https://help.launchpad.net/ListHelp
