Skip to content

Commit

Permalink
Ironic: Get IP address for volume connector
Browse files Browse the repository at this point in the history 
When storage network for booting an instance from iSCSI volume is
managed by neutron, an IP address for volume connector cannot be
registered by an operator in advance. A MAC address can be registered
as a volume connector for an Ironic node. The IP address may be
required depending on cinder backend drivers.

This patch gets an IP address for a volume connector based on a MAC
address assigned to a volume connector in ironic. To bind VIFs to
ironic ports before connecting a volume, VIFs are attached earlier
with a new virt driver API. Nova can get an IP address assigned to
a VIF attached to an ironic port by retrieving the port with the
MAC address.

Co-Authored-By: Satoru Moriya <satoru.moriya.br@hitachi.com>
Implements: blueprint ironic-volume-connector-ip
Change-Id: I999bbfc0e28ec43390298deb59e2b6f6e10bf8ea
  • Loading branch information
@mriedem
2 people authored and mriedem committed Jan 25, 2018
1 parent 74deea4 commit 23d935b
Show file tree
Hide file tree
Showing 7 changed files with 275 additions and 28 deletions.
9 changes: 9 additions & 0 deletions nova/compute/manager.py
View file
Expand Up @@ -2189,6 +2189,11 @@ def _build_resources(self, context, instance, requested_networks,
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,
Expand All @@ -2209,11 +2214,14 @@ def _build_resources(self, context, instance, requested_networks,
# 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:
Expand All @@ -2222,6 +2230,7 @@ def _build_resources(self, context, instance, requested_networks,
# 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)
Expand Down
28 changes: 24 additions & 4 deletions nova/tests/unit/compute/test_compute_mgr.py
View file
Expand Up @@ -5221,8 +5221,12 @@ def test_build_resources_buildabort_reraise(self, mock_notify, mock_save,
@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
Expand All @@ -5240,6 +5244,8 @@ def test_build_resources_reraises_on_failed_bdm_prep(self, mock_prep,
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!'))
Expand Down Expand Up @@ -5293,7 +5299,12 @@ def test_validate_instance_group_policy_handles_hint_list(self, mock_get):
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',
Expand All @@ -5319,6 +5330,8 @@ def test_failed_bdm_prep_from_delete_raises_unexpected(self):
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):
Expand Down Expand Up @@ -5430,8 +5443,13 @@ def test_build_resources_instance_not_found_before_yield(
@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={})
Expand All @@ -5445,6 +5463,8 @@ def test_build_resources_unexpected_task_error_before_yield(
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(
Expand Down
116 changes: 96 additions & 20 deletions nova/tests/unit/virt/ironic/test_driver.py
View file
Expand Up @@ -32,6 +32,7 @@
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
Expand Down Expand Up @@ -1030,9 +1031,8 @@ def test_get_info_http_not_found(self, mock_gbiu):
@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)
Expand All @@ -1059,7 +1059,6 @@ def _test_spawn(self, mock_sf, mock_pvifs, mock_aiitn, mock_wait_active,
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)
Expand Down Expand Up @@ -1099,11 +1098,10 @@ def test_spawn_with_configdrive(self, mock_required_by, mock_configdrive):
@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'
Expand Down Expand Up @@ -1356,10 +1354,9 @@ def test_spawn_node_driver_validation_fail(self, mock_avti, mock_node,
@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'
Expand Down Expand Up @@ -1389,9 +1386,8 @@ class TestException(Exception):
@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
Expand Down Expand Up @@ -1422,10 +1418,9 @@ class TestException(Exception):
@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'
Expand All @@ -1451,10 +1446,9 @@ def test_spawn_node_trigger_deploy_fail(self, mock_cleanup_deploy,
@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'
Expand All @@ -1481,10 +1475,9 @@ def test_spawn_node_trigger_deploy_fail2(self, mock_cleanup_deploy,
@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
Expand Down Expand Up @@ -1514,9 +1507,8 @@ def test_spawn_node_trigger_deploy_fail3(self, mock_destroy,
@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,
Expand Down Expand Up @@ -2192,29 +2184,113 @@ def test_get_volume_connector(self, mock_node):
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,
properties=node_props)
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')
Expand Down
3 changes: 3 additions & 0 deletions nova/tests/unit/virt/ironic/utils.py
View file
Expand Up @@ -156,6 +156,9 @@ def delete(self, volume_target_id):

class FakePortClient(object):

def list(self, address=None, node=None):
pass

def get(self, port_uuid):
pass

Expand Down

0 comments on commit 23d935b

Please sign in to comment.