Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUG] Salt-proxy module __pillar__ and opts["pillar"] not refreshed on saltutil.refresh_pillar #58197

Open
dmulyalin opened this issue Aug 14, 2020 · 3 comments
Labels
Bug broken, incorrect, or confusing behavior Proxy-Minion severity-medium 3rd level, incorrect or bad functionality, confusing and lacks a work around
Milestone

Comments

@dmulyalin
Copy link

dmulyalin commented Aug 14, 2020

Description
Salt-proxy module in-memory pillar dunder __pillar__ and opts["pillar"] not refreshed on saltutil.refresh_pillar call.

That is true for both multiprocessing set to True or False

Setup
Setup dummy proxy in base env
https://github.com/saltstack/salt/blob/master/salt/proxy/dummy.py

Add these two functions to dummy.py salt-proxy:

...

def check_pillar():
    return __pillar__

def check_opts():
    return __opts__

And created this execution module:
[salt://_modules/dummy.py]

# -*- coding: utf-8 -*-
from __future__ import absolute_import

__virtualname__ = "dummy"
__proxyenabled__ = ["dummy"]

def __virtual__():
    return __virtualname__

def check_pillar():
    return __proxy__["dummy.check_pillar"]()

def check_opts():
    return __proxy__["dummy.check_opts"]()

Steps to Reproduce the behavior
Run salt-proxy dummy module, update pillar data, run salt dummy-proxy-id saltutil.refresh_pillar, after that run and compare output from these commands:

1 - salt dummy-proxy-id pillar.items
2 - salt dummy-proxy-id dummy.check_opts
3 - salt dummy-proxy-id dummy.check_pillar

to observe that (1) returns updated pillar info while 2 and 3 pillar data remains the same. The only way to update salt-proxy pillar data is to restart the process.

Expected behavior
Expected behavior described here:
https://docs.saltstack.com/en/latest/topics/pillar/index.html#pillar-in-memory

namely:
... the in-memory pillar data is available as the __pillar__ dunder dictionary.

The in-memory pillar data is generated on minion start, and can be refreshed using the saltutil.refresh_pillar function

Versions Report

salt --versions-report (Provided by running salt --versions-report. Please also mention any differences in master/minion versions.)
Salt Version:
           Salt: 3001

Dependency Versions:
           cffi: 1.12.3
       cherrypy: unknown
       dateutil: 2.8.0
      docker-py: Not Installed
          gitdb: Not Installed
      gitpython: Not Installed
         Jinja2: 2.11.1
        libgit2: Not Installed
       M2Crypto: 0.35.2
           Mako: Not Installed
   msgpack-pure: Not Installed
 msgpack-python: 0.6.2
   mysql-python: Not Installed
      pycparser: 2.19
       pycrypto: 2.6.1
   pycryptodome: Not Installed
         pygit2: Not Installed
         Python: 3.6.8 (default, Apr  2 2020, 13:34:55)
   python-gnupg: Not Installed
         PyYAML: 3.13
          PyZMQ: 17.0.0
          smmap: Not Installed
        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-1127.13.1.el7.x86_64
         system: Linux
        version: CentOS Linux 7 Core

Additional context
Using __salt__['pillar.items']() as a workaround to get latest pillar data from master works, but seems to me as a less optimal solution compared to updating opt["pillar"] or __pillar__ dunder on saltutil.refresh_pillar call.

Did some debugging and was able to get expected behavior by appending this line:

self.opts["pillar"] = yield async_pillar.compile_pillar()

with this block of code:

                self.opts["pillar"] =  yield async_pillar.compile_pillar()
+               log.debug("Refreshing proxy opts['pillar']")
+               self.proxy.opts["pillar"] = self.opts["pillar"]
+               log.debug("refreshing proxy __pillar__ dunder")
+               self.proxy.pack["__pillar__"] = self.opts["pillar"]
+               self.proxy.reload_modules()

While that works, not sure that this is the most optinal way of doing it, and not sure that this is actually should be done at all, as it seems to me that being able to refresh pillar data for minion is a basic functionality and that problem should be cought long ago, unless it is expected that salt-proxy minion pillar data should not be update on saltutil.refresh_pillar call.

Please advise.

@dmulyalin dmulyalin added the Bug broken, incorrect, or confusing behavior label Aug 14, 2020
@dmulyalin
Copy link
Author

Noticed such a thing - running self.proxy.reload_modules() on saltutil.refresh_pillar for salt-proxy minion causes negative side-effect in a form of destroying all the structures that were created during init() function call on proxy-minion initialisation.

@garethgreenaway garethgreenaway added severity-medium 3rd level, incorrect or bad functionality, confusing and lacks a work around and removed needs-triage labels Oct 27, 2020
@garethgreenaway garethgreenaway added this to the Approved milestone Oct 27, 2020
@garethgreenaway
Copy link
Contributor

@dmulyalin Thanks for the report. Proxy minions should have the ability to refresh the values from pillar, it does get a little complicated since their configuration is stored as pillar values. Will need to dive into this and see what is happening. Thanks!

@garethgreenaway garethgreenaway removed their assignment Jan 11, 2021
@cbosdo
Copy link
Contributor

cbosdo commented Jul 21, 2021

I hit a similar issue without the proxy minions too. The minion modules LazyLoader doesn't get the refreshed pillar, so modules will be updated with the old pillar. This issue can be observed when removing a beacon from the pillar: the beacon will continue to be called after the saltutil.refresh_pillar until a saltutil.refresh_modules is called.

Adding a self.module_refresh(force_refresh) after the pillar refresh in the minion code you pointed to also fixes the issue. I haven't looked at the states loading code, but may be those would have a similar issue.

cbosdo added a commit to cbosdo/uyuni that referenced this issue Jul 21, 2021
As mentioned in saltstack/salt#58197 (comment)
the refresh_pillar doesn't set the new pillar on the modules. This
causes the beacons to still use the old configuraton.
cbosdo added a commit to uyuni-project/uyuni that referenced this issue Jul 21, 2021
As mentioned in saltstack/salt#58197 (comment)
the refresh_pillar doesn't set the new pillar on the modules. This
causes the beacons to still use the old configuraton.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug broken, incorrect, or confusing behavior Proxy-Minion severity-medium 3rd level, incorrect or bad functionality, confusing and lacks a work around
Projects
None yet
Development

No branches or pull requests

5 participants