Skip to content

Commit

Permalink
Retry if RPM lock is temporarily unavailable (#547) (#551)
Browse files Browse the repository at this point in the history 
* Retry if RPM lock is temporarily unavailable

Backported from saltstack/salt#62204

Signed-off-by: Witek Bedyk <witold.bedyk@suse.com>

* Sync formating fixes from upstream

Signed-off-by: Witek Bedyk <witold.bedyk@suse.com>

Signed-off-by: Witek Bedyk <witold.bedyk@suse.com>
Co-authored-by: Witek Bedyk <witold.bedyk@suse.com>
  • Loading branch information
@meaksh @witekest
meaksh and witekest authored Aug 29, 2022
1 parent 144bd25 commit 5459d07
Show file tree
Hide file tree
Showing 3 changed files with 140 additions and 44 deletions.
1 change: 1 addition & 0 deletions changelog/62204.fixed
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fixed Zypper module failing on RPM lock file being temporarily unavailable.
106 changes: 76 additions & 30 deletions salt/modules/zypperpkg.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@

# Import python libs
from __future__ import absolute_import, print_function, unicode_literals

import errno
import fnmatch
import logging
import re
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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'

Expand Down Expand Up @@ -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?
Expand All @@ -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():
Expand Down Expand Up @@ -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,
Expand All @@ -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()

Expand Down
77 changes: 63 additions & 14 deletions tests/unit/modules/test_zypperpkg.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@

# Import Python Libs
from __future__ import absolute_import

import errno
import os
from xml.dom import minidom

Expand Down Expand Up @@ -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',
Expand Down Expand Up @@ -168,9 +172,13 @@ def __call__(self, *args, **kwargs):
# Test exceptions
stdout_xml_snippet = '<?xml version="1.0"?><stream><message type="error">Booya!</message></stream>'
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')
Expand All @@ -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):
Expand Down Expand Up @@ -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()

0 comments on commit 5459d07

Please sign in to comment.