diff --git a/charms/tempest-k8s/src/charm.py b/charms/tempest-k8s/src/charm.py index 87af0942..3be980cb 100755 --- a/charms/tempest-k8s/src/charm.py +++ b/charms/tempest-k8s/src/charm.py @@ -142,9 +142,15 @@ class TempestOperatorCharm(sunbeam_charm.OSBaseOperatorCharmK8S): if not schedule.valid: return "" - # if tempest env isn't ready, or if the logging relation isn't joined + # if tempest env isn't ready, + # or if the logging relation isn't joined, + # or if keystone isn't ready, # then we can't start scheduling periodic tests - if not (self.is_tempest_ready() and self.loki.ready): + if not ( + self.is_tempest_ready() + and self.loki.ready + and self.user_id_ops.ready + ): return "" return schedule.value diff --git a/charms/tempest-k8s/src/handlers.py b/charms/tempest-k8s/src/handlers.py index e3a71dfc..461763fd 100644 --- a/charms/tempest-k8s/src/handlers.py +++ b/charms/tempest-k8s/src/handlers.py @@ -280,6 +280,49 @@ class TempestUserIdentityRelationHandler(sunbeam_rhandlers.RelationHandler): CREDENTIALS_SECRET_PREFIX = "tempest-user-identity-resource-" CONFIGURE_SECRET_PREFIX = "configure-credential-" + teardown_ops = [ + { + "name": "show_domain", + "params": { + "name": OPENSTACK_DOMAIN, + }, + }, + { + "name": "delete_project", + "params": { + "name": OPENSTACK_PROJECT, + "domain": "{{ show_domain[0].id }}", + }, + }, + { + "name": "delete_user", + "params": { + "name": OPENSTACK_USER, + "domain": "{{ show_domain[0].id }}", + }, + }, + { + "name": "update_domain", + "params": { + "domain": "{{ show_domain[0].id }}", + "enable": False, + }, + }, + { + "name": "delete_domain", + "params": { + "name": "{{ show_domain[0].id }}", + }, + }, + ] + + list_endpoint_ops = [ + { + "name": "list_endpoint", + "params": {"name": "keystone", "interface": "admin"}, + }, + ] + resource_identifiers: FrozenSet[str] = frozenset( { "name", @@ -301,10 +344,14 @@ class TempestUserIdentityRelationHandler(sunbeam_rhandlers.RelationHandler): @property def ready(self) -> bool: """Whether the relation is ready.""" + # We define that keystone relation is ready, + # once we have all the responses to ops requests, + # and the details have been stored + # in the credentials secret maintained by the charm. content = self.get_user_credential() - if content and content.get("auth-url") is not None: - return True - return False + return bool( + content and content.get("auth-url") and content.get("domain-id") + ) @property def label(self) -> str: @@ -390,6 +437,21 @@ class TempestUserIdentityRelationHandler(sunbeam_rhandlers.RelationHandler): self.charm.leader_set({self.label: credential_secret.id}) return credential_secret.id + def _delete_secret(self): + """Delete the credentials secret if exists. + + Is a no-op if the charm is not leader, + because non-leader units cannot set application data. + """ + if not self.model.unit.is_leader(): + return + + credential_id = self.charm.leader_get(self.label) + if credential_id: + secret = self.model.get_secret(id=credential_id) + secret.remove_all_revisions() + self.charm.leader_set({self.label: ""}) + def _generate_password(self, length: int) -> str: """Utility function to generate secure random string for password.""" alphabet = string.ascii_letters + string.digits @@ -458,65 +520,13 @@ class TempestUserIdentityRelationHandler(sunbeam_rhandlers.RelationHandler): ] return setup_ops - def _list_endpoint_ops(self) -> List[dict]: - """List endpoint ops.""" - list_endpoint_ops = [ - { - "name": "list_endpoint", - "params": {"name": "keystone", "interface": "admin"}, - }, - ] - return list_endpoint_ops - - def _teardown_tempest_resource_ops(self) -> List[dict]: - """Tear down openstack resource ops.""" - credential_id = self._ensure_credential() - credential_secret = self.model.get_secret(id=credential_id) - content = credential_secret.get_content(refresh=True) - username = content.get("username") - teardown_ops = [ - { - "name": "show_domain", - "params": { - "name": OPENSTACK_DOMAIN, - }, - }, - { - "name": "delete_project", - "params": { - "name": OPENSTACK_PROJECT, - "domain": "{{ show_domain[0].id }}", - }, - }, - { - "name": "delete_user", - "params": { - "name": username, - "domain": "{{ show_domain[0].id }}", - }, - }, - { - "name": "update_domain", - "params": { - "domain": "{{ show_domain[0].id }}", - "enable": False, - }, - }, - { - "name": "delete_domain", - "params": { - "name": "{{ show_domain[0].id }}", - }, - }, - ] - return teardown_ops - def _setup_tempest_resource_request(self) -> dict: """Set up openstack resource for tempest.""" ops = [] - ops.extend(self._teardown_tempest_resource_ops()) + # Teardown before setup to ensure it begins with a clean environment. + ops.extend(self.teardown_ops) ops.extend(self._setup_tempest_resource_ops()) - ops.extend(self._list_endpoint_ops()) + ops.extend(self.list_endpoint_ops) request = { "id": self._hash_ops(ops), "tag": "setup_tempest_resource", @@ -524,17 +534,6 @@ class TempestUserIdentityRelationHandler(sunbeam_rhandlers.RelationHandler): } return request - def _teardown_tempest_resource_request(self) -> dict: - """Tear down openstack resources for tempest.""" - ops = [] - ops.extend(self._teardown_tempest_resource_ops()) - request = { - "id": self._hash_ops(ops), - "tag": "teardown_tempest_resource", - "ops": ops, - } - return request - def _process_list_endpoint_response(self, response: dict) -> None: """Process extra ops request: `_list_endpoint_ops`.""" for op in response.get("ops", []): @@ -569,6 +568,11 @@ class TempestUserIdentityRelationHandler(sunbeam_rhandlers.RelationHandler): logger.info("Identity ops provider ready: setup tempest resources") self.interface.request_ops(self._setup_tempest_resource_request()) self._grant_ops_secret(event.relation) + + # Mark tempest as not ready, + # so that the tempest environment is definitely re-inited on rejoin. + self.charm.set_tempest_ready(False) + self.callback_f(event) def _on_response_available(self, event) -> None: @@ -585,30 +589,38 @@ class TempestUserIdentityRelationHandler(sunbeam_rhandlers.RelationHandler): def _on_provider_goneaway(self, event) -> None: """Handle gone_away event.""" + # If it's not the leader, skip these steps, + # because the cleanup should only happen from a single leader unit. + # Either way, multiple units are not supported or tested currently. if not self.model.unit.is_leader(): return logger.info("Identity ops provider gone away") - credential = self.get_user_credential() - env = { - "OS_AUTH_URL": credential.get("auth-url"), - "OS_USERNAME": credential.get("username"), - "OS_PASSWORD": credential.get("password"), - "OS_PROJECT_NAME": credential.get("project-name"), - "OS_DOMAIN_ID": credential.get("domain-id"), - "OS_USER_DOMAIN_ID": credential.get("domain-id"), - "OS_PROJECT_DOMAIN_ID": credential.get("domain-id"), - } - try: - # do an extensive clean-up upon identity relation removal - run_extensive_cleanup(env) - - except CleanUpError as e: - logger.warning("Clean-up failed: %s", str(e)) # If the relation is going away, then tempest is no longer ready, # and the environment should be inited again if rejoined. self.charm.set_tempest_ready(False) + credential = self.get_user_credential() + if credential and credential.get("auth-url"): + env = { + "OS_AUTH_URL": credential.get("auth-url"), + "OS_USERNAME": credential.get("username"), + "OS_PASSWORD": credential.get("password"), + "OS_PROJECT_NAME": credential.get("project-name"), + "OS_DOMAIN_ID": credential.get("domain-id"), + "OS_USER_DOMAIN_ID": credential.get("domain-id"), + "OS_PROJECT_DOMAIN_ID": credential.get("domain-id"), + } + try: + # do an extensive clean-up upon identity relation removal + run_extensive_cleanup(env) + except CleanUpError as e: + logger.warning("Clean-up failed: %s", str(e)) + + # Delete the stored keystone credentials, + # because they are no longer valid. + self._delete_secret() + self.callback_f(event) diff --git a/charms/tempest-k8s/src/utils/cleanup.py b/charms/tempest-k8s/src/utils/cleanup.py index 597b9c39..6528bd7c 100644 --- a/charms/tempest-k8s/src/utils/cleanup.py +++ b/charms/tempest-k8s/src/utils/cleanup.py @@ -143,6 +143,11 @@ def _cleanup_networks_resources(conn: Connection, project_id: str) -> None: # Delete ports and routers for router in conn.network.routers(project_id=project_id): if router.name.startswith(RESOURCE_PREFIX): + # Ports attached via the external gateway info + # cannot be removed/deleted via the ports api, + # so external_gateway_info must be unset before removing other ports. + if router.external_gateway_info: + conn.network.update_router(router, external_gateway_info=None) for port in conn.network.ports(device_id=router.id): conn.network.remove_interface_from_router( router, port_id=port.id diff --git a/charms/tempest-k8s/tests/unit/test_tempest_charm.py b/charms/tempest-k8s/tests/unit/test_tempest_charm.py index 10b64ca8..79c1134a 100644 --- a/charms/tempest-k8s/tests/unit/test_tempest_charm.py +++ b/charms/tempest-k8s/tests/unit/test_tempest_charm.py @@ -21,6 +21,7 @@ import pathlib from unittest.mock import ( MagicMock, Mock, + call, patch, ) @@ -122,6 +123,7 @@ class TestTempestOperatorCharm(test_utils.CharmTestCase): def add_identity_ops_relation(self, harness): """Add identity resource relation.""" + self.harness.charm.set_tempest_ready = Mock() rel_id = harness.add_relation("identity-ops", "keystone") harness.add_relation_unit(rel_id, "keystone/0") harness.charm.user_id_ops.callback_f = Mock() @@ -202,7 +204,6 @@ class TestTempestOperatorCharm(test_utils.CharmTestCase): self.harness ) - self.harness.charm.set_tempest_ready = Mock() self.harness.charm.is_tempest_ready = Mock(return_value=True) self.harness.update_config({"schedule": "0 0 */7 * *"}) @@ -485,12 +486,14 @@ class TestTempestOperatorCharm(test_utils.CharmTestCase): mock_pebble = Mock() mock_pebble.init_tempest = Mock(side_effect=RuntimeError) self.harness.charm.pebble_handler = Mock(return_value=mock_pebble) - self.harness.charm.set_tempest_ready = Mock() self.harness.charm.is_tempest_ready = Mock(return_value=False) self.harness.update_config({"schedule": "*/21 * * * *"}) - self.harness.charm.set_tempest_ready.assert_called_once_with(False) + self.harness.charm.set_tempest_ready.has_calls( + [call(False), call(False)] + ) + self.assertEqual(self.harness.charm.set_tempest_ready.call_count, 2) self.assertIn( "tempest init failed", self.harness.charm.status.message() ) @@ -535,9 +538,6 @@ class TestTempestOperatorCharm(test_utils.CharmTestCase): def test_set_tempest_ready(self): """Test the tempest ready set method.""" test_utils.set_all_pebbles_ready(self.harness) - self.add_logging_relation(self.harness) - self.add_identity_ops_relation(self.harness) - self.add_grafana_dashboard_relation(self.harness) self.harness.charm.peers = Mock() self.harness.charm.set_tempest_ready(True)