nginxinc / ansible-role-nginx

Ansible role for installing NGINX
https://galaxy.ansible.com/nginxinc/nginx
Apache License 2.0
644 stars 348 forks source link

Bug with lograte formating #672

Closed tibz7 closed 1 year ago

tibz7 commented 1 year ago

Describe the bug

logrotate "daily" is badly formatted

To reproduce

  1. Just run the role normally I guess its a default behavour
  2. cat /etc/logrotate.d/nginx
    
    {
    d
    a
    i
    l
    y
    postrotate
        if [ -f /var/run/nginx.pid ]; then
            kill -USR1 `cat /var/run/nginx.pid`
        fi
    endscript
    }


A clear and concise description of what you expected to happen.

### Your environment

- 0.8.0
- Version of 2.15

- Target deployment platform
debian 12
alessfg commented 1 year ago

Hey @tibz7 -- I cannot reproduce this error.

When I use the following playbook:

---
- name: Converge
  hosts: all
  tasks:
    - name: Install NGINX
      ansible.builtin.include_role:
        name: nginxinc.nginx
      vars:
        nginx_logrotate_conf_enable: true
        nginx_logrotate_conf:
          paths:
            - /var/log/nginx/*.log
          options:
            - daily

This is what I get:

[root@debian-bookworm /]# cat /etc/logrotate.d/nginx
/var/log/nginx/*.log
{
    daily
    postrotate
        nginx -s reopen
    endscript
}

But I can reproduce the error when I run this playbook:

---
- name: Converge
  hosts: all
  tasks:
    - name: Install NGINX
      ansible.builtin.include_role:
        name: nginxinc.nginx
      vars:
        nginx_logrotate_conf_enable: true
        nginx_logrotate_conf:
          paths:
            - /var/log/nginx/*.log
          options: daily

options expects a list/array, not a single word. That being said, I'll create a PR to fix this edge case, but for next time, try to follow the pattern showcased in the defaults/ directory as closely as possible to avoid issues like this 😄