File fix-virt-states-to-not-fail-on-vms-already-stop... of Package salt.14890

Overview Repositories Revisions Requests Users Attributes Meta

File fix-virt-states-to-not-fail-on-vms-already-stopped.-.patch of Package salt.14890

From de0b7d8eaf50813008533afc66f4ddef75f0456d Mon Sep 17 00:00:00 2001
From: Cedric Bosdonnat <cbosdonnat@suse.com>
Date: Mon, 16 Dec 2019 11:27:49 +0100
Subject: [PATCH] Fix virt states to not fail on VMs already stopped.
 (#195)

The virt.stopped and virt.powered_off states need to do nothing and
not fail if one of the targeted VMs is already in shutdown state.
---
 salt/states/virt.py            | 45 ++++++++++++++++++++--------------
 tests/unit/states/test_virt.py | 36 +++++++++++++++++++++++++++
 2 files changed, 63 insertions(+), 18 deletions(-)

diff --git a/salt/states/virt.py b/salt/states/virt.py
index 32a9e31ae5..68e9ac6fb6 100644
--- a/salt/states/virt.py
+++ b/salt/states/virt.py
@@ -145,35 +145,45 @@ def keys(name, basepath='/etc/pki', **kwargs):
     return ret
 
 
-def _virt_call(domain, function, section, comment,
+def _virt_call(domain, function, section, comment, state=None,
                connection=None, username=None, password=None, **kwargs):
     '''
     Helper to call the virt functions. Wildcards supported.
 
-    :param domain:
-    :param function:
-    :param section:
-    :param comment:
-    :return:
+    :param domain: the domain to apply the function on. Can contain wildcards.
+    :param function: virt function to call
+    :param section: key for the changed domains in the return changes dictionary
+    :param comment: comment to return
+    :param state: the expected final state of the VM. If None the VM state won't be checked.
+    :return: the salt state return
     '''
     ret = {'name': domain, 'changes': {}, 'result': True, 'comment': ''}
     targeted_domains = fnmatch.filter(__salt__['virt.list_domains'](), domain)
     changed_domains = list()
     ignored_domains = list()
+    noaction_domains = list()
     for targeted_domain in targeted_domains:
         try:
-            response = __salt__['virt.{0}'.format(function)](targeted_domain,
-                                                             connection=connection,
-                                                             username=username,
-                                                             password=password,
-                                                             **kwargs)
-            if isinstance(response, dict):
-                response = response['name']
-            changed_domains.append({'domain': targeted_domain, function: response})
+            action_needed = True
+            # If a state has been provided, use it to see if we have something to do
+            if state is not None:
+                domain_state = __salt__['virt.vm_state'](targeted_domain)
+                action_needed = domain_state.get(targeted_domain) != state
+            if action_needed:
+                response = __salt__['virt.{0}'.format(function)](targeted_domain,
+                                                                 connection=connection,
+                                                                 username=username,
+                                                                 password=password,
+                                                                 **kwargs)
+                if isinstance(response, dict):
+                    response = response['name']
+                changed_domains.append({'domain': targeted_domain, function: response})
+            else:
+                noaction_domains.append(targeted_domain)
         except libvirt.libvirtError as err:
             ignored_domains.append({'domain': targeted_domain, 'issue': six.text_type(err)})
     if not changed_domains:
-        ret['result'] = False
+        ret['result'] = not ignored_domains and bool(targeted_domains)
         ret['comment'] = 'No changes had happened'
         if ignored_domains:
             ret['changes'] = {'ignored': ignored_domains}
@@ -206,7 +216,7 @@ def stopped(name, connection=None, username=None, password=None):
           virt.stopped
     '''
 
-    return _virt_call(name, 'shutdown', 'stopped', "Machine has been shut down",
+    return _virt_call(name, 'shutdown', 'stopped', 'Machine has been shut down', state='shutdown',
                       connection=connection, username=username, password=password)
 
 
@@ -231,8 +241,7 @@ def powered_off(name, connection=None, username=None, password=None):
         domain_name:
           virt.stopped
     '''
-
-    return _virt_call(name, 'stop', 'unpowered', 'Machine has been powered off',
+    return _virt_call(name, 'stop', 'unpowered', 'Machine has been powered off', state='shutdown',
                       connection=connection, username=username, password=password)
 
 
diff --git a/tests/unit/states/test_virt.py b/tests/unit/states/test_virt.py
index 2904fa224d..2af5caca1b 100644
--- a/tests/unit/states/test_virt.py
+++ b/tests/unit/states/test_virt.py
@@ -378,8 +378,11 @@ class LibvirtTestCase(TestCase, LoaderModuleMockMixin):
                'result': True}
 
         shutdown_mock = MagicMock(return_value=True)
+
+        # Normal case
         with patch.dict(virt.__salt__, {  # pylint: disable=no-member
                     'virt.list_domains': MagicMock(return_value=['myvm', 'vm1']),
+                    'virt.vm_state': MagicMock(return_value={'myvm': 'running'}),
                     'virt.shutdown': shutdown_mock
                 }):
             ret.update({'changes': {
@@ -389,8 +392,10 @@ class LibvirtTestCase(TestCase, LoaderModuleMockMixin):
             self.assertDictEqual(virt.stopped('myvm'), ret)
             shutdown_mock.assert_called_with('myvm', connection=None, username=None, password=None)
 
+        # Normal case with user-provided connection parameters
         with patch.dict(virt.__salt__, {  # pylint: disable=no-member
                     'virt.list_domains': MagicMock(return_value=['myvm', 'vm1']),
+                    'virt.vm_state': MagicMock(return_value={'myvm': 'running'}),
                     'virt.shutdown': shutdown_mock,
                 }):
             self.assertDictEqual(virt.stopped('myvm',
@@ -399,8 +404,10 @@ class LibvirtTestCase(TestCase, LoaderModuleMockMixin):
                                               password='secret'), ret)
             shutdown_mock.assert_called_with('myvm', connection='myconnection', username='user', password='secret')
 
+        # Case where an error occurred during the shutdown
         with patch.dict(virt.__salt__, {  # pylint: disable=no-member
                     'virt.list_domains': MagicMock(return_value=['myvm', 'vm1']),
+                    'virt.vm_state': MagicMock(return_value={'myvm': 'running'}),
                     'virt.shutdown': MagicMock(side_effect=self.mock_libvirt.libvirtError('Some error'))
                 }):
             ret.update({'changes': {'ignored': [{'domain': 'myvm', 'issue': 'Some error'}]},
@@ -408,10 +415,21 @@ class LibvirtTestCase(TestCase, LoaderModuleMockMixin):
                         'comment': 'No changes had happened'})
             self.assertDictEqual(virt.stopped('myvm'), ret)
 
+        # Case there the domain doesn't exist
         with patch.dict(virt.__salt__, {'virt.list_domains': MagicMock(return_value=[])}):  # pylint: disable=no-member
             ret.update({'changes': {}, 'result': False, 'comment': 'No changes had happened'})
             self.assertDictEqual(virt.stopped('myvm'), ret)
 
+        # Case where the domain is already stopped
+        with patch.dict(virt.__salt__, {  # pylint: disable=no-member
+                    'virt.list_domains': MagicMock(return_value=['myvm', 'vm1']),
+                    'virt.vm_state': MagicMock(return_value={'myvm': 'shutdown'})
+                }):
+            ret.update({'changes': {},
+                        'result': True,
+                        'comment': 'No changes had happened'})
+            self.assertDictEqual(virt.stopped('myvm'), ret)
+
     def test_powered_off(self):
         '''
         powered_off state test cases.
@@ -421,8 +439,11 @@ class LibvirtTestCase(TestCase, LoaderModuleMockMixin):
                'result': True}
 
         stop_mock = MagicMock(return_value=True)
+
+        # Normal case
         with patch.dict(virt.__salt__, {  # pylint: disable=no-member
                     'virt.list_domains': MagicMock(return_value=['myvm', 'vm1']),
+                    'virt.vm_state': MagicMock(return_value={'myvm': 'running'}),
                     'virt.stop': stop_mock
                 }):
             ret.update({'changes': {
@@ -432,8 +453,10 @@ class LibvirtTestCase(TestCase, LoaderModuleMockMixin):
             self.assertDictEqual(virt.powered_off('myvm'), ret)
             stop_mock.assert_called_with('myvm', connection=None, username=None, password=None)
 
+        # Normal case with user-provided connection parameters
         with patch.dict(virt.__salt__, {  # pylint: disable=no-member
                     'virt.list_domains': MagicMock(return_value=['myvm', 'vm1']),
+                    'virt.vm_state': MagicMock(return_value={'myvm': 'running'}),
                     'virt.stop': stop_mock,
                 }):
             self.assertDictEqual(virt.powered_off('myvm',
@@ -442,8 +465,10 @@ class LibvirtTestCase(TestCase, LoaderModuleMockMixin):
                                                   password='secret'), ret)
             stop_mock.assert_called_with('myvm', connection='myconnection', username='user', password='secret')
 
+        # Case where an error occurred during the poweroff
         with patch.dict(virt.__salt__, {  # pylint: disable=no-member
                     'virt.list_domains': MagicMock(return_value=['myvm', 'vm1']),
+                    'virt.vm_state': MagicMock(return_value={'myvm': 'running'}),
                     'virt.stop': MagicMock(side_effect=self.mock_libvirt.libvirtError('Some error'))
                 }):
             ret.update({'changes': {'ignored': [{'domain': 'myvm', 'issue': 'Some error'}]},
@@ -451,10 +476,21 @@ class LibvirtTestCase(TestCase, LoaderModuleMockMixin):
                         'comment': 'No changes had happened'})
             self.assertDictEqual(virt.powered_off('myvm'), ret)
 
+        # Case there the domain doesn't exist
         with patch.dict(virt.__salt__, {'virt.list_domains': MagicMock(return_value=[])}):  # pylint: disable=no-member
             ret.update({'changes': {}, 'result': False, 'comment': 'No changes had happened'})
             self.assertDictEqual(virt.powered_off('myvm'), ret)
 
+        # Case where the domain is already stopped
+        with patch.dict(virt.__salt__, {  # pylint: disable=no-member
+                    'virt.list_domains': MagicMock(return_value=['myvm', 'vm1']),
+                    'virt.vm_state': MagicMock(return_value={'myvm': 'shutdown'})
+                }):
+            ret.update({'changes': {},
+                        'result': True,
+                        'comment': 'No changes had happened'})
+            self.assertDictEqual(virt.powered_off('myvm'), ret)
+
     def test_snapshot(self):
         '''
         snapshot state test cases.
-- 
2.23.0

Places

File fix-virt-states-to-not-fail-on-vms-already-stopped.-.patch of Package salt.14890

Places