diff --git a/metalsmith/_cmd.py b/metalsmith/_cmd.py index aee00fa..cced8ee 100644 --- a/metalsmith/_cmd.py +++ b/metalsmith/_cmd.py @@ -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, diff --git a/metalsmith/_nics.py b/metalsmith/_nics.py new file mode 100644 index 0000000..785002e --- /dev/null +++ b/metalsmith/_nics.py @@ -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": ""}``. + :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": ""}`` + or ``{"network": "", "fixed_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)}) diff --git a/metalsmith/_provisioner.py b/metalsmith/_provisioner.py index 19a06ef..0723722 100644 --- a/metalsmith/_provisioner.py +++ b/metalsmith/_provisioner.py @@ -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": ""}``) or a network to create a port on (``{"network": ""}``). + 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) diff --git a/metalsmith/_utils.py b/metalsmith/_utils.py index 9aa0d19..9e6cb69 100644 --- a/metalsmith/_utils.py +++ b/metalsmith/_utils.py @@ -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 = {} diff --git a/metalsmith/test/test_cmd.py b/metalsmith/test/test_cmd.py index 739b996..a067e84 100644 --- a/metalsmith/test/test_cmd.py +++ b/metalsmith/test/test_cmd.py @@ -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'] diff --git a/metalsmith/test/test_provisioner.py b/metalsmith/test/test_provisioner.py index 4523468..d5258d7 100644 --- a/metalsmith/test/test_provisioner.py +++ b/metalsmith/test/test_provisioner.py @@ -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, diff --git a/roles/metalsmith_deployment/README.rst b/roles/metalsmith_deployment/README.rst index dd588d2..2524538 100644 --- a/roles/metalsmith_deployment/README.rst +++ b/roles/metalsmith_deployment/README.rst @@ -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: