openstack/nova
Code Issues Proposed changes

Merge "Ironic: Get IP address for volume connector"

Browse Source
This commit is contained in:
Zuul 2018-01-27 22:05:30 +00:00 committed by Gerrit Code Review
commit 55ea961cef
7 changed files with 275 additions and 28 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

9
nova/compute/manager.py
View File

@ -2186,6 +2186,11 @@ class ComputeManager(manager.Manager):
reason=msg)
try:
# Depending on a virt driver, some network configuration is
# necessary before preparing block devices.
self.driver.prepare_networks_before_block_device_mapping(
instance, network_info)
# Verify that all the BDMs have a device_name set and assign a
# default to the ones missing it with the help of the driver.
self._default_block_device_names(instance, image_meta,
@ -2206,11 +2211,14 @@ class ComputeManager(manager.Manager):
# Make sure the async call finishes
if network_info is not None:
network_info.wait(do_raise=False)
self.driver.clean_networks_preparation(instance,
network_info)
except (exception.UnexpectedTaskStateError,
exception.OverQuota, exception.InvalidBDM) as e:
# Make sure the async call finishes
if network_info is not None:
network_info.wait(do_raise=False)
self.driver.clean_networks_preparation(instance, network_info)
raise exception.BuildAbortException(instance_uuid=instance.uuid,
reason=e.format_message())
except Exception:
@ -2219,6 +2227,7 @@ class ComputeManager(manager.Manager):
# Make sure the async call finishes
if network_info is not None:
network_info.wait(do_raise=False)
self.driver.clean_networks_preparation(instance, network_info)
msg = _('Failure prepping block device.')
raise exception.BuildAbortException(instance_uuid=instance.uuid,
reason=msg)

28
nova/tests/unit/compute/test_compute_mgr.py
View File

@ -5322,8 +5322,12 @@ class ComputeManagerBuildInstanceTestCase(test.NoDBTestCase):
@mock.patch.object(objects.Instance, 'save')
@mock.patch.object(manager.ComputeManager, '_build_networks_for_instance')
@mock.patch.object(manager.ComputeManager, '_prep_block_device')
def test_build_resources_reraises_on_failed_bdm_prep(self, mock_prep,
mock_build, mock_save):
@mock.patch.object(virt_driver.ComputeDriver,
'prepare_networks_before_block_device_mapping')
@mock.patch.object(virt_driver.ComputeDriver,
'clean_networks_preparation')
def test_build_resources_reraises_on_failed_bdm_prep(
self, mock_clean, mock_prepnet, mock_prep, mock_build, mock_save):
mock_save.return_value = self.instance
mock_build.return_value = self.network_info
mock_prep.side_effect = test.TestingException
@ -5341,6 +5345,8 @@ class ComputeManagerBuildInstanceTestCase(test.NoDBTestCase):
self.requested_networks, self.security_groups)
mock_prep.assert_called_once_with(self.context, self.instance,
self.block_device_mapping)
mock_prepnet.assert_called_once_with(self.instance, self.network_info)
mock_clean.assert_called_once_with(self.instance, self.network_info)
@mock.patch('nova.virt.block_device.attach_block_devices',
side_effect=exception.VolumeNotCreated('oops!'))
@ -5394,7 +5400,12 @@ class ComputeManagerBuildInstanceTestCase(test.NoDBTestCase):
instance, hints)
mock_get.assert_called_once_with(self.context, uuids.group_hint)
def test_failed_bdm_prep_from_delete_raises_unexpected(self):
@mock.patch.object(virt_driver.ComputeDriver,
'prepare_networks_before_block_device_mapping')
@mock.patch.object(virt_driver.ComputeDriver,
'clean_networks_preparation')
def test_failed_bdm_prep_from_delete_raises_unexpected(self, mock_clean,
mock_prepnet):
with test.nested(
mock.patch.object(self.compute,
'_build_networks_for_instance',
@ -5420,6 +5431,8 @@ class ComputeManagerBuildInstanceTestCase(test.NoDBTestCase):
self.requested_networks, self.security_groups)])
save.assert_has_calls([mock.call()])
mock_prepnet.assert_called_once_with(self.instance, self.network_info)
mock_clean.assert_called_once_with(self.instance, self.network_info)
@mock.patch.object(manager.ComputeManager, '_build_networks_for_instance')
def test_build_resources_aborts_on_failed_network_alloc(self, mock_build):
@ -5531,8 +5544,13 @@ class ComputeManagerBuildInstanceTestCase(test.NoDBTestCase):
@mock.patch(
'nova.compute.manager.ComputeManager._build_networks_for_instance')
@mock.patch('nova.objects.Instance.save')
@mock.patch.object(virt_driver.ComputeDriver,
'prepare_networks_before_block_device_mapping')
@mock.patch.object(virt_driver.ComputeDriver,
'clean_networks_preparation')
def test_build_resources_unexpected_task_error_before_yield(
self, mock_save, mock_build_network, mock_info_wait):
self, mock_clean, mock_prepnet, mock_save, mock_build_network,
mock_info_wait):
mock_build_network.return_value = self.network_info
mock_save.side_effect = exception.UnexpectedTaskStateError(
instance_uuid=uuids.instance, expected={}, actual={})
@ -5546,6 +5564,8 @@ class ComputeManagerBuildInstanceTestCase(test.NoDBTestCase):
mock_build_network.assert_called_once_with(self.context, self.instance,
self.requested_networks, self.security_groups)
mock_info_wait.assert_called_once_with(do_raise=False)
mock_prepnet.assert_called_once_with(self.instance, self.network_info)
mock_clean.assert_called_once_with(self.instance, self.network_info)
@mock.patch('nova.network.model.NetworkInfoAsyncWrapper.wait')
@mock.patch(

116
nova/tests/unit/virt/ironic/test_driver.py
View File

@ -32,6 +32,7 @@ from nova.compute import vm_states
from nova.console import type as console_type
from nova import context as nova_context
from nova import exception
from nova.network import model as network_model
from nova import objects
from nova.objects import fields
from nova import servicegroup
@ -1030,9 +1031,8 @@ class IronicDriverTestCase(test.NoDBTestCase):
@mock.patch.object(ironic_driver.IronicDriver, '_wait_for_active')
@mock.patch.object(ironic_driver.IronicDriver,
'_add_instance_info_to_node')
@mock.patch.object(ironic_driver.IronicDriver, '_plug_vifs')
@mock.patch.object(ironic_driver.IronicDriver, '_start_firewall')
def _test_spawn(self, mock_sf, mock_pvifs, mock_aiitn, mock_wait_active,
def _test_spawn(self, mock_sf, mock_aiitn, mock_wait_active,
mock_avti, mock_node, mock_looping, mock_save):
node_uuid = 'aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee'
node = ironic_utils.get_test_node(driver='fake', uuid=node_uuid)
@ -1059,7 +1059,6 @@ class IronicDriverTestCase(test.NoDBTestCase):
test.MatchType(objects.ImageMeta),
fake_flavor, block_device_info=None)
mock_avti.assert_called_once_with(self.ctx, instance, None)
mock_pvifs.assert_called_once_with(node, instance, None)
mock_sf.assert_called_once_with(instance, None)
mock_node.set_provision_state.assert_called_once_with(node_uuid,
'active', configdrive=mock.ANY)
@ -1099,11 +1098,10 @@ class IronicDriverTestCase(test.NoDBTestCase):
@mock.patch.object(ironic_driver.IronicDriver, '_wait_for_active')
@mock.patch.object(ironic_driver.IronicDriver,
'_add_instance_info_to_node')
@mock.patch.object(ironic_driver.IronicDriver, '_plug_vifs')
@mock.patch.object(ironic_driver.IronicDriver, '_start_firewall')
def test_spawn_destroyed_after_failure(self, mock_sf, mock_pvifs,
mock_aiitn, mock_wait_active,
mock_avti, mock_destroy, mock_node,
def test_spawn_destroyed_after_failure(self, mock_sf, mock_aiitn,
mock_wait_active, mock_avti,
mock_destroy, mock_node,
mock_looping, mock_required_by):
mock_required_by.return_value = False
node_uuid = 'aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee'
@ -1356,10 +1354,9 @@ class IronicDriverTestCase(test.NoDBTestCase):
@mock.patch.object(FAKE_CLIENT, 'node')
@mock.patch.object(ironic_driver.IronicDriver, '_add_volume_target_info')
@mock.patch.object(ironic_driver.IronicDriver, '_start_firewall')
@mock.patch.object(ironic_driver.IronicDriver, '_plug_vifs')
@mock.patch.object(ironic_driver.IronicDriver, '_cleanup_deploy')
def test_spawn_node_prepare_for_deploy_fail(self, mock_cleanup_deploy,
mock_pvifs, mock_sf, mock_avti,
mock_sf, mock_avti,
mock_node, mock_required_by):
mock_required_by.return_value = False
node_uuid = 'aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee'
@ -1389,9 +1386,8 @@ class IronicDriverTestCase(test.NoDBTestCase):
@mock.patch.object(ironic_driver.IronicDriver, '_add_volume_target_info')
@mock.patch.object(ironic_driver.IronicDriver, '_generate_configdrive')
@mock.patch.object(ironic_driver.IronicDriver, '_start_firewall')
@mock.patch.object(ironic_driver.IronicDriver, '_plug_vifs')
def test_spawn_node_configdrive_fail(self,
mock_pvifs, mock_sf, mock_configdrive,
mock_sf, mock_configdrive,
mock_avti, mock_node, mock_save,
mock_required_by):
mock_required_by.return_value = True
@ -1422,10 +1418,9 @@ class IronicDriverTestCase(test.NoDBTestCase):
@mock.patch.object(FAKE_CLIENT, 'node')
@mock.patch.object(ironic_driver.IronicDriver, '_add_volume_target_info')
@mock.patch.object(ironic_driver.IronicDriver, '_start_firewall')
@mock.patch.object(ironic_driver.IronicDriver, '_plug_vifs')
@mock.patch.object(ironic_driver.IronicDriver, '_cleanup_deploy')
def test_spawn_node_trigger_deploy_fail(self, mock_cleanup_deploy,
mock_pvifs, mock_sf, mock_avti,
mock_sf, mock_avti,
mock_node, mock_required_by):
mock_required_by.return_value = False
node_uuid = 'aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee'
@ -1451,10 +1446,9 @@ class IronicDriverTestCase(test.NoDBTestCase):
@mock.patch.object(FAKE_CLIENT, 'node')
@mock.patch.object(ironic_driver.IronicDriver, '_add_volume_target_info')
@mock.patch.object(ironic_driver.IronicDriver, '_start_firewall')
@mock.patch.object(ironic_driver.IronicDriver, '_plug_vifs')
@mock.patch.object(ironic_driver.IronicDriver, '_cleanup_deploy')
def test_spawn_node_trigger_deploy_fail2(self, mock_cleanup_deploy,
mock_pvifs, mock_sf, mock_avti,
mock_sf, mock_avti,
mock_node, mock_required_by):
mock_required_by.return_value = False
node_uuid = 'aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee'
@ -1481,10 +1475,9 @@ class IronicDriverTestCase(test.NoDBTestCase):
@mock.patch.object(FAKE_CLIENT, 'node')
@mock.patch.object(ironic_driver.IronicDriver, '_add_volume_target_info')
@mock.patch.object(ironic_driver.IronicDriver, '_start_firewall')
@mock.patch.object(ironic_driver.IronicDriver, '_plug_vifs')
@mock.patch.object(ironic_driver.IronicDriver, 'destroy')
def test_spawn_node_trigger_deploy_fail3(self, mock_destroy,
mock_pvifs, mock_sf, mock_avti,
mock_sf, mock_avti,
mock_node, mock_looping,
mock_required_by):
mock_required_by.return_value = False
@ -1514,9 +1507,8 @@ class IronicDriverTestCase(test.NoDBTestCase):
@mock.patch.object(FAKE_CLIENT, 'node')
@mock.patch.object(ironic_driver.IronicDriver, '_add_volume_target_info')
@mock.patch.object(ironic_driver.IronicDriver, '_wait_for_active')
@mock.patch.object(ironic_driver.IronicDriver, '_plug_vifs')
@mock.patch.object(ironic_driver.IronicDriver, '_start_firewall')
def test_spawn_sets_default_ephemeral_device(self, mock_sf, mock_pvifs,
def test_spawn_sets_default_ephemeral_device(self, mock_sf,
mock_wait, mock_avti,
mock_node, mock_save,
mock_looping,
@ -2192,8 +2184,13 @@ class IronicDriverTestCase(test.NoDBTestCase):
mock_node.list_volume_connectors.assert_called_once_with(
node_uuid, detail=True)
@mock.patch.object(objects.instance.Instance, 'get_network_info')
@mock.patch.object(FAKE_CLIENT, 'node')
def test_get_volume_connector_no_ip(self, mock_node):
@mock.patch.object(FAKE_CLIENT.port, 'list')
@mock.patch.object(FAKE_CLIENT.portgroup, 'list')
def _test_get_volume_connector_no_ip(
self, mac_specified, mock_pgroup, mock_port, mock_node,
mock_nw_info, portgroup_exist=False):
node_uuid = uuids.node_uuid
node_props = {'cpu_arch': 'x86_64'}
node = ironic_utils.get_test_node(uuid=node_uuid,
@ -2201,20 +2198,99 @@ class IronicDriverTestCase(test.NoDBTestCase):
connectors = [ironic_utils.get_test_volume_connector(
node_uuid=node_uuid, type='iqn',
connector_id='iqn.test')]
if mac_specified:
connectors.append(ironic_utils.get_test_volume_connector(
node_uuid=node_uuid, type='mac',
connector_id='11:22:33:44:55:66'))
fixed_ip = network_model.FixedIP(address='1.2.3.4', version=4)
subnet = network_model.Subnet(ips=[fixed_ip])
network = network_model.Network(subnets=[subnet])
vif = network_model.VIF(
id='aaaaaaaa-vv11-cccc-dddd-eeeeeeeeeeee', network=network)
expected_props = {'initiator': 'iqn.test',
'ip': '1.2.3.4',
'host': '1.2.3.4',
'multipath': False,
'os_type': 'baremetal',
'platform': 'x86_64'}
mock_node.get.return_value = node
mock_node.list_volume_connectors.return_value = connectors
mock_nw_info.return_value = [vif]
instance = fake_instance.fake_instance_obj(self.ctx, node=node_uuid)
port = ironic_utils.get_test_port(
node_uuid=node_uuid, address='11:22:33:44:55:66',
internal_info={'tenant_vif_port_id': vif['id']})
mock_port.return_value = [port]
if portgroup_exist:
portgroup = ironic_utils.get_test_portgroup(
node_uuid=node_uuid, address='11:22:33:44:55:66',
extra={'vif_port_id': vif['id']})
mock_pgroup.return_value = [portgroup]
else:
mock_pgroup.return_value = []
props = self.driver.get_volume_connector(instance)
self.assertEqual(expected_props, props)
mock_node.get.assert_called_once_with(node_uuid)
mock_node.list_volume_connectors.assert_called_once_with(
node_uuid, detail=True)
if mac_specified:
mock_pgroup.assert_called_once_with(
node=node_uuid, address='11:22:33:44:55:66', detail=True)
if not portgroup_exist:
mock_port.assert_called_once_with(
node=node_uuid, address='11:22:33:44:55:66', detail=True)
else:
mock_port.assert_not_called()
else:
mock_pgroup.assert_not_called()
mock_port.assert_not_called()
def test_get_volume_connector_no_ip_with_mac(self):
self._test_get_volume_connector_no_ip(True)
def test_get_volume_connector_no_ip_with_mac_with_portgroup(self):
self._test_get_volume_connector_no_ip(True, portgroup_exist=True)
def test_get_volume_connector_no_ip_without_mac(self):
self._test_get_volume_connector_no_ip(False)
@mock.patch.object(ironic_driver.IronicDriver, 'plug_vifs')
def test_prepare_networks_before_block_device_mapping(self, mock_pvifs):
instance = fake_instance.fake_instance_obj(self.ctx)
network_info = utils.get_test_network_info()
self.driver.prepare_networks_before_block_device_mapping(instance,
network_info)
mock_pvifs.assert_called_once_with(instance, network_info)
@mock.patch.object(ironic_driver.IronicDriver, 'plug_vifs')
def test_prepare_networks_before_block_device_mapping_error(self,
mock_pvifs):
instance = fake_instance.fake_instance_obj(self.ctx)
network_info = utils.get_test_network_info()
mock_pvifs.side_effect = ironic_exception.BadRequest('fake error')
self.assertRaises(
ironic_exception.BadRequest,
self.driver.prepare_networks_before_block_device_mapping,
instance, network_info)
mock_pvifs.assert_called_once_with(instance, network_info)
@mock.patch.object(ironic_driver.IronicDriver, 'unplug_vifs')
def test_clean_networks_preparation(self, mock_upvifs):
instance = fake_instance.fake_instance_obj(self.ctx)
network_info = utils.get_test_network_info()
self.driver.clean_networks_preparation(instance, network_info)
mock_upvifs.assert_called_once_with(instance, network_info)
@mock.patch.object(ironic_driver.IronicDriver, 'unplug_vifs')
def test_clean_networks_preparation_error(self, mock_upvifs):
instance = fake_instance.fake_instance_obj(self.ctx)
network_info = utils.get_test_network_info()
mock_upvifs.side_effect = ironic_exception.BadRequest('fake error')
self.driver.clean_networks_preparation(instance, network_info)
mock_upvifs.assert_called_once_with(instance, network_info)
@mock.patch.object(FAKE_CLIENT, 'node')
@mock.patch.object(ironic_driver.LOG, 'error')

3
nova/tests/unit/virt/ironic/utils.py
View File

@ -156,6 +156,9 @@ class FakeVolumeTargetClient(object):
class FakePortClient(object):
def list(self, address=None, node=None):
pass
def get(self, port_uuid):
pass

27
nova/virt/driver.py
View File

@ -507,6 +507,33 @@ class ComputeDriver(object):
"""
raise NotImplementedError()
def prepare_networks_before_block_device_mapping(self, instance,
network_info):
"""Prepare networks before the block devices are mapped to instance.
Drivers who need network information for block device preparation can
do some network preparation necessary for block device preparation.
:param nova.objects.instance.Instance instance:
The instance whose networks are prepared.
:param nova.network.model.NetworkInfoAsyncWrapper network_info:
The network information of the given `instance`.
"""
pass
def clean_networks_preparation(self, instance, network_info):
"""Clean networks preparation when block device mapping is failed.
Drivers who need network information for block device preparaion should
clean the preparation when block device mapping is failed.
:param nova.objects.instance.Instance instance:
The instance whose networks are prepared.
:param nova.network.model.NetworkInfoAsyncWrapper network_info:
The network information of the given `instance`.
"""
pass
def attach_interface(self, context, instance, image_meta, vif):
"""Use hotplug to add a network interface to a running instance.

109
nova/virt/ironic/driver.py
View File

@ -1063,7 +1063,6 @@ class IronicDriver(virt_driver.ComputeDriver):
# prepare for the deploy
try:
self._plug_vifs(node, instance, network_info)
self._start_firewall(instance, network_info)
except Exception:
with excutils.save_and_reraise_exception():
@ -1783,6 +1782,44 @@ class IronicDriver(virt_driver.ComputeDriver):
def need_legacy_block_device_info(self):
return False
def prepare_networks_before_block_device_mapping(self, instance,
network_info):
"""Prepare networks before the block devices are mapped to instance.
Plug VIFs before block device preparation. In case where storage
network is managed by neutron and a MAC address is specified as a
volume connector to a node, we can get the IP address assigned to
the connector. An IP address of volume connector may be required by
some volume backend drivers. For getting the IP address, VIFs need to
be plugged before block device preparation so that a VIF is assigned to
a MAC address.
"""
try:
self.plug_vifs(instance, network_info)
except Exception:
with excutils.save_and_reraise_exception():
LOG.error("Error preparing deploy for instance "
"%(instance)s on baremetal node %(node)s.",
{'instance': instance.uuid,
'node': instance.node},
instance=instance)
def clean_networks_preparation(self, instance, network_info):
"""Clean networks preparation when block device mapping is failed.
Unplug VIFs when block device preparation is failed.
"""
try:
self.unplug_vifs(instance, network_info)
except Exception as e:
LOG.warning('Error detaching VIF from node %(node)s '
'after deploy failed; %(reason)s',
{'node': instance.node,
'reason': six.text_type(e)},
instance=instance)
def get_volume_connector(self, instance):
"""Get connector information for the instance for attaching to volumes.
@ -1798,8 +1835,14 @@ class IronicDriver(virt_driver.ComputeDriver):
'host': hostname
}
An IP address is set if a volume connector with type ip is assigned to
a node. An IP address is also set if a node has a volume connector with
type mac. An IP address is got from a VIF attached to an ironic port
or portgroup with the MAC address. Otherwise, an IP address of one
of VIFs is used.
:param instance: nova instance
:returns: A connector information dictionary
:return: A connector information dictionary
"""
node = self.ironicclient.call("node.get", instance.node)
properties = self._parse_node_properties(node)
@ -1810,8 +1853,13 @@ class IronicDriver(virt_driver.ComputeDriver):
values.setdefault(conn.type, []).append(conn.connector_id)
props = {}
if values.get('ip'):
props['ip'] = props['host'] = values['ip'][0]
ip = self._get_volume_connector_ip(instance, node, values)
if ip:
LOG.debug('Volume connector IP address for node %(node)s is '
'%(ip)s.',
{'node': node.uuid, 'ip': ip},
instance=instance)
props['ip'] = props['host'] = ip
if values.get('iqn'):
props['initiator'] = values['iqn'][0]
if values.get('wwpn'):
@ -1825,3 +1873,56 @@ class IronicDriver(virt_driver.ComputeDriver):
# we should at least set the value to False.
props['multipath'] = False
return props
def _get_volume_connector_ip(self, instance, node, values):
if values.get('ip'):
LOG.debug('Node %s has an IP address for volume connector',
node.uuid, instance=instance)
return values['ip'][0]
vif_id = self._get_vif_from_macs(node, values.get('mac', []), instance)
# retrieve VIF and get the IP address
nw_info = instance.get_network_info()
if vif_id:
fixed_ips = [ip for vif in nw_info if vif['id'] == vif_id
for ip in vif.fixed_ips()]
else:
fixed_ips = [ip for vif in nw_info for ip in vif.fixed_ips()]
fixed_ips_v4 = [ip for ip in fixed_ips if ip['version'] == 4]
if fixed_ips_v4:
return fixed_ips_v4[0]['address']
elif fixed_ips:
return fixed_ips[0]['address']
return None
def _get_vif_from_macs(self, node, macs, instance):
"""Get a VIF from specified MACs.
Retrieve ports and portgroups which have specified MAC addresses and
return a UUID of a VIF attached to a port or a portgroup found first.
:param node: The node object.
:param mac: A list of MAC addresses of volume connectors.
:param instance: nova instance, used for logging.
:return: A UUID of a VIF assigned to one of the MAC addresses.
"""
for mac in macs:
for method in ['portgroup.list', 'port.list']:
ports = self.ironicclient.call(method,
node=node.uuid,
address=mac,
detail=True)
for p in ports:
vif_id = (p.internal_info.get('tenant_vif_port_id') or
p.extra.get('vif_port_id'))
if vif_id:
LOG.debug('VIF %(vif)s for volume connector is '
'retrieved with MAC %(mac)s of node '
'%(node)s',
{'vif': vif_id,
'mac': mac,
'node': node.uuid},
instance=instance)
return vif_id
return None

11
releasenotes/notes/bp-ironic-volume-connector-ip-467396a516dc668a.yaml Normal file
View File

@ -0,0 +1,11 @@
---
features:
- |
When creating a baremetal instance with volumes, the ironic driver will
now pass an IP address of an iSCSI initiator to the block storage service
because the volume backend may require the IP address for access control.
If an IP address is set to an ironic node as a volume connector resource,
the address is used. If a node has MAC addresses as volume connector
resources, an IP address is retrieved from VIFs associated with the MAC
addresses. IPv4 addresses are given priority over IPv6 addresses if both
are available.