Non-deterministic pillar rendering -- A simple example

brianthelion commented 6 years ago

Description of Issue/Question

Without explicitly setting the pillarenv, pillar rendering is non-deterministic under salt-call --local.

Setup

#! /bin/bash

LOG_LVL=all

ROOT_DIR=$(mktemp -d)

CONFIG_DIR=$ROOT_DIR/etc/salt
FILE_ROOT_BASE=$(mktemp -d)
PILLAR_ROOT_BASE=$ROOT_DIR/srv/pillar/base
PILLAR_ROOT_FOO=$ROOT_DIR/srv/pillar/foo

mkdir -p $CONFIG_DIR

cat <<EOF > $CONFIG_DIR/minion
root_dir: $ROOT_DIR
pillar_roots:
  base:
    - $PILLAR_ROOT_BASE
  foo:
    - $PILLAR_ROOT_FOO
EOF

cat <<EOF > $CONFIG_DIR/minion_id
my_minion
EOF

mkdir -p $PILLAR_ROOT_BASE
cat <<EOF > $PILLAR_ROOT_BASE/top.sls
base:
  'my_minion':
    - bar
EOF

cat <<EOF > $PILLAR_ROOT_BASE/bar.sls
A:
 B: 'base'
EOF

mkdir -p $PILLAR_ROOT_FOO
cat <<EOF > $PILLAR_ROOT_FOO/top.sls
foo:
  'my_minion':
    - baz
EOF

cat <<EOF > $PILLAR_ROOT_FOO/baz.sls
A:
 B: 'foo'
EOF

for i in {1..10}
do
    salt-call --local --log-file-level=$LOG_LVL --config-dir=$CONFIG_DIR pillar.items;
done

echo "=================="

for i in {1..10}
do
    salt-call --local --log-file-level=$LOG_LVL --config-dir=$CONFIG_DIR pillar.items pillarenv=base;
done

rm -rf $ROOT_DIR $FILE_ROOT_BASE $FILE_ROOT_FOO

salt --versions-report

Steps to Reproduce Issue

Run the script. Example output:

local:
    ----------
    A:
        ----------
        B:
            foo
local:
    ----------
    A:
        ----------
        B:
            base
local:
    ----------
    A:
        ----------
        B:
            foo
local:
    ----------
    A:
        ----------
        B:
            foo
local:
    ----------
    A:
        ----------
        B:
            foo
local:
    ----------
    A:
        ----------
        B:
            foo
local:
    ----------
    A:
        ----------
        B:
            base
local:
    ----------
    A:
        ----------
        B:
            foo
local:
    ----------
    A:
        ----------
        B:
            base
local:
    ----------
    A:
        ----------
        B:
            base
==================
local:
    ----------
    A:
        ----------
        B:
            base
local:
    ----------
    A:
        ----------
        B:
            base
local:
    ----------
    A:
        ----------
        B:
            base
local:
    ----------
    A:
        ----------
        B:
            base
local:
    ----------
    A:
        ----------
        B:
            base
local:
    ----------
    A:
        ----------
        B:
            base
local:
    ----------
    A:
        ----------
        B:
            base
local:
    ----------
    A:
        ----------
        B:
            base
local:
    ----------
    A:
        ----------
        B:
            base
local:
    ----------
    A:
        ----------
        B:
            base

Versions Report

Salt Version:
           Salt: 2017.7.0-939-g2b7adcc

Dependency Versions:
           cffi: Not Installed
       cherrypy: Not Installed
       dateutil: 2.6.1
      docker-py: 2.6.1
          gitdb: 2.0.3
      gitpython: 2.1.7
          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.4.8
   mysql-python: Not Installed
      pycparser: Not Installed
       pycrypto: 2.6.1
   pycryptodome: Not Installed
         pygit2: Not Installed
         Python: 3.5.3 (default, Nov 23 2017, 11:34:05)
   python-gnupg: Not Installed
         PyYAML: 3.12
          PyZMQ: 16.0.3
           RAET: Not Installed
          smmap: 2.0.3
        timelib: Not Installed
        Tornado: 4.5.2
            ZMQ: 4.1.6

System Versions:
           dist: Ubuntu 17.04 zesty
         locale: UTF-8
        machine: x86_64
        release: 4.12.0-041200rc6-generic
         system: Linux
        version: Ubuntu 17.04 zesty

brianthelion commented 6 years ago

https://github.com/saltstack/salt/issues/19332 -- "Nondeterminism in Pillar" (Jan 2015)
https://github.com/saltstack/salt/issues/24501 -- "Order in top.sls file is not respected for pillar data in local mode" (Jun 2015)
https://github.com/saltstack/salt/issues/37322 -- "Feature request: allow master_tops to be single source of truth" (Oct 2016)

It appears that non-determinism in the pillar is a classic salt "feature" that may or may not have been removed. My code shows that it's still lurking. In many of these cases, the answer seems to be, "It will be OK when OrderedDict comes." But that's not what we're seeing now, so I turned to the documentation and found mixed messages about if/how the non-determinism can be avoided:

salt.pillar.get_pillar: How multiple remotes are handled -- Note! Prior to the 2017.7.0 release, remotes would be evaluated in a non-deterministic order.
Configuring the Master: Pillar merging options -- No mention of non-determinism.
Storing static data in the pillar: Pillar namespace merges -- Note! Remember: conflicting keys will be overwritten in a non-deterministic manner!

gtmanfred commented 6 years ago

@brianthelion thanks for the effort to document this!

@terminalmage can you take a look at this when you get a chance. It looks like you are working on some of this for Fluorine.

Thanks! Daniel

whytewolf commented 6 years ago

ZD-2198

stale[bot] commented 5 years ago

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[bot] commented 4 years ago

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.

whytewolf commented 4 years ago

this is not stale

stale[bot] commented 4 years ago

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

stale[bot] commented 4 years ago

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.

brianthelion commented 4 years ago

This is not stale.

stale[bot] commented 4 years ago

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

tj90241 commented 3 years ago

Bumped into this and pushed a non-complete PR... #59918

The PR assumes cpython>=3.6 and retains non-deterministic behavior for older pythons. I could change it to support order pythons by not assuming defaultdict is ordered, but the patch would not be as elegant.

sagetherage commented 3 years ago

We are aware that the insertion-order preservation nature of dict objects has been declared to be an official part of the Python language spec; however, this is not necessarily true in Salt.

tj90241 commented 3 years ago

FWIW, I have been using the patch from the PR above on a dozen machines for a few weeks now and the problem has not re-emerged for me. The pillar roots are always scanned in the order in which they appear in the config.

sagetherage commented 3 years ago

@tj90241 I have unlinked the PR your closed PR mostly for GitHub syntax searching on linked issues to PRs. Leaving this open until it can get picked up, again. Thank you!

saltstack / salt