Merge pull request #36676 from vutny/redhat-alternatives-detect-fail

`alternatives.install` state: detect `alternatives` command failed

salt/modules/alternatives.py

@@ -10,6 +10,10 @@ from __future__ import absolute_import
 import os
 import logging
 
+# Import Salt libs
+import salt.utils
+
+
 __outputter__ = {
     'display': 'txt',
     'install': 'txt',
@@ -59,6 +63,39 @@ def display(name):
     return out['stdout']
 
 
+def show_link(name):
+    '''
+    Display master link for the alternative
+
+    .. versionadded:: 2015.8.13,2016.3.4,Carbon
+
+    CLI Example:
+
+    .. code-block:: bash
+
+        salt '*' alternatives.show_link editor
+    '''
+
+    path = '/var/lib/dpkg/alternatives/{0}'.format(name)
+    if _get_cmd() == 'alternatives':
+        path = '/var/lib/alternatives/{0}'.format(name)
+
+    try:
+        with salt.utils.fopen(path, 'rb') as r_file:
+            return r_file.readlines()[1]
+    except OSError:
+        log.error(
+            'alternatives: {0} does not exist'.format(name)
+        )
+    except (IOError, IndexError) as exc:
+        log.error(
+            'alternatives: unable to get master link for {0}. '
+            'Exception: {1}'.format(name, exc)
+        )
+
+    return False
+
+
 def show_current(name):
     '''
     Display the current highest-priority alternative for a given alternatives

salt/states/alternatives.py

@@ -33,6 +33,13 @@ __func_alias__ = {
 }
 
 
+def __virtual__():
+    '''
+    Only load if alternatives execution module is available.
+    '''
+    return True if 'alternatives.auto' in __salt__ else False
+
+
 def install(name, link, path, priority):
     '''
     Install new alternative for defined <name>
@@ -63,24 +70,33 @@ def install(name, link, path, priority):
            'comment': ''}
 
     isinstalled = __salt__['alternatives.check_installed'](name, path)
-    if not isinstalled:
+    if isinstalled:
+        ret['comment'] = 'Alternatives for {0} is already set to {1}'.format(name, path)
+    else:
         if __opts__['test']:
             ret['comment'] = (
                 'Alternative will be set for {0} to {1} with priority {2}'
             ).format(name, path, priority)
             ret['result'] = None
             return ret
-        __salt__['alternatives.install'](name, link, path, priority)
-        ret['comment'] = (
-            'Setting alternative for {0} to {1} with priority {2}'
-        ).format(name, path, priority)
-        ret['changes'] = {'name': name,
-                          'link': link,
-                          'path': path,
-                          'priority': priority}
-        return ret
 
-    ret['comment'] = 'Alternatives for {0} is already set to {1}'.format(name, path)
+        out = __salt__['alternatives.install'](name, link, path, priority)
+        current = __salt__['alternatives.show_current'](name)
+        master_link = __salt__['alternatives.show_link'](name)
+        if current == path and master_link == link:
+            ret['comment'] = (
+                'Alternative for {0} set to path {1} with priority {2}'
+            ).format(name, current, priority)
+            ret['changes'] = {'name': name,
+                              'link': link,
+                              'path': path,
+                              'priority': priority}
+        else:
+            ret['result'] = False
+            ret['comment'] = (
+                'Alternative for {0} not installed: {1}'
+            ).format(name, out)
+
     return ret
 
 
@@ -214,7 +230,6 @@ def set_(name, path):
         __salt__['alternatives.set'](name, path)
         current = __salt__['alternatives.show_current'](name)
         if current == path:
-            ret['result'] = True
             ret['comment'] = (
                 'Alternative for {0} set to path {1}'
             ).format(name, current)

tests/unit/modules/alternatives_test.py

@@ -37,7 +37,7 @@ class AlternativesTestCase(TestCase):
                     python_shell=False
                 )
 
-        with patch.dict(alternatives.__grains__, {'os_family': 'Ubuntu'}):
+        with patch.dict(alternatives.__grains__, {'os_family': 'Suse'}):
             mock = MagicMock(
                 return_value={'retcode': 0, 'stdout': 'undoubtedly-salt'}
             )

tests/unit/states/alternatives_test.py

@@ -49,26 +49,33 @@ class AlternativesTestCase(TestCase):
                'changes': {},
                'comment': ''}
 
-        mock = MagicMock(side_effect=[True, False, False])
-        mock_bool = MagicMock(return_value=True)
+        bad_link = '/bin/pager'
+        err = 'the primary link for {0} must be {1}'.format(name, link)
+
+        mock = MagicMock(side_effect=[True, False, False, False])
+        mock_out = MagicMock(side_effect=['', err])
+        mock_path = MagicMock(return_value=path)
+        mock_link = MagicMock(return_value=link)
         with patch.dict(alternatives.__salt__,
                         {'alternatives.check_installed': mock,
-                         'alternatives.install': mock_bool}):
+                         'alternatives.install': mock_out,
+                         'alternatives.show_current': mock_path,
+                         'alternatives.show_link': mock_link}):
             comt = ('Alternatives for {0} is already set to {1}'
-                    ).format(name, path)
+                   ).format(name, path)
             ret.update({'comment': comt, 'result': True})
             self.assertDictEqual(alternatives.install(name, link, path,
                                                       priority), ret)
 
             comt = (('Alternative will be set for {0} to {1} with priority {2}'
-                     ).format(name, path, priority))
+                    ).format(name, path, priority))
             ret.update({'comment': comt, 'result': None})
             with patch.dict(alternatives.__opts__, {'test': True}):
                 self.assertDictEqual(alternatives.install(name, link, path,
                                                           priority), ret)
 
-            comt = ('Setting alternative for {0} to {1} with priority {2}'
-                    ).format(name, path, priority)
+            comt = ('Alternative for {0} set to path {1} with priority {2}'
+                   ).format(name, path, priority)
             ret.update({'comment': comt, 'result': True,
                         'changes': {'name': name, 'link': link, 'path': path,
                                     'priority': priority}})
@@ -76,6 +83,14 @@ class AlternativesTestCase(TestCase):
                 self.assertDictEqual(alternatives.install(name, link, path,
                                                           priority), ret)
 
+            comt = ('Alternative for {0} not installed: {1}'
+                   ).format(name, err)
+            ret.update({'comment': comt, 'result': False,
+                        'changes': {}, 'link': bad_link})
+            with patch.dict(alternatives.__opts__, {'test': False}):
+                self.assertDictEqual(alternatives.install(name, bad_link, path,
+                                                          priority), ret)
+
     # 'remove' function tests: 1
 
     def test_remove(self):
@@ -94,10 +109,10 @@ class AlternativesTestCase(TestCase):
 
         mock = MagicMock(side_effect=[True, True, True, False, False])
         mock_bool = MagicMock(return_value=True)
-        mock_bol = MagicMock(side_effect=[False, True, True, False])
+        mock_show = MagicMock(side_effect=[False, True, True, False])
         with patch.dict(alternatives.__salt__,
                         {'alternatives.check_exists': mock,
-                         'alternatives.show_current': mock_bol,
+                         'alternatives.show_current': mock_show,
                          'alternatives.remove': mock_bool}):
             comt = ('Alternative for {0} will be removed'.format(name))
             ret.update({'comment': comt})
@@ -116,7 +131,7 @@ class AlternativesTestCase(TestCase):
                 self.assertDictEqual(alternatives.remove(name, path), ret)
 
             comt = ('Alternative for {0} is set to it\'s default path True'
-                    ).format(name)
+                   ).format(name)
             ret.update({'comment': comt, 'result': True, 'changes': {}})
             self.assertDictEqual(alternatives.remove(name, path), ret)
 
@@ -175,17 +190,17 @@ class AlternativesTestCase(TestCase):
 
         mock = MagicMock(side_effect=[path, path, ''])
         mock_bool = MagicMock(return_value=True)
-        mock_bol = MagicMock(side_effect=[path, False, False, False, False])
+        mock_show = MagicMock(side_effect=[path, False, False, False, False])
         with patch.dict(alternatives.__salt__,
                         {'alternatives.display': mock,
-                         'alternatives.show_current': mock_bol,
+                         'alternatives.show_current': mock_show,
                          'alternatives.set': mock_bool}):
             comt = ('Alternative for {0} already set to {1}'.format(name, path))
             ret.update({'comment': comt})
             self.assertDictEqual(alternatives.set_(name, path), ret)
 
             comt = ('Alternative for {0} will be set to path False'
-                    ).format(name)
+                   ).format(name)
             ret.update({'comment': comt, 'result': None})
             with patch.dict(alternatives.__opts__, {'test': True}):
                 self.assertDictEqual(alternatives.set_(name, path), ret)