diff --git a/metalsmith/_exceptions.py b/metalsmith/_exceptions.py index 6029d41..78e6f7a 100644 --- a/metalsmith/_exceptions.py +++ b/metalsmith/_exceptions.py @@ -63,3 +63,7 @@ class InvalidImage(Error): class InvalidNetwork(Error): """Requested network is invalid and cannot be used.""" + + +class UnknownRootDiskSize(Error): + """Cannot determine the root disk size.""" diff --git a/metalsmith/_scheduler.py b/metalsmith/_scheduler.py index 5eb8db6..c90a819 100644 --- a/metalsmith/_scheduler.py +++ b/metalsmith/_scheduler.py @@ -180,22 +180,6 @@ class ValidationFilter(Filter): self._failed_validation.append(message) return False - try: - assert int(node.properties['local_gb']) > 0 - except KeyError: - message = 'No local_gb for node %s' % _utils.log_node(node) - LOG.warning(message) - self._failed_validation.append(message) - return False - except (TypeError, AssertionError): - message = ('The local_gb for node %(node)s is invalid: ' - 'expected positive integer, got %(value)s' % - {'node': _utils.log_node(node), - 'value': node.properties['local_gb']}) - LOG.warning(message) - self._failed_validation.append(message) - return False - return True def fail(self): diff --git a/metalsmith/_utils.py b/metalsmith/_utils.py index ccaf672..f819698 100644 --- a/metalsmith/_utils.py +++ b/metalsmith/_utils.py @@ -19,6 +19,8 @@ import os import shutil import tempfile +from metalsmith import _exceptions + def log_node(node): if node.name: @@ -69,6 +71,19 @@ def get_root_disk(root_disk_size, node): raise ValueError("The root_disk_size argument must be " "a positive integer, got %d" % root_disk_size) else: + try: + assert int(node.properties['local_gb']) > 0 + except KeyError: + raise _exceptions.UnknownRootDiskSize( + 'No local_gb for node %s and no root disk size requested' % + log_node(node)) + except (TypeError, ValueError, AssertionError): + raise _exceptions.UnknownRootDiskSize( + 'The local_gb for node %(node)s is invalid: ' + 'expected positive integer, got %(value)s' % + {'node': log_node(node), + 'value': node.properties['local_gb']}) + # allow for partitioning and config drive root_disk_size = int(node.properties['local_gb']) - 1 diff --git a/metalsmith/test/test_provisioner.py b/metalsmith/test/test_provisioner.py index 40bd7e2..24c185a 100644 --- a/metalsmith/test/test_provisioner.py +++ b/metalsmith/test/test_provisioner.py @@ -102,6 +102,32 @@ class TestProvisionNode(Base): self.assertFalse(self.api.release_node.called) self.assertFalse(self.api.delete_port.called) + def test_with_root_disk_size(self): + self.pr.provision_node(self.node, 'image', ['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.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_wait(self): self.api.get_port.return_value = mock.Mock( spec=['fixed_ips'], @@ -239,6 +265,35 @@ class TestProvisionNode(Base): self.assertFalse(self.api.create_port.called) self.assertFalse(self.api.node_action.called) + def test_no_local_gb(self): + self.node.properties = {} + self.assertRaises(_exceptions.UnknownRootDiskSize, + self.pr.provision_node, + self.node, 'image', ['network']) + self.assertFalse(self.api.create_port.called) + self.assertFalse(self.api.node_action.called) + + def test_invalid_local_gb(self): + for value in (None, 'meow', -42, []): + self.node.properties = {'local_gb': value} + self.assertRaises(_exceptions.UnknownRootDiskSize, + self.pr.provision_node, + self.node, 'image', ['network']) + self.assertFalse(self.api.create_port.called) + self.assertFalse(self.api.node_action.called) + + def test_invalid_root_disk_size(self): + self.assertRaises(TypeError, + self.pr.provision_node, + self.node, 'image', ['network'], + root_disk_size={}) + self.assertRaises(ValueError, + self.pr.provision_node, + self.node, 'image', ['network'], + root_disk_size=0) + self.assertFalse(self.api.create_port.called) + self.assertFalse(self.api.node_action.called) + class TestUnprovisionNode(Base): diff --git a/metalsmith/test/test_scheduler.py b/metalsmith/test/test_scheduler.py index 84e4f2b..f185333 100644 --- a/metalsmith/test/test_scheduler.py +++ b/metalsmith/test/test_scheduler.py @@ -173,33 +173,11 @@ class TestValidationFilter(testtools.TestCase): {'profile': 'compute'}) def test_pass(self): - node = mock.Mock(properties={'local_gb': 100}, - spec=['properties', 'uuid', 'name']) + node = mock.Mock(spec=['uuid', 'name']) self.assertTrue(self.fltr(node)) - def test_fail_without_local_gb(self): - node = mock.Mock(properties={}, - spec=['properties', 'uuid', 'name']) - self.assertFalse(self.fltr(node)) - - self.assertRaisesRegex(_exceptions.ValidationFailed, - 'All available nodes have failed validation: ' - 'No local_gb for node', - self.fltr.fail) - - def test_fail_malformed_local_gb(self): - node = mock.Mock(properties={'local_gb': []}, - spec=['properties', 'uuid', 'name']) - self.assertFalse(self.fltr(node)) - - self.assertRaisesRegex(_exceptions.ValidationFailed, - 'All available nodes have failed validation: ' - 'The local_gb for node .* is invalid', - self.fltr.fail) - def test_fail_validation(self): - node = mock.Mock(properties={'local_gb': 100}, - spec=['properties', 'uuid', 'name']) + node = mock.Mock(spec=['uuid', 'name']) self.api.validate_node.side_effect = RuntimeError('boom') self.assertFalse(self.fltr(node))