Add custom logrotate config

[clwluvw]

Proposed changes

Closes: #250

Checklist

• I have read the CONTRIBUTING document
• I have added Molecule tests that prove my fix is effective or that my feature works
• I have checked that all unit tests pass after adding my changes
• If required, I have updated necessary documentation (defaults/main/ and README.md)

[clwluvw]

@alessfg Are these test failures related to my changes? It seems there is a problem with nginx.conf

[alessfg]

They are related to your changes (tests on master are passing), but I can't quite place where the issue is 🤔

[clwluvw]

They are related to your changes (tests on master are passing), but I can't quite place where the issue is

I have just run logrotate on Alpine and CentOS in tests and it gets failed because there is some problems in nginx.conf but I don't change anything in nginx.conf!

[alessfg]

There's a bunch of warning messages, but those won't make the build fail. The actual error seems to be error: error running shared postrotate script for '/var/log/nginx/*.log.

[clwluvw]

There's a bunch of warning messages, but those won't make the build fail. The actual error seems to be error: error running shared postrotate script for '/var/log/nginx/*.log.

@alessfg I think it's because of:

[error] invalid PID number \"\" in \"/var/run/nginx.pid\"\nerror

[alessfg]

That could be related, yeah. Maybe when logrotate tries to run it can't read the NGINX PID? If you run the PR against an Alpine/CentOS(or RHEL) VM does it work (besides the warning messages)? If it works then it could be an issue related to the Docker images for Alpine/CentOS.

[clwluvw]

That could be related, yeah. Maybe when logrotate tries to run it can't read the NGINX PID? If you run the PR against an Alpine/CentOS(or RHEL) VM does it work (besides the warning messages)? If it works then it could be an issue related to the Docker images for Alpine/CentOS.

I'm not enough familiar with Apline/CentOS and I don't know whats going wrong. Do you have any idea about it?

[alessfg]

I'm trying to debug it on my end but it might take some time before I find a solution. Looks like the issue is indeed caused by a PID error, but everything works fine on master, so lograte must have messed up NGINX's PID in Alpine and CentOS somehow. I'm no logrotate expert myself but I'll see if I can figure out what's causing the issue per se.

[magicalyak]

Please add two spaces before the # comment in defaults/main/logrotate.yaml line 14

And for the template, change it to this

{% for path in nginx_logrotate_conf.paths %}
{{ path }}
{% endfor %}
{
{% for option in nginx_logrotate_conf.options %}
    {{ option }}
{% endfor %}
    prerotate
        if [ -d /etc/logrotate.d/httpd-prerotate ]; then \
            run-parts /etc/logrotate.d/httpd-prerotate; \
        fi \
    endscript
    postrotate
{% if ansible_os_family == "Debian" %}
        invoke-rc.d nginx rotate >/dev/null 2>&1
{% elif ansible_os_family == "RedHat" %}
	{% if ansible_distribution_major_version == "6" %}
		service nginx start
		service nginx reload
	{% else %}
		systemctl start nginx
		systemctl reload nginx
	{% endif %}
{% elif ansible_os_family == "Alpine" %}
		service nginx start
        service nginx reload
{% else %}
        nginx -s reopen
{% endif %}
    endscript
}

[magicalyak]

The changes above should resolve the issues and pass the tests. The nginx service isn't running yet, so no pid is there...and thus the error.

[clwluvw]

Works fine

[alessfg]

I added a task to use flush_handlers instead of booting the NGINX service via the logrotate config. I'll merge the PR if tests pass 😄

[clwluvw]

@alessfg Wasn't that better to add run logrotate as an handler so in flushing the handlers first nginx starts and then logrotate would be run? 😄

[alessfg]

I don't know that it would necessarily be better but yep that would've worked too 😄

[clwluvw]

I don't know that it would necessarily be better but yep that would've worked too

Oh okay. I thought the flush_handlers couldn't be a best practice 😅