Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
mephi42 opened this issue Jun 14, 2018 · 7 comments
Open

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

mephi42 opened this issue Jun 14, 2018 · 7 comments
Labels
Bug broken, incorrect, or confusing behavior help-wanted Community help is needed to resolve this pending-community-assignment Pending community contributor assignment severity-medium 3rd level, incorrect or bad functionality, confusing and lacks a work around ZD The issue is related to a Zendesk customer support ticket.
Milestone

Comments

@mephi42
Copy link
Contributor

mephi42 commented Jun 14, 2018

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

@garethgreenaway garethgreenaway added this to the Approved milestone Jun 14, 2018
@garethgreenaway garethgreenaway added Bug broken, incorrect, or confusing behavior severity-medium 3rd level, incorrect or bad functionality, confusing and lacks a work around P2 Priority 2 labels Jun 14, 2018
@boltronics
Copy link
Contributor

boltronics commented Jul 26, 2018

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
Copy link
Contributor

MadsRC commented Feb 4, 2019

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
Copy link

nergdron commented Aug 7, 2019

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.

@sagetherage sagetherage removed the P2 Priority 2 label Jun 3, 2020
@doesitblend doesitblend added the ZD The issue is related to a Zendesk customer support ticket. label Feb 26, 2021
@doesitblend
Copy link
Collaborator

ZD-6406

@sagetherage sagetherage added the Phosphorus v3005.0 Release code name and version label May 18, 2021
@sagetherage sagetherage modified the milestones: Approved, Phosphorus May 18, 2021
@nergdron
Copy link

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

@Behner
Copy link

Behner commented Dec 17, 2021

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

@sfishback
Copy link

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.

@Ch3LL Ch3LL removed the Phosphorus v3005.0 Release code name and version label Mar 30, 2022
@anilsil anilsil removed this from the Chlorine v3007.0 milestone May 11, 2023
@twangboy twangboy added help-wanted Community help is needed to resolve this pending-community-assignment Pending community contributor assignment labels Sep 6, 2023
@twangboy twangboy added this to the Approved milestone Sep 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug broken, incorrect, or confusing behavior help-wanted Community help is needed to resolve this pending-community-assignment Pending community contributor assignment severity-medium 3rd level, incorrect or bad functionality, confusing and lacks a work around ZD The issue is related to a Zendesk customer support ticket.
Projects
None yet
Development

No branches or pull requests