openstack/sunbeam-charms
Code Issues Proposed changes

Undo __post_init__ failure tolerance

Browse Source 
In previous implementation, __post_init__ was allowed to handle failures
of type `BaseStatusExceptionError`. While this should be ok in an ideal
world, this made external libraries failing to bind to juju events.
These libraries do not necessarily provide a way to request out-of-bound
the resources they provide, therefore, we cannot allow ourselves to drop
events.

The only failing point was at the beginning of charm deployments, when
no ingress-internal is present. Remove exception and fallback to
sensible default.

Since the k8s operators are always deployed to a K8S model, fallback to
`<app name>.<model name>.svc`. This is always resolvable from within the
k8s cluster.

This is a last resort fallback at the internal address is going to
resolved this way:
- Is there an ingress relation? If yes, take that address
- Is there a binding for `identity-service`? If yes, take that address
- Fallback on that built dns resolvable name

Keystone actually knows how to handle svc endpoint updates, this makes
the charms more resilient.

Change-Id: Ie70698206d5b396ee4a57c79d9a2612dccd4ce1c
Signed-off-by: Guillaume Boutry <guillaume.boutry@canonical.com>
This commit is contained in:
Guillaume Boutry 2025-03-15 14:42:08 +01:00
parent 4bb00b66e0
commit 8e79100125
No known key found for this signature in database
GPG Key ID: 0DD77DC1796E98CD
3 changed files with 83 additions and 26 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

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

@ -636,6 +636,11 @@ class OSBaseOperatorCharmK8S(OSBaseOperatorCharm):
super().__post_init__()
self.pebble_handlers = self.get_pebble_handlers()
@property
def service_dns(self) -> str:
"""Dns name for the service."""
return f"{self.app.name}.{self.model.name}.svc"
def get_pebble_handlers(self) -> List[sunbeam_chandlers.PebbleHandler]:
"""Pebble handlers for the operator."""
return [
@ -1025,15 +1030,16 @@ class OSBaseOperatorAPICharm(OSBaseOperatorCharmK8S):
@property
def admin_url(self) -> str:
"""Url for accessing the admin endpoint for this service."""
"""Url for accessing the admin endpoint for this service.
Fallback to k8s resolvable hostname if no identity-service relation.
"""
binding = self.model.get_binding("identity-service")
if binding and binding.network and binding.network.ingress_address:
return self.add_explicit_port(
self.service_url(str(binding.network.ingress_address))
)
raise sunbeam_guard.WaitingExceptionError(
"No admin address found for service"
)
return self.add_explicit_port(self.service_url(self.service_dns))
@property
def internal_url(self) -> str:
@ -1052,9 +1058,7 @@ class OSBaseOperatorAPICharm(OSBaseOperatorCharmK8S):
return self.add_explicit_port(
self.service_url(str(binding.network.ingress_address))
)
raise sunbeam_guard.WaitingExceptionError(
"No internal address found for service"
)
return self.add_explicit_port(self.service_url(self.service_dns))
def get_pebble_handlers(self) -> list[sunbeam_chandlers.PebbleHandler]:
"""Pebble handlers for the service."""

20
ops-sunbeam/ops_sunbeam/core.py
View File

@ -25,9 +25,6 @@ from typing import (
)
import ops_sunbeam.tracing as sunbeam_tracing
from ops_sunbeam.guard import (
BaseStatusExceptionError,
)
if TYPE_CHECKING:
from ops_sunbeam.charm import (
@ -104,20 +101,5 @@ class PostInitMeta(type):
def __call__(cls, *args, **kw):
"""Call __post_init__ after __init__."""
instance = super().__call__(*args, **kw)
try:
instance.__post_init__()
except BaseStatusExceptionError as e:
# Allow post init to raise an ops_sunbeam status
# exception without causing the charm to error.
# This status will be collected and set on the
# unit.
# import here to avoid circular import
from ops_sunbeam.charm import (
OSBaseOperatorCharm,
)
if isinstance(instance, OSBaseOperatorCharm):
instance.status.set(e.to_status())
else:
raise e
instance.__post_init__()
return instance

71
ops-sunbeam/tests/unit_tests/test_core.py
View File

@ -484,6 +484,77 @@ class TestOSBaseOperatorAPICharm(_TestOSBaseOperatorAPICharm):
"https://test.org:8443/something",
)
def test_admin_url_id_svc(self):
"""Test admin_url with service ID."""
test_utils.add_complete_identity_relation(self.harness)
self.assertEqual(
self.harness.charm.admin_url,
"http://10.0.0.10:789",
)
def test_admin_url_fallback_to_service_dns(self):
"""Test admin_url fallback to service DNS."""
with patch.object(
self.harness.charm.model,
"get_binding",
MagicMock(return_value=None),
):
self.assertEqual(
self.harness.charm.admin_url,
"http://my-service.test-model.svc:789",
)
def test_internal_url_ingress_internal(self):
"""Test internal_url with internal ingress."""
test_utils.add_complete_ingress_relation(self.harness)
self.assertEqual(
self.harness.charm.internal_url,
"http://internal-url:80/",
)
def test_internal_url_fallback_to_id_svc(self):
"""Test internal_url with service ID."""
test_utils.add_complete_identity_relation(self.harness)
self.assertEqual(
self.harness.charm.internal_url,
"http://10.0.0.10:789",
)
def test_internal_url_fallback_to_service_dns(self):
"""Test internal fallback to service DNS."""
with patch.object(
self.harness.charm.model,
"get_binding",
MagicMock(return_value=None),
):
self.assertEqual(
self.harness.charm.internal_url,
"http://my-service.test-model.svc:789",
)
def test_public_url_ingress_public(self):
"""Test public_url with public ingress."""
test_utils.add_complete_ingress_relation(self.harness)
self.assertEqual(
self.harness.charm.public_url,
"http://public-url:80/",
)
def test_public_url_fallback_to_internal(self):
"""Test public_url fallback to internal."""
self.assertEqual(
self.harness.charm.public_url,
self.harness.charm.internal_url,
)
def test_public_url_attribute_error(self):
"""Test public_url with attribute error."""
del self.harness.charm.ingress_public
self.assertEqual(
self.harness.charm.public_url,
self.harness.charm.internal_url,
)
class TestOSBaseOperatorMultiSVCAPICharm(_TestOSBaseOperatorAPICharm):
"""Test Charm with multiple services."""