From 0176390531aa7bba56a12960334be4de2dd409ef Mon Sep 17 00:00:00 2001 From: Eric Fried Date: Thu, 15 Aug 2019 09:16:58 -0500 Subject: [PATCH] Introduces SDK to IronicDriver and uses for node.get We would like nova not to use ironicclient, but instead to invoke the ironic API directly through an openstacksdk connection. This commit sets up the framework to do that, and uses it for one such API call, GET /v1/nodes/{node_ident} [1]. The framework simply comprises a new attribute on the IronicDriver, ironic_connection, built using the get_sdk_adapter util. The ironicclient attribute will go away along with the entire client_wrapper module once all python-ironicclient calls have been swapped out. The API call is invoked via the get_node SDK method [2]. [1] https://developer.openstack.org/api-ref/baremetal/?expanded=show-node-details-detail#show-node-details [2] https://docs.openstack.org/openstacksdk/latest/user/proxies/baremetal.html#openstack.baremetal.v1._proxy.Proxy.get_node Blueprint: openstacksdk-in-nova Change-Id: I641f1b07beb0c44001f8018326105bcb3fc603cf --- nova/tests/unit/virt/ironic/test_driver.py | 270 ++++++++++++--------- nova/tests/unit/virt/ironic/utils.py | 63 ++++- nova/virt/ironic/driver.py | 24 +- 3 files changed, 222 insertions(+), 135 deletions(-) diff --git a/nova/tests/unit/virt/ironic/test_driver.py b/nova/tests/unit/virt/ironic/test_driver.py index 72a3de06f5c..79462aadce6 100644 --- a/nova/tests/unit/virt/ironic/test_driver.py +++ b/nova/tests/unit/virt/ironic/test_driver.py @@ -15,8 +15,10 @@ """Tests for the ironic driver.""" +import fixtures from ironicclient import exc as ironic_exception import mock +from openstack import exceptions as sdk_exc from oslo_config import cfg from oslo_service import loopingcall from oslo_utils.fixture import uuidsentinel as uuids @@ -39,7 +41,7 @@ from nova import objects from nova import servicegroup from nova import test -from nova.tests import fixtures +from nova.tests import fixtures as nova_fixtures from nova.tests.unit import fake_block_device from nova.tests.unit import fake_instance from nova.tests.unit import matchers as nova_matchers @@ -96,7 +98,7 @@ def _get_stats(): def _get_cached_node(**kw): """Return a fake node object representative of objects in the cache.""" - return ironic_utils.get_test_node(fields=ironic_driver._NODE_FIELDS, **kw) + return ironic_utils.get_test_node(**kw) def _make_compute_service(hostname): @@ -118,8 +120,15 @@ def setUp(self, mock_sg, mock_hash): self.driver = ironic_driver.IronicDriver(None) self.driver.virtapi = fake.FakeVirtAPI() + + self.mock_conn = self.useFixture( + fixtures.MockPatchObject(self.driver, '_ironic_connection')).mock + self.ctx = nova_context.get_admin_context() - self.instance_uuid = uuidutils.generate_uuid() + + # TODO(dustinc): Remove once all id/uuid usages are normalized. + self.instance_id = uuidutils.generate_uuid() + self.instance_uuid = self.instance_id self.ptree = provider_tree.ProviderTree() self.ptree.new_root(mock.sentinel.nodename, mock.sentinel.nodename) @@ -152,6 +161,26 @@ def test__get_hypervisor_type(self): def test__get_hypervisor_version(self): self.assertEqual(1, self.driver._get_hypervisor_version()) + def test__get_node(self): + node_id = uuidutils.generate_uuid() + node = _get_cached_node(id=node_id) + self.mock_conn.get_node.return_value = node + + actual = self.driver._get_node(node_id) + + self.assertEqual(node, actual) + self.mock_conn.get_node.assert_called_once_with( + node_id, fields=ironic_driver._NODE_FIELDS) + + def test__get_node_not_found(self): + node_id = uuidutils.generate_uuid() + self.mock_conn.get_node.side_effect = sdk_exc.ResourceNotFound + + self.assertRaises(sdk_exc.ResourceNotFound, + self.driver._get_node, node_id) + self.mock_conn.get_node.assert_called_once_with( + node_id, fields=ironic_driver._NODE_FIELDS) + @mock.patch.object(FAKE_CLIENT.node, 'get_by_instance_uuid') def test__validate_instance_and_node(self, mock_gbiui): node_uuid = 'aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee' @@ -560,23 +589,22 @@ def test_list_instance_uuids(self, mock_call): self.assertEqual(sorted(expected), sorted(uuids)) @mock.patch.object(FAKE_CLIENT.node, 'list') - @mock.patch.object(FAKE_CLIENT.node, 'get') @mock.patch.object(objects.InstanceList, 'get_uuids_by_host') @mock.patch.object(objects.ServiceList, 'get_all_computes_by_hv_type') def test_node_is_available_empty_cache_empty_list(self, mock_services, - mock_instances, mock_get, + mock_instances, mock_list): node = _get_cached_node() - mock_get.return_value = node + self.mock_conn.get_node.return_value = node mock_list.return_value = [] - self.assertTrue(self.driver.node_is_available(node.uuid)) - mock_get.assert_called_with(node.uuid, - fields=ironic_driver._NODE_FIELDS) + self.assertTrue(self.driver.node_is_available(node.id)) + self.mock_conn.get_node.assert_called_with( + node.id, fields=ironic_driver._NODE_FIELDS) mock_list.assert_called_with(fields=ironic_driver._NODE_FIELDS, limit=0) - mock_get.side_effect = ironic_exception.NotFound - self.assertFalse(self.driver.node_is_available(node.uuid)) + self.mock_conn.get_node.side_effect = sdk_exc.ResourceNotFound + self.assertFalse(self.driver.node_is_available(node.id)) @mock.patch.object(FAKE_CLIENT.node, 'list') @mock.patch.object(FAKE_CLIENT.node, 'get') @@ -975,26 +1003,25 @@ def test_update_provider_tree_with_traits(self, mock_nfc, mock_nr, result = self.ptree.data(mock.sentinel.nodename).traits self.assertEqual(set(traits), result) - @mock.patch.object(FAKE_CLIENT.node, 'get') @mock.patch.object(FAKE_CLIENT.node, 'list') @mock.patch.object(objects.InstanceList, 'get_uuids_by_host') @mock.patch.object(objects.ServiceList, 'get_all_computes_by_hv_type') @mock.patch.object(ironic_driver.IronicDriver, '_node_resource') def test_get_available_resource(self, mock_nr, mock_services, - mock_instances, mock_list, mock_get): + mock_instances, mock_list): node = _get_cached_node() - node_2 = _get_cached_node(uuid=uuidutils.generate_uuid()) + node_2 = _get_cached_node(id=uuidutils.generate_uuid()) fake_resource = 'fake-resource' - mock_get.return_value = node + self.mock_conn.get_node.return_value = node # ensure cache gets populated without the node we want mock_list.return_value = [node_2] mock_nr.return_value = fake_resource - result = self.driver.get_available_resource(node.uuid) + result = self.driver.get_available_resource(node.id) self.assertEqual(fake_resource, result) mock_nr.assert_called_once_with(node) - mock_get.assert_called_once_with(node.uuid, - fields=ironic_driver._NODE_FIELDS) + self.mock_conn.get_node.assert_called_once_with( + node.id, fields=ironic_driver._NODE_FIELDS) @mock.patch.object(FAKE_CLIENT.node, 'get') @mock.patch.object(FAKE_CLIENT.node, 'list') @@ -1159,13 +1186,13 @@ def test_get_info_http_not_found(self, mock_gbiu, mock_svc_by_hv, @mock.patch.object(ironic_driver.IronicDriver, '_start_firewall') def _test_spawn(self, mock_sf, mock_aiitn, mock_wait_active, mock_avti, mock_node, mock_looping, mock_save): - node_uuid = 'aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee' - node = _get_cached_node(driver='fake', uuid=node_uuid) - instance = fake_instance.fake_instance_obj(self.ctx, node=node_uuid) + node_id = 'aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee' + node = _get_cached_node(driver='fake', id=node_id) + instance = fake_instance.fake_instance_obj(self.ctx, node=node_id) fake_flavor = objects.Flavor(ephemeral_gb=0) instance.flavor = fake_flavor - mock_node.get.return_value = node + self.mock_conn.get_node.return_value = node mock_node.validate.return_value = ironic_utils.get_test_validation() mock_node.get_by_instance_uuid.return_value = node mock_node.set_provision_state.return_value = mock.MagicMock() @@ -1177,16 +1204,16 @@ def _test_spawn(self, mock_sf, mock_aiitn, mock_wait_active, self.driver.spawn(self.ctx, instance, image_meta, [], None, {}) - mock_node.get.assert_called_once_with( - node_uuid, fields=ironic_driver._NODE_FIELDS) - mock_node.validate.assert_called_once_with(node_uuid) + self.mock_conn.get_node.assert_called_once_with( + node_id, fields=ironic_driver._NODE_FIELDS) + mock_node.validate.assert_called_once_with(node_id) mock_aiitn.assert_called_once_with(node, instance, test.MatchType(objects.ImageMeta), fake_flavor, block_device_info=None) mock_avti.assert_called_once_with(self.ctx, instance, None) mock_sf.assert_called_once_with(instance, None) - mock_node.set_provision_state.assert_called_once_with(node_uuid, - 'active', configdrive=mock.ANY) + mock_node.set_provision_state.assert_called_once_with( + node_id, 'active', configdrive=mock.ANY) self.assertIsNone(instance.default_ephemeral_device) self.assertFalse(mock_save.called) @@ -1454,25 +1481,24 @@ def test__cleanup_volume_target_info_bad_request(self, mock_lvt, def test_spawn_node_driver_validation_fail(self, mock_avti, mock_node, mock_required_by): mock_required_by.return_value = False - node_uuid = 'aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee' - node = _get_cached_node(driver='fake', uuid=node_uuid) + node_id = 'aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee' + node = _get_cached_node(driver='fake', id=node_id) flavor = ironic_utils.get_test_flavor() - instance = fake_instance.fake_instance_obj(self.ctx, node=node_uuid) + instance = fake_instance.fake_instance_obj(self.ctx, node=node_id) instance.flavor = flavor mock_node.validate.return_value = ironic_utils.get_test_validation( power={'result': False}, deploy={'result': False}, storage={'result': False}) - mock_node.get.return_value = node + self.mock_conn.get_node.return_value = node image_meta = ironic_utils.get_test_image_meta() self.assertRaises(exception.ValidationError, self.driver.spawn, self.ctx, instance, image_meta, [], None, {}) - self.assertEqual(1, mock_node.get.call_count) - mock_node.get.assert_called_once_with( - node_uuid, fields=ironic_driver._NODE_FIELDS) + self.mock_conn.get_node.assert_called_once_with( + node_id, fields=ironic_driver._NODE_FIELDS) mock_avti.assert_called_once_with(self.ctx, instance, None) - mock_node.validate.assert_called_once_with(node_uuid) + mock_node.validate.assert_called_once_with(node_id) @mock.patch.object(configdrive, 'required_by') @mock.patch.object(FAKE_CLIENT, 'node') @@ -1483,12 +1509,12 @@ def test_spawn_node_prepare_for_deploy_fail(self, mock_cleanup_deploy, mock_sf, mock_avti, mock_node, mock_required_by): mock_required_by.return_value = False - node_uuid = 'aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee' - node = _get_cached_node(driver='fake', uuid=node_uuid) + node_id = 'aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee' + node = _get_cached_node(driver='fake', id=node_id) flavor = ironic_utils.get_test_flavor() - instance = fake_instance.fake_instance_obj(self.ctx, node=node_uuid) + instance = fake_instance.fake_instance_obj(self.ctx, node=node_id) instance.flavor = flavor - mock_node.get.return_value = node + self.mock_conn.get_node.return_value = node mock_node.validate.return_value = ironic_utils.get_test_validation() image_meta = ironic_utils.get_test_image_meta() @@ -1496,9 +1522,9 @@ def test_spawn_node_prepare_for_deploy_fail(self, mock_cleanup_deploy, self.assertRaises(test.TestingException, self.driver.spawn, self.ctx, instance, image_meta, [], None, {}) - mock_node.get.assert_called_once_with( - node_uuid, fields=ironic_driver._NODE_FIELDS) - mock_node.validate.assert_called_once_with(node_uuid) + self.mock_conn.get_node.assert_called_once_with( + node_id, fields=ironic_driver._NODE_FIELDS) + mock_node.validate.assert_called_once_with(node_id) mock_cleanup_deploy.assert_called_with(node, instance, None) @mock.patch.object(configdrive, 'required_by') @@ -1512,12 +1538,12 @@ def test_spawn_node_configdrive_fail(self, mock_avti, mock_node, mock_save, mock_required_by): mock_required_by.return_value = True - node_uuid = 'aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee' - node = _get_cached_node(driver='fake', uuid=node_uuid) + node_id = 'aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee' + node = _get_cached_node(driver='fake', id=node_id) flavor = ironic_utils.get_test_flavor() - instance = fake_instance.fake_instance_obj(self.ctx, node=node_uuid) + instance = fake_instance.fake_instance_obj(self.ctx, node=node_id) instance.flavor = flavor - mock_node.get.return_value = node + self.mock_conn.get_node.return_value = node mock_node.validate.return_value = ironic_utils.get_test_validation() image_meta = ironic_utils.get_test_image_meta() @@ -1527,9 +1553,9 @@ def test_spawn_node_configdrive_fail(self, self.assertRaises(test.TestingException, self.driver.spawn, self.ctx, instance, image_meta, [], None, {}) - mock_node.get.assert_called_once_with( - node_uuid, fields=ironic_driver._NODE_FIELDS) - mock_node.validate.assert_called_once_with(node_uuid) + self.mock_conn.get_node.assert_called_once_with( + node_id, fields=ironic_driver._NODE_FIELDS) + mock_node.validate.assert_called_once_with(node_id) mock_cleanup_deploy.assert_called_with(node, instance, None) @mock.patch.object(configdrive, 'required_by') @@ -1541,23 +1567,23 @@ def test_spawn_node_trigger_deploy_fail(self, mock_cleanup_deploy, mock_sf, mock_avti, mock_node, mock_required_by): mock_required_by.return_value = False - node_uuid = 'aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee' - node = _get_cached_node(driver='fake', uuid=node_uuid) + node_id = 'aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee' + node = _get_cached_node(driver='fake', id=node_id) flavor = ironic_utils.get_test_flavor() - instance = fake_instance.fake_instance_obj(self.ctx, node=node_uuid) + instance = fake_instance.fake_instance_obj(self.ctx, node=node_id) instance.flavor = flavor image_meta = ironic_utils.get_test_image_meta() - mock_node.get.return_value = node + self.mock_conn.get_node.return_value = node mock_node.validate.return_value = ironic_utils.get_test_validation() mock_node.set_provision_state.side_effect = exception.NovaException() self.assertRaises(exception.NovaException, self.driver.spawn, self.ctx, instance, image_meta, [], None, {}) - mock_node.get.assert_called_once_with( - node_uuid, fields=ironic_driver._NODE_FIELDS) - mock_node.validate.assert_called_once_with(node_uuid) + self.mock_conn.get_node.assert_called_once_with( + node_id, fields=ironic_driver._NODE_FIELDS) + mock_node.validate.assert_called_once_with(node_id) mock_cleanup_deploy.assert_called_once_with(node, instance, None) @mock.patch.object(configdrive, 'required_by') @@ -1569,23 +1595,23 @@ def test_spawn_node_trigger_deploy_fail2(self, mock_cleanup_deploy, mock_sf, mock_avti, mock_node, mock_required_by): mock_required_by.return_value = False - node_uuid = 'aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee' - node = _get_cached_node(driver='fake', uuid=node_uuid) + node_id = 'aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee' + node = _get_cached_node(driver='fake', id=node_id) flavor = ironic_utils.get_test_flavor() - instance = fake_instance.fake_instance_obj(self.ctx, node=node_uuid) + instance = fake_instance.fake_instance_obj(self.ctx, node=node_id) instance.flavor = flavor image_meta = ironic_utils.get_test_image_meta() - mock_node.get.return_value = node + self.mock_conn.get_node.return_value = node mock_node.validate.return_value = ironic_utils.get_test_validation() mock_node.set_provision_state.side_effect = ironic_exception.BadRequest self.assertRaises(ironic_exception.BadRequest, self.driver.spawn, self.ctx, instance, image_meta, [], None, {}) - mock_node.get.assert_called_once_with( - node_uuid, fields=ironic_driver._NODE_FIELDS) - mock_node.validate.assert_called_once_with(node_uuid) + self.mock_conn.get_node.assert_called_once_with( + node_id, fields=ironic_driver._NODE_FIELDS) + mock_node.validate.assert_called_once_with(node_id) mock_cleanup_deploy.assert_called_once_with(node, instance, None) @mock.patch.object(configdrive, 'required_by') @@ -1599,14 +1625,14 @@ def test_spawn_node_trigger_deploy_fail3(self, mock_destroy, mock_node, mock_looping, mock_required_by): mock_required_by.return_value = False - node_uuid = 'aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee' - node = _get_cached_node(driver='fake', uuid=node_uuid) + node_id = 'aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee' + node = _get_cached_node(driver='fake', id=node_id) flavor = ironic_utils.get_test_flavor() - instance = fake_instance.fake_instance_obj(self.ctx, node=node_uuid) + instance = fake_instance.fake_instance_obj(self.ctx, node=node_id) instance.flavor = flavor image_meta = ironic_utils.get_test_image_meta() - mock_node.get.return_value = node + self.mock_conn.get_node.return_value = node mock_node.validate.return_value = ironic_utils.get_test_validation() fake_looping_call = FakeLoopingCall() @@ -1967,20 +1993,19 @@ def test_plug_vifs_with_port(self, mock_vatt): # asserts mock_vatt.assert_called_with(node.uuid, vif_id) - @mock.patch.object(FAKE_CLIENT.node, 'get') @mock.patch.object(ironic_driver.IronicDriver, '_plug_vifs') - def test_plug_vifs(self, mock__plug_vifs, mock_get): - node_uuid = 'aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee' - node = _get_cached_node(uuid=node_uuid) + def test_plug_vifs(self, mock__plug_vifs): + node_id = 'aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee' + node = _get_cached_node(id=node_id) - mock_get.return_value = node + self.mock_conn.get_node.return_value = node instance = fake_instance.fake_instance_obj(self.ctx, - node=node_uuid) + node=node_id) network_info = utils.get_test_network_info() self.driver.plug_vifs(instance, network_info) - mock_get.assert_called_once_with(node_uuid, - fields=ironic_driver._NODE_FIELDS) + self.mock_conn.get_node.assert_called_once_with( + node_id, fields=ironic_driver._NODE_FIELDS) mock__plug_vifs.assert_called_once_with(node, instance, network_info) @mock.patch.object(FAKE_CLIENT.node, 'vif_attach') @@ -2079,33 +2104,33 @@ def test_plug_vifs_no_network_info(self, mock_vatt): @mock.patch.object(FAKE_CLIENT, 'node') def test_unplug_vifs(self, mock_node): - node_uuid = 'aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee' - node = _get_cached_node(uuid=node_uuid) - mock_node.get.return_value = node + node_id = 'aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee' + node = _get_cached_node(id=node_id) + self.mock_conn.get_node.return_value = node instance = fake_instance.fake_instance_obj(self.ctx, - node=node_uuid) + node=node_id) network_info = utils.get_test_network_info() vif_id = six.text_type(network_info[0]['id']) self.driver.unplug_vifs(instance, network_info) # asserts - mock_node.get.assert_called_once_with( - node_uuid, fields=ironic_driver._NODE_FIELDS) - mock_node.vif_detach.assert_called_once_with(node.uuid, vif_id) + self.mock_conn.get_node.assert_called_once_with( + node_id, fields=ironic_driver._NODE_FIELDS) + mock_node.vif_detach.assert_called_once_with(node.id, vif_id) @mock.patch.object(FAKE_CLIENT, 'node') def test_unplug_vifs_port_not_associated(self, mock_node): - node_uuid = 'aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee' - node = _get_cached_node(uuid=node_uuid) + node_id = 'aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee' + node = _get_cached_node(id=node_id) - mock_node.get.return_value = node - instance = fake_instance.fake_instance_obj(self.ctx, node=node_uuid) + self.mock_conn.get_node.return_value = node + instance = fake_instance.fake_instance_obj(self.ctx, node=node_id) network_info = utils.get_test_network_info() self.driver.unplug_vifs(instance, network_info) - mock_node.get.assert_called_once_with( - node_uuid, fields=ironic_driver._NODE_FIELDS) + self.mock_conn.get_node.assert_called_once_with( + node_id, fields=ironic_driver._NODE_FIELDS) self.assertEqual(len(network_info), mock_node.vif_detach.call_count) @mock.patch.object(FAKE_CLIENT.node, 'vif_detach') @@ -2166,16 +2191,13 @@ def test_refresh_security_group_rules(self, mock_risr): @mock.patch.object(FAKE_CLIENT.node, 'set_provision_state') @mock.patch.object(ironic_driver.IronicDriver, '_add_instance_info_to_node') - @mock.patch.object(FAKE_CLIENT.node, 'get') @mock.patch.object(objects.Instance, 'save') - def _test_rebuild(self, mock_save, mock_get, mock_add_instance_info, - mock_set_pstate, mock_looping, mock_wait_active, - preserve=False): - node_uuid = uuidutils.generate_uuid() - node = _get_cached_node( - uuid=node_uuid, instance_uuid=self.instance_uuid, - instance_type_id=5) - mock_get.return_value = node + def _test_rebuild(self, mock_save, mock_add_instance_info, mock_set_pstate, + mock_looping, mock_wait_active, preserve=False): + node_id = uuidutils.generate_uuid() + node = _get_cached_node(id=node_id, instance_id=self.instance_id, + instance_type_id=5) + self.mock_conn.get_node.return_value = node image_meta = ironic_utils.get_test_image_meta() flavor_id = 5 @@ -2183,7 +2205,7 @@ def _test_rebuild(self, mock_save, mock_get, mock_add_instance_info, instance = fake_instance.fake_instance_obj(self.ctx, uuid=self.instance_uuid, - node=node_uuid, + node=node_id, instance_type_id=flavor_id) instance.flavor = flavor @@ -2202,7 +2224,7 @@ def _test_rebuild(self, mock_save, mock_get, mock_add_instance_info, node, instance, test.MatchType(objects.ImageMeta), flavor, preserve) - mock_set_pstate.assert_called_once_with(node_uuid, + mock_set_pstate.assert_called_once_with(node_id, ironic_states.REBUILD, configdrive=mock.ANY) mock_looping.assert_called_once_with(mock_wait_active, instance) @@ -2498,21 +2520,20 @@ def test_ironicclient_bad_response(self, mock_error, mock_node): @mock.patch.object(cw.IronicClientWrapper, 'call') def test_prepare_for_spawn(self, mock_call): node = ironic_utils.get_test_node(driver='fake') - mock_call.side_effect = [node, None] + self.mock_conn.get_node.return_value = node instance = fake_instance.fake_instance_obj(self.ctx, node=node.uuid) self.driver.prepare_for_spawn(instance) expected_patch = [{'path': '/instance_uuid', 'op': 'add', 'value': instance.uuid}] - self.assertEqual(2, mock_call.call_count) - mock_call.assert_has_calls( - [mock.call('node.get', node.uuid, - fields=('uuid', 'power_state', 'target_power_state', - 'provision_state', 'target_provision_state', - 'last_error', 'maintenance', 'properties', - 'instance_uuid', 'traits', 'resource_class')), - mock.call('node.update', node.uuid, - expected_patch, retry_on_conflict=False)]) + self.mock_conn.get_node.assert_called_once_with( + node.uuid, + fields=('uuid', 'power_state', 'target_power_state', + 'provision_state', 'target_provision_state', 'last_error', + 'maintenance', 'properties', 'instance_uuid', 'traits', + 'resource_class')) + mock_call.assert_called_once_with( + 'node.update', node.uuid, expected_patch, retry_on_conflict=False) @mock.patch.object(cw.IronicClientWrapper, 'call') def test__set_instance_uuid(self, mock_call): @@ -2536,9 +2557,9 @@ def test_prepare_for_spawn_invalid_instance(self): @mock.patch.object(cw.IronicClientWrapper, 'call') def test_prepare_for_spawn_conflict(self, mock_call): node = ironic_utils.get_test_node(driver='fake') - mock_call.side_effect = [node, ironic_exception.BadRequest] - instance = fake_instance.fake_instance_obj(self.ctx, - node=node.uuid) + self.mock_conn.get_node.return_value = node + mock_call.side_effect = ironic_exception.BadRequest + instance = fake_instance.fake_instance_obj(self.ctx, node=node.id) self.assertRaises(exception.InstanceDeployFailure, self.driver.prepare_for_spawn, instance) @@ -2602,7 +2623,10 @@ def setUp(self): self.driver.node_cache = {} # Since the code we're testing runs in a spawn_n green thread, ensure # that the thread completes. - self.useFixture(fixtures.SpawnIsSynchronousFixture()) + self.useFixture(nova_fixtures.SpawnIsSynchronousFixture()) + + self.mock_conn = self.useFixture( + fixtures.MockPatchObject(self.driver, '_ironic_connection')).mock @mock.patch.object(ironic_driver.IronicDriver, '_get_node_list') @mock.patch.object(objects.ServiceList, 'get_all_computes_by_hv_type') @@ -3144,12 +3168,15 @@ def setUp(self, mock_services): self.driver = ironic_driver.IronicDriver(None) self.driver.virtapi = fake.FakeVirtAPI() self.ctx = nova_context.get_admin_context() - node_uuid = uuidutils.generate_uuid() - self.node = _get_cached_node(driver='fake', uuid=node_uuid) + node_id = uuidutils.generate_uuid() + self.node = _get_cached_node(driver='fake', id=node_id) self.instance = fake_instance.fake_instance_obj(self.ctx, - node=node_uuid) + node=node_id) self.network_info = utils.get_test_network_info() + self.mock_conn = self.useFixture( + fixtures.MockPatchObject(self.driver, '_ironic_connection')).mock + def test_generate_configdrive(self, mock_cd_builder, mock_instance_meta): mock_instance_meta.return_value = 'fake-instance' mock_make_drive = mock.MagicMock(make_drive=lambda *_: None) @@ -3570,11 +3597,16 @@ def setUp(self, mock_services): super(IronicDriverConsoleTestCase, self).setUp() self.driver = ironic_driver.IronicDriver(fake.FakeVirtAPI()) + + self.mock_conn = self.useFixture( + fixtures.MockPatchObject(self.driver, '_ironic_connection')).mock + self.ctx = nova_context.get_admin_context() - node_uuid = uuidutils.generate_uuid() - self.node = _get_cached_node(driver='fake', uuid=node_uuid) + + node_id = uuidutils.generate_uuid() + self.node = _get_cached_node(driver='fake', id=node_id) self.instance = fake_instance.fake_instance_obj(self.ctx, - node=node_uuid) + node=node_id) # mock retries configs to avoid sleeps and make tests run quicker CONF.set_default('api_max_retries', default=1, group='ironic') diff --git a/nova/tests/unit/virt/ironic/utils.py b/nova/tests/unit/virt/ironic/utils.py index 0ccc30cb6e9..0c46d00043c 100644 --- a/nova/tests/unit/virt/ironic/utils.py +++ b/nova/tests/unit/virt/ironic/utils.py @@ -28,7 +28,14 @@ def get_test_validation(**kw): def get_test_node(fields=None, **kw): - node = {'uuid': kw.get('uuid', 'eeeeeeee-dddd-cccc-bbbb-aaaaaaaaaaaa'), + # TODO(dustinc): Once the usages of id/uuid, maintenance/is_maintenance, + # and portgroup/port_group are normalized, the duplicates can be removed + _id = kw.get('id') or kw.get('uuid', + 'eeeeeeee-dddd-cccc-bbbb-aaaaaaaaaaaa') + _instance_id = kw.get('instance_id') or kw.get('instance_uuid') + _is_maintenance = kw.get('is_maintenance') or kw.get('maintenance', False) + node = {'uuid': _id, + 'id': _id, 'chassis_uuid': kw.get('chassis_uuid'), 'power_state': kw.get('power_state', ironic_states.NOSTATE), @@ -39,13 +46,15 @@ def get_test_node(fields=None, **kw): 'target_provision_state': kw.get('target_provision_state', ironic_states.NOSTATE), 'last_error': kw.get('last_error'), - 'instance_uuid': kw.get('instance_uuid'), + 'instance_uuid': _instance_id, + 'instance_id': _instance_id, 'instance_info': kw.get('instance_info'), 'driver': kw.get('driver', 'fake'), 'driver_info': kw.get('driver_info', {}), 'properties': kw.get('properties', {}), 'reservation': kw.get('reservation'), - 'maintenance': kw.get('maintenance', False), + 'maintenance': _is_maintenance, + 'is_maintenance': _is_maintenance, 'network_interface': kw.get('network_interface'), 'resource_class': kw.get('resource_class'), 'traits': kw.get('traits', []), @@ -58,21 +67,37 @@ def get_test_node(fields=None, **kw): def get_test_port(**kw): + # TODO(dustinc): Once the usages of id/uuid, maintenance/is_maintenance, + # and portgroup/port_group are normalized, the duplicates can be removed + _id = kw.get('id') or kw.get('uuid', + 'gggggggg-uuuu-qqqq-ffff-llllllllllll') + _node_id = kw.get('node_uuid') or kw.get('node_id', get_test_node().id) + _port_group_id = kw.get('port_group_id') or kw.get('portgroup_uuid') return type('port', (object,), - {'uuid': kw.get('uuid', 'gggggggg-uuuu-qqqq-ffff-llllllllllll'), - 'node_uuid': kw.get('node_uuid', get_test_node().uuid), + {'uuid': _id, + 'id': _id, + 'node_uuid': _node_id, + 'node_id': _node_id, 'address': kw.get('address', 'FF:FF:FF:FF:FF:FF'), 'extra': kw.get('extra', {}), 'internal_info': kw.get('internal_info', {}), - 'portgroup_uuid': kw.get('portgroup_uuid'), + 'portgroup_uuid': _port_group_id, + 'port_group_id': _port_group_id, 'created_at': kw.get('created_at'), 'updated_at': kw.get('updated_at')})() def get_test_portgroup(**kw): + # TODO(dustinc): Once the usages of id/uuid, maintenance/is_maintenance, + # and portgroup/port_group are normalized, the duplicates can be removed + _id = kw.get('id') or kw.get('uuid', + 'deaffeed-1234-5678-9012-fedcbafedcba') + _node_id = kw.get('node_id') or kw.get('node_uuid', get_test_node().id) return type('portgroup', (object,), - {'uuid': kw.get('uuid', 'deaffeed-1234-5678-9012-fedcbafedcba'), - 'node_uuid': kw.get('node_uuid', get_test_node().uuid), + {'uuid': _id, + 'id': _id, + 'node_uuid': _node_id, + 'node_id': _node_id, 'address': kw.get('address', 'EE:EE:EE:EE:EE:EE'), 'extra': kw.get('extra', {}), 'internal_info': kw.get('internal_info', {}), @@ -104,9 +129,16 @@ def get_test_vif(**kw): def get_test_volume_connector(**kw): + # TODO(dustinc): Once the usages of id/uuid, maintenance/is_maintenance, + # and portgroup/port_group are normalized, the duplicates can be removed + _id = kw.get('id') or kw.get('uuid', + 'hhhhhhhh-qqqq-uuuu-mmmm-bbbbbbbbbbbb') + _node_id = kw.get('node_id') or kw.get('node_uuid', get_test_node().id) return type('volume_connector', (object,), - {'uuid': kw.get('uuid', 'hhhhhhhh-qqqq-uuuu-mmmm-bbbbbbbbbbbb'), - 'node_uuid': kw.get('node_uuid', get_test_node().uuid), + {'uuid': _id, + 'id': _id, + 'node_uuid': _node_id, + 'node_id': _node_id, 'type': kw.get('type', 'iqn'), 'connector_id': kw.get('connector_id', 'iqn.test'), 'extra': kw.get('extra', {}), @@ -115,9 +147,16 @@ def get_test_volume_connector(**kw): def get_test_volume_target(**kw): + # TODO(dustinc): Once the usages of id/uuid, maintenance/is_maintenance, + # and portgroup/port_group are normalized, the duplicates can be removed + _id = kw.get('id') or kw.get('uuid', + 'aaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee') + _node_id = kw.get('node_id') or kw.get('node_uuid', get_test_node().id) return type('volume_target', (object,), - {'uuid': kw.get('uuid', 'aaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee'), - 'node_uuid': kw.get('node_uuid', get_test_node().uuid), + {'uuid': _id, + 'id': _id, + 'node_uuid': _node_id, + 'node_id': _node_id, 'volume_type': kw.get('volume_type', 'iscsi'), 'properties': kw.get('properties', {}), 'boot_index': kw.get('boot_index', 0), diff --git a/nova/virt/ironic/driver.py b/nova/virt/ironic/driver.py index f624ffad360..0d16ccf6652 100644 --- a/nova/virt/ironic/driver.py +++ b/nova/virt/ironic/driver.py @@ -25,6 +25,7 @@ import tempfile import time +from openstack import exceptions as sdk_exc from oslo_log import log as logging from oslo_serialization import jsonutils from oslo_service import loopingcall @@ -197,20 +198,35 @@ def __init__(self, virtapi, read_only=False): self.servicegroup_api = servicegroup.API() self.ironicclient = client_wrapper.IronicClientWrapper() + self._ironic_connection = None # This is needed for the instance flavor migration in Pike, and should # be removed in Queens. Since this will run several times in the life # of the driver, track the instances that have already been migrated. self._migrated_instance_uuids = set() - def _get_node(self, node_uuid): + @property + def ironic_connection(self): + if self._ironic_connection is None: + # Ask get_sdk_adapter to raise ServiceUnavailable if the baremetal + # service isn't ready yet. Consumers of ironic_connection are set + # up to handle this and raise VirtDriverNotReady as appropriate. + self._ironic_connection = utils.get_sdk_adapter( + 'baremetal', check_service=True) + return self._ironic_connection + + def _get_node(self, node_id): """Get a node by its UUID. Some methods pass in variables named nodename, but are actually UUID's. """ - return self.ironicclient.call('node.get', node_uuid, - fields=_NODE_FIELDS) + node = self.ironic_connection.get_node(node_id, fields=_NODE_FIELDS) + # TODO(dustinc): Make consumers use the right fields and remove this + node.uuid = node.id + node.instance_uuid = node.instance_id + node.maintenance = node.is_maintenance + return node def _validate_instance_and_node(self, instance): """Get the node associated with the instance. @@ -698,7 +714,7 @@ def node_is_available(self, nodename): # nodename is the ironic node's UUID. self._get_node(nodename) return True - except ironic.exc.NotFound: + except sdk_exc.ResourceNotFound: return False def _refresh_hash_ring(self, ctxt):