Accept optional fixed_ip for nics

Covers a popular case of nodes having a pre-defined IP.

Also moves NICs code to a separate module for clarity.

Change-Id: Id8272cc30728ca68e7ce2efd4f3a2f9887ef7449
Story: #2002171
Task: #26340
This commit is contained in:
Dmitry Tantsur 2018-09-12 18:03:20 +02:00
parent 98ad1d86cd
commit 47fe222b6d
7 changed files with 289 additions and 128 deletions

View File

@ -35,10 +35,17 @@ def _is_http(smth):
class NICAction(argparse.Action):
def __call__(self, parser, namespace, values, option_string=None):
assert option_string in ('--port', '--network')
assert option_string in ('--port', '--network', '--ip')
nics = getattr(namespace, self.dest, None) or []
if option_string == '--network':
nics.append({'network': values})
elif option_string == '--ip':
try:
network, ip = values.split(':', 1)
except ValueError:
raise argparse.ArgumentError(
self, '--ip format is NETWORK:IP, got %s' % values)
nics.append({'network': network, 'fixed_ip': ip})
else:
nics.append({'port': values})
setattr(namespace, self.dest, nics)
@ -177,6 +184,8 @@ def _parse_args(args, config):
dest='nics', action=NICAction)
deploy.add_argument('--port', help='port to attach (name or UUID)',
dest='nics', action=NICAction)
deploy.add_argument('--ip', help='attach IP from the network',
dest='nics', metavar='NETWORK:IP', action=NICAction)
deploy.add_argument('--netboot', action='store_true',
help='boot from network instead of local disk')
deploy.add_argument('--root-size', type=int,

168
metalsmith/_nics.py Normal file
View File

@ -0,0 +1,168 @@
# Copyright 2018 Red Hat, Inc.
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
# implied.
# See the License for the specific language governing permissions and
# limitations under the License.
import collections
import logging
from metalsmith import _utils
from metalsmith import exceptions
LOG = logging.getLogger(__name__)
class NICs(object):
"""Requested NICs."""
def __init__(self, api, node, nics):
if nics is None:
nics = []
if not isinstance(nics, collections.Sequence):
raise TypeError("NICs must be a list of dicts")
for nic in nics:
if not isinstance(nic, collections.Mapping):
raise TypeError("Each NIC must be a dict got %s" % nic)
self._node = node
self._api = api
self._nics = nics
self._validated = None
self.created_ports = []
self.attached_ports = []
def validate(self):
"""Validate provided NIC records."""
if self._validated is not None:
return
result = []
for nic in self._nics:
if 'port' in nic:
result.append(('port', self._get_port(nic)))
elif 'network' in nic:
result.append(('network', self._get_network(nic)))
else:
raise exceptions.InvalidNIC(
'Unknown NIC record type, export "port" or "network", '
'got %s' % nic)
self._validated = result
def create_and_attach_ports(self):
"""Attach ports to the node, creating them if requested."""
self.validate()
for nic_type, nic in self._validated:
if nic_type == 'network':
port = self._api.connection.network.create_port(**nic)
self.created_ports.append(port.id)
LOG.info('Created port %(port)s for node %(node)s with '
'%(nic)s', {'port': _utils.log_res(port),
'node': _utils.log_node(self._node),
'nic': nic})
else:
port = nic
self._api.attach_port_to_node(self._node.uuid, port.id)
LOG.info('Attached port %(port)s to node %(node)s',
{'port': _utils.log_res(port),
'node': _utils.log_node(self._node)})
self.attached_ports.append(port.id)
def detach_and_delete_ports(self):
"""Detach attached port and delete previously created ones."""
detach_and_delete_ports(self._api, self._node, self.created_ports,
self.attached_ports)
def _get_port(self, nic):
"""Validate and get the NIC information for a port.
:param nic: NIC information in the form ``{"port": "<port ident>"}``.
:returns: `Port` object to use.
"""
unexpected = set(nic) - {'port'}
if unexpected:
raise exceptions.InvalidNIC(
'Unexpected fields for a port: %s' % ', '.join(unexpected))
try:
port = self._api.connection.network.find_port(
nic['port'], ignore_missing=False)
except Exception as exc:
raise exceptions.InvalidNIC(
'Cannot find port %(port)s: %(error)s' %
{'port': nic['port'], 'error': exc})
return port
def _get_network(self, nic):
"""Validate and get the NIC information for a network.
:param nic: NIC information in the form ``{"network": "<net ident>"}``
or ``{"network": "<net ident>", "fixed_ip": "<desired IP>"}``.
:returns: keyword arguments to use when creating a port.
"""
unexpected = set(nic) - {'network', 'fixed_ip'}
if unexpected:
raise exceptions.InvalidNIC(
'Unexpected fields for a network: %s' % ', '.join(unexpected))
try:
network = self._api.connection.network.find_network(
nic['network'], ignore_missing=False)
except Exception as exc:
raise exceptions.InvalidNIC(
'Cannot find network %(net)s: %(error)s' %
{'net': nic['network'], 'error': exc})
port_args = {'network_id': network.id}
if nic.get('fixed_ip'):
port_args['fixed_ips'] = [{'ip_address': nic['fixed_ip']}]
return port_args
def detach_and_delete_ports(api, node, created_ports, attached_ports):
"""Detach attached port and delete previously created ones.
:param api: `Api` instance.
:param node: `Node` object to detach ports from.
:param created_ports: List of IDs of previously created ports.
:param attached_ports: List of IDs of previously attached_ports.
"""
for port_id in set(attached_ports + created_ports):
LOG.debug('Detaching port %(port)s from node %(node)s',
{'port': port_id, 'node': node.uuid})
try:
api.detach_port_from_node(node, port_id)
except Exception as exc:
LOG.debug('Failed to remove VIF %(vif)s from node %(node)s, '
'assuming already removed: %(exc)s',
{'vif': port_id, 'node': _utils.log_node(node),
'exc': exc})
for port_id in created_ports:
LOG.debug('Deleting port %s', port_id)
try:
api.connection.network.delete_port(port_id,
ignore_missing=False)
except Exception as exc:
LOG.warning('Failed to delete neutron port %(port)s: %(exc)s',
{'port': port_id, 'exc': exc})
else:
LOG.info('Deleted port %(port)s for node %(node)s',
{'port': port_id, 'node': _utils.log_node(node)})

View File

@ -24,6 +24,7 @@ import six
from metalsmith import _config
from metalsmith import _instance
from metalsmith import _nics
from metalsmith import _os_api
from metalsmith import _scheduler
from metalsmith import _utils
@ -218,6 +219,8 @@ class Provisioner(object):
Each item is a dict with a key describing the type of the NIC:
either a port (``{"port": "<port name or ID>"}``) or a network
to create a port on (``{"network": "<network name or ID>"}``).
A network record can optionally feature a ``fixed_ip`` argument
to use this specific fixed IP from a suitable subnet.
:param root_size_gb: The size of the root partition. By default
the value of the local_gb property is used.
:param swap_size_mb: The size of the swap partition. It's an error
@ -253,8 +256,7 @@ class Provisioner(object):
root_size_gb = root_disk_size
node = self._check_node_for_deploy(node)
created_ports = []
attached_ports = []
nics = _nics.NICs(self._api, node, nics)
try:
hostname = self._check_hostname(node, hostname)
@ -262,7 +264,7 @@ class Provisioner(object):
image._validate(self.connection)
nics = self._get_nics(nics or [])
nics.validate()
if capabilities is None:
capabilities = node.instance_info.get('capabilities') or {}
@ -272,15 +274,14 @@ class Provisioner(object):
_utils.log_node(node))
return node
self._create_and_attach_ports(node, nics,
created_ports, attached_ports)
nics.create_and_attach_ports()
capabilities['boot_option'] = 'netboot' if netboot else 'local'
updates = {'/instance_info/root_gb': root_size_gb,
'/instance_info/capabilities': capabilities,
'/extra/%s' % _CREATED_PORTS: created_ports,
'/extra/%s' % _ATTACHED_PORTS: attached_ports,
'/extra/%s' % _CREATED_PORTS: nics.created_ports,
'/extra/%s' % _ATTACHED_PORTS: nics.attached_ports,
'/instance_info/%s' % _os_api.HOSTNAME_FIELD: hostname}
updates.update(image._node_updates(self.connection))
if traits is not None:
@ -304,8 +305,7 @@ class Provisioner(object):
try:
LOG.error('Deploy attempt failed on node %s, cleaning up',
_utils.log_node(node))
self._clean_up(node, created_ports=created_ports,
attached_ports=attached_ports)
self._clean_up(node, nics=nics)
except Exception:
LOG.exception('Clean up failed')
@ -326,97 +326,6 @@ class Provisioner(object):
return instance
def _get_nics(self, nics):
"""Validate and get the NICs."""
_utils.validate_nics(nics)
result = []
for nic in nics:
nic_type, nic_id = next(iter(nic.items()))
if nic_type == 'network':
try:
network = self.connection.network.find_network(
nic_id, ignore_missing=False)
except Exception as exc:
raise exceptions.InvalidNIC(
'Cannot find network %(net)s: %(error)s' %
{'net': nic_id, 'error': exc})
else:
result.append((nic_type, network))
else:
try:
port = self.connection.network.find_port(
nic_id, ignore_missing=False)
except Exception as exc:
raise exceptions.InvalidNIC(
'Cannot find port %(port)s: %(error)s' %
{'port': nic_id, 'error': exc})
else:
result.append((nic_type, port))
return result
def _create_and_attach_ports(self, node, nics, created_ports,
attached_ports):
"""Create and attach ports on given networks."""
for nic_type, nic in nics:
if nic_type == 'network':
port = self.connection.network.create_port(network_id=nic.id)
created_ports.append(port.id)
LOG.info('Created port %(port)s for node %(node)s on '
'network %(net)s',
{'port': _utils.log_res(port),
'node': _utils.log_node(node),
'net': _utils.log_res(nic)})
else:
port = nic
self._api.attach_port_to_node(node.uuid, port.id)
LOG.info('Attached port %(port)s to node %(node)s',
{'port': _utils.log_res(port),
'node': _utils.log_node(node)})
attached_ports.append(port.id)
def _delete_ports(self, node, created_ports=None, attached_ports=None):
if created_ports is None:
created_ports = node.extra.get(_CREATED_PORTS, [])
if attached_ports is None:
attached_ports = node.extra.get(_ATTACHED_PORTS, [])
for port_id in set(attached_ports + created_ports):
LOG.debug('Detaching port %(port)s from node %(node)s',
{'port': port_id, 'node': node.uuid})
try:
self._api.detach_port_from_node(node, port_id)
except Exception as exc:
LOG.debug('Failed to remove VIF %(vif)s from node %(node)s, '
'assuming already removed: %(exc)s',
{'vif': port_id, 'node': _utils.log_node(node),
'exc': exc})
for port_id in created_ports:
LOG.debug('Deleting port %s', port_id)
try:
self.connection.network.delete_port(port_id,
ignore_missing=False)
except Exception as exc:
LOG.warning('Failed to delete neutron port %(port)s: %(exc)s',
{'port': port_id, 'exc': exc})
else:
LOG.info('Deleted port %(port)s for node %(node)s',
{'port': port_id, 'node': _utils.log_node(node)})
update = {'/extra/%s' % item: _os_api.REMOVE
for item in (_CREATED_PORTS, _ATTACHED_PORTS)}
update['/instance_info/%s' % _os_api.HOSTNAME_FIELD] = _os_api.REMOVE
LOG.debug('Updating node %(node)s with %(updates)s',
{'node': _utils.log_node(node), 'updates': update})
try:
self._api.update_node(node, update)
except Exception as exc:
LOG.debug('Failed to clear node %(node)s extra: %(exc)s',
{'node': _utils.log_node(node), 'exc': exc})
def wait_for_provisioning(self, nodes, timeout=None, delay=15):
"""Wait for nodes to be provisioned.
@ -495,8 +404,26 @@ class Provisioner(object):
LOG.debug('All nodes reached state %s', state)
return finished_nodes
def _clean_up(self, node, created_ports=None, attached_ports=None):
self._delete_ports(node, created_ports, attached_ports)
def _clean_up(self, node, nics=None):
if nics is None:
created_ports = node.extra.get(_CREATED_PORTS, [])
attached_ports = node.extra.get(_ATTACHED_PORTS, [])
_nics.detach_and_delete_ports(self._api, node, created_ports,
attached_ports)
else:
nics.detach_and_delete_ports()
update = {'/extra/%s' % item: _os_api.REMOVE
for item in (_CREATED_PORTS, _ATTACHED_PORTS)}
update['/instance_info/%s' % _os_api.HOSTNAME_FIELD] = _os_api.REMOVE
LOG.debug('Updating node %(node)s with %(updates)s',
{'node': _utils.log_node(node), 'updates': update})
try:
self._api.update_node(node, update)
except Exception as exc:
LOG.debug('Failed to clear node %(node)s extra: %(exc)s',
{'node': _utils.log_node(node), 'exc': exc})
LOG.debug('Releasing lock on node %s', _utils.log_node(node))
self._api.release_node(node)

View File

@ -13,7 +13,6 @@
# See the License for the specific language governing permissions and
# limitations under the License.
import collections
import re
import six
@ -94,26 +93,6 @@ def is_hostname_safe(hostname):
return _HOSTNAME_RE.match(hostname) is not None
def validate_nics(nics):
"""Validate NICs."""
if not isinstance(nics, collections.Sequence):
raise TypeError("NICs must be a list of dicts")
unknown_nic_types = set()
for nic in nics:
if not isinstance(nic, collections.Mapping) or len(nic) != 1:
raise TypeError("Each NIC must be a dict with one item, "
"got %s" % nic)
nic_type = next(iter(nic))
if nic_type not in ('port', 'network'):
unknown_nic_types.add(nic_type)
if unknown_nic_types:
raise ValueError("Unexpected NIC type(s) %s, supported values are "
"'port' and 'network'" % ', '.join(unknown_nic_types))
def parse_checksums(checksums):
"""Parse standard checksums file."""
result = {}

View File

@ -331,6 +331,20 @@ class TestDeploy(testtools.TestCase):
{'nics': [{'network': 'net1'}, {'port': 'port1'},
{'port': 'port2'}, {'network': 'net2'}]})
def test_args_ips(self, mock_pr):
args = ['deploy', '--image', 'myimg', '--resource-class', 'compute',
'--ip', 'private:10.0.0.2', '--ip', 'public:8.0.8.0']
self._check(mock_pr, args, {},
{'nics': [{'network': 'private', 'fixed_ip': '10.0.0.2'},
{'network': 'public', 'fixed_ip': '8.0.8.0'}]})
def test_args_bad_ip(self, mock_pr):
args = ['deploy', '--image', 'myimg', '--resource-class', 'compute',
'--ip', 'private:10.0.0.2', '--ip', 'public']
self.assertRaises(SystemExit, _cmd.main, args)
self.assertFalse(mock_pr.return_value.reserve_node.called)
self.assertFalse(mock_pr.return_value.provision_node.called)
def test_args_hostname(self, mock_pr):
args = ['deploy', '--network', 'mynet', '--image', 'myimg',
'--hostname', 'host', '--resource-class', 'compute']

View File

@ -336,6 +336,26 @@ class TestProvisionNode(Base):
self.assertFalse(self.api.release_node.called)
self.assertFalse(self.conn.network.delete_port.called)
def test_ok_without_nics(self):
self.updates['/extra/metalsmith_created_ports'] = []
self.updates['/extra/metalsmith_attached_ports'] = []
inst = self.pr.provision_node(self.node, 'image')
self.assertEqual(inst.uuid, self.node.uuid)
self.assertEqual(inst.node, self.node)
self.assertFalse(self.conn.network.create_port.called)
self.assertFalse(self.conn.network.find_port.called)
self.assertFalse(self.api.attach_port_to_node.called)
self.api.update_node.assert_called_once_with(self.node, self.updates)
self.api.validate_node.assert_called_once_with(self.node,
validate_deploy=True)
self.api.node_action.assert_called_once_with(self.node, 'active',
configdrive=mock.ANY)
self.assertFalse(self.wait_mock.called)
self.assertFalse(self.api.release_node.called)
self.assertFalse(self.conn.network.delete_port.called)
def test_ok_with_source(self):
inst = self.pr.provision_node(self.node, sources.GlanceImage('image'),
[{'network': 'network'}])
@ -470,6 +490,28 @@ class TestProvisionNode(Base):
self.assertFalse(self.api.release_node.called)
self.assertFalse(self.conn.network.delete_port.called)
def test_with_ip(self):
inst = self.pr.provision_node(self.node, 'image',
[{'network': 'network',
'fixed_ip': '10.0.0.2'}])
self.assertEqual(inst.uuid, self.node.uuid)
self.assertEqual(inst.node, self.node)
self.conn.network.create_port.assert_called_once_with(
network_id=self.conn.network.find_network.return_value.id,
fixed_ips=[{'ip_address': '10.0.0.2'}])
self.api.attach_port_to_node.assert_called_once_with(
self.node.uuid, self.conn.network.create_port.return_value.id)
self.api.update_node.assert_called_once_with(self.node, self.updates)
self.api.validate_node.assert_called_once_with(self.node,
validate_deploy=True)
self.api.node_action.assert_called_once_with(self.node, 'active',
configdrive=mock.ANY)
self.assertFalse(self.wait_mock.called)
self.assertFalse(self.api.release_node.called)
self.assertFalse(self.conn.network.delete_port.called)
def test_whole_disk(self):
self.image.kernel_id = None
self.image.ramdisk_id = None
@ -1128,20 +1170,19 @@ abcd and-not-image-again
self.assertFalse(self.conn.network.create_port.called)
self.assertFalse(self.api.attach_port_to_node.called)
self.assertFalse(self.api.node_action.called)
self.api.release_node.assert_called_once_with(self.node)
def test_invalid_nic(self):
for item in ('string', ['string'], [{1: 2, 3: 4}]):
for item in ('string', ['string']):
self.assertRaisesRegex(TypeError, 'must be a dict',
self.pr.provision_node,
self.node, 'image', item)
self.assertFalse(self.conn.network.create_port.called)
self.assertFalse(self.api.attach_port_to_node.called)
self.assertFalse(self.api.node_action.called)
self.api.release_node.assert_called_with(self.node)
def test_invalid_nic_type(self):
self.assertRaisesRegex(ValueError, r'Unexpected NIC type\(s\) foo',
self.assertRaisesRegex(exceptions.InvalidNIC,
'Unknown NIC record type',
self.pr.provision_node,
self.node, 'image', [{'foo': 'bar'}])
self.assertFalse(self.conn.network.create_port.called)
@ -1149,6 +1190,19 @@ abcd and-not-image-again
self.assertFalse(self.api.node_action.called)
self.api.release_node.assert_called_once_with(self.node)
def test_invalid_nic_type_fields(self):
for item in ({'port': '1234', 'foo': 'bar'},
{'port': '1234', 'network': '4321'},
{'network': '4321', 'foo': 'bar'}):
self.assertRaisesRegex(exceptions.InvalidNIC,
'Unexpected fields',
self.pr.provision_node,
self.node, 'image', [item])
self.assertFalse(self.conn.network.create_port.called)
self.assertFalse(self.api.attach_port_to_node.called)
self.assertFalse(self.api.node_action.called)
self.api.release_node.assert_called_with(self.node)
def test_invalid_hostname(self):
self.assertRaisesRegex(ValueError, 'n_1 cannot be used as a hostname',
self.pr.provision_node,

View File

@ -88,6 +88,16 @@ Each instances has the following attributes:
- network: private
- network: ctlplane
can optionally take a fixed IP to assign:
.. code-block:: yaml
nics:
- network: private
fixed_ip: 10.0.0.2
- network: ctlplane
fixed_ip: 192.168.42.30
``port``
uses the provided pre-created port: