File 0050-Fix-service-state-returning-stacktrace-bsc... of Package salt.4663

Overview Repositories Revisions Requests Users Attributes Meta

File 0050-Fix-service-state-returning-stacktrace-bsc-1027044.patch of Package salt.4663

From b26d228bfeff7a902fde4a9de6f77d488ec8d7c8 Mon Sep 17 00:00:00 2001
From: Erik Johnson <palehose@gmail.com>
Date: Wed, 28 Sep 2016 15:29:25 -0500
Subject: [PATCH 50/50] Fix service state returning stacktrace (bsc#1027044)

Move check for service availability to a helper function

This allows for us to check for unit file changes at the beginning of
the available() function without invoking the available() function a
second time, potentially resulting in a recursive loop if not handled
delicately.

systemd.py: check retcode for service availability in systemd >= 231

systemd 231 changed the output for "systemctl status" for unrecognized
services. This causes Salt to fail to be able to determine if the
service could be found, resulting in an exception being raised.

systemd 231 also made a change to the retcode of "systemctl status" to
distinguish unrecognized services from ones which simply are not
running.

This commit does a systemd version check, and for versions >= 231 checks
the retcode to determine whether or not a given service is available.

Handle cases where retcode/output feature is backported

This will handle the case of RHEL 7.3, which maintains an older version
of systemd with the retcode/output changes from "systemctl status" in
systemd 231 backported.

Update systemd module unit tests

The _systemctl_status() function has changed its return data, this
commit updates the affected tests to reflect this.
---
 patches                            |  1 +
 salt/modules/systemd.py            | 51 ++++++++++++++++++++-------
 tests/unit/modules/systemd_test.py | 72 +++++++++++++++++++++++++++++++++-----
 3 files changed, 102 insertions(+), 22 deletions(-)
 create mode 160000 patches

diff --git a/patches b/patches
new file mode 160000
index 0000000000..fc57687bc8
--- /dev/null
+++ b/patches
@@ -0,0 +1 @@
+Subproject commit fc57687bc86714ec5353b649fd0a5d1a75b329c6
diff --git a/salt/modules/systemd.py b/salt/modules/systemd.py
index 40dd695075..27476d713b 100644
--- a/salt/modules/systemd.py
+++ b/salt/modules/systemd.py
@@ -65,6 +65,39 @@ def _canonical_unit_name(name):
     return '%s.service' % name
 
 
+def _check_available(name):
+    '''
+    Returns boolean telling whether or not the named service is available
+    '''
+    _status = _systemctl_status(name)
+    sd_version = salt.utils.systemd.version(__context__)
+    if sd_version is not None and sd_version >= 231:
+        # systemd 231 changed the output of "systemctl status" for unknown
+        # services, and also made it return an exit status of 4. If we are on
+        # a new enough version, check the retcode, otherwise fall back to
+        # parsing the "systemctl status" output.
+        # See: https://github.com/systemd/systemd/pull/3385
+        # Also: https://github.com/systemd/systemd/commit/3dced37
+        return 0 <= _status['retcode'] < 4
+
+    out = _status['stdout'].lower()
+    if 'could not be found' in out:
+        # Catch cases where the systemd version is < 231 but the return code
+        # and output changes have been backported (e.g. RHEL 7.3).
+        return False
+
+    for line in salt.utils.itertools.split(out, '\n'):
+        match = re.match(r'\s+loaded:\s+(\S+)', line)
+        if match:
+            ret = match.group(1) != 'not-found'
+            break
+    else:
+        raise CommandExecutionError(
+            'Failed to get information on unit \'%s\'' % name
+        )
+    return ret
+
+
 def _check_for_unit_changes(name):
     '''
     Check for modified/updated unit files, and run a daemon-reload if any are
@@ -264,9 +297,10 @@ def _systemctl_status(name):
     contextkey = 'systemd._systemctl_status.%s' % name
     if contextkey in __context__:
         return __context__[contextkey]
-    __context__[contextkey] = __salt__['cmd.run'](
+    __context__[contextkey] = __salt__['cmd.run_all'](
         _systemctl_cmd('status', name),
         python_shell=False,
+        redirect_stderr=True,
         ignore_retcode=True
     )
     return __context__[contextkey]
@@ -300,7 +334,7 @@ def _unit_file_changed(name):
     Returns True if systemctl reports that the unit file has changed, otherwise
     returns False.
     '''
-    return "'systemctl daemon-reload'" in _systemctl_status(name).lower()
+    return "'systemctl daemon-reload'" in _systemctl_status(name)['stdout'].lower()
 
 
 def systemctl_reload():
@@ -475,17 +509,8 @@ def available(name):
 
         salt '*' service.available sshd
     '''
-    out = _systemctl_status(name).lower()
-    for line in salt.utils.itertools.split(out, '\n'):
-        match = re.match(r'\s+loaded:\s+(\S+)', line)
-        if match:
-            ret = match.group(1) != 'not-found'
-            break
-    else:
-        raise CommandExecutionError(
-            'Failed to get information on unit \'%s\'' % name
-        )
-    return ret
+    _check_for_unit_changes(name)
+    return _check_available(name)
 
 
 def missing(name):
diff --git a/tests/unit/modules/systemd_test.py b/tests/unit/modules/systemd_test.py
index ee9472855d..e6950024cb 100644
--- a/tests/unit/modules/systemd_test.py
+++ b/tests/unit/modules/systemd_test.py
@@ -28,15 +28,35 @@ systemd.__salt__ = {}
 systemd.__context__ = {}
 
 _SYSTEMCTL_STATUS = {
-    'sshd.service': '''\
+    'sshd.service': {
+        'stdout': '''\
 * sshd.service - OpenSSH Daemon
    Loaded: loaded (/usr/lib/systemd/system/sshd.service; disabled; vendor preset: disabled)
    Active: inactive (dead)''',
+        'stderr': '',
+        'retcode': 3,
+        'pid': 12345,
+    },
 
-    'foo.service': '''\
+    'foo.service': {
+        'stdout': '''\
 * foo.service
    Loaded: not-found (Reason: No such file or directory)
-   Active: inactive (dead)'''
+   Active: inactive (dead)''',
+        'stderr': '',
+        'retcode': 3,
+        'pid': 12345,
+    },
+}
+
+# This reflects systemd >= 231 behavior
+_SYSTEMCTL_STATUS_GTE_231 = {
+    'bar.service': {
+        'stdout': 'Unit bar.service could not be found.',
+        'stderr': '',
+        'retcode': 4,
+        'pid': 12345,
+    },
 }
 
 _LIST_UNIT_FILES = '''\
@@ -169,18 +189,52 @@ class SystemdTestCase(TestCase):
         Test to check that the given service is available
         '''
         mock = MagicMock(side_effect=lambda x: _SYSTEMCTL_STATUS[x])
-        with patch.object(systemd, '_systemctl_status', mock):
-            self.assertTrue(systemd.available('sshd.service'))
-            self.assertFalse(systemd.available('foo.service'))
+
+        # systemd < 231
+        with patch.dict(systemd.__context__, {'salt.utils.systemd.version': 230}):
+            with patch.object(systemd, '_systemctl_status', mock):
+                self.assertTrue(systemd.available('sshd.service'))
+                self.assertFalse(systemd.available('foo.service'))
+
+        # systemd >= 231
+        with patch.dict(systemd.__context__, {'salt.utils.systemd.version': 231}):
+            with patch.dict(_SYSTEMCTL_STATUS, _SYSTEMCTL_STATUS_GTE_231):
+                with patch.object(systemd, '_systemctl_status', mock):
+                    self.assertTrue(systemd.available('sshd.service'))
+                    self.assertFalse(systemd.available('bar.service'))
+
+        # systemd < 231 with retcode/output changes backported (e.g. RHEL 7.3)
+        with patch.dict(systemd.__context__, {'salt.utils.systemd.version': 219}):
+            with patch.dict(_SYSTEMCTL_STATUS, _SYSTEMCTL_STATUS_GTE_231):
+                with patch.object(systemd, '_systemctl_status', mock):
+                    self.assertTrue(systemd.available('sshd.service'))
+                    self.assertFalse(systemd.available('bar.service'))
 
     def test_missing(self):
         '''
             Test to the inverse of service.available.
         '''
         mock = MagicMock(side_effect=lambda x: _SYSTEMCTL_STATUS[x])
-        with patch.object(systemd, '_systemctl_status', mock):
-            self.assertFalse(systemd.missing('sshd.service'))
-            self.assertTrue(systemd.missing('foo.service'))
+
+        # systemd < 231
+        with patch.dict(systemd.__context__, {'salt.utils.systemd.version': 230}):
+            with patch.object(systemd, '_systemctl_status', mock):
+                self.assertFalse(systemd.missing('sshd.service'))
+                self.assertTrue(systemd.missing('foo.service'))
+
+        # systemd >= 231
+        with patch.dict(systemd.__context__, {'salt.utils.systemd.version': 231}):
+            with patch.dict(_SYSTEMCTL_STATUS, _SYSTEMCTL_STATUS_GTE_231):
+                with patch.object(systemd, '_systemctl_status', mock):
+                    self.assertFalse(systemd.missing('sshd.service'))
+                    self.assertTrue(systemd.missing('bar.service'))
+
+        # systemd < 231 with retcode/output changes backported (e.g. RHEL 7.3)
+        with patch.dict(systemd.__context__, {'salt.utils.systemd.version': 219}):
+            with patch.dict(_SYSTEMCTL_STATUS, _SYSTEMCTL_STATUS_GTE_231):
+                with patch.object(systemd, '_systemctl_status', mock):
+                    self.assertFalse(systemd.missing('sshd.service'))
+                    self.assertTrue(systemd.missing('bar.service'))
 
     def test_show(self):
         '''
-- 
2.11.0

Places

File 0050-Fix-service-state-returning-stacktrace-bsc-1027044.patch of Package salt.4663

Places