ddaff062ff
OBS-URL: https://build.opensuse.org/package/show/systemsmanagement:saltstack/salt?expand=0&rev=159
218 lines
11 KiB
Diff
218 lines
11 KiB
Diff
From 46aeb732ed61051d436ac748bd95a50c6379682a 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
|
|
|
|
|