Salt: Beacon inotify problem

[Zambozo]

Description of Issue/Question

I am trying to set up beacon as in the example doc for

 inotify:
    - files:
        /etc/important_file:
          mask:
            - modify

and i keep getting this error on the salt-minion:

2018-07-21 13:25:23,094 [salt.minion      ][CRITICAL][24187] The beacon errored:
Traceback (most recent call last):
  File "/usr/lib/python2.7/site-packages/salt/minion.py", line 2230, in handle_beacons
    beacons = self.process_beacons(self.functions)
  File "/usr/lib/python2.7/site-packages/salt/minion.py", line 409, in process_beacons
    return self.beacons.process(b_conf, self.opts['grains'])  # pylint: disable=no-member
  File "/usr/lib/python2.7/site-packages/salt/beacons/__init__.py", line 99, in process
    raw = self.beacons[fun_str](b_config[mod])
  File "/usr/lib/python2.7/site-packages/salt/beacons/inotify.py", line 261, in beacon
    if isinstance(config[path], dict):
TypeError: list indices must be integers, not dict

Setup

(Please provide relevant configs and/or SLS files (Be sure to remove sensitive info).)

[root]$ cat beacons.conf
beacons:
  load:
    - averages:
        1m:
          - 0.0
          - 2.0
        5m:
          - 0.1
          - 1.5
        15m:
          - 0.1
          - 1.0
    - emitatstartup: True
    - onchangeonly: False
    - interval: 60
  inotify:
    - files:
        /etc/important_file:
          mask:
            - modify

Steps to Reproduce Issue

(Include debug logs if possible and relevant.)

Versions Report

(Provided by running salt --versions-report. Please also mention any differences in master/minion versions.)

Salt Version:
           Salt: 2018.3.2

Dependency Versions:
           cffi: 1.9.1
       cherrypy: 3.2.2
       dateutil: 2.7.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.21.1
           Mako: Not Installed
   msgpack-pure: Not Installed
 msgpack-python: 0.4.8
   mysql-python: Not Installed
      pycparser: 2.17
       pycrypto: 2.6.1
   pycryptodome: Not Installed
         pygit2: Not Installed
         Python: 2.7.5 (default, Nov  6 2016, 00:28:07)
   python-gnupg: Not Installed
         PyYAML: 3.12
          PyZMQ: 15.3.0
           RAET: Not Installed
          smmap: Not Installed
        timelib: Not Installed
        Tornado: 4.2.1
            ZMQ: 4.1.4

System Versions:
           dist: centos 7.2.1511 Core
         locale: UTF-8
        machine: x86_64
        release: 3.10.0-327.36.3.el7.x86_64
         system: Linux
        version: CentOS Linux 7.2.1511 Core

[Ch3LL]

i'm not able to replicate this. do you have the pyinotify dependency installed?

[Zambozo]

Hello,

Could you please point me to check the dependencies ?

[root]$ pip install pyinotify
Requirement already satisfied: pyinotify in /usr/lib/python2.7/site-packages (0.9.4)

[Ch3LL]

yeah seems its installed there.

Can you confirm that your minion has version: 2018.3.2 as well? just want to make sure that version report is for the minion and not the master. When i try to look at your stack trace in the code its not on the correct line number, which makes me think your minion is on a different version.

[steveno]

I ran into this issue today. I can confirm that my master was 2018.3.0 but the minion was 2017.7.3. Upgrading the minion to 2018.3.0 resolved the issue and the error message no longer appears in the logs.

[Ch3LL]

thanks for confirming @steveno really appreciate that.

@Zambozo can you confirm your minion version is up to date as well?

[arizvisa]

I have the exact same problem. I've been spotting the code with a bunch of print-fuck statements trying to narrow this down.

Here's the test-case I used. This should result in a list of dictionaries (these get merged), where "files" is a list of dictionaries.

$ salt-call --local beacons.add inotify '[{ "files": [{ "/path/to/something": {"mask":["create","delete","modify"]} }] }]'

It looks like the issue is that the following code (validate) in beacons/inotify.py first checks that the config is a list, and then combines that list into a single dictionary _config. Afterwards, it walks through the list _config.get('files'). For each instance of this loop, path is an item in a list. This is then used to index into _config.get('files'). If config['files'] is supposed to be a list, then that for-loop should be yielding the index into the list, and not the item from the list.

(sorry about this paste without line numbers, i have this code running in a container and as soon as i stop the container the file will be gone).

    # Configuration for inotify beacon should be a dict of dicts
    if not isinstance(config, list):
        return False, 'Configuration for inotify beacon must be a list.'
    else:
       _config = {}
        list(map(_config.update, config))                            # consolidate list of dictionaries into one

        if 'files' not in _config:
            return False, 'Configuration for inotify beacon must include files.'
        else:
            for path in _config.get('files'):                        # iterate through keys or items (docs says items)

                if not isinstance(_config['files'][path], dict):     # XXX: use "path" as a key
                    return False, ('Configuration for inotify beacon must '
                                   'be a list of dictionaries.')
...

So using a for-loop like this will yield either an item or a key for path. If we pass a dictionary (which wil cause path to be a key instead of a list-item), then you get this obvious validation error:

$ salt-call --local beacons.add inotify '[{ "files": {"/srv/container/image": {"mask":["create","delete","modify"]} } }]'

local:
    ----------
    comment:
        Beacon inotify configuration invalid, not adding.
        Configuration for inotify beacon must be a list of dictionaries.
    result:
        False

Because of this exception the result event doesn't get fired. (It might help stability to wrap the different function callbacks (manage_*) in minion.py in a try-except that logs the exception and guarantees the return of a value to get_event or whichev.)

Regardless though, here's my version:

$ salt-call --versions
Salt Version:
           Salt: 2019.2.0-n/a-b348034

Dependency Versions:
           cffi: 1.11.5
       cherrypy: Not Installed
       dateutil: 2.7.5
      docker-py: Not Installed
          gitdb: 2.0.3
      gitpython: 2.1.11
          ioflo: 1.7.5
         Jinja2: 2.10
        libgit2: 0.27.4
        libnacl: 1.6.1
       M2Crypto: Not Installed
           Mako: 1.0.7
   msgpack-pure: Not Installed
 msgpack-python: 0.5.6
   mysql-python: Not Installed
      pycparser: 2.14
       pycrypto: 2.6.1
   pycryptodome: 3.7.0
         pygit2: 0.27.2
         Python: 2.7.15 (default, Oct 15 2018, 15:26:09)
   python-gnupg: Not Installed
         PyYAML: 4.2
          PyZMQ: 17.0.0
           RAET: 0.6.8
          smmap: 2.0.3
        timelib: Not Installed
        Tornado: 5.0.2
            ZMQ: 4.1.6

System Versions:
           dist: fedora 29 Twenty Nine
         locale: ANSI_X3.4-1968
        machine: x86_64
        release: 4.14.88-coreos
         system: Linux
        version: Fedora 29 Twenty Nine

[arizvisa]

Okay, killed the container and here's the backtrace with the correct line numbers:

2019-01-25 22:06:17,981 [tornado.application:792 ][ERROR   ][14] Exception in callback <functools.partial object at 0x7f8cd072b940>
Traceback (most recent call last):
  File "/usr/lib64/python2.7/site-packages/tornado/ioloop.py", line 759, in _run_callback
    ret = callback()
  File "/usr/lib64/python2.7/site-packages/tornado/stack_context.py", line 276, in null_wrapper
    return fn(*args, **kwargs)
  File "/usr/lib64/python2.7/site-packages/tornado/ioloop.py", line 780, in _discard_future_result
    future.result()
  File "/usr/lib64/python2.7/site-packages/tornado/concurrent.py", line 260, in result
    raise_exc_info(self._exc_info)
  File "/usr/lib64/python2.7/site-packages/tornado/gen.py", line 1107, in run
    yielded = self.gen.throw(*exc_info)
  File "/usr/lib/python2.7/site-packages/salt/minion.py", line 963, in handle_event
    yield [minion.handle_event(package) for minion in self.minions]
  File "/usr/lib64/python2.7/site-packages/tornado/gen.py", line 1099, in run
    value = future.result()
  File "/usr/lib64/python2.7/site-packages/tornado/concurrent.py", line 260, in result
    raise_exc_info(self._exc_info)
  File "/usr/lib64/python2.7/site-packages/tornado/gen.py", line 849, in callback
    result_list.append(f.result())
  File "/usr/lib64/python2.7/site-packages/tornado/concurrent.py", line 260, in result
    raise_exc_info(self._exc_info)
  File "/usr/lib64/python2.7/site-packages/tornado/gen.py", line 315, in wrapper
    yielded = next(result)
  File "/usr/lib/python2.7/site-packages/salt/minion.py", line 2377, in handle_event
    self.manage_beacons(tag, data)
  File "/usr/lib/python2.7/site-packages/salt/minion.py", line 2282, in manage_beacons
    self.beacons.validate_beacon(name, beacon_data)
  File "/usr/lib/python2.7/site-packages/salt/beacons/__init__.py", line 266, in validate_beacon
    valid, vcomment = self.beacons[validate_str](beacon_data)
  File "/usr/lib/python2.7/site-packages/salt/beacons/inotify.py", line 122, in validate
    if not isinstance(_config['files'][path], dict):
TypeError: list indices must be integers, not dict

So the next question is, why does the error message not appear in the logs during initialization of the minion. With trace logs enabled, one should see "Beacon processing" (due to Beacon.process in beacons/__init__.py) but for some reason in my 2019.2 instance this isn't the case. I did some digging and the only reference to something.process is in minion.py via a method MinionBase.process_beacons. This method is only called by "Minion.setup_beacons" which looks like:

    def setup_beacons(self, before_connect=False):
        '''
        Set up the beacons.
        This is safe to call multiple times.
        '''
        self._setup_core()

        loop_interval = self.opts['loop_interval']
        new_periodic_callbacks = {}

        if 'beacons' not in self.periodic_callbacks:
            self.beacons = salt.beacons.Beacon(self.opts, self.functions)

            def handle_beacons():
                # Process Beacons
                beacons = None
                try:
                    beacons = self.process_beacons(self.functions)
                except Exception:
                    log.critical('The beacon errored: ', exc_info=True)
                if beacons and self.connected:
                    self._fire_master(events=beacons)

            new_periodic_callbacks['beacons'] = tornado.ioloop.PeriodicCallback(
                    handle_beacons, loop_interval * 1000)
            if before_connect:
                # Make sure there is a chance for one iteration to occur before connect
                handle_beacons()

So the validation function is only called when before_connect is true, and if you grep for that it then looks like calling this function with that parameter set to true is barricaded behind an undocumented option:

            if self.opts.get('beacons_before_connect', False):
                self.setup_beacons(before_connect=True)

Anyways, this is why you don't see any error messages about validation of the inotify beacon (or any beacon actually). To test, try configuring with a bunk beacon (but correct yaml of course) or a beacon with a dictionary rather than a list.

[Ch3LL]

thanks for all that additional information

ping @garethgreenaway can you take a look here?

[fres0]

Description of Issue/Question

reverting a file on modification or deletion is not working.

My way is straight from the example:
(https://docs.saltstack.com/en/latest/topics/beacons/)

inotify:
    - files:
        /etc/important_file:
          mask:
            - modify
            - delete
    - disable_during_state_run: True

on salt-master this error is seen in "salt-master -l debug"

Traceback (most recent call last):
  File "/usr/lib/python3.6/site-packages/salt/utils/templates.py", line 392, in render_jinja_tmpl
    output = template.render(**decoded_context)
  File "/usr/lib/python3.6/site-packages/jinja2/asyncsupport.py", line 76, in render
    return original_render(self, *args, **kwargs)
  File "/usr/lib/python3.6/site-packages/jinja2/environment.py", line 1008, in render
    return self.environment.handle_exception(exc_info, True)
  File "/usr/lib/python3.6/site-packages/jinja2/environment.py", line 780, in handle_exception
    reraise(exc_type, exc_value, tb)
  File "/usr/lib/python3.6/site-packages/jinja2/_compat.py", line 37, in reraise
    raise value.with_traceback(tb)
  File "<template>", line 3, in top-level template code
  File "/usr/lib/python3.6/site-packages/jinja2/environment.py", line 411, in getitem
    return obj[argument]
jinja2.exceptions.UndefinedError: 'dict object' has no attribute 'data'

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/usr/lib/python3.6/site-packages/salt/utils/templates.py", line 169, in render_tmpl
    output = render_str(tmplstr, context, tmplpath)
  File "/usr/lib/python3.6/site-packages/salt/utils/templates.py", line 402, in render_jinja_tmpl
    buf=tmplstr)
salt.exceptions.SaltRenderError: Jinja variable 'dict object' has no attribute 'data'
[ERROR   ] Failed to render "/data/saltstack/salt/var/cache/salt/master/files/base/reactor/revert.sls":
Traceback (most recent call last):
  File "/usr/lib/python3.6/site-packages/salt/utils/templates.py", line 392, in render_jinja_tmpl
    output = template.render(**decoded_context)
  File "/usr/lib/python3.6/site-packages/jinja2/asyncsupport.py", line 76, in render
    return original_render(self, *args, **kwargs)
  File "/usr/lib/python3.6/site-packages/jinja2/environment.py", line 1008, in render
    return self.environment.handle_exception(exc_info, True)
  File "/usr/lib/python3.6/site-packages/jinja2/environment.py", line 780, in handle_exception
    reraise(exc_type, exc_value, tb)
  File "/usr/lib/python3.6/site-packages/jinja2/_compat.py", line 37, in reraise
    raise value.with_traceback(tb)
  File "<template>", line 3, in top-level template code
  File "/usr/lib/python3.6/site-packages/jinja2/environment.py", line 411, in getitem
    return obj[argument]
jinja2.exceptions.UndefinedError: 'dict object' has no attribute 'data'

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/usr/lib/python3.6/site-packages/salt/utils/reactor.py", line 97, in render_reaction
    data=data)
  File "/usr/lib/python3.6/site-packages/salt/state.py", line 386, in render_template
    **kwargs)
  File "/usr/lib/python3.6/site-packages/salt/template.py", line 101, in compile_template
    ret = render(input_data, saltenv, sls, **render_kwargs)
  File "/usr/lib/python3.6/site-packages/salt/renderers/jinja.py", line 70, in render
    **kws)
  File "/usr/lib/python3.6/site-packages/salt/utils/templates.py", line 169, in render_tmpl
    output = render_str(tmplstr, context, tmplpath)
  File "/usr/lib/python3.6/site-packages/salt/utils/templates.py", line 402, in render_jinja_tmpl
    buf=tmplstr)
salt.exceptions.SaltRenderError: Jinja variable 'dict object' has no attribute 'data'

Setup

(Please provide relevant configs and/or SLS files (Be sure to remove sensitive info).)

# cat beacons.conf 
beacons:
  inotify:
    - files:
       /etc/important_file:
          mask:
            - modify
            - delete
    - disable_during_state_run: True

Versions Report
(Provided by running salt --versions-report. Please also mention any differences in master/minion versions.)

minion

# salt-call --versions-report
Salt Version:
           Salt: 2019.2.0

Dependency Versions:
           cffi: 1.11.2
       cherrypy: 10.2.1
       dateutil: 2.6.1
      docker-py: Not Installed
          gitdb: Not Installed
      gitpython: Not Installed
          ioflo: Not Installed
         Jinja2: 2.10.1
        libgit2: 0.26.3
        libnacl: 1.6.1
       M2Crypto: 0.28.2
           Mako: 1.0.7
   msgpack-pure: Not Installed
 msgpack-python: 0.5.4
   mysql-python: Not Installed
      pycparser: 2.17
       pycrypto: 2.6.1
   pycryptodome: 3.4.6
         pygit2: 0.26.0
         Python: 3.6.8 (default, Apr 30 2019, 13:27:23) [GCC]
   python-gnupg: 0.4.4
         PyYAML: 3.12
          PyZMQ: 17.0.0
           RAET: Not Installed
          smmap: 0.9.0
        timelib: Not Installed
        Tornado: 4.5.3
            ZMQ: 4.2.3

System Versions:
           dist:   
         locale: UTF-8
        machine: x86_64
        release: 4.12.14-150.22-default
         system: Linux
        version: Not Installed

master

Salt Version:
           Salt: 2019.2.0

Dependency Versions:
           cffi: 1.11.2
       cherrypy: unknown
       dateutil: 2.6.1
      docker-py: Not Installed
          gitdb: Not Installed
      gitpython: Not Installed
          ioflo: Not Installed
         Jinja2: 2.10.1
        libgit2: 0.26.3
        libnacl: 1.6.1
       M2Crypto: 0.28.2
           Mako: 1.0.7
   msgpack-pure: Not Installed
 msgpack-python: 0.5.4
   mysql-python: Not Installed
      pycparser: 2.17
       pycrypto: 2.6.1
   pycryptodome: 3.4.6
         pygit2: 0.26.0
         Python: 3.6.8 (default, Apr 30 2019, 13:27:23) [GCC]
   python-gnupg: 0.4.4
         PyYAML: 3.12
          PyZMQ: 17.0.0
           RAET: Not Installed
          smmap: 0.9.0
        timelib: Not Installed
        Tornado: 4.5.3
            ZMQ: 4.2.3

System Versions:
           dist:   
         locale: UTF-8
        machine: x86_64
        release: 4.12.14-150.22-default
         system: Linux
        version: Not Installed

master and minion

# pip install pyinotify
Requirement already satisfied: pyinotify in /usr/lib/python3.6/site-packages (0.9.6)

on minion:

modifying the "important_file" with:
echo "" >> /etc/important_file

results in this message (salt-run state.event pretty=True):

salt/beacon/minion01/inotify//etc/important_file        {
    "_stamp": "2019-07-18T07:40:39.495488",
    "change": "IN_MODIFY",
    "id": "minion01",
    "path": "/etc/important_file"
}

from the beacons example page the message in the event bus is supposed to look like this:

{
 "_stamp": "2015-09-09T15:59:37.972753",
 "data": {
     "change": "IN_IGNORED",
     "id": "larry",
     "path": "/etc/important_file"
 },
 "tag": "salt/beacon/larry/inotify//etc/important_file"
}

the missing part:

"data": {}

triggering an event from the minion with:

salt-call event.send 'salt/beacon/minion01/inotify//etc/important_file' '{'path': '/etc/important_file', 'change': 'IN_MODIFY', 'id': 'minion01'}'

and the data structure looks good again.

salt/beacon/minion01/inotify//etc/important_file        {                                                                                                                             
    "_stamp": "2019-07-18T07:42:45.353787",                                                                                                                                                   
    "cmd": "_minion_event",                                                                                                                                                                   
    "data": {                                                                                                                                                                                 
        "__pub_fun": "event.send",                                                                                                                                                            
        "__pub_jid": "20190718094245291424",                                                                                                                                                  
        "__pub_pid": 11626,                                                                                                                                                                   
        "__pub_tgt": "salt-call",                                                                                                                                                             
        "change": "IN_MODIFY",                                                                                                                                                                
        "id": "minion01",                                                                                                                                                                       
        "path": "/etc/important_file"                                                                                                                                               
    },                                                                                                                                                                                        
    "id": "minion01",                                                                                                                                                                           
    "tag": "salt/beacon/minion01/inotify//etc/important_file"                                                                                                                         
}   

[stale[bot]]

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.

[arizvisa]

This issue shouldn't be closed as It doesn't seem that this issue has been associated with a PR. This issue should result in 2 fixes iirc.

One of them would be to document the undocumented "beacons_before_connect" option. This option prevents the beacon from being validated and giving a warning at the only time the beacons are initialized.

The other fix is that the iteration through the beacons is assuming an invalid type which is a common problem resulting in a handful of the issues I've found during my time w/ salt (#55708 comes to mind). This type should be asserted through some means so that the loop is 100% sure it's iterating through a list and not a dict.

---

An example of the assertion I'm trying to describe could be something like salt.utils.assertion.type(variable, list) or salt.utils.assertion.type(variable, dict) which can check an explicit type and raise an exception during development (to help track down who's at fault for the incorrect type), and explicitly force the type during production w/ a warning log so that the community can identify these issues (and it doesn't interfere w/ a production environment). This has the benefit of fixing the symptom, but not hiding the root cause.

If the salt.utils.assertion namespace ends up being created, we can then start adding things like salt.utils.assertion.iterable(variable) for checking interface compatibility. But again, any solution will work. I just think exposing a tool like this to saltstack will help reduce these type issues in the future as long as we're consistent about it.

[stale[bot]]

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

[0rbiter]

Sometimes someone just has to actually post a workaround, even if it might seem obvious to others. This works:

beacons:
  inotify:
    - { "files": { "/etc/important_file": { "mask" : ["modify", "delete_self", "create"] } } }
    - disable_during_state_run: True