From 1f30344dbfd5e838dd96c15b37979917df1a4767 Mon Sep 17 00:00:00 2001 From: Myles Penner Date: Thu, 27 Feb 2025 09:05:05 -0800 Subject: [PATCH] Add ingress healthcheck params to charms This change adds a healthcheck params dict to k8s charms for use by traefik via the ingress relation. It contains a path, interval, and timeout value. This allows traefik to detect down nodes and remove them from the loadbalancer rotation. Unless overridden in the charm, a default path of "/" is passed in the ingress relation. Interval and timeout are optional and will use default values of 30s and 5s, respectively, unless overridden in the charm. Some charms define a "/healthcheck" path in api-paste.ini which has been used in place of the default "/" path. Closes-Bug: #2077269 Change-Id: I355728f338e9a29fcf202cc629a977b49b2d8990 --- charms/aodh-k8s/src/charm.py | 5 + charms/barbican-k8s/src/charm.py | 5 + charms/cinder-k8s/src/charm.py | 5 + charms/designate-k8s/src/charm.py | 5 + charms/glance-k8s/src/charm.py | 5 + charms/gnocchi-k8s/src/charm.py | 5 + charms/keystone-k8s/src/charm.py | 7 +- charms/magnum-k8s/src/charm.py | 5 + charms/neutron-k8s/src/charm.py | 5 + .../lib/charms/traefik_k8s/v2/ingress.py | 122 ++++++++++++++---- ops-sunbeam/ops_sunbeam/charm.py | 40 ++++++ ops-sunbeam/ops_sunbeam/relation_handlers.py | 5 + 12 files changed, 189 insertions(+), 25 deletions(-) diff --git a/charms/aodh-k8s/src/charm.py b/charms/aodh-k8s/src/charm.py index 72ef75b6..9757ff22 100755 --- a/charms/aodh-k8s/src/charm.py +++ b/charms/aodh-k8s/src/charm.py @@ -283,6 +283,11 @@ class AodhOperatorCharm(sunbeam_charm.OSBaseOperatorAPICharm): """Ingress Port for API service.""" return 8042 + @property + def ingress_healthcheck_path(self): + """Healthcheck path for ingress relation.""" + return "/healthcheck" + def get_pebble_handlers( self, ) -> List[sunbeam_chandlers.ServicePebbleHandler]: diff --git a/charms/barbican-k8s/src/charm.py b/charms/barbican-k8s/src/charm.py index 52a2047b..fd6acaa7 100755 --- a/charms/barbican-k8s/src/charm.py +++ b/charms/barbican-k8s/src/charm.py @@ -418,6 +418,11 @@ class BarbicanOperatorCharm(sunbeam_charm.OSBaseOperatorAPICharm): """Default port.""" return 9311 + @property + def ingress_healthcheck_path(self): + """Healthcheck path for ingress relation.""" + return "/healthcheck" + @property def healthcheck_http_url(self) -> str: """Healthcheck HTTP URL for the service.""" diff --git a/charms/cinder-k8s/src/charm.py b/charms/cinder-k8s/src/charm.py index a514c562..7bd7897f 100755 --- a/charms/cinder-k8s/src/charm.py +++ b/charms/cinder-k8s/src/charm.py @@ -298,6 +298,11 @@ class CinderOperatorCharm(sunbeam_charm.OSBaseOperatorAPICharm): """Public ingress port for service.""" return 8776 + @property + def ingress_healthcheck_path(self): + """Healthcheck path for ingress relation.""" + return "/healthcheck" + @property def service_conf(self) -> str: """Service default configuration file.""" diff --git a/charms/designate-k8s/src/charm.py b/charms/designate-k8s/src/charm.py index 1bdf6868..1d50d9c7 100755 --- a/charms/designate-k8s/src/charm.py +++ b/charms/designate-k8s/src/charm.py @@ -455,6 +455,11 @@ class DesignateOperatorCharm(sunbeam_charm.OSBaseOperatorAPICharm): """Ingress Port for API service.""" return 9001 + @property + def ingress_healthcheck_path(self): + """Healthcheck path for ingress relation.""" + return "/healthcheck" + @property def ns_records(self) -> List[str]: """Get nameserver records.""" diff --git a/charms/glance-k8s/src/charm.py b/charms/glance-k8s/src/charm.py index 2f32df4d..69049ff0 100755 --- a/charms/glance-k8s/src/charm.py +++ b/charms/glance-k8s/src/charm.py @@ -422,6 +422,11 @@ class GlanceOperatorCharm(sunbeam_charm.OSBaseOperatorAPICharm): """Default ingress port.""" return 9292 + @property + def ingress_healthcheck_path(self): + """Healthcheck path for ingress relation.""" + return "/healthcheck" + @property def healthcheck_http_url(self) -> str: """Healthcheck HTTP URL for the service.""" diff --git a/charms/gnocchi-k8s/src/charm.py b/charms/gnocchi-k8s/src/charm.py index cbc89d8e..a32da590 100755 --- a/charms/gnocchi-k8s/src/charm.py +++ b/charms/gnocchi-k8s/src/charm.py @@ -196,6 +196,11 @@ class GnocchiOperatorCharm(sunbeam_charm.OSBaseOperatorAPICharm): """Ingress Port for API service.""" return 8041 + @property + def ingress_healthcheck_path(self): + """Healthcheck path for ingress relation.""" + return "/healthcheck" + @property def healthcheck_http_url(self) -> str: """Healthcheck HTTP URL for the service.""" diff --git a/charms/keystone-k8s/src/charm.py b/charms/keystone-k8s/src/charm.py index 5af5572f..27c6df48 100755 --- a/charms/keystone-k8s/src/charm.py +++ b/charms/keystone-k8s/src/charm.py @@ -1440,7 +1440,12 @@ export OS_AUTH_VERSION=3 @property def healthcheck_http_url(self) -> str: """Healthcheck HTTP URL for the service.""" - return f"http://localhost:{self.default_public_ingress_port}/v3" + return f"http://localhost:{self.default_public_ingress_port}/{self.ingress_healthcheck_path}" + + @property + def ingress_healthcheck_path(self): + """Healthcheck path for ingress relation.""" + return "/healthcheck" def _create_fernet_secret(self) -> None: """Create fernet juju secret. diff --git a/charms/magnum-k8s/src/charm.py b/charms/magnum-k8s/src/charm.py index be0b8459..001fb496 100755 --- a/charms/magnum-k8s/src/charm.py +++ b/charms/magnum-k8s/src/charm.py @@ -184,6 +184,11 @@ class MagnumOperatorCharm(sunbeam_charm.OSBaseOperatorAPICharm): """Default public ingress port.""" return 9511 + @property + def ingress_healthcheck_path(self): + """Healthcheck path for ingress relation.""" + return "/healthcheck" + @property def config_contexts(self) -> List[sunbeam_config_contexts.ConfigContext]: """Generate list of configuration adapters for the charm.""" diff --git a/charms/neutron-k8s/src/charm.py b/charms/neutron-k8s/src/charm.py index 9b111d89..3c468da3 100755 --- a/charms/neutron-k8s/src/charm.py +++ b/charms/neutron-k8s/src/charm.py @@ -302,6 +302,11 @@ class NeutronOperatorCharm(sunbeam_charm.OSBaseOperatorAPICharm): """Public ingress port.""" return 9696 + @property + def ingress_healthcheck_path(self): + """Healthcheck path for ingress relation.""" + return "/healthcheck" + @property def service_user(self) -> str: """Service user file and directory ownership.""" diff --git a/libs/external/lib/charms/traefik_k8s/v2/ingress.py b/libs/external/lib/charms/traefik_k8s/v2/ingress.py index ed464eac..b30b857a 100644 --- a/libs/external/lib/charms/traefik_k8s/v2/ingress.py +++ b/libs/external/lib/charms/traefik_k8s/v2/ingress.py @@ -56,13 +56,25 @@ import logging import socket import typing from dataclasses import dataclass -from typing import Any, Callable, Dict, List, MutableMapping, Optional, Sequence, Tuple, Union +from functools import partial +from typing import ( + Any, + Callable, + Dict, + List, + MutableMapping, + Optional, + Sequence, + Tuple, + Union, + cast, +) import pydantic from ops.charm import CharmBase, RelationBrokenEvent, RelationEvent from ops.framework import EventSource, Object, ObjectEvents, StoredState from ops.model import ModelError, Relation, Unit -from pydantic import AnyHttpUrl, BaseModel, Field, validator +from pydantic import AnyHttpUrl, BaseModel, Field # The unique Charmhub library identifier, never change it LIBID = "e6de2a5cd5b34422a204668f3b8f90d2" @@ -72,7 +84,7 @@ LIBAPI = 2 # Increment this PATCH version before using `charmcraft publish-lib` or reset # to 0 if you are raising the major API version -LIBPATCH = 12 +LIBPATCH = 15 PYDEPS = ["pydantic"] @@ -84,6 +96,9 @@ BUILTIN_JUJU_KEYS = {"ingress-address", "private-address", "egress-subnets"} PYDANTIC_IS_V1 = int(pydantic.version.VERSION.split(".")[0]) < 2 if PYDANTIC_IS_V1: + from pydantic import validator + + input_validator = partial(validator, pre=True) class DatabagModel(BaseModel): # type: ignore """Base databag model.""" @@ -143,7 +158,9 @@ if PYDANTIC_IS_V1: return databag else: - from pydantic import ConfigDict + from pydantic import ConfigDict, field_validator + + input_validator = partial(field_validator, mode="before") class DatabagModel(BaseModel): """Base databag model.""" @@ -171,7 +188,7 @@ else: k: json.loads(v) for k, v in databag.items() # Don't attempt to parse model-external values - if k in {(f.alias or n) for n, f in cls.__fields__.items()} # type: ignore + if k in {(f.alias or n) for n, f in cls.model_fields.items()} # type: ignore } except json.JSONDecodeError as e: msg = f"invalid databag contents: expecting json. {databag}" @@ -220,7 +237,7 @@ class IngressUrl(BaseModel): class IngressProviderAppData(DatabagModel): """Ingress application databag schema.""" - ingress: IngressUrl + ingress: Optional[IngressUrl] = None class ProviderSchema(BaseModel): @@ -229,12 +246,32 @@ class ProviderSchema(BaseModel): app: IngressProviderAppData +class IngressHealthCheck(BaseModel): + """HealthCheck schema for Ingress.""" + + path: str = Field(description="The health check endpoint path (required).") + scheme: Optional[str] = Field( + default=None, description="Replaces the server URL scheme for the health check endpoint." + ) + hostname: Optional[str] = Field( + default=None, description="Hostname to be set in the health check request." + ) + port: Optional[int] = Field( + default=None, description="Replaces the server URL port for the health check endpoint." + ) + interval: str = Field(default="30s", description="Frequency of the health check calls.") + timeout: str = Field(default="5s", description="Maximum duration for a health check request.") + + class IngressRequirerAppData(DatabagModel): """Ingress requirer application databag model.""" model: str = Field(description="The model the application is in.") name: str = Field(description="the name of the app requesting ingress.") port: int = Field(description="The port the app wishes to be exposed.") + healthcheck_params: Optional[IngressHealthCheck] = Field( + default=None, description="Optional health check configuration for ingress." + ) # fields on top of vanilla 'ingress' interface: strip_prefix: Optional[bool] = Field( @@ -252,14 +289,14 @@ class IngressRequirerAppData(DatabagModel): default="http", description="What scheme to use in the generated ingress url" ) - @validator("scheme", pre=True) + @input_validator("scheme") def validate_scheme(cls, scheme): # noqa: N805 # pydantic wants 'cls' as first arg """Validate scheme arg.""" if scheme not in {"http", "https", "h2c"}: raise ValueError("invalid scheme: should be one of `http|https|h2c`") return scheme - @validator("port", pre=True) + @input_validator("port") def validate_port(cls, port): # noqa: N805 # pydantic wants 'cls' as first arg """Validate port.""" assert isinstance(port, int), type(port) @@ -277,13 +314,13 @@ class IngressRequirerUnitData(DatabagModel): "IP can only be None if the IP information can't be retrieved from juju.", ) - @validator("host", pre=True) + @input_validator("host") def validate_host(cls, host): # noqa: N805 # pydantic wants 'cls' as first arg """Validate host.""" assert isinstance(host, str), type(host) return host - @validator("ip", pre=True) + @input_validator("ip") def validate_ip(cls, ip): # noqa: N805 # pydantic wants 'cls' as first arg """Validate ip.""" if ip is None: @@ -462,7 +499,10 @@ class IngressPerAppProvider(_IngressPerAppBase): event.relation, data.app.name, data.app.model, - [unit.dict() for unit in data.units], + [ + unit.dict() if PYDANTIC_IS_V1 else unit.model_dump(mode="json") + for unit in data.units + ], data.app.strip_prefix or False, data.app.redirect_https or False, ) @@ -549,7 +589,16 @@ class IngressPerAppProvider(_IngressPerAppBase): def publish_url(self, relation: Relation, url: str): """Publish to the app databag the ingress url.""" ingress_url = {"url": url} - IngressProviderAppData(ingress=ingress_url).dump(relation.data[self.app]) # type: ignore + try: + IngressProviderAppData(ingress=ingress_url).dump(relation.data[self.app]) # type: ignore + except pydantic.ValidationError as e: + # If we cannot validate the url as valid, publish an empty databag and log the error. + log.error(f"Failed to validate ingress url '{url}' - got ValidationError {e}") + log.error( + "url was not published to ingress relation for {relation.app}. This error is likely due to an" + " error or misconfiguration of the charm calling this library." + ) + IngressProviderAppData(ingress=None).dump(relation.data[self.app]) # type: ignore @property def proxied_endpoints(self) -> Dict[str, Dict[str, str]]: @@ -587,10 +636,14 @@ class IngressPerAppProvider(_IngressPerAppBase): if not ingress_data: log.warning(f"relation {ingress_relation} not ready yet: try again in some time.") continue + + # Validation above means ingress cannot be None, but type checker doesn't know that. + ingress = ingress_data.ingress + ingress = cast(IngressProviderAppData, ingress) if PYDANTIC_IS_V1: - results[ingress_relation.app.name] = ingress_data.ingress.dict() + results[ingress_relation.app.name] = ingress.dict() else: - results[ingress_relation.app.name] = ingress_data.ingress.model_dump(mode=json) # type: ignore + results[ingress_relation.app.name] = ingress.model_dump(mode="json") return results @@ -635,6 +688,7 @@ class IngressPerAppRequirer(_IngressPerAppBase): # fixme: this is horrible UX. # shall we switch to manually calling provide_ingress_requirements with all args when ready? scheme: Union[Callable[[], str], str] = lambda: "http", + healthcheck_params: Optional[Dict[str, Any]] = None, ): """Constructor for IngressRequirer. @@ -644,23 +698,34 @@ class IngressPerAppRequirer(_IngressPerAppBase): All request args must be given as keyword args. Args: - charm: the charm that is instantiating the library. - relation_name: the name of the relation endpoint to bind to (defaults to `ingress`); - relation must be of interface type `ingress` and have "limit: 1") + charm: The charm that is instantiating the library. + relation_name: The name of the relation endpoint to bind to (defaults to "ingress"); + the relation must be of interface type "ingress" and have a limit of 1. host: Hostname to be used by the ingress provider to address the requiring application; if unspecified, the default Kubernetes service name will be used. ip: Alternative addressing method other than host to be used by the ingress provider; - if unspecified, binding address from juju network API will be used. - strip_prefix: configure Traefik to strip the path prefix. - redirect_https: redirect incoming requests to HTTPS. - scheme: callable returning the scheme to use when constructing the ingress url. - Or a string, if the scheme is known and stable at charm-init-time. + if unspecified, the binding address from the Juju network API will be used. + healthcheck_params: Optional dictionary containing health check + configuration parameters conforming to the IngressHealthCheck schema. The dictionary must include: + - "path" (str): The health check endpoint path (required). + It may also include: + - "scheme" (Optional[str]): Replaces the server URL scheme for the health check endpoint. + - "hostname" (Optional[str]): Hostname to be set in the health check request. + - "port" (Optional[int]): Replaces the server URL port for the health check endpoint. + - "interval" (str): Frequency of the health check calls (defaults to "30s" if omitted). + - "timeout" (str): Maximum duration for a health check request (defaults to "5s" if omitted). + If provided, "path" is required while "interval" and "timeout" will use Traefik's defaults when not specified. + strip_prefix: Configure Traefik to strip the path prefix. + redirect_https: Redirect incoming requests to HTTPS. + scheme: Either a callable that returns the scheme to use when constructing the ingress URL, + or a string if the scheme is known and stable at charm initialization. Request Args: port: the port of the service """ super().__init__(charm, relation_name) self.charm: CharmBase = charm + self.healthcheck_params = healthcheck_params self.relation_name = relation_name self._strip_prefix = strip_prefix self._redirect_https = redirect_https @@ -792,6 +857,11 @@ class IngressPerAppRequirer(_IngressPerAppBase): port=port, strip_prefix=self._strip_prefix, # type: ignore # pyright does not like aliases redirect_https=self._redirect_https, # type: ignore # pyright does not like aliases + healthcheck_params=( + IngressHealthCheck(**self.healthcheck_params) + if self.healthcheck_params + else None + ), ).dump(app_databag) except pydantic.ValidationError as e: msg = "failed to validate app data" @@ -825,7 +895,11 @@ class IngressPerAppRequirer(_IngressPerAppBase): if not databag: # not ready yet return None - return str(IngressProviderAppData.load(databag).ingress.url) + ingress = IngressProviderAppData.load(databag).ingress + if ingress is None: + return None + + return str(ingress.url) @property def url(self) -> Optional[str]: @@ -837,4 +911,4 @@ class IngressPerAppRequirer(_IngressPerAppBase): typing.cast(Optional[str], self._stored.current_url) # type: ignore or self._get_url_from_relation_data() ) - return data + return data \ No newline at end of file diff --git a/ops-sunbeam/ops_sunbeam/charm.py b/ops-sunbeam/ops_sunbeam/charm.py index b29ad79a..d6827f9b 100644 --- a/ops-sunbeam/ops_sunbeam/charm.py +++ b/ops-sunbeam/ops_sunbeam/charm.py @@ -865,6 +865,44 @@ class OSBaseOperatorAPICharm(OSBaseOperatorCharmK8S): """List of endpoints for this service.""" return [] + @property + def ingress_healthcheck_path(self): + """Default ingress healthcheck path. + + This value can be overridden at the charm level as shown in + keystone-k8s/src/charm.py. + """ + return "/" + + @property + def ingress_healthcheck_interval(self): + """Default ingress healthcheck interval. + + This value can be overridden at the charm level. Time values + following Golang time.ParseDuration() format are valid. + """ + return "30s" + + @property + def ingress_healthcheck_timeout(self): + """Default ingress healthcheck timeout. + + This value can be overridden at the charm level. Time values + following Golang time.ParseDuration() format are valid. + """ + return "5s" + + @property + def ingress_healthcheck_params(self): + """Dictionary of ingress healthcheck values.""" + params = { + "path": self.ingress_healthcheck_path, + "interval": self.ingress_healthcheck_interval, + "timeout": self.ingress_healthcheck_timeout, + } + + return params + def get_relation_handlers( self, handlers: list[sunbeam_rhandlers.RelationHandler] | None = None ) -> list[sunbeam_rhandlers.RelationHandler]: @@ -878,6 +916,7 @@ class OSBaseOperatorAPICharm(OSBaseOperatorCharmK8S): "ingress-internal", self.service_name, self.default_public_ingress_port, + self.ingress_healthcheck_params, self._ingress_changed, "ingress-internal" in self.mandatory_relations, ) @@ -888,6 +927,7 @@ class OSBaseOperatorAPICharm(OSBaseOperatorCharmK8S): "ingress-public", self.service_name, self.default_public_ingress_port, + self.ingress_healthcheck_params, self._ingress_changed, "ingress-public" in self.mandatory_relations, ) diff --git a/ops-sunbeam/ops_sunbeam/relation_handlers.py b/ops-sunbeam/ops_sunbeam/relation_handlers.py index a7298b22..da7c144b 100644 --- a/ops-sunbeam/ops_sunbeam/relation_handlers.py +++ b/ops-sunbeam/ops_sunbeam/relation_handlers.py @@ -22,7 +22,9 @@ import secrets import string import typing from typing import ( + Any, Callable, + Dict, ) from urllib.parse import ( urlparse, @@ -193,6 +195,7 @@ class IngressHandler(RelationHandler): relation_name: str, service_name: str, default_ingress_port: int, + ingress_healthcheck_params: Dict[str, Any], callback_f: Callable, mandatory: bool = False, ) -> None: @@ -200,6 +203,7 @@ class IngressHandler(RelationHandler): super().__init__(charm, relation_name, callback_f, mandatory) self.default_ingress_port = default_ingress_port self.service_name = service_name + self.ingress_healthcheck_params = ingress_healthcheck_params def setup_event_handler(self) -> ops.framework.Object: """Configure event handlers for an Ingress relation.""" @@ -212,6 +216,7 @@ class IngressHandler(RelationHandler): self.charm, self.relation_name, port=self.default_ingress_port, + healthcheck_params=self.ingress_healthcheck_params, ) self.framework.observe(interface.on.ready, self._on_ingress_ready) self.framework.observe(interface.on.revoked, self._on_ingress_revoked)