From 3f7f055e2ea866ec0c74d36928c4156290f7d59f Mon Sep 17 00:00:00 2001 From: Lucas Alvares Gomes Date: Wed, 3 Sep 2014 17:50:55 +0100 Subject: Driver merge review comments from 111425 This review contain fixes that addresses the review comments raised in https://review.openstack.org/#/c/111425 revision 19 and 22. Once these changes are merged here in Ironic, they will be proposed as updates to the patch series over in Nova. Change-Id: Idf4ea36670e5832600c5ff5f814e06aa096e156e --- ironic/nova/tests/virt/ironic/test_driver.py | 12 ++++-------- ironic/nova/virt/ironic/driver.py | 25 +++++++++---------------- 2 files changed, 13 insertions(+), 24 deletions(-) (limited to 'ironic/nova') diff --git a/ironic/nova/tests/virt/ironic/test_driver.py b/ironic/nova/tests/virt/ironic/test_driver.py index 1910bf376..30dba2bc7 100644 --- a/ironic/nova/tests/virt/ironic/test_driver.py +++ b/ironic/nova/tests/virt/ironic/test_driver.py @@ -1,5 +1,3 @@ -# coding=utf-8 -# # Copyright 2014 Red Hat, Inc. # Copyright 2013 Hewlett-Packard Development Company, L.P. # @@ -72,9 +70,7 @@ def _get_properties(): 'cpu_arch': 'x86_64'} def _get_stats(): - return {'cpu_arch': 'x86_64', - 'ironic_driver': - 'ironic.nova.virt.ironic.driver.IronicDriver'} + return {'cpu_arch': 'x86_64'} FAKE_CLIENT_WRAPPER = FakeClientWrapper() @@ -98,10 +94,10 @@ class IronicDriverTestCase(test.NoDBTestCase): self.assertIsInstance(self.driver, ironic_driver.IronicDriver) def test__get_hypervisor_type(self): - self.assertEqual(self.driver._get_hypervisor_type(), 'ironic') + self.assertEqual('ironic', self.driver._get_hypervisor_type()) def test__get_hypervisor_version(self): - self.assertEqual(self.driver._get_hypervisor_version(), 1) + self.assertEqual(1, self.driver._get_hypervisor_version()) @mock.patch.object(FAKE_CLIENT.node, 'get_by_instance_uuid') def test__validate_instance_and_node(self, mock_gbiui): @@ -418,7 +414,7 @@ class IronicDriverTestCase(test.NoDBTestCase): mock_gbiu.return_value = node - # ironic_states.POWER_ON should me be mapped to + # ironic_states.POWER_ON should be mapped to # nova_states.RUNNING memory_kib = properties['memory_mb'] * 1024 expected = {'state': nova_states.RUNNING, diff --git a/ironic/nova/virt/ironic/driver.py b/ironic/nova/virt/ironic/driver.py index 92697083b..3a99be8ae 100644 --- a/ironic/nova/virt/ironic/driver.py +++ b/ironic/nova/virt/ironic/driver.py @@ -100,15 +100,10 @@ def map_power_state(state): def _validate_instance_and_node(icli, instance): - """Get and validate a node's uuid out of a manager instance dict. - - The compute manager is meant to know the node uuid, so missing uuid is - a significant issue - it may mean we've been passed someone else's data. - - Check with the Ironic service that this node is still associated with - this instance. This catches situations where Nova's instance dict - contains stale data (eg, a delete on an instance that's already gone). + """Get the node associated with the instance. + Check with the Ironic service that this instance is associated with a + node, and return the node. """ try: return icli.call("node.get_by_instance_uuid", instance['uuid']) @@ -139,7 +134,8 @@ def _log_ironic_polling(what, node, instance): class IronicDriver(virt_driver.ComputeDriver): """Hypervisor driver for Ironic - bare metal provisioning.""" - capabilities = {"has_imagecache": False} + capabilities = {"has_imagecache": False, + "supports_recreate": False} def __init__(self, virtapi, read_only=False): super(IronicDriver, self).__init__(virtapi) @@ -155,10 +151,6 @@ class IronicDriver(virt_driver.ComputeDriver): self.firewall_driver = firewall.load_driver( default='nova.virt.firewall.NoopFirewallDriver') - extra_specs = {} - extra_specs["ironic_driver"] = \ - "ironic.nova.virt.ironic.driver.IronicDriver" - self.extra_specs = extra_specs self.node_cache = {} self.node_cache_time = 0 @@ -169,10 +161,11 @@ class IronicDriver(virt_driver.ComputeDriver): logger.setLevel(level) def _node_resources_unavailable(self, node_obj): - """Determine whether the node's resources are in an acceptable state. + """Determine whether the node's resources are in an unacceptable state. Determines whether the node's resources should be presented to Nova for use based on the current power and maintenance state. + Returns True if unacceptable. """ bad_states = [ironic_states.ERROR, ironic_states.NOSTATE] return (node_obj.maintenance or @@ -185,7 +178,7 @@ class IronicDriver(virt_driver.ComputeDriver): local_gb = int(node.properties.get('local_gb', 0)) cpu_arch = str(node.properties.get('cpu_arch', 'NotFound')) - nodes_extra_specs = self.extra_specs.copy() + nodes_extra_specs = {} # NOTE(deva): In Havana and Icehouse, the flavor was required to link # to an arch-specific deploy kernel and ramdisk pair, and so the flavor @@ -382,7 +375,7 @@ class IronicDriver(virt_driver.ComputeDriver): """ icli = client_wrapper.IronicClientWrapper() node_list = icli.call("node.list", associated=True) - return list(set(n.instance_uuid for n in node_list)) + return list(n.instance_uuid for n in node_list) def node_is_available(self, nodename): """Confirms a Nova hypervisor node exists in the Ironic inventory. -- cgit v1.2.1