From 05ff9e15b311e2aa852c59092034b55aaa021a90 Mon Sep 17 00:00:00 2001
From: Alexander Graul <mail@agraul.de>
Date: Tue, 8 Dec 2020 15:35:49 +0100
Subject: [PATCH] Add pkg.services_need_restart (#302)

* Add utils.systemd.pid_to_service function

This function translates a given PID to the systemd service name in case
the process belongs to a running service. It uses DBUS for the
translation if DBUS is available, falling back to parsing
``systemctl status -o json'' output.

* Add zypperpkg.services_need_restart

pkg.services_need_restart returns a list of system services that were
affected by package manager operations such as updates, downgrades or
reinstallations without having been restarted. This might cause issues,
e.g. in the case a shared object was loaded by a process and then
replaced by the package manager.

(cherry picked from commit b950fcdbd6cc8cb08e1413a0ed05e0ae21717cea)

* Add aptpkg.services_need_restart

pkg.services_need_restart returns a list of system services that were
affected by package manager operations such as updates, downgrades or
reinstallations without having been restarted. This might cause issues,
e.g. in the case a shared object was loaded by a process and then
replaced by the package manager.

Requires checkrestart, which is part of the debian-goodies package and
available from official Ubuntu and Debian repositories.

(cherry picked from commit b981f6ecb1a551b98c5cebab4975fc09c6a55a22)

* Add yumpkg.services_need_restart

pkg.services_need_restart returns a list of system services that were
affected by package manager operations such as updates, downgrades or
reinstallations without having been restarted. This might cause issues,
e.g. in the case a shared object was loaded by a process and then
replaced by the package manager.

Requires dnf with the needs-restarting plugin, which is part of
dnf-plugins-core and installed by default on RHEL/CentOS/Fedora.
Also requires systemd for the mapping between PIDs and systemd services.

(cherry picked from commit 5e2be1095729c9f73394e852b82749950957e6fb)

* Add changelog entry for issue #58261

(cherry picked from commit 148877ed8ff7a47132c1186274739e648f7acf1c)

* Simplify dnf needs-restarting output parsing

Co-authored-by: Wayne Werner <waynejwerner@gmail.com>
(cherry picked from commit beb5d60f3cc64b880ec25ca188f8a73f6ec493dd)
---
 changelog/58261.added                |  1 +
 salt/modules/aptpkg.py               | 40 +++++++++++++++-
 salt/modules/yumpkg.py               | 36 +++++++++++++++
 salt/modules/zypperpkg.py            | 25 ++++++++++
 salt/utils/systemd.py                | 69 ++++++++++++++++++++++++++++
 tests/unit/modules/test_aptpkg.py    | 33 ++++++++++++-
 tests/unit/modules/test_yumpkg.py    | 31 +++++++++++++
 tests/unit/modules/test_zypperpkg.py | 14 ++++++
 8 files changed, 247 insertions(+), 2 deletions(-)
 create mode 100644 changelog/58261.added

diff --git a/changelog/58261.added b/changelog/58261.added
new file mode 100644
index 0000000000..537a43e80d
--- /dev/null
+++ b/changelog/58261.added
@@ -0,0 +1 @@
+Added ``pkg.services_need_restart`` which lists system services that should be restarted after package management operations.
diff --git a/salt/modules/aptpkg.py b/salt/modules/aptpkg.py
index 28b8597ef5..2fe31e936d 100644
--- a/salt/modules/aptpkg.py
+++ b/salt/modules/aptpkg.py
@@ -48,7 +48,10 @@ import salt.utils.versions
 import salt.utils.yaml
 import salt.utils.environment
 from salt.exceptions import (
-    CommandExecutionError, MinionError, SaltInvocationError
+    CommandExecutionError,
+    CommandNotFoundError,
+    MinionError,
+    SaltInvocationError,
 )
 
 log = logging.getLogger(__name__)
@@ -2975,3 +2978,38 @@ def list_downloaded(root=None, **kwargs):
                 'creation_date_time': datetime.datetime.utcfromtimestamp(pkg_timestamp).isoformat(),
             }
     return ret
+
+
+def services_need_restart(**kwargs):
+    """
+    .. versionadded:: NEXT
+
+    List services that use files which have been changed by the
+    package manager. It might be needed to restart them.
+
+    Requires checkrestart from the debian-goodies package.
+
+    CLI Examples:
+
+    .. code-block:: bash
+
+        salt '*' pkg.services_need_restart
+    """
+    if not salt.utils.path.which_bin(["checkrestart"]):
+        raise CommandNotFoundError(
+            "'checkrestart' is needed. It is part of the 'debian-goodies' "
+            "package which can be installed from official repositories."
+        )
+
+    cmd = ["checkrestart", "--machine"]
+    services = set()
+
+    cr_output = __salt__["cmd.run_stdout"](cmd, python_shell=False)
+    for line in cr_output.split("\n"):
+        if not line.startswith("SERVICE:"):
+            continue
+        end_of_name = line.find(",")
+        service = line[8:end_of_name]  # skip "SERVICE:"
+        services.add(service)
+
+    return list(services)
diff --git a/salt/modules/yumpkg.py b/salt/modules/yumpkg.py
index 85a2dbd857..deb17f5af8 100644
--- a/salt/modules/yumpkg.py
+++ b/salt/modules/yumpkg.py
@@ -3359,3 +3359,39 @@ def del_repo_key(keyid, root=None, **kwargs):
 
     """
     return __salt__["lowpkg.remove_gpg_key"](keyid, root)
+
+
+def services_need_restart(**kwargs):
+    """
+    .. versionadded:: NEXT
+
+    List services that use files which have been changed by the
+    package manager. It might be needed to restart them.
+
+    Requires systemd.
+
+    CLI Examples:
+
+    .. code-block:: bash
+
+        salt '*' pkg.services_need_restart
+    """
+    if _yum() != "dnf":
+        raise CommandExecutionError("dnf is required to list outdated services.")
+    if not salt.utils.systemd.booted(__context__):
+        raise CommandExecutionError("systemd is required to list outdated services.")
+
+    cmd = ["dnf", "--quiet", "needs-restarting"]
+    dnf_output = __salt__["cmd.run_stdout"](cmd, python_shell=False)
+    if not dnf_output:
+        return []
+
+    services = set()
+    for line in dnf_output.split("\n"):
+        pid, has_delim, _ = line.partition(":")
+        if has_delim:
+            service = salt.utils.systemd.pid_to_service(pid.strip())
+            if service:
+                services.add(service)
+
+    return list(services)
diff --git a/salt/modules/zypperpkg.py b/salt/modules/zypperpkg.py
index fab7736701..8200adfe24 100644
--- a/salt/modules/zypperpkg.py
+++ b/salt/modules/zypperpkg.py
@@ -2967,3 +2967,28 @@ def del_repo_key(keyid, root=None, **kwargs):
 
     """
     return __salt__["lowpkg.remove_gpg_key"](keyid, root)
+
+
+def services_need_restart(root=None, **kwargs):
+    """
+    .. versionadded:: NEXT
+
+    List services that use files which have been changed by the
+    package manager. It might be needed to restart them.
+
+    root
+        operate on a different root directory.
+
+    CLI Examples:
+
+    .. code-block:: bash
+
+        salt '*' pkg.services_need_restart
+
+    """
+    cmd = ["ps", "-sss"]
+
+    zypper_output = __zypper__(root=root).nolock.call(*cmd)
+    services = zypper_output.split()
+
+    return services
diff --git a/salt/utils/systemd.py b/salt/utils/systemd.py
index 674b6d419f..6d51d87d2f 100644
--- a/salt/utils/systemd.py
+++ b/salt/utils/systemd.py
@@ -14,6 +14,12 @@ from salt.exceptions import SaltInvocationError
 import salt.utils.path
 import salt.utils.stringutils
 
+try:
+    import dbus
+except ImportError:
+    dbus = None
+
+
 log = logging.getLogger(__name__)
 
 
@@ -114,3 +120,66 @@ def has_scope(context=None):
     if _sd_version is None:
         return False
     return _sd_version >= 205
+
+
+def pid_to_service(pid):
+    """
+    Check if a PID belongs to a systemd service and return its name.
+    Return None if the PID does not belong to a service.
+
+    Uses DBUS if available.
+    """
+    if dbus:
+        return _pid_to_service_dbus(pid)
+    else:
+        return _pid_to_service_systemctl(pid)
+
+
+def _pid_to_service_systemctl(pid):
+    systemd_cmd = ["systemctl", "--output", "json", "status", str(pid)]
+    try:
+        systemd_output = subprocess.run(
+            systemd_cmd, check=True, text=True, capture_output=True
+        )
+        status_json = salt.utils.json.find_json(systemd_output.stdout)
+    except (ValueError, subprocess.CalledProcessError):
+        return None
+
+    name = status_json.get("_SYSTEMD_UNIT")
+    if name and name.endswith(".service"):
+        return _strip_suffix(name)
+    else:
+        return None
+
+
+def _pid_to_service_dbus(pid):
+    """
+    Use DBUS to check if a PID belongs to a running systemd service and return the service name if it does.
+    """
+    bus = dbus.SystemBus()
+    systemd_object = bus.get_object(
+        "org.freedesktop.systemd1", "/org/freedesktop/systemd1"
+    )
+    systemd = dbus.Interface(systemd_object, "org.freedesktop.systemd1.Manager")
+    try:
+        service_path = systemd.GetUnitByPID(pid)
+        service_object = bus.get_object("org.freedesktop.systemd1", service_path)
+        service_props = dbus.Interface(
+            service_object, "org.freedesktop.DBus.Properties"
+        )
+        service_name = service_props.Get("org.freedesktop.systemd1.Unit", "Id")
+        name = str(service_name)
+
+        if name and name.endswith(".service"):
+            return _strip_suffix(name)
+        else:
+            return None
+    except dbus.DBusException:
+        return None
+
+
+def _strip_suffix(service_name):
+    """
+    Strip ".service" suffix from a given service name.
+    """
+    return service_name[:-8]
diff --git a/tests/unit/modules/test_aptpkg.py b/tests/unit/modules/test_aptpkg.py
index b0193aeaf7..c29b6a8a50 100644
--- a/tests/unit/modules/test_aptpkg.py
+++ b/tests/unit/modules/test_aptpkg.py
@@ -18,7 +18,11 @@ from tests.support.mock import Mock, MagicMock, patch
 
 # Import Salt Libs
 from salt.ext import six
-from salt.exceptions import CommandExecutionError, SaltInvocationError
+from salt.exceptions import (
+    CommandExecutionError,
+    CommandNotFoundError,
+    SaltInvocationError,
+)
 import salt.modules.aptpkg as aptpkg
 import pytest
 import textwrap
@@ -679,3 +683,30 @@ class AptUtilsTestCase(TestCase, LoaderModuleMockMixin):
             aptpkg.__salt__['cmd.run_all'].assert_called_once_with(
                 ['dpkg', '-l', 'python'], env={}, ignore_retcode=False,
                 output_loglevel='quiet', python_shell=True, username='Darth Vader')
+
+    def test_services_need_restart_checkrestart_missing(self):
+        """Test that the user is informed about the required dependency."""
+
+        with patch("salt.utils.path.which_bin", Mock(return_value=None)):
+            self.assertRaises(CommandNotFoundError, aptpkg.services_need_restart)
+
+    @patch("salt.utils.path.which_bin", Mock(return_value="/usr/sbin/checkrestart"))
+    def test_services_need_restart(self):
+        """
+        Test that checkrestart output is parsed correctly
+        """
+        cr_output = """
+PROCESSES: 24
+PROGRAMS: 17
+PACKAGES: 8
+SERVICE:rsyslog,385,/usr/sbin/rsyslogd
+SERVICE:cups-daemon,390,/usr/sbin/cupsd
+       """
+
+        with patch.dict(
+            aptpkg.__salt__, {"cmd.run_stdout": Mock(return_value=cr_output)}
+        ):
+            assert sorted(aptpkg.services_need_restart()) == [
+                "cups-daemon",
+                "rsyslog",
+            ]
diff --git a/tests/unit/modules/test_yumpkg.py b/tests/unit/modules/test_yumpkg.py
index dfe00a7181..f4192f5fbf 100644
--- a/tests/unit/modules/test_yumpkg.py
+++ b/tests/unit/modules/test_yumpkg.py
@@ -10,6 +10,7 @@ from tests.support.unit import TestCase, skipIf
 from tests.support.mock import (
     Mock,
     MagicMock,
+    call,
     mock_open,
     patch,
 )
@@ -893,3 +894,33 @@ class YumUtilsTestCase(TestCase, LoaderModuleMockMixin):
             yumpkg.__salt__['cmd.run_all'].assert_called_once_with(
                 ['fake-yum', '-y', '--do-something'], env={}, ignore_retcode=False,
                 output_loglevel='quiet', python_shell=True, username='Darth Vader')
+
+    @skipIf(not salt.utils.systemd.booted(), "Requires systemd")
+    @patch("salt.modules.yumpkg._yum", Mock(return_value="dnf"))
+    def test_services_need_restart(self):
+        """
+        Test that dnf needs-restarting output is parsed and
+        salt.utils.systemd.pid_to_service is called as expected.
+        """
+        expected = ["firewalld", "salt-minion"]
+
+        dnf_mock = Mock(
+            return_value="123 : /usr/bin/firewalld\n456 : /usr/bin/salt-minion\n"
+        )
+        systemd_mock = Mock(side_effect=["firewalld", "salt-minion"])
+        with patch.dict(yumpkg.__salt__, {"cmd.run_stdout": dnf_mock}), patch(
+            "salt.utils.systemd.pid_to_service", systemd_mock
+        ):
+            assert sorted(yumpkg.services_need_restart()) == expected
+            systemd_mock.assert_has_calls([call("123"), call("456")])
+
+    @patch("salt.modules.yumpkg._yum", Mock(return_value="dnf"))
+    def test_services_need_restart_requires_systemd(self):
+        """Test that yumpkg.services_need_restart raises an error if systemd is unavailable."""
+        with patch("salt.utils.systemd.booted", Mock(return_value=False)):
+            pytest.raises(CommandExecutionError, yumpkg.services_need_restart)
+
+    @patch("salt.modules.yumpkg._yum", Mock(return_value="yum"))
+    def test_services_need_restart_requires_dnf(self):
+        """Test that yumpkg.services_need_restart raises an error if DNF is unavailable."""
+        pytest.raises(CommandExecutionError, yumpkg.services_need_restart)
diff --git a/tests/unit/modules/test_zypperpkg.py b/tests/unit/modules/test_zypperpkg.py
index 1f2a7dc4b2..5adb0ba016 100644
--- a/tests/unit/modules/test_zypperpkg.py
+++ b/tests/unit/modules/test_zypperpkg.py
@@ -1766,3 +1766,17 @@ pattern() = package-c"""
         with patch.dict(zypper.__salt__, salt_mock):
             self.assertTrue(zypper.del_repo_key(keyid="keyid", root="/mnt"))
             salt_mock["lowpkg.remove_gpg_key"].assert_called_once_with("keyid", "/mnt")
+
+    def test_services_need_restart(self):
+        """
+        Test that zypper ps is used correctly to list services that need to
+        be restarted.
+        """
+        expected = ["salt-minion", "firewalld"]
+        zypper_output = "salt-minion\nfirewalld"
+        zypper_mock = Mock()
+        zypper_mock(root=None).nolock.call = Mock(return_value=zypper_output)
+
+        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")
-- 
2.29.1


