From 5459d07aa94d05c2e8f942f024966931a6f90508 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pablo=20Su=C3=A1rez=20Hern=C3=A1ndez?= Date: Mon, 29 Aug 2022 14:30:11 +0100 Subject: [PATCH] Retry if RPM lock is temporarily unavailable (#547) (#551) * 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 Signed-off-by: Witek Bedyk Co-authored-by: Witek Bedyk --- changelog/62204.fixed | 1 + salt/modules/zypperpkg.py | 106 +++++++++++++++++++-------- tests/unit/modules/test_zypperpkg.py | 77 +++++++++++++++---- 3 files changed, 140 insertions(+), 44 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 1602ae09db..65da8d8f6d 100644 --- a/salt/modules/zypperpkg.py +++ b/salt/modules/zypperpkg.py @@ -14,6 +14,8 @@ # Import python libs from __future__ import absolute_import, print_function, unicode_literals + +import errno import fnmatch import logging import re @@ -46,6 +48,9 @@ import salt.utils.environment from salt.exceptions import CommandExecutionError, MinionError, SaltInvocationError +if salt.utils.files.is_fcntl_available(): + import fcntl + log = logging.getLogger(__name__) HAS_ZYPP = False @@ -103,6 +108,7 @@ class _Zypper(object): 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' @@ -268,14 +274,31 @@ def _is_error(self): return self.exit_code not in self.SUCCESS_EXIT_CODES 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? :return: - ''' + """ 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? @@ -296,7 +319,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(): @@ -369,32 +392,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 {0}.'.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: {0}'.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']({'success': not self.error_msg, @@ -409,6 +411,50 @@ def __call(self, *args, **kwargs): 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 f9a3bbfca1..9cdbeff64a 100644 --- a/tests/unit/modules/test_zypperpkg.py +++ b/tests/unit/modules/test_zypperpkg.py @@ -5,6 +5,8 @@ # Import Python Libs from __future__ import absolute_import + +import errno import os from xml.dom import minidom @@ -109,7 +111,9 @@ def test_list_upgrades(self): 'stderr': None, 'retcode': 0 } - with patch.dict(zypper.__salt__, {'cmd.run_all': MagicMock(return_value=ref_out)}): + 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 {'SUSEConnect': '0.2.33-7.1', @@ -168,9 +172,13 @@ def __call__(self, *args, **kwargs): # Test exceptions stdout_xml_snippet = 'Booya!' sniffer = RunSniffer(stdout=stdout_xml_snippet, retcode=1) - with patch.dict('salt.modules.zypperpkg.__salt__', {'cmd.run_all': sniffer}): - with self.assertRaisesRegex(CommandExecutionError, '^Zypper command failure: Booya!$'): - zypper.__zypper__.xml.call('crashme') + 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!$" + ): + zypper.__zypper__.xml.call("crashme") with self.assertRaisesRegex(CommandExecutionError, "^Zypper command failure: Check Zypper's logs.$"): zypper.__zypper__.call('crashme again') @@ -195,19 +203,26 @@ def test_list_upgrades_error_handling(self): 'stderr': '', 'retcode': 1, } - with patch.dict('salt.modules.zypperpkg.__salt__', {'cmd.run_all': MagicMock(return_value=ref_out)}): - with self.assertRaisesRegex(CommandExecutionError, - "^Zypper command failure: Some handled zypper internal error{0}Another zypper internal error$".format(os.linesep)): + 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" + " zypper internal error$".format(os.linesep), + ): zypper.list_upgrades(refresh=False) # Test unhandled error - ref_out = { - 'retcode': 1, - 'stdout': '', - 'stderr': '' - } - with patch.dict('salt.modules.zypperpkg.__salt__', {'cmd.run_all': MagicMock(return_value=ref_out)}): - with self.assertRaisesRegex(CommandExecutionError, "^Zypper command failure: Check Zypper's logs.$"): + ref_out = {"retcode": 1, "stdout": "", "stderr": ""} + 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.$" + ): zypper.list_upgrades(refresh=False) def test_list_products(self): @@ -2257,3 +2272,37 @@ def test_normalize_name(self): assert result == "foo", result result = zypper.normalize_name("foo.noarch") assert result == "foo", result + + 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()