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

Proposed by Bryce Harrington on 2020-11-13
Status: Approved
Approved by: Bryce Harrington on 2020-11-19
Approved revision: 42766cb730542fe227b855b8eb553f15820d6795
Proposed branch: ~bryce/ubuntu/+source/apache2:sru-lp1832182-graceful-bionic
Merge into: ubuntu/+source/apache2:ubuntu/bionic-devel
Diff against target: 98 lines (+40/-18)
2 files modified
debian/apache2ctl (+30/-18)
debian/changelog (+10/-0)
Reviewer Review Type Date Requested Status
Christian Ehrhardt  2020-11-13 Approve on 2020-11-17
Ubuntu Server Dev import team 2020-11-13 Pending
Canonical Server Team 2020-11-13 Pending
Review via email: mp+393679@code.launchpad.net

Description of the change

Backport to bionic of fix from
https://code.launchpad.net/~bryce/ubuntu/+source/apache2/+git/apache2/+merge/393426

The start command was updated to work with systemd earlier; this extends the same fix for the 'graceful' command.

PPA: https://launchpad.net/~bryce/+archive/ubuntu/apache2-sru-lp1832182
SRU: https://bugs.launchpad.net/ubuntu/+source/apache2/+bug/1832182
Forwarded to Debian: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=927302

To post a comment you must log in.
Christian Ehrhardt  (paelzer) wrote :

As reference, that old fix for start was at https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=852543 in pkg/import/2.4.25-3

Christian Ehrhardt  (paelzer) wrote :

I remember reviewing the same fix for another release.
This one still LGTM and makes sense to avoid the direct restart breaking status detection.

Also changelog and version are ok.

The only thing I didn't get to this time is testing, have you run your PPA through upgrade tests already?

review: Approve
Christian Ehrhardt  (paelzer) wrote :
review: Needs Information
Christian Ehrhardt  (paelzer) wrote :

Approved per discussion in the groovy MP

review: Approve
Bryce Harrington (bryce) wrote :

Sounds good, I've tagged, pushed, and uploaded this.

Unmerged commits

42766cb... by Bryce Harrington on 2020-11-13

changelog

9587404... 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.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/debian/apache2ctl b/debian/apache2ctl
2index 404b9f9..1358f2a 100755
3--- a/debian/apache2ctl
4+++ b/debian/apache2ctl
5@@ -143,6 +143,18 @@ mkdir_chown () {
6 fi
7 }
8
9+need_systemd () {
10+ # Detect if systemd is in use and should be used for managing
11+ # the Apache2 httpd service. Returns 0 if so, 1 otherwise.
12+ if [ -z "${APACHE_STARTED_BY_SYSTEMD}" ]; then
13+ case "$(readlink -f /proc/1/exe)" in
14+ *systemd*)
15+ return 0
16+ ;;
17+ esac
18+ fi
19+ return 1
20+}
21
22 [ ! -d ${APACHE_RUN_DIR:-/var/run/apache2} ] && mkdir -p ${APACHE_RUN_DIR:-/var/run/apache2}
23 [ ! -d ${APACHE_LOCK_DIR:-/var/lock/apache2} ] && mkdir_chown ${APACHE_RUN_USER:-www-data} ${APACHE_LOCK_DIR:-/var/lock/apache2}
24@@ -153,38 +165,38 @@ start)
25 # (this is bad if there are several apache2 instances running)
26 rm -f ${APACHE_RUN_DIR:-/var/run/apache2}/*ssl_scache*
27
28- need_systemd=false
29- if [ -z "$APACHE_STARTED_BY_SYSTEMD" ] ; then
30- case "$(readlink -f /proc/1/exe)" in
31- *systemd*)
32- need_systemd=true
33- ;;
34- *)
35- ;;
36- esac
37- fi
38- if $need_systemd ; then
39+ if need_systemd; then
40 # If running on systemd we should not start httpd without systemd
41 # or systemd will get confused about the status of httpd.
42- echo "Invoking 'systemctl start $APACHE_SYSTEMD_SERVICE'."
43- echo "Use 'systemctl status $APACHE_SYSTEMD_SERVICE' for more info."
44- systemctl start "$APACHE_SYSTEMD_SERVICE"
45+ echo "Invoking 'systemctl start ${APACHE_SYSTEMD_SERVICE}'."
46+ echo "Use 'systemctl status ${APACHE_SYSTEMD_SERVICE}' for more info."
47+ systemctl start "${APACHE_SYSTEMD_SERVICE}"
48 else
49 unset APACHE_STARTED_BY_SYSTEMD
50- $HTTPD ${APACHE_ARGUMENTS} -k "$ARGV"
51+ ${HTTPD} ${APACHE_ARGUMENTS} -k "${ARGV}"
52 fi
53
54 ERROR=$?
55 ;;
56 stop|graceful-stop)
57- $HTTPD ${APACHE_ARGUMENTS} -k "$ARGV"
58+ ${HTTPD} ${APACHE_ARGUMENTS} -k "$ARGV"
59 ERROR=$?
60 ;;
61 restart|graceful)
62 if $HTTPD ${APACHE_ARGUMENTS} -t 2> /dev/null ; then
63- $HTTPD ${APACHE_ARGUMENTS} -k "$ARGV"
64+ if need_systemd; then
65+ # If running on systemd we should not directly restart httpd since
66+ # systemd would be confused about httpd's status.
67+ # (See LP: #1832182)
68+ echo "Invoking 'systemctl restart ${APACHE_SYSTEMD_SERVICE}'."
69+ echo "Use 'systemctl status ${APACHE_SYSTEMD_SERVICE}' for more info."
70+ systemctl restart "${APACHE_SYSTEMD_SERVICE}"
71+ else
72+ unset APACHE_STARTED_BY_SYSTEMD
73+ ${HTTPD} ${APACHE_ARGUMENTS} -k "${ARGV}"
74+ fi
75 else
76- $HTTPD ${APACHE_ARGUMENTS} -t
77+ ${HTTPD} ${APACHE_ARGUMENTS} -t
78 fi
79 ERROR=$?
80 ;;
81diff --git a/debian/changelog b/debian/changelog
82index 3537cc5..de2df68 100644
83--- a/debian/changelog
84+++ b/debian/changelog
85@@ -1,3 +1,13 @@
86+apache2 (2.4.29-1ubuntu4.15) bionic; urgency=medium
87+
88+ * d/apache2ctl: Also use systemd for graceful if it is in use.
89+ (LP: #1832182)
90+ - This extends an earlier fix for the start command to behave
91+ similarly for restart / graceful. Fixes service failures on
92+ unattended upgrade.
93+
94+ -- Bryce Harrington <bryce@canonical.com> Fri, 13 Nov 2020 01:36:35 +0000
95+
96 apache2 (2.4.29-1ubuntu4.14) bionic-security; urgency=medium
97
98 * SECURITY UPDATE: mod_rewrite redirect issue

Subscribers

People subscribed via source and target branches