From b4107309ceea04fcdaf3b87b7bf27d64f9823226 Mon Sep 17 00:00:00 2001 From: Tony Meyer Date: Fri, 24 Nov 2023 11:48:23 +1300 Subject: [PATCH] feat: expose the ability to set Pebble log targets (#1074) Expose the ability to set Pebble log targets. Add a real-pebble test that a layer with log forwarding can be set and retrieved. Explain how to test bleeding-edge pebble functionality. Use master@HEAD version of pebble for the test. --- .github/workflows/framework-tests.yaml | 2 +- CHANGES.md | 1 + HACKING.md | 8 +- ops/pebble.py | 71 ++++++++++++- test/test_pebble.py | 139 ++++++++++++++++++++++++- test/test_real_pebble.py | 26 +++++ 6 files changed, 236 insertions(+), 11 deletions(-) diff --git a/.github/workflows/framework-tests.yaml b/.github/workflows/framework-tests.yaml index 8a69adfc0..c3bdd4aa4 100644 --- a/.github/workflows/framework-tests.yaml +++ b/.github/workflows/framework-tests.yaml @@ -77,7 +77,7 @@ jobs: run: pip install tox~=4.2 - name: Install Pebble - run: go install github.com/canonical/pebble/cmd/pebble@latest + run: go install github.com/canonical/pebble/cmd/pebble@master - name: Start Pebble run: | diff --git a/CHANGES.md b/CHANGES.md index 3ff93840c..dd9b2c86f 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -1,5 +1,6 @@ # 2.9.0 +* Added log target support to `ops.pebble` layers and plans. * Added `Harness.run_action()`, `testing.ActionOutput`, and `testing.ActionFailed` # 2.8.0 diff --git a/HACKING.md b/HACKING.md index 85a046859..ff556d41b 100644 --- a/HACKING.md +++ b/HACKING.md @@ -52,20 +52,20 @@ pytest ## Pebble Tests -The framework has some tests that interact with a real/live pebble server. To +The framework has some tests that interact with a real/live Pebble server. To run these tests, you must have [pebble](https://github.com/canonical/pebble) installed and available in your path. If you have the Go toolchain installed, -you can run `go install github.com/canonical/pebble/cmd/pebble@latest`. This will +you can run `go install github.com/canonical/pebble/cmd/pebble@master`. This will install pebble to `$GOBIN` if it is set or `$HOME/go/bin` otherwise. Add `$GOBIN` to your path (e.g. `export PATH=$PATH:$GOBIN` or `export PATH=$PATH:$HOME/go/bin` in your `.bashrc`) and you are ready to run the real -pebble tests: +Pebble tests: ```sh tox -e pebble ``` -To do this even more manually, you could start the pebble server yourself: +To do this even more manually, you could start the Pebble server yourself: ```sh export PEBBLE=$HOME/pebble diff --git a/ops/pebble.py b/ops/pebble.py index e1e8e3648..ac4278c72 100644 --- a/ops/pebble.py +++ b/ops/pebble.py @@ -124,16 +124,27 @@ 'threshold': Optional[int]}, total=False) +# In Python 3.11+ 'services' and 'labels' should be NotRequired, and total=True. +LogTargetDict = typing.TypedDict('LogTargetDict', + {'override': Union[Literal['merge'], Literal['replace']], + 'type': Literal['loki'], + 'location': str, + 'services': List[str], + 'labels': Dict[str, str]}, + total=False) + LayerDict = typing.TypedDict('LayerDict', {'summary': str, 'description': str, 'services': Dict[str, ServiceDict], - 'checks': Dict[str, CheckDict]}, + 'checks': Dict[str, CheckDict], + 'log-targets': Dict[str, LogTargetDict]}, total=False) PlanDict = typing.TypedDict('PlanDict', {'services': Dict[str, ServiceDict], - 'checks': Dict[str, CheckDict]}, + 'checks': Dict[str, CheckDict], + 'log-targets': Dict[str, LogTargetDict]}, total=False) _AuthDict = TypedDict('_AuthDict', @@ -718,6 +729,9 @@ def __init__(self, raw: str): for name, service in d.get('services', {}).items()} self._checks: Dict[str, Check] = {name: Check(name, check) for name, check in d.get('checks', {}).items()} + self._log_targets: Dict[str, LogTarget] = { + name: LogTarget(name, target) + for name, target in d.get('log-targets', {}).items()} @property def services(self) -> Dict[str, 'Service']: @@ -735,11 +749,20 @@ def checks(self) -> Dict[str, 'Check']: """ return self._checks + @property + def log_targets(self) -> Dict[str, 'LogTarget']: + """This plan's log targets mapping (maps log target name to :class:`LogTarget`). + + This property is currently read-only. + """ + return self._log_targets + def to_dict(self) -> 'PlanDict': """Convert this plan to its dict representation.""" fields = [ ('services', {name: service.to_dict() for name, service in self._services.items()}), ('checks', {name: check.to_dict() for name, check in self._checks.items()}), + ('log-targets', {name: target.to_dict() for name, target in self._log_targets.items()}) ] dct = {name: value for name, value in fields if value} return typing.cast('PlanDict', dct) @@ -766,6 +789,8 @@ class Layer: services: Dict[str, 'Service'] #: Mapping of check to :class:`Check` defined by this layer. checks: Dict[str, 'Check'] + #: Mapping of target to :class:`LogTarget` defined by this layer. + log_targets: Dict[str, 'LogTarget'] def __init__(self, raw: Optional[Union[str, 'LayerDict']] = None): if isinstance(raw, str): @@ -780,6 +805,8 @@ def __init__(self, raw: Optional[Union[str, 'LayerDict']] = None): for name, service in d.get('services', {}).items()} self.checks = {name: Check(name, check) for name, check in d.get('checks', {}).items()} + self.log_targets = {name: LogTarget(name, target) + for name, target in d.get('log-targets', {}).items()} def to_yaml(self) -> str: """Convert this layer to its YAML representation.""" @@ -792,6 +819,7 @@ def to_dict(self) -> 'LayerDict': ('description', self.description), ('services', {name: service.to_dict() for name, service in self.services.items()}), ('checks', {name: check.to_dict() for name, check in self.checks.items()}), + ('log-targets', {name: target.to_dict() for name, target in self.log_targets.items()}) ] dct = {name: value for name, value in fields if value} return typing.cast('LayerDict', dct) @@ -1028,6 +1056,45 @@ class CheckStatus(enum.Enum): DOWN = 'down' +class LogTarget: + """Represents a log target in a Pebble configuration layer.""" + + def __init__(self, name: str, raw: Optional['LogTargetDict'] = None): + self.name = name + dct: LogTargetDict = raw or {} + self.override: str = dct.get('override', '') + self.type = dct.get('type', '') + self.location = dct.get('location', '') + self.services: List[str] = list(dct.get('services', [])) + labels = dct.get('labels') + if labels is not None: + labels = copy.deepcopy(labels) + self.labels: Optional[Dict[str, str]] = labels + + def to_dict(self) -> 'LogTargetDict': + """Convert this log target object to its dict representation.""" + fields = [ + ('override', self.override), + ('type', self.type), + ('location', self.location), + ('services', self.services), + ('labels', self.labels), + ] + dct = {name: value for name, value in fields if value} + return typing.cast('LogTargetDict', dct) + + def __repr__(self): + return f'LogTarget({self.to_dict()!r})' + + def __eq__(self, other: Union['LogTargetDict', 'LogTarget']): + if isinstance(other, dict): + return self.to_dict() == other + elif isinstance(other, LogTarget): + return self.to_dict() == other.to_dict() + else: + return NotImplemented + + class FileType(enum.Enum): """Enum of file types.""" diff --git a/test/test_pebble.py b/test/test_pebble.py index c72431fb5..c86d266ea 100644 --- a/test/test_pebble.py +++ b/test/test_pebble.py @@ -452,7 +452,12 @@ def test_services(self): plan = pebble.Plan('') self.assertEqual(plan.services, {}) - plan = pebble.Plan('services:\n foo:\n override: replace\n command: echo foo') + plan = pebble.Plan(""" +services: + foo: + override: replace + command: echo foo +""") self.assertEqual(len(plan.services), 1) self.assertEqual(plan.services['foo'].name, 'foo') @@ -467,8 +472,13 @@ def test_checks(self): plan = pebble.Plan('') self.assertEqual(plan.checks, {}) - plan = pebble.Plan( - 'checks:\n bar:\n override: replace\n http:\n url: https://example.com/') + plan = pebble.Plan(""" +checks: + bar: + override: replace + http: + url: https://example.com/ +""") self.assertEqual(len(plan.checks), 1) self.assertEqual(plan.checks['bar'].name, 'bar') @@ -479,6 +489,29 @@ def test_checks(self): with self.assertRaises(AttributeError): plan.checks = {} # type: ignore + def test_log_targets(self): + plan = pebble.Plan('') + self.assertEqual(plan.log_targets, {}) + + location = "https://example.com:3100/loki/api/v1/push" + plan = pebble.Plan(f""" +log-targets: + baz: + override: replace + type: loki + location: {location} +""") + + self.assertEqual(len(plan.log_targets), 1) + self.assertEqual(plan.log_targets['baz'].name, 'baz') + self.assertEqual(plan.log_targets['baz'].override, 'replace') + self.assertEqual(plan.log_targets['baz'].type, "loki") + self.assertEqual(plan.log_targets['baz'].location, location) + + # Should be read-only ("can't set attribute") + with self.assertRaises(AttributeError): + plan.log_targets = {} # type: ignore + def test_yaml(self): # Starting with nothing, we get the empty result plan = pebble.Plan('') @@ -496,6 +529,12 @@ def test_yaml(self): bar: http: https://example.com/ + +log-targets: + baz: + override: replace + type: loki + location: https://example.com:3100/loki/api/v1/push ''' plan = pebble.Plan(raw) reformed = yaml.safe_dump(yaml.safe_load(raw)) @@ -503,7 +542,12 @@ def test_yaml(self): self.assertEqual(str(plan), reformed) def test_service_equality(self): - plan = pebble.Plan('services:\n foo:\n override: replace\n command: echo foo') + plan = pebble.Plan(""" +services: + foo: + override: replace + command: echo foo +""") old_service = pebble.Service(name="foo", raw={ @@ -524,6 +568,8 @@ def _assert_empty(self, layer: pebble.Layer): self.assertEqual(layer.summary, '') self.assertEqual(layer.description, '') self.assertEqual(layer.services, {}) + self.assertEqual(layer.checks, {}) + self.assertEqual(layer.log_targets, {}) self.assertEqual(layer.to_dict(), {}) def test_no_args(self): @@ -546,6 +592,17 @@ def test_dict(self): 'summary': 'Bar', 'command': 'echo bar', }, + }, + 'log-targets': { + 'baz': { + 'override': 'merge', + 'type': 'loki', + 'location': 'https://example.com', + 'services': ['foo'], + 'labels': { + 'key': 'value $VAR', + } + }, } } s = pebble.Layer(d) @@ -557,6 +614,12 @@ def test_dict(self): self.assertEqual(s.services['bar'].name, 'bar') self.assertEqual(s.services['bar'].summary, 'Bar') self.assertEqual(s.services['bar'].command, 'echo bar') + self.assertEqual(s.log_targets['baz'].name, 'baz') + self.assertEqual(s.log_targets['baz'].override, 'merge') + self.assertEqual(s.log_targets['baz'].type, 'loki') + self.assertEqual(s.log_targets['baz'].location, 'https://example.com') + self.assertEqual(s.log_targets['baz'].services, ['foo']) + self.assertEqual(s.log_targets['baz'].labels, {'key': 'value $VAR'}) self.assertEqual(s.to_dict(), d) @@ -569,6 +632,11 @@ def test_yaml(self): http: url: https://example.com/ description: The quick brown fox! +log-targets: + baz: + location: https://example.com:3100 + override: replace + type: loki services: bar: command: echo bar @@ -604,6 +672,10 @@ def test_yaml(self): self.assertEqual(s.checks['chk'].name, 'chk') self.assertEqual(s.checks['chk'].http, {'url': 'https://example.com/'}) + self.assertEqual(s.log_targets['baz'].name, 'baz') + self.assertEqual(s.log_targets['baz'].override, 'replace') + self.assertEqual(s.log_targets['baz'].location, 'https://example.com:3100') + self.assertEqual(s.to_yaml(), yaml) self.assertEqual(str(s), yaml) @@ -884,6 +956,65 @@ def test_equality(self): self.assertNotEqual(one, 5) +class TestLogTarget(unittest.TestCase): + def _assert_empty(self, target: pebble.LogTarget, name: str): + self.assertEqual(target.name, name) + self.assertEqual(target.override, '') + self.assertEqual(target.type, '') + self.assertEqual(target.location, '') + self.assertEqual(target.services, []) + self.assertIs(target.labels, None) + + def test_name_only(self): + target = pebble.LogTarget('tgt') + self._assert_empty(target, 'tgt') + + def test_dict(self): + d: pebble.LogTargetDict = { + 'override': 'replace', + 'type': 'loki', + 'location': 'https://example.com:3100/loki/api/v1/push', + 'services': ['+all'], + 'labels': {'key': 'val', 'key2': 'val2'} + } + target = pebble.LogTarget('tgt', d) + self.assertEqual(target.name, 'tgt') + self.assertEqual(target.override, 'replace') + self.assertEqual(target.type, 'loki') + self.assertEqual(target.location, 'https://example.com:3100/loki/api/v1/push') + self.assertEqual(target.services, ['+all']) + self.assertEqual(target.labels, {'key': 'val', 'key2': 'val2'}) + + self.assertEqual(target.to_dict(), d) + + # Ensure pebble.Target has made copies of mutable objects. + assert target.services is not None and target.labels is not None + target.services[0] = '-all' + self.assertEqual(d['services'], ['+all']) + target.labels['key'] = 'val3' + assert d['labels'] is not None + self.assertEqual(d['labels']['key'], 'val') + + def test_equality(self): + d: pebble.LogTargetDict = { + 'override': 'replace', + 'type': 'loki', + 'location': 'https://example.com', + 'services': ['foo', 'bar'], + 'labels': {'k': 'v'} + } + one = pebble.LogTarget('one', d) + two = pebble.LogTarget('two', d) + self.assertEqual(one, two) + self.assertEqual(one, d) + self.assertEqual(two, d) + self.assertEqual(one, one.to_dict()) + self.assertEqual(two, two.to_dict()) + d['override'] = 'merge' + self.assertNotEqual(one, d) + self.assertNotEqual(one, 5) + + class TestServiceInfo(unittest.TestCase): def test_service_startup(self): self.assertEqual(list(pebble.ServiceStartup), [ diff --git a/test/test_real_pebble.py b/test/test_real_pebble.py index 2fb03a2bb..40ecdf222 100644 --- a/test/test_real_pebble.py +++ b/test/test_real_pebble.py @@ -259,6 +259,32 @@ def stdin_thread(): self.assertEqual(reads, [b'one\n', b'2\n', b'THREE\n']) + def test_log_forwarding(self): + self.client.add_layer("log-forwarder", { + "services": { + "tired": { + "override": "replace", + "command": "sleep 1", + }, + }, + "log-targets": { + "pretend-loki": { + "type": "loki", + "override": "replace", + "location": "https://example.com", + "services": ["all"], + "labels": {"foo": "bar"}, + }, + }, + }, combine=True) + plan = self.client.get_plan() + self.assertEqual(len(plan.log_targets), 1) + self.assertEqual(plan.log_targets["pretend-loki"].type, "loki") + self.assertEqual(plan.log_targets["pretend-loki"].override, "replace") + self.assertEqual(plan.log_targets["pretend-loki"].location, "https://example.com") + self.assertEqual(plan.log_targets["pretend-loki"].services, ["all"]) + self.assertEqual(plan.log_targets["pretend-loki"].labels, {"foo": "bar"}) + @unittest.skipUnless(os.getenv('RUN_REAL_PEBBLE_TESTS'), 'RUN_REAL_PEBBLE_TESTS not set') class TestPebbleStorageAPIsUsingRealPebble(unittest.TestCase, PebbleStorageAPIsTestMixin):