From e55cf73890aa104281775c0d2fe4f9f75ab2ec97 Mon Sep 17 00:00:00 2001 From: Jim Rollenhagen Date: Fri, 25 Mar 2016 17:11:51 -0700 Subject: [PATCH] Ironic: enable multitenant networking This replaces the unused code (due to API versioning) in the ironic driver's implementation of `network_binding_host_id`, to code that actually enables the multitenant networking support in ironic. While building this out, we changed direction and ended with the node's field being called `network_interface` instead of `network_provider`. This patch changes this to match, and bumps the ironic API version used to 1.20, where this feature is introduced. The old code also assumed ironic would have only two network interfaces, "neutron" and "none". This became three - "neutron", "flat", and "noop".[0] "neutron" is the only interface that requires returning None from `network_binding_host_id`, while the others should match the old behavior. Change the code to only do this for the neutron network interface, instead of doing this for anything not called 'flat'. [0] https://git.openstack.org/cgit/openstack/ironic/tree/setup.cfg#n90 Depends-On: I09a42c8e54d7782c591415e53fccade972ae8bdb Depends-On: I3c135a4a2c79cfb0b9d63d9d31009330c2abb680 Change-Id: I9d036fd5d209ccd321fbd28117660494a7bcb74d Implements: blueprint ironic-networks-support Co-Authored-By: Hironori Shiina --- .../unit/virt/ironic/test_client_wrapper.py | 6 +-- nova/tests/unit/virt/ironic/test_driver.py | 17 +++++---- nova/tests/unit/virt/ironic/utils.py | 2 +- nova/virt/ironic/client_wrapper.py | 2 +- nova/virt/ironic/driver.py | 38 ++++++++++++------- ...ltitenant-networking-6f124964831d4a6c.yaml | 10 +++++ 6 files changed, 49 insertions(+), 26 deletions(-) create mode 100644 releasenotes/notes/ironic-multitenant-networking-6f124964831d4a6c.yaml diff --git a/nova/tests/unit/virt/ironic/test_client_wrapper.py b/nova/tests/unit/virt/ironic/test_client_wrapper.py index f3658945d663..60873ab3bf73 100644 --- a/nova/tests/unit/virt/ironic/test_client_wrapper.py +++ b/nova/tests/unit/virt/ironic/test_client_wrapper.py @@ -74,7 +74,7 @@ class IronicClientWrapperTestCase(test.NoDBTestCase): 'ironic_url': CONF.ironic.api_endpoint, 'max_retries': CONF.ironic.api_max_retries, 'retry_interval': CONF.ironic.api_retry_interval, - 'os_ironic_api_version': '1.8'} + 'os_ironic_api_version': '1.20'} mock_ir_cli.assert_called_once_with(1, **expected) @mock.patch.object(ironic_client, 'get_client') @@ -87,7 +87,7 @@ class IronicClientWrapperTestCase(test.NoDBTestCase): 'ironic_url': CONF.ironic.api_endpoint, 'max_retries': CONF.ironic.api_max_retries, 'retry_interval': CONF.ironic.api_retry_interval, - 'os_ironic_api_version': '1.8'} + 'os_ironic_api_version': '1.20'} mock_ir_cli.assert_called_once_with(1, **expected) @mock.patch.object(ironic_client, 'get_client') @@ -101,7 +101,7 @@ class IronicClientWrapperTestCase(test.NoDBTestCase): 'ironic_url': CONF.ironic.api_endpoint, 'max_retries': CONF.ironic.api_max_retries, 'retry_interval': CONF.ironic.api_retry_interval, - 'os_ironic_api_version': '1.8', + 'os_ironic_api_version': '1.20', 'os_cacert': 'fake-cafile', 'ca_file': 'fake-cafile'} mock_ir_cli.assert_called_once_with(1, **expected) diff --git a/nova/tests/unit/virt/ironic/test_driver.py b/nova/tests/unit/virt/ironic/test_driver.py index aa30375cbf80..c705af4c8321 100644 --- a/nova/tests/unit/virt/ironic/test_driver.py +++ b/nova/tests/unit/virt/ironic/test_driver.py @@ -1625,32 +1625,33 @@ 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, is_neutron, mock_get): + def _test_network_binding_host_id(self, network_interface, mock_get): node_uuid = uuidutils.generate_uuid() hostname = 'ironic-compute' instance = fake_instance.fake_instance_obj(self.ctx, node=node_uuid, host=hostname) - if is_neutron: - provider = 'neutron' + if network_interface == 'neutron': expected = None else: - provider = 'none' expected = hostname node = ironic_utils.get_test_node(uuid=node_uuid, instance_uuid=self.instance_uuid, instance_type_id=5, - network_provider=provider) + network_interface=network_interface) 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(True) + self._test_network_binding_host_id('neutron') - def test_network_binding_host_id_none(self): - self._test_network_binding_host_id(False) + 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') @mock.patch.object(instance_metadata, 'InstanceMetadata') diff --git a/nova/tests/unit/virt/ironic/utils.py b/nova/tests/unit/virt/ironic/utils.py index 6913aa4e3e86..d7820dc008d7 100644 --- a/nova/tests/unit/virt/ironic/utils.py +++ b/nova/tests/unit/virt/ironic/utils.py @@ -45,7 +45,7 @@ def get_test_node(**kw): 'properties': kw.get('properties', {}), 'reservation': kw.get('reservation'), 'maintenance': kw.get('maintenance', False), - 'network_provider': kw.get('network_provider'), + 'network_interface': kw.get('network_interface'), 'extra': kw.get('extra', {}), 'updated_at': kw.get('created_at'), 'created_at': kw.get('updated_at')})() diff --git a/nova/virt/ironic/client_wrapper.py b/nova/virt/ironic/client_wrapper.py index fd02c2846e2b..85f38302d159 100644 --- a/nova/virt/ironic/client_wrapper.py +++ b/nova/virt/ironic/client_wrapper.py @@ -29,7 +29,7 @@ CONF = cfg.CONF ironic = None # The API version required by the Ironic driver -IRONIC_API_VERSION = (1, 8) +IRONIC_API_VERSION = (1, 20) class IronicClientWrapper(object): diff --git a/nova/virt/ironic/driver.py b/nova/virt/ironic/driver.py index 64769864f54f..f9f7ed9b8e37 100644 --- a/nova/virt/ironic/driver.py +++ b/nova/virt/ironic/driver.py @@ -1142,22 +1142,34 @@ class IronicDriver(virt_driver.ComputeDriver): def network_binding_host_id(self, context, instance): """Get host ID to associate with network ports. - This defines the binding:host_id parameter to the port-create - calls for Neutron. If using a flat network, use the default behavior - and allow the port to bind immediately. If using 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. + This defines the binding:host_id parameter to the port-create calls for + 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. + + NOTE: the late binding is important for security. If an ML2 mechanism + manages to connect the tenant network to the baremetal machine before + deployment is done (e.g. port-create time), then the tenant potentially + has access to the deploy agent, which may contain firmware blobs or + secrets. ML2 mechanisms may be able to connect the port without the + switchport info that comes from ironic, if they store that switchport + info for some reason. As such, we should *never* pass binding:host_id + in the port-create call when using the 'neutron' network_interface, + because a null binding:host_id indicates to Neutron that it should + not connect the port yet. :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 + :returns: a string representing the host ID, or None """ - node = self._get_node(instance.node) - if getattr(node, 'network_provider', 'none') == 'none': - # flat network, go ahead and allow the port to be bound - return super(IronicDriver, self).network_binding_host_id( - context, instance) - return 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) diff --git a/releasenotes/notes/ironic-multitenant-networking-6f124964831d4a6c.yaml b/releasenotes/notes/ironic-multitenant-networking-6f124964831d4a6c.yaml new file mode 100644 index 000000000000..b263e68049ec --- /dev/null +++ b/releasenotes/notes/ironic-multitenant-networking-6f124964831d4a6c.yaml @@ -0,0 +1,10 @@ +--- +features: + - Multitenant networking for the ironic compute driver is now supported. + To enable this feature, ironic nodes must be using the 'neutron' + network_interface. +upgrade: + - The ironic driver now requires python-ironicclient>=1.5.0 (previously + >=1.1.0), and requires the ironic service to support API version 1.20 or + higher. As usual, ironic should be upgraded before nova for a + smooth upgrade process.