Skip to content

all: manage docker group with systemd-sysusers #1187

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
p5 wants to merge 1 commit into from
Closed

all: manage docker group with systemd-sysusers #1187

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 6 additions & 13 deletions deb/common/docker-ce.postinst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,19 +2,12 @@
set -e

case "$1" in
configure)
if [ -z "$2" ]; then
if ! getent group docker > /dev/null; then
groupadd --system docker
fi
fi
;;
abort-*)
# How'd we get here??
exit 1
;;
*)
;;
configure) ;;
abort-*)
# How'd we get here??
exit 1
;;
*) ;;
Comment on lines +5 to +10
Copy link
Copy Markdown
Member

@thaJeztah thaJeztah May 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Effectively, this is now equivalent to;

Suggested change
configure) ;;
abort-*)
# How'd we get here??
exit 1
;;
*) ;;
abort-*)
# How'd we get here??
exit 1
;;
*) ;;

Or... basically; we don't have anything custom remaining now, so I think we can remove the docker-ce.postinst file altogether.

I just tried if it's happy if the file isn't there (because the #DEBHELPER# comment below is what's used to insert generated bits, and it looks like it is; removing the file will still produce a postinst script, just with the case removed (which I think doesn't add real value now?) cc @tianon to make sure I'm not hallucinating 😂 ❤️

diff --git a./docker-ce_0.0.0~20250508105329.6f85626-1~debian.13~trixie_arm64-previous/DEBIAN/md5sums b./docker-ce_0.0.0~20250508105329.6f85626-1~debian.13~trixie_arm64/DEBIAN/md5sums
index 04ebf3669b..1db4a3a285 100644
--- a./docker-ce_0.0.0~20250508105329.6f85626-1~debian.13~trixie_arm64-previous/DEBIAN/md5sums
+++ b./docker-ce_0.0.0~20250508105329.6f85626-1~debian.13~trixie_arm64/DEBIAN/md5sums
@@ -1,8 +1,8 @@
-9dba7f8bd7235f337e08c00987edee0c  usr/bin/docker-proxy
-3809c3a45c0b395593de797d0578d68f  usr/bin/dockerd
+01b414b91936c2ac11e3066d0bd92a07  usr/bin/docker-proxy
+12fb9c92f61730cd98f7fc47183fd8d8  usr/bin/dockerd
 6020c4cb3076e677850f6781a7cd1e19  usr/lib/systemd/system/docker.service
 10dee472f4cbf7a37b74df1d0c547417  usr/lib/systemd/system/docker.socket
 8fcccc716a2aa339573bcac200c2d8ff  usr/libexec/docker/docker-init
 7fc9bfe14206ba304c222b144cebc0c8  usr/share/doc/docker-ce/README.md
-599a382c17777a038c522f2aa8c692e1  usr/share/doc/docker-ce/changelog.Debian.gz
+9c8df20c53886167e2c6e768b52d9537  usr/share/doc/docker-ce/changelog.Debian.gz
 fd1b6d022b18abbe978be6e7fcc0a28b  usr/share/man/man8/dockerd.8.gz
diff --git a./docker-ce_0.0.0~20250508105329.6f85626-1~debian.13~trixie_arm64-previous/DEBIAN/postinst b./docker-ce_0.0.0~20250508105329.6f85626-1~debian.13~trixie_arm64/DEBIAN/postinst
index e2a38e5072..f344702c3a 100644
--- a./docker-ce_0.0.0~20250508105329.6f85626-1~debian.13~trixie_arm64-previous/DEBIAN/postinst
+++ b./docker-ce_0.0.0~20250508105329.6f85626-1~debian.13~trixie_arm64/DEBIAN/postinst
@@ -1,15 +1,5 @@
 #!/bin/sh
 set -e
-
-case "$1" in
-configure) ;;
-abort-*)
-	# How'd we get here??
-	exit 1
-	;;
-*) ;;
-esac
-
 # Automatically added by dh_apparmor/4.1.0-1
 if [ "$1" = "configure" ]; then
     APP_PROFILE="/etc/apparmor.d/docker-ce"
@@ -92,4 +82,3 @@ if [ "$1" = "configure" ] || [ "$1" = "abort-upgrade" ] || [ "$1" = "abort-decon
 	fi
 fi
 # End automatically added section
-

Copy link
Copy Markdown
Contributor

@tianon tianon May 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct if we're not doing anything else, we should just drop the file completely 🚀

esac

#DEBHELPER#
6 changes: 6 additions & 0 deletions deb/common/rules
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,12 @@ override_dh_auto_install:
install -D -p -m 0755 engine/contrib/dockerd-rootless-setuptool.sh debian/docker-ce-rootless-extras/usr/bin/dockerd-rootless-setuptool.sh
# TODO: how can we install vpnkit?

# install systemd sysusers config
mkdir -p debian/docker-ce/etc/sysusers.d
echo "g docker -" >> debian/docker-ce/etc/sysusers.d/docker.conf
chmod 0644 debian/docker-ce/etc/sysusers.d/docker.conf
# install -D -p -m 0644 engine/contrib/systemd-sysusers/docker.conf debian/docker-ce/etc/sysusers.d/docker.conf
Comment on lines +127 to +130
Copy link
Copy Markdown
Member

@thaJeztah thaJeztah May 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moby PR was merged, so this can now use;

Suggested change
mkdir -p debian/docker-ce/etc/sysusers.d
echo "g docker -" >> debian/docker-ce/etc/sysusers.d/docker.conf
chmod 0644 debian/docker-ce/etc/sysusers.d/docker.conf
# install -D -p -m 0644 engine/contrib/systemd-sysusers/docker.conf debian/docker-ce/etc/sysusers.d/docker.conf
install -D -p -m 0644 engine/contrib/systemd-sysusers/docker.conf debian/docker-ce/etc/sysusers.d/docker.conf

Copy link
Copy Markdown
Contributor

@tianon tianon May 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should actually be a (dangling in Git) symlink and use debhelper: https://manpages.debian.org/bookworm/debhelper/dh_installsysusers.1.en.html

Copy link
Copy Markdown
Contributor

@tianon tianon May 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(most/all of this section should be using dh_install too, but that's orthogonal)


override_dh_installinit:
# use "docker" as our service name, not "docker-ce"
dh_installinit --name=docker
Expand Down
10 changes: 7 additions & 3 deletions rpm/SPECS/docker-ce.spec
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,12 @@ install -D -p -m 0755 $(readlink -f engine/bundles/dynbinary-daemon/dockerd) ${R
install -D -p -m 0755 $(readlink -f engine/bundles/dynbinary-daemon/docker-proxy) ${RPM_BUILD_ROOT}%{_bindir}/docker-proxy
install -D -p -m 0755 /usr/local/bin/docker-init ${RPM_BUILD_ROOT}%{_libexecdir}/docker/docker-init

# install systemd sysusers config
mkdir -p ${RPM_BUILD_ROOT}%{_sysusersdir}
echo "g docker -" >> ${RPM_BUILD_ROOT}%{_sysusersdir}/docker.conf
chmod 0644 ${RPM_BUILD_ROOT}%{_sysusersdir}/docker.conf
# install -D -p -m 0644 engine/contrib/systemd-sysusers/docker.conf ${RPM_BUILD_ROOT}%{_sysusersdir}/docker.conf
Comment on lines +87 to +91
Copy link
Copy Markdown
Member

@thaJeztah thaJeztah May 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here

Suggested change
# install systemd sysusers config
mkdir -p ${RPM_BUILD_ROOT}%{_sysusersdir}
echo "g docker -" >> ${RPM_BUILD_ROOT}%{_sysusersdir}/docker.conf
chmod 0644 ${RPM_BUILD_ROOT}%{_sysusersdir}/docker.conf
# install -D -p -m 0644 engine/contrib/systemd-sysusers/docker.conf ${RPM_BUILD_ROOT}%{_sysusersdir}/docker.conf
# install systemd sysusers config
install -D -p -m 0644 engine/contrib/systemd-sysusers/docker.conf ${RPM_BUILD_ROOT}%{_sysusersdir}/docker.conf


# install systemd scripts
install -D -p -m 0644 engine/contrib/init/systemd/docker.service ${RPM_BUILD_ROOT}%{_unitdir}/docker.service
install -D -p -m 0644 engine/contrib/init/systemd/docker.socket ${RPM_BUILD_ROOT}%{_unitdir}/docker.socket
Expand All @@ -100,14 +106,12 @@ mkdir -p ${RPM_BUILD_ROOT}/etc/docker
%{_libexecdir}/docker/docker-init
%{_unitdir}/docker.service
%{_unitdir}/docker.socket
%{_sysusersdir}/docker.conf
%{_mandir}/man*/*
%dir /etc/docker

%post
%systemd_post docker.service
if ! getent group docker > /dev/null; then
groupadd --system docker
fi

%preun
%systemd_preun docker.service docker.socket
Expand Down