Error when using multiple grains.list_present with dict types in name

[Popovych49]

Description of Issue/Question

Hi,

There is a weird behavior of grains.list_present when dict-type in name.

When there is two 'grains.list_present' which follow each other, the second state failed.

The first state create the key of the second state, without the value...

Setup

test:
  grains.list_present:
    - name: key:subkey:subsubkey1
    - value: value1

test1:
  grains.list_present:
    - name: key:subkey:subsubkey2
    - value: value2

Steps to Reproduce Issue

Launch the previous sls and you will have the following result.

minion1:
----------
          ID: test
    Function: grains.list_present
        Name: key:subkey:subsubkey1
      Result: True
     Comment: Append value value1 to grain key:subkey:subsubkey1
     Started: 09:42:42.402113
    Duration: 271.755 ms
     Changes:
              ----------
              new:
                  ----------
                  key:
                      ----------
                      subkey:
                          ----------
                          subsubkey1:
                              - value1
                          subsubkey2:
                              ----------
----------
          ID: test1
    Function: grains.list_present
        Name: key:subkey:subsubkey2
      Result: False
     Comment: Failed append value value2 to grain key:subkey:subsubkey2
     Started: 09:42:42.676705
    Duration: 3.28 ms
     Changes:

Summary for minion1
------------
Succeeded: 1 (changed=1)
Failed:    1
------------
Total states run:     2
Total run time: 275.035 ms

Versions Report

Salt Version:
	   Salt: 2018.3.0

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.7.2
        libgit2: Not Installed
        libnacl: Not Installed
       M2Crypto: 0.21.1
           Mako: Not Installed
   msgpack-pure: Not Installed
 msgpack-python: 0.5.1
   mysql-python: Not Installed
      pycparser: Not Installed
       pycrypto: 2.6.1
   pycryptodome: Not Installed
         pygit2: Not Installed
         Python: 2.7.5 (default, Aug  4 2017, 00:39:18)
   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.4.1708 Core
         locale: UTF-8
        machine: x86_64
        release: 3.10.0-693.11.6.el7.x86_64
         system: Linux
        version: CentOS Linux 7.4.1708 Core

[Ch3LL]

looks like i'm able to replicate this in older versions of salt as well including 2017.7.2 so this is not a regression. If I run the state the second time it looks like it resolves okay.

[Popovych49]

Yes, I found it in 2017.7.2 and test it in 2018

I think it resolve the second time because of this cinematic :

• At the end of the first time, the key of second state is delete because no value.
• The second time, the first state is not launch because no changes are expected, so the second state works fine

[stale]

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.

[Popovych49]

Hi, do you have any news about this issue?
Thanks :),

[stale]

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

[Ch3LL]

@Popovych49 no one is currently assigned to this issue due to other higher priority issues, so please feel free to push a PR if you are able to. thanks

[stale]

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.

[stale]

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

[tgriffitts-ns]

Yeah, so we've run into this issue too. The problem seems to be this logic:

https://github.com/saltstack/salt/blob/master/salt/utils/data.py#L852-L854

This relies on ptr[each] throwing a KeyError exception if the key doesn't exist. The problem is that the new YAML loader methods create defaultdict objects which don't throw the KeyError exception if the key doesn't exist but instead return a default empty dictionary.

The fix is simple, simply check if the key exists in dict to branch logic instead of relying on the exception being thrown. This patch fixed things for us. I am also including a no-whitespace patch which shows the 2 lines we changed because the real patch indents a block of code and we're patching against 3002.8 so the indented block may not look the same for you. Be sure to change the 2 lines from the no-whitespace patch and indent the same block in your version of salt from the real patch.

I did check the latest version of salt as of this comment and this method still depends on the exception being thrown to check if the key doesn't exist.

traverse_dict_and_list.patch.txt
traverse_dict_and_list-no-ws.patch.txt

Hope this is useful.

[tgriffitts-ns]

This was our test case which failed in a number of different ways before the patch. It always required a few state.apply calls to get all the grains set, but sometimes would fail in a unrecoverable state with a list grain being set to the defaultdict {} and then could never add a value to the list.

This should fail for you before the patch and work for you without fail and without repeating state.apply calls, after the patch

grains-test|a:
  grains.list_present:
    - name: core-services:monitored
    - value: "basic"

grains-test|b:
  grains.list_present:
    - name: core-services:mon-config:rules
    - value: "rules1"

grains-test|c:
  grains.list_present:
    - name: core-services:mon-config:store-servers
    - value: "1.1.1.1"

grains-test|d:
  grains.list_present:
    - name: core-services:mon-config:rules
    - value: "rules2"

[iodbh]

Ran into this issue as well, adding - reload_grains: true to the list_present states seems to work around the issue