diff --git a/coordinator/lib/charms/loki_k8s/v1/loki_push_api.py b/coordinator/lib/charms/loki_k8s/v1/loki_push_api.py index 309ee12..3207d68 100644 --- a/coordinator/lib/charms/loki_k8s/v1/loki_push_api.py +++ b/coordinator/lib/charms/loki_k8s/v1/loki_push_api.py @@ -478,6 +478,24 @@ def _alert_rules_error(self, event): Units of consumer charm send their alert rules over app relation data using the `alert_rules` key. +## Alert rules encoding + +The consumer publishes its alert rules to the `alert_rules` key of its application +databag. Because large deployments can produce enough alert rules to exceed Juju's +relation data size limit, the rules can be stored LZMA-compressed and base64-encoded +instead of as plain JSON. + +Compression is negotiated over the relation: the provider advertises the encodings it +is able to read in the `alert_rules_encodings` key of its own application databag, and +the consumer picks the best encoding both sides support. A consumer related to a +provider running an older version of this library (which advertises nothing) keeps +writing plain JSON, so upgrades are safe in any order. + +An admin can decode compressed rules with: +```bash + | base64 -d | xz -d | jq +``` + ## Charm logging The `charms.loki_k8s.v0.charm_logging` library can be used in conjunction with this one to configure python's logging module to forward all logs to Loki via the loki-push-api interface. @@ -501,6 +519,7 @@ def __init__(self, ...): import copy import json import logging +import lzma import os import platform import re @@ -511,12 +530,12 @@ def __init__(self, ...): from hashlib import sha256 from io import BytesIO from pathlib import Path -from typing import Any, Dict, List, Optional, Tuple, Union, cast +from typing import Any, Dict, Final, List, Mapping, Optional, Tuple, Union, cast from urllib import request from urllib.error import URLError import yaml -from cosl import CosTool, JujuTopology +from cosl import CosTool, JujuTopology, LZMABase64 from cosl.rules import AlertRules from cosl.types import OfficialRuleFileFormat from ops.charm import ( @@ -544,7 +563,7 @@ def __init__(self, ...): # Increment this PATCH version before using `charmcraft publish-lib` or reset # to 0 if you are raising the major API version -LIBPATCH = 32 +LIBPATCH = 34 PYDEPS = ["cosl"] @@ -593,6 +612,123 @@ def __init__(self, ...): HTTP_LISTEN_PORT_START = 9080 # even start port GRPC_LISTEN_PORT_START = 9095 # odd start port +ALERT_RULES_KEY: Final[str] = "alert_rules" +"""Databag key holding the consumer's alert rules.""" + +ALERT_RULES_ENCODINGS_KEY: Final[str] = "alert_rules_encodings" +"""Databag key with which the provider advertises the encodings it can read.""" + +JSON_ENCODING: Final[str] = "json" +"""Plain JSON alert rules, as written by every version of this library.""" + +LZMA_ENCODING: Final[str] = "lzma" +"""LZMA-compressed, base64-encoded JSON alert rules.""" + +SUPPORTED_ALERT_RULES_ENCODINGS: Final[Tuple[str, ...]] = (LZMA_ENCODING, JSON_ENCODING) +"""Alert rules encodings this library can read and write, most preferred first. + +This is in preference order, not sorted: it is a constant, so the bytes written to the +databag are stable across hooks, which is what matters for avoiding spurious +relation-changed events. +""" + + +def _encode_alert_rules(rules: Mapping[str, Any], encoding: str = JSON_ENCODING) -> str: + """Serialize alert rules for storing them in a relation databag. + + Args: + rules: alert rules in the official Loki rule file format. + encoding: one of `SUPPORTED_ALERT_RULES_ENCODINGS`. Anything else is treated + as `JSON_ENCODING`, because plain JSON is readable by every version of + this library. + + Returns: + The serialized alert rules. + """ + # Sort keys to prevent unnecessary relation-changed churn from key reordering. + serialized = json.dumps(rules, sort_keys=True) + if encoding == LZMA_ENCODING: + return LZMABase64.compress(serialized) + return serialized + + +def _decode_alert_rules(raw: str) -> OfficialRuleFileFormat: + """Deserialize alert rules read from a relation databag. + + Both plain JSON and LZMA-compressed, base64-encoded JSON are accepted, regardless + of the encodings this library advertises, so that a provider can always read the + rules of a consumer running any version of this library. + + Args: + raw: the raw databag value. + + Returns: + The alert rules in the official Loki rule file format. + + Raises: + ValueError: if `raw` is neither valid JSON nor a valid compressed payload, or if it + decodes to something other than a JSON object. + """ + if not raw: + return cast(OfficialRuleFileFormat, {}) + + try: + decoded = json.loads(raw) + except json.JSONDecodeError: + # Not JSON, so this must be a compressed payload. + decoded = raw + + if isinstance(decoded, str): + # A compressed payload, either bare or (as pydantic based libraries write it) + # JSON-encoded. + try: + decoded = json.loads(LZMABase64.decompress(decoded)) + except (ValueError, lzma.LZMAError) as e: + raise ValueError(f"Could not decompress alert rules: {e}") from e + + if not isinstance(decoded, dict): + raise ValueError(f"Alert rules must be a JSON object, not {type(decoded).__name__}") + + return cast(OfficialRuleFileFormat, decoded) + + +def _best_alert_rules_encoding(remote_app_databag: Optional[Mapping[str, str]]) -> str: + """Return the best alert rules encoding the remote application is able to read. + + Providers advertise the encodings they support in their application databag. + Providers running an older version of this library advertise nothing, in which + case plain JSON is used for backwards compatibility. + + Args: + remote_app_databag: the remote application databag, or None if it is not + readable yet (e.g. the relation is still being set up). + + Returns: + One of `SUPPORTED_ALERT_RULES_ENCODINGS`. + """ + raw = remote_app_databag.get(ALERT_RULES_ENCODINGS_KEY, "[]") if remote_app_databag else "[]" + + try: + advertised = json.loads(raw) + if not isinstance(advertised, list): + raise TypeError("expected a list, got {}".format(type(advertised).__name__)) + except (json.JSONDecodeError, TypeError) as e: + logger.warning( + "Ignoring malformed '%s' (%s); assuming the remote end is only able to read " + "uncompressed alert rules.", + ALERT_RULES_ENCODINGS_KEY, + e, + ) + return JSON_ENCODING + + for encoding in SUPPORTED_ALERT_RULES_ENCODINGS: + if encoding in advertised: + return encoding + + # Either nothing was advertised (an older provider), or only encodings this library + # does not know about. Plain JSON is the encoding every version can read. + return JSON_ENCODING + class LokiPushApiError(Exception): """Base class for errors raised by this module.""" @@ -940,6 +1076,13 @@ def __init__( self.framework.observe(events.relation_changed, self._on_logging_relation_changed) self.framework.observe(events.relation_departed, self._on_logging_relation_departed) self.framework.observe(events.relation_broken, self._on_logging_relation_broken) + # Consumers only compress their alert rules if we advertise that we can read them, + # so make sure the advertisement is (re)published after a leadership change, when no + # relation event may fire. + self.framework.observe( + self._charm.on.leader_elected, + self._publish_encodings_to_all_relation_databags, + ) def _on_lifecycle_event(self, _): # Upgrade event or other charm-level event @@ -968,6 +1111,7 @@ def _on_logging_relation_joined(self, event: RelationJoinedEvent): if self._charm.unit.is_leader(): event.relation.data[self._charm.app].update(self._promtail_binary_url) logger.debug("Saved promtail binary url: %s", self._promtail_binary_url) + self._publish_alert_rules_encodings(event.relation) def _on_logging_relation_changed(self, event: HookEvent): """Handle changes in related consumers. @@ -1046,6 +1190,7 @@ def _process_logging_relation_changed(self, relation: Relation) -> bool: """ relation.data[self._charm.unit]["public_address"] = socket.getfqdn() or "" self.update_endpoint(relation=relation) + self._publish_alert_rules_encodings(relation) # Ensure promtail binary URL is set in app data. This is normally done on # relation_joined, but charms using the reconcile pattern may miss that event @@ -1056,6 +1201,28 @@ def _process_logging_relation_changed(self, relation: Relation) -> bool: return self._should_update_alert_rules(relation) + def _publish_encodings_to_all_relation_databags(self, _: Optional[HookEvent]) -> None: + for relation in self._charm.model.relations[self._relation_name]: + self._publish_alert_rules_encodings(relation) + + def _publish_alert_rules_encodings(self, relation: Relation) -> None: + """Advertise the alert rules encodings this library is able to read. + + Consumers use this to decide whether they may compress their alert rules: a + consumer related to a provider that does not advertise anything keeps writing + plain JSON, which every version of this library can read. + + Args: + relation: The relation whose data to update. + """ + if not self._charm.unit.is_leader(): + # Only the leader unit can write to app data. + return + + relation.data[self._charm.app][ALERT_RULES_ENCODINGS_KEY] = json.dumps( + list(SUPPORTED_ALERT_RULES_ENCODINGS) + ) + @property def _promtail_binary_url(self) -> dict: """URL from which Promtail binary can be downloaded.""" @@ -1103,6 +1270,7 @@ def update_endpoint(self, url: str = "", relation: Optional[Relation] = None) -> for relation in relations_list: relation.data[self._charm.unit].update({"endpoint": json.dumps(endpoint)}) + self._publish_alert_rules_encodings(relation) logger.debug("Saved endpoint in unit relation data") @@ -1159,15 +1327,25 @@ def alerts(self) -> dict: # noqa: C901 metadata indexed by relation ID. """ alerts = {} # type: Dict[str, dict] # mapping b/w juju identifiers and alert rule files + unreadable: Dict[int, str] = {} for relation in self._charm.model.relations[self._relation_name]: if not relation.units or not relation.app: continue - alert_rules = json.loads(relation.data[relation.app].get("alert_rules", "{}")) + try: + alert_rules = _decode_alert_rules( + relation.data[relation.app].get(ALERT_RULES_KEY, "{}") + ) + except Exception as e: + # Never let unreadable remote data break the provider: a consumer could + # be writing rules in a format this version of the library predates. + unreadable[relation.id] = str(e) + continue + if not alert_rules: continue - alert_rules = self._inject_alert_expr_labels(alert_rules) + alert_rules = self._inject_alert_expr_labels(cast(Dict[str, Any], alert_rules)) identifier, topology = self._get_identifier_by_alert_rules(alert_rules) if not topology: @@ -1208,6 +1386,12 @@ def alerts(self) -> dict: # noqa: C901 alerts[identifier] = alert_rules + if unreadable: + logger.error( + "Could not read the alert rules published over relation(s): %s", + "; ".join("{} ({})".format(rel_id, err) for rel_id, err in unreadable.items()), + ) + return alerts def has_invalid_alert_rules(self) -> bool: @@ -1404,9 +1588,9 @@ def _handle_alert_rules(self, relation): ) relation.data[self._charm.app]["metadata"] = json.dumps(self.topology.as_dict()) - relation.data[self._charm.app]["alert_rules"] = json.dumps( - alert_rules_as_dict, - sort_keys=True, # sort, to prevent unnecessary relation_changed events + remote_app_databag = relation.data.get(relation.app) if relation.app else None + relation.data[self._charm.app][ALERT_RULES_KEY] = _encode_alert_rules( + alert_rules_as_dict, _best_alert_rules_encoding(remote_app_databag) ) @property @@ -1424,7 +1608,9 @@ def loki_endpoints(self) -> List[dict]: seen_urls = set() for relation in self._charm.model.relations[self._relation_name]: - for unit in relation.units: + # Sort the units so the endpoints list order is stable across runs, + # otherwise the generated promtail config flaps. + for unit in sorted(relation.units, key=lambda u: u.name): if unit.app == self._charm.app: continue @@ -1589,6 +1775,11 @@ def _on_logging_relation_changed(self, event: RelationEvent): loki_push_api_alert_rules_error: This event is emitted when an invalid alert rules file is encountered or if `alert_rules_path` is empty. """ + # The provider advertises the alert rules encodings it supports over relation data, + # which may only become known after relation_joined; (re)send alert rules here so the + # negotiated encoding is picked up. + self._handle_alert_rules(event.relation) # pyright: ignore + if self._charm.unit.is_leader(): ev = json.loads(event.relation.data[event.app].get("event", "{}")) diff --git a/coordinator/tests/unit/test_alert_rule_filtering.py b/coordinator/tests/unit/test_alert_rule_filtering.py index f5c4cfa..3bdacbf 100644 --- a/coordinator/tests/unit/test_alert_rule_filtering.py +++ b/coordinator/tests/unit/test_alert_rule_filtering.py @@ -6,7 +6,16 @@ import socket from unittest.mock import patch +import pytest import yaml +from charms.loki_k8s.v1.loki_push_api import ( + ALERT_RULES_ENCODINGS_KEY, + JSON_ENCODING, + LZMA_ENCODING, + SUPPORTED_ALERT_RULES_ENCODINGS, + _best_alert_rules_encoding, + _encode_alert_rules, +) from cosl import JujuTopology from ops.model import ActiveStatus, BlockedStatus from scenario import Container, Exec, Relation, State @@ -237,3 +246,328 @@ def test_invalid_relation_becoming_valid_recovers_to_active( assert not _relation_errors(recovered_relation) assert _written_group_names(context, recovered_state) == {"valid-group"} assert isinstance(recovered_state.unit_status, ActiveStatus) + + +COMPRESSED_ALERT_RULES_RELATION = Relation( + "logging", + remote_app_name="app-compressed", + remote_app_data={ + "alert_rules": _encode_alert_rules( + json.loads(_alert_rules("compressed-group")), LZMA_ENCODING + ), + "metadata": _metadata("app-compressed"), + }, +) + + +def test_alerts_decodes_lzma_compressed_payload( + context, s3, all_worker, nginx_prometheus_exporter_container +): + # GIVEN a relation whose alert_rules payload is LZMA-compressed + nginx_container = _nginx_container_with_lokitool_sync(_rule_path("app-compressed")) + state_in = State( + relations=[s3, all_worker, COMPRESSED_ALERT_RULES_RELATION], + containers=[nginx_container, nginx_prometheus_exporter_container], + leader=True, + ) + + # WHEN the relation changed event is processed + state_out = context.run( + context.on.relation_changed(COMPRESSED_ALERT_RULES_RELATION), state_in + ) + + # THEN the compressed rules are decoded and written, and the unit remains active + assert _written_group_names(context, state_out) == {"compressed-group"} + assert isinstance(state_out.unit_status, ActiveStatus) + + +def test_alerts_skips_corrupt_alert_rules_and_logs_error( + context, s3, all_worker, nginx_prometheus_exporter_container, caplog +): + # GIVEN a relation with a corrupt/garbage alert_rules value + corrupt_relation = Relation( + "logging", + remote_app_name="app-corrupt", + remote_app_data={ + "alert_rules": "not valid json, nor valid lzma/base64", + "metadata": _metadata("app-corrupt"), + }, + ) + nginx_container = _nginx_container_with_lokitool_sync() + state_in = State( + relations=[s3, all_worker, corrupt_relation], + containers=[nginx_container, nginx_prometheus_exporter_container], + leader=True, + ) + + # WHEN the relation changed event is processed + # THEN it must not raise, and no rules are written + state_out = context.run(context.on.relation_changed(corrupt_relation), state_in) + assert _written_group_names(context, state_out) == set() + assert "Could not read the alert rules published over relation" in caplog.text + + +def test_alerts_skips_unreadable_relation_but_returns_others( + context, s3, all_worker, nginx_prometheus_exporter_container, caplog +): + """One relation with unreadable alert_rules doesn't prevent others from being read. + + Regression test for the log-aggregation change: a single malformed relation is + collected and logged once, without affecting unrelated, healthy relations. + """ + # GIVEN one relation with corrupt alert_rules and one with valid alert_rules + corrupt_relation = Relation( + "logging", + remote_app_name="app-corrupt", + remote_app_data={ + "alert_rules": "not valid json, nor valid lzma/base64", + "metadata": _metadata("app-corrupt"), + }, + ) + nginx_container = _nginx_container_with_lokitool_sync(_rule_path("app-valid")) + state_in = State( + relations=[s3, all_worker, corrupt_relation, VALID_RELATION], + containers=[nginx_container, nginx_prometheus_exporter_container], + leader=True, + ) + + # WHEN the relation changed event is processed for the valid relation + state_out = context.run(context.on.relation_changed(VALID_RELATION), state_in) + + # THEN the valid relation's rules are still written... + assert _written_group_names(context, state_out) == {"valid-group"} + assert isinstance(state_out.unit_status, ActiveStatus) + # ...and the corrupt one is reported, not silently dropped. + assert "Could not read the alert rules published over relation" in caplog.text + + +def test_alerts_skips_double_json_encoded_payload_and_logs_error( + context, s3, all_worker, nginx_prometheus_exporter_container, caplog +): + """A plain JSON *string* (not a compressed payload) is rejected with a clear error. + + Regression test for https://github.com/canonical/prometheus-k8s-operator/pull/864 + review feedback from @Abuelodelanada: ``json.loads('"foo"')`` returns the Python + string ``"foo"``, which then falls into the "this must be a compressed payload" + branch and fails to decompress. This must not raise an unhandled/opaque exception. + """ + # GIVEN a relation whose alert_rules value is a JSON string literal + double_encoded_relation = Relation( + "logging", + remote_app_name="app-double-encoded", + remote_app_data={ + "alert_rules": json.dumps("foo"), + "metadata": _metadata("app-double-encoded"), + }, + ) + nginx_container = _nginx_container_with_lokitool_sync() + state_in = State( + relations=[s3, all_worker, double_encoded_relation], + containers=[nginx_container, nginx_prometheus_exporter_container], + leader=True, + ) + + # WHEN the relation changed event is processed + # THEN it must not raise, and no rules are written + state_out = context.run(context.on.relation_changed(double_encoded_relation), state_in) + assert _written_group_names(context, state_out) == set() + assert "Could not read the alert rules published over relation" in caplog.text + assert "Could not decompress alert rules" in caplog.text + + +def test_alerts_skips_non_object_payload_and_logs_error( + context, s3, all_worker, nginx_prometheus_exporter_container, caplog +): + """A syntactically valid JSON payload that isn't an object is rejected clearly.""" + # GIVEN a relation whose alert_rules value decodes to a JSON list, not an object + non_object_relation = Relation( + "logging", + remote_app_name="app-non-object", + remote_app_data={ + "alert_rules": json.dumps([1, 2, 3]), + "metadata": _metadata("app-non-object"), + }, + ) + nginx_container = _nginx_container_with_lokitool_sync() + state_in = State( + relations=[s3, all_worker, non_object_relation], + containers=[nginx_container, nginx_prometheus_exporter_container], + leader=True, + ) + + # WHEN the relation changed event is processed + # THEN it must not raise, and no rules are written + state_out = context.run(context.on.relation_changed(non_object_relation), state_in) + assert _written_group_names(context, state_out) == set() + assert "Could not read the alert rules published over relation" in caplog.text + assert "Alert rules must be a JSON object" in caplog.text + + +def test_provider_advertises_alert_rules_encodings_on_relation_joined( + context, s3, all_worker, nginx_prometheus_exporter_container +): + # GIVEN a fresh logging relation with no rules yet + logging_relation = Relation("logging", remote_app_name="app-fresh") + nginx_container = _nginx_container_with_lokitool_sync() + state_in = State( + relations=[s3, all_worker, logging_relation], + containers=[nginx_container, nginx_prometheus_exporter_container], + leader=True, + ) + + # WHEN the relation joined event is processed + state_out = context.run(context.on.relation_joined(logging_relation), state_in) + + # THEN the provider advertises the supported alert rules encodings + relation = state_out.get_relation(logging_relation.id) + advertised = relation.local_app_data[ALERT_RULES_ENCODINGS_KEY] + assert json.loads(advertised) == list(SUPPORTED_ALERT_RULES_ENCODINGS) + + +def test_provider_advertises_alert_rules_encodings_on_relation_changed( + context, s3, all_worker, nginx_prometheus_exporter_container +): + # GIVEN a relation already carrying valid rules + nginx_container = _nginx_container_with_lokitool_sync(_rule_path("app-valid")) + state_in = State( + relations=[s3, all_worker, VALID_RELATION], + containers=[nginx_container, nginx_prometheus_exporter_container], + leader=True, + ) + + # WHEN the relation changed event is processed + state_out = context.run(context.on.relation_changed(VALID_RELATION), state_in) + + # THEN the provider (re)advertises the supported alert rules encodings + relation = state_out.get_relation(VALID_RELATION.id) + advertised = relation.local_app_data[ALERT_RULES_ENCODINGS_KEY] + assert json.loads(advertised) == list(SUPPORTED_ALERT_RULES_ENCODINGS) + + +def test_provider_advertises_alert_rules_encodings_on_leader_elected( + context, s3, all_worker, nginx_prometheus_exporter_container +): + # GIVEN a fresh logging relation and this unit just became leader + logging_relation = Relation("logging", remote_app_name="app-fresh") + nginx_container = _nginx_container_with_lokitool_sync() + state_in = State( + relations=[s3, all_worker, logging_relation], + containers=[nginx_container, nginx_prometheus_exporter_container], + leader=True, + ) + + # WHEN the leader elected event is processed + state_out = context.run(context.on.leader_elected(), state_in) + + # THEN the provider advertises the supported alert rules encodings + relation = state_out.get_relation(logging_relation.id) + advertised = relation.local_app_data[ALERT_RULES_ENCODINGS_KEY] + assert json.loads(advertised) == list(SUPPORTED_ALERT_RULES_ENCODINGS) + + +def test_provider_advertises_alert_rules_encodings_on_upgrade_charm( + context, s3, all_worker, nginx_prometheus_exporter_container +): + """The provider (re)advertises supported alert rules encodings on upgrade-charm. + + Unlike `prometheus_remote_write`'s `MetricsEndpointProvider` (which has no + equivalent lifecycle wiring and had to gain a brand new `upgrade_charm` observer + for this), this is already covered here for free by the existing + `_on_lifecycle_event` handler, which every relation already runs through on + `upgrade_charm` and calls `_publish_alert_rules_encodings` as part of + `_process_logging_relation_changed`. This test pins that behavior down + explicitly, so a future refactor of the lifecycle-event plumbing doesn't + silently drop it. + """ + # GIVEN an existing logging relation + logging_relation = Relation("logging", remote_app_name="app-fresh") + nginx_container = _nginx_container_with_lokitool_sync() + state_in = State( + relations=[s3, all_worker, logging_relation], + containers=[nginx_container, nginx_prometheus_exporter_container], + leader=True, + ) + + # WHEN the charm is upgraded + state_out = context.run(context.on.upgrade_charm(), state_in) + + # THEN the provider (re)advertises the supported alert rules encodings + relation = state_out.get_relation(logging_relation.id) + advertised = relation.local_app_data[ALERT_RULES_ENCODINGS_KEY] + assert json.loads(advertised) == list(SUPPORTED_ALERT_RULES_ENCODINGS) + + +def test_encode_alert_rules_json_default(): + """Default encoding is plain, sorted-keys JSON (legacy-compatible).""" + rules = json.loads(_alert_rules("encode-test")) + encoded = _encode_alert_rules(rules, JSON_ENCODING) + assert json.loads(encoded) == rules + # Not compressed: readable directly as JSON. + assert encoded.startswith("{") + + +def test_unknown_encoding_falls_back_to_json(): + """An encoding this library doesn't know about is treated as plain JSON.""" + rules = json.loads(_alert_rules("unknown-encoding-test")) + encoded = _encode_alert_rules(rules, "brotli") + assert json.loads(encoded) == rules + + +@pytest.mark.parametrize("encoding", SUPPORTED_ALERT_RULES_ENCODINGS) +def test_encoding_is_deterministic(encoding): + """The same rules, with keys in a different order, encode to identical bytes. + + Juju compares relation-databag values byte for byte to decide whether to emit + relation-changed; an unstable key order would trigger spurious relation-changed + events on every hook. + """ + rules = json.loads(_alert_rules("determinism-test")) + reordered = json.loads(json.dumps(rules)) + rule = reordered["groups"][0]["rules"][0] + reordered["groups"][0]["rules"][0] = dict(reversed(list(rule.items()))) + assert list(reordered["groups"][0]["rules"][0]) != list(rules["groups"][0]["rules"][0]) + + assert _encode_alert_rules(reordered, encoding) == _encode_alert_rules(rules, encoding) + + +@pytest.mark.parametrize( + "remote_app_databag, expected", + [ + pytest.param(None, JSON_ENCODING, id="unreadable_databag"), + pytest.param({}, JSON_ENCODING, id="no_advertisement"), + pytest.param({ALERT_RULES_ENCODINGS_KEY: "[]"}, JSON_ENCODING, id="nothing_advertised"), + pytest.param( + {ALERT_RULES_ENCODINGS_KEY: json.dumps([JSON_ENCODING])}, + JSON_ENCODING, + id="json_only", + ), + pytest.param( + {ALERT_RULES_ENCODINGS_KEY: json.dumps(["brotli"])}, + JSON_ENCODING, + id="unknown_encoding", + ), + pytest.param( + {ALERT_RULES_ENCODINGS_KEY: "not json"}, + JSON_ENCODING, + id="malformed_advertisement", + ), + pytest.param( + {ALERT_RULES_ENCODINGS_KEY: json.dumps({"lzma": True})}, + JSON_ENCODING, + id="not_a_list", + ), + pytest.param( + {ALERT_RULES_ENCODINGS_KEY: json.dumps([LZMA_ENCODING, JSON_ENCODING])}, + LZMA_ENCODING, + id="lzma_advertised", + ), + pytest.param( + {ALERT_RULES_ENCODINGS_KEY: json.dumps(["brotli", LZMA_ENCODING])}, + LZMA_ENCODING, + id="lzma_among_unknown_encodings", + ), + ], +) +def test_encoding_negotiation(remote_app_databag, expected): + """Exhaustive matrix for `_best_alert_rules_encoding`, direct against the pure function.""" + assert _best_alert_rules_encoding(remote_app_databag) == expected diff --git a/tests/integration/conftest.py b/tests/integration/conftest.py index 6088865..373d416 100644 --- a/tests/integration/conftest.py +++ b/tests/integration/conftest.py @@ -47,11 +47,27 @@ def charm_and_channel_and_resources( except subprocess.CalledProcessError: logger.warning("Failed to build Loki %s. Trying again!", role) continue + pth = _resolve_packed_charm(pth, REPO_ROOT / role) os.environ[charm_path_key] = str(pth) return pth, None, get_resources(REPO_ROOT / role) raise subprocess.CalledProcessError(1, f"pack {role}") +def _resolve_packed_charm(packed: Path, project_dir: Path) -> Path: + # charmcraft 4.4.1 and 4.4.2 ignore the output directory passed to + # `charmcraft pack` and leave the .charm file inside the project + # directory instead: https://github.com/canonical/charmcraft/issues/2854 + # Same workaround as Juju's test helpers: https://github.com/juju/juju/pull/23174 + if packed.is_file(): + return packed + in_project_dir = project_dir / packed.name + if in_project_dir.is_file(): + return in_project_dir + raise FileNotFoundError( + f"packed charm {packed.name} not found in {packed.parent} or {project_dir}" + ) + + @fixture(scope="session") def coordinator_charm(): """Loki coordinator used for integration testing.""" diff --git a/worker/lib/charms/loki_k8s/v1/loki_push_api.py b/worker/lib/charms/loki_k8s/v1/loki_push_api.py index e005222..3207d68 100644 --- a/worker/lib/charms/loki_k8s/v1/loki_push_api.py +++ b/worker/lib/charms/loki_k8s/v1/loki_push_api.py @@ -478,6 +478,24 @@ def _alert_rules_error(self, event): Units of consumer charm send their alert rules over app relation data using the `alert_rules` key. +## Alert rules encoding + +The consumer publishes its alert rules to the `alert_rules` key of its application +databag. Because large deployments can produce enough alert rules to exceed Juju's +relation data size limit, the rules can be stored LZMA-compressed and base64-encoded +instead of as plain JSON. + +Compression is negotiated over the relation: the provider advertises the encodings it +is able to read in the `alert_rules_encodings` key of its own application databag, and +the consumer picks the best encoding both sides support. A consumer related to a +provider running an older version of this library (which advertises nothing) keeps +writing plain JSON, so upgrades are safe in any order. + +An admin can decode compressed rules with: +```bash + | base64 -d | xz -d | jq +``` + ## Charm logging The `charms.loki_k8s.v0.charm_logging` library can be used in conjunction with this one to configure python's logging module to forward all logs to Loki via the loki-push-api interface. @@ -501,6 +519,7 @@ def __init__(self, ...): import copy import json import logging +import lzma import os import platform import re @@ -511,12 +530,12 @@ def __init__(self, ...): from hashlib import sha256 from io import BytesIO from pathlib import Path -from typing import Any, Dict, List, Optional, Tuple, Union, cast +from typing import Any, Dict, Final, List, Mapping, Optional, Tuple, Union, cast from urllib import request from urllib.error import URLError import yaml -from cosl import CosTool, JujuTopology +from cosl import CosTool, JujuTopology, LZMABase64 from cosl.rules import AlertRules from cosl.types import OfficialRuleFileFormat from ops.charm import ( @@ -544,7 +563,7 @@ def __init__(self, ...): # Increment this PATCH version before using `charmcraft publish-lib` or reset # to 0 if you are raising the major API version -LIBPATCH = 31 +LIBPATCH = 34 PYDEPS = ["cosl"] @@ -593,6 +612,123 @@ def __init__(self, ...): HTTP_LISTEN_PORT_START = 9080 # even start port GRPC_LISTEN_PORT_START = 9095 # odd start port +ALERT_RULES_KEY: Final[str] = "alert_rules" +"""Databag key holding the consumer's alert rules.""" + +ALERT_RULES_ENCODINGS_KEY: Final[str] = "alert_rules_encodings" +"""Databag key with which the provider advertises the encodings it can read.""" + +JSON_ENCODING: Final[str] = "json" +"""Plain JSON alert rules, as written by every version of this library.""" + +LZMA_ENCODING: Final[str] = "lzma" +"""LZMA-compressed, base64-encoded JSON alert rules.""" + +SUPPORTED_ALERT_RULES_ENCODINGS: Final[Tuple[str, ...]] = (LZMA_ENCODING, JSON_ENCODING) +"""Alert rules encodings this library can read and write, most preferred first. + +This is in preference order, not sorted: it is a constant, so the bytes written to the +databag are stable across hooks, which is what matters for avoiding spurious +relation-changed events. +""" + + +def _encode_alert_rules(rules: Mapping[str, Any], encoding: str = JSON_ENCODING) -> str: + """Serialize alert rules for storing them in a relation databag. + + Args: + rules: alert rules in the official Loki rule file format. + encoding: one of `SUPPORTED_ALERT_RULES_ENCODINGS`. Anything else is treated + as `JSON_ENCODING`, because plain JSON is readable by every version of + this library. + + Returns: + The serialized alert rules. + """ + # Sort keys to prevent unnecessary relation-changed churn from key reordering. + serialized = json.dumps(rules, sort_keys=True) + if encoding == LZMA_ENCODING: + return LZMABase64.compress(serialized) + return serialized + + +def _decode_alert_rules(raw: str) -> OfficialRuleFileFormat: + """Deserialize alert rules read from a relation databag. + + Both plain JSON and LZMA-compressed, base64-encoded JSON are accepted, regardless + of the encodings this library advertises, so that a provider can always read the + rules of a consumer running any version of this library. + + Args: + raw: the raw databag value. + + Returns: + The alert rules in the official Loki rule file format. + + Raises: + ValueError: if `raw` is neither valid JSON nor a valid compressed payload, or if it + decodes to something other than a JSON object. + """ + if not raw: + return cast(OfficialRuleFileFormat, {}) + + try: + decoded = json.loads(raw) + except json.JSONDecodeError: + # Not JSON, so this must be a compressed payload. + decoded = raw + + if isinstance(decoded, str): + # A compressed payload, either bare or (as pydantic based libraries write it) + # JSON-encoded. + try: + decoded = json.loads(LZMABase64.decompress(decoded)) + except (ValueError, lzma.LZMAError) as e: + raise ValueError(f"Could not decompress alert rules: {e}") from e + + if not isinstance(decoded, dict): + raise ValueError(f"Alert rules must be a JSON object, not {type(decoded).__name__}") + + return cast(OfficialRuleFileFormat, decoded) + + +def _best_alert_rules_encoding(remote_app_databag: Optional[Mapping[str, str]]) -> str: + """Return the best alert rules encoding the remote application is able to read. + + Providers advertise the encodings they support in their application databag. + Providers running an older version of this library advertise nothing, in which + case plain JSON is used for backwards compatibility. + + Args: + remote_app_databag: the remote application databag, or None if it is not + readable yet (e.g. the relation is still being set up). + + Returns: + One of `SUPPORTED_ALERT_RULES_ENCODINGS`. + """ + raw = remote_app_databag.get(ALERT_RULES_ENCODINGS_KEY, "[]") if remote_app_databag else "[]" + + try: + advertised = json.loads(raw) + if not isinstance(advertised, list): + raise TypeError("expected a list, got {}".format(type(advertised).__name__)) + except (json.JSONDecodeError, TypeError) as e: + logger.warning( + "Ignoring malformed '%s' (%s); assuming the remote end is only able to read " + "uncompressed alert rules.", + ALERT_RULES_ENCODINGS_KEY, + e, + ) + return JSON_ENCODING + + for encoding in SUPPORTED_ALERT_RULES_ENCODINGS: + if encoding in advertised: + return encoding + + # Either nothing was advertised (an older provider), or only encodings this library + # does not know about. Plain JSON is the encoding every version can read. + return JSON_ENCODING + class LokiPushApiError(Exception): """Base class for errors raised by this module.""" @@ -940,6 +1076,13 @@ def __init__( self.framework.observe(events.relation_changed, self._on_logging_relation_changed) self.framework.observe(events.relation_departed, self._on_logging_relation_departed) self.framework.observe(events.relation_broken, self._on_logging_relation_broken) + # Consumers only compress their alert rules if we advertise that we can read them, + # so make sure the advertisement is (re)published after a leadership change, when no + # relation event may fire. + self.framework.observe( + self._charm.on.leader_elected, + self._publish_encodings_to_all_relation_databags, + ) def _on_lifecycle_event(self, _): # Upgrade event or other charm-level event @@ -968,6 +1111,7 @@ def _on_logging_relation_joined(self, event: RelationJoinedEvent): if self._charm.unit.is_leader(): event.relation.data[self._charm.app].update(self._promtail_binary_url) logger.debug("Saved promtail binary url: %s", self._promtail_binary_url) + self._publish_alert_rules_encodings(event.relation) def _on_logging_relation_changed(self, event: HookEvent): """Handle changes in related consumers. @@ -1046,6 +1190,7 @@ def _process_logging_relation_changed(self, relation: Relation) -> bool: """ relation.data[self._charm.unit]["public_address"] = socket.getfqdn() or "" self.update_endpoint(relation=relation) + self._publish_alert_rules_encodings(relation) # Ensure promtail binary URL is set in app data. This is normally done on # relation_joined, but charms using the reconcile pattern may miss that event @@ -1056,6 +1201,28 @@ def _process_logging_relation_changed(self, relation: Relation) -> bool: return self._should_update_alert_rules(relation) + def _publish_encodings_to_all_relation_databags(self, _: Optional[HookEvent]) -> None: + for relation in self._charm.model.relations[self._relation_name]: + self._publish_alert_rules_encodings(relation) + + def _publish_alert_rules_encodings(self, relation: Relation) -> None: + """Advertise the alert rules encodings this library is able to read. + + Consumers use this to decide whether they may compress their alert rules: a + consumer related to a provider that does not advertise anything keeps writing + plain JSON, which every version of this library can read. + + Args: + relation: The relation whose data to update. + """ + if not self._charm.unit.is_leader(): + # Only the leader unit can write to app data. + return + + relation.data[self._charm.app][ALERT_RULES_ENCODINGS_KEY] = json.dumps( + list(SUPPORTED_ALERT_RULES_ENCODINGS) + ) + @property def _promtail_binary_url(self) -> dict: """URL from which Promtail binary can be downloaded.""" @@ -1103,6 +1270,7 @@ def update_endpoint(self, url: str = "", relation: Optional[Relation] = None) -> for relation in relations_list: relation.data[self._charm.unit].update({"endpoint": json.dumps(endpoint)}) + self._publish_alert_rules_encodings(relation) logger.debug("Saved endpoint in unit relation data") @@ -1159,15 +1327,25 @@ def alerts(self) -> dict: # noqa: C901 metadata indexed by relation ID. """ alerts = {} # type: Dict[str, dict] # mapping b/w juju identifiers and alert rule files + unreadable: Dict[int, str] = {} for relation in self._charm.model.relations[self._relation_name]: if not relation.units or not relation.app: continue - alert_rules = json.loads(relation.data[relation.app].get("alert_rules", "{}")) + try: + alert_rules = _decode_alert_rules( + relation.data[relation.app].get(ALERT_RULES_KEY, "{}") + ) + except Exception as e: + # Never let unreadable remote data break the provider: a consumer could + # be writing rules in a format this version of the library predates. + unreadable[relation.id] = str(e) + continue + if not alert_rules: continue - alert_rules = self._inject_alert_expr_labels(alert_rules) + alert_rules = self._inject_alert_expr_labels(cast(Dict[str, Any], alert_rules)) identifier, topology = self._get_identifier_by_alert_rules(alert_rules) if not topology: @@ -1208,6 +1386,12 @@ def alerts(self) -> dict: # noqa: C901 alerts[identifier] = alert_rules + if unreadable: + logger.error( + "Could not read the alert rules published over relation(s): %s", + "; ".join("{} ({})".format(rel_id, err) for rel_id, err in unreadable.items()), + ) + return alerts def has_invalid_alert_rules(self) -> bool: @@ -1404,9 +1588,9 @@ def _handle_alert_rules(self, relation): ) relation.data[self._charm.app]["metadata"] = json.dumps(self.topology.as_dict()) - relation.data[self._charm.app]["alert_rules"] = json.dumps( - alert_rules_as_dict, - sort_keys=True, # sort, to prevent unnecessary relation_changed events + remote_app_databag = relation.data.get(relation.app) if relation.app else None + relation.data[self._charm.app][ALERT_RULES_KEY] = _encode_alert_rules( + alert_rules_as_dict, _best_alert_rules_encoding(remote_app_databag) ) @property @@ -1424,7 +1608,9 @@ def loki_endpoints(self) -> List[dict]: seen_urls = set() for relation in self._charm.model.relations[self._relation_name]: - for unit in relation.units: + # Sort the units so the endpoints list order is stable across runs, + # otherwise the generated promtail config flaps. + for unit in sorted(relation.units, key=lambda u: u.name): if unit.app == self._charm.app: continue @@ -1589,6 +1775,11 @@ def _on_logging_relation_changed(self, event: RelationEvent): loki_push_api_alert_rules_error: This event is emitted when an invalid alert rules file is encountered or if `alert_rules_path` is empty. """ + # The provider advertises the alert rules encodings it supports over relation data, + # which may only become known after relation_joined; (re)send alert rules here so the + # negotiated encoding is picked up. + self._handle_alert_rules(event.relation) # pyright: ignore + if self._charm.unit.is_leader(): ev = json.loads(event.relation.data[event.app].get("event", "{}")) @@ -1787,22 +1978,13 @@ def _on_relation_changed(self, event: RelationEvent) -> None: self._handle_alert_rules(event.relation) if self._charm.unit.is_leader(): - ev = json.loads(event.relation.data[event.app].get("event", "{}")) - - if ev: - valid = bool(ev.get("valid", True)) - errors = ev.get("errors", "") - - if valid and not errors: - self.on.alert_rule_status_changed.emit(valid=valid) - else: - self.on.alert_rule_status_changed.emit(valid=valid, errors=errors) + self._handle_alert_rule_status_changed(event) for container in self._containers.values(): if not container.can_connect(): continue if self.model.relations[self._relation_name]: - if "promtail" not in container.get_plan().services: + if not self._is_promtail_set_up(container): self._setup_promtail(container) continue @@ -1815,11 +1997,24 @@ def _on_relation_changed(self, event: RelationEvent) -> None: # Loki may send endpoints late. Don't necessarily start, there may be # no clients if new_config["clients"]: - container.restart(WORKLOAD_SERVICE_NAME) - self.on.log_proxy_endpoint_joined.emit() + if self._restart_promtail(container): + self.on.log_proxy_endpoint_joined.emit() else: self.on.promtail_digest_error.emit("No promtail client endpoints available!") + def _handle_alert_rule_status_changed(self, event: RelationEvent) -> None: + """Relay the alert rule validation status reported by the Loki provider.""" + ev = json.loads(event.relation.data[event.app].get("event", "{}")) + + if ev: + valid = bool(ev.get("valid", True)) + errors = ev.get("errors", "") + + if valid and not errors: + self.on.alert_rule_status_changed.emit(valid=valid) + else: + self.on.alert_rule_status_changed.emit(valid=valid, errors=errors) + def _on_relation_departed(self, _: RelationEvent) -> None: """Event handler for `relation_departed`. @@ -1838,11 +2033,28 @@ def _on_relation_departed(self, _: RelationEvent) -> None: container.push(WORKLOAD_CONFIG_PATH, yaml.safe_dump(new_config), make_dirs=True) if new_config["clients"]: - container.restart(WORKLOAD_SERVICE_NAME) + self._restart_promtail(container) else: container.stop(WORKLOAD_SERVICE_NAME) self.on.log_proxy_endpoint_departed.emit() + def _restart_promtail(self, container: Container) -> bool: + """Restart promtail, surfacing a Pebble failure as a digest error. + + Args: + container: the workload container running the promtail service. + + Returns: + True on success, False if the restart failed. + """ + try: + container.restart(WORKLOAD_SERVICE_NAME) + except ChangeError as e: + logger.warning("Failed to restart promtail: %s", e) + self.on.promtail_digest_error.emit(str(e)) + return False + return True + def _add_pebble_layer(self, workload_binary_path: str, container: Container) -> None: """Adds Pebble layer that manages Promtail service in Workload container. @@ -2202,6 +2414,35 @@ def _generate_static_configs(self, config: dict, container_name: str) -> list: return static_configs + def _promtail_binary_spec(self) -> dict: + """The promtail binary metadata advertised on the log-proxy relation.""" + relations = self._charm.model.relations[self._relation_name] + if not relations: + return {} + relation = relations[0] + return json.loads(relation.data[relation.app].get("promtail_binary_zip_url", "{}")) + + def _is_promtail_set_up(self, container: Container) -> bool: + """Whether promtail is fully usable in this container. + + Unlike ``_is_promtail_installed`` (binary only), this also requires the + pebble service to be registered. + + The pebble plan alone is not proof: if the workload container lost its + ephemeral filesystem (e.g. after pod churn) the layer may still be in + the plan while the runtime-pushed binary is gone. Trusting the plan and + restarting unconditionally wedges the unit + (https://github.com/canonical/loki-k8s-operator/issues/659). + """ + if "promtail" not in container.get_plan().services: + return False + promtail_info = self._promtail_binary_spec().get(self._arch) + if not promtail_info: + # No promtail binary advertised for this architecture (or nothing + # published on the relation yet), so promtail cannot be running here. + return False + return self._is_promtail_installed(promtail_info, container) + def _setup_promtail(self, container: Container) -> None: # Use the first relations = self._charm.model.relations[self._relation_name] @@ -2216,10 +2457,22 @@ def _setup_promtail(self, container: Container) -> None: relation.data[relation.app].get("promtail_binary_zip_url", "{}") ) if not promtail_binaries: + # The Loki charm hasn't published the binary metadata yet; a later + # relation-changed will bring us back here. + return + + if self._arch not in promtail_binaries: + msg = f"No promtail binary available for architecture {self._arch}" + logger.warning(msg) + self.on.promtail_digest_error.emit(msg) return self._create_directories(container) - self._ensure_promtail_binary(promtail_binaries, container) + if not self._ensure_promtail_binary(promtail_binaries, container): + # Do not add the pebble layer: a service whose command points to a + # missing binary would wedge the unit on every restart attempt + # (https://github.com/canonical/loki-k8s-operator/issues/659). + return container.push( WORKLOAD_CONFIG_PATH, @@ -2242,9 +2495,18 @@ def _setup_promtail(self, container: Container) -> None: else: self.on.promtail_digest_error.emit("No promtail client endpoints available!") - def _ensure_promtail_binary(self, promtail_binaries: dict, container: Container): + def _ensure_promtail_binary(self, promtail_binaries: dict, container: Container) -> bool: + """Ensure the promtail binary is present in the workload container. + + Args: + promtail_binaries: dictionary of promtail binaries per architecture. + container: container in which promtail must be installed. + + Returns: + True if the binary is available, False if it could not be obtained. + """ if self._is_promtail_installed(promtail_binaries[self._arch], container): - return + return True try: self._obtain_promtail(promtail_binaries[self._arch], container) @@ -2252,6 +2514,8 @@ def _ensure_promtail_binary(self, promtail_binaries: dict, container: Container) msg = f"Promtail binary couldn't be downloaded - {str(e)}" logger.warning(msg) self.on.promtail_digest_error.emit(msg) + return False + return True def _is_promtail_installed(self, promtail_info: dict, container: Container) -> bool: """Determine if promtail has already been installed to the container.