logrotate.get logs warnings "Block" '%s' not present or empty."

[boltronics]

Description of Issue

I have a logrotate state sls file that builds up a logrotate configuration from scratch. In doing this, it seems unavoidable that loud WARNING messages will be presented to the user, leading users to worry that something is wrong.

Setup

logrotate.sls state file:

Install logrotate:
  pkg.installed:
    - name: logrotate

{%- for file,  settings in salt['pillar.get']('logrotate', {}).items() %}
# If a /etc/logrotate.d/<file> with the appropriate section doesn't
# pre-exist, logrotate.set will populate the required entries into
# /etc/logrotate.conf instead.
Create /etc/logrotate.d/{{ settings.name }}:
  file.managed:
    - name: /etc/logrotate.d/{{ settings.name }}
    - user: root
    - group: root
    - mode: '0644'
    - contents: |
        {{ file }} {
        }
    - require:
      - pkg: logrotate

{%- 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 (which by default
    # makes using prereq against it pointless), so this is a hacky
    # work-around.
    - unless: >-
        grep -qE '^\s*{{ setting }}\s*{{ value }}$'
        /etc/logrotate.d/{{ settings.name }}
    - prereq_in:
      - file: Create /etc/logrotate.d/{{ settings.name }}
{%- endfor %}
{%- endfor %}

logrotate.sls pillar file:

logrotate:
  '/myapp/log/file/path/*':
    name: myapp
    settings:
      daily: ''
      rotate: 7
      missingok: ''
      notifempty: ''
      compress: ''
      nocreate: ''

Steps to Reproduce Issue

With the above setup, run:

$ sudo salt-call -l warning state.sls logrotate
[WARNING ] Block '/myapp/log/file/path/*' not present or empty.
[WARNING ] Block '/myapp/log/file/path/*' not present or empty.
[WARNING ] Block '/myapp/log/file/path/*' not present or empty.
[WARNING ] Block '/myapp/log/file/path/*' not present or empty.
[WARNING ] Block '/myapp/log/file/path/*' not present or empty.
[WARNING ] Block '/myapp/log/file/path/*' not present or empty.
...

Versions Report

salt-call --versions-report
Salt Version:
           Salt: 2017.7.8
 
Dependency Versions:
           cffi: Not Installed
       cherrypy: Not Installed
       dateutil: 2.5.3
      docker-py: Not Installed
          gitdb: Not Installed
      gitpython: Not Installed
          ioflo: Not Installed
         Jinja2: 2.8
        libgit2: Not Installed
        libnacl: Not Installed
       M2Crypto: 0.24.0
           Mako: Not Installed
   msgpack-pure: Not Installed
 msgpack-python: 0.4.8
   mysql-python: 1.3.7
      pycparser: Not Installed
       pycrypto: 2.6.1
   pycryptodome: Not Installed
         pygit2: Not Installed
         Python: 2.7.13 (default, Sep 26 2018, 18:42:22)
   python-gnupg: 0.3.9
         PyYAML: 3.12
          PyZMQ: 16.0.2
           RAET: Not Installed
          smmap: Not Installed
        timelib: Not Installed
        Tornado: 4.4.3
            ZMQ: 4.2.1
 
System Versions:
           dist: debian 9.9 
         locale: UTF-8
        machine: x86_64
        release: 4.19.0-5-amd64
         system: Linux
        version: debian 9.9 
 

[boltronics]

Here is where the issue is:
https://github.com/saltstack/salt/blob/v2019.2.0/salt/modules/logrotate.py#L169

It's printed whenever the file is empty, which it will be when building the file up from scratch. As such, a warning is too heavy handed since there is no problem and it's to be expected.

I believe a simple info log would suffice, which can easily be hidden or at least won't confuse the user into thinking something is broken. Please let me know if this would be acceptable.

[dwoz]

@boltronics Thanks for pointing this out, I agree and thing this could probably actually be a debug log.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

If this issue is closed prematurely, please leave a comment and we will gladly reopen the issue.

[stale]

Thank you for updating this issue. It is no longer marked as stale.