Support Ironic interface attach/detach in nova virt

In order to support different Ironic network drivers that may need to
implement vif to pif mapping in different ways, we've added new APIs
in Ironic to abstract this behaviour. This patch switches the Ironic
virt driver over to use this API. As part of this change the virt driver
will no longer provide the macs_for_instance function, as Ironic will
handle updating the Neutron ports during the provisioning process.

Update interface mac addresses in configdrive network_metadata with
ironic node ports values.

Co-Authored-By: Vasyl Saienko <vsaienko@mirantis.com>
Implements: blueprint ironic-plug-unplug-vifs-update
Change-Id: I4d70423ca978885a982c7eb5bd1efcc024d2b777
Partial-Bug: #1582188
Closes-Bug: #1656854
This commit is contained in:
Sam Betts 2016-09-01 16:44:06 +01:00 committed by Matt Riedemann
parent b34809dfae
commit c7b46c4778
9 changed files with 131 additions and 212 deletions

View File

@ -72,7 +72,7 @@ class IronicClientWrapperTestCase(test.NoDBTestCase):
expected = {'session': 'session',
'max_retries': CONF.ironic.api_max_retries,
'retry_interval': CONF.ironic.api_retry_interval,
'os_ironic_api_version': '1.21',
'os_ironic_api_version': '1.28',
'ironic_url': None}
mock_ir_cli.assert_called_once_with(1, **expected)

View File

@ -827,27 +827,6 @@ class IronicDriverTestCase(test.NoDBTestCase):
self.assertEqual(hardware.InstanceInfo(state=nova_states.NOSTATE),
result)
@mock.patch.object(FAKE_CLIENT, 'node')
def test_macs_for_instance(self, mock_node):
node = ironic_utils.get_test_node()
port = ironic_utils.get_test_port()
mock_node.get.return_value = node
mock_node.list_ports.return_value = [port]
instance = fake_instance.fake_instance_obj(self.ctx,
node=node.uuid)
result = self.driver.macs_for_instance(instance)
self.assertEqual(set([port.address]), result)
mock_node.list_ports.assert_called_once_with(node.uuid)
@mock.patch.object(FAKE_CLIENT.node, 'get')
def test_macs_for_instance_http_not_found(self, mock_get):
mock_get.side_effect = ironic_exception.NotFound()
instance = fake_instance.fake_instance_obj(
self.ctx, node=uuidutils.generate_uuid())
result = self.driver.macs_for_instance(instance)
self.assertIsNone(result)
@mock.patch.object(objects.Instance, 'save')
@mock.patch.object(loopingcall, 'FixedIntervalLoopingCall')
@mock.patch.object(FAKE_CLIENT, 'node')
@ -969,6 +948,8 @@ class IronicDriverTestCase(test.NoDBTestCase):
'value': instance.display_name, 'op': 'add'},
{'path': '/instance_info/vcpus', 'op': 'add',
'value': str(instance.flavor.vcpus)},
{'path': '/instance_info/nova_host_id', 'op': 'add',
'value': instance.host},
{'path': '/instance_info/memory_mb', 'op': 'add',
'value': str(instance.flavor.memory_mb)},
{'path': '/instance_info/local_gb', 'op': 'add',
@ -1392,31 +1373,20 @@ class IronicDriverTestCase(test.NoDBTestCase):
self.driver.power_off(instance)
mock_sp.assert_called_once_with(node.uuid, 'off')
@mock.patch.object(FAKE_CLIENT.node, 'list_ports')
@mock.patch.object(FAKE_CLIENT.port, 'update')
@mock.patch.object(ironic_driver.IronicDriver, '_unplug_vifs')
def test_plug_vifs_with_port(self, mock_uvifs, mock_port_udt, mock_lp):
@mock.patch.object(FAKE_CLIENT.node, 'vif_attach')
def test_plug_vifs_with_port(self, mock_vatt):
node_uuid = 'aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee'
node = ironic_utils.get_test_node(uuid=node_uuid)
# make the address be consistent with network_info's
port = ironic_utils.get_test_port(address=utils.FAKE_VIF_MAC)
mock_lp.return_value = [port]
instance = fake_instance.fake_instance_obj(self.ctx,
node=node_uuid)
network_info = utils.get_test_network_info()
vif_id = six.text_type(network_info[0]['id'])
port_id = six.text_type(network_info[0]['id'])
expected_patch = [{'op': 'add',
'path': '/extra/vif_port_id',
'value': port_id}]
self.driver._plug_vifs(node, instance, network_info)
# asserts
mock_uvifs.assert_called_once_with(node, instance, network_info)
mock_lp.assert_called_once_with(node_uuid)
mock_port_udt.assert_called_with(port.uuid, expected_patch)
mock_vatt.assert_called_with(node.uuid, vif_id)
@mock.patch.object(FAKE_CLIENT.node, 'get')
@mock.patch.object(ironic_driver.IronicDriver, '_plug_vifs')
@ -1434,82 +1404,67 @@ class IronicDriverTestCase(test.NoDBTestCase):
fields=ironic_driver._NODE_FIELDS)
mock__plug_vifs.assert_called_once_with(node, instance, network_info)
@mock.patch.object(FAKE_CLIENT.port, 'update')
@mock.patch.object(FAKE_CLIENT.node, 'list_ports')
@mock.patch.object(ironic_driver.IronicDriver, '_unplug_vifs')
def test_plug_vifs_multiple_ports(self, mock_uvifs, mock_lp,
mock_port_udt):
@mock.patch.object(FAKE_CLIENT.node, 'vif_attach')
def test_plug_vifs_multiple_ports(self, mock_vatt):
node_uuid = 'aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee'
node = ironic_utils.get_test_node(uuid=node_uuid)
first_ironic_port_uuid = 'aaaaaaaa-bbbb-1111-dddd-eeeeeeeeeeee'
first_port = ironic_utils.get_test_port(uuid=first_ironic_port_uuid,
node_uuid=node_uuid,
address='11:FF:FF:FF:FF:FF')
second_ironic_port_uuid = 'aaaaaaaa-bbbb-2222-dddd-eeeeeeeeeeee'
second_port = ironic_utils.get_test_port(uuid=second_ironic_port_uuid,
node_uuid=node_uuid,
address='22:FF:FF:FF:FF:FF')
mock_lp.return_value = [second_port, first_port]
instance = fake_instance.fake_instance_obj(self.ctx,
node=node_uuid)
first_vif_id = 'aaaaaaaa-vv11-cccc-dddd-eeeeeeeeeeee'
second_vif_id = 'aaaaaaaa-vv22-cccc-dddd-eeeeeeeeeeee'
first_vif = ironic_utils.get_test_vif(
address='22:FF:FF:FF:FF:FF',
id=second_vif_id)
second_vif = ironic_utils.get_test_vif(
address='11:FF:FF:FF:FF:FF',
id=first_vif_id)
first_vif = ironic_utils.get_test_vif(address='22:FF:FF:FF:FF:FF',
id=first_vif_id)
second_vif = ironic_utils.get_test_vif(address='11:FF:FF:FF:FF:FF',
id=second_vif_id)
network_info = [first_vif, second_vif]
self.driver._plug_vifs(node, instance, network_info)
# asserts
mock_uvifs.assert_called_once_with(node, instance, network_info)
mock_lp.assert_called_once_with(node_uuid)
calls = (mock.call(first_ironic_port_uuid,
[{'op': 'add', 'path': '/extra/vif_port_id',
'value': first_vif_id}]),
mock.call(second_ironic_port_uuid,
[{'op': 'add', 'path': '/extra/vif_port_id',
'value': second_vif_id}]))
mock_port_udt.assert_has_calls(calls, any_order=True)
calls = (mock.call(node.uuid, first_vif_id),
mock.call(node.uuid, second_vif_id))
mock_vatt.assert_has_calls(calls, any_order=True)
@mock.patch.object(FAKE_CLIENT.port, 'update')
@mock.patch.object(FAKE_CLIENT.node, 'list_ports')
@mock.patch.object(ironic_driver.IronicDriver, '_unplug_vifs')
def test_plug_vifs_count_mismatch(self, mock_uvifs, mock_lp,
mock_port_udt):
@mock.patch.object(FAKE_CLIENT, 'node')
def test_plug_vifs_failure(self, mock_node):
node_uuid = 'aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee'
node = ironic_utils.get_test_node(uuid=node_uuid)
port = ironic_utils.get_test_port()
mock_lp.return_value = [port]
instance = fake_instance.fake_instance_obj(self.ctx,
node=node_uuid)
# len(network_info) > len(ports)
network_info = (utils.get_test_network_info() +
utils.get_test_network_info())
self.assertRaises(exception.NovaException,
first_vif_id = 'aaaaaaaa-vv11-cccc-dddd-eeeeeeeeeeee'
second_vif_id = 'aaaaaaaa-vv22-cccc-dddd-eeeeeeeeeeee'
first_vif = ironic_utils.get_test_vif(address='22:FF:FF:FF:FF:FF',
id=first_vif_id)
second_vif = ironic_utils.get_test_vif(address='11:FF:FF:FF:FF:FF',
id=second_vif_id)
mock_node.vif_attach.side_effect = [None,
ironic_exception.BadRequest()]
network_info = [first_vif, second_vif]
self.assertRaises(exception.VirtualInterfacePlugException,
self.driver._plug_vifs, node, instance,
network_info)
# asserts
mock_uvifs.assert_called_once_with(node, instance, network_info)
mock_lp.assert_called_once_with(node_uuid)
# assert port.update() was not called
self.assertFalse(mock_port_udt.called)
@mock.patch.object(FAKE_CLIENT.port, 'update')
@mock.patch.object(FAKE_CLIENT.node, 'list_ports')
@mock.patch.object(ironic_driver.IronicDriver, '_unplug_vifs')
def test_plug_vifs_no_network_info(self, mock_uvifs, mock_lp,
mock_port_udt):
@mock.patch.object(FAKE_CLIENT, 'node')
def test_plug_vifs_already_attached(self, mock_node):
node_uuid = 'aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee'
node = ironic_utils.get_test_node(uuid=node_uuid)
port = ironic_utils.get_test_port()
instance = fake_instance.fake_instance_obj(self.ctx,
node=node_uuid)
first_vif_id = 'aaaaaaaa-vv11-cccc-dddd-eeeeeeeeeeee'
second_vif_id = 'aaaaaaaa-vv22-cccc-dddd-eeeeeeeeeeee'
first_vif = ironic_utils.get_test_vif(address='22:FF:FF:FF:FF:FF',
id=first_vif_id)
second_vif = ironic_utils.get_test_vif(address='11:FF:FF:FF:FF:FF',
id=second_vif_id)
mock_node.vif_attach.side_effect = [ironic_exception.Conflict(),
None]
network_info = [first_vif, second_vif]
self.driver._plug_vifs(node, instance, network_info)
self.assertEqual(2, mock_node.vif_attach.call_count)
mock_lp.return_value = [port]
@mock.patch.object(FAKE_CLIENT.node, 'vif_attach')
def test_plug_vifs_no_network_info(self, mock_vatt):
node_uuid = 'aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee'
node = ironic_utils.get_test_node(uuid=node_uuid)
instance = fake_instance.fake_instance_obj(self.ctx,
node=node_uuid)
@ -1517,60 +1472,45 @@ class IronicDriverTestCase(test.NoDBTestCase):
self.driver._plug_vifs(node, instance, network_info)
# asserts
mock_uvifs.assert_called_once_with(node, instance, network_info)
mock_lp.assert_called_once_with(node_uuid)
# assert port.update() was not called
self.assertFalse(mock_port_udt.called)
self.assertFalse(mock_vatt.called)
@mock.patch.object(FAKE_CLIENT.port, 'update')
@mock.patch.object(FAKE_CLIENT, 'node')
def test_unplug_vifs(self, mock_node, mock_update):
def test_unplug_vifs(self, mock_node):
node_uuid = 'aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee'
node = ironic_utils.get_test_node(uuid=node_uuid)
port = ironic_utils.get_test_port(extra={'vif_port_id': 'fake-vif'})
mock_node.get.return_value = node
mock_node.list_ports.return_value = [port]
instance = fake_instance.fake_instance_obj(self.ctx,
node=node_uuid)
expected_patch = [{'op': 'remove', 'path':
'/extra/vif_port_id'}]
self.driver.unplug_vifs(instance,
utils.get_test_network_info())
network_info = utils.get_test_network_info()
vif_id = six.text_type(network_info[0]['id'])
self.driver.unplug_vifs(instance, network_info)
# asserts
mock_node.get.assert_called_once_with(
node_uuid, fields=ironic_driver._NODE_FIELDS)
mock_node.list_ports.assert_called_once_with(node_uuid, detail=True)
mock_update.assert_called_once_with(port.uuid, expected_patch)
mock_node.vif_detach.assert_called_once_with(node.uuid, vif_id)
@mock.patch.object(FAKE_CLIENT.port, 'update')
@mock.patch.object(FAKE_CLIENT, 'node')
def test_unplug_vifs_port_not_associated(self, mock_node, mock_update):
def test_unplug_vifs_port_not_associated(self, mock_node):
node_uuid = 'aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee'
node = ironic_utils.get_test_node(uuid=node_uuid)
port = ironic_utils.get_test_port(extra={})
mock_node.get.return_value = node
mock_node.list_ports.return_value = [port]
instance = fake_instance.fake_instance_obj(self.ctx, node=node_uuid)
self.driver.unplug_vifs(instance, utils.get_test_network_info())
network_info = utils.get_test_network_info()
self.driver.unplug_vifs(instance, network_info)
mock_node.get.assert_called_once_with(
node_uuid, fields=ironic_driver._NODE_FIELDS)
mock_node.list_ports.assert_called_once_with(node_uuid, detail=True)
# assert port.update() was not called
self.assertFalse(mock_update.called)
self.assertEqual(len(network_info), mock_node.vif_detach.call_count)
@mock.patch.object(FAKE_CLIENT.port, 'update')
def test_unplug_vifs_no_network_info(self, mock_update):
@mock.patch.object(FAKE_CLIENT.node, 'vif_detach')
def test_unplug_vifs_no_network_info(self, mock_vdet):
instance = fake_instance.fake_instance_obj(self.ctx)
network_info = []
self.driver.unplug_vifs(instance, network_info)
# assert port.update() was not called
self.assertFalse(mock_update.called)
self.assertFalse(mock_vdet.called)
@mock.patch.object(firewall.NoopFirewallDriver, 'unfilter_instance',
create=True)
@ -1697,33 +1637,19 @@ class IronicDriverTestCase(test.NoDBTestCase):
detach_block_devices=None, attach_block_devices=None)
@mock.patch.object(FAKE_CLIENT.node, 'get')
def _test_network_binding_host_id(self, network_interface, mock_get):
def test_network_binding_host_id(self, mock_get):
node_uuid = uuidutils.generate_uuid()
hostname = 'ironic-compute'
instance = fake_instance.fake_instance_obj(self.ctx,
node=node_uuid,
host=hostname)
if network_interface == 'neutron':
expected = None
else:
expected = hostname
node = ironic_utils.get_test_node(uuid=node_uuid,
instance_uuid=self.instance_uuid,
instance_type_id=5,
network_interface=network_interface)
network_interface='flat')
mock_get.return_value = node
host_id = self.driver.network_binding_host_id(self.ctx, instance)
self.assertEqual(expected, host_id)
def test_network_binding_host_id_neutron(self):
self._test_network_binding_host_id('neutron')
def test_network_binding_host_id_flat(self):
self._test_network_binding_host_id('flat')
def test_network_binding_host_id_noop(self):
self._test_network_binding_host_id('noop')
self.assertIsNone(host_id)
@mock.patch.object(instance_metadata, 'InstanceMetadata')

View File

@ -59,7 +59,10 @@ class IronicDriverFieldsTestCase(test.NoDBTestCase):
'op': 'add'},
{'path': '/instance_info/local_gb',
'value': str(self.node.properties.get('local_gb', 0)),
'op': 'add'}
'op': 'add'},
{'path': '/instance_info/nova_host_id',
'value': u'fake-host',
'op': 'add'},
]
def assertPatchEqual(self, expected, observed):

View File

@ -58,6 +58,7 @@ def get_test_port(**kw):
'node_uuid': kw.get('node_uuid', get_test_node().uuid),
'address': kw.get('address', 'FF:FF:FF:FF:FF:FF'),
'extra': kw.get('extra', {}),
'internal_info': kw.get('internal_info', {}),
'created_at': kw.get('created_at'),
'updated_at': kw.get('updated_at')})()
@ -120,7 +121,7 @@ class FakeNodeClient(object):
def get_by_instance_uuid(self, instance_uuid, fields=None):
pass
def list_ports(self, node_uuid):
def list_ports(self, node_uuid, detail=False):
pass
def set_power_state(self, node_uuid, target):
@ -135,6 +136,12 @@ class FakeNodeClient(object):
def validate(self, node_uuid):
pass
def vif_attach(self, node_uuid, port_id):
pass
def vif_detach(self, node_uuid, port_id):
pass
class FakeClient(object):

View File

@ -1242,6 +1242,8 @@ class ComputeDriver(object):
"""Does the driver want networks deallocated on reschedule?"""
return False
# NOTE(vsaienko) This method is deprecated, don't use it!
# TODO(vsaienko) Remove this function in Ocata.
def macs_for_instance(self, instance):
"""What MAC addresses must this instance have?

View File

@ -32,7 +32,7 @@ ironic = None
IRONIC_GROUP = nova.conf.ironic.ironic_group
# The API version required by the Ironic driver
IRONIC_API_VERSION = (1, 21)
IRONIC_API_VERSION = (1, 28)
class IronicClientWrapper(object):

View File

@ -683,24 +683,6 @@ class IronicDriver(virt_driver.ComputeDriver):
"""
return True
def macs_for_instance(self, instance):
"""List the MAC addresses of an instance.
List of MAC addresses for the node which this instance is
associated with.
:param instance: the instance object.
:return: None, or a set of MAC ids (e.g. set(['12:34:56:78:90:ab'])).
None means 'no constraints', a set means 'these and only these
MAC addresses'.
"""
try:
node = self._get_node(instance.node)
except ironic.exc.NotFound:
return None
ports = self.ironicclient.call("node.list_ports", node.uuid)
return set([p.address for p in ports])
def _get_network_metadata(self, node, network_info):
"""Gets a more complete representation of the instance network info.
@ -715,10 +697,21 @@ class IronicDriver(virt_driver.ComputeDriver):
ports = self.ironicclient.call("node.list_ports",
node.uuid, detail=True)
for port in ports:
for link in base_metadata['links']:
if link['ethernet_mac_address'] == port.address:
link['type'] = 'phy'
# TODO(vsaienko) add support of portgroups
vif_id_to_objects = {'ports': {}}
for p in ports:
vif_id = (p.internal_info.get('tenant_vif_port_id') or
p.extra.get('vif_port_id'))
if vif_id:
vif_id_to_objects['ports'][vif_id] = p
for link in base_metadata['links']:
vif_id = link['vif_id']
if vif_id in vif_id_to_objects['ports']:
p = vif_id_to_objects['ports'][vif_id]
# Ironic updates neutron port's address during attachment
link.update({'ethernet_mac_address': p.address,
'type': 'phy'})
return base_metadata
@ -1091,32 +1084,20 @@ class IronicDriver(virt_driver.ComputeDriver):
LOG.debug("plug: instance_uuid=%(uuid)s vif=%(network_info)s",
{'uuid': instance.uuid,
'network_info': network_info_str})
# start by ensuring the ports are clear
self._unplug_vifs(node, instance, network_info)
ports = self.ironicclient.call("node.list_ports", node.uuid)
if len(network_info) > len(ports):
raise exception.VirtualInterfacePlugException(_(
"Ironic node: %(id)s virtual to physical interface count"
" mismatch"
" (Vif count: %(vif_count)d, Pif count: %(pif_count)d)")
% {'id': node.uuid,
'vif_count': len(network_info),
'pif_count': len(ports)})
if len(network_info) > 0:
# not needed if no vif are defined
for vif in network_info:
for pif in ports:
if vif['address'] == pif.address:
# attach what neutron needs directly to the port
port_id = six.text_type(vif['id'])
patch = [{'op': 'add',
'path': '/extra/vif_port_id',
'value': port_id}]
self.ironicclient.call("port.update", pif.uuid, patch)
break
for vif in network_info:
port_id = six.text_type(vif['id'])
try:
self.ironicclient.call("node.vif_attach", node.uuid, port_id,
retry_on_conflict=False)
except ironic.exc.BadRequest as e:
msg = (_("Cannot attach VIF %(vif)s to the node %(node)s due "
"to error: %(err)s") % {'vif': port_id,
'node': node.uuid, 'err': e})
LOG.error(msg)
raise exception.VirtualInterfacePlugException(msg)
except ironic.exc.Conflict:
# NOTE (vsaienko) Pass since VIF already attached.
pass
def _unplug_vifs(self, node, instance, network_info):
# NOTE(PhilDay): Accessing network_info will block if the thread
@ -1126,19 +1107,16 @@ class IronicDriver(virt_driver.ComputeDriver):
LOG.debug("unplug: instance_uuid=%(uuid)s vif=%(network_info)s",
{'uuid': instance.uuid,
'network_info': network_info_str})
if network_info and len(network_info) > 0:
ports = self.ironicclient.call("node.list_ports", node.uuid,
detail=True)
# not needed if no vif are defined
for pif in ports:
if 'vif_port_id' in pif.extra:
# we can not attach a dict directly
patch = [{'op': 'remove', 'path': '/extra/vif_port_id'}]
try:
self.ironicclient.call("port.update", pif.uuid, patch)
except ironic.exc.BadRequest:
pass
if not network_info:
return
for vif in network_info:
port_id = six.text_type(vif['id'])
try:
self.ironicclient.call("node.vif_detach", node.uuid,
port_id)
except ironic.exc.BadRequest:
LOG.debug("VIF %(vif)s isn't attached to Ironic node %(node)s",
{'vif': port_id, 'node': node.uuid})
def plug_vifs(self, instance, network_info):
"""Plug VIFs into networks.
@ -1240,8 +1218,7 @@ class IronicDriver(virt_driver.ComputeDriver):
Neutron. If using the neutron network interface (separate networks for
the control plane and tenants), return None here to indicate that the
port should not yet be bound; Ironic will make a port-update call to
Neutron later to tell Neutron to bind the port. Otherwise, use the
default behavior and allow the port to be bound immediately.
Neutron later to tell Neutron to bind the port.
NOTE: the late binding is important for security. If an ML2 mechanism
manages to connect the tenant network to the baremetal machine before
@ -1257,16 +1234,12 @@ class IronicDriver(virt_driver.ComputeDriver):
:param context: request context
:param instance: nova.objects.instance.Instance that the network
ports will be associated with
:returns: a string representing the host ID, or None
:returns: None
"""
node = self.ironicclient.call('node.get', instance.node,
fields=['network_interface'])
if node.network_interface == 'neutron':
return None
# flat network, go ahead and allow the port to be bound
return super(IronicDriver, self).network_binding_host_id(
context, instance)
# NOTE(vsaienko) Ironic will set binding:host_id later with port-update
# call when updating mac address or setting binding:profile
# to tell Neutron to bind the port.
return None
def _get_node_console_with_reset(self, instance):
"""Acquire console information for an instance.

View File

@ -65,6 +65,8 @@ class GenericDriverFields(object):
'op': 'add', 'value': instance.display_name})
patch.append({'path': '/instance_info/vcpus', 'op': 'add',
'value': str(instance.flavor.vcpus)})
patch.append({'path': '/instance_info/nova_host_id', 'op': 'add',
'value': instance.get('host')})
patch.append({'path': '/instance_info/memory_mb', 'op': 'add',
'value': str(instance.flavor.memory_mb)})
patch.append({'path': '/instance_info/local_gb', 'op': 'add',

View File

@ -0,0 +1,6 @@
---
upgrade:
- The Ironic driver now requires python-ironicclient>=1.9.0, and requires
Ironic service to support API version 1.28 or
higher. As usual, Ironic should be upgraded before Nova for a
smooth upgrade process.