Fix Nginx postrotate log script

[mathew-c]

Addresses one of the two issues raised here: #315

Per man service:

The existence of an upstart  job  of  the  same name 
as  a script in /etc/init.d will cause the upstart job
to take precedence over the init.d script.

So, invoke-rc.d nginx rotate >/dev/null 2>&1 fails because the rotate function has been removed since the switch of nginx to Upstart. This is seen when running sudo logrotate -vf /etc/logrotate.d/wordpress-sites on a vanilla bedrock/trellis/sage install, which will result in:

error: error running shared postrotate script for '"/srv/www/**/logs/*.log" '

Of note, the script should send the -USR1 signal to the master process to avoid the old worker processes which are shutting down from still using the old log file(s).

[swalkinshaw]

There might be a really obvious reason why I'm wrong, but can't this just be service nginx reload?

[mathew-c]

No, if you use service nginx reload won't work completely. The new worker processes will log to the correct file(s), but the old worker processes won't. They'll continue to use the old log file(s) as long as they hang around.

It won't be much of a discrepancy, but it is there. The busier the site, the bigger the discrepancy.

[louim]

@Alan-c is right. This is the recommendend way for Nginx to handle log file reopening. See: http://nginx.org/en/docs/control.html

Alan, you should do $(<"/var/run/nginx.pid"). See : http://stackoverflow.com/questions/21432883/pid-cat-pidfile-or-read-pid-pidfile

[mathew-c]

Thanks for the reference @louim. I'll clean up the syntax.

[mathew-c]

Or..... I can ask for input. strace does indicate a performance benefit to read pid < file versus cat file. However, after trying everything I can think of to get the command working inside /etc/logrotate.d/wordpress-sites, I'm stuck with variations along the lines of this...

    PIDfile="/var/run/nginx.pid"
    if test ! -f $PIDfile
    then
      read nginxPID < $PIDfile
      kill -USR1 $nginxPID
    fi

or other similar. All attempts I've tried to get $(<"/var/run/nginx.pid") working with the kill command haven't found success. Not sure if it's an issue with syntax or what, but only works if I split it into two lines (read then kill) and explicitly use read. Depending on what I've tried I get either nothing at all (no error/no successful kill command) or permission denied errors.

Any thoughts?

[mathew-c]

@louim Thanks for looking at it. I can get it to work outside ok, but for some reason that is eluding me it's failing inside, including on fresh installs. FWIW, a simple echo as opposed to kill fails inside as well - no output.

[louim]

I just looked a little bit more into it. nginx service actually have a rotate which seems to be exactly what we want. @Alan-c want to try it?

[mathew-c]

I'll test it out. I had seen a minor reference to it at Stack Exchange when I initially looked at the issue, but I stuck with the kill command based-solution I'd used before.

According to Nginx Essentials [ that's a clean link, not an affiliate one ] on p.40, rotate sends a USR1 signal, so it should be good.

[louim]

Yep, I checked what the rotate command does:

rotate_logs() {
    # Rotate log files
    start-stop-daemon --stop --signal USR1 --quiet --pidfile $PID --name $NAME
    return 0
}

[mathew-c]

Tests work nicely. Good catch, @louim. Command even adds brings a nice message to logrotate -d: * Re-opening nginx log files nginx.

[mathew-c]

UUOC award presented to the first commit. I humbly accept on it's behalf. 😏 See above convo on the outdated diff.

[austinpray]

This is sick

[swalkinshaw]

@Alan-c possible to get a squash? Looks good 👍

[mathew-c]

Squashed it.

[swalkinshaw]

Thanks!