salt: [BUG] 3006.5 "grains" extmods folder cleaned in unexpected situations

Description In 3006.5, the extmods grains folder is removed in many situations:

when the --local option is passed to a salt-call command and the minion has previously sync’d grains from a master, losing all previously synchronized grain modules

The folder also seems to be removed if the minion cannot connect to the master. I.E. a mastered minion, whose master is down, will clean its grains folder and lose all sync’d grains until the master is “alive” again and they can be resync’d.

The folder is removed on other commands, even when the master is alive, for example, salt-call test.ping will remove the grains extmods folder and the minion will lose sync’d grains.

Setup (Please provide relevant configs and/or SLS files (be sure to remove sensitive info. There is no general set-up of Salt.)

Please be as specific as possible and give set-up details.

  • on-prem machine
  • VM (Virtualbox, KVM, etc. please specify)
  • VM running on a cloud service, please be explicit and add details
  • container (Kubernetes, Docker, containerd, etc. please specify)
  • or a combination, please be explicit
  • jails if it is FreeBSD
  • classic packaging
  • onedir packaging
  • used bootstrap to install

Steps to Reproduce the behavior

Situation 1: “–local” option

configure a minion with a master that provides additional grain modules via the _grains folder sync the grains from the master salt-call saltutil.sync_grains or salt-call saltutil.sync_all, /var/cache/salt/minion/grains should have the grains modules from the _grains folder on the master use salt-call --local <anything> and the /var/cache/salt/minion/grains folder will be removed - other extmods folders (like modules, states, etc do not seem to be affected).

Situation 2: module commands:

salt-call saltutil.sync_all extmods grains folder exists salt-call test.ping grains extmods folder is removed (other modules tested where extmods grains folder is removed: grains.get, grains.items

Situation 3: master is down

salt-call saltutil.sync_all - extmods grains folder exists …take master offline… salt-call grains.items - extmods grains folder is removed

Expected behavior

The grains extmods folder is not cleaned in any of the specified scenarios.

Screenshots If applicable, add screenshots to help explain your problem.

Versions Report

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

Python Version:
        Python: 3.10.13 (main, Nov 15 2023, 04:34:27) [GCC 11.2.0]

Dependency Versions:
          cffi: 1.14.6
      cherrypy: 18.6.1
      dateutil: 2.8.1
     docker-py: Not Installed
         gitdb: Not Installed
     gitpython: Not Installed
        Jinja2: 3.1.2
       libgit2: Not Installed
  looseversion: 1.0.2
      M2Crypto: Not Installed
          Mako: Not Installed
       msgpack: 1.0.2
  msgpack-pure: Not Installed
  mysql-python: Not Installed
     packaging: 22.0
     pycparser: 2.21
      pycrypto: Not Installed
  pycryptodome: 3.9.8
        pygit2: Not Installed
  python-gnupg: 0.4.8
        PyYAML: 6.0.1
         PyZMQ: 23.2.0
        relenv: 0.14.2
         smmap: Not Installed
       timelib: 0.2.4
       Tornado: 4.5.3
           ZMQ: 4.3.4

System Versions:
          dist: rhel 9.3 Plow
        locale: utf-8
       machine: x86_64
       release: 5.14.0-362.13.1.el9_3.x86_64
        system: Linux
       version: Red Hat Enterprise Linux 9.3 Plow

Additional context Based on 3006.5 release notes alone, I would suspect this is a side effect of #65186

A minion that is explicitly configured as “local” does not seem to exhibit any of these symptoms.

About this issue

  • Original URL
  • State: closed
  • Created 7 months ago
  • Reactions: 4
  • Comments: 30 (27 by maintainers)

Commits related to this issue

Most upvoted comments

A patch for 3006.5/6:

{% if grains.saltversion in ["3006.5", "3006.6"] %}
patch:
  pkg.installed

grains_sync_patch:
  file.patch:
    - name: '{{ grains.saltpath }}'
    - source: salt://65738.diff
    - strip: 2
    - require:
        - pkg: patch

restart_salt_minion:
  cmd.run:
    - name: 'salt-call service.restart salt-minion'
    - bg: true
    - onchanges:
      - file: grains_sync_patch
{%- endif %}
65738.diff
diff --git a/salt/fileclient.py b/salt/fileclient.py
index 42e7120aab18..443861cc03fd 100644
--- a/salt/fileclient.py
+++ b/salt/fileclient.py
@@ -45,15 +45,12 @@
 MAX_FILENAME_LENGTH = 255
 
 
-def get_file_client(opts, pillar=False, force_local=False):
+def get_file_client(opts, pillar=False):
     """
     Read in the ``file_client`` option and return the correct type of file
     server
     """
-    if force_local:
-        client = "local"
-    else:
-        client = opts.get("file_client", "remote")
+    client = opts.get("file_client", "remote")
 
     if pillar and client == "local":
         client = "pillar"
diff --git a/salt/minion.py b/salt/minion.py
index 0c5c77a91e50..15d46b2dacf9 100644
--- a/salt/minion.py
+++ b/salt/minion.py
@@ -114,29 +114,6 @@
 # 6. Handle publications
 
 
-def _sync_grains(opts):
-    # need sync of custom grains as may be used in pillar compilation
-    # if coming up initially and remote client, the first sync _grains
-    # doesn't have opts["master_uri"] set yet during the sync, so need
-    # to force local, otherwise will throw an exception when attempting
-    # to retrieve opts["master_uri"] when retrieving key for remote communication
-    # in addition opts sometimes does not contain extmod_whitelist and extmod_blacklist
-    # hence set those to defaults, empty dict, if not part of opts, as ref'd in
-    # salt.utils.extmod sync function
-    if opts.get("extmod_whitelist", None) is None:
-        opts["extmod_whitelist"] = {}
-
-    if opts.get("extmod_blacklist", None) is None:
-        opts["extmod_blacklist"] = {}
-
-    if opts.get("file_client", "remote") == "remote" and not opts.get(
-        "master_uri", None
-    ):
-        salt.utils.extmods.sync(opts, "grains", force_local=True)
-    else:
-        salt.utils.extmods.sync(opts, "grains")
-
-
 def resolve_dns(opts, fallback=True):
     """
     Resolves the master_ip and master_uri options
@@ -944,7 +921,6 @@ def __init__(self, opts, context=None):
         # Late setup of the opts grains, so we can log from the grains module
         import salt.loader
 
-        _sync_grains(opts)
         opts["grains"] = salt.loader.grains(opts)
         super().__init__(opts)
 
diff --git a/salt/utils/extmods.py b/salt/utils/extmods.py
index 6a4d5c14440c..a7dc265476f5 100644
--- a/salt/utils/extmods.py
+++ b/salt/utils/extmods.py
@@ -39,7 +39,6 @@ def sync(
     saltenv=None,
     extmod_whitelist=None,
     extmod_blacklist=None,
-    force_local=False,
 ):
     """
     Sync custom modules into the extension_modules directory
@@ -83,9 +82,7 @@ def sync(
                         "Cannot create cache module directory %s. Check permissions.",
                         mod_dir,
                     )
-            with salt.fileclient.get_file_client(
-                opts, pillar=False, force_local=force_local
-            ) as fileclient:
+            with salt.fileclient.get_file_client(opts) as fileclient:
                 for sub_env in saltenv:
                     log.info("Syncing %s for environment '%s'", form, sub_env)
                     cache = []

Reverting changes in PR https://github.com/saltstack/salt/pull/65186, since incomplete solution, will redo in another PR later, but want to quickly fix the issue, and will address the original issue #65027 with better testing etc.

the breakage in Salt 3006.5 has been rolled back and normal grains should be available in the next bug release which will be soon, had to get the CVE release out first