openstack/ironic
Code Issues Proposed changes

Allow setting {provisioning,cleaning,rescuing}_network in driver_info

Browse Source 
These values have priority over ones defined in ironic.conf. Also modify
validation code to only used network UUID cached on task.network if:
1. the requested XXX_network is UUID, not name
2. the requested XXX_network is the same as the cached one.

Co-Authored-By: Dmitry Tantsur <dtantsur@redhat.com>
Closes-Bug: #1614876
Change-Id: I4caec34d85304fe887bcc28b7528cceceb3acfe8
This commit is contained in:
M V P Nitesh 2017-07-06 12:29:33 +05:30 committed by Dmitry Tantsur
parent 6ee297f7a5
commit 95d7e602e5
8 changed files with 294 additions and 40 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

20
doc/source/install/configure-tenant-networks.rst
View File

@ -40,18 +40,30 @@ provisioning will happen in a multi-tenant environment (which means using the
default, otherwise ``noop`` is the default.
#. Define a provider network in the Networking service, which we shall refer to
as the "provisioning" network, and add it in the ``neutron`` section of the
ironic-conductor configuration file. Using the ``neutron`` network interface
as the "provisioning" network. Using the ``neutron`` network interface
requires that ``provisioning_network`` and ``cleaning_network``
configuration options are set to valid identifiers (UUID or name) of
networks in the Networking service. If these options are not set correctly,
cleaning or provisioning will fail to start::
cleaning or provisioning will fail to start. There are two ways to set these
values:
- Under the ``neutron`` section of ironic configuration file:
.. code-block:: ini
[neutron]
...
cleaning_network = $CLEAN_UUID_OR_NAME
provisioning_network = $PROVISION_UUID_OR_NAME
- Under ``provisioning_network`` and ``cleaning_network`` keys of the node's
``driver_info`` field as ``driver_info['provisioning_network']`` and
``driver_info['cleaning_network']`` respectively.
.. note::
If these ``provisioning_network`` and ``cleaning_network`` values are
not specified in node's `driver_info` then ironic falls back to the
configuration in the ``neutron`` section.
Please refer to :doc:`configure-cleaning` for more information about
cleaning.

14
doc/source/install/enrollment.rst
View File

@ -249,6 +249,20 @@ and may be combined if desired.
See :doc:`configure-glance-images` for details.
#. Optionally you can specify the provisioning and/or cleaning network UUID
or name in the node's ``driver_info``. The ``neutron`` network interface
requires both ``provisioning_network`` and ``cleaning_network``, while
the ``flat`` network interface requires the ``cleaning_network`` to be set
either in the configuration or on the nodes. For example:
.. code-block:: console
$ openstack baremetal node set $NODE_UUID \
--driver-info cleaning_network=$CLEAN_UUID_OR_NAME \
--driver-info provisioning_network=$PROVISION_UUID_OR_NAME
See :doc:`configure-tenant-networks` for details.
#. You must also inform the Bare Metal service of the network interface cards
which are part of the node by creating a port with each NIC's MAC address.
These MAC addresses are passed to the Networking service during instance

45
ironic/common/neutron.py
View File

@ -568,25 +568,46 @@ class NeutronNetworkInterfaceMixin(object):
_provisioning_network_uuid = None
_rescuing_network_uuid = None
def get_cleaning_network_uuid(self, context=None):
if self._cleaning_network_uuid is None:
def get_cleaning_network_uuid(self, task):
cleaning_network = (
task.node.driver_info.get('cleaning_network') or
CONF.neutron.cleaning_network
)
# NOTE(dtantsur): if the last used cleaning network UUID is
# the same as the new one, we can skip validating it.
if (self._cleaning_network_uuid is None or
self._cleaning_network_uuid != cleaning_network):
self._cleaning_network_uuid = validate_network(
CONF.neutron.cleaning_network,
_('cleaning network'), context=context)
cleaning_network, _('cleaning network'),
context=task.context)
return self._cleaning_network_uuid
def get_provisioning_network_uuid(self, context=None):
if self._provisioning_network_uuid is None:
def get_provisioning_network_uuid(self, task):
provisioning_network = (
task.node.driver_info.get('provisioning_network') or
CONF.neutron.provisioning_network
)
# NOTE(dtantsur): if the last used provisioning network UUID is
# the same as the new one, we can skip validating it.
if (self._provisioning_network_uuid is None or
self._provisioning_network_uuid != provisioning_network):
self._provisioning_network_uuid = validate_network(
CONF.neutron.provisioning_network,
_('provisioning network'), context=context)
provisioning_network, _('provisioning network'),
context=task.context)
return self._provisioning_network_uuid
def get_rescuing_network_uuid(self, context=None):
# TODO(stendulker): FlatNetwork should not use this method.
# FlatNetwork uses tenant network for rescue operation.
if self._rescuing_network_uuid is None:
def get_rescuing_network_uuid(self, task):
rescuing_network = (
task.node.driver_info.get('rescuing_network') or
CONF.neutron.rescuing_network
)
# NOTE(dtantsur): if the last used provisioning network UUID is
# the same as the new one, we can skip validating it.
if (self._rescuing_network_uuid is None or
self._rescuing_network_uuid != rescuing_network):
self._rescuing_network_uuid = validate_network(
CONF.neutron.rescuing_network,
_('rescuing network'), context=context)
rescuing_network, _('rescuing network'),
context=task.context)
return self._rescuing_network_uuid

9
ironic/drivers/modules/network/flat.py
View File

@ -51,7 +51,7 @@ class FlatNetwork(common.NeutronVIFPortIDMixin,
is invalid.
:raises: MissingParameterValue, if some parameters are missing.
"""
self.get_cleaning_network_uuid(context=task.context)
self.get_cleaning_network_uuid(task)
def add_provisioning_network(self, task):
"""Add the provisioning network to a node.
@ -117,11 +117,10 @@ class FlatNetwork(common.NeutronVIFPortIDMixin,
"""
# If we have left over ports from a previous cleaning, remove them
neutron.rollback_ports(task,
self.get_cleaning_network_uuid(
context=task.context))
self.get_cleaning_network_uuid(task))
LOG.info('Adding cleaning network to node %s', task.node.uuid)
vifs = neutron.add_ports_to_network(
task, self.get_cleaning_network_uuid(context=task.context))
task, self.get_cleaning_network_uuid(task))
for port in task.ports:
if port.uuid in vifs:
internal_info = port.internal_info
@ -139,7 +138,7 @@ class FlatNetwork(common.NeutronVIFPortIDMixin,
LOG.info('Removing ports from cleaning network for node %s',
task.node.uuid)
neutron.remove_ports_from_network(
task, self.get_cleaning_network_uuid(context=task.context))
task, self.get_cleaning_network_uuid(task))
for port in task.ports:
if 'cleaning_vif_port_id' in port.internal_info:
internal_info = port.internal_info

24
ironic/drivers/modules/network/neutron.py
View File

@ -57,8 +57,8 @@ class NeutronNetwork(common.NeutronVIFPortIDMixin,
is invalid.
:raises: MissingParameterValue, if some parameters are missing.
"""
self.get_cleaning_network_uuid(context=task.context)
self.get_provisioning_network_uuid(context=task.context)
self.get_cleaning_network_uuid(task)
self.get_provisioning_network_uuid(task)
def add_provisioning_network(self, task):
"""Add the provisioning network to a node.
@ -69,11 +69,11 @@ class NeutronNetwork(common.NeutronVIFPortIDMixin,
# If we have left over ports from a previous provision attempt, remove
# them
neutron.rollback_ports(
task, self.get_provisioning_network_uuid(context=task.context))
task, self.get_provisioning_network_uuid(task))
LOG.info('Adding provisioning network to node %s',
task.node.uuid)
vifs = neutron.add_ports_to_network(
task, self.get_provisioning_network_uuid(context=task.context),
task, self.get_provisioning_network_uuid(task),
security_groups=CONF.neutron.provisioning_network_security_groups)
for port in task.ports:
if port.uuid in vifs:
@ -91,7 +91,7 @@ class NeutronNetwork(common.NeutronVIFPortIDMixin,
LOG.info('Removing provisioning network from node %s',
task.node.uuid)
neutron.remove_ports_from_network(
task, self.get_provisioning_network_uuid(context=task.context))
task, self.get_provisioning_network_uuid(task))
for port in task.ports:
if 'provisioning_vif_port_id' in port.internal_info:
internal_info = port.internal_info
@ -107,13 +107,12 @@ class NeutronNetwork(common.NeutronVIFPortIDMixin,
:returns: a dictionary in the form {port.uuid: neutron_port['id']}
"""
# If we have left over ports from a previous cleaning, remove them
neutron.rollback_ports(task, self.get_cleaning_network_uuid(
context=task.context))
neutron.rollback_ports(task, self.get_cleaning_network_uuid(task))
LOG.info('Adding cleaning network to node %s', task.node.uuid)
security_groups = CONF.neutron.cleaning_network_security_groups
vifs = neutron.add_ports_to_network(
task,
self.get_cleaning_network_uuid(context=task.context),
self.get_cleaning_network_uuid(task),
security_groups=security_groups)
for port in task.ports:
if port.uuid in vifs:
@ -132,7 +131,7 @@ class NeutronNetwork(common.NeutronVIFPortIDMixin,
LOG.info('Removing cleaning network from node %s',
task.node.uuid)
neutron.remove_ports_from_network(
task, self.get_cleaning_network_uuid(context=task.context))
task, self.get_cleaning_network_uuid(task))
for port in task.ports:
if 'cleaning_vif_port_id' in port.internal_info:
internal_info = port.internal_info
@ -147,13 +146,12 @@ class NeutronNetwork(common.NeutronVIFPortIDMixin,
:returns: a dictionary in the form {port.uuid: neutron_port['id']}
"""
# If we have left over ports from a previous rescue, remove them
neutron.rollback_ports(task, self.get_rescuing_network_uuid(
context=task.context))
neutron.rollback_ports(task, self.get_rescuing_network_uuid(task))
LOG.info('Adding rescuing network to node %s', task.node.uuid)
security_groups = CONF.neutron.rescuing_network_security_groups
vifs = neutron.add_ports_to_network(
task,
self.get_rescuing_network_uuid(context=task.context),
self.get_rescuing_network_uuid(task),
security_groups=security_groups)
for port in task.ports:
if port.uuid in vifs:
@ -172,7 +170,7 @@ class NeutronNetwork(common.NeutronVIFPortIDMixin,
LOG.info('Removing rescuing network from node %s',
task.node.uuid)
neutron.remove_ports_from_network(
task, self.get_rescuing_network_uuid(context=task.context))
task, self.get_rescuing_network_uuid(task))
for port in task.ports:
if 'rescuing_vif_port_id' in port.internal_info:
internal_info = port.internal_info

58
ironic/tests/unit/drivers/modules/network/test_flat.py
View File

@ -81,6 +81,19 @@ class TestFlatInterface(db_base.DbTestCase):
CONF.neutron.cleaning_network,
'cleaning network', context=task.context)
@mock.patch.object(neutron, 'validate_network', autospec=True)
def test_validate_from_node(self, validate_mock):
cleaning_network_uuid = '3aea0de6-4b92-44da-9aa0-52d134c83fdf'
driver_info = self.node.driver_info
driver_info['cleaning_network'] = cleaning_network_uuid
self.node.driver_info = driver_info
self.node.save()
with task_manager.acquire(self.context, self.node.id) as task:
self.interface.validate(task)
validate_mock.assert_called_once_with(
cleaning_network_uuid,
'cleaning network', context=task.context)
@mock.patch.object(neutron, 'validate_network',
side_effect=lambda n, t, context=None: n)
@mock.patch.object(neutron, 'add_ports_to_network')
@ -101,6 +114,32 @@ class TestFlatInterface(db_base.DbTestCase):
self.assertEqual('vif-port-id',
self.port.internal_info['cleaning_vif_port_id'])
@mock.patch.object(neutron, 'validate_network',
side_effect=lambda n, t, context=None: n)
@mock.patch.object(neutron, 'add_ports_to_network')
@mock.patch.object(neutron, 'rollback_ports')
def test_add_cleaning_network_from_node(self, rollback_mock, add_mock,
validate_mock):
add_mock.return_value = {self.port.uuid: 'vif-port-id'}
# Make sure that changing the network UUID works
for cleaning_network_uuid in ['3aea0de6-4b92-44da-9aa0-52d134c83fdf',
'438be438-6aae-4fb1-bbcb-613ad7a38286']:
driver_info = self.node.driver_info
driver_info['cleaning_network'] = cleaning_network_uuid
self.node.driver_info = driver_info
self.node.save()
with task_manager.acquire(self.context, self.node.id) as task:
self.interface.add_cleaning_network(task)
rollback_mock.assert_called_with(
task, cleaning_network_uuid)
add_mock.assert_called_with(task, cleaning_network_uuid)
validate_mock.assert_called_with(
cleaning_network_uuid,
'cleaning network', context=task.context)
self.port.refresh()
self.assertEqual('vif-port-id',
self.port.internal_info['cleaning_vif_port_id'])
@mock.patch.object(neutron, 'validate_network',
side_effect=lambda n, t, context=None: n)
@mock.patch.object(neutron, 'remove_ports_from_network')
@ -115,6 +154,25 @@ class TestFlatInterface(db_base.DbTestCase):
self.port.refresh()
self.assertNotIn('cleaning_vif_port_id', self.port.internal_info)
@mock.patch.object(neutron, 'validate_network',
side_effect=lambda n, t, context=None: n)
@mock.patch.object(neutron, 'remove_ports_from_network')
def test_remove_cleaning_network_from_node(self, remove_mock,
validate_mock):
cleaning_network_uuid = '3aea0de6-4b92-44da-9aa0-52d134c83fdf'
driver_info = self.node.driver_info
driver_info['cleaning_network'] = cleaning_network_uuid
self.node.driver_info = driver_info
self.node.save()
with task_manager.acquire(self.context, self.node.id) as task:
self.interface.remove_cleaning_network(task)
remove_mock.assert_called_once_with(task, cleaning_network_uuid)
validate_mock.assert_called_once_with(
cleaning_network_uuid,
'cleaning network', context=task.context)
self.port.refresh()
self.assertNotIn('cleaning_vif_port_id', self.port.internal_info)
@mock.patch.object(neutron, 'get_client')
def test_add_provisioning_network_set_binding_host_id(
self, client_mock):

136
ironic/tests/unit/drivers/modules/network/test_neutron.py
View File

@ -127,6 +127,37 @@ class NeutronInterfaceTestCase(db_base.DbTestCase):
self.assertEqual(self.neutron_port['id'],
self.port.internal_info['provisioning_vif_port_id'])
@mock.patch.object(neutron_common, 'validate_network',
side_effect=lambda n, t, context=None: n)
@mock.patch.object(neutron_common, 'rollback_ports')
@mock.patch.object(neutron_common, 'add_ports_to_network')
def test_add_provisioning_network_from_node(self, add_ports_mock,
rollback_mock, validate_mock):
self.port.internal_info = {'provisioning_vif_port_id': 'vif-port-id'}
self.port.save()
add_ports_mock.return_value = {self.port.uuid: self.neutron_port['id']}
# Make sure that changing the network UUID works
for provisioning_network_uuid in [
'3aea0de6-4b92-44da-9aa0-52d134c83fdf',
'438be438-6aae-4fb1-bbcb-613ad7a38286']:
driver_info = self.node.driver_info
driver_info['provisioning_network'] = provisioning_network_uuid
self.node.driver_info = driver_info
self.node.save()
with task_manager.acquire(self.context, self.node.id) as task:
self.interface.add_provisioning_network(task)
rollback_mock.assert_called_with(
task, provisioning_network_uuid)
add_ports_mock.assert_called_with(
task, provisioning_network_uuid,
security_groups=[])
validate_mock.assert_called_with(
provisioning_network_uuid,
'provisioning network', context=task.context)
self.port.refresh()
self.assertEqual(self.neutron_port['id'],
self.port.internal_info['provisioning_vif_port_id'])
@mock.patch.object(neutron_common, 'validate_network',
lambda n, t, context=None: n)
@mock.patch.object(neutron_common, 'rollback_ports')
@ -169,6 +200,28 @@ class NeutronInterfaceTestCase(db_base.DbTestCase):
self.port.refresh()
self.assertNotIn('provisioning_vif_port_id', self.port.internal_info)
@mock.patch.object(neutron_common, 'validate_network',
side_effect=lambda n, t, context=None: n)
@mock.patch.object(neutron_common, 'remove_ports_from_network')
def test_remove_provisioning_network_from_node(self, remove_ports_mock,
validate_mock):
self.port.internal_info = {'provisioning_vif_port_id': 'vif-port-id'}
self.port.save()
provisioning_network_uuid = '3aea0de6-4b92-44da-9aa0-52d134c83f9c'
driver_info = self.node.driver_info
driver_info['provisioning_network'] = provisioning_network_uuid
self.node.driver_info = driver_info
self.node.save()
with task_manager.acquire(self.context, self.node.id) as task:
self.interface.remove_provisioning_network(task)
remove_ports_mock.assert_called_once_with(
task, provisioning_network_uuid)
validate_mock.assert_called_once_with(
provisioning_network_uuid,
'provisioning network', context=task.context)
self.port.refresh()
self.assertNotIn('provisioning_vif_port_id', self.port.internal_info)
@mock.patch.object(neutron_common, 'validate_network',
side_effect=lambda n, t, context=None: n)
@mock.patch.object(neutron_common, 'rollback_ports')
@ -188,6 +241,31 @@ class NeutronInterfaceTestCase(db_base.DbTestCase):
self.assertEqual(self.neutron_port['id'],
self.port.internal_info['cleaning_vif_port_id'])
@mock.patch.object(neutron_common, 'validate_network',
side_effect=lambda n, t, context=None: n)
@mock.patch.object(neutron_common, 'rollback_ports')
@mock.patch.object(neutron_common, 'add_ports_to_network')
def test_add_cleaning_network_from_node(self, add_ports_mock,
rollback_mock, validate_mock):
add_ports_mock.return_value = {self.port.uuid: self.neutron_port['id']}
# Make sure that changing the network UUID works
for cleaning_network_uuid in ['3aea0de6-4b92-44da-9aa0-52d134c83fdf',
'438be438-6aae-4fb1-bbcb-613ad7a38286']:
driver_info = self.node.driver_info
driver_info['cleaning_network'] = cleaning_network_uuid
self.node.driver_info = driver_info
self.node.save()
with task_manager.acquire(self.context, self.node.id) as task:
res = self.interface.add_cleaning_network(task)
rollback_mock.assert_called_with(task, cleaning_network_uuid)
self.assertEqual(res, add_ports_mock.return_value)
validate_mock.assert_called_with(
cleaning_network_uuid,
'cleaning network', context=task.context)
self.port.refresh()
self.assertEqual(self.neutron_port['id'],
self.port.internal_info['cleaning_vif_port_id'])
@mock.patch.object(neutron_common, 'validate_network',
lambda n, t, context=None: n)
@mock.patch.object(neutron_common, 'rollback_ports')
@ -227,6 +305,28 @@ class NeutronInterfaceTestCase(db_base.DbTestCase):
self.port.refresh()
self.assertNotIn('cleaning_vif_port_id', self.port.internal_info)
@mock.patch.object(neutron_common, 'validate_network',
side_effect=lambda n, t, context=None: n)
@mock.patch.object(neutron_common, 'remove_ports_from_network')
def test_remove_cleaning_network_from_node(self, remove_ports_mock,
validate_mock):
self.port.internal_info = {'cleaning_vif_port_id': 'vif-port-id'}
self.port.save()
cleaning_network_uuid = '3aea0de6-4b92-44da-9aa0-52d134c83fdf'
driver_info = self.node.driver_info
driver_info['cleaning_network'] = cleaning_network_uuid
self.node.driver_info = driver_info
self.node.save()
with task_manager.acquire(self.context, self.node.id) as task:
self.interface.remove_cleaning_network(task)
remove_ports_mock.assert_called_once_with(
task, cleaning_network_uuid)
validate_mock.assert_called_once_with(
cleaning_network_uuid,
'cleaning network', context=task.context)
self.port.refresh()
self.assertNotIn('cleaning_vif_port_id', self.port.internal_info)
@mock.patch.object(neutron_common, 'validate_network',
side_effect=lambda n, t, context=None: n)
@mock.patch.object(neutron_common, 'rollback_ports')
@ -258,6 +358,42 @@ class NeutronInterfaceTestCase(db_base.DbTestCase):
other_port.internal_info['rescuing_vif_port_id'])
self.assertNotIn('rescuing_vif_port_id', self.port.internal_info)
@mock.patch.object(neutron_common, 'validate_network',
side_effect=lambda n, t, context=None: n)
@mock.patch.object(neutron_common, 'rollback_ports')
@mock.patch.object(neutron_common, 'add_ports_to_network')
def test_add_rescuing_network_from_node(self, add_ports_mock,
rollback_mock, validate_mock):
other_port = utils.create_test_port(
self.context, node_id=self.node.id,
address='52:54:00:cf:2d:33',
uuid=uuidutils.generate_uuid(),
extra={'vif_port_id': uuidutils.generate_uuid()})
neutron_other_port = {'id': uuidutils.generate_uuid(),
'mac_address': '52:54:00:cf:2d:33'}
add_ports_mock.return_value = {
other_port.uuid: neutron_other_port['id']}
rescuing_network_uuid = '3aea0de6-4b92-44da-9aa0-52d134c83fdf'
driver_info = self.node.driver_info
driver_info['rescuing_network'] = rescuing_network_uuid
self.node.driver_info = driver_info
self.node.save()
with task_manager.acquire(self.context, self.node.id) as task:
res = self.interface.add_rescuing_network(task)
add_ports_mock.assert_called_once_with(
task, rescuing_network_uuid,
security_groups=[])
rollback_mock.assert_called_once_with(
task, rescuing_network_uuid)
self.assertEqual(add_ports_mock.return_value, res)
validate_mock.assert_called_once_with(
rescuing_network_uuid,
'rescuing network', context=task.context)
other_port.refresh()
self.assertEqual(neutron_other_port['id'],
other_port.internal_info['rescuing_vif_port_id'])
self.assertNotIn('rescuing_vif_port_id', self.port.internal_info)
@mock.patch.object(neutron_common, 'validate_network',
lambda n, t, context=None: n)
@mock.patch.object(neutron_common, 'rollback_ports')

16
releasenotes/notes/setting_provisioning_cleaning_network-fb60caa1cf59cdcf.yaml Normal file
View File

@ -0,0 +1,16 @@
---
features:
- |
Allows specifying the provisioning and cleaning networks on a node as
``driver_info['cleaning_network']`` and
``driver_info['provisioning_network']`` respectively. If these
values are defined in the node's driver_info at the time of
provisioning or cleaning the baremetal node, they will be used.
Otherwise, the configuration options ``cleaning_network`` and
``provisioning_network`` are used as before.
fixes:
- |
A network UUID for provisioning and cleaning network is no longer cached
locally if the requested network (either via node's ``driver_info`` or
via configuration options) is specified as a network name. Fixes the
situation when a network is re-created with the same name.