Skip to content

[master] deb: use upstream init script#926

Merged
neersighted merged 2 commits into
docker:masterfrom
neersighted:upstream_init
Jul 21, 2023
Merged

[master] deb: use upstream init script#926
neersighted merged 2 commits into
docker:masterfrom
neersighted:upstream_init

Conversation

@neersighted
Copy link
Copy Markdown
Member

@neersighted neersighted commented Jul 21, 2023

As per the title; prefer the upstream version of the init script.

This resolves #899.

@neersighted neersighted requested review from thaJeztah and tianon July 21, 2023 16:47
tianon
tianon previously approved these changes Jul 21, 2023
@neersighted
Copy link
Copy Markdown
Member Author

--- docker-ce-packaging/deb/common/docker-ce.docker.init	2023-07-21 10:51:08
+++ moby/contrib/init/sysvinit-debian/docker	2023-07-18 13:15:44
@@ -44,14 +44,6 @@
 	exit 1
 fi
 
-check_init() {
-	# see also init_is_upstart in /lib/lsb/init-functions (which isn't available in Ubuntu 12.04, or we'd use it directly)
-	if [ -x /sbin/initctl ] && /sbin/initctl version 2>/dev/null | grep -q upstart; then
-		log_failure_msg "$DOCKER_DESC is managed via upstart, try using service $BASE $1"
-		exit 1
-	fi
-}
-
 fail_unless_root() {
 	if [ "$(id -u)" != '0' ]; then
 		log_failure_msg "$DOCKER_DESC must be run as root"
@@ -59,37 +51,10 @@
 	fi
 }
 
-cgroupfs_mount() {
-	# see also https://github.com/tianon/cgroupfs-mount/blob/master/cgroupfs-mount
-	if grep -v '^#' /etc/fstab | grep -q cgroup \
-		|| [ ! -e /proc/cgroups ] \
-		|| [ ! -d /sys/fs/cgroup ]; then
-		return
-	fi
-	if ! mountpoint -q /sys/fs/cgroup; then
-		mount -t tmpfs -o uid=0,gid=0,mode=0755 cgroup /sys/fs/cgroup
-	fi
-	(
-		cd /sys/fs/cgroup
-		for sys in $(awk '!/^#/ { if ($4 == 1) print $1 }' /proc/cgroups); do
-			mkdir -p $sys
-			if ! mountpoint -q $sys; then
-				if ! mount -n -t cgroup -o $sys cgroup $sys; then
-					rmdir $sys || true
-				fi
-			fi
-		done
-	)
-}
-
 case "$1" in
 	start)
-		check_init
-		
 		fail_unless_root
 
-		cgroupfs_mount
-
 		touch "$DOCKER_LOGFILE"
 		chgrp docker "$DOCKER_LOGFILE"
 
@@ -110,14 +75,13 @@
 			--pidfile "$DOCKER_SSD_PIDFILE" \
 			--make-pidfile \
 			-- \
-				-p "$DOCKER_PIDFILE" \
-				$DOCKER_OPTS \
-					>> "$DOCKER_LOGFILE" 2>&1
+			-p "$DOCKER_PIDFILE" \
+			$DOCKER_OPTS \
+			>> "$DOCKER_LOGFILE" 2>&1
 		log_end_msg $?
 		;;
 
 	stop)
-		check_init
 		fail_unless_root
 		if [ -f "$DOCKER_SSD_PIDFILE" ]; then
 			log_begin_msg "Stopping $DOCKER_DESC: $BASE"
@@ -129,9 +93,8 @@
 		;;
 
 	restart)
-		check_init
 		fail_unless_root
-		docker_pid=`cat "$DOCKER_SSD_PIDFILE" 2>/dev/null`
+		docker_pid=$(cat "$DOCKER_SSD_PIDFILE" 2> /dev/null || true)
 		[ -n "$docker_pid" ] \
 			&& ps -p $docker_pid > /dev/null 2>&1 \
 			&& $0 stop
@@ -139,13 +102,11 @@
 		;;
 
 	force-reload)
-		check_init
 		fail_unless_root
 		$0 restart
 		;;
 
 	status)
-		check_init
 		status_of_proc -p "$DOCKER_SSD_PIDFILE" "$DOCKERD" "$DOCKER_DESC"
 		;;
 

thaJeztah
thaJeztah previously approved these changes Jul 21, 2023
Copy link
Copy Markdown
Member

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

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

LGTM if green 👍

@neersighted neersighted dismissed stale reviews from thaJeztah and tianon via 8bd9ca4 July 21, 2023 16:56
@neersighted
Copy link
Copy Markdown
Member Author

Went ahead and did it for the default file as well; diff is empty.

@thaJeztah
Copy link
Copy Markdown
Member

was about to ask; one of the symlinks is to a directory, not a file, that's correct?

Signed-off-by: Bjorn Neergaard <bjorn.neergaard@docker.com>
Signed-off-by: Bjorn Neergaard <bjorn.neergaard@docker.com>
@neersighted
Copy link
Copy Markdown
Member Author

This entire "deliberate dangling symlink" business is dangerous and #925 is definitely a priority here, I'd say.

@thaJeztah
Copy link
Copy Markdown
Member

thaJeztah commented Jul 21, 2023

For the 24.0 branch, the diff looks like this;

diff --git a/deb/common/docker-ce.docker.init b/Users/thajeztah/Projects/docker/contrib/init/sysvinit-debian/docker
index 9c8fa6be73..90dbe3c956 100755
--- a/deb/common/docker-ce.docker.init
+++ b/Users/thajeztah/Projects/docker/contrib/init/sysvinit-debian/docker
@@ -46,7 +46,7 @@ fi

 check_init() {
        # see also init_is_upstart in /lib/lsb/init-functions (which isn't available in Ubuntu 12.04, or we'd use it directly)
-       if [ -x /sbin/initctl ] && /sbin/initctl version 2>/dev/null | grep -q upstart; then
+       if [ -x /sbin/initctl ] && /sbin/initctl version 2> /dev/null | grep -q upstart; then
                log_failure_msg "$DOCKER_DESC is managed via upstart, try using service $BASE $1"
                exit 1
        fi
@@ -85,7 +85,7 @@ cgroupfs_mount() {
 case "$1" in
        start)
                check_init
-
+
                fail_unless_root

                cgroupfs_mount
@@ -110,9 +110,9 @@ case "$1" in
                        --pidfile "$DOCKER_SSD_PIDFILE" \
                        --make-pidfile \
                        -- \
-                               -p "$DOCKER_PIDFILE" \
-                               $DOCKER_OPTS \
-                                       >> "$DOCKER_LOGFILE" 2>&1
+                       -p "$DOCKER_PIDFILE" \
+                       $DOCKER_OPTS \
+                       >> "$DOCKER_LOGFILE" 2>&1
                log_end_msg $?
                ;;

@@ -131,7 +131,7 @@ case "$1" in
        restart)
                check_init
                fail_unless_root
-               docker_pid=`cat "$DOCKER_SSD_PIDFILE" 2>/dev/null`
+               docker_pid=$(cat "$DOCKER_SSD_PIDFILE" 2> /dev/null || true)
                [ -n "$docker_pid" ] \
                        && ps -p $docker_pid > /dev/null 2>&1 \
                        && $0 stop

Copy link
Copy Markdown
Contributor

@tianon tianon left a comment

Choose a reason for hiding this comment

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

(verified both paths via copy paste into my checkout of docker/docker and ls -l 👍)

Copy link
Copy Markdown
Member

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

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

re-LGTM (with the paths fixed 😅)

@neersighted neersighted merged commit 52080d0 into docker:master Jul 21, 2023
@neersighted neersighted deleted the upstream_init branch July 21, 2023 17:38
@neersighted neersighted changed the title deb: use upstream init script [master] deb: use upstream init script Jul 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Use upstream init scripts

3 participants