saltstack-formulas / logrotate-formula

http://docs.saltstack.com/en/latest/topics/development/conventions/formulas.html
Other
23 stars 71 forks source link

Bug: when job name is a path, no path in logrotate file #33

Closed daks closed 5 years ago

daks commented 6 years ago

Hello,

while writing tests for jobs (see #32) I found that a job configured like this

logrotate:
  jobs:
    /tmp/var/log/mysql/error:
      config:
        - weekly
        - missingok
        - rotate 52
        - compress
        - delaycompress
        - notifempty
        - create 640 root adm
        - sharedscripts

generates the following logrotate file

# vim: sw=2 sts=2 ts=2 sw et
#
# This file is managed by salt. Do not edit by hand.
#

{
  weekly
  missingok
  rotate 52
  compress
  delaycompress
  notifempty
  create 640 root adm
  sharedscripts
}

This file is missing the path /tmp/var/log/mysql/error so the generated file is useless, even if logrotate do work.

(test is commented out at the moment while this bug exists https://github.com/saltstack-formulas/logrotate-formula/pull/32/files#diff-c5d9a182ed31382e14c9db96116cfd04R10

danny-smit commented 6 years ago

By coincidence I ran into the same issue this week. I noticed commit fdfb263e094f3b76b6c34993d240f02a0a9e5b50 changed the bahavior with respect to this issue bit.

I think this patch should fix that without side effects:

--- a/logrotate/jobs.sls
+++ b/logrotate/jobs.sls
@@ -18,10 +18,10 @@ logrotate-{{ key }}:
       - pkg: logrotate-pkg
     - context:
       {% if value is mapping %}
-      path: {{ value.get('path', []) }}
+      path: {{ value.get('path', [key]) }}
       data: {{ value.get('config', []) }}
       {% else %}
-      path: {{ key }}
+      path: [ {{ key }} ]
       data: {{ value }}
       {% endif %}
 {%- endfor -%}