From 3f68d4175b6b2c6342aecac86c2da7bd5710d42a Mon Sep 17 00:00:00 2001 From: Stamatis Katsaounis Date: Sat, 20 Apr 2024 00:46:49 +0300 Subject: [PATCH 1/2] fix: include log-targets when adding a new layer with harness --- ops/pebble.py | 17 +++++++ ops/testing.py | 15 ++++++ test/test_testing.py | 114 +++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 146 insertions(+) diff --git a/ops/pebble.py b/ops/pebble.py index 638885f18..bf917e8cf 100644 --- a/ops/pebble.py +++ b/ops/pebble.py @@ -1117,6 +1117,23 @@ def to_dict(self) -> 'LogTargetDict': dct = {name: value for name, value in fields if value} return typing.cast('LogTargetDict', dct) + def _merge(self, other: 'LogTarget'): + """Merges this log target object with another log target definition. + + For attributes present in both objects, the passed in log target + attributes take precedence. + """ + if other.type != '': + self.type = other.type + if other.location != '': + self.location = other.location + self.services.extend(other.services) + if other.labels is not None: + if self.labels is None: + self.labels = {} + for name, value in other.labels.items(): + self.labels[name] = value + def __repr__(self): return f'LogTarget({self.to_dict()!r})' diff --git a/ops/testing.py b/ops/testing.py index 882d8b07d..5029a16fc 100644 --- a/ops/testing.py +++ b/ops/testing.py @@ -3067,6 +3067,21 @@ def add_layer( s._merge(service) else: layer.services[name] = service + for name, log_target in layer_obj.log_targets.items(): + if not log_target.override: + raise RuntimeError(f'400 Bad Request: layer "{label}" must define' + f'"override" for log target "{name}"') + if log_target.override not in ('merge', 'replace'): + raise RuntimeError(f'400 Bad Request: layer "{label}" has invalid ' + f'"override" value for log target "{name}"') + elif log_target.override == 'replace': + layer.log_targets[name] = log_target + elif log_target.override == 'merge': + if combine and name in layer.log_targets: + l_t = layer.log_targets[name] + l_t._merge(log_target) + else: + layer.log_targets[name] = log_target else: self._layers[label] = layer_obj diff --git a/test/test_testing.py b/test/test_testing.py index 96694b58a..82c153ee8 100644 --- a/test/test_testing.py +++ b/test/test_testing.py @@ -3792,6 +3792,120 @@ def test_add_layer_combine_override_unknown(self): override: blah ''', combine=True) + def test_add_layer_combine_log_targets_no_override(self): + client = self.get_testing_client() + client.add_layer('foo', pebble.Layer('''\ + log-targets: + loki-0: + override: replace + type: loki + location: loki.localhost + services: + - serv + ''')) + with pytest.raises(RuntimeError): + client.add_layer('foo', '''\ + log-targets: + loki-0: + type: loki + location: loki.localhost + services: + - serv + ''', combine=True) + + def test_add_layer_combine_log_targets_override_replace(self): + client = self.get_testing_client() + plan = client.get_plan() + assert isinstance(plan, pebble.Plan) + assert plan.to_yaml() == '{}\n' + client.add_layer('foo', pebble.Layer('''\ + log-targets: + loki-0: + override: replace + type: loki + location: loki.localhost + services: + - serv + ''')) + client.add_layer('foo', pebble.Layer('''\ + log-targets: + loki-0: + override: replace + type: loki + location: loki.localhost + services: + - all + '''), combine=True) + plan = client.get_plan() + # The YAML should be normalized + assert textwrap.dedent('''\ + log-targets: + loki-0: + location: loki.localhost + override: replace + services: + - all + type: loki + ''') == plan.to_yaml() + + def test_add_layer_combine_log_targets_override_merge(self): + client = self.get_testing_client() + plan = client.get_plan() + assert isinstance(plan, pebble.Plan) + assert plan.to_yaml() == '{}\n' + client.add_layer('foo', pebble.Layer('''\ + log-targets: + loki-0: + override: merge + type: loki + location: loki.localhost + services: + - serv + ''')) + client.add_layer('foo', pebble.Layer('''\ + log-targets: + loki-0: + override: merge + type: loki + location: loki.localhost + services: + - all + '''), combine=True) + plan = client.get_plan() + # The YAML should be normalized + assert textwrap.dedent('''\ + log-targets: + loki-0: + location: loki.localhost + override: merge + services: + - serv + - all + type: loki + ''') == plan.to_yaml() + + def test_add_layer_combine_log_targets_override_unknown(self): + client = self.get_testing_client() + client.add_layer('foo', pebble.Layer('''\ + log-targets: + loki-0: + location: loki.localhost + override: replace + services: + - foo + type: loki + ''')) + with pytest.raises(RuntimeError): + client.add_layer('foo', pebble.Layer('''\ + log-targets: + loki-0: + location: loki.localhost + override: bar + services: + - foo + type: loki + '''), combine=True) + def test_get_services_none(self): client = self.get_testing_client() service_info = client.get_services() From 96654ff6501075c2c1bb69b16d65d8b31f18241d Mon Sep 17 00:00:00 2001 From: Stamatis Katsaounis Date: Tue, 23 Apr 2024 01:06:41 +0300 Subject: [PATCH 2/2] fix: add missing checks from add_layer function --- ops/pebble.py | 68 +++++++++++++++ ops/testing.py | 15 ++++ test/test_testing.py | 201 +++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 284 insertions(+) diff --git a/ops/pebble.py b/ops/pebble.py index bf917e8cf..a308d3185 100644 --- a/ops/pebble.py +++ b/ops/pebble.py @@ -1062,6 +1062,74 @@ def to_dict(self) -> 'CheckDict': dct = {name: value for name, value in fields if value} return typing.cast('CheckDict', dct) + def _merge(self, other: 'Check'): + """Merges this check object with another check definition. + + For attributes present in both objects, the passed in check + attributes take precedence. + """ + if other.level != '': + self.level = other.level + if other.period != '': + self.period = other.period + if other.timeout != '': + self.timeout = other.timeout + if other.threshold is not None: + self.threshold = other.threshold + if other.http is not None: + if other_url := other.http.get('url'): + if self.http is None: + self.http = {} + self.http['url'] = other_url + if other_headers := other.http.get('headers'): + if self.http is None: + self.http = {'headers': {}} + for header, value in other_headers.items(): + self.http['headers'][header] = value + if other.tcp is not None: + if other_port := other.tcp.get('port'): + if self.tcp is None: + self.tcp = {} + self.tcp['port'] = other_port + if other_host := other.tcp.get('host'): + if self.tcp is None: + self.tcp = {} + self.tcp['host'] = other_host + if other.exec is not None: + if other_command := other.exec.get('command'): + if self.exec is None: + self.exec = {} + self.exec['command'] = other_command + if other_service_context := other.exec.get('service-context'): + if self.exec is None: + self.exec = {} + self.exec['service-context'] = other_service_context + if other_environment := other.exec.get('environment'): + if self.exec is None: + self.exec = {'environment': {}} + for environment, value in other_environment.items(): + self.exec['environment'][environment] = value + if other_user_id := other.exec.get('user-id'): + if self.exec is None: + self.exec = {} + self.exec['user-id'] = other_user_id + if other_user := other.exec.get('user'): + if self.exec is None: + self.exec = {} + self.exec['user'] = other_user + if other_group_id := other.exec.get('group-id'): + if self.exec is None: + self.exec = {} + self.exec['group-id'] = other_group_id + if other_group := other.exec.get('group'): + if self.exec is None: + self.exec = {} + self.exec['group'] = other_group + if other_working_dir := other.exec.get('working-dir'): + if self.exec is None: + self.exec = {} + self.exec['working-dir'] = other_working_dir + def __repr__(self) -> str: return f'Check({self.to_dict()!r})' diff --git a/ops/testing.py b/ops/testing.py index 5029a16fc..096aa5d23 100644 --- a/ops/testing.py +++ b/ops/testing.py @@ -3082,6 +3082,21 @@ def add_layer( l_t._merge(log_target) else: layer.log_targets[name] = log_target + for name, check in layer_obj.checks.items(): + if not check.override: + raise RuntimeError(f'400 Bad Request: layer "{label}" must define' + f'"override" for check "{name}"') + if check.override not in ('merge', 'replace'): + raise RuntimeError(f'400 Bad Request: layer "{label}" has invalid ' + f'"override" value for check "{name}"') + elif check.override == 'replace': + layer.checks[name] = check + elif check.override == 'merge': + if combine and name in layer.checks: + c = layer.checks[name] + c._merge(check) + else: + layer.checks[name] = check else: self._layers[label] = layer_obj diff --git a/test/test_testing.py b/test/test_testing.py index 82c153ee8..6baa8695e 100644 --- a/test/test_testing.py +++ b/test/test_testing.py @@ -3906,6 +3906,207 @@ def test_add_layer_combine_log_targets_override_unknown(self): type: loki '''), combine=True) + def test_add_layer_combine_checks_no_override(self): + client = self.get_testing_client() + client.add_layer('foo', pebble.Layer('''\ + checks: + check-0: + override: replace + http: + url: svc/check + ''')) + with pytest.raises(RuntimeError): + client.add_layer('foo', '''\ + checks: + check-0: + http: + url: svc/check + ''', combine=True) + + def test_add_layer_combine_checks_override_replace(self): + client = self.get_testing_client() + plan = client.get_plan() + assert isinstance(plan, pebble.Plan) + assert plan.to_yaml() == '{}\n' + client.add_layer('foo', pebble.Layer('''\ + checks: + check-0: + override: replace + http: + url: svc/check + headers: + Connection: keep-alive + check-1: + override: replace + tcp: + port: 9000 + host: foo.bar + check-2: + override: replace + exec: + command: ls -l + service-context: foo + environment: + FOO: bar + user: foo + user-id: 42 + group: bar + group-id: 24 + working-dir: /tmp + ''')) + client.add_layer('foo', pebble.Layer('''\ + checks: + check-0: + override: replace + http: + url: svc/check + headers: + Connection: close + check-1: + override: replace + tcp: + port: 8080 + host: bar.foo + check-2: + override: replace + exec: + command: cat /var/log/syslog + service-context: bar + environment: + FOO: bar + FOO_FOO: bar_bar + user: foobar + user-id: 420 + group: barfoo + group-id: 240 + working-dir: /var + '''), combine=True) + plan = client.get_plan() + # The YAML should be normalized + assert textwrap.dedent('''\ + checks: + check-0: + http: + headers: + Connection: close + url: svc/check + override: replace + check-1: + override: replace + tcp: + host: bar.foo + port: 8080 + check-2: + exec: + command: cat /var/log/syslog + environment: + FOO: bar + FOO_FOO: bar_bar + group: barfoo + group-id: 240 + service-context: bar + user: foobar + user-id: 420 + working-dir: /var + override: replace + ''') == plan.to_yaml() + + def test_add_layer_combine_checks_override_merge(self): + client = self.get_testing_client() + plan = client.get_plan() + assert isinstance(plan, pebble.Plan) + assert plan.to_yaml() == '{}\n' + client.add_layer('foo', pebble.Layer('''\ + checks: + check-0: + level: alive + http: + headers: + Connection: close + url: svc/check + override: merge + check-1: + exec: + command: cat /var/log/syslog + environment: + FOO: bar + group: barfoo + group-id: 240 + service-context: bar + user: foobar + user-id: 420 + working-dir: /var + override: merge + ''')) + client.add_layer('foo', pebble.Layer('''\ + checks: + check-0: + level: ready + http: + headers: + Connection: keep-alive + Accept: text/html + url: svc/check + override: merge + check-1: + exec: + command: ls -l /tmp/ + environment: + FOO: foo + BAR: foo_bar + group: barfoo + group-id: 31 + service-context: bar + user: foobar + user-id: 13 + working-dir: /var + override: merge + '''), combine=True) + plan = client.get_plan() + # The YAML should be normalized + assert textwrap.dedent('''\ + checks: + check-0: + http: + headers: + Accept: text/html + Connection: keep-alive + url: svc/check + level: ready + override: merge + check-1: + exec: + command: ls -l /tmp/ + environment: + BAR: foo_bar + FOO: foo + group: barfoo + group-id: 31 + service-context: bar + user: foobar + user-id: 13 + working-dir: /var + override: merge + ''') == plan.to_yaml() + + def test_add_layer_combine_checks_override_unknown(self): + client = self.get_testing_client() + client.add_layer('foo', pebble.Layer('''\ + checks: + check-0: + override: replace + http: + url: svc/check + ''')) + with pytest.raises(RuntimeError): + client.add_layer('foo', pebble.Layer('''\ + checks: + check-0: + override: foobar + http: + url: svc/check + '''), combine=True) + def test_get_services_none(self): client = self.get_testing_client() service_info = client.get_services()