Mine merge error when using Napalm Proxy - Python salt

Description of Issue/Question

I am getting an error when trying to create a napalm proxy Unable to merge mine functions from the pillar in the opts, for proxy

This seems to be caused by inconsistencies in the code merging the minefunctions key from config and pillar data. The mine.update documentation indicates that the minefunctions key should be a dictionary. But the code in salt.minion.ProxyMinion.postmaster_init() treats the key as a list.

            if 'mine_functions' in self.opts['pillar']:
                general_proxy_mines = self.opts.get('mine_functions', [])
                specific_proxy_mines = self.opts['pillar']['mine_functions']
                try:
                    self.opts['mine_functions'] = general_proxy_mines + specific_proxy_mines

So if you configure minefunctions in your pillar (or your config) as a list, then you hit a problem in mine.update where it fetches __salt__'config.merge' which will return a list, and assigned this to mdata which is assumed to be a dict further down the code.

    if not mine_functions:
        m_data = __salt__['config.merge']('mine_functions', {})
        # If we don't have any mine functions configured, then we should just bail out
        if not m_data:
            return
    elif mine_functions and isinstance(mine_functions, list):
        m_data = dict((fun, {}) for fun in mine_functions)
    elif mine_functions and isinstance(mine_functions, dict):
        m_data = mine_functions
    else:
        return

I worked around this by changing the code in mine.update by assigning the minefunctions key from config.merge to minefunctions and unchaining the enclosing if statement (if not minefunctions:) from the next two if statements that check the type of minefunctions.

But basically someone needs to decide if mine_functions should be a list or a dict or both and support whatever is decided as the right representation.

Setup

/etc/salt/proxy.d/env.conf
saltenv: proxy
pillarenv: proxy

/srv/proxy/pillar/napalm/init.sls
mine_functions:
  bgp.neighbors: []
  net.ipaddrs: []
  net.lldp: []
  net.mac: []
  net.arp: []
  net.interfaces: []
mine_interval: 60

Steps to Reproduce Issue

Create a napalm proxy using the setup above. When you start the proxy, you will get the error failing to merge the mine functions.

If you change mine_functions into a list

/srv/proxy/pillar/napalm/init.sls
mine_functions:
  - bgp.neighbors
  - net.ipaddrs
  - net.lldp
  - net.mac
  - net.arp
  - net.interfaces
mine_interval: 60

This will fix the merge error. But when you run mine.update on the proxy, you then get an error

[ERROR   ] Function bgp.neighbors in mine_functions failed to execute
[DEBUG   ] Error: Traceback (most recent call last):
  File "/usr/lib/python3/dist-packages/salt/modules/mine.py", line 151, in update
    if m_data[func] and isinstance(m_data[func], dict):
TypeError: list indices must be integers or slices, not str

Changing the code in mine.update to look like this:

    if not mine_functions:
        mine_data = __salt__['config.merge']('mine_functions', {})
        # If we don't have any mine functions configured, then we should just bail out
        if not mine_data:
            return
    if mine_functions and isinstance(mine_functions, list):
        m_data = dict((fun, {}) for fun in mine_functions)
    elif mine_functions and isinstance(mine_functions, dict):
        m_data = mine_functions
    else:
        return

Fixes the problem for me

Versions Report

Salt Version:
           Salt: 2018.3.3

Dependency Versions:
           cffi: 1.11.5
       cherrypy: unknown
       dateutil: 2.6.1
      docker-py: Not Installed
          gitdb: 2.0.3
      gitpython: 2.1.8
          ioflo: Not Installed
         Jinja2: 2.10
        libgit2: Not Installed
        libnacl: Not Installed
       M2Crypto: Not Installed
           Mako: Not Installed
   msgpack-pure: Not Installed
 msgpack-python: 0.5.6
   mysql-python: Not Installed
      pycparser: 2.18
       pycrypto: 2.6.1
   pycryptodome: Not Installed
         pygit2: Not Installed
         Python: 3.6.7 (default, Oct 22 2018, 11:32:17)
   python-gnupg: 0.4.1
         PyYAML: 3.12
          PyZMQ: 16.0.2
           RAET: Not Installed
          smmap: 2.0.3
        timelib: Not Installed
        Tornado: 4.5.3
            ZMQ: 4.2.5

System Versions:
           dist: Ubuntu 18.04 bionic
         locale: UTF-8
        machine: x86_64
        release: 4.15.0-39-generic
         system: Linux
        version: Ubuntu 18.04 bionic
Asked Oct 11 '21 16:10
avatar mngan
mngan

9 Answer:

ping @saltstack/team-core any opinions on the proposed changes?

1
Answered Dec 04 '18 at 14:28
avatar  of Ch3LL
Ch3LL

The proposed change just makes the code support a list of mine functions. But if you need to send any args to your mine functions, then a dict needs to be supported. So the proposed change doesn't address that case. So someone who has more insight into the direction that things are going here should determine what the right solution should be. What I proposed is to just highlight how I could get one case going, but it definitely isn't a proper fix.

1
Answered Dec 06 '18 at 01:13
avatar  of mngan
mngan

@mirceaulinic has worked a lot on the Napalm support and I'd like to get his input here.

1
Answered Dec 06 '18 at 04:26
avatar  of terminalmage
terminalmage

I've tripped over this issue in 2019.2.2. In my case self.opts['pillar']['mine_functions'] is returning a dict and self.opts.get('mine_functions', []) was returning the default empty list.

It's not clear to me if both lists and dicts should be supported, or if receiving one type is a bug.

If I make the assumption that dicts are the only valid type, the following change works:

 if 'mine_functions' in self.opts['pillar']:
            proxy_mines = OrderedDict(self.opts.get('mine_functions', {}))
            specific_proxy_mines = self.opts['pillar']['mine_functions']
            try:
                proxy_mines.update(specific_proxy_mines)
                self.opts['mine_functions'] = proxy_mines·
            except TypeError as terr:
                log.error('Unable to merge mine functions from the pillar in the opts, for proxy {}'.format(
                    self.opts['id']))
1
Answered Oct 24 '19 at 13:26
avatar  of danfoster
danfoster

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.

1
Answered Jan 07 '20 at 17:05
avatar  of stale[bot]
stale[bot]

Oh, missed this one. Yes, I think that's the issue as you pointed out @danfoster, mine_functions shouldn't default to list.

1
Answered Jan 09 '20 at 09:13
avatar  of mirceaulinic
mirceaulinic

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

1
Answered Jan 09 '20 at 09:13
avatar  of stale[bot]
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.

1
Answered Feb 08 '20 at 10:37
avatar  of stale[bot]
stale[bot]

@waynew @dmurphy18 can both or either of you do a little follow up on this Napalm issue, please?

1
Answered Feb 11 '20 at 20:33
avatar  of sagetherage
sagetherage