From ca499e2bfef7253d3efa8e38f50d92182def248c Mon Sep 17 00:00:00 2001 From: Witek Bedyk Date: Mon, 29 Aug 2022 14:16:00 +0200 Subject: [PATCH] Retry if RPM lock is temporarily unavailable (#547) * Retry if RPM lock is temporarily unavailable Backported from saltstack/salt#62204 Signed-off-by: Witek Bedyk * Sync formating fixes from upstream Signed-off-by: Witek Bedyk --- changelog/62204.fixed | 1 + salt/modules/zypperpkg.py | 117 +++++++++++++++++---------- tests/unit/modules/test_zypperpkg.py | 45 ++++++++++- 3 files changed, 115 insertions(+), 48 deletions(-) create mode 100644 changelog/62204.fixed diff --git a/changelog/62204.fixed b/changelog/62204.fixed new file mode 100644 index 0000000000..59f1914593 --- /dev/null +++ b/changelog/62204.fixed @@ -0,0 +1 @@ +Fixed Zypper module failing on RPM lock file being temporarily unavailable. diff --git a/salt/modules/zypperpkg.py b/salt/modules/zypperpkg.py index 2c36e2968a..c787d4009d 100644 --- a/salt/modules/zypperpkg.py +++ b/salt/modules/zypperpkg.py @@ -14,6 +14,7 @@ import configparser import datetime +import errno import fnmatch import logging import os @@ -39,6 +40,9 @@ # pylint: disable=import-error,redefined-builtin,no-name-in-module from salt.utils.versions import LooseVersion +if salt.utils.files.is_fcntl_available(): + import fcntl + log = logging.getLogger(__name__) HAS_ZYPP = False @@ -106,6 +110,7 @@ class _Zypper: XML_DIRECTIVES = ["-x", "--xmlout"] # ZYPPER_LOCK is not affected by --root ZYPPER_LOCK = "/var/run/zypp.pid" + RPM_LOCK = "/var/lib/rpm/.rpm.lock" TAG_RELEASED = "zypper/released" TAG_BLOCKED = "zypper/blocked" @@ -276,7 +281,7 @@ def _is_error(self): and self.exit_code not in self.WARNING_EXIT_CODES ) - def _is_lock(self): + def _is_zypper_lock(self): """ Is this is a lock error code? @@ -284,6 +289,23 @@ def _is_lock(self): """ return self.exit_code == self.LOCK_EXIT_CODE + def _is_rpm_lock(self): + """ + Is this an RPM lock error? + """ + if salt.utils.files.is_fcntl_available(): + if self.exit_code > 0 and os.path.exists(self.RPM_LOCK): + with salt.utils.files.fopen(self.RPM_LOCK, mode="w+") as rfh: + try: + fcntl.lockf(rfh, fcntl.LOCK_EX | fcntl.LOCK_NB) + except OSError as err: + if err.errno == errno.EAGAIN: + return True + else: + fcntl.lockf(rfh, fcntl.LOCK_UN) + + return False + def _is_xml_mode(self): """ Is Zypper's output is in XML format? @@ -306,7 +328,7 @@ def _check_result(self): raise CommandExecutionError("No output result from Zypper?") self.exit_code = self.__call_result["retcode"] - if self._is_lock(): + if self._is_zypper_lock() or self._is_rpm_lock(): return False if self._is_error(): @@ -387,48 +409,11 @@ def __call(self, *args, **kwargs): if self._check_result(): break - if os.path.exists(self.ZYPPER_LOCK): - try: - with salt.utils.files.fopen(self.ZYPPER_LOCK) as rfh: - data = __salt__["ps.proc_info"]( - int(rfh.readline()), - attrs=["pid", "name", "cmdline", "create_time"], - ) - data["cmdline"] = " ".join(data["cmdline"]) - data["info"] = "Blocking process created at {}.".format( - datetime.datetime.utcfromtimestamp( - data["create_time"] - ).isoformat() - ) - data["success"] = True - except Exception as err: # pylint: disable=broad-except - data = { - "info": ( - "Unable to retrieve information about blocking process: {}".format( - err.message - ) - ), - "success": False, - } - else: - data = { - "info": "Zypper is locked, but no Zypper lock has been found.", - "success": False, - } - - if not data["success"]: - log.debug("Unable to collect data about blocking process.") - else: - log.debug("Collected data about blocking process.") - - __salt__["event.fire_master"](data, self.TAG_BLOCKED) - log.debug( - "Fired a Zypper blocked event to the master with the data: %s", data - ) - log.debug("Waiting 5 seconds for Zypper gets released...") - time.sleep(5) - if not was_blocked: - was_blocked = True + if self._is_zypper_lock(): + self._handle_zypper_lock_file() + if self._is_rpm_lock(): + self._handle_rpm_lock_file() + was_blocked = True if was_blocked: __salt__["event.fire_master"]( @@ -451,6 +436,50 @@ def __call(self, *args, **kwargs): or self.__call_result["stdout"] ) + def _handle_zypper_lock_file(self): + if os.path.exists(self.ZYPPER_LOCK): + try: + with salt.utils.files.fopen(self.ZYPPER_LOCK) as rfh: + data = __salt__["ps.proc_info"]( + int(rfh.readline()), + attrs=["pid", "name", "cmdline", "create_time"], + ) + data["cmdline"] = " ".join(data["cmdline"]) + data["info"] = "Blocking process created at {}.".format( + datetime.datetime.utcfromtimestamp( + data["create_time"] + ).isoformat() + ) + data["success"] = True + except Exception as err: # pylint: disable=broad-except + data = { + "info": ( + "Unable to retrieve information about " + "blocking process: {}".format(err) + ), + "success": False, + } + else: + data = { + "info": "Zypper is locked, but no Zypper lock has been found.", + "success": False, + } + if not data["success"]: + log.debug("Unable to collect data about blocking process.") + else: + log.debug("Collected data about blocking process.") + __salt__["event.fire_master"](data, self.TAG_BLOCKED) + log.debug("Fired a Zypper blocked event to the master with the data: %s", data) + log.debug("Waiting 5 seconds for Zypper gets released...") + time.sleep(5) + + def _handle_rpm_lock_file(self): + data = {"info": "RPM is temporarily locked.", "success": True} + __salt__["event.fire_master"](data, self.TAG_BLOCKED) + log.debug("Fired an RPM blocked event to the master with the data: %s", data) + log.debug("Waiting 5 seconds for RPM to get released...") + time.sleep(5) + __zypper__ = _Zypper() diff --git a/tests/unit/modules/test_zypperpkg.py b/tests/unit/modules/test_zypperpkg.py index 3f1560a385..37d555844c 100644 --- a/tests/unit/modules/test_zypperpkg.py +++ b/tests/unit/modules/test_zypperpkg.py @@ -4,6 +4,7 @@ import configparser +import errno import io import os from xml.dom import minidom @@ -97,7 +98,7 @@ def test_list_upgrades(self): } with patch.dict( zypper.__salt__, {"cmd.run_all": MagicMock(return_value=ref_out)} - ): + ), patch.object(zypper.__zypper__, "_is_rpm_lock", return_value=False): upgrades = zypper.list_upgrades(refresh=False) self.assertEqual(len(upgrades), 3) for pkg, version in { @@ -198,7 +199,9 @@ def __call__(self, *args, **kwargs): ' type="error">Booya!' ) sniffer = RunSniffer(stdout=stdout_xml_snippet, retcode=1) - with patch.dict("salt.modules.zypperpkg.__salt__", {"cmd.run_all": sniffer}): + with patch.dict( + "salt.modules.zypperpkg.__salt__", {"cmd.run_all": sniffer} + ), patch.object(zypper.__zypper__, "_is_rpm_lock", return_value=False): with self.assertRaisesRegex( CommandExecutionError, "^Zypper command failure: Booya!$" ): @@ -232,7 +235,7 @@ def test_list_upgrades_error_handling(self): with patch.dict( "salt.modules.zypperpkg.__salt__", {"cmd.run_all": MagicMock(return_value=ref_out)}, - ): + ), patch.object(zypper.__zypper__, "_is_rpm_lock", return_value=False): with self.assertRaisesRegex( CommandExecutionError, "^Zypper command failure: Some handled zypper internal error{}Another" @@ -245,7 +248,7 @@ def test_list_upgrades_error_handling(self): with patch.dict( "salt.modules.zypperpkg.__salt__", {"cmd.run_all": MagicMock(return_value=ref_out)}, - ): + ), patch.object(zypper.__zypper__, "_is_rpm_lock", return_value=False): with self.assertRaisesRegex( CommandExecutionError, "^Zypper command failure: Check Zypper's logs.$" ): @@ -2064,3 +2067,37 @@ def test_search_not_found(self): python_shell=False, env={"ZYPP_READONLY_HACK": "1"}, ) + + def test_is_rpm_lock_no_error(self): + with patch.object(os.path, "exists", return_value=True): + self.assertFalse(zypper.__zypper__._is_rpm_lock()) + + def test_rpm_lock_does_not_exist(self): + if salt.utils.files.is_fcntl_available(): + zypper.__zypper__.exit_code = 1 + with patch.object( + os.path, "exists", return_value=False + ) as mock_path_exists: + self.assertFalse(zypper.__zypper__._is_rpm_lock()) + mock_path_exists.assert_called_with(zypper.__zypper__.RPM_LOCK) + zypper.__zypper__._reset() + + def test_rpm_lock_acquirable(self): + if salt.utils.files.is_fcntl_available(): + zypper.__zypper__.exit_code = 1 + with patch.object(os.path, "exists", return_value=True), patch( + "fcntl.lockf", side_effect=OSError(errno.EAGAIN, "") + ) as lockf_mock, patch("salt.utils.files.fopen", mock_open()): + self.assertTrue(zypper.__zypper__._is_rpm_lock()) + lockf_mock.assert_called() + zypper.__zypper__._reset() + + def test_rpm_lock_not_acquirable(self): + if salt.utils.files.is_fcntl_available(): + zypper.__zypper__.exit_code = 1 + with patch.object(os.path, "exists", return_value=True), patch( + "fcntl.lockf" + ) as lockf_mock, patch("salt.utils.files.fopen", mock_open()): + self.assertFalse(zypper.__zypper__._is_rpm_lock()) + self.assertEqual(lockf_mock.call_count, 2) + zypper.__zypper__._reset()