Merge pull request #30628 from rallytime/fix-25658

Refactor rabbitmq_policy states to use test=true functionality correctly
saltstack · Jan 26, 2016 · ef6c4e8 · ef6c4e8
2 parents 80d0e42 + 1e8e860
commit ef6c4e8
Show file tree

Hide file tree

Showing 2 changed files with 49 additions and 36 deletions.
diff --git a/salt/states/rabbitmq_policy.py b/salt/states/rabbitmq_policy.py
@@ -75,33 +75,40 @@ def present(name,
         ret['comment'] = 'Policy {0} {1} is already present'.format(vhost, name)
         return ret
 
-    if __opts__['test']:
-        ret['result'] = None
-        if not policy:
+    if not policy:
+        ret['changes'].update({'old': {}, 'new': name})
+        if __opts__['test']:
             ret['comment'] = 'Policy {0} {1} is set to be created'.format(vhost, name)
-        elif updates:
+        else:
+            log.debug('Policy doesn\'t exist - Creating')
+            result = __salt__['rabbitmq.set_policy'](vhost,
+                                                     name,
+                                                     pattern,
+                                                     definition,
+                                                     priority=priority,
+                                                     runas=runas)
+    elif updates:
+        ret['changes'].update({'old': policy, 'new': updates})
+        if __opts__['test']:
             ret['comment'] = 'Policy {0} {1} is set to be updated'.format(vhost, name)
-    else:
-        changes = {'new': '', 'old': ''}
-        if not policy:
-            changes['new'] = policy
-            log.debug(
-                "Policy doesn't exist - Creating")
-            result = __salt__['rabbitmq.set_policy'](
-                vhost, name, pattern, definition, priority=priority, runas=runas)
-        elif updates:
-            changes['old'] = policy
-            changes['new'] = updates
+        else:
             log.debug('Policy exists but needs updating')
-            result = __salt__['rabbitmq.set_policy'](
-                vhost, name, pattern, definition, priority=priority, runas=runas)
-
-        if 'Error' in result:
-            ret['result'] = False
-            ret['comment'] = result['Error']
-        elif 'Set' in result:
-            ret['comment'] = result['Set']
-            ret['changes'] = changes
+            result = __salt__['rabbitmq.set_policy'](vhost,
+                                                     name,
+                                                     pattern,
+                                                     definition,
+                                                     priority=priority,
+                                                     runas=runas)
+
+    if 'Error' in result:
+        ret['result'] = False
+        ret['comment'] = result['Error']
+    elif ret['changes'] == {}:
+        ret['comment'] = '\'{0}\' is already in the desired state.'.format(name)
+    elif __opts__['test']:
+        ret['result'] = None
+    elif 'Set' in result:
+        ret['comment'] = result['Set']
 
     return ret
 
@@ -125,19 +132,23 @@ def absent(name,
         vhost, name, runas=runas)
 
     if not policy_exists:
-        ret['comment'] = 'Policy {0} {1} is not present'.format(vhost, name)
+        ret['comment'] = 'Policy \'{0} {1}\' is not present.'.format(vhost, name)
         return ret
 
-    if __opts__['test']:
-        ret['result'] = None
-        ret['comment'] = 'Removing policy {0} {1}'.format(vhost, name)
-    else:
+    if not __opts__['test']:
         result = __salt__['rabbitmq.delete_policy'](vhost, name, runas=runas)
         if 'Error' in result:
             ret['result'] = False
             ret['comment'] = result['Error']
+            return ret
         elif 'Deleted' in result:
             ret['comment'] = 'Deleted'
-            ret['changes'] = {'new': '', 'old': name}
+
+    # If we've reached this far before returning, we have changes.
+    ret['changes'] = {'new': '', 'old': name}
+
+    if __opts__['test']:
+        ret['result'] = None
+        ret['comment'] = 'Policy \'{0} {1}\' will be removed.'.format(vhost, name)
 
     return ret
diff --git a/tests/unit/states/rabbitmq_policy_test.py b/tests/unit/states/rabbitmq_policy_test.py
@@ -56,8 +56,9 @@ def test_present(self):
                                                          definition), ret)
 
             with patch.dict(rabbitmq_policy.__opts__, {'test': True}):
-                comt = ('Policy / HA is set to be created')
-                ret.update({'comment': comt, 'result': None})
+                comment = 'Policy / HA is set to be created'
+                changes = {'new': 'HA', 'old': {}}
+                ret.update({'comment': comment, 'result': None, 'changes': changes})
                 self.assertDictEqual(rabbitmq_policy.present(name, pattern,
                                                              definition), ret)
 
@@ -77,13 +78,14 @@ def test_absent(self):
         mock = MagicMock(side_effect=[False, True])
         with patch.dict(rabbitmq_policy.__salt__,
                         {'rabbitmq.policy_exists': mock}):
-            comt = ('Policy / HA is not present')
-            ret.update({'comment': comt})
+            comment = 'Policy \'/ HA\' is not present.'
+            ret.update({'comment': comment})
             self.assertDictEqual(rabbitmq_policy.absent(name), ret)
 
             with patch.dict(rabbitmq_policy.__opts__, {'test': True}):
-                comt = ('Removing policy / HA')
-                ret.update({'comment': comt, 'result': None})
+                comment = 'Policy \'/ HA\' will be removed.'
+                changes = {'new': '', 'old': 'HA'}
+                ret.update({'comment': comment, 'result': None, 'changes': changes})
                 self.assertDictEqual(rabbitmq_policy.absent(name), ret)