Salt masters re-render pillar data in various state modules (including state.sls)

[jtraub91]

Description of Issue

Pulling large amounts of pillar data from the salt master can be an expensive operation. For this reason, pillar refreshes should only occur when explicitly requested or heavily implied (e.g. minion startup, saltutil.sync_all, saltutil.refresh_pillar, state.highstate, state.apply with no state specified). However, through observation and troubleshooting it is evident fresh pillar data is being pulled from the master to the minions when executing at least the following commands pillar.items, state.sls <state> state.apply <state>.

Setup

Normal salt installation. Put some dummy pillar data and apply via top, do not perform a pillar refresh nor restart of salt-minion.service.

My test case pillar looked like this:

# /srv/pillar/base.sls
yoyo: toy

# /srv/pillar/top.sls
base:
  '*':
    - base

Steps to Reproduce Issue

Run some commands to reveal issue:

# salt \* pillar.item yoyo
minion1:
    ----------
    yoyo:

^ This is expected behavior without a pillar refresh, but:

# salt \* pillar.items
minion1:
    yoyo:
        toy

is not.

Furthermore, the pillar will be pulled during state.sls and state.apply. Let's create a state to test this:

# /srv/salt/yoyo.sls
echo_yoyo:
  cmd.run:
    - name: echo {{ pillar['yoyo'] }}

And call it:

# salt \* state.sls yoyo
minion1:
----------
          ID: echo_yoyo
    Function: cmd.run
        Name: echo toy
      Result: True
     Comment: Command "echo toy" run
     Started: 18:57:07.672240
    Duration: 15.125 ms
     Changes:
              ----------
              pid:
                  xxxxx
              retcode:
                  0
              stderr:
              stdout:
                  toy

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

and

# salt \* state.apply yoyo
minion1:
----------
          ID: echo_yoyo
    Function: cmd.run
        Name: echo toy
      Result: True
     Comment: Command "echo toy" run
     Started: 18:58:09.340982
    Duration: 21.442 ms
     Changes:
              ----------
              pid:
                  xxxxx
              retcode:
                  0
              stderr:
              stdout:
                  toy

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

Versions Report

# salt --versions-report
Salt Version:
           Salt: 2019.2.0

Dependency Versions:
           cffi: Not Installed
       cherrypy: Not Installed
       dateutil: 2.7.3
      docker-py: Not Installed
          gitdb: 2.0.5
      gitpython: 2.1.11
          ioflo: Not Installed
         Jinja2: 2.10
        libgit2: Not Installed
        libnacl: Not Installed
       M2Crypto: 0.31.0
           Mako: 1.0.7
   msgpack-pure: Not Installed
 msgpack-python: 0.5.6
   mysql-python: Not Installed
      pycparser: Not Installed
       pycrypto: 2.6.1
   pycryptodome: Not Installed
         pygit2: Not Installed
         Python: 2.7.16 (default, Oct  7 2019, 17:36:04)
   python-gnupg: 0.4.3
         PyYAML: 3.13
          PyZMQ: 17.1.2
           RAET: Not Installed
          smmap: 2.0.5
        timelib: Not Installed
        Tornado: 5.1.1
            ZMQ: 4.3.1

System Versions:
           dist: Ubuntu 19.04 disco
         locale: UTF-8
        machine: x86_64
        release: 5.0.0-25-generic
         system: Linux
        version: Ubuntu 19.04 disco

[jtraub91]

It appears evident from the source, that when calling state.sls a 'highstate' is in fact being called with the pillar data being requested 'fresh' from the master.

https://github.com/saltstack/salt/blob/master/salt/modules/state.py#L1301
https://github.com/saltstack/salt/blob/master/salt/state.py#L4075
https://github.com/saltstack/salt/blob/master/salt/state.py#L741

[alan-cugler]

Verified in my cluster tests. ran in salt version 2019.2.0, 2019.2.1, & 2019.2.2

[Paulo-Nunes]

pillar.items is documented to pull fresh pillar data.
That is its intended behavior: https://docs.saltstack.com/en/latest/ref/modules/all/salt.modules.pillar.html#salt.modules.pillar.items

I think what you are looking for is pillar.raw https://docs.saltstack.com/en/latest/ref/modules/all/salt.modules.pillar.html#salt.modules.pillar.raw

I think it is intuitive to use state.apply / state.sls and expect it to be acting on the latest pillar. I think that is also its intended behavior. Maybe ask for a flag to be set to avoid getting new pillar when executing these commands rather than changing the default behavior.

I can only see this as "Working as intended"

[max-arnold]

I tend to agree that this is intended behavior. You can enable pillar caching to reduce the load: https://docs.saltstack.com/en/latest/ref/configuration/master.html#pillar-cache-options

However, there is a recently fixed related regression: #54941

[alan-cugler]

Thanks for the clarification on pillar.items I didnt realize asking for a printout included a refresh. I reused the same test but used pillar.rawinstead as suggested. This resulted in a no refresh pillars as wanted.

Now for the flag suggestion, did you mean as a CLI flag or as a master.conf key:value flag?

[grichmond-salt]

Note: state.apply (without an argument) and state.highstate are intended to generate the latest pillar data. For performance reasons, state.sls foo or state.apply foo should NOT request the latest pillar data from the master.

[grichmond-salt]

The attached screenshot shows the issue.

A call to state.sls is made (at left), which should NOT request the latest pillar data. The master's log (at right) shows pillar being generated. This is absolutely horrible for performance in large environments.

[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.

[dwoz]

Looks like the change in behavior for state.sls was introduced here:

#41679
8d6fdb7

[dwoz]

Looks like the change in behavior for state.sls was introduced here:

#41679
8d6fdb7

Okay, the above comment is totally wrong. The PR I linked to is not changing the behavior. Also, I've verified that we do intend to request fresh pillar data any time we run a state (including state.sls). This was a part of the original design of the pillar system and has been working consistently in this way since it's implementation.

[thatch45]

Thanks for tracking this down @dwoz I originally wrote the state system to always reload the pillar when calling state runs. this is critical to ensure execution consistency.
When this is a performance issue the best thing to do is to enable the pillar cache on the master and set the pillar cache timeout accordingly. The pillar cache can either be in memory or on disk depending on your requirements when the pillar is cached on disk it is stored in a msgpack file that is VERY quickly read.

I suggest this issue is closed and that we open another issue to ensure that documentation is accurate everywhere and that we add an option to state runs to not refresh the pillar. They all possible bases here should be fully covered.

[sagetherage]

done @thatch45

[jtraub91]

@sagetherage have you added the option for state runs to not refresh the pillar (or opened the issue for it)?