openstack/sunbeam-charms
Code Issues Proposed changes

Improve cleanup on keystone relation broken

Browse Source 
This fixes some issues observed in various situations involving
keystone relation removing and rejoining.

- unset external_gateway_info from routers before removing ports
  (fix issue with failing to remove routers, ports, and networks
  if an external network is attached to the routers)
- check credentials are available before using them in cleanup
- delete the credentials secret on keystone relation broken
- ensure tempest will be re-inited on rejoin keystone relation
- check if keystone is ready as well when setting the schedule
- tidy and remove some unnecessary or dead code

Change-Id: Id3cd9fb26cb7c0d661f11393d58508e50d481df1
This commit is contained in:
Samuel Allan 2024-02-27 14:23:04 +10:30
parent 979c11cf2e
commit 0db2a45a44
No known key found for this signature in database
GPG Key ID: 622F8E99C893BD61
4 changed files with 116 additions and 93 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

10
charms/tempest-k8s/src/charm.py
View File

@ -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

182
charms/tempest-k8s/src/handlers.py
View File

@ -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)

5
charms/tempest-k8s/src/utils/cleanup.py
View File

@ -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

12
charms/tempest-k8s/tests/unit/test_tempest_charm.py
View File

@ -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)