From 0eb9ae7445d6b6077d55c54ce9c4337e7d8ebc8a Mon Sep 17 00:00:00 2001 From: Julia Kreger Date: Tue, 26 Mar 2024 08:47:26 -0700 Subject: [PATCH] Leverage the remote microversion without requiring human config Tempest plugins are... weird. The challenge is to have a branchless utility which can be loaded and help provide feedback if the remote service is correctly responding as we would expect. This works great in theory, until you have to either do some sort of negative test, or plan in advance, or until you have some sort of mixed state environment. This also weirdly restraints testing against older versions on older branches, requiring further care and feeding to keep things passing. And the way issues like these are resolved, originally, was to leverage manual human configuration. The problem is, that doesn't always work and operationally becomes an increased burden. So the logical path forward is for the plugin to automatically skip specific tests *based upon* the remote offered API microversion, much like many of the tests do if a driver or running configuration does not exist. This can be done because when we compose tests, we have a minimum and maximum API version where we know the test is valid, and if the remote endpoint is outside of that bound. The result is now the plugin will query the remote endpoint and collect the minimum and maximum API versions as part of skip version testing, so if either are defined on a test class, then we make a decision automatically removing the need to configure aspects specifically. Change-Id: I197e6c30c8514e1f72cb1ce3ebad851802632203 --- ironic_tempest_plugin/services/baremetal/base.py | 14 ++++++++++++++ ironic_tempest_plugin/tests/api/base.py | 11 +++++++++++ .../tests/scenario/baremetal_manager.py | 12 ++++++++++++ 3 files changed, 37 insertions(+) diff --git a/ironic_tempest_plugin/services/baremetal/base.py b/ironic_tempest_plugin/services/baremetal/base.py index 2acad18d..f23310fb 100644 --- a/ironic_tempest_plugin/services/baremetal/base.py +++ b/ironic_tempest_plugin/services/baremetal/base.py @@ -63,9 +63,23 @@ class BaremetalClient(rest_client.RestClient): def get_headers(self): headers = super(BaremetalClient, self).get_headers() if BAREMETAL_MICROVERSION: + # NOTE(TheJulia): This is not great, because it can blind a test + # to the actual version supported. headers[self.api_microversion_header_name] = BAREMETAL_MICROVERSION return headers + def get_raw_headers(self): + """A proper get headers without guessing the microversion.""" + return super(BaremetalClient, self).get_headers() + + def get_min_max_api_microversions(self): + """Returns a tuple of minimum and remote microversions.""" + _, resp_body = self._show_request(None, uri='/') + version = resp_body.get('default_version', {}) + api_min = version.get('min_version') + api_max = version.get('version') + return (api_min, api_max) + def request(self, *args, **kwargs): resp, resp_body = super(BaremetalClient, self).request(*args, **kwargs) latest_microversion = api_version_utils.LATEST_MICROVERSION diff --git a/ironic_tempest_plugin/tests/api/base.py b/ironic_tempest_plugin/tests/api/base.py index 6ebb162a..59f2f946 100644 --- a/ironic_tempest_plugin/tests/api/base.py +++ b/ironic_tempest_plugin/tests/api/base.py @@ -76,6 +76,8 @@ class BaseBaremetalTest(api_version_utils.BaseMicroversionTest, cfg_min_version = CONF.baremetal.min_microversion cfg_max_version = CONF.baremetal.max_microversion + + # Check versions and skip based upon *configuration* api_version_utils.check_skip_with_microversion(cls.min_microversion, cls.max_microversion, cfg_min_version, @@ -100,6 +102,15 @@ class BaseBaremetalTest(api_version_utils.BaseMicroversionTest, else: cls.client = cls.os_admin.baremetal.BaremetalClient() + # Skip the test if the class version doesn't match. + if cls.min_microversion or cls.max_microversion: + api_min, api_max = cls.client.get_min_max_api_microversions() + api_version_utils.check_skip_with_microversion( + cls.min_microversion, + cls.max_microversion, + api_min, + api_max) + @classmethod def resource_setup(cls): super(BaseBaremetalTest, cls).resource_setup() diff --git a/ironic_tempest_plugin/tests/scenario/baremetal_manager.py b/ironic_tempest_plugin/tests/scenario/baremetal_manager.py index 27b3c9d7..9ceecb70 100644 --- a/ironic_tempest_plugin/tests/scenario/baremetal_manager.py +++ b/ironic_tempest_plugin/tests/scenario/baremetal_manager.py @@ -82,6 +82,7 @@ class BaremetalScenarioTest(manager.ScenarioTest): super(BaremetalScenarioTest, cls).skip_checks() if not CONF.service_available.ironic: raise cls.skipException('Ironic is not enabled.') + # This is the configuration based skip test cfg_min_version = CONF.baremetal.min_microversion cfg_max_version = CONF.baremetal.max_microversion api_version_utils.check_skip_with_microversion(cls.min_microversion, @@ -96,6 +97,17 @@ class BaremetalScenarioTest(manager.ScenarioTest): client = cls.os_system_admin.baremetal.BaremetalClient() else: client = cls.os_admin.baremetal.BaremetalClient() + + # This is the automatic based upon remote version skip check, + # as it requires an API client to obtain the remote version. + if cls.min_microversion or cls.max_microversion: + api_min, api_max = client.get_min_max_api_microversions() + api_version_utils.check_skip_with_microversion( + cls.min_microversion, + cls.max_microversion, + api_min, + api_max) + cls.baremetal_client = client @classmethod