From 4de4fb4a5aeb25e05f85d4696d60cd2794084348 Mon Sep 17 00:00:00 2001 From: Monty Taylor Date: Tue, 5 Sep 2017 09:46:41 -0500 Subject: [PATCH] Begin converting baremetal node tests Shamelessly based upon pre-existing requests mock code in shade, with the attempt to replace the existing baremetal client mock testing. This updates the way we're creating the ironic_client so that it always does discovery, which _baremetal_client will always have to do because of how the versioned discovery document is in ironic. It also puts in a workaround for a bug that was found in keystoneauth's new version discovery that we should eventually take out. Change-Id: I0e811d69c3cba50f164cb2a3a1302b88fba51308 --- shade/_legacy_clients.py | 41 ++++++++++++++ shade/openstackcloud.py | 15 +++--- shade/tests/unit/base.py | 29 ++++++++++ shade/tests/unit/fixtures/baremetal.json | 30 +++++++++++ shade/tests/unit/test_baremetal_node.py | 59 ++++++++++++++++++++ shade/tests/unit/test_operator_noauth.py | 69 ++++++++++++++---------- shade/tests/unit/test_shade_operator.py | 20 ------- 7 files changed, 210 insertions(+), 53 deletions(-) create mode 100644 shade/tests/unit/fixtures/baremetal.json create mode 100644 shade/tests/unit/test_baremetal_node.py diff --git a/shade/_legacy_clients.py b/shade/_legacy_clients.py index b5ae535aa..e67b35d3d 100644 --- a/shade/_legacy_clients.py +++ b/shade/_legacy_clients.py @@ -12,9 +12,11 @@ import importlib import warnings +from keystoneauth1 import plugin from os_client_config import constructors from shade import _utils +from shade import exc class LegacyClientFactoryMixin(object): @@ -144,8 +146,47 @@ class LegacyClientFactoryMixin(object): def _get_legacy_ironic_microversion(self): return '1.6' + def _join_ksa_version(self, version): + return ".".join([str(x) for x in version]) + @property def ironic_client(self): + # Trigger discovery from ksa. This will make ironicclient and + # keystoneauth1.adapter.Adapter code paths both go through discovery. + # ironicclient does its own magic with discovery, so we won't + # pass an endpoint_override here like we do for keystoneclient. + # Just so it's not wasted though, make sure we can handle the + # min microversion we need. + needed = self._get_legacy_ironic_microversion() + + # TODO(mordred) Bug in ksa - don't do microversion matching for + # auth_type = admin_token. Remove this if when the fix lands. + if (hasattr(plugin.BaseAuthPlugin, 'get_endpoint_data') or + self.cloud_config.config['auth_type'] not in ( + 'admin_token', 'none')): + # TODO(mordred) once we're on REST properly, we need a better + # method for matching requested and available microversion + endpoint_data = self._baremetal_client.get_endpoint_data() + if not endpoint_data.min_microversion: + raise exc.OpenStackCloudException( + "shade needs an ironic that supports microversions") + if endpoint_data.min_microversion[1] > int(needed[-1]): + raise exc.OpenStackCloudException( + "shade needs an ironic that supports microversion {needed}" + " but the ironic found has a minimum microversion" + " of {found}".format( + needed=needed, + found=self._join_ksa_version( + endpoint_data.min_microversion))) + if endpoint_data.max_microversion[1] < int(needed[-1]): + raise exc.OpenStackCloudException( + "shade needs an ironic that supports microversion {needed}" + " but the ironic found has a maximum microversion" + " of {found}".format( + needed=needed, + found=self._join_ksa_version( + endpoint_data.max_microversion))) + return self._create_legacy_client( 'ironic', 'baremetal', deprecated=False, module_name='ironicclient.client.Client', diff --git a/shade/openstackcloud.py b/shade/openstackcloud.py index 3785b654f..d711ec71c 100644 --- a/shade/openstackcloud.py +++ b/shade/openstackcloud.py @@ -418,7 +418,8 @@ class OpenStackCloud( # data.api_version can be None if no version was detected, such # as with neutron - api_version = adapter.get_api_major_version() + api_version = adapter.get_api_major_version( + endpoint_override=self.cloud_config.get_endpoint(service_type)) api_major = self._get_major_version_id(api_version) # If we detect a different version that was configured, warn the user. @@ -468,11 +469,13 @@ class OpenStackCloud( def _baremetal_client(self): if 'baremetal' not in self._raw_clients: client = self._get_raw_client('baremetal') - # TODO(mordred) Fix this once we've migrated all the way to REST - # Don't bother with version discovery - there is only one version - # of ironic. This is what ironicclient does, fwiw. - client.endpoint_override = urllib.parse.urljoin( - client.get_endpoint(), 'v1') + # Do this to force version discovery. We need to do that, because + # the endpoint-override trick we do for neutron because + # ironicclient just appends a /v1 won't work and will break + # keystoneauth - because ironic's versioned discovery endpoint + # is non-compliant and doesn't return an actual version dict. + client = self._get_versioned_client( + 'baremetal', min_version=1, max_version='1.latest') self._raw_clients['baremetal'] = client return self._raw_clients['baremetal'] diff --git a/shade/tests/unit/base.py b/shade/tests/unit/base.py index bbbcab7b0..be24b1731 100644 --- a/shade/tests/unit/base.py +++ b/shade/tests/unit/base.py @@ -466,6 +466,12 @@ class RequestsMockTestCase(BaseTestCase): return dict(method='GET', uri="https://dns.example.com/", text=open(discovery_fixture, 'r').read()) + def get_ironic_discovery_mock_dict(self): + discovery_fixture = os.path.join( + self.fixtures_directory, "baremetal.json") + return dict(method='GET', uri="https://bare-metal.example.com/", + text=open(discovery_fixture, 'r').read()) + def use_glance(self, image_version_json='image-version.json'): # NOTE(notmorgan): This method is only meant to be used in "setUp" # where the ordering of the url being registered is tightly controlled @@ -484,6 +490,15 @@ class RequestsMockTestCase(BaseTestCase): self.__do_register_uris([ self.get_designate_discovery_mock_dict()]) + def use_ironic(self): + # NOTE(TheJulia): This method is only meant to be used in "setUp" + # where the ordering of the url being registered is tightly controlled + # if the functionality of .use_ironic is meant to be used during an + # actual test case, use .get_ironic_discovery_mock and apply to the + # right location in the mock_uris when calling .register_uris + self.__do_register_uris([ + self.get_ironic_discovery_mock_dict()]) + def register_uris(self, uri_mock_list=None): """Mock a list of URIs and responses via requests mock. @@ -613,3 +628,17 @@ class RequestsMockTestCase(BaseTestCase): if do_count: self.assertEqual( len(self.calls), len(self.adapter.request_history)) + + +class IronicTestCase(RequestsMockTestCase): + + def setUp(self): + super(IronicTestCase, self).setUp() + self.use_ironic() + self.uuid = str(uuid.uuid4()) + self.name = self.getUniqueString('name') + + def get_mock_url(self, resource=None, append=None, qs_elements=None): + return super(IronicTestCase, self).get_mock_url( + service_type='baremetal', interface='public', resource=resource, + append=append, base_url_append='v1', qs_elements=qs_elements) diff --git a/shade/tests/unit/fixtures/baremetal.json b/shade/tests/unit/fixtures/baremetal.json new file mode 100644 index 000000000..fa0a9e7a7 --- /dev/null +++ b/shade/tests/unit/fixtures/baremetal.json @@ -0,0 +1,30 @@ +{ + "default_version": { + "id": "v1", + "links": [ + { + "href": "https://bare-metal.example.com/v1/", + "rel": "self" + } + ], + "min_version": "1.1", + "status": "CURRENT", + "version": "1.33" + }, + "description": "Ironic is an OpenStack project which aims to provision baremetal machines.", + "name": "OpenStack Ironic API", + "versions": [ + { + "id": "v1", + "links": [ + { + "href": "https://bare-metal.example.com/v1/", + "rel": "self" + } + ], + "min_version": "1.1", + "status": "CURRENT", + "version": "1.33" + } + ] +} diff --git a/shade/tests/unit/test_baremetal_node.py b/shade/tests/unit/test_baremetal_node.py new file mode 100644 index 000000000..2ae1af632 --- /dev/null +++ b/shade/tests/unit/test_baremetal_node.py @@ -0,0 +1,59 @@ +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +""" +test_baremetal_node +---------------------------------- + +Tests for baremetal node related operations +""" + +import uuid + +from shade.tests import fakes +from shade.tests.unit import base + + +class TestBaremetalNode(base.IronicTestCase): + + def setUp(self): + super(TestBaremetalNode, self).setUp() + self.fake_baremetal_node = fakes.make_fake_machine( + self.name, self.uuid) + + def test_list_machines(self): + fake_baremetal_two = fakes.make_fake_machine('two', str(uuid.uuid4())) + self.register_uris([ + dict(method='GET', + uri=self.get_mock_url(resource='nodes'), + json={'nodes': [self.fake_baremetal_node, + fake_baremetal_two]}), + ]) + + machines = self.op_cloud.list_machines() + self.assertEqual(2, len(machines)) + self.assertEqual(self.fake_baremetal_node, machines[0]) + self.assert_calls() + + def test_get_machine(self): + self.register_uris([ + dict(method='GET', + uri=self.get_mock_url( + resource='nodes', + append=[self.fake_baremetal_node['uuid']]), + json=self.fake_baremetal_node), + ]) + + machine = self.op_cloud.get_machine(self.fake_baremetal_node['uuid']) + self.assertEqual(machine['uuid'], + self.fake_baremetal_node['uuid']) + self.assert_calls() diff --git a/shade/tests/unit/test_operator_noauth.py b/shade/tests/unit/test_operator_noauth.py index 37b760f3f..e489d2e84 100644 --- a/shade/tests/unit/test_operator_noauth.py +++ b/shade/tests/unit/test_operator_noauth.py @@ -12,15 +12,13 @@ # See the License for the specific language governing permissions and # limitations under the License. -import mock +from keystoneauth1 import plugin -import ironicclient -from os_client_config import cloud_config import shade -from shade.tests import base +from shade.tests.unit import base -class TestShadeOperatorNoAuth(base.TestCase): +class TestShadeOperatorNoAuth(base.RequestsMockTestCase): def setUp(self): """Setup Noauth OperatorCloud tests @@ -28,32 +26,49 @@ class TestShadeOperatorNoAuth(base.TestCase): URL in the auth data. This is permits testing of the basic mechanism that enables Ironic noauth mode to be utilized with Shade. + + Uses base.RequestsMockTestCase instead of IronicTestCase because + we need to do completely different things with discovery. """ super(TestShadeOperatorNoAuth, self).setUp() - self.cloud_noauth = shade.operator_cloud( - auth_type='admin_token', - auth=dict(endpoint="http://localhost:6385"), - validate=False, - ) + # By clearing the URI registry, we remove all calls to a keystone + # catalog or getting a token + self._uri_registry.clear() + # TODO(mordred) Remove this if with next KSA release + if hasattr(plugin.BaseAuthPlugin, 'get_endpoint_data'): + self.use_ironic() + self.register_uris([ + dict(method='GET', + uri=self.get_mock_url( + service_type='baremetal', base_url_append='v1', + resource='nodes'), + json={'nodes': []}), + ]) - @mock.patch.object(cloud_config.CloudConfig, 'get_session') - @mock.patch.object(ironicclient.client, 'Client') - def test_ironic_noauth_selection_using_a_task( - self, mock_client, get_session_mock): + def test_ironic_noauth_none_auth_type(self): """Test noauth selection for Ironic in OperatorCloud - Utilize a task to trigger the client connection attempt - and evaluate if get_session_endpoint was called while the client - was still called. - - We want session_endpoint to be called because we're storing the - endpoint in a noauth token Session object now. + The new way of doing this is with the keystoneauth none plugin. """ - session_mock = mock.Mock() - session_mock.get_endpoint.return_value = None - session_mock.get_token.return_value = 'yankee' - get_session_mock.return_value = session_mock + self.cloud_noauth = shade.operator_cloud( + auth_type='none', + baremetal_endpoint_override="https://bare-metal.example.com") - self.cloud_noauth.patch_machine('name', {}) - self.assertTrue(get_session_mock.called) - self.assertTrue(mock_client.called) + self.cloud_noauth.list_machines() + + self.assert_calls() + + def test_ironic_noauth_admin_token_auth_type(self): + """Test noauth selection for Ironic in OperatorCloud + + The old way of doing this was to abuse admin_token. + """ + self.cloud_noauth = shade.operator_cloud( + auth_type='admin_token', + auth=dict( + endpoint='https://bare-metal.example.com', + token='ignored')) + + self.cloud_noauth.list_machines() + + self.assert_calls() diff --git a/shade/tests/unit/test_shade_operator.py b/shade/tests/unit/test_shade_operator.py index f14333a5c..fec16a594 100644 --- a/shade/tests/unit/test_shade_operator.py +++ b/shade/tests/unit/test_shade_operator.py @@ -37,29 +37,9 @@ class TestShadeOperator(base.RequestsMockTestCase): machine_id=self.machine_id, machine_name=self.machine_name) - def get_ironic_mock_url(self, append=None, *args, **kwargs): - if append: - # TODO(mordred): Remove when we do version discovery - # properly everywhere - append.insert(0, 'v1') - return self.get_mock_url('baremetal', append=append, *args, **kwargs) - def test_operator_cloud(self): self.assertIsInstance(self.op_cloud, shade.OperatorCloud) - def test_get_machine(self): - - self.register_uris([ - dict(method='GET', - uri=self.get_ironic_mock_url( - append=['nodes', self.machine_name]), - json=self.node), - ]) - machine = self.op_cloud.get_machine(self.machine_name) - self.assertEqual(self.node, machine) - - self.assert_calls() - @mock.patch.object(shade.OperatorCloud, 'ironic_client') def test_get_machine_by_mac(self, mock_client): class port_value(object):