openstack/sunbeam-charms
Code Issues Proposed changes

[all] only publish active status at the end of the hook

Browse Source 
The status_compound by designed set the unit's status many times during
hook execution. The unit's status is directly published to the
controller. This leads to outside observers seeing active status (and
lot of chatter around statuses) when the unit is in fact not ready.

With this change, units will only publish an active status at the end of
the hook execution. All other levels are still directly published to the
controller.
Units will no longer publish the WaitingStatus("no status yet"). This
creates a lot of chatter and holds little value.

Re-organize keystone __init__ not to publish false `Service not
bootstrap` status.

Closes-Bug: #2067016
Change-Id: Ie73b95972a44833ba4509f8fd2c2f52ed476004d
This commit is contained in:
Guillaume Boutry 2024-06-05 19:14:44 +02:00
parent 573a8e56c2
commit 52377e84cf
No known key found for this signature in database
GPG Key ID: E95E3326872E55DE
6 changed files with 59 additions and 27 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
charms/keystone-k8s/src/charm.py
View File

@ -70,7 +70,6 @@ from ops.main import (
main,
)
from ops.model import (
ActiveStatus,
MaintenanceStatus,
ModelError,
Relation,
@ -342,10 +341,12 @@ class KeystoneOperatorCharm(sunbeam_charm.OSBaseOperatorAPICharm):
SEND_CA_CERT_RELATION_NAME = "send-ca-cert"
def __init__(self, framework):
super().__init__(framework)
# NOTE(gboutry): super().__init__ will call self.bootstrapped() which tries to
# make use of the keystone_manager
self.keystone_manager = manager.KeystoneManager(
self, KEYSTONE_CONTAINER
)
super().__init__(framework)
self._state.set_default(admin_domain_name="admin_domain")
self._state.set_default(admin_domain_id=None)
self._state.set_default(default_domain_id=None)
@ -388,8 +389,6 @@ class KeystoneOperatorCharm(sunbeam_charm.OSBaseOperatorAPICharm):
self.on.list_ca_certs_action,
self._list_ca_certs_action,
)
if self.bootstrapped():
self.bootstrap_status.set(ActiveStatus())
def _retrieve_or_set_secret(
self,

15
charms/keystone-k8s/src/utils/manager.py
View File

@ -21,6 +21,7 @@ from typing import (
)
import ops.pebble
import ops_sunbeam.charm as sunbeam_charm
import ops_sunbeam.guard as sunbeam_guard
from keystoneauth1 import (
session,
@ -31,9 +32,6 @@ from keystoneauth1.identity import (
from keystoneclient.v3 import (
client,
)
from ops import (
framework,
)
from ops.model import (
MaintenanceStatus,
)
@ -45,12 +43,15 @@ from utils.client import (
logger = logging.getLogger(__name__)
class KeystoneManager(framework.Object):
class KeystoneManager:
"""Class for managing interactions with keystone api."""
def __init__(self, charm, container_name):
def __init__(
self,
charm: sunbeam_charm.OSBaseOperatorCharmK8S,
container_name: str,
):
"""Setup the manager."""
super().__init__(charm, "keystone-manager")
self.charm = charm
self.container_name = container_name
self._api = None
@ -61,7 +62,7 @@ class KeystoneManager(framework.Object):
pebble_handler = self.charm.get_named_pebble_handler(
self.container_name
)
return pebble_handler.execute(cmd, exception_on_error=True, **kwargs)
return pebble_handler.execute(cmd, exception_on_error, **kwargs)
@property
def api(self):

1
charms/sunbeam-clusterd/tests/unit/test_charm.py
View File

@ -65,6 +65,7 @@ class TestCharm(test_utils.CharmTestCase):
self.harness.set_leader()
self.harness.charm.on.config_changed.emit()
self.harness.evaluate_status()
self.assertEqual(self.harness.charm.unit.status, ops.ActiveStatus())
self.ensure_snap_present.assert_called()
self.harness.charm._clusterd.bootstrap.assert_called_once()

13
ops-sunbeam/ops_sunbeam/charm.py
View File

@ -109,6 +109,9 @@ class OSBaseOperatorCharm(ops.charm.CharmBase):
self.framework.observe(self.on.secret_changed, self._on_secret_changed)
self.framework.observe(self.on.secret_rotate, self._on_secret_rotate)
self.framework.observe(self.on.secret_remove, self._on_secret_remove)
self.framework.observe(
self.on.collect_unit_status, self._on_collect_unit_status_event
)
def can_add_handler(
self,
@ -345,6 +348,16 @@ class OSBaseOperatorCharm(ops.charm.CharmBase):
"""
return {"database": self.service_name.replace("-", "_")}
def _on_collect_unit_status_event(self, event: ops.CollectStatusEvent):
"""Publish the unit status.
collect_unit_status is called at the end of the hook's execution,
making it the best place to publish an active status.
"""
status = self.status_pool.compute_status()
if status:
event.add_status(status)
def _on_config_changed(self, event: ops.framework.EventBase) -> None:
self.configure_charm(event)

47
ops-sunbeam/ops_sunbeam/compound_status.py
View File

@ -44,7 +44,6 @@ from ops.model import (
ActiveStatus,
StatusBase,
UnknownStatus,
WaitingStatus,
)
from ops.storage import (
NoSnapshotError,
@ -222,10 +221,10 @@ class StatusPool(Object):
self._charm.framework.save_snapshot(self._state)
self._charm.framework._storage.commit()
def on_update(self) -> None:
"""Update the unit status with the current highest priority status.
def compute_status(self) -> StatusBase | None:
"""Compute the status to show to the user.
Use as a hook to run whenever a status is updated in the pool.
This is the highest priority status in the pool.
"""
status = (
sorted(self._pool.values(), key=lambda x: x.priority())[0]
@ -233,18 +232,34 @@ class StatusPool(Object):
else None
)
if status is None or status.status.name == "unknown":
self._charm.unit.status = WaitingStatus("no status set yet")
elif status.status.name == "active" and not status.message():
return None
if status.status.name == "active" and not status.message():
# Avoid status name prefix if everything is active with no message.
# If there's a message, then we want the prefix
# to help identify where the message originates.
self._charm.unit.status = ActiveStatus("")
else:
message = status.message()
self._charm.unit.status = StatusBase.from_name(
status.status.name,
"({}){}".format(
status.label,
" " + message if message else "",
),
)
return ActiveStatus("")
return StatusBase.from_name(
status.status.name,
"({}){}".format(
status.label,
" " + status.message() if status.message() else "",
),
)
def on_update(self) -> None:
"""Update the unit status with the current highest priority status.
Use as a hook to run whenever a status is updated in the pool.
on_update will never update the unit status to active, because this is
synced directly to the controller. Making the unit pass to active
multiple times during a hook while it's not.
It is the charm's responsibility to set the unit status to active,
collect_unit_status is the best place to set a status at end of
the hook.
"""
status = self.compute_status()
if not status:
return
if status.name != "active":
self._charm.unit.status = status

3
ops-sunbeam/tests/unit_tests/test_compound_status.py
View File

@ -178,12 +178,15 @@ class TestCompoundStatus(test_utils.CharmTestCase):
# also need to manually activate other default statuses
pool._pool["container:my-service"].set(ActiveStatus(""))
self.harness.evaluate_status()
# all empty messages should end up as an empty unit status
self.assertEqual(self.harness.charm.unit.status, ActiveStatus(""))
# if there's a message (on the highest priority status),
# it should also show the status prefix
status2.set(ActiveStatus("a message"))
self.harness.evaluate_status()
self.assertEqual(
self.harness.charm.unit.status, ActiveStatus("(test2) a message")
)