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

Proposed by Bryce Harrington
Status: Merged
Approved by: Bryce Harrington
Approved revision: a6873885675d0b34f76c9e7baac224c4f6da79cd
Merge reported by: Christian Ehrhardt 
Merged at revision: a6873885675d0b34f76c9e7baac224c4f6da79cd
Proposed branch: ~bryce/ubuntu/+source/apache2:sru-lp1832182-graceful-xenial
Merge into: ubuntu/+source/apache2:ubuntu/xenial-devel
Diff against target: 80 lines (+43/-3)
2 files modified
debian/apache2ctl (+36/-3)
debian/changelog (+7/-0)
Reviewer Review Type Date Requested Status
Christian Ehrhardt  (community) Approve
git-ubuntu developers Pending
Canonical Server Pending
Review via email: mp+393678@code.launchpad.net

Description of the change

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

In bionic and newer, the 'start' command has already been updated to work with systemd, but that fix appears to have not been backported to xenial. (I'm not certain why; guessing it's just an oversight?)

This branch backports that older fix along with this expansion of the fix to also cover 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

n.b. I will be SRUing this branch along with a fix for https://bugs.launchpad.net/ubuntu/+source/apache2/+bug/1899611, but doing this MP in isolation for review convenience.

To post a comment you must log in.
Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

This MP only has the changelog commit - forgot something to push?

Revision history for this message
Christian Ehrhardt  (paelzer) :
review: Needs Fixing
826c7d3... by Bryce Harrington

  * d/apache2ctl: Use systemd for start and graceful if in use.
    (LP: #1832182)

Revision history for this message
Bryce Harrington (bryce) wrote :

Thanks for catching that. This branch is slightly different from the others in that it didn't have the prior fix for start, and I just forgot to commit the combined changes.

Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

In regard to the discussion we had in the MPs for newer releases - in Xenial start didn't issue the messages yet. We could now either go:
a) on the SRU let us not add stdout messages to start&graceful
or
b) let us add the same that was added in later releases (matches what is proposed right now)

Should we maybe get an SRU member to comment on their preference before uploading to proposed?

+1 from the MP review POV.

review: Approve
Revision history for this message
Bryce Harrington (bryce) wrote :

I've gone ahead and uploaded it. Since the messages print only for the currently bugged case I don't think the extra verbosity is going to be an issue. But I'll mention it in the SRU to highlight the situation for the SRU reviewer. I'm combining this with the conf file fix for MP 1899611 so there may end up being some discussion on this already.

Thanks again for the reviews.

Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

This has been merged, pushed to proposed and removed from there again.
Never the less in regard to the MP - this is merged.

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 63db198..845b40c 100755
3--- a/debian/apache2ctl
4+++ b/debian/apache2ctl
5@@ -131,6 +131,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@@ -140,7 +152,18 @@ start)
25 # ssl_scache shouldn't be here if we're just starting up.
26 # (this is bad if there are several apache2 instances running)
27 rm -f ${APACHE_RUN_DIR:-/var/run/apache2}/*ssl_scache*
28- $HTTPD ${APACHE_ARGUMENTS} -k $ARGV
29+
30+ if need_systemd; then
31+ # If running on systemd we should not start httpd without systemd
32+ # or systemd will get confused about the status of httpd.
33+ echo "Invoking 'systemctl start ${APACHE_SYSTEMD_SERVICE}'."
34+ echo "Use 'systemctl status ${APACHE_SYSTEMD_SERVICE}' for more info."
35+ systemctl start "${APACHE_SYSTEMD_SERVICE}"
36+ else
37+ unset APACHE_STARTED_BY_SYSTEMD
38+ ${HTTPD} ${APACHE_ARGUMENTS} -k "${ARGV}"
39+ fi
40+
41 ERROR=$?
42 ;;
43 stop|graceful-stop)
44@@ -149,9 +172,19 @@ stop|graceful-stop)
45 ;;
46 restart|graceful)
47 if $HTTPD ${APACHE_ARGUMENTS} -t 2> /dev/null ; then
48- $HTTPD ${APACHE_ARGUMENTS} -k $ARGV
49+ if need_systemd; then
50+ # If running on systemd we should not directly restart httpd since
51+ # systemd would be confused about httpd's status.
52+ # (See LP: #1832182)
53+ echo "Invoking 'systemctl restart ${APACHE_SYSTEMD_SERVICE}'."
54+ echo "Use 'systemctl status ${APACHE_SYSTEMD_SERVICE}' for more info."
55+ systemctl restart "${APACHE_SYSTEMD_SERVICE}"
56+ else
57+ unset APACHE_STARTED_BY_SYSTEMD
58+ ${HTTPD} ${APACHE_ARGUMENTS} -k "${ARGV}"
59+ fi
60 else
61- $HTTPD ${APACHE_ARGUMENTS} -t
62+ ${HTTPD} ${APACHE_ARGUMENTS} -t
63 fi
64 ERROR=$?
65 ;;
66diff --git a/debian/changelog b/debian/changelog
67index 659f7b8..502810f 100644
68--- a/debian/changelog
69+++ b/debian/changelog
70@@ -1,3 +1,10 @@
71+apache2 (2.4.18-2ubuntu3.18) xenial; urgency=medium
72+
73+ * d/apache2ctl: Use systemd for start and graceful if in use.
74+ (LP: #1832182)
75+
76+ -- Bryce Harrington <bryce@canonical.com> Fri, 13 Nov 2020 01:36:15 +0000
77+
78 apache2 (2.4.18-2ubuntu3.17) xenial-security; urgency=medium
79
80 * SECURITY UPDATE: mod_rewrite redirect issue

Subscribers

People subscribed via source and target branches