Sign Up
Log In
Log In
or
Sign Up
Places
All Projects
Status Monitor
Collapse sidebar
openSUSE:Step:FrontRunner
salt.24747
state.apply-don-t-check-for-cached-pillar-error...
Overview
Repositories
Revisions
Requests
Users
Attributes
Meta
File state.apply-don-t-check-for-cached-pillar-errors.patch of Package salt.24747
From 5880703551d82a68a0e2f3108878124d8ae98bf0 Mon Sep 17 00:00:00 2001 From: Alexander Graul <agraul@suse.com> Date: Tue, 25 Jan 2022 17:20:55 +0100 Subject: [PATCH] state.apply: don't check for cached pillar errors state.apply request new pillar data from the server. This done to always have the most up-to-date pillar to work with. Previously, checking for pillar errors looked at both the new pillar and the in-memory pillar. The latter might contain pillar rendering errors even if the former does not. For this reason, only the new pillar should be checked, not both. --- changelog/52354.fixed | 1 + changelog/57180.fixed | 1 + changelog/59339.fixed | 1 + salt/modules/state.py | 17 ++- .../modules/state/test_state_pillar_errors.py | 131 ++++++++++++++++++ .../pytests/unit/modules/state/test_state.py | 115 +++++---------- 6 files changed, 177 insertions(+), 89 deletions(-) create mode 100644 changelog/52354.fixed create mode 100644 changelog/57180.fixed create mode 100644 changelog/59339.fixed create mode 100644 tests/pytests/integration/modules/state/test_state_pillar_errors.py diff --git a/changelog/52354.fixed b/changelog/52354.fixed new file mode 100644 index 0000000000..af885d77fa --- /dev/null +++ b/changelog/52354.fixed @@ -0,0 +1 @@ +Don't check for cached pillar errors on state.apply diff --git a/changelog/57180.fixed b/changelog/57180.fixed new file mode 100644 index 0000000000..af885d77fa --- /dev/null +++ b/changelog/57180.fixed @@ -0,0 +1 @@ +Don't check for cached pillar errors on state.apply diff --git a/changelog/59339.fixed b/changelog/59339.fixed new file mode 100644 index 0000000000..af885d77fa --- /dev/null +++ b/changelog/59339.fixed @@ -0,0 +1 @@ +Don't check for cached pillar errors on state.apply diff --git a/salt/modules/state.py b/salt/modules/state.py index 0c3dfc3317..0027744229 100644 --- a/salt/modules/state.py +++ b/salt/modules/state.py @@ -106,18 +106,17 @@ def _set_retcode(ret, highstate=None): def _get_pillar_errors(kwargs, pillar=None): """ - Checks all pillars (external and internal) for errors. - Return an error message, if anywhere or None. + Check pillar for errors. + + If a pillar is passed, it will be checked. Otherwise, the in-memory pillar + will checked instead. Passing kwargs['force'] = True short cuts the check + and always returns None, indicating no errors. :param kwargs: dictionary of options - :param pillar: external pillar - :return: None or an error message + :param pillar: pillar + :return: None or a list of error messages """ - return ( - None - if kwargs.get("force") - else (pillar or {}).get("_errors", __pillar__.get("_errors")) or None - ) + return None if kwargs.get("force") else (pillar or __pillar__).get("_errors") def _wait(jid): diff --git a/tests/pytests/integration/modules/state/test_state_pillar_errors.py b/tests/pytests/integration/modules/state/test_state_pillar_errors.py new file mode 100644 index 0000000000..af65a05945 --- /dev/null +++ b/tests/pytests/integration/modules/state/test_state_pillar_errors.py @@ -0,0 +1,131 @@ +#!/usr/bin/python3 + +import textwrap + +import pytest +from saltfactories.utils.functional import StateResult + +pytestmark = [ + pytest.mark.slow_test, +] + + +@pytest.fixture(scope="module") +def reset_pillar(salt_call_cli): + try: + # Run tests + yield + finally: + # Refresh pillar once all tests are done. + ret = salt_call_cli.run("saltutil.refresh_pillar", wait=True) + assert ret.exitcode == 0 + assert ret.json is True + + +@pytest.fixture +def testfile_path(tmp_path, base_env_state_tree_root_dir): + testfile = tmp_path / "testfile" + sls_contents = textwrap.dedent( + """ + {}: + file: + - managed + - source: salt://testfile + - makedirs: true + """.format(testfile) + ) + with pytest.helpers.temp_file( + "sls-id-test.sls", sls_contents, base_env_state_tree_root_dir + ): + yield testfile + + +@pytest.mark.usefixtures("testfile_path", "reset_pillar") +def test_state_apply_aborts_on_pillar_error( + salt_cli, + salt_minion, + base_env_pillar_tree_root_dir, +): + """ + Test state.apply with error in pillar. + """ + pillar_top_file = textwrap.dedent( + """ + base: + '{}': + - basic + """ + ).format(salt_minion.id) + basic_pillar_file = textwrap.dedent( + """ + syntax_error + """ + ) + + with pytest.helpers.temp_file( + "top.sls", pillar_top_file, base_env_pillar_tree_root_dir + ), pytest.helpers.temp_file( + "basic.sls", basic_pillar_file, base_env_pillar_tree_root_dir + ): + expected_comment = [ + "Pillar failed to render with the following messages:", + "SLS 'basic' does not render to a dictionary", + ] + shell_result = salt_cli.run( + "state.apply", "sls-id-test", minion_tgt=salt_minion.id + ) + assert shell_result.exitcode == 1 + assert shell_result.json == expected_comment + + +@pytest.mark.usefixtures("testfile_path", "reset_pillar") +def test_state_apply_continues_after_pillar_error_is_fixed( + salt_cli, + salt_minion, + base_env_pillar_tree_root_dir, +): + """ + Test state.apply with error in pillar. + """ + pillar_top_file = textwrap.dedent( + """ + base: + '{}': + - basic + """.format(salt_minion.id) + ) + basic_pillar_file_error = textwrap.dedent( + """ + syntax_error + """ + ) + basic_pillar_file = textwrap.dedent( + """ + syntax_error: Fixed! + """ + ) + + # save pillar render error in minion's in-memory pillar + with pytest.helpers.temp_file( + "top.sls", pillar_top_file, base_env_pillar_tree_root_dir + ), pytest.helpers.temp_file( + "basic.sls", basic_pillar_file_error, base_env_pillar_tree_root_dir + ): + shell_result = salt_cli.run( + "saltutil.refresh_pillar", minion_tgt=salt_minion.id + ) + assert shell_result.exitcode == 0 + + # run state.apply with fixed pillar render error + with pytest.helpers.temp_file( + "top.sls", pillar_top_file, base_env_pillar_tree_root_dir + ), pytest.helpers.temp_file( + "basic.sls", basic_pillar_file, base_env_pillar_tree_root_dir + ): + shell_result = salt_cli.run( + "state.apply", "sls-id-test", minion_tgt=salt_minion.id + ) + assert shell_result.exitcode == 0 + state_result = StateResult(shell_result.json) + assert state_result.result is True + assert state_result.changes == {"diff": "New file", "mode": "0644"} diff --git a/tests/pytests/unit/modules/state/test_state.py b/tests/pytests/unit/modules/state/test_state.py index 02fd2dd307..30cda303cc 100644 --- a/tests/pytests/unit/modules/state/test_state.py +++ b/tests/pytests/unit/modules/state/test_state.py @@ -1,14 +1,16 @@ """ :codeauthor: Rahul Handay <rahulha@saltstack.com> """ - import datetime import logging import os +from collections import namedtuple import pytest + import salt.config import salt.loader +import salt.loader.context import salt.modules.config as config import salt.modules.state as state import salt.state @@ -1200,85 +1202,6 @@ def test_lock_saltenv(): ) -def test_get_pillar_errors_CC(): - """ - Test _get_pillar_errors function. - CC: External clean, Internal clean - :return: - """ - for int_pillar, ext_pillar in [ - ({"foo": "bar"}, {"fred": "baz"}), - ({"foo": "bar"}, None), - ({}, {"fred": "baz"}), - ]: - with patch("salt.modules.state.__pillar__", int_pillar): - for opts, res in [ - ({"force": True}, None), - ({"force": False}, None), - ({}, None), - ]: - assert res == state._get_pillar_errors(kwargs=opts, pillar=ext_pillar) - - -def test_get_pillar_errors_EC(): - """ - Test _get_pillar_errors function. - EC: External erroneous, Internal clean - :return: - """ - errors = ["failure", "everywhere"] - for int_pillar, ext_pillar in [ - ({"foo": "bar"}, {"fred": "baz", "_errors": errors}), - ({}, {"fred": "baz", "_errors": errors}), - ]: - with patch("salt.modules.state.__pillar__", int_pillar): - for opts, res in [ - ({"force": True}, None), - ({"force": False}, errors), - ({}, errors), - ]: - assert res == state._get_pillar_errors(kwargs=opts, pillar=ext_pillar) - - -def test_get_pillar_errors_EE(): - """ - Test _get_pillar_errors function. - CC: External erroneous, Internal erroneous - :return: - """ - errors = ["failure", "everywhere"] - for int_pillar, ext_pillar in [ - ({"foo": "bar", "_errors": errors}, {"fred": "baz", "_errors": errors}) - ]: - with patch("salt.modules.state.__pillar__", int_pillar): - for opts, res in [ - ({"force": True}, None), - ({"force": False}, errors), - ({}, errors), - ]: - assert res == state._get_pillar_errors(kwargs=opts, pillar=ext_pillar) - - -def test_get_pillar_errors_CE(): - """ - Test _get_pillar_errors function. - CC: External clean, Internal erroneous - :return: - """ - errors = ["failure", "everywhere"] - for int_pillar, ext_pillar in [ - ({"foo": "bar", "_errors": errors}, {"fred": "baz"}), - ({"foo": "bar", "_errors": errors}, None), - ]: - with patch("salt.modules.state.__pillar__", int_pillar): - for opts, res in [ - ({"force": True}, None), - ({"force": False}, errors), - ({}, errors), - ]: - assert res == state._get_pillar_errors(kwargs=opts, pillar=ext_pillar) - - def test_event(): """ test state.event runner @@ -1318,3 +1241,35 @@ def test_event(): if _expected in x.args[0]: found = True assert found is True + + +PillarPair = namedtuple("PillarPair", ["in_memory", "fresh"]) +pillar_combinations = [ + (PillarPair({"foo": "bar"}, {"fred": "baz"}), None), + (PillarPair({"foo": "bar"}, {"fred": "baz", "_errors": ["Failure"]}), ["Failure"]), + (PillarPair({"foo": "bar"}, None), None), + (PillarPair({"foo": "bar", "_errors": ["Failure"]}, None), ["Failure"]), + (PillarPair({"foo": "bar", "_errors": ["Failure"]}, {"fred": "baz"}), None), +] + + +@pytest.mark.parametrize("pillar,expected_errors", pillar_combinations) +def test_get_pillar_errors(pillar: PillarPair, expected_errors): + """ + test _get_pillar_errors function + + There are three cases to consider: + 1. kwargs['force'] is True -> None, no matter what's in pillar/__pillar__ + 2. pillar kwarg is available -> only check pillar, no matter what's in __pillar__ + 3. pillar kwarg is not available -> check __pillar__ + """ + ctx = salt.loader.context.LoaderContext() + named_ctx = ctx.named_context("__pillar__", pillar.in_memory) + with patch("salt.modules.state.__pillar__", named_ctx, create=True): + assert ( + state._get_pillar_errors(kwargs={"force": True}, pillar=pillar.fresh) + is None + ) + assert ( + state._get_pillar_errors(kwargs={}, pillar=pillar.fresh) == expected_errors + ) -- 2.34.1
Locations
Projects
Search
Status Monitor
Help
OpenBuildService.org
Documentation
API Documentation
Code of Conduct
Contact
Support
@OBShq
Terms
openSUSE Build Service is sponsored by
The Open Build Service is an
openSUSE project
.
Sign Up
Log In
Places
Places
All Projects
Status Monitor
