Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Retry if RPM lock is temporarily unavailable #62204

Merged
merged 22 commits into from
Aug 22, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog/62204.fixed
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fixed Zypper module failing on RPM lock file being temporarily unavailable.
117 changes: 73 additions & 44 deletions salt/modules/zypperpkg.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@

import configparser
import datetime
import errno
import fnmatch
import logging
import os
Expand All @@ -37,6 +38,9 @@
from salt.exceptions import CommandExecutionError, MinionError, SaltInvocationError
from salt.utils.versions import LooseVersion

if salt.utils.files.is_fcntl_available():
import fcntl

log = logging.getLogger(__name__)

HAS_ZYPP = False
Expand Down Expand Up @@ -102,6 +106,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"

Expand Down Expand Up @@ -232,14 +237,31 @@ 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?

: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 @@ -262,7 +284,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 @@ -330,48 +352,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"](
Expand All @@ -394,6 +379,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()

Expand Down
47 changes: 42 additions & 5 deletions tests/unit/modules/test_zypperpkg.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@


import configparser
import errno
import io
import os
from xml.dom import minidom
Expand All @@ -14,7 +15,7 @@
import salt.utils.pkg
from salt.exceptions import CommandExecutionError
from tests.support.mixins import LoaderModuleMockMixin
from tests.support.mock import MagicMock, Mock, call, patch
from tests.support.mock import MagicMock, Mock, call, mock_open, patch
from tests.support.unit import TestCase


Expand Down Expand Up @@ -94,7 +95,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):
witekest marked this conversation as resolved.
Show resolved Hide resolved
upgrades = zypper.list_upgrades(refresh=False)
self.assertEqual(len(upgrades), 3)
for pkg, version in {
Expand Down Expand Up @@ -194,7 +195,9 @@ def __call__(self, *args, **kwargs):
' 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 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!$"
):
Expand Down Expand Up @@ -228,7 +231,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):
witekest marked this conversation as resolved.
Show resolved Hide resolved
with self.assertRaisesRegex(
CommandExecutionError,
"^Zypper command failure: Some handled zypper internal error{}Another"
Expand All @@ -241,7 +244,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):
witekest marked this conversation as resolved.
Show resolved Hide resolved
with self.assertRaisesRegex(
CommandExecutionError, "^Zypper command failure: Check Zypper's logs.$"
):
Expand Down Expand Up @@ -2063,3 +2066,37 @@ def test_services_need_restart(self):
with patch("salt.modules.zypperpkg.__zypper__", zypper_mock):
assert zypper.services_need_restart() == expected
zypper_mock(root=None).nolock.call.assert_called_with("ps", "-sss")

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()