Merge ~bryce/ubuntu/+source/apache2:sru-lp1832182-graceful-groovy into ubuntu/+source/apache2:ubuntu/devel

[Bryce Harrington]

Fix for a server-next Apache2 bug. The objective is to SRU this to all past LTS releases, so please review with that intent in mind.

The bug description for LP: #1832182 is prepped for the SRU, so I likewise would appreciate a detailed review of that text.

PPA: https://launchpad.net/~bryce/+archive/ubuntu/apache2-sru-lp1832182/

[Christian Ehrhardt  (paelzer)]

SRU Template - LGTM
Changelog - LGTM

Actual change:
- ack to move the systemd check to a function

I needed some time for "APACHE_STARTED_BY_SYSTEMD".
That is set in the environment by the .service files.
So I assume if APACHE_STARTED_BY_SYSTEMD isn't set we probe (the new function).
But if it is set, then we exit 1 which means "no need for systemd"

is that
a) to unbreak loops where we start this from systemd, starting systemd, starting systemd, ...
   Then it makes sense but I wanted to ask ito be sure
b) wrong and should actually "exit 0" if APACHE_STARTED_BY_SYSTEMD is set?

Further in the change there is some whitespace damage with the "exit" calls not using spaces but tabs.

If you could fix the latter and explain the former then this should be fine to go.

review: Needs Fixing

[Christian Ehrhardt  (paelzer)]

Any updates on that ? it clutters my "active reviews" page :-P

[Bryce Harrington (bryce)]

Sorry, yeah I realized from your review that my original implementation was totally borked so have recoded it entirely. I noticed another corner case where the service state could get forgotten, so thought I should do some broader testing before reproposing the MP. Unfortunately, the past couple weeks were heavily side-tracked with other higher urgencies, but this (and php-parser) are back at the top of the stack so hope to have the MP up tomorrow or Monday.

[Bryce Harrington (bryce)]

Superseded by https://code.launchpad.net/~bryce/ubuntu/+source/apache2/+git/apache2/+merge/393426

5b03b08... by Bryce Harrington on 2020-11-13

changelog

Unmerged commits

5b03b08... by Bryce Harrington on 2020-11-13

changelog

37b368f... by Bryce Harrington on 2020-10-05

* d/apache2ctl: Also use systemd for graceful if it is in use. (LP: #1832182) - This extends an earlier fix for the start command to behave similarly for restart / graceful. Fixes service failures on unattended upgrade.

diff --git a/debian/apache2ctl b/debian/apache2ctl
index 404b9f9..1358f2a 100755
--- a/debian/apache2ctl
+++ b/debian/apache2ctl
@@ -143,6 +143,18 @@ mkdir_chown () {
     fi
 }
 
+need_systemd () {
+    # Detect if systemd is in use and should be used for managing
+    # the Apache2 httpd service.  Returns 0 if so, 1 otherwise.
+    if [ -z "${APACHE_STARTED_BY_SYSTEMD}" ]; then
+	case "$(readlink -f /proc/1/exe)" in
+	    *systemd*)
+		return 0
+		;;
+	esac
+    fi
+    return 1
+}
 
 [ ! -d ${APACHE_RUN_DIR:-/var/run/apache2} ] && mkdir -p ${APACHE_RUN_DIR:-/var/run/apache2}
 [ ! -d ${APACHE_LOCK_DIR:-/var/lock/apache2} ] && mkdir_chown ${APACHE_RUN_USER:-www-data} ${APACHE_LOCK_DIR:-/var/lock/apache2}
@@ -153,38 +165,38 @@ start)
     # (this is bad if there are several apache2 instances running)
     rm -f ${APACHE_RUN_DIR:-/var/run/apache2}/*ssl_scache*
 
-    need_systemd=false
-    if [ -z "$APACHE_STARTED_BY_SYSTEMD" ] ; then
-        case "$(readlink -f /proc/1/exe)" in
-        *systemd*)
-            need_systemd=true
-            ;;
-        *)
-            ;;
-        esac
-    fi
-    if $need_systemd ; then
+    if need_systemd; then
         # If running on systemd we should not start httpd without systemd
         # or systemd will get confused about the status of httpd.
-        echo "Invoking 'systemctl start $APACHE_SYSTEMD_SERVICE'."
-        echo "Use 'systemctl status $APACHE_SYSTEMD_SERVICE' for more info."
-        systemctl start "$APACHE_SYSTEMD_SERVICE"
+        echo "Invoking 'systemctl start ${APACHE_SYSTEMD_SERVICE}'."
+        echo "Use 'systemctl status ${APACHE_SYSTEMD_SERVICE}' for more info."
+        systemctl start "${APACHE_SYSTEMD_SERVICE}"
     else
         unset APACHE_STARTED_BY_SYSTEMD
-        $HTTPD ${APACHE_ARGUMENTS} -k "$ARGV"
+        ${HTTPD} ${APACHE_ARGUMENTS} -k "${ARGV}"
     fi
 
     ERROR=$?
     ;;
 stop|graceful-stop)
-    $HTTPD ${APACHE_ARGUMENTS} -k "$ARGV"
+    ${HTTPD} ${APACHE_ARGUMENTS} -k "$ARGV"
     ERROR=$?
     ;;
 restart|graceful)
     if $HTTPD ${APACHE_ARGUMENTS} -t 2> /dev/null ; then
-        $HTTPD ${APACHE_ARGUMENTS} -k "$ARGV"
+        if need_systemd; then
+            # If running on systemd we should not directly restart httpd since
+            # systemd would be confused about httpd's status.
+            # (See LP: #1832182)
+            echo "Invoking 'systemctl restart ${APACHE_SYSTEMD_SERVICE}'."
+            echo "Use 'systemctl status ${APACHE_SYSTEMD_SERVICE}' for more info."
+            systemctl restart "${APACHE_SYSTEMD_SERVICE}"
+        else
+            unset APACHE_STARTED_BY_SYSTEMD
+            ${HTTPD} ${APACHE_ARGUMENTS} -k "${ARGV}"
+        fi
     else
-        $HTTPD ${APACHE_ARGUMENTS} -t
+        ${HTTPD} ${APACHE_ARGUMENTS} -t
     fi
     ERROR=$?
     ;;
diff --git a/debian/changelog b/debian/changelog
index 6295e4e..517a355 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,13 @@
+apache2 (2.4.46-1ubuntu1.1) groovy; urgency=medium
+
+  * d/apache2ctl: Also use systemd for graceful if it is in use.
+    (LP: #1832182)
+    - This extends an earlier fix for the start command to behave
+      similarly for restart / graceful.  Fixes service failures on
+      unattended upgrade.
+
+ -- Bryce Harrington <[e-mail]>  Fri, 13 Nov 2020 01:36:38 +0000
+
 apache2 (2.4.46-1ubuntu1) groovy; urgency=medium
 
   * Merge with Debian unstable. Remaining changes: