New module.run format causing traceback from state file

[sdodsley]

Description of Issue/Question

Trying to use the new module.run format and getting a traceback failure from the minion.

I have followed the instructions regarding the new format and including the required settings in the minion config file

Setup

Minion config file contains the following:

use_superseded:
  - module.run

top.sls

base:
  '*':
    - storage

storage.sls

storage:
  module.run:
    - test.echo:
      - text: "Hello"

Steps to Reproduce Issue

# salt sn1-pool-e01-15.puretec.purestorage.com state.highstate
sn1-pool-e01-15.puretec.purestorage.com:
    The minion function caused an exception: Traceback (most recent call last):
      File "/etc/salt/salt/minion.py", line 1523, in _thread_return
        return_data = minion_instance.executors[fname](opts, data, func, args, kwargs)
      File "/etc/salt/salt/executors/direct_call.py", line 12, in execute
        return func(*args, **kwargs)
      File "/etc/salt/salt/modules/state.py", line 890, in highstate
        orchestration_jid=orchestration_jid)
      File "/etc/salt/salt/state.py", line 3756, in call_highstate
        return self.state.call_high(high, orchestration_jid)
      File "/etc/salt/salt/state.py", line 2591, in call_high
        ret = self.call_chunks(chunks)
      File "/etc/salt/salt/state.py", line 2075, in call_chunks
        running = self.call_chunk(low, running, chunks)
      File "/etc/salt/salt/state.py", line 2424, in call_chunk
        running[tag] = self.call(low, chunks, running)
      File "/etc/salt/salt/state.py", line 1929, in call
        self.verify_ret_for_export(ret)
      File "/etc/salt/salt/state.py", line 1024, in verify_ret_for_export
        raise SaltException(msg.format(key, repr(ret[key])))
    SaltException: The value for the name key in the state return must be a string, found ['test.echo']

Minion log file:

2017-09-27 05:23:02,635 [salt.crypt                                     :400 ][DEBUG   ][37571] Initializing new AsyncAuth for (u'/etc/salt/pki/minion', 'sn1-pool-e01-15.puretec.purestorage.com', u'tcp://10.21.113.17:4506')
2017-09-27 05:23:02,646 [salt.minion                                    :1625][DEBUG   ][37571] minion return: {u'fun_args': [], u'jid': '20170927052327447373', u'return': u'The minion function caused an exception: Traceback (most recent call last):\n  File "/etc/salt/salt/minion.py", line 1523, in _thread_return\n    return_data = minion_instance.executors[fname](opts, data, func, args, kwargs)\n  File "/etc/salt/salt/executors/direct_call.py", line 12, in execute\n    return func(*args, **kwargs)\n  File "/etc/salt/salt/modules/state.py", line 890, in highstate\n    orchestration_jid=orchestration_jid)\n  File "/etc/salt/salt/state.py", line 3756, in call_highstate\n    return self.state.call_high(high, orchestration_jid)\n  File "/etc/salt/salt/state.py", line 2591, in call_high\n    ret = self.call_chunks(chunks)\n  File "/etc/salt/salt/state.py", line 2075, in call_chunks\n    running = self.call_chunk(low, running, chunks)\n  File "/etc/salt/salt/state.py", line 2424, in call_chunk\n    running[tag] = self.call(low, chunks, running)\n  File "/etc/salt/salt/state.py", line 1929, in call\n    self.verify_ret_for_export(ret)\n  File "/etc/salt/salt/state.py", line 1024, in verify_ret_for_export\n    raise SaltException(msg.format(key, repr(ret[key])))\nSaltException: The value for the name key in the state return must be a string, found [\'test.echo\']\n', u'success': False, u'fun': 'state.highstate', u'out': u'nested'}
2017-09-27 05:23:02,662 [salt.config                                    :2158][DEBUG   ][37187] Including configuration from '/etc/salt/minion.d/_schedule.conf'
2017-09-27 05:23:02,663 [salt.config                                    :2006][DEBUG   ][37187] Reading configuration from /etc/salt/minion.d/_schedule.conf
2017-09-27 05:23:03,056 [salt.minion                                    :2060][DEBUG   ][37187] Minion of 'sn1-pool-e01-13' is handling event tag '_salt_error'
2017-09-27 05:23:03,056 [salt.minion                                    :2231][DEBUG   ][37187] Forwarding salt error event tag=_salt_error
2017-09-27 05:23:03,057 [salt.transport.zeromq                          :86  ][DEBUG   ][37187] Initializing new AsyncZeroMQReqChannel for (u'/etc/salt/pki/minion', 'sn1-pool-e01-15.puretec.purestorage.com', u'tcp://10.21.113.17:4506', 'aes')
2017-09-27 05:23:03,057 [salt.crypt                                     :400 ][DEBUG   ][37187] Initializing new AsyncAuth for (u'/etc/salt/pki/minion', 'sn1-pool-e01-15.puretec.purestorage.com', u'tcp://10.21.113.17:4506')
2017-09-27 05:23:03,182 [salt.utils.lazy                                :97  ][DEBUG   ][37187] LazyLoaded config.merge
2017-09-27 05:23:03,185 [salt.utils.lazy                                :97  ][DEBUG   ][37187] LazyLoaded mine.update

Versions Report

Master and minion are exactly the same version and operating system

Salt Version:
           Salt: 2017.7.1-3912-g74379ba

Dependency Versions:
           cffi: Not Installed
       cherrypy: Not Installed
       dateutil: Not Installed
      docker-py: Not Installed
          gitdb: Not Installed
      gitpython: Not Installed
          ioflo: Not Installed
         Jinja2: 2.9.6
        libgit2: Not Installed
        libnacl: Not Installed
       M2Crypto: Not Installed
           Mako: Not Installed
   msgpack-pure: Not Installed
 msgpack-python: 0.4.8
   mysql-python: Not Installed
      pycparser: Not Installed
       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: 16.0.2
           RAET: Not Installed
          smmap: Not Installed
        timelib: Not Installed
        Tornado: 4.5.2
            ZMQ: 4.1.6

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

[gtmanfred]

I am unable to replicate this issue, but I do see another one right now, on the 2017.7 branch.

[root@salt ~]# salt-call --local state.apply test
[ERROR   ] 'test.echo' failed: Function expects 1 parameters, got only 0
local:
----------
          ID: storage
    Function: module.run
      Result: False
     Comment: 'test.echo' failed: Function expects 1 parameters, got only 0
     Started: 20:03:53.514074
    Duration: 9.261 ms
     Changes:

Summary for local
------------
Succeeded: 0
Failed:    1
------------
Total states run:     1
Total run time:   9.261 ms

@isbm can you take a look at this?

Thanks,
Daniel

[gtmanfred]

It appears that this is broken by ea83513

[sdodsley]

@gtmanfred so are you saying that my original issue is also caused by this, or are we now looking at a different issue?

[gtmanfred]

In general it seems like module.run doesn't seem to be working right now, and should be looked at.

[aneeshusa]

@sdodsley Your original issue is coming from a new lint I added in #43070; it looks like module.run has a non-standard return format, which the new code is trapping on. Long term it would be good to update module.run to fix this, but I'm not sure how much effort that is.

In the meantime, it may be prudent to lower the verify_ret_for_export lint from a hard exception to a logged warning; I suggested this in my PR since I had run into some other misbehaving modules there, and I'm not sure how many similar issues are still lurking. We could also leave it as-is and fix any bug reports as they come in, if there are likely only a few.

[sdodsley]

@aneeshusa looks like the module.run old format is going to be deprecated in Oxygen and the method failing here will be the default I think we need to at the very least fix the lint to be a warning. This will allow malformed responses to be tracked and fixed at a later date.

[gtmanfred]

The old format is being deprecated in Selenium. We are working on fixing this before 2017.7.2 is out, and then we will also be fixing it in develop, which is where you are seeing it fail.

…

On Thu, Sep 28, 2017 at 5:57 AM, Simon Dodsley ***@***.***> wrote: @aneeshusa <https://github.com/aneeshusa> looks like the '''module.run''' old format is going to be deprecated in Oxygen and the method failing here will be the default I think we need to at the very least fix the lint to be a warning. This will allow malformed responses to be tracked and fixed at a later date. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#43770 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAssoeZLnIk1MXYX1XNtXa7UrefFw7gNks5sm4mlgaJpZM4Plsm2> .

[sdodsley]

@gtmanfred Is Selenium after Fluorine? The Nitrogen release notes say this is being deprecated in Oxygen.

[gtmanfred]

Sorry, it is Sodium. https://github.com/saltstack/salt/blob/2017.7/salt/states/module.py#L218

…

On Thu, Sep 28, 2017 at 11:53 AM, Simon Dodsley ***@***.***> wrote: @gtmanfred <https://github.com/gtmanfred> Is Selenium after Fluorine? The Nitrogen release notes say this is being deprecated in Oxygen. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#43770 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAssoQLg6uLTGBUqINMoPiB60JK4Wqi0ks5sm90hgaJpZM4Plsm2> .

[Ch3LL]

is this still an issue?

[sagetherage]

@sdodsley @gtmanfred is this still an issue?

[sdodsley]

I can't seem to reproduce it after pulling the latest code.

[sagetherage]

Thank you for checking @sdodsley closing as no longer an issue.