Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

virt state fixes #54196

Merged
merged 6 commits into from
Dec 14, 2019
Merged

virt state fixes #54196

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
51443a1
virt.network_define doesn't have vport as positional argument
cbosdo Aug 12, 2019
a7414f5
Fix virt.pool_running state documentation
cbosdo Aug 12, 2019
547f827
Get virt.pool_running to start the pool after creating it
cbosdo Aug 12, 2019
23a3fe5
Fix states to match virt.{network,pool}_infos return
cbosdo Aug 13, 2019
8965d47
Fix virt.running use of virt.vm_state
cbosdo Aug 13, 2019
991ef2c
Fix virt states to not fail on VMs already stopped.
cbosdo Oct 25, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
71 changes: 43 additions & 28 deletions salt/states/virt.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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}
Expand Down Expand Up @@ -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)


Expand All @@ -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)


Expand Down Expand Up @@ -389,8 +398,8 @@ def running(name,

try:
try:
__salt__['virt.vm_state'](name)
if __salt__['virt.vm_state'](name) != 'running':
domain_state = __salt__['virt.vm_state'](name)
if domain_state.get(name) != 'running':
action_msg = 'started'
if update:
status = __salt__['virt.update'](name,
Expand Down Expand Up @@ -670,7 +679,7 @@ def network_running(name,
try:
info = __salt__['virt.network_info'](name, connection=connection, username=username, password=password)
if info:
if info['active']:
if info[name]['active']:
ret['comment'] = 'Network {0} exists and is running'.format(name)
else:
__salt__['virt.network_start'](name, connection=connection, username=username, password=password)
Expand All @@ -680,7 +689,7 @@ def network_running(name,
__salt__['virt.network_define'](name,
bridge,
forward,
vport,
vport=vport,
tag=tag,
autostart=autostart,
start=True,
Expand Down Expand Up @@ -744,11 +753,11 @@ def pool_running(name,
- owner: 1000
- group: 100
- source:
- dir: samba_share
- hosts:
one.example.com
two.example.com
- format: cifs
dir: samba_share
hosts:
- one.example.com
- two.example.com
format: cifs
- autostart: True

'''
Expand All @@ -761,7 +770,7 @@ def pool_running(name,
try:
info = __salt__['virt.pool_info'](name, connection=connection, username=username, password=password)
if info:
if info['state'] == 'running':
if info[name]['state'] == 'running':
ret['comment'] = 'Pool {0} exists and is running'.format(name)
else:
__salt__['virt.pool_start'](name, connection=connection, username=username, password=password)
Expand Down Expand Up @@ -795,6 +804,12 @@ def pool_running(name,
connection=connection,
username=username,
password=password)

__salt__['virt.pool_start'](name,
connection=connection,
username=username,
password=password)

ret['changes'][name] = 'Pool defined and started'
ret['comment'] = 'Pool {0} defined and started'.format(name)
except libvirt.libvirtError as err:
Expand Down
63 changes: 51 additions & 12 deletions tests/unit/states/test_virt.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -229,7 +229,7 @@ def test_running(self):
'result': True,
'comment': 'myvm is running'}
with patch.dict(virt.__salt__, { # pylint: disable=no-member
'virt.vm_state': MagicMock(return_value='stopped'),
'virt.vm_state': MagicMock(return_value={'myvm': 'stopped'}),
'virt.start': MagicMock(return_value=0),
}):
ret.update({'changes': {'myvm': 'Domain started'},
Expand Down Expand Up @@ -322,15 +322,15 @@ def test_running(self):
password='supersecret')

with patch.dict(virt.__salt__, { # pylint: disable=no-member
'virt.vm_state': MagicMock(return_value='stopped'),
'virt.vm_state': MagicMock(return_value={'myvm': 'stopped'}),
'virt.start': MagicMock(side_effect=[self.mock_libvirt.libvirtError('libvirt error msg')])
}):
ret.update({'changes': {}, 'result': False, 'comment': 'libvirt error msg'})
self.assertDictEqual(virt.running('myvm'), ret)

# Working update case when running
with patch.dict(virt.__salt__, { # pylint: disable=no-member
'virt.vm_state': MagicMock(return_value='running'),
'virt.vm_state': MagicMock(return_value={'myvm': 'running'}),
'virt.update': MagicMock(return_value={'definition': True, 'cpu': True})
}):
ret.update({'changes': {'myvm': {'definition': True, 'cpu': True}},
Expand All @@ -340,7 +340,7 @@ def test_running(self):

# Working update case when stopped
with patch.dict(virt.__salt__, { # pylint: disable=no-member
'virt.vm_state': MagicMock(return_value='stopped'),
'virt.vm_state': MagicMock(return_value={'myvm': 'stopped'}),
'virt.start': MagicMock(return_value=0),
'virt.update': MagicMock(return_value={'definition': True})
}):
Expand All @@ -351,7 +351,7 @@ def test_running(self):

# Failed live update case
with patch.dict(virt.__salt__, { # pylint: disable=no-member
'virt.vm_state': MagicMock(return_value='running'),
'virt.vm_state': MagicMock(return_value={'myvm': 'running'}),
'virt.update': MagicMock(return_value={'definition': True, 'cpu': False, 'errors': ['some error']})
}):
ret.update({'changes': {'myvm': {'definition': True, 'cpu': False, 'errors': ['some error']}},
Expand All @@ -361,7 +361,7 @@ def test_running(self):

# Failed definition update case
with patch.dict(virt.__salt__, { # pylint: disable=no-member
'virt.vm_state': MagicMock(return_value='running'),
'virt.vm_state': MagicMock(return_value={'myvm': 'running'}),
'virt.update': MagicMock(side_effect=[self.mock_libvirt.libvirtError('error message')])
}):
ret.update({'changes': {},
Expand All @@ -378,8 +378,11 @@ def test_stopped(self):
'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': {
Expand All @@ -389,8 +392,10 @@ def test_stopped(self):
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',
Expand All @@ -399,19 +404,32 @@ def test_stopped(self):
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'}]},
'result': False,
'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.
Expand All @@ -421,8 +439,11 @@ def test_powered_off(self):
'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': {
Expand All @@ -432,8 +453,10 @@ def test_powered_off(self):
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',
Expand All @@ -442,19 +465,32 @@ def test_powered_off(self):
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'}]},
'result': False,
'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.
Expand Down Expand Up @@ -573,7 +609,7 @@ def test_network_running(self):
define_mock.assert_called_with('mynet',
'br2',
'bridge',
'openvswitch',
vport='openvswitch',
tag=180,
autostart=False,
start=True,
Expand All @@ -582,15 +618,15 @@ def test_network_running(self):
password='secret')

with patch.dict(virt.__salt__, { # pylint: disable=no-member
'virt.network_info': MagicMock(return_value={'active': True}),
'virt.network_info': MagicMock(return_value={'mynet': {'active': True}}),
'virt.network_define': define_mock,
}):
ret.update({'changes': {}, 'comment': 'Network mynet exists and is running'})
self.assertDictEqual(virt.network_running('mynet', 'br2', 'bridge'), ret)

start_mock = MagicMock(return_value=True)
with patch.dict(virt.__salt__, { # pylint: disable=no-member
'virt.network_info': MagicMock(return_value={'active': False}),
'virt.network_info': MagicMock(return_value={'mynet': {'active': False}}),
'virt.network_start': start_mock,
'virt.network_define': define_mock,
}):
Expand Down Expand Up @@ -666,10 +702,13 @@ def test_pool_running(self):
connection='myconnection',
username='user',
password='secret')
mocks['start'].assert_not_called()
mocks['start'].assert_called_with('mypool',
connection='myconnection',
username='user',
password='secret')

with patch.dict(virt.__salt__, { # pylint: disable=no-member
'virt.pool_info': MagicMock(return_value={'state': 'running'}),
'virt.pool_info': MagicMock(return_value={'mypool': {'state': 'running'}}),
}):
ret.update({'changes': {}, 'comment': 'Pool mypool exists and is running'})
self.assertDictEqual(virt.pool_running('mypool',
Expand All @@ -680,7 +719,7 @@ def test_pool_running(self):
for mock in mocks:
mocks[mock].reset_mock()
with patch.dict(virt.__salt__, { # pylint: disable=no-member
'virt.pool_info': MagicMock(return_value={'state': 'stopped'}),
'virt.pool_info': MagicMock(return_value={'mypool': {'state': 'stopped'}}),
'virt.pool_build': mocks['build'],
'virt.pool_start': mocks['start']
}):
Expand Down