Skip to content

Commit

Permalink
2019.2.0 PR 54196 backport (#173)
Browse files Browse the repository at this point in the history 
* virt.network_define doesn't have vport as positional argument

virt.network_running state calls virt.network_define with vport as a
positional argument resulting in an error at runtime. Fix the state to
use the vport named argument instead.

* Fix virt.pool_running state documentation

virt.pool_running needs the source to be a dictionary, which the
documentation was not reflecting. Along the same lines the source hosts
need to be a list, adjust the example to show it.

* Get virt.pool_running to start the pool after creating it

Commit 25b9681 is wrong in assuming the pool build also starts it. The
pool needs to be stopped before building it, but we still need to start
it after the build: libvirt won't do it automagically for us.

* Fix states to match virt.{network,pool}_infos return

virt.network_infos and virt.pool_infos return the infos as a dictionary
with the network or pool name as a key even when there is only one
value. Adapt the network_running and pool_running states to this.

* Fix virt.running use of virt.vm_state

vm_state return a dictionary with the VM name as a key. Fix virt.running
state and its tests to match this. See issue #53107.
  • Loading branch information
@cbosdo @brejoc
cbosdo authored and brejoc committed Sep 3, 2019
1 parent 76d0ec5 commit 3119bc2
Show file tree
Hide file tree
Showing 2 changed files with 31 additions and 22 deletions.
26 changes: 16 additions & 10 deletions salt/states/virt.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -389,8 +389,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, None) != 'running':
action_msg = 'started'
if update:
status = __salt__['virt.update'](name,
Expand Down Expand Up @@ -670,7 +670,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 +680,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 +744,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 +761,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 +795,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
27 changes: 15 additions & 12 deletions tests/unit/states/test_virt.py
View file
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 Down Expand Up @@ -573,7 +573,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 +582,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 +666,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 +683,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 3119bc2

Please sign in to comment.