From cda96d5ca3f84a5b883fee521bbabbe1c283c2a0 Mon Sep 17 00:00:00 2001 From: Julia Kreger Date: Fri, 20 Jan 2023 08:05:41 -0800 Subject: [PATCH] Prevent protected node tests from orphaning test nodes While investigating failures where four orphaned nodes were observed after running the tempest plugin against an operating environment, it was discovered that each node was created by the TestProtectedNode class. A review of the logs indicated that tearDown() was being called, but resource_cleanup was never being called which houses the unified resource cleanup. Upon reviewing the pattern in tempest itself, generally resource_cleanup is used instead of teardown, however with our class structure, that doesn't seem to work, so instead we just explicitly call to unset the protected state for each node. We should now be consistent, and the four TestProtectedNode tests should no longer orphan four nodes when run. Also added some additional logging to help spotting cases where orphaned nodes might be occuring, in order to aid in troubleshooting when reviewing Tempest logs. Change-Id: I4dc6140f1dbcf1bb891fa522523957c1d8002df4 --- ironic_tempest_plugin/tests/api/admin/test_nodes.py | 10 +++------- ironic_tempest_plugin/tests/api/base.py | 12 +++++++++--- 2 files changed, 12 insertions(+), 10 deletions(-) diff --git a/ironic_tempest_plugin/tests/api/admin/test_nodes.py b/ironic_tempest_plugin/tests/api/admin/test_nodes.py index 6d26a26..8e8e217 100644 --- a/ironic_tempest_plugin/tests/api/admin/test_nodes.py +++ b/ironic_tempest_plugin/tests/api/admin/test_nodes.py @@ -869,13 +869,6 @@ class TestNodeProtected(base.BaseBaremetalTest): _, self.node = self.create_node(self.chassis['uuid']) self.provide_node(self.node['uuid']) - def tearDown(self): - try: - self.client.update_node(self.node['uuid'], protected=False) - except Exception: - pass - super(TestNodeProtected, self).tearDown() - @decorators.idempotent_id('52f0cb1c-ad7b-43dc-8e22-a76438b67716') def test_node_protected_set_unset(self): self.deploy_node(self.node['uuid']) @@ -905,6 +898,7 @@ class TestNodeProtected(base.BaseBaremetalTest): self.assertRaises(lib_exc.Forbidden, self.set_node_provision_state, self.node['uuid'], 'rebuild', 'active') + self.client.update_node(self.node['uuid'], protected=False) @decorators.idempotent_id('04a21b51-2991-4213-8c2f-a96cfdada802') def test_node_protected_from_deletion(self): @@ -918,6 +912,7 @@ class TestNodeProtected(base.BaseBaremetalTest): self.assertRaises(lib_exc.Forbidden, self.client.delete_node, self.node['uuid']) + self.client.update_node(self.node['uuid'], protected=False) @decorators.attr(type='negative') @decorators.idempotent_id('1c819f4c-6c1d-4150-ba4a-3b0dcb3c8694') @@ -933,6 +928,7 @@ class TestNodeProtected(base.BaseBaremetalTest): self.assertRaises(lib_exc.BadRequest, self.client.update_node, self.node['uuid'], protected_reason='reason!') + self.client.update_node(self.node['uuid'], protected=False) class TestNodesProtectedOldApi(base.BaseBaremetalTest): diff --git a/ironic_tempest_plugin/tests/api/base.py b/ironic_tempest_plugin/tests/api/base.py index c541dd3..5469579 100644 --- a/ironic_tempest_plugin/tests/api/base.py +++ b/ironic_tempest_plugin/tests/api/base.py @@ -12,6 +12,7 @@ import functools +from oslo_log import log as logging from tempest import config from tempest.lib.common import api_version_utils from tempest.lib.common.utils import data_utils @@ -22,6 +23,8 @@ from ironic_tempest_plugin.common import waiters from ironic_tempest_plugin.services.baremetal import base from ironic_tempest_plugin.tests.api.admin import api_microversion_fixture + +LOG = logging.getLogger(__name__) CONF = config.CONF @@ -124,7 +127,8 @@ class BaseBaremetalTest(api_version_utils.BaseMicroversionTest, cls.set_node_provision_state(node, 'deleted', ['available', None]) except lib_exc.BadRequest: - pass + LOG.warning('Cleanup: Failed to unprovision node: %s', + node) # Delete allocations explicitly after unprovisioning instances, but # before deleting nodes. @@ -132,13 +136,15 @@ class BaseBaremetalTest(api_version_utils.BaseMicroversionTest, try: cls.client.delete_allocation(allocation) except lib_exc.NotFound: - pass + LOG.warning('Cleanup: Failed to delete allocation: %s', + allocation) for node in cls.created_objects['node']: try: cls.client.update_node(node, instance_uuid=None) except lib_exc.TempestException: - pass + LOG.warning('Cleanup: Failed to delete node: %s', + node) for resource in RESOURCE_TYPES: uuids = cls.created_objects[resource]