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

Function in mine.update failed to execute #56496

Closed
OrangeDog opened this issue Apr 2, 2020 · 10 comments
Closed

Function in mine.update failed to execute #56496

OrangeDog opened this issue Apr 2, 2020 · 10 comments
Assignees
Labels
Bug broken, incorrect, or confusing behavior Grains Regression The issue is a bug that breaks functionality known to work in previous releases. ZRelease-Sodium retired label
Milestone

Comments

@OrangeDog
Copy link
Contributor

OrangeDog commented Apr 2, 2020

Description of Issue

After upgrade 2019.2.3 -> 3000.1, minions now log this on startup.

Function cmd.run in mine.update failed to execute
Function grains.get in mine.update failed to execute

Setup

Pillar:

mine_functions:
  network.ip_addrs: []
  public_ssh_host_keys:
    mine_function: cmd.run
    cmd: cat /etc/ssh/ssh_host_*_key.pub
    python_shell: true
  public_ssh_host_names:
    mine_function: grains.get
    key: fqdn

Steps to Reproduce Issue

[DEBUG] Subprocess SignalHandlingProcess-1:3-Schedule-highstate added
[DEBUG] schedule.handle_func: adding this job to the jobcache with data {'id': 'my-minion', 'fun': 'mine.update', 'fun_args': [], 'schedule': '__mine_interval', 'jid': '20200402095405998171', 'pid': 27261}
[DEBUG] LazyLoaded mine.update
[DEBUG] LazyLoaded config.merge
[DEBUG] LazyLoaded network.ip_addrs
[DEBUG] schedule.handle_func: adding this job to the jobcache with data {'id': 'my-minion', 'fun': 'state.apply', 'fun_args': [], 'schedule': 'highstate', 'jid': '20200402095406011098', 'pid': 27262}
[DEBUG] LazyLoaded state.apply
[DEBUG] LazyLoaded grains.get
[DEBUG] LazyLoaded saltutil.is_running
[DEBUG] LazyLoaded cmd.run
[ERROR] Function cmd.run in mine.update failed to execute
[DEBUG] Error: Traceback (most recent call last):
File "/usr/lib/python3/dist-packages/salt/modules/mine.py", line 190, in update
**function_kwargs
File "/usr/lib/python3/dist-packages/salt/utils/functools.py", line 140, in call_function
'got only {1}'.format(_exp_prm, _passed_prm))
salt.exceptions.SaltInvocationError: Function expects 1 positional parameters, got only 0

[DEBUG] LazyLoaded grains.get
[ERROR] Function grains.get in mine.update failed to execute
[DEBUG] Error: Traceback (most recent call last):
File "/usr/lib/python3/dist-packages/salt/modules/mine.py", line 190, in update
**function_kwargs
File "/usr/lib/python3/dist-packages/salt/utils/functools.py", line 140, in call_function
'got only {1}'.format(_exp_prm, _passed_prm))
salt.exceptions.SaltInvocationError: Function expects 1 positional parameters, got only 0

[DEBUG] LazyLoaded config.get
[DEBUG] key: test, ret: _|-
[DEBUG] MinionEvent PUB socket URI: /var/run/salt/minion/minion_event_13ab08ec76_pub.ipc
[DEBUG] MinionEvent PULL socket URI: /var/run/salt/minion/minion_event_13ab08ec76_pull.ipc
[DEBUG] Sending event: tag = _minion_mine; data = {'cmd': '_mine', 'data': {'network.ip_addrs': ['x.x.x.x']}, 'id': 'my-minion', 'clear': False, '_stamp': '2020-04-02T09:54:06.029994'}
[DEBUG] Minion of 'salt' is handling event tag '_minion_mine'

Versions Report

Salt Version:
           Salt: 3000.1

Dependency Versions:
           cffi: Not Installed
       cherrypy: Not Installed
       dateutil: 2.6.1
      docker-py: Not Installed
          gitdb: 2.0.3
      gitpython: 2.1.8
         Jinja2: 2.10
        libgit2: 0.26.0
       M2Crypto: 0.32.0
           Mako: Not Installed
   msgpack-pure: Not Installed
 msgpack-python: 0.5.6
   mysql-python: Not Installed
      pycparser: Not Installed
       pycrypto: 2.6.1
   pycryptodome: Not Installed
         pygit2: 0.26.2
         Python: 3.6.9 (default, Nov  7 2019, 10:44:02)
   python-gnupg: 0.4.1
         PyYAML: 3.12
          PyZMQ: 16.0.2
          smmap: 2.0.3
        timelib: Not Installed
        Tornado: 4.5.3
            ZMQ: 4.2.5

System Versions:
           dist: Ubuntu 18.04 bionic
         locale: ISO-8859-1
        machine: x86_64
        release: 4.15.0-91-generic
         system: Linux
        version: Ubuntu 18.04 bionic
@OrangeDog
Copy link
Contributor Author

They continue to fail on every mine update.

@Akm0d Akm0d added Bug broken, incorrect, or confusing behavior Grains Regression The issue is a bug that breaks functionality known to work in previous releases. labels Apr 3, 2020
@Akm0d Akm0d added this to the Approved milestone Apr 3, 2020
@Ch3LL Ch3LL self-assigned this Apr 6, 2020
@sagetherage sagetherage added the ZRelease-Sodium retired label label Apr 7, 2020
@pgporada
Copy link
Contributor

pgporada commented Apr 9, 2020

I'm hitting this as well over at #56593.

@pgporada
Copy link
Contributor

pgporada commented Apr 9, 2020

Hey @OrangeDog,

Here's a "fix", but it is super gross. What happens here is that cmd.run will take - cat /etc/ssh/ssh_host_*_key.pub as a positional argument instead of a keyword argument. Note that - python_shell: true is a keyword argument (kwarg) still.

mine_functions:
  network.ip_addrs: []
  public_ssh_host_keys:
    - mine_function: cmd.run
    - cat /etc/ssh/ssh_host_*_key.pub
    - python_shell: true
  public_ssh_host_names:
    mine_function: grains.get
    key: fqdn

Another "fix" is to patch /usr/lib/python2.7/site-packages/salt/utils/functools.py

    if missing:
        raise SaltInvocationError('Missing arguments: {0}'.format(', '.join(missing)))
#    elif _exp_prm > _passed_prm:
#        raise SaltInvocationError('Function expects {0} positional parameters, '
#                                  'got only {1}'.format(_exp_prm, _passed_prm))

When either of those happen you'll be able to successfully run mine.update's again on v3000.1.

@yuriks
Copy link

yuriks commented Apr 9, 2020

This seems to also have been introduced in da29e15. It's now using salt.utils.functools.call_function which has broken/unexpected behavior when used to specify positional arguments using kwargs (e.g., setting cmd in cmd.shell instead of passing it as a positional arg).

call_function appropriately handles that, but incorrect error checking at the end of the function causes it to throw an exception even if no arguments are missing:

elif _exp_prm > _passed_prm:

The previous loop checks for positional arguments that don't have any value specified, populating the missing list. It correctly finds that there are no missing arguments, but then raises and exception anyway, because the number didn't match. Simply removing that elif branch seems to make everything work fine.

edit: what @pgporada said, I'll leave this post here because it has a few more details on the code

@vriabyk
Copy link

vriabyk commented Apr 29, 2020

hey, just wanted to say that I am experiencing pretty the same problem. I defined mine_functions in pillar (worked before upgrading to 3000.1):

mine_functions:
  prometheus_targets:
    mine_function: cmd.run
    env:
      - ENVIRONMENT: staging
    cmd: '/bin/get_prometheus_targets.sh'

and it caused the same errors mentioned by @OrangeDog .

Salt version:

salt --versions-report
Salt Version:
           Salt: 3000.1
 
Dependency Versions:
           cffi: 1.14.0
       cherrypy: 3.2.3
       dateutil: 2.4.2
      docker-py: Not Installed
          gitdb: 0.6.4
      gitpython: 1.0.1
         Jinja2: 2.8
        libgit2: Not Installed
       M2Crypto: Not Installed
           Mako: Not Installed
   msgpack-pure: Not Installed
 msgpack-python: 0.6.2
   mysql-python: Not Installed
      pycparser: 2.20
       pycrypto: 2.6.1
   pycryptodome: Not Installed
         pygit2: Not Installed
         Python: 3.5.2 (default, Apr 16 2020, 17:47:17)
   python-gnupg: 0.3.8
         PyYAML: 3.11
          PyZMQ: 15.2.0
          smmap: 0.9.0
        timelib: Not Installed
        Tornado: 4.5.3
            ZMQ: 4.1.4
 
System Versions:
           dist: Ubuntu 16.04 xenial
         locale: UTF-8
        machine: x86_64
        release: 4.4.0-177-generic
         system: Linux
        version: Ubuntu 16.04 xenial

Then I changed pillar as was suggested by @pgporada :

mine_functions:
  prometheus_targets:
    - mine_function: cmd.run
    - env:
      - ENVIRONMENT: staging
    - bash /bin/get_prometheus_targets.sh

and it started working.

@OrangeDog
Copy link
Contributor Author

OrangeDog commented Apr 30, 2020

For some reason I had to refresh the pillar twice to get it to actually update, but this works for the grain.

public_ssh_host_names:
  - mine_function: grains.get
  - fqdn

@cmcmarrow
Copy link
Contributor

@cmcmarrow
Copy link
Contributor

cmcmarrow commented May 12, 2020

@OrangeDog https://github.com/saltstack/salt/pull/56635/files seem to have fix your error. I ran your pillar data on develop and got no error.

@sagetherage
Copy link
Contributor

this will be released in Sodium

@vg22
Copy link

vg22 commented Jun 23, 2020

Is it fixed in sodium? I am still getting the same error after upgrading.
My pillar with mine function is:

{% set os = grains['os'] %}

{% if os  == 'Windows' %}
{% set interface_name = (grains['ip_interfaces'] | difference(['lo']))[1] %}
{% set ip_address = grains['ip_interfaces'][interface_name][0] %}
{% else %}
{% set interface_name = (grains['ip_interfaces'] | difference(['lo']))[0] %}
{% set ip_address = grains['ip_interfaces'][interface_name][0] %}
{% endif %}

# Define functions for use by the Salt Mine
mine_functions:
    ip_address:
      - mine_function: cmd.run
      - 'echo {{ ip_address }}'

If I run config.get mine_fucntions, I can see the function is there:

aws-vm1:
    ----------
    ip_address:
        |_
          ----------
          mine_function:
              cmd.run
        - echo 10.93.143.251

But if run mine.get, it is empty:

salt aws-vm1  mine.get  "aws-vm1" "ip_address"
aws-vm1:
   ----------

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 Grains Regression The issue is a bug that breaks functionality known to work in previous releases. ZRelease-Sodium retired label
Projects
None yet
Development

No branches or pull requests

9 participants