File fix-virt-states-to-not-fail-on-vms-already-stopped.-.patch of Package salt.14889
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
