diff --git a/metalsmith/_cmd.py b/metalsmith/_cmd.py index 6a273ad..854ff57 100644 --- a/metalsmith/_cmd.py +++ b/metalsmith/_cmd.py @@ -25,6 +25,17 @@ from metalsmith import _provisioner LOG = logging.getLogger(__name__) +class NICAction(argparse.Action): + def __call__(self, parser, namespace, values, option_string=None): + assert option_string in ('--port', '--network') + nics = getattr(namespace, self.dest, None) or [] + if option_string == '--network': + nics.append({'network': values}) + else: + nics.append({'port': values}) + setattr(namespace, self.dest, nics) + + def _do_deploy(api, args, wait=None): capabilities = dict(item.split('=', 1) for item in args.capability) if args.ssh_public_key: @@ -36,7 +47,7 @@ def _do_deploy(api, args, wait=None): node = api.reserve_node(args.resource_class, capabilities=capabilities) api.provision_node(node, image_ref=args.image, - network_refs=[args.network], + nics=args.nics, root_disk_size=args.root_disk_size, ssh_keys=ssh_keys, netboot=args.netboot, @@ -72,7 +83,9 @@ def _parse_args(args, config): deploy.add_argument('--image', help='image to use (name or UUID)', required=True) deploy.add_argument('--network', help='network to use (name or UUID)', - required=True), + dest='nics', action=NICAction) + deploy.add_argument('--port', help='port to attach (name or UUID)', + dest='nics', action=NICAction) deploy.add_argument('--netboot', action='store_true', help='boot from network instead of local disk') deploy.add_argument('--root-disk-size', type=int, diff --git a/metalsmith/_exceptions.py b/metalsmith/_exceptions.py index 78e6f7a..ec58a40 100644 --- a/metalsmith/_exceptions.py +++ b/metalsmith/_exceptions.py @@ -61,8 +61,8 @@ class InvalidImage(Error): """Requested image is invalid and cannot be used.""" -class InvalidNetwork(Error): - """Requested network is invalid and cannot be used.""" +class InvalidNIC(Error): + """Requested NIC is invalid and cannot be used.""" class UnknownRootDiskSize(Error): diff --git a/metalsmith/_os_api.py b/metalsmith/_os_api.py index 5d05c22..9220d07 100644 --- a/metalsmith/_os_api.py +++ b/metalsmith/_os_api.py @@ -89,7 +89,8 @@ class API(object): return node def get_port(self, port_id): - return self.connection.network.get_port(port_id) + return self.connection.network.find_port(port_id, + ignore_missing=False) def list_node_attached_ports(self, node): return self.ironic.node.vif_list(_node_id(node)) diff --git a/metalsmith/_provisioner.py b/metalsmith/_provisioner.py index 8d4bb5e..d67ebee 100644 --- a/metalsmith/_provisioner.py +++ b/metalsmith/_provisioner.py @@ -13,6 +13,7 @@ # See the License for the specific language governing permissions and # limitations under the License. +import collections import logging import random @@ -27,6 +28,7 @@ from metalsmith import _utils LOG = logging.getLogger(__name__) _CREATED_PORTS = 'metalsmith_created_ports' +_ATTACHED_PORTS = 'metalsmith_attached_ports' class Provisioner(object): @@ -63,14 +65,16 @@ class Provisioner(object): return _scheduler.schedule_node(nodes, filters, reserver, dry_run=self._dry_run) - def provision_node(self, node, image_ref, network_refs, - root_disk_size=None, ssh_keys=None, netboot=False, - wait=None): + def provision_node(self, node, image_ref, nics=None, root_disk_size=None, + ssh_keys=None, netboot=False, wait=None): """Provision the node with the given image. :param node: Node object, UUID or name. :param image_ref: Image name or UUID to provision. - :param network_refs: List of network names or UUIDs to use. + :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": ""}``). :param root_disk_size: The size of the root partition. By default the value of the local_gb property is used. :param ssh_keys: list of public parts of the SSH keys to upload @@ -81,6 +85,7 @@ class Provisioner(object): :return: Reservation """ created_ports = [] + attached_ports = [] try: node = self._api.get_node(node) @@ -96,21 +101,23 @@ class Provisioner(object): LOG.debug('Image: %s', image) - networks = self._get_networks(network_refs) + nics = self._get_nics(nics or []) if self._dry_run: LOG.warning('Dry run, not provisioning node %s', _utils.log_node(node)) return node - self._create_ports(node, networks, created_ports) + self._create_and_attach_ports(node, nics, + created_ports, attached_ports) target_caps = {'boot_option': 'netboot' if netboot else 'local'} updates = {'/instance_info/image_source': image.id, '/instance_info/root_gb': root_disk_size, '/instance_info/capabilities': target_caps, - '/extra/%s' % _CREATED_PORTS: created_ports} + '/extra/%s' % _CREATED_PORTS: created_ports, + '/extra/%s' % _ATTACHED_PORTS: attached_ports} for prop in ('kernel', 'ramdisk'): value = getattr(image, '%s_id' % prop, None) @@ -134,7 +141,7 @@ class Provisioner(object): with excutils.save_and_reraise_exception(): LOG.error('Deploy attempt failed on node %s, cleaning up', _utils.log_node(node)) - self._clean_up(node, created_ports) + self._clean_up(node, created_ports, attached_ports) if wait is not None: LOG.info('Deploy succeeded on node %s', _utils.log_node(node)) @@ -157,45 +164,73 @@ class Provisioner(object): else: LOG.warning('No IPs for node %s', _utils.log_node(node)) - def _clean_up(self, node, created_ports): + def _clean_up(self, node, created_ports, attached_ports): try: - self._delete_ports(node, created_ports) + self._delete_ports(node, created_ports, attached_ports) self._api.release_node(node) except Exception: LOG.exception('Clean up failed') - def _get_networks(self, network_refs): - """Validate and get the networks.""" - networks = [] - for network_ref in network_refs: - try: - network = self._api.get_network(network_ref) - except Exception as exc: - raise _exceptions.InvalidNetwork( - 'Cannot find network %(net)s: %(error)s' % - {'net': network_ref, 'error': exc}) + def _get_nics(self, nics): + """Validate and get the NICs.""" + result = [] + if not isinstance(nics, collections.Sequence): + raise TypeError("NICs must be a list of dicts") - LOG.debug('Network: %s', network) - networks.append(network) - return networks + 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) - def _create_ports(self, node, networks, created_ports): + nic_type, nic_id = next(iter(nic.items())) + if nic_type == 'network': + try: + network = self._api.get_network(nic_id) + 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)) + elif nic_type == 'port': + try: + port = self._api.get_port(nic_id) + 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)) + else: + raise ValueError("Unexpected NIC type %s, supported values: " + "'port', 'network'" % nic_type) + + return result + + def _create_and_attach_ports(self, node, nics, created_ports, + attached_ports): """Create and attach ports on given networks.""" - for network in networks: - port = self._api.create_port(network_id=network.id) - created_ports.append(port.id) - LOG.debug('Created Neutron port %s', port) + for nic_type, nic in nics: + if nic_type == 'network': + port = self._api.create_port(network_id=nic.id) + created_ports.append(port.id) + LOG.debug('Created Neutron port %s', port) + else: + port = nic self._api.attach_port_to_node(node.uuid, port.id) LOG.info('Attached port %(port)s to node %(node)s', {'port': port.id, 'node': _utils.log_node(node)}) + attached_ports.append(port.id) - def _delete_ports(self, node, created_ports=None): + 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 created_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: @@ -206,12 +241,21 @@ class Provisioner(object): {'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._api.delete_port(port_id) except Exception: LOG.warning('Failed to delete neutron port %s', port_id) + update = {'/extra/%s' % item: _os_api.REMOVE + for item in (_CREATED_PORTS, _ATTACHED_PORTS)} + try: + self._api.update_node(node, update) + except Exception as exc: + LOG.warning('Failed to clear node %(node)s extra: %(exc)s', + {'node': _utils.log_node(node), 'exc': exc}) + def unprovision_node(self, node, wait=None): """Unprovision a previously provisioned node. diff --git a/metalsmith/test/test_cmd.py b/metalsmith/test/test_cmd.py index a3d5ad6..e202ab8 100644 --- a/metalsmith/test/test_cmd.py +++ b/metalsmith/test/test_cmd.py @@ -38,7 +38,7 @@ class TestMain(testtools.TestCase): mock_pr.return_value.provision_node.assert_called_once_with( mock_pr.return_value.reserve_node.return_value, image_ref='myimg', - network_refs=['mynet'], + nics=[{'network': 'mynet'}], root_disk_size=None, ssh_keys=[], netboot=False, @@ -58,7 +58,7 @@ class TestMain(testtools.TestCase): mock_pr.return_value.provision_node.assert_called_once_with( mock_pr.return_value.reserve_node.return_value, image_ref='myimg', - network_refs=['mynet'], + nics=[{'network': 'mynet'}], root_disk_size=None, ssh_keys=[], netboot=False, @@ -78,7 +78,7 @@ class TestMain(testtools.TestCase): mock_pr.return_value.provision_node.assert_called_once_with( mock_pr.return_value.reserve_node.return_value, image_ref='myimg', - network_refs=['mynet'], + nics=[{'network': 'mynet'}], root_disk_size=None, ssh_keys=[], netboot=False, @@ -98,7 +98,7 @@ class TestMain(testtools.TestCase): mock_pr.return_value.provision_node.assert_called_once_with( mock_pr.return_value.reserve_node.return_value, image_ref='myimg', - network_refs=['mynet'], + nics=[{'network': 'mynet'}], root_disk_size=None, ssh_keys=[], netboot=False, @@ -135,7 +135,7 @@ class TestMain(testtools.TestCase): mock_pr.return_value.provision_node.assert_called_once_with( mock_pr.return_value.reserve_node.return_value, image_ref='myimg', - network_refs=['mynet'], + nics=[{'network': 'mynet'}], root_disk_size=None, ssh_keys=[], netboot=False, @@ -159,8 +159,68 @@ class TestMain(testtools.TestCase): mock_pr.return_value.provision_node.assert_called_once_with( mock_pr.return_value.reserve_node.return_value, image_ref='myimg', - network_refs=['mynet'], + nics=[{'network': 'mynet'}], root_disk_size=None, ssh_keys=['foo'], netboot=False, wait=1800) + + def test_args_port(self, mock_os_conf, mock_pr): + args = ['deploy', '--port', 'myport', '--image', 'myimg', 'compute'] + _cmd.main(args) + mock_pr.assert_called_once_with( + cloud_region=mock_os_conf.return_value.get_one.return_value, + dry_run=False) + mock_pr.return_value.reserve_node.assert_called_once_with( + resource_class='compute', + capabilities={} + ) + mock_pr.return_value.provision_node.assert_called_once_with( + mock_pr.return_value.reserve_node.return_value, + image_ref='myimg', + nics=[{'port': 'myport'}], + root_disk_size=None, + ssh_keys=[], + netboot=False, + wait=1800) + + def test_args_no_nics(self, mock_os_conf, mock_pr): + args = ['deploy', '--image', 'myimg', 'compute'] + _cmd.main(args) + mock_pr.assert_called_once_with( + cloud_region=mock_os_conf.return_value.get_one.return_value, + dry_run=False) + mock_pr.return_value.reserve_node.assert_called_once_with( + resource_class='compute', + capabilities={} + ) + mock_pr.return_value.provision_node.assert_called_once_with( + mock_pr.return_value.reserve_node.return_value, + image_ref='myimg', + nics=None, + root_disk_size=None, + ssh_keys=[], + netboot=False, + wait=1800) + + def test_args_networks_and_ports(self, mock_os_conf, mock_pr): + args = ['deploy', '--network', 'net1', '--port', 'port1', + '--port', 'port2', '--network', 'net2', + '--image', 'myimg', 'compute'] + _cmd.main(args) + mock_pr.assert_called_once_with( + cloud_region=mock_os_conf.return_value.get_one.return_value, + dry_run=False) + mock_pr.return_value.reserve_node.assert_called_once_with( + resource_class='compute', + capabilities={} + ) + mock_pr.return_value.provision_node.assert_called_once_with( + mock_pr.return_value.reserve_node.return_value, + image_ref='myimg', + nics=[{'network': 'net1'}, {'port': 'port1'}, + {'port': 'port2'}, {'network': 'net2'}], + root_disk_size=None, + ssh_keys=[], + netboot=False, + wait=1800) diff --git a/metalsmith/test/test_provisioner.py b/metalsmith/test/test_provisioner.py index 8ff1895..df1a8ce 100644 --- a/metalsmith/test/test_provisioner.py +++ b/metalsmith/test/test_provisioner.py @@ -75,25 +75,63 @@ class TestReserveNode(Base): self.assertIs(node, expected) +CLEAN_UP = { + '/extra/metalsmith_created_ports': _os_api.REMOVE, + '/extra/metalsmith_attached_ports': _os_api.REMOVE +} + + class TestProvisionNode(Base): + def setUp(self): + super(TestProvisionNode, self).setUp() + image = self.api.get_image_info.return_value + self.updates = { + '/instance_info/ramdisk': image.ramdisk_id, + '/instance_info/kernel': image.kernel_id, + '/instance_info/image_source': image.id, + '/instance_info/root_gb': 99, # 100 - 1 + '/instance_info/capabilities': {'boot_option': 'local'}, + '/extra/metalsmith_created_ports': [ + self.api.create_port.return_value.id + ], + '/extra/metalsmith_attached_ports': [ + self.api.create_port.return_value.id + ] + } + def test_ok(self): - self.pr.provision_node(self.node, 'image', ['network']) + self.pr.provision_node(self.node, 'image', [{'network': 'network'}]) self.api.create_port.assert_called_once_with( network_id=self.api.get_network.return_value.id) self.api.attach_port_to_node.assert_called_once_with( self.node.uuid, self.api.create_port.return_value.id) - image = self.api.get_image_info.return_value - updates = {'/instance_info/ramdisk': image.ramdisk_id, - '/instance_info/kernel': image.kernel_id, - '/instance_info/image_source': image.id, - '/instance_info/root_gb': 99, # 100 - 1 - '/instance_info/capabilities': {'boot_option': 'local'}, - '/extra/metalsmith_created_ports': [ - self.api.create_port.return_value.id - ]} - self.api.update_node.assert_called_once_with(self.node, updates) + 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.api.wait_for_node_state.called) + self.assertFalse(self.api.release_node.called) + self.assertFalse(self.api.delete_port.called) + + def test_with_ports(self): + self.updates['/extra/metalsmith_created_ports'] = [] + self.updates['/extra/metalsmith_attached_ports'] = [ + self.api.get_port.return_value.id + ] * 2 + + self.pr.provision_node(self.node, 'image', + [{'port': 'port1'}, {'port': 'port2'}]) + + self.assertFalse(self.api.create_port.called) + self.api.attach_port_to_node.assert_called_with( + self.node.uuid, self.api.get_port.return_value.id) + self.assertEqual(2, self.api.attach_port_to_node.call_count) + self.assertEqual([mock.call('port1'), mock.call('port2')], + self.api.get_port.call_args_list) + 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', @@ -106,20 +144,16 @@ class TestProvisionNode(Base): image = self.api.get_image_info.return_value image.kernel_id = None image.ramdisk_id = None + del self.updates['/instance_info/kernel'] + del self.updates['/instance_info/ramdisk'] - self.pr.provision_node(self.node, 'image', ['network']) + self.pr.provision_node(self.node, 'image', [{'network': 'network'}]) self.api.create_port.assert_called_once_with( network_id=self.api.get_network.return_value.id) self.api.attach_port_to_node.assert_called_once_with( self.node.uuid, self.api.create_port.return_value.id) - updates = {'/instance_info/image_source': image.id, - '/instance_info/root_gb': 99, # 100 - 1 - '/instance_info/capabilities': {'boot_option': 'local'}, - '/extra/metalsmith_created_ports': [ - self.api.create_port.return_value.id - ]} - self.api.update_node.assert_called_once_with(self.node, updates) + 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', @@ -129,23 +163,16 @@ class TestProvisionNode(Base): self.assertFalse(self.api.delete_port.called) def test_with_root_disk_size(self): - self.pr.provision_node(self.node, 'image', ['network'], + self.updates['/instance_info/root_gb'] = 50 + + self.pr.provision_node(self.node, 'image', [{'network': 'network'}], root_disk_size=50) self.api.create_port.assert_called_once_with( network_id=self.api.get_network.return_value.id) self.api.attach_port_to_node.assert_called_once_with( self.node.uuid, self.api.create_port.return_value.id) - image = self.api.get_image_info.return_value - updates = {'/instance_info/ramdisk': image.ramdisk_id, - '/instance_info/kernel': image.kernel_id, - '/instance_info/image_source': image.id, - '/instance_info/root_gb': 50, - '/instance_info/capabilities': {'boot_option': 'local'}, - '/extra/metalsmith_created_ports': [ - self.api.create_port.return_value.id - ]} - self.api.update_node.assert_called_once_with(self.node, updates) + 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', @@ -159,22 +186,14 @@ class TestProvisionNode(Base): spec=['fixed_ips'], fixed_ips=[{'ip_address': '192.168.1.5'}, {}] ) - self.pr.provision_node(self.node, 'image', ['network'], wait=3600) + self.pr.provision_node(self.node, 'image', [{'network': 'network'}], + wait=3600) self.api.create_port.assert_called_once_with( network_id=self.api.get_network.return_value.id) self.api.attach_port_to_node.assert_called_once_with( self.node.uuid, self.api.create_port.return_value.id) - image = self.api.get_image_info.return_value - updates = {'/instance_info/ramdisk': image.ramdisk_id, - '/instance_info/kernel': image.kernel_id, - '/instance_info/image_source': image.id, - '/instance_info/root_gb': 99, # 100 - 1 - '/instance_info/capabilities': {'boot_option': 'local'}, - '/extra/metalsmith_created_ports': [ - self.api.create_port.return_value.id - ]} - self.api.update_node.assert_called_once_with(self.node, updates) + 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', @@ -192,7 +211,8 @@ class TestProvisionNode(Base): self.api.get_port.return_value = mock.Mock( spec=['fixed_ips'], fixed_ips=[] ) - self.pr.provision_node(self.node, 'image', ['network'], wait=3600) + self.pr.provision_node(self.node, 'image', [{'network': 'network'}], + wait=3600) self.api.node_action.assert_called_once_with(self.node, 'active', configdrive=mock.ANY) @@ -203,7 +223,7 @@ class TestProvisionNode(Base): def test_dry_run(self): self.pr._dry_run = True - self.pr.provision_node(self.node, 'image', ['network']) + self.pr.provision_node(self.node, 'image', [{'network': 'network'}]) self.assertFalse(self.api.create_port.called) self.assertFalse(self.api.attach_port_to_node.called) @@ -217,21 +237,27 @@ class TestProvisionNode(Base): self.api.node_action.side_effect = RuntimeError('boom') self.assertRaisesRegex(RuntimeError, 'boom', self.pr.provision_node, self.node, - 'image', ['network'], wait=3600) + 'image', [{'network': 'n1'}, {'port': 'p1'}], + wait=3600) + self.api.update_node.assert_any_call(self.node, CLEAN_UP) self.assertFalse(self.api.wait_for_node_state.called) self.api.release_node.assert_called_once_with(self.node) self.api.delete_port.assert_called_once_with( self.api.create_port.return_value.id) - self.api.detach_port_from_node.assert_called_once_with( - self.node, self.api.create_port.return_value.id) + calls = [ + mock.call(self.node, self.api.create_port.return_value.id), + mock.call(self.node, self.api.get_port.return_value.id) + ] + self.api.detach_port_from_node.assert_has_calls(calls, any_order=True) def test_port_creation_failure(self): self.api.create_port.side_effect = RuntimeError('boom') self.assertRaisesRegex(RuntimeError, 'boom', self.pr.provision_node, self.node, - 'image', ['network'], wait=3600) + 'image', [{'network': 'network'}], wait=3600) + self.api.update_node.assert_called_once_with(self.node, CLEAN_UP) self.assertFalse(self.api.node_action.called) self.api.release_node.assert_called_once_with(self.node) self.assertFalse(self.api.delete_port.called) @@ -241,8 +267,9 @@ class TestProvisionNode(Base): self.api.attach_port_to_node.side_effect = RuntimeError('boom') self.assertRaisesRegex(RuntimeError, 'boom', self.pr.provision_node, self.node, - 'image', ['network'], wait=3600) + 'image', [{'network': 'network'}], wait=3600) + self.api.update_node.assert_called_once_with(self.node, CLEAN_UP) self.assertFalse(self.api.node_action.called) self.api.release_node.assert_called_once_with(self.node) self.api.delete_port.assert_called_once_with( @@ -259,7 +286,8 @@ class TestProvisionNode(Base): self.api.node_action.side_effect = RuntimeError('boom') self.assertRaisesRegex(RuntimeError, 'boom', self.pr.provision_node, self.node, - 'image', ['network'], wait=3600) + 'image', [{'network': 'network'}], + wait=3600) self.assertFalse(self.api.wait_for_node_state.called) self.api.release_node.assert_called_once_with(self.node) @@ -270,20 +298,46 @@ class TestProvisionNode(Base): self.assertEqual(mock_log_exc.called, failed_call == 'release_node') + def test_failure_during_extra_update_on_deploy_failure(self): + self.api.update_node.side_effect = [self.node, AssertionError()] + self.api.node_action.side_effect = RuntimeError('boom') + self.assertRaisesRegex(RuntimeError, 'boom', + self.pr.provision_node, self.node, + 'image', [{'network': 'network'}], + wait=3600) + + self.assertFalse(self.api.wait_for_node_state.called) + self.api.release_node.assert_called_once_with(self.node) + self.api.delete_port.assert_called_once_with( + self.api.create_port.return_value.id) + self.api.detach_port_from_node.assert_called_once_with( + self.node, self.api.create_port.return_value.id) + def test_missing_image(self): self.api.get_image_info.side_effect = RuntimeError('Not found') self.assertRaisesRegex(_exceptions.InvalidImage, 'Not found', self.pr.provision_node, - self.node, 'image', ['network']) - self.assertFalse(self.api.update_node.called) + self.node, 'image', [{'network': 'network'}]) + self.api.update_node.assert_called_once_with(self.node, CLEAN_UP) self.assertFalse(self.api.node_action.called) self.api.release_node.assert_called_once_with(self.node) def test_invalid_network(self): self.api.get_network.side_effect = RuntimeError('Not found') - self.assertRaisesRegex(_exceptions.InvalidNetwork, 'Not found', + self.assertRaisesRegex(_exceptions.InvalidNIC, 'Not found', self.pr.provision_node, - self.node, 'image', ['network']) + self.node, 'image', [{'network': 'network'}]) + self.api.update_node.assert_called_once_with(self.node, CLEAN_UP) + self.assertFalse(self.api.create_port.called) + self.assertFalse(self.api.node_action.called) + self.api.release_node.assert_called_once_with(self.node) + + def test_invalid_port(self): + self.api.get_port.side_effect = RuntimeError('Not found') + self.assertRaisesRegex(_exceptions.InvalidNIC, 'Not found', + self.pr.provision_node, + self.node, 'image', [{'port': 'port1'}]) + self.api.update_node.assert_called_once_with(self.node, CLEAN_UP) self.assertFalse(self.api.create_port.called) self.assertFalse(self.api.node_action.called) self.api.release_node.assert_called_once_with(self.node) @@ -292,7 +346,7 @@ class TestProvisionNode(Base): self.node.properties = {} self.assertRaises(_exceptions.UnknownRootDiskSize, self.pr.provision_node, - self.node, 'image', ['network']) + self.node, 'image', [{'network': 'network'}]) self.assertFalse(self.api.create_port.called) self.assertFalse(self.api.node_action.called) self.api.release_node.assert_called_once_with(self.node) @@ -302,7 +356,7 @@ class TestProvisionNode(Base): self.node.properties = {'local_gb': value} self.assertRaises(_exceptions.UnknownRootDiskSize, self.pr.provision_node, - self.node, 'image', ['network']) + self.node, 'image', [{'network': 'network'}]) self.assertFalse(self.api.create_port.called) self.assertFalse(self.api.node_action.called) self.api.release_node.assert_called_with(self.node) @@ -310,16 +364,44 @@ class TestProvisionNode(Base): def test_invalid_root_disk_size(self): self.assertRaises(TypeError, self.pr.provision_node, - self.node, 'image', ['network'], + self.node, 'image', [{'network': 'network'}], root_disk_size={}) self.assertRaises(ValueError, self.pr.provision_node, - self.node, 'image', ['network'], + self.node, 'image', [{'network': 'network'}], root_disk_size=0) self.assertFalse(self.api.create_port.called) self.assertFalse(self.api.node_action.called) self.api.release_node.assert_called_with(self.node) + def test_invalid_nics(self): + self.assertRaisesRegex(TypeError, 'must be a list', + self.pr.provision_node, + self.node, 'image', 42) + self.assertFalse(self.api.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}]): + self.assertRaisesRegex(TypeError, 'must be a dict', + self.pr.provision_node, + self.node, 'image', item) + self.assertFalse(self.api.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, 'Unexpected NIC type foo', + self.pr.provision_node, + self.node, 'image', [{'foo': 'bar'}]) + self.assertFalse(self.api.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) + class TestUnprovisionNode(Base): @@ -333,6 +415,20 @@ class TestUnprovisionNode(Base): self.api.node_action.assert_called_once_with(self.node, 'deleted') self.api.release_node.assert_called_once_with(self.node) self.assertFalse(self.api.wait_for_node_state.called) + self.api.update_node.assert_called_once_with(self.node, CLEAN_UP) + + def test_with_attached(self): + self.node.extra['metalsmith_created_ports'] = ['port1'] + self.node.extra['metalsmith_attached_ports'] = ['port1', 'port2'] + self.pr.unprovision_node(self.node) + + self.api.delete_port.assert_called_once_with('port1') + calls = [mock.call(self.node, 'port1'), mock.call(self.node, 'port2')] + self.api.detach_port_from_node.assert_has_calls(calls, any_order=True) + self.api.node_action.assert_called_once_with(self.node, 'deleted') + self.api.release_node.assert_called_once_with(self.node) + self.assertFalse(self.api.wait_for_node_state.called) + self.api.update_node.assert_called_once_with(self.node, CLEAN_UP) def test_with_wait(self): self.node.extra['metalsmith_created_ports'] = ['port1'] @@ -357,3 +453,4 @@ class TestUnprovisionNode(Base): self.assertFalse(self.api.delete_port.called) self.assertFalse(self.api.detach_port_from_node.called) self.assertFalse(self.api.wait_for_node_state.called) + self.assertFalse(self.api.update_node.called) diff --git a/playbooks/integration/exercise.yaml b/playbooks/integration/exercise.yaml index 4e95c7a..1724256 100644 --- a/playbooks/integration/exercise.yaml +++ b/playbooks/integration/exercise.yaml @@ -1,7 +1,21 @@ +- name: Create a port + command: openstack port create --network private test-port + when: precreate_port + +- name: Set port argument + set_fact: + nic: --port test-port + when: precreate_port + +- name: Set network argument + set_fact: + nic: --network private + when: not precreate_port + - name: Deploy a node command: > metalsmith --debug deploy - --network private + {{ nic }} --image {{ image }} --ssh-public-key {{ ssh_key_file }} --root-disk-size 9 @@ -28,10 +42,37 @@ command: metalsmith --debug undeploy {{ active_node }} - name: Get the current status of the deployed node - command: openstack baremetal node show {{ active_node }} -f value -c provision_state + command: openstack baremetal node show {{ active_node }} -f json register: undeployed_node_result +- name: Parse node state + set_fact: + undeployed_node: "{{ undeployed_node_result.stdout | from_json }}" + - name: Check that the node was undeployed fail: - msg: The node is in unexpected status {{ undeployed_node_result.stdout }} - when: undeployed_node_result.stdout != "available" + msg: The node is in unexpected status {{ undeployed_node }} + when: undeployed_node.provision_state != "available" + +- name: Check that the node extra was cleared + fail: + msg: The node still has extra {{ undeployed_node }} + when: undeployed_node.extra != {} + +- name: Get attached VIFs for the node + command: openstack baremetal node vif list {{ active_node }} -f value -c ID + register: vif_list_output + +- name: Check that no VIFs are still attached + fail: + msg: Some VIFs are still attached + when: vif_list_output.stdout != "" + +- name: Show remaining ports + command: openstack port list + +- name: Delete created port + command: openstack port delete test-port + when: precreate_port + # FIXME(dtantsur): fails because of ironic mis-behavior + ignore_errors: true diff --git a/playbooks/integration/run.yaml b/playbooks/integration/run.yaml index 6c27250..7129b20 100644 --- a/playbooks/integration/run.yaml +++ b/playbooks/integration/run.yaml @@ -51,7 +51,9 @@ - include: exercise.yaml vars: image: "{{ cirros_uec_image }}" + precreate_port: false - include: exercise.yaml vars: image: "{{ cirros_disk_image }}" + precreate_port: true