"service nginx upgrade" does not respect custom config file location

[CEbhNwPM]

This affects the RPM postuninstall scriptlet in the nginx.org RPM packages, which executes "service nginx upgrade" when upgrading nginx via dnf.

I have a systemd unit override like this:

ExecStart=
ExecStart=/usr/sbin/nginx -c /etc/nginx/nginx-custom.conf 

I do this to avoid modifying the packaged nginx.conf and have all customized configuration in files that are not tracked by the package manager.

Specifically, this line in /usr/libexec/initscripts/legacy-actions/nginx/upgrade uses the wrong config file:
${nginx} -t -c ${conffile} -q || return 6

This test can fail in any number of scenarios. For example, my nginx-custom.conf defines a custom log_format which is referenced in included conf.d/*.conf files.
In this case, dnf returns "Binary upgrade failed, please check nginx's error.log" and the running process is not upgraded.

It would be much better if the upgrade script used the command line and arguments of the running nginx process instead of guessing, just like it gets the PID file from the systemd unit as well.

[thresheek]

Hi @CEbhNwPM!

Indeed, it's a valid issue. I've transferred it to the repository we use to track changes in nginx packaging.

To fix that, I suppose we could provide an overridable default for conffile in /usr/libexec/initscripts/legacy-actions/nginx/upgrade and in /usr/lib/systemd/system/nginx.service. Then, a system administrator could override it via /etc/sysconfig/nginx.

Something like the following (I'm going to put a PR for it at some point):

diff --git a/rpm/SOURCES/nginx.service b/rpm/SOURCES/nginx.service
index e509a2e..55384a9 100644
--- a/rpm/SOURCES/nginx.service
+++ b/rpm/SOURCES/nginx.service
@@ -7,7 +7,9 @@ Wants=network-online.target
 [Service]
 Type=forking
 PIDFile=/var/run/nginx.pid
-ExecStart=/usr/sbin/nginx -c /etc/nginx/nginx.conf
+Environment="conffile=/etc/nginx/nginx.conf"
+EnvironmentFile=/etc/sysconfig/nginx
+ExecStart=/usr/sbin/nginx -c ${conffile}
 ExecReload=/bin/sh -c "/bin/kill -s HUP $(/bin/cat /var/run/nginx.pid)"
 ExecStop=/bin/sh -c "/bin/kill -s TERM $(/bin/cat /var/run/nginx.pid)"

diff --git a/rpm/SOURCES/nginx.upgrade.sh b/rpm/SOURCES/nginx.upgrade.sh
index 25f5c22..7a881c6 100644
--- a/rpm/SOURCES/nginx.upgrade.sh
+++ b/rpm/SOURCES/nginx.upgrade.sh
@@ -8,7 +8,7 @@ fi

 prog=nginx
 nginx=/usr/sbin/nginx
-conffile=/etc/nginx/nginx.conf
+conffile=${conffile:-/etc/nginx/nginx.conf}
 pidfile=`/usr/bin/systemctl show -p PIDFile nginx.service | sed 's/^PIDFile=//' | tr ' ' '\n'`
 SLEEPSEC=${SLEEPSEC:-1}
 UPGRADEWAITLOOPS=${UPGRADEWAITLOOPS:-5}

I don't like the idea of parsing arguments and CLI, to be honest. I believe it's best to provide a reasonable configuration instead of guessing.

[CEbhNwPM]

Excellent, thank you very much for the quick and clean solution. As long as /etc/sysconfig/nginx is not included in the package, this will work.

[thresheek]

That's actually one of the things I see that packages do differently, some include %config(noreplace) /etc/sysconfig/%name, and some don't. (At least on Alma Linux 9). I tend not to, but need to explore this more.

[thresheek]

Hey @CEbhNwPM - I've pushed the changes to both main and stable branches for our packages. Note that this does not mean the packages will be modified right now - this will only apply to the future releases, e.g. 1.27.4 and 1.26.3 when they're out. There is no ETA for those at this moment.

I can keep this FR open until those are released and notify you of changes.