logrotate.set fails on CentOS 7's /etc/logrotate.d/syslog

[mephi42]

Here is the content of my /etc/logrotate.d/syslog (it was this way out of the box):

/var/log/cron
/var/log/maillog
/var/log/messages
/var/log/secure
/var/log/spooler
{
    missingok
    sharedscripts
    postrotate
    /bin/kill -HUP `cat /var/run/syslogd.pid 2> /dev/null` 2> /dev/null || true
    endscript
}

Here is my SLS file:

logrotate-messages-rotate:
  logrotate.set:
    - key: /var/log/messages
    - value: maxsize
    - setting: 100M

First, a minor issue: if I ask Salt to edit /etc/logrotate.d/syslog directly using conf_file directive, it will fail with KeyError: include files. The following patch helped:

--- a/salt/modules/logrotate.py
+++ b/salt/modules/logrotate.py
@@ -209,8 +209,9 @@ def set_(key, value, setting=None, conf_file=_DEFAULT_CONF):
     and make changes in the appropriate file.
     '''
     conf = _parse_conf(conf_file)
-    for include in conf['include files']:
-        if key in conf['include files'][include]:
+    includes = conf.get('include files', {})
+    for include in includes:
+        if key in includes[include]:
             conf_file = os.path.join(conf['include'], include)

     new_line = six.text_type()

Now, the main issue: Error: A setting for a dict was declared, but the configuration line given is not a dict. I dumped the parsed conf, and apparently the parser did not process multiple file syntax correctly:

{
   '/var/log/secure':True,
   '/var/log/messages':True,
   '/var/log/cron':True,
   '/var/log/maillog':True,
   '/var/log/spooler':{
      'postrotate':True,
      'sharedscripts':True,
      'missingok':True,
      '/bin/kill':'-HUP `cat /var/run/syslogd.pid 2> /dev/null` 2> /dev/null || true',
      'endscript':True
   },
}

(Edit) Ugly workaround for now:

/etc/logrotate.d/syslog:
  file.line:
    - content: maxsize 100M
    - mode: ensure
    - after: "{"

Versions Report

v2017.7.3

[boltronics]

I too am hitting similar issues with logrotate.set. The easiest way to hit it is to leave value empty (since many options do not take an argument). My work-around is to set setting to a string containing a space.

I then hit an issue where it's showing changes every time it executes, even when no changes are required. I worked around that by using an - unless: grep -q <regex> <file> argument.

So my state ends up looking like this:

{%- for file,  settings in salt['pillar.get']('logrotate', {}).items() %}
{%- for setting, value in settings['settings'].items() %}
Set {{ setting }} in /etc/logrotate.d/{{ settings.name }}:
  logrotate.set:
    - key: {{ file }}
    - value: {{ setting }}
{%- if value %}
    - setting: {{ value }}
{%- else %}
    # Here we avoid a "SaltInvocationError: Error: /path/to/logs/*
    # includes a dict, and a specific setting inside the dict was not
    # declared" error which seems to be Salt issue #48125.
    - setting: ' '
{%- endif %}
    # logrotate.set always seems to report changes, so this is a
    # hacky work-around.
    - unless: >-
        grep -qE '^\s*{{ setting }}\s*{{ value }}$'
        /etc/logrotate.d/{{ settings.name }}
{%- endfor %}
{%- endfor %}

The module seems very buggy in general.

[MadsRC]

I just ran into this issue on a Debian 9 running 2018.3.2 (Oxygen). Solution for me was to remove the "conf_file" directive.

Since it's been some time since my last PR, I'll see if I can find time to look at this in the coming week.

[nergdron]

just ran into this on ubuntu 18.04, with salt 2019.2.0. since this is a bug that breaks documented behaviour (ie: using conf_file doesn't work right now), it'd be great to get this resolved.

[doesitblend]

ZD-6406

[nergdron]

It's been 2 years since this was reported, any updates from the team on when this might get addressed?

[Behner]

I'd say this state is broken. It could really use some attention.

[sfishback]

Using salt-3003-1. Made a tiny mod to the 'conf_file' line and it's spitting out a python3 "KeyError: 'include files'" error. Changed from the default /etc/logrotate.conf to /etc/logrotate.d/blah. Sigh have to rewrite the state now.