Skip to content

Commit

Permalink
Merge pull request #54196 from cbosdo/2019.2.1-virt-state-fixes
Browse files Browse the repository at this point in the history 
virt state fixes
  • Loading branch information
@dwoz
dwoz authored Dec 14, 2019
2 parents 18acbd5 + 991ef2c commit 882a832
Show file tree
Hide file tree
Showing 2 changed files with 94 additions and 40 deletions.
71 changes: 43 additions & 28 deletions salt/states/virt.py
View file
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
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,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 @@ -319,15 +319,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 @@ -337,7 +337,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 @@ -348,7 +348,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 @@ -358,7 +358,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 @@ -375,8 +375,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 @@ -386,8 +389,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 @@ -396,19 +401,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 @@ -418,8 +436,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 @@ -429,8 +450,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 @@ -439,19 +462,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 @@ -570,7 +606,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 @@ -579,15 +615,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 @@ -663,10 +699,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 @@ -677,7 +716,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

0 comments on commit 882a832

Please sign in to comment.