Merge 3006.x into 3007.x

changelog/65727.fixed.md

@@ -0,0 +1 @@
+catch only ret/ events not all returning events.

changelog/65932.fixed.md

@@ -0,0 +1 @@
+Ensure minion channels are closed on any master connection error.

changelog/65987.fixed.md

@@ -0,0 +1 @@
+Fix RPM package systemd scriptlets to make RPM packages more universal

changelog/66061.fixed.md

@@ -0,0 +1,2 @@
+Fix request channel default timeout regression. In 3006.5 it was changed from
+60 to 30 and is now set back to 60 by default.

pkg/rpm/salt.spec

@@ -432,17 +432,33 @@ find /etc/salt /opt/saltstack/salt /var/log/salt /var/cache/salt /var/run/salt \
 
 
 # assumes systemd for RHEL 7 & 8 & 9
+# foregoing %systemd_* scriptlets due to RHEL 7/8 vs. RHEL 9 incompatibilities
+## - Using hardcoded scriptlet definitions from RHEL 7/8 that are forward-compatible
 %preun master
 # RHEL 9 is giving warning msg if syndic is not installed, supress it
-%systemd_preun salt-syndic.service > /dev/null 2>&1
-
+# %%systemd_preun salt-syndic.service > /dev/null 2>&1
+if [ $1 -eq 0 ] ; then
+  # Package removal, not upgrade
+  systemctl --no-reload disable salt-syndic.service > /dev/null 2>&1 || :
+  systemctl stop salt-syndic.service > /dev/null 2>&1 || :
+fi
 
 %preun minion
-%systemd_preun salt-minion.service
+# %%systemd_preun salt-minion.service
+if [ $1 -eq 0 ] ; then
+  # Package removal, not upgrade
+  systemctl --no-reload disable salt-minion.service > /dev/null 2>&1 || :
+  systemctl stop salt-minion.service > /dev/null 2>&1 || :
+fi
 
 
 %preun api
-%systemd_preun salt-api.service
+# %%systemd_preun salt-api.service
+if [ $1 -eq 0 ] ; then
+  # Package removal, not upgrade
+  systemctl --no-reload disable salt-api.service > /dev/null 2>&1 || :
+  systemctl stop salt-api.service > /dev/null 2>&1 || :
+fi
 
 
 %post
@@ -456,7 +472,14 @@ ln -s -f /opt/saltstack/salt/salt-cloud %{_bindir}/salt-cloud
 
 
 %post master
-%systemd_post salt-master.service
+# %%systemd_post salt-master.service
+if [ $1 -gt 1 ] ; then
+  # Upgrade
+  systemctl retry-restart salt-master.service >/dev/null 2>&1 || :
+else
+  # Initial installation
+  systemctl preset salt-master.service >/dev/null 2>&1 || :
+fi
 ln -s -f /opt/saltstack/salt/salt %{_bindir}/salt
 ln -s -f /opt/saltstack/salt/salt-cp %{_bindir}/salt-cp
 ln -s -f /opt/saltstack/salt/salt-key %{_bindir}/salt-key
@@ -477,11 +500,25 @@ if [ $1 -lt 2 ]; then
 fi
 
 %post syndic
-%systemd_post salt-syndic.service
+# %%systemd_post salt-syndic.service
+if [ $1 -gt 1 ] ; then
+  # Upgrade
+  systemctl retry-restart salt-syndic.service >/dev/null 2>&1 || :
+else
+  # Initial installation
+  systemctl preset salt-syndic.service >/dev/null 2>&1 || :
+fi
 ln -s -f /opt/saltstack/salt/salt-syndic %{_bindir}/salt-syndic
 
 %post minion
-%systemd_post salt-minion.service
+# %%systemd_post salt-minion.service
+if [ $1 -gt 1 ] ; then
+  # Upgrade
+  systemctl retry-restart salt-minion.service >/dev/null 2>&1 || :
+else
+  # Initial installation
+  systemctl preset salt-minion.service >/dev/null 2>&1 || :
+fi
 ln -s -f /opt/saltstack/salt/salt-minion %{_bindir}/salt-minion
 ln -s -f /opt/saltstack/salt/salt-call %{_bindir}/salt-call
 ln -s -f /opt/saltstack/salt/salt-proxy %{_bindir}/salt-proxy
@@ -503,7 +540,14 @@ fi
 ln -s -f /opt/saltstack/salt/salt-ssh %{_bindir}/salt-ssh
 
 %post api
-%systemd_post salt-api.service
+# %%systemd_post salt-api.service
+if [ $1 -gt 1 ] ; then
+  # Upgrade
+  systemctl retry-restart salt-api.service >/dev/null 2>&1 || :
+else
+  # Initial installation
+  systemctl preset salt-api.service >/dev/null 2>&1 || :
+fi
 ln -s -f /opt/saltstack/salt/salt-api %{_bindir}/salt-api
 
 
@@ -544,7 +588,12 @@ if [ $1 -eq 0 ]; then
 fi
 
 %postun master
-%systemd_postun_with_restart salt-master.service
+# %%systemd_postun_with_restart salt-master.service
+systemctl daemon-reload >/dev/null 2>&1 || :
+if [ $1 -ge 1 ] ; then
+  # Package upgrade, not uninstall
+  systemctl try-restart salt-master.service >/dev/null 2>&1 || :
+fi
 if [ $1 -eq 0 ]; then
   if [ $(cat /etc/os-release | grep VERSION_ID | cut -d '=' -f 2 | sed  's/\"//g' | cut -d '.' -f 1) = "8" ]; then
     if [ -z "$(rpm -qi salt-minion | grep Name | grep salt-minion)" ]; then
@@ -560,10 +609,20 @@ if [ $1 -eq 0 ]; then
 fi
 
 %postun syndic
-%systemd_postun_with_restart salt-syndic.service
+# %%systemd_postun_with_restart salt-syndic.service
+systemctl daemon-reload >/dev/null 2>&1 || :
+if [ $1 -ge 1 ] ; then
+  # Package upgrade, not uninstall
+  systemctl try-restart salt-syndic.service >/dev/null 2>&1 || :
+fi
 
 %postun minion
-%systemd_postun_with_restart salt-minion.service
+# %%systemd_postun_with_restart salt-minion.service
+systemctl daemon-reload >/dev/null 2>&1 || :
+if [ $1 -ge 1 ] ; then
+  # Package upgrade, not uninstall
+  systemctl try-restart salt-minion.service >/dev/null 2>&1 || :
+fi
 if [ $1 -eq 0 ]; then
   if [ $(cat /etc/os-release | grep VERSION_ID | cut -d '=' -f 2 | sed  's/\"//g' | cut -d '.' -f 1) = "8" ]; then
     if [ -z "$(rpm -qi salt-master | grep Name | grep salt-master)" ]; then
@@ -579,8 +638,12 @@ if [ $1 -eq 0 ]; then
 fi
 
 %postun api
-%systemd_postun_with_restart salt-api.service
-
+# %%systemd_postun_with_restart salt-api.service
+systemctl daemon-reload >/dev/null 2>&1 || :
+if [ $1 -ge 1 ] ; then
+  # Package upgrade, not uninstall
+  systemctl try-restart salt-api.service >/dev/null 2>&1 || :
+fi
 
 %changelog
 * Fri Jan 26 2024 Salt Project Packaging <saltproject-packaging@vmware.com> - 3006.6

salt/client/__init__.py

@@ -1203,7 +1203,7 @@ class LocalClient:
                     continue
 
                 # Anything below this point is expected to be a job return event.
-                if not raw["tag"].startswith(f"salt/job/{jid}/ret"):
+                if not raw["tag"].startswith(f"salt/job/{jid}/ret/"):
                     log.debug("Skipping non return event: %s", raw["tag"])
                     continue
                 if "return" not in raw["data"]:

salt/config/__init__.py

@@ -1066,7 +1066,7 @@ DEFAULT_MINION_OPTS = immutabletypes.freeze(
         "pillar_cache": False,
         "pillar_cache_ttl": 3600,
         "pillar_cache_backend": "disk",
-        "request_channel_timeout": 30,
+        "request_channel_timeout": 60,
         "request_channel_tries": 3,
         "gpg_cache": False,
         "gpg_cache_ttl": 86400,

salt/modules/state.py

@@ -637,7 +637,8 @@ def apply_(mods=None, **kwargs):
 
     .. rubric:: APPLYING ALL STATES CONFIGURED IN TOP.SLS (A.K.A. :ref:`HIGHSTATE <running-highstate>`)
 
-    To apply all configured states, simply run ``state.apply``:
+    To apply all configured states, simply run ``state.apply`` with no SLS
+    targets, like so:
 
     .. code-block:: bash
 

tests/integration/modules/test_decorators.py

@@ -4,6 +4,7 @@ from tests.support.case import ModuleCase
 
 
 @pytest.mark.windows_whitelisted
+@pytest.mark.timeout_unless_on_windows(120)
 class DecoratorTest(ModuleCase):
     @pytest.mark.slow_test
     def test_module(self):

tests/integration/modules/test_saltcheck.py

@@ -7,6 +7,7 @@ import pytest
 from tests.support.case import ModuleCase
 
 
+@pytest.mark.timeout_unless_on_windows(120)
 class SaltcheckModuleTest(ModuleCase):
     """
     Test the saltcheck module

tests/integration/ssh/test_state.py

@@ -18,6 +18,7 @@ log = logging.getLogger(__name__)
 
 
 @pytest.mark.slow_test
+@pytest.mark.timeout_unless_on_windows(120)
 class SSHStateTest(SSHCase):
     """
     testing the state system with salt-ssh

tests/pytests/functional/cli/test_batch.py

@@ -140,7 +140,7 @@ class MockSubscriber:
             },
             use_bin_type=True,
         )
-        tag = f"salt/job/{jid}/ret".encode()
+        tag = f"salt/job/{jid}/ret/{minion_id}".encode()
         return b"".join([tag, b"\n\n", dumped])
 
     def connect(self, timeout=None):

tests/pytests/functional/modules/test_ansiblegate.py

@@ -10,6 +10,7 @@ pytestmark = [
         reason="ansible is not installed",
     ),
     pytest.mark.slow_test,
+    pytest.mark.timeout_unless_on_windows(120),
 ]
 
 

tests/pytests/integration/cli/test_matcher.py

@@ -7,6 +7,7 @@ import salt.defaults.exitcodes
 pytestmark = [
     pytest.mark.core_test,
     pytest.mark.windows_whitelisted,
+    pytest.mark.timeout_unless_on_windows(120),
 ]
 
 

tests/pytests/integration/cli/test_salt_deltaproxy.py

@@ -19,6 +19,7 @@ pytestmark = [
         reason="Deltaproxy minions do not currently work on spawning platforms.",
     ),
     pytest.mark.core_test,
+    pytest.mark.timeout_unless_on_windows(320),
 ]
 
 

tests/pytests/pkg/downgrade/test_salt_downgrade.py

@@ -1,4 +1,5 @@
 import packaging.version
+import psutil
 import pytest
 from pytestskipmarkers.utils import platform
 
@@ -34,6 +35,19 @@ def test_salt_downgrade(salt_call_cli, install_salt):
     use_lib = salt_call_cli.run("--local", "github.get_repo_info", repo)
     assert "Authentication information could" in use_lib.stderr
 
+    # Verify there is a running minion by getting its PID
+    if platform.is_windows():
+        process_name = "salt-minion.exe"
+    else:
+        process_name = "salt-minion"
+    old_pid = None
+    for proc in psutil.process_iter():
+        if process_name in proc.name():
+            if psutil.Process(proc.ppid()).name() != process_name:
+                old_pid = proc.pid
+                break
+    assert old_pid is not None
+
     # Downgrade Salt to the previous version and test
     install_salt.install(downgrade=True)
     bin_file = "salt"
@@ -45,6 +59,17 @@ def test_salt_downgrade(salt_call_cli, install_salt):
     elif platform.is_darwin() and install_salt.classic:
         bin_file = install_salt.bin_dir / "salt-call"
 
+    # Verify there is a new running minion by getting its PID and comparing it
+    # with the PID from before the upgrade
+    new_pid = None
+    for proc in psutil.process_iter():
+        if process_name in proc.name():
+            if psutil.Process(proc.ppid()).name() != process_name:
+                new_pid = proc.pid
+                break
+    assert new_pid is not None
+    assert new_pid != old_pid
+
     ret = install_salt.proc.run(bin_file, "--version")
     assert ret.returncode == 0
     assert packaging.version.parse(

tests/pytests/pkg/integration/test_enabled_disabled.py

@@ -7,24 +7,14 @@ def test_services(install_salt, salt_cli, salt_minion):
     """
     Check if Services are enabled/disabled
     """
+    services_disabled = []
+    services_enabled = []
     if install_salt.distro_id in ("ubuntu", "debian"):
         services_enabled = ["salt-master", "salt-minion", "salt-syndic", "salt-api"]
-        services_disabled = []
     elif install_salt.distro_id in ("centos", "redhat", "amzn", "fedora"):
-        services_enabled = []
         services_disabled = ["salt-master", "salt-minion", "salt-syndic", "salt-api"]
     elif install_salt.distro_id == "photon":
-        if float(install_salt.distro_version) < 5:
-            services_enabled = []
-            services_disabled = [
-                "salt-master",
-                "salt-minion",
-                "salt-syndic",
-                "salt-api",
-            ]
-        else:
-            services_enabled = ["salt-master", "salt-minion", "salt-syndic", "salt-api"]
-            services_disabled = []
+        services_enabled = ["salt-master", "salt-minion", "salt-syndic", "salt-api"]
     elif platform.is_darwin():
         services_enabled = ["salt-minion"]
         services_disabled = []

tests/pytests/pkg/upgrade/test_salt_upgrade.py

@@ -1,5 +1,7 @@
 import packaging.version
+import psutil
 import pytest
+from pytestskipmarkers.utils import platform
 
 
 def test_salt_upgrade(salt_call_cli, install_salt):
@@ -29,6 +31,19 @@ def test_salt_upgrade(salt_call_cli, install_salt):
     use_lib = salt_call_cli.run("--local", "github.get_repo_info", repo)
     assert "Authentication information could" in use_lib.stderr
 
+    # Verify there is a running minion by getting its PID
+    if platform.is_windows():
+        process_name = "salt-minion.exe"
+    else:
+        process_name = "salt-minion"
+    old_pid = None
+    for proc in psutil.process_iter():
+        if process_name in proc.name():
+            if psutil.Process(proc.ppid()).name() != process_name:
+                old_pid = proc.pid
+                break
+    assert old_pid is not None
+
     # Upgrade Salt from previous version and test
     install_salt.install(upgrade=True)
     ret = salt_call_cli.run("test.version")
@@ -37,8 +52,20 @@ def test_salt_upgrade(salt_call_cli, install_salt):
         install_salt.artifact_version
     )
 
+    # Verify there is a new running minion by getting its PID and comparing it
+    # with the PID from before the upgrade
+    new_pid = None
+    for proc in psutil.process_iter():
+        if process_name in proc.name():
+            if psutil.Process(proc.ppid()).name() != process_name:
+                new_pid = proc.pid
+                break
+    assert new_pid is not None
+    assert new_pid != old_pid
+
     if install_salt.relenv:
         new_py_version = install_salt.package_python_version()
         if new_py_version == original_py_version:
             # test pip install after an upgrade
             use_lib = salt_call_cli.run("--local", "github.get_repo_info", repo)
+            assert "Authentication information could" in use_lib.stderr

tests/pytests/unit/test_client.py

@@ -247,3 +247,44 @@ def test_pub_win32(salt_master_factory):
                 "test.ping",
                 tgt_type="nodegroup",
             )
+
+
+def test_invalid_event_tag_65727(master_opts, caplog):
+    """
+    LocalClient.get_iter_returns handles non return event tags.
+    """
+    minions = ()
+    jid = "0815"
+    raw_return = {"id": "fake-id", "jid": jid, "data": "", "return": "fake-return"}
+    expected_return = {"fake-id": {"ret": "fake-return"}}
+
+    def returns_iter():
+        # Invalid return
+        yield {
+            "tag": "salt/job/0815/return/",
+            "data": {
+                "return": "fpp",
+                "id": "fake-id",
+            },
+        }
+        # Valid return
+        yield {
+            "tag": "salt/job/0815/ret/",
+            "data": {
+                "return": "fpp",
+                "id": "fake-id",
+            },
+        }
+
+    with client.LocalClient(mopts=master_opts) as local_client:
+        # Returning a truthy value, the real method returns a salt returner but it's not used.
+        local_client.returns_for_job = MagicMock(return_value=True)
+        # Mock iter returns, we'll return one invalid and one valid return event.
+        local_client.get_returns_no_block = MagicMock(return_value=returns_iter())
+        with caplog.at_level(logging.DEBUG):
+            # Validate we don't choke on the bad return, the method returns a
+            # valid respons and the invalid event tag is getting logged to
+            # debug.
+            for ret in local_client.get_iter_returns(jid, {"fake-id"}):
+                assert ret == {"fake-id": {"ret": "fpp"}}
+            assert "Skipping non return event: salt/job/0815/return/" in caplog.text