Merge pull request #47578 from Ch3LL/mac_service_disabled

Ensure mac_service.disabled is correctly querying services

salt/modules/mac_service.py

@@ -496,7 +496,7 @@ def enabled(name, runas=None):
         return False
 
 
-def disabled(name, runas=None):
+def disabled(name, runas=None, domain='system'):
     '''
     Check if the specified service is not enabled. This is the opposite of
     ``service.enabled``
@@ -505,6 +505,8 @@ def disabled(name, runas=None):
 
     :param str runas: User to run launchctl commands
 
+    :param str domain: domain to check for disabled services. Default is system.
+
     :return: True if the specified service is NOT enabled, otherwise False
     :rtype: bool
 
@@ -514,8 +516,22 @@ def disabled(name, runas=None):
 
         salt '*' service.disabled org.cups.cupsd
     '''
-    # A service is disabled if it is not enabled
-    return not enabled(name, runas=runas)
+    ret = False
+    disabled = launchctl('print-disabled',
+                         domain,
+                         return_stdout=True,
+                         output_loglevel='trace',
+                         runas=runas)
+    for service in disabled.split("\n"):
+        if name in service:
+            srv_name = service.split("=>")[0].split("\"")[1]
+            status = service.split("=>")[1]
+            if name != srv_name:
+                pass
+            else:
+                return True if 'true' in status.lower() else False
+
+    return False
 
 
 def get_all(runas=None):

tests/integration/modules/test_service.py

@@ -10,6 +10,7 @@ from tests.support.unit import skipIf
 
 # Import Salt libs
 import salt.utils
+import salt.utils.systemd
 
 
 @destructiveTest
@@ -101,7 +102,38 @@ class ServiceModuleTest(ModuleCase):
         self.assertTrue(self.run_function('service.enable', [self.service_name]))
 
         self.assertTrue(self.run_function('service.disable', [self.service_name]))
-        self.assertIn(self.service_name, self.run_function('service.get_disabled'))
+        if salt.utils.is_darwin():
+            self.assertTrue(self.run_function('service.disabled', [self.service_name]))
+        else:
+            self.assertIn(self.service_name, self.run_function('service.get_disabled'))
+
+    def test_service_disable_doesnot_exist(self):
+        '''
+        test service.get_disabled and service.disable module
+        when service name does not exist
+        '''
+        # enable service before test
+        srv_name = 'doesnotexist'
+        enable = self.run_function('service.enable', [srv_name])
+        systemd = salt.utils.systemd.booted()
+
+        # check service was not enabled
+        if systemd:
+            self.assertIn('ERROR', enable)
+        else:
+            self.assertFalse(enable)
+
+        # check service was not disabled
+        if tuple(self.run_function('grains.item', ['osrelease_info'])['osrelease_info']) == (14, 0o4) and not systemd:
+            # currently upstart does not have a mechanism to report if disabling a service fails if does not exist
+            self.assertTrue(self.run_function('service.disable', [srv_name]))
+        else:
+            self.assertFalse(self.run_function('service.disable', [srv_name]))
+
+        if salt.utils.is_darwin():
+            self.assertFalse(self.run_function('service.disabled', [srv_name]))
+        else:
+            self.assertNotIn(srv_name, self.run_function('service.get_disabled'))
 
     @skipIf(not salt.utils.is_windows(), 'Windows Only Test')
     def test_service_get_service_name(self):

tests/unit/modules/test_mac_service.py

@@ -0,0 +1,69 @@
+# -*- coding: utf-8 -*-
+'''
+    :codeauthor: :email:`Megan Wilhite<mwilhite@saltstack.com>`
+'''
+
+# Import Python libs
+from __future__ import absolute_import
+
+# Import Salt Libs
+import salt.modules.mac_service as mac_service
+
+# Import Salt Testing Libs
+from tests.support.mixins import LoaderModuleMockMixin
+from tests.support.unit import skipIf, TestCase
+from tests.support.mock import (
+    MagicMock,
+    patch,
+    NO_MOCK,
+    NO_MOCK_REASON
+)
+
+
+@skipIf(NO_MOCK, NO_MOCK_REASON)
+class MacServiceTestCase(TestCase, LoaderModuleMockMixin):
+    '''
+    TestCase for salt.modules.mac_service module
+    '''
+    def setup_loader_modules(self):
+        return {mac_service: {}}
+
+    def test_service_disabled_when_enabled(self):
+        '''
+        test service.disabled when service is enabled
+        '''
+        srv_name = 'com.apple.atrun'
+        cmd = 'disabled services = {\n\t"com.saltstack.salt.minion" => false\n\t"com.apple.atrun" => false\n{'
+
+        with patch.object(mac_service, 'launchctl', MagicMock(return_value=cmd)):
+            self.assertFalse(mac_service.disabled(srv_name))
+
+    def test_service_disabled_when_disabled(self):
+        '''
+        test service.disabled when service is disabled
+        '''
+        srv_name = 'com.apple.atrun'
+        cmd = 'disabled services = {\n\t"com.saltstack.salt.minion" => false\n\t"com.apple.atrun" => true\n{'
+
+        with patch.object(mac_service, 'launchctl', MagicMock(return_value=cmd)):
+            self.assertTrue(mac_service.disabled(srv_name))
+
+    def test_service_disabled_srvname_wrong(self):
+        '''
+        test service.disabled when service is just slightly wrong
+        '''
+        srv_names = ['com.apple.atru', 'com', 'apple']
+        cmd = 'disabled services = {\n\t"com.saltstack.salt.minion" => false\n\t"com.apple.atrun" => true\n}'
+        for name in srv_names:
+            with patch.object(mac_service, 'launchctl', MagicMock(return_value=cmd)):
+                self.assertFalse(mac_service.disabled(name))
+
+    def test_service_disabled_status_upper_case(self):
+        '''
+        test service.disabled when disabled status is uppercase
+        '''
+        srv_name = 'com.apple.atrun'
+        cmd = 'disabled services = {\n\t"com.saltstack.salt.minion" => false\n\t"com.apple.atrun" => True\n{'
+
+        with patch.object(mac_service, 'launchctl', MagicMock(return_value=cmd)):
+            self.assertTrue(mac_service.disabled(srv_name))