diff --git a/metalsmith/_cmd.py b/metalsmith/_cmd.py index 4d5b3a5..752a417 100644 --- a/metalsmith/_cmd.py +++ b/metalsmith/_cmd.py @@ -31,11 +31,9 @@ LOG = logging.getLogger(__name__) class NICAction(argparse.Action): def __call__(self, parser, namespace, values, option_string=None): - assert option_string in ('--port', '--network', '--ip') + assert option_string in ('--port', '--network', '--ip', '--subnet') nics = getattr(namespace, self.dest, None) or [] - if option_string == '--network': - nics.append({'network': values}) - elif option_string == '--ip': + if option_string == '--ip': try: network, ip = values.split(':', 1) except ValueError: @@ -43,7 +41,7 @@ class NICAction(argparse.Action): self, '--ip format is NETWORK:IP, got %s' % values) nics.append({'network': network, 'fixed_ip': ip}) else: - nics.append({'port': values}) + nics.append({option_string[2:]: values}) setattr(namespace, self.dest, nics) @@ -143,8 +141,10 @@ def _parse_args(args, config): help='image MD5 checksum or URL with checksums') deploy.add_argument('--image-kernel', help='URL of the image\'s kernel') deploy.add_argument('--image-ramdisk', help='URL of the image\'s ramdisk') - deploy.add_argument('--network', help='network to use (name or UUID)', - dest='nics', action=NICAction) + deploy.add_argument('--network', help='network to create a port on ' + '(name or UUID)', dest='nics', action=NICAction) + deploy.add_argument('--subnet', help='subnet to create a port on ' + '(name or UUID)', 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', diff --git a/metalsmith/_nics.py b/metalsmith/_nics.py index 8be912d..bcfbcbc 100644 --- a/metalsmith/_nics.py +++ b/metalsmith/_nics.py @@ -16,6 +16,8 @@ import collections import logging +from openstack import exceptions as sdk_exc + from metalsmith import _utils from metalsmith import exceptions @@ -55,10 +57,12 @@ class NICs(object): result.append(('port', self._get_port(nic))) elif 'network' in nic: result.append(('network', self._get_network(nic))) + elif 'subnet' in nic: + result.append(('subnet', self._get_subnet(nic))) else: raise exceptions.InvalidNIC( - 'Unknown NIC record type, export "port" or "network", ' - 'got %s' % nic) + 'Unknown NIC record type, export "port", "subnet" or ' + '"network", got %s' % nic) self._validated = result @@ -67,7 +71,7 @@ class NICs(object): self.validate() for nic_type, nic in self._validated: - if nic_type == 'network': + if nic_type != 'port': port = self._connection.network.create_port(**nic) self.created_ports.append(port.id) LOG.info('Created port %(port)s for node %(node)s with ' @@ -103,7 +107,7 @@ class NICs(object): try: port = self._connection.network.find_port( nic['port'], ignore_missing=False) - except Exception as exc: + except sdk_exc.SDKException as exc: raise exceptions.InvalidNIC( 'Cannot find port %(port)s: %(error)s' % {'port': nic['port'], 'error': exc}) @@ -125,7 +129,7 @@ class NICs(object): try: network = self._connection.network.find_network( nic['network'], ignore_missing=False) - except Exception as exc: + except sdk_exc.SDKException as exc: raise exceptions.InvalidNIC( 'Cannot find network %(net)s: %(error)s' % {'net': nic['network'], 'error': exc}) @@ -136,6 +140,35 @@ class NICs(object): return port_args + def _get_subnet(self, nic): + """Validate and get the NIC information for a subnet. + + :param nic: NIC information in the form ``{"subnet": ""}``. + :returns: keyword arguments to use when creating a port. + """ + unexpected = set(nic) - {'subnet'} + if unexpected: + raise exceptions.InvalidNIC( + 'Unexpected fields for a subnet: %s' % ', '.join(unexpected)) + + try: + subnet = self._connection.network.find_subnet( + nic['subnet'], ignore_missing=False) + except sdk_exc.SDKException as exc: + raise exceptions.InvalidNIC( + 'Cannot find subnet %(sub)s: %(error)s' % + {'sub': nic['subnet'], 'error': exc}) + + try: + network = self._connection.network.get_network(subnet.network_id) + except sdk_exc.SDKException as exc: + raise exceptions.InvalidNIC( + 'Cannot find network %(net)s for subnet %(sub)s: %(error)s' % + {'net': subnet.network_id, 'sub': nic['subnet'], 'error': exc}) + + return {'network_id': network.id, + 'fixed_ips': [{'subnet_id': subnet.id}]} + def detach_and_delete_ports(connection, node, created_ports, attached_ports): """Detach attached port and delete previously created ones. diff --git a/metalsmith/_provisioner.py b/metalsmith/_provisioner.py index ebc9f0f..1d5a1ac 100644 --- a/metalsmith/_provisioner.py +++ b/metalsmith/_provisioner.py @@ -217,10 +217,16 @@ class Provisioner(_utils.GetNodeMixin): `Image` name or UUID. :param nics: List of virtual NICs to attach to physical ports. 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. + + * ``{"port": ""}`` to use the provided pre-created + port. + * ``{"network": ""}`` to create a port on the + provided network. Optionally, a ``fixed_ip`` argument can be used + to specify an IP address. + * ``{"subnet": ""}`` to create a port with an IP + address from the provided subnet. The network is determined from + the 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 diff --git a/metalsmith/test/test_cmd.py b/metalsmith/test/test_cmd.py index c081f62..69d18bc 100644 --- a/metalsmith/test/test_cmd.py +++ b/metalsmith/test/test_cmd.py @@ -342,6 +342,11 @@ class TestDeploy(testtools.TestCase): {'nics': [{'network': 'private', 'fixed_ip': '10.0.0.2'}, {'network': 'public', 'fixed_ip': '8.0.8.0'}]}) + def test_args_subnet(self, mock_pr): + args = ['deploy', '--subnet', 'mysubnet', '--image', 'myimg', + '--resource-class', 'compute'] + self._check(mock_pr, args, {}, {'nics': [{'subnet': 'mysubnet'}]}) + def test_args_bad_ip(self, mock_pr): args = ['deploy', '--image', 'myimg', '--resource-class', 'compute', '--ip', 'private:10.0.0.2', '--ip', 'public'] diff --git a/metalsmith/test/test_provisioner.py b/metalsmith/test/test_provisioner.py index 93d1988..c948ec5 100644 --- a/metalsmith/test/test_provisioner.py +++ b/metalsmith/test/test_provisioner.py @@ -466,6 +466,28 @@ class TestProvisionNode(Base): self.api.baremetal.wait_for_nodes_provision_state.called) self.assertFalse(self.api.network.delete_port.called) + def test_with_subnet(self): + inst = self.pr.provision_node(self.node, 'image', + [{'subnet': 'subnet'}]) + + self.assertEqual(inst.uuid, self.node.id) + self.assertEqual(inst.node, self.node) + + self.api.network.create_port.assert_called_once_with( + network_id=self.api.network.get_network.return_value.id, + fixed_ips=[{'subnet_id': + self.api.network.find_subnet.return_value.id}]) + self.api.baremetal.attach_vif_to_node.assert_called_once_with( + self.node, self.api.network.create_port.return_value.id) + self.api.baremetal.update_node.assert_called_once_with( + self.node, extra=self.extra, instance_info=self.instance_info) + self.api.baremetal.validate_node.assert_called_once_with(self.node) + self.api.baremetal.set_node_provision_state.assert_called_once_with( + self.node, 'active', config_drive=mock.ANY) + self.assertFalse( + self.api.baremetal.wait_for_nodes_provision_state.called) + self.assertFalse(self.api.network.delete_port.called) + def test_whole_disk(self): self.image.kernel_id = None self.image.ramdisk_id = None @@ -1036,7 +1058,8 @@ abcd and-not-image-again self.assertFalse(self.api.baremetal.set_node_provision_state.called) def test_invalid_network(self): - self.api.network.find_network.side_effect = RuntimeError('Not found') + self.api.network.find_network.side_effect = os_exc.SDKException( + 'Not found') self.assertRaisesRegex(exceptions.InvalidNIC, 'Not found', self.pr.provision_node, self.node, 'image', [{'network': 'network'}]) @@ -1046,7 +1069,8 @@ abcd and-not-image-again self.assertFalse(self.api.baremetal.set_node_provision_state.called) def test_invalid_port(self): - self.api.network.find_port.side_effect = RuntimeError('Not found') + self.api.network.find_port.side_effect = os_exc.SDKException( + 'Not found') self.assertRaisesRegex(exceptions.InvalidNIC, 'Not found', self.pr.provision_node, self.node, 'image', [{'port': 'port1'}]) @@ -1055,6 +1079,29 @@ abcd and-not-image-again self.assertFalse(self.api.network.create_port.called) self.assertFalse(self.api.baremetal.set_node_provision_state.called) + def test_invalid_subnet(self): + self.api.network.find_subnet.side_effect = os_exc.SDKException( + 'Not found') + self.assertRaisesRegex(exceptions.InvalidNIC, 'Not found', + self.pr.provision_node, + self.node, 'image', [{'subnet': 'subnet'}]) + self.api.baremetal.update_node.assert_called_once_with( + self.node, extra={}, instance_info={}, instance_id=None) + self.assertFalse(self.api.network.create_port.called) + self.assertFalse(self.api.baremetal.set_node_provision_state.called) + + def test_invalid_network_of_subnet(self): + # NOTE(dtantsur): I doubt this can happen, maybe some race? + self.api.network.get_network.side_effect = os_exc.SDKException( + 'Not found') + self.assertRaisesRegex(exceptions.InvalidNIC, 'Not found', + self.pr.provision_node, + self.node, 'image', [{'subnet': 'subnet'}]) + self.api.baremetal.update_node.assert_called_once_with( + self.node, extra={}, instance_info={}, instance_id=None) + self.assertFalse(self.api.network.create_port.called) + self.assertFalse(self.api.baremetal.set_node_provision_state.called) + def test_no_local_gb(self): self.node.properties = {} self.assertRaises(exceptions.UnknownRootDiskSize, @@ -1113,7 +1160,8 @@ abcd and-not-image-again def test_invalid_nic_type_fields(self): for item in ({'port': '1234', 'foo': 'bar'}, {'port': '1234', 'network': '4321'}, - {'network': '4321', 'foo': 'bar'}): + {'network': '4321', 'foo': 'bar'}, + {'subnet': '4321', 'foo': 'bar'}): self.assertRaisesRegex(exceptions.InvalidNIC, 'Unexpected fields', self.pr.provision_node, diff --git a/releasenotes/notes/subnet-1c177e4b40cc607c.yaml b/releasenotes/notes/subnet-1c177e4b40cc607c.yaml new file mode 100644 index 0000000..66fac9b --- /dev/null +++ b/releasenotes/notes/subnet-1c177e4b40cc607c.yaml @@ -0,0 +1,7 @@ +--- +features: + - | + Allows specifying a subnet for the ``nics`` argument of the + ``provision_node`` call as ``{"subnet": ""}``. + - | + Adds a new CLI argument ``--subnet`` to create a port on the given subnet. diff --git a/roles/metalsmith_deployment/README.rst b/roles/metalsmith_deployment/README.rst index 2524538..cee7810 100644 --- a/roles/metalsmith_deployment/README.rst +++ b/roles/metalsmith_deployment/README.rst @@ -106,6 +106,14 @@ Each instances has the following attributes: nics: - port: b2254316-7867-4615-9fb7-911b3f38ca2a + ``subnet`` + creates a port on the given subnet, for example: + + .. code-block:: yaml + + nics: + - subnet: private-subnet1 + ``resource_class`` (defaults to ``metalsmith_resource_class``) requested node's resource class. ``root_size`` (defaults to ``metalsmith_root_size``)