salt: [REGRESSION] file.managed: state.apply "'Changes' should be a dictionary" error and unuseful output if invalid 'source' file path is specified

Description of Issue

Hello!

In Salt 2019.2.2 (and likely any 2019.2.x version), when using the file.managed state and specifying a source file that doesn’t actually exist (e.g. source: salt://path/to/nonexistent/file.txt), state.apply will return a "'Changes' should be a dictionary" error instead of returning the real error, i.e. that I’m trying to use a file that doesn’t exist in SaltFS. This is a break from 2018.3.x which actually reports a useful Source file not found error.

Setup

[root@localhost salt]# pwd
/tmp/salt

[root@localhost salt]# ls
state.sls

[root@localhost salt]# cat state.sls
/tmp/salt/target_file.txt:
  file.managed:
    - source: salt://not_a_file.txt

Steps to Reproduce Issue

Output when using Salt 2019.2.2:

[root@localhost salt]# salt-call --local --file-root=/tmp/salt/ state.apply state test=True
[ERROR   ] (False, u"Source file salt://not_a_file.txt not found in saltenv 'base'")
local:
The State execution failed to record the order in which all states were executed. The state return missing data is:
{u'changes': {},
 u'comment': u"An exception occurred in this state: 'Changes' should be a dictionary.",
 u'name': u'later',
 u'result': False}
----------
          ID: /tmp/salt/target_file.txt
    Function: file.managed
      Result: False
     Comment: An exception occurred in this state: 'Changes' should be a dictionary.
     Changes:

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

When using Salt 2018.3.4 the output is much more useful:

[root@localhost salt]# salt-call --local --file-root=/tmp/salt/ state.apply state test=True
[ERROR   ] Source file salt://not_a_file.txt not found
local:
----------
          ID: /tmp/salt/target_file.txt
    Function: file.managed
      Result: False
     Comment: Source file salt://not_a_file.txt not found
     Started: 11:00:08.781896
    Duration: 26.166 ms
     Changes:   Invalid Changes data: (False, u'Source file salt://not_a_file.txt not found')

Summary for local
------------
Succeeded: 0 (changed=1)
Failed:    1
------------
Total states run:     1
Total run time:  26.166 ms

Versions Report

Salt Version:
           Salt: 2019.2.2

Dependency Versions:
           cffi: 1.6.0
       cherrypy: Not Installed
       dateutil: 1.5
      docker-py: Not Installed
          gitdb: Not Installed
      gitpython: Not Installed
          ioflo: Not Installed
         Jinja2: 2.7.2
        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.14
       pycrypto: 2.6.1
   pycryptodome: Not Installed
         pygit2: Not Installed
         Python: 2.7.5 (default, Aug  7 2019, 00:51:29)
   python-gnupg: Not Installed
         PyYAML: 3.11
          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.7.1908 Core
         locale: UTF-8
        machine: x86_64
        release: 3.10.0-1062.4.1.el7.x86_64
         system: Linux
        version: CentOS Linux 7.7.1908 Core

About this issue

  • Original URL
  • State: open
  • Created 5 years ago
  • Reactions: 8
  • Comments: 31 (14 by maintainers)

Most upvoted comments

I just spent over an hour on this issue. I really think the error message could be better here.

Also seeing this issue

Salt Master:

Salt Version: Salt: 3002.2

Dependency Versions: cffi: Not Installed cherrypy: Not Installed dateutil: Not Installed docker-py: Not Installed gitdb: 0.6.4 gitpython: 1.0.1 Jinja2: 2.11.1 libgit2: Not Installed M2Crypto: 0.35.2 Mako: Not Installed msgpack: 0.6.2 msgpack-pure: Not Installed mysql-python: Not Installed pycparser: Not Installed pycrypto: Not Installed pycryptodome: Not Installed pygit2: Not Installed Python: 3.6.8 (default, Nov 16 2020, 16:55:22) python-gnupg: Not Installed PyYAML: 3.13 PyZMQ: 17.0.0 smmap: 0.9.0 timelib: Not Installed Tornado: 4.5.3 ZMQ: 4.1.4

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

Additionally updated the master to 3003.1 and still having issue.

I also ran into this issue on 3002.5, so it still exists.

In my case the cause was setting up a new salt master on 3002.5, from my git repo of stack/pillar, but forgetting that there are some files deliberately not checked into git which need to be hand copied over from the old master. The resulting error message is very obscure (I only realised quickly when I found this issue) for what should have been “source file not found”; if I’d got “source file not found” I’d have immediately realised what I’d forgotten to do when setting up the new salt server 😃

Ewen

I still saw this issue today in 3002.2. Thanks to @9numbernine9 for pointing out what the error would read, fixed my silly mistake.

Issue is still persistent 😦.

root@vagrant-test:~# salt-call --versions
Salt Version:
          Salt: 3004
 
Dependency Versions:
          cffi: Not Installed
      cherrypy: Not Installed
      dateutil: 2.8.1
     docker-py: Not Installed
         gitdb: 4.0.5
     gitpython: 3.1.14
        Jinja2: 2.11.3
       libgit2: Not Installed
      M2Crypto: Not Installed
          Mako: Not Installed
       msgpack: 1.0.0
  msgpack-pure: Not Installed
  mysql-python: Not Installed
     pycparser: Not Installed
      pycrypto: Not Installed
  pycryptodome: 3.9.7
        pygit2: Not Installed
        Python: 3.9.2 (default, Feb 28 2021, 17:03:44)
  python-gnupg: Not Installed
        PyYAML: 5.3.1
         PyZMQ: 20.0.0
         smmap: 4.0.0
       timelib: Not Installed
       Tornado: 4.5.3
           ZMQ: 4.3.4
 
System Versions:
          dist: debian 11 bullseye
        locale: utf-8
       machine: x86_64
       release: 5.10.0-9-amd64
        system: Linux
       version: Debian GNU/Linux 11 bullseye

Still happening

freebsd_vm:
The State execution failed to record the order in which all states were executed. The state return missing data is:
{'changes': {},
 'comment': "An exception occurred in this state: 'Changes' should be a "
            'dictionary.',
 'name': 'later',
 'result': False}

Looks like we didn’t get to this work in Aluminium I will put it into Silicon and get it reassigned

Thanks for reporting this! I’m thinking that there may have been a code path that has been introduced which leads to reporting this error, or possibly the error message has changed. But I agree, the error message should have a better message.

@dractyl I will try your patch next week. Thank you !

should_be_a_dictionary_patch.txt

This patch fixed it for me.

             if isinstance(ret["changes"], tuple):
                 ret["result"], ret["comment"] = ret["changes"]
             ...
             return ret

ret[“changes”] is clearly a tuple here, but it does not get changed before being returned, so the error message is correct; it’s definitely not a dictionary.

By changing it like the below, which appears to be in line with the intention of the code (No changes because the source file does not exist), the bug is fixed.

             if isinstance(ret["changes"], tuple):
                 ret["result"], ret["comment"] = ret["changes"]
                 ret["changes"] = {}
             ...
             return ret

I knew this looked familiar. Ran into it again!

Just ran into this error on salt-3001.7 as well