From 77ffe2b13ddfb2c9f312560b2ef91c3fe7519738 Mon Sep 17 00:00:00 2001 From: "Daniel A. Wozniak" Date: Sun, 17 Dec 2023 23:01:11 -0700 Subject: [PATCH 01/13] Even more reliable pillar_timeout test --- tests/pytests/integration/minion/test_return_retries.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/pytests/integration/minion/test_return_retries.py b/tests/pytests/integration/minion/test_return_retries.py index 5e44d17317c..d6456e4a47e 100644 --- a/tests/pytests/integration/minion/test_return_retries.py +++ b/tests/pytests/integration/minion/test_return_retries.py @@ -55,7 +55,7 @@ def test_publish_retry(salt_master, salt_minion_retry, salt_cli, salt_run_cli): @pytest.mark.slow_test def test_pillar_timeout(salt_master_factory): cmd = """ - python -c "import time; time.sleep(3.0); print('{\\"foo\\": \\"bar\\"}');\" + python -c "import time; time.sleep(5); print('{\\"foo\\": \\"bar\\"}');\" """.strip() master_overrides = { "ext_pillar": [ From 5b58900c6a0f36754b67e9ce43e25d794f9dcaa0 Mon Sep 17 00:00:00 2001 From: "Daniel A. Wozniak" Date: Wed, 13 Dec 2023 17:13:57 -0700 Subject: [PATCH 02/13] Fix usage of __salt__ in __utils__ Fix cases where loaded execution modules call a __utils__ method which in-turn calls a execution module via __salt__ method. --- salt/loader/__init__.py | 4 +++ salt/loader/context.py | 9 ++++- tests/pytests/unit/loader/test_loader.py | 24 +++++++++++++ .../unit/loader/test_loading_modules.py | 35 +++++++++++++++++++ 4 files changed, 71 insertions(+), 1 deletion(-) create mode 100644 tests/pytests/unit/loader/test_loading_modules.py diff --git a/salt/loader/__init__.py b/salt/loader/__init__.py index 8f2a69dc6b6..38382a32a51 100644 --- a/salt/loader/__init__.py +++ b/salt/loader/__init__.py @@ -324,6 +324,10 @@ def minion_mods( pack_self="__salt__", ) + # Allow the usage of salt dunder in utils modules. + if utils and isinstance(utils, LazyLoader): + utils.pack["__salt__"] = ret + # Load any provider overrides from the configuration file providers option # Note: Providers can be pkg, service, user or group - not to be confused # with cloud providers. diff --git a/salt/loader/context.py b/salt/loader/context.py index 6bbfe4dbd81..560b05a8d2b 100644 --- a/salt/loader/context.py +++ b/salt/loader/context.py @@ -12,6 +12,8 @@ except ImportError: # Py<3.7 import contextvars +import salt.exceptions + DEFAULT_CTX_VAR = "loader_ctxvar" loader_ctxvar = contextvars.ContextVar(DEFAULT_CTX_VAR) @@ -69,7 +71,12 @@ class NamedLoaderContext(collections.abc.MutableMapping): return loader.pack[self.name] if self.name == loader.pack_self: return loader - return loader.pack[self.name] + try: + return loader.pack[self.name] + except KeyError: + raise salt.exceptions.LoaderError( + f"LazyLoader does not have a packed value for: {self.name}" + ) def get(self, key, default=None): return self.value().get(key, default) diff --git a/tests/pytests/unit/loader/test_loader.py b/tests/pytests/unit/loader/test_loader.py index f4a4b51a58f..cb1a082b2a0 100644 --- a/tests/pytests/unit/loader/test_loader.py +++ b/tests/pytests/unit/loader/test_loader.py @@ -10,8 +10,10 @@ import textwrap import pytest +import salt.exceptions import salt.loader import salt.loader.lazy +import tests.support.helpers @pytest.fixture @@ -62,3 +64,25 @@ def test_raw_mod_functions(): ret = salt.loader.raw_mod(opts, "grains", "get") for k, v in ret.items(): assert isinstance(v, salt.loader.lazy.LoadedFunc) + + +def test_named_loader_context_name_not_packed(tmp_path): + opts = { + "optimization_order": [0], + } + (tmp_path / "mymod.py").write_text( + tests.support.helpers.dedent( + """ + from salt.loader.dunder import loader_context + __not_packed__ = loader_context.named_context("__not_packed__") + def foobar(): + return __not_packed__["not.packed"]() + """ + ) + ) + loader = salt.loader.LazyLoader([tmp_path], opts) + with pytest.raises( + salt.exceptions.LoaderError, + match="LazyLoader does not have a packed value for: __not_packed__", + ): + loader["mymod.foobar"]() diff --git a/tests/pytests/unit/loader/test_loading_modules.py b/tests/pytests/unit/loader/test_loading_modules.py new file mode 100644 index 00000000000..36ba7a4a06c --- /dev/null +++ b/tests/pytests/unit/loader/test_loading_modules.py @@ -0,0 +1,35 @@ +import pytest + +import salt.loader +import salt.loader.lazy + +import salt.modules.boto_vpc +import salt.modules.virt + + +@pytest.fixture +def minion_mods(minion_opts): + utils = salt.loader.utils(minion_opts) + return salt.loader.minion_mods(minion_opts, utils=utils) + + +@pytest.mark.skipIf(not salt.modules.boto_vpc.HAS_BOTO, "boto must be installed.") +def test_load_boto_vpc(minion_mods): + func = None + try: + func = minion_mods["boto_vpc.check_vpc"] + except KeyError: + pytest.fail("loader should not raise KeyError") + assert func is not None + assert isinstance(func, salt.loader.lazy.LoadedFunc) + + +@pytest.mark.skipIf(not salt.modules.virt.HAS_LIBVIRT, "libvirt-python must be installed.") +def test_load_virt(minion_mods): + func = None + try: + func = minion_mods["virt.ctrl_alt_del"] + except KeyError: + pytest.fail("loader should not raise KeyError") + assert func is not None + assert isinstance(func, salt.loader.lazy.LoadedFunc) From 57d046a7db3395f0c9cd4a93eb643c1f291f2833 Mon Sep 17 00:00:00 2001 From: "Daniel A. Wozniak" Date: Wed, 13 Dec 2023 17:18:19 -0700 Subject: [PATCH 03/13] Add changelog for #65691 --- changelog/65691.fixed.md | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog/65691.fixed.md diff --git a/changelog/65691.fixed.md b/changelog/65691.fixed.md new file mode 100644 index 00000000000..3b5192f80c0 --- /dev/null +++ b/changelog/65691.fixed.md @@ -0,0 +1 @@ +Fix boto execution module loading From 76343092f8f424f628e03995e7d439cbacf3c004 Mon Sep 17 00:00:00 2001 From: "Daniel A. Wozniak" Date: Thu, 14 Dec 2023 14:49:30 -0700 Subject: [PATCH 04/13] Proper test skip --- tests/pytests/unit/loader/test_loader.py | 20 ++++++++----------- .../unit/loader/test_loading_modules.py | 9 ++++++--- 2 files changed, 14 insertions(+), 15 deletions(-) diff --git a/tests/pytests/unit/loader/test_loader.py b/tests/pytests/unit/loader/test_loader.py index cb1a082b2a0..3c26b435c8c 100644 --- a/tests/pytests/unit/loader/test_loader.py +++ b/tests/pytests/unit/loader/test_loader.py @@ -13,7 +13,6 @@ import pytest import salt.exceptions import salt.loader import salt.loader.lazy -import tests.support.helpers @pytest.fixture @@ -70,19 +69,16 @@ def test_named_loader_context_name_not_packed(tmp_path): opts = { "optimization_order": [0], } - (tmp_path / "mymod.py").write_text( - tests.support.helpers.dedent( - """ + contents = """ from salt.loader.dunder import loader_context __not_packed__ = loader_context.named_context("__not_packed__") def foobar(): return __not_packed__["not.packed"]() """ - ) - ) - loader = salt.loader.LazyLoader([tmp_path], opts) - with pytest.raises( - salt.exceptions.LoaderError, - match="LazyLoader does not have a packed value for: __not_packed__", - ): - loader["mymod.foobar"]() + with pytest.helpers.temp_file("mymod.py", contents, directory=tmp_path): + loader = salt.loader.LazyLoader([tmp_path], opts) + with pytest.raises( + salt.exceptions.LoaderError, + match="LazyLoader does not have a packed value for: __not_packed__", + ): + loader["mymod.foobar"]() diff --git a/tests/pytests/unit/loader/test_loading_modules.py b/tests/pytests/unit/loader/test_loading_modules.py index 36ba7a4a06c..861e9197ecf 100644 --- a/tests/pytests/unit/loader/test_loading_modules.py +++ b/tests/pytests/unit/loader/test_loading_modules.py @@ -2,7 +2,6 @@ import pytest import salt.loader import salt.loader.lazy - import salt.modules.boto_vpc import salt.modules.virt @@ -13,7 +12,9 @@ def minion_mods(minion_opts): return salt.loader.minion_mods(minion_opts, utils=utils) -@pytest.mark.skipIf(not salt.modules.boto_vpc.HAS_BOTO, "boto must be installed.") +@pytest.mark.skipif( + not salt.modules.boto_vpc.HAS_BOTO, reason="boto must be installed." +) def test_load_boto_vpc(minion_mods): func = None try: @@ -24,7 +25,9 @@ def test_load_boto_vpc(minion_mods): assert isinstance(func, salt.loader.lazy.LoadedFunc) -@pytest.mark.skipIf(not salt.modules.virt.HAS_LIBVIRT, "libvirt-python must be installed.") +@pytest.mark.skipif( + not salt.modules.virt.HAS_LIBVIRT, reason="libvirt-python must be installed." +) def test_load_virt(minion_mods): func = None try: From 4e6624412d37be3fc32ae26ed656634b8e8a0fbd Mon Sep 17 00:00:00 2001 From: "Daniel A. Wozniak" Date: Mon, 18 Dec 2023 00:37:52 -0700 Subject: [PATCH 05/13] Fix vault tests --- salt/utils/vault.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/salt/utils/vault.py b/salt/utils/vault.py index cbd2aec2b0b..07fcf3929b1 100644 --- a/salt/utils/vault.py +++ b/salt/utils/vault.py @@ -36,8 +36,9 @@ def __virtual__(): logging.getLogger("requests").setLevel(logging.WARNING) return True except Exception as e: # pylint: disable=broad-except - log.error("Could not load __salt__: %s", e) + log.error("Could not load __salt__: %s", e, exc_info=True) return False + return True def _get_token_and_url_from_master(): From ea65abcd588deec7bb1dfc4b1f655f169866a709 Mon Sep 17 00:00:00 2001 From: "Daniel A. Wozniak" Date: Mon, 18 Dec 2023 17:26:51 -0700 Subject: [PATCH 06/13] Fix up test and close minion channels --- salt/minion.py | 18 +++++++++++++++++- .../integration/minion/test_return_retries.py | 4 ++-- 2 files changed, 19 insertions(+), 3 deletions(-) diff --git a/salt/minion.py b/salt/minion.py index 29afda23504..0c5c77a91e5 100644 --- a/salt/minion.py +++ b/salt/minion.py @@ -1286,6 +1286,7 @@ class Minion(MinionBase): self.ready = False self.jid_queue = [] if jid_queue is None else jid_queue self.periodic_callbacks = {} + self.req_channel = None if io_loop is None: self.io_loop = salt.ext.tornado.ioloop.IOLoop.current() @@ -1396,6 +1397,16 @@ class Minion(MinionBase): """ Return a future which will complete when you are connected to a master """ + if hasattr(self, "pub_channel") and self.pub_channel: + self.pub_channel.on_recv(None) + if hasattr(self.pub_channel, "auth"): + self.pub_channel.auth.invalidate() + if hasattr(self.pub_channel, "close"): + self.pub_channel.close() + if hasattr(self, "req_channel") and self.req_channel: + self.req_channel.close() + self.req_channel = None + # Consider refactoring so that eval_master does not have a subtle side-effect on the contents of the opts array master, self.pub_channel = yield self.eval_master( self.opts, self.timeout, self.safe, failed @@ -2870,7 +2881,9 @@ class Minion(MinionBase): self.pub_channel.auth.invalidate() if hasattr(self.pub_channel, "close"): self.pub_channel.close() - del self.pub_channel + if hasattr(self, "req_channel") and self.req_channel: + self.req_channel.close() + self.req_channel = None # if eval_master finds a new master for us, self.connected # will be True again on successful master authentication @@ -3303,6 +3316,9 @@ class Minion(MinionBase): if hasattr(self.pub_channel, "close"): self.pub_channel.close() del self.pub_channel + if hasattr(self, "req_channel") and self.req_channel: + self.req_channel.close() + self.req_channel = None if hasattr(self, "periodic_callbacks"): for cb in self.periodic_callbacks.values(): cb.stop() diff --git a/tests/pytests/integration/minion/test_return_retries.py b/tests/pytests/integration/minion/test_return_retries.py index d6456e4a47e..feefdfac6a1 100644 --- a/tests/pytests/integration/minion/test_return_retries.py +++ b/tests/pytests/integration/minion/test_return_retries.py @@ -28,7 +28,7 @@ def salt_minion_retry(salt_master, salt_minion_id): @pytest.mark.slow_test def test_publish_retry(salt_master, salt_minion_retry, salt_cli, salt_run_cli): # run job that takes some time for warmup - rtn = salt_cli.run("test.sleep", "5", "--async", minion_tgt=salt_minion_retry.id) + rtn = salt_cli.run("test.sleep", "3.5", "--async", minion_tgt=salt_minion_retry.id) # obtain JID jid = rtn.stdout.strip().split(" ")[-1] @@ -55,7 +55,7 @@ def test_publish_retry(salt_master, salt_minion_retry, salt_cli, salt_run_cli): @pytest.mark.slow_test def test_pillar_timeout(salt_master_factory): cmd = """ - python -c "import time; time.sleep(5); print('{\\"foo\\": \\"bar\\"}');\" + python -c "import time; time.sleep(4); print('{\\"foo\\": \\"bar\\"}');\" """.strip() master_overrides = { "ext_pillar": [ From d53fb4cade66f52c1be4ee9e1340d7d57d9698b8 Mon Sep 17 00:00:00 2001 From: "Daniel A. Wozniak" Date: Mon, 18 Dec 2023 22:44:40 -0700 Subject: [PATCH 07/13] Even more reliable pillar timeout test --- .../integration/minion/test_return_retries.py | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/tests/pytests/integration/minion/test_return_retries.py b/tests/pytests/integration/minion/test_return_retries.py index feefdfac6a1..f70609cdb1a 100644 --- a/tests/pytests/integration/minion/test_return_retries.py +++ b/tests/pytests/integration/minion/test_return_retries.py @@ -3,6 +3,8 @@ import time import pytest from saltfactories.utils import random_string +import salt.utils.files + @pytest.fixture(scope="function") def salt_minion_retry(salt_master, salt_minion_id): @@ -53,13 +55,15 @@ def test_publish_retry(salt_master, salt_minion_retry, salt_cli, salt_run_cli): @pytest.mark.slow_test -def test_pillar_timeout(salt_master_factory): - cmd = """ - python -c "import time; time.sleep(4); print('{\\"foo\\": \\"bar\\"}');\" - """.strip() +def test_pillar_timeout(salt_master_factory, tmp_path): + cmd = 'print(\'{"foo": "bar"}\');\n' + + with salt.utils.files.fopen(tmp_path / "script.py", "w") as fp: + fp.write(cmd) + master_overrides = { "ext_pillar": [ - {"cmd_json": cmd}, + {"cmd_json": f"python {tmp_path / 'script.py'}"}, ], "auto_accept": True, "worker_threads": 2, @@ -105,8 +109,10 @@ def test_pillar_timeout(salt_master_factory): "{}.sls".format(sls_name), sls_contents ) with master.started(), minion1.started(), minion2.started(), minion3.started(), minion4.started(), sls_tempfile: + cmd = 'import time; time.sleep(6); print(\'{"foo": "bang"}\');\n' + with salt.utils.files.fopen(tmp_path / "script.py", "w") as fp: + fp.write(cmd) proc = cli.run("state.sls", sls_name, minion_tgt="*") - print(proc) # At least one minion should have a Pillar timeout assert proc.returncode == 1 minion_timed_out = False From 76ec03ac64da5e52cbcd6c57df6990079ef6fcc2 Mon Sep 17 00:00:00 2001 From: "Daniel A. Wozniak" Date: Tue, 19 Dec 2023 10:20:33 -0700 Subject: [PATCH 08/13] Add explicit timeout for test_salt_documentation test --- tests/pytests/integration/cli/test_matcher.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/pytests/integration/cli/test_matcher.py b/tests/pytests/integration/cli/test_matcher.py index 2e91eba7ad4..5d3ef4e3309 100644 --- a/tests/pytests/integration/cli/test_matcher.py +++ b/tests/pytests/integration/cli/test_matcher.py @@ -506,7 +506,9 @@ def test_salt_documentation(salt_cli, salt_minion): """ Test to see if we're supporting --doc """ - ret = salt_cli.run("-d", "test", minion_tgt=salt_minion.id) + # Setting an explicity long timeout otherwise this test may fail when the + # system is under load. + ret = salt_cli.run("-d", "test", minion_tgt=salt_minion.id, _timeout=90) assert ret.returncode == 0 assert "test.ping" in ret.data From a57fb1c82e39272bcdff43d6bf81cd832f7716fe Mon Sep 17 00:00:00 2001 From: "Daniel A. Wozniak" Date: Thu, 21 Dec 2023 21:02:49 -0700 Subject: [PATCH 09/13] Increase beacon add timeout for more reliable test --- tests/pytests/scenarios/multimaster/beacons/test_inotify.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/pytests/scenarios/multimaster/beacons/test_inotify.py b/tests/pytests/scenarios/multimaster/beacons/test_inotify.py index c16384d7b6f..16babf1cf74 100644 --- a/tests/pytests/scenarios/multimaster/beacons/test_inotify.py +++ b/tests/pytests/scenarios/multimaster/beacons/test_inotify.py @@ -46,6 +46,7 @@ def setup_beacons(mm_master_1_salt_cli, salt_mm_minion_1, inotify_test_path): "inotify", beacon_data=[{"files": {str(inotify_test_path): {"mask": ["create"]}}}], minion_tgt=salt_mm_minion_1.id, + timeout=60, ) assert ret.returncode == 0 log.debug("Inotify beacon add returned: %s", ret.data or ret.stdout) From 048028466a8b6a29493d495140d7e09217c37d9a Mon Sep 17 00:00:00 2001 From: "Daniel A. Wozniak" Date: Thu, 21 Dec 2023 22:59:05 -0700 Subject: [PATCH 10/13] Do not add dunder salt outside of loader context --- salt/utils/azurearm.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/salt/utils/azurearm.py b/salt/utils/azurearm.py index 276cbb66b36..9ae128273cc 100644 --- a/salt/utils/azurearm.py +++ b/salt/utils/azurearm.py @@ -47,8 +47,6 @@ try: except ImportError: HAS_AZURE = False -__opts__ = salt.config.minion_config("/etc/salt/minion") -__salt__ = salt.loader.minion_mods(__opts__) log = logging.getLogger(__name__) From 4a8908cd11eb5200ce0dea1af129a1d6b3f95a0f Mon Sep 17 00:00:00 2001 From: David Murphy < dmurphy@saltstack.com> Date: Wed, 20 Dec 2023 10:14:37 -0700 Subject: [PATCH 11/13] Ensure initial _sync_grains only occurs if masterless minion in class SMinion initialization --- changelog/65692.fixed.md | 1 + salt/minion.py | 5 +++-- 2 files changed, 4 insertions(+), 2 deletions(-) create mode 100644 changelog/65692.fixed.md diff --git a/changelog/65692.fixed.md b/changelog/65692.fixed.md new file mode 100644 index 00000000000..8dc59a2f562 --- /dev/null +++ b/changelog/65692.fixed.md @@ -0,0 +1 @@ +Ensure initial _sync_grains only occurs if masterless minion in class SMinion initialization diff --git a/salt/minion.py b/salt/minion.py index 0c5c77a91e5..196e919e626 100644 --- a/salt/minion.py +++ b/salt/minion.py @@ -116,7 +116,8 @@ log = logging.getLogger(__name__) 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 + # if coming up initially and local client (masterless minion) + # if local client (masterless minion), 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 @@ -129,7 +130,7 @@ def _sync_grains(opts): if opts.get("extmod_blacklist", None) is None: opts["extmod_blacklist"] = {} - if opts.get("file_client", "remote") == "remote" and not opts.get( + if opts.get("file_client", "remote") == "local" and not opts.get( "master_uri", None ): salt.utils.extmods.sync(opts, "grains", force_local=True) From 7b0c0afdc376761a8858b946cbff747c1b690e52 Mon Sep 17 00:00:00 2001 From: David Murphy < dmurphy@saltstack.com> Date: Wed, 20 Dec 2023 13:34:09 -0700 Subject: [PATCH 12/13] Updated _sync_grains to only operate if masterless minion --- salt/minion.py | 28 +++++++++++----------------- 1 file changed, 11 insertions(+), 17 deletions(-) diff --git a/salt/minion.py b/salt/minion.py index 196e919e626..7c5bd5798b8 100644 --- a/salt/minion.py +++ b/salt/minion.py @@ -115,27 +115,21 @@ log = logging.getLogger(__name__) def _sync_grains(opts): - # need sync of custom grains as may be used in pillar compilation - # if coming up initially and local client (masterless minion) - # if local client (masterless minion), 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 + # if local client (masterless minion), need sync of custom grains + # as they may be used in pillar compilation + # in addition, with masterless minion some opts may not be filled + # at this point of syncing,for example 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 "local" == opts.get("file_client", "remote"): + 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("extmod_blacklist", None) is None: + opts["extmod_blacklist"] = {} - if opts.get("file_client", "remote") == "local" 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): From 70c22eff6262e247f26ff3ef1e69b833c7f777c3 Mon Sep 17 00:00:00 2001 From: David Murphy < dmurphy@saltstack.com> Date: Wed, 20 Dec 2023 14:52:50 -0700 Subject: [PATCH 13/13] Removed PR 65185 changes since incomplete solution --- changelog/65692.fixed.md | 2 +- salt/fileclient.py | 7 +- salt/minion.py | 19 ----- salt/utils/extmods.py | 5 +- .../pytests/integration/cli/test_salt_call.py | 71 ------------------- 5 files changed, 4 insertions(+), 100 deletions(-) diff --git a/changelog/65692.fixed.md b/changelog/65692.fixed.md index 8dc59a2f562..b4eef6c93d4 100644 --- a/changelog/65692.fixed.md +++ b/changelog/65692.fixed.md @@ -1 +1 @@ -Ensure initial _sync_grains only occurs if masterless minion in class SMinion initialization +Removed PR 65185 changes since incomplete solution diff --git a/salt/fileclient.py b/salt/fileclient.py index 42e7120aab1..443861cc03f 100644 --- a/salt/fileclient.py +++ b/salt/fileclient.py @@ -45,15 +45,12 @@ log = logging.getLogger(__name__) 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 7c5bd5798b8..15d46b2dacf 100644 --- a/salt/minion.py +++ b/salt/minion.py @@ -114,24 +114,6 @@ log = logging.getLogger(__name__) # 6. Handle publications -def _sync_grains(opts): - # if local client (masterless minion), need sync of custom grains - # as they may be used in pillar compilation - # in addition, with masterless minion some opts may not be filled - # at this point of syncing,for example 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 "local" == opts.get("file_client", "remote"): - if opts.get("extmod_whitelist", None) is None: - opts["extmod_whitelist"] = {} - - if opts.get("extmod_blacklist", None) is None: - opts["extmod_blacklist"] = {} - - salt.utils.extmods.sync(opts, "grains", force_local=True) - - def resolve_dns(opts, fallback=True): """ Resolves the master_ip and master_uri options @@ -939,7 +921,6 @@ class SMinion(MinionBase): # 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 6a4d5c14440..a7dc265476f 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 = [] diff --git a/tests/pytests/integration/cli/test_salt_call.py b/tests/pytests/integration/cli/test_salt_call.py index b1af43050e1..6aa6730ec5c 100644 --- a/tests/pytests/integration/cli/test_salt_call.py +++ b/tests/pytests/integration/cli/test_salt_call.py @@ -429,74 +429,3 @@ def test_local_salt_call_no_function_no_retcode(salt_call_cli): assert "test" in ret.data assert ret.data["test"] == "'test' is not available." assert "test.echo" in ret.data - - -def test_state_highstate_custom_grains(salt_master, salt_minion_factory): - """ - This test ensure that custom grains in salt://_grains are loaded before pillar compilation - to ensure that any use of custom grains in pillar files are available, this implies that - a sync of grains occurs before loading the regular /etc/salt/grains or configuration file - grains, as well as the usual grains. - - Note: cannot use salt_minion and salt_call_cli, since these will be loaded before - the pillar and custom_grains files are written, hence using salt_minion_factory. - """ - pillar_top_sls = """ - base: - '*': - - defaults - """ - - pillar_defaults_sls = """ - mypillar: "{{ grains['custom_grain'] }}" - """ - - salt_top_sls = """ - base: - '*': - - test - """ - - salt_test_sls = """ - "donothing": - test.nop: [] - """ - - salt_custom_grains_py = """ - def main(): - return {'custom_grain': 'test_value'} - """ - assert salt_master.is_running() - with salt_minion_factory.started(): - salt_minion = salt_minion_factory - salt_call_cli = salt_minion_factory.salt_call_cli() - with salt_minion.pillar_tree.base.temp_file( - "top.sls", pillar_top_sls - ), salt_minion.pillar_tree.base.temp_file( - "defaults.sls", pillar_defaults_sls - ), salt_minion.state_tree.base.temp_file( - "top.sls", salt_top_sls - ), salt_minion.state_tree.base.temp_file( - "test.sls", salt_test_sls - ), salt_minion.state_tree.base.temp_file( - "_grains/custom_grain.py", salt_custom_grains_py - ): - ret = salt_call_cli.run("--local", "state.highstate") - assert ret.returncode == 0 - ret = salt_call_cli.run("--local", "pillar.items") - assert ret.returncode == 0 - assert ret.data - pillar_items = ret.data - assert "mypillar" in pillar_items - assert pillar_items["mypillar"] == "test_value" - - -def test_salt_call_versions(salt_call_cli, caplog): - """ - Call test.versions without '--local' to test grains - are sync'd without any missing keys in opts - """ - with caplog.at_level(logging.DEBUG): - ret = salt_call_cli.run("test.versions") - assert ret.returncode == 0 - assert "Failed to sync grains module: 'master_uri'" not in caplog.messages