diff --git a/charmcraft-22.04.yaml b/charmcraft-22.04.yaml index 5227a39..29d9de3 100644 --- a/charmcraft-22.04.yaml +++ b/charmcraft-22.04.yaml @@ -119,6 +119,12 @@ peers: config: options: + classic_snap: + description: | + Choose whether to use the classic snap over the strictly confined + one. Defaults to "true". + type: boolean + default: true tls_insecure_skip_verify: description: | Flag to skip the verification for insecure TLS. diff --git a/pyproject.toml b/pyproject.toml index 7ab48a5..f4e606f 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -8,25 +8,21 @@ branch = true [tool.coverage.report] show_missing = true -# Formatting tools configuration -[tool.black] -line-length = 99 -target-version = ["py38"] - # Linting tools configuration [tool.ruff] line-length = 99 exclude = ["__pycache__", "*.egg_info"] + +[tool.ruff.lint] select = ["E", "W", "F", "C", "N", "R", "D", "I001"] -# Ignore E501 because using black creates errors with this # Ignore D107 Missing docstring in __init__ -ignore = ["C901", "E501", "D107", "RET504"] +ignore = ["C901", "D107", "E501", "RET504"] -[tool.ruff.per-file-ignores] +[tool.ruff.lint.per-file-ignores] # D100, D101, D102, D103: Ignore missing docstrings in tests "tests/*" = ["D100","D101","D102","D103"] -[tool.ruff.pydocstyle] +[tool.ruff.lint.pydocstyle] convention = "google" # Static analysis tools configuration diff --git a/src/charm.py b/src/charm.py index 06f6d05..5787ec1 100755 --- a/src/charm.py +++ b/src/charm.py @@ -4,6 +4,7 @@ # See LICENSE file for licensing details. """A juju charm for Grafana Agent on Kubernetes.""" + import logging import os import re @@ -12,6 +13,7 @@ from pathlib import Path from typing import Any, Dict, List, Optional, Set, Union, get_args +import yaml from charms.grafana_agent.v0.cos_agent import COSAgentRequirer, ReceiverProtocol from charms.operator_libs_linux.v2 import snap # type: ignore from charms.tempo_k8s.v1.charm_tracing import trace_charm @@ -186,7 +188,8 @@ def __init__(self, *args): self._on_cos_data_changed, ) self.framework.observe( - self._cos.on.validation_error, self._on_cos_validation_error # pyright: ignore + self._cos.on.validation_error, + self._on_cos_validation_error, # pyright: ignore ) self.framework.observe(self.on["juju_info"].relation_joined, self._on_juju_info_joined) self.framework.observe(self.on.install, self.on_install) @@ -226,6 +229,14 @@ def _on_cos_validation_error(self, event): self._update_status() + def _verify_snap_track(self) -> None: + try: + # install_ga_snap calls snap.ensure so it should do the right thing whether the track + # changes or not. + install_ga_snap(classic=bool(self.config["classic_snap"])) + except (snap.SnapError, SnapSpecError) as e: + raise GrafanaAgentInstallError("Failed to refresh grafana-agent.") from e + def on_install(self, _event) -> None: """Install the Grafana Agent snap.""" self._install() @@ -234,7 +245,7 @@ def _install(self) -> None: """Install/refresh the Grafana Agent snap.""" self.unit.status = MaintenanceStatus("Installing grafana-agent snap") try: - install_ga_snap(classic=False) + install_ga_snap(classic=bool(self.config["classic_snap"])) except (snap.SnapError, SnapSpecError) as e: raise GrafanaAgentInstallError("Failed to install grafana-agent.") from e @@ -402,7 +413,9 @@ def _additional_integrations(self) -> Dict[str, Any]: ) return { "node_exporter": { - "rootfs_path": "/var/lib/snapd/hostfs", + "rootfs_path": "/" + if bool(self.config["classic_snap"]) + else "/var/lib/snapd/hostfs", "enabled": True, "enable_collectors": [ "logind", @@ -509,73 +522,158 @@ def relabeling_config(self) -> list: } ] + topology_relabels # type: ignore + def _evaluate_log_paths(self, paths: List[str], snap: str, app: str) -> List[str]: + """Evaluate each log path using snap to resolve environment variables. + + Raises: + Exception: If echo fails. + """ + # There is a potential for shell injection here. It seems okay because the potential + # attacking charm has root access on the machine already anyway. + new_paths = [] + for path in paths: + cmd = f"echo 'echo {path}' | snap run --shell {snap}.{app}" + p = subprocess.run(cmd, shell=True, capture_output=True, text=True) + if p.returncode != 0: + raise Exception( + f"Failed to evaluate path with command: {cmd}\nSTDOUT: {p.stdout}\nSTDERR: {p.stderr}" + ) + new_paths.append(p.stdout.strip()) + return new_paths + + def _snap_plug_job( + self, owner: str, target_path: str, app: str, unit: str, label_path: str + ) -> dict: + job_name = f"{owner}-{label_path.replace('/', '-')}" + job = { + "job_name": job_name, + "static_configs": [ + { + "targets": ["localhost"], + "labels": { + "job": job_name, + "__path__": target_path, + **{ # from grafana-agent's topology + k: v + for k, v in self._instance_topology.items() + if k not in ["juju_unit", "juju_application"] + }, + # from the topology of the charm owning the snap + "juju_application": app, + "juju_unit": unit, + "snap_name": owner, + }, + } + ], + "pipeline_stages": [ + { + "drop": { + "expression": ".*file is a directory.*", + }, + }, + ], + } + + job["relabel_configs"] = [ + { + "source_labels": ["__path__"], + "target_label": "path", + "replacement": label_path if label_path.startswith("/") else f"/{label_path}", + } + ] + return job + + def _path_label(self, path): + """Best effort at figuring out what the path label should be. + + Try to make the path reflect what it would normally be with a non snap version of the + software. + """ + match = re.match("^.*(var/log/.*$)", path) + if match: + return match.group(1) + match = re.match("^/var/snap/.*/common/(.*)$", path) + if match: + return match.group(1) + # We couldn't figure it out so just use the full path. + return path + @property def _snap_plugs_logging_configs(self) -> List[Dict[str, Any]]: """One logging config for each separate snap connected over the "logs" endpoint.""" agent_fstab = SnapFstab(Path("/var/lib/snapd/mount/snap.grafana-agent.fstab")) - shared_logs_configs = [] - endpoint_owners = { - endpoint.owner: {"juju_application": topology.application, "juju_unit": topology.unit} - for endpoint, topology in self._cos.snap_log_endpoints_with_topology - } - for fstab_entry in agent_fstab.entries: - if fstab_entry.owner not in endpoint_owners.keys(): - continue - target_path = ( - f"{fstab_entry.target}/**" - if fstab_entry - else "/snap/grafana-agent/current/shared-logs/**" - ) - job_name = f"{fstab_entry.owner}-{fstab_entry.endpoint_source.replace('/', '-')}" - job = { - "job_name": job_name, - "static_configs": [ - { - "targets": ["localhost"], - "labels": { - "job": job_name, - "__path__": target_path, - **{ # from grafana-agent's topology - k: v - for k, v in self._instance_topology.items() - if k not in ["juju_unit", "juju_application"] - }, - # from the topology of the charm owning the snap - **endpoint_owners[fstab_entry.owner], - "snap_name": fstab_entry.owner, - }, - } - ], - "pipeline_stages": [ - { - "drop": { - "expression": ".*file is a directory.*", - }, - }, - ], - } + if self.config["classic_snap"]: + # Iterate through each logging endpoint. + for endpoint, topology in self._cos.snap_log_endpoints_with_topology: + try: + with open(f"/snap/{endpoint.owner}/current/meta/snap.yaml") as f: + snap_yaml = yaml.safe_load(f) + except FileNotFoundError: + logger.error( + f"snap file for {endpoint.owner} not found. It is likely not installed. Skipping." + ) + continue + # Get the directories we need to monitor. + log_dirs = snap_yaml["slots"][endpoint.name]["source"]["read"] + for key in snap_yaml["apps"].keys(): + snap_app_name = key # Just use any app. + break + # Evaluate any variables in the paths. + log_dirs = self._evaluate_log_paths( + paths=log_dirs, snap=endpoint.owner, app=snap_app_name + ) + # Create a job for each path. + for path in log_dirs: + job = self._snap_plug_job( + endpoint.owner, + f"{path}/**", + topology.application, + str(topology.unit), + self._path_label(path), + ) + shared_logs_configs.append(job) - job["relabel_configs"] = [ - { - "source_labels": ["__path__"], - "target_label": "path", - "replacement": fstab_entry.relative_target, + else: + endpoint_owners = { + endpoint.owner: { + "juju_application": topology.application, + "juju_unit": topology.unit, } - ] - - shared_logs_configs.append(job) + for endpoint, topology in self._cos.snap_log_endpoints_with_topology + } + for fstab_entry in agent_fstab.entries: + if fstab_entry.owner not in endpoint_owners.keys(): + continue + + target_path = ( + f"{fstab_entry.target}/**" + if fstab_entry + else "/snap/grafana-agent/current/shared-logs/**" + ) + + job = self._snap_plug_job( + fstab_entry.owner, + target_path, + endpoint_owners[fstab_entry.owner]["juju_application"], + endpoint_owners[fstab_entry.owner]["juju_unit"], + fstab_entry.relative_target, + ) + shared_logs_configs.append(job) return shared_logs_configs def _connect_logging_snap_endpoints(self): - for plug in self._cos.snap_log_endpoints: - try: - self.snap.connect("logs", service=plug.owner, slot=plug.name) - except snap.SnapError as e: - logger.error(f"error connecting plug {plug} to grafana-agent:logs") - logger.error(e.message) + # We need to run _verify_snap_track so we make sure we have refreshed BEFORE connecting. + self._verify_snap_track() + if not self.config["classic_snap"]: + for plug in self._cos.snap_log_endpoints: + try: + self.snap.connect("logs", service=plug.owner, slot=plug.name) + except snap.SnapError as e: + logger.error(f"error connecting plug {plug} to grafana-agent:logs") + logger.error(e.message) def positions_dir(self) -> str: """Return the positions directory.""" diff --git a/src/grafana_agent.py b/src/grafana_agent.py index 023e9c0..5e46bba 100644 --- a/src/grafana_agent.py +++ b/src/grafana_agent.py @@ -2,6 +2,7 @@ # See LICENSE file for licensing details. """Common logic for both k8s and machine charms for Grafana Agent.""" + import json import logging import os @@ -270,6 +271,7 @@ def _on_loki_push_api_endpoint_departed(self, _event=None): def _on_config_changed(self, _event=None): """Rebuild the config.""" + self._verify_snap_track() self._update_config() self._update_status() @@ -296,6 +298,9 @@ def _on_cert_transfer_removed(self, event: CertificateTransferRemovedEvent): self.run(["update-ca-certificates", "--fresh"]) # Abstract Methods + def _verify_snap_track(self) -> None: + raise NotImplementedError("Please override the _verify_snap_track method") + @property def is_k8s(self) -> bool: """Is this a k8s charm.""" @@ -608,9 +613,7 @@ def _delete_file_if_exists(self, file_path): def _on_dashboard_status_changed(self, _event=None): """Re-initialize dashboards to forward.""" # TODO: add constructor arg for `inject_dropdowns=False` instead of 'private' method? - self._grafana_dashboards_provider._reinitialize_dashboard_data( - inject_dropdowns=False - ) # noqa + self._grafana_dashboards_provider._reinitialize_dashboard_data(inject_dropdowns=False) # noqa self._update_status() def _enhance_endpoints_with_tls(self, endpoints) -> List[Dict[str, Any]]: @@ -922,12 +925,12 @@ def _loki_config(self) -> Dict[str, Union[Any, List[Any]]]: for config in configs: for scrape_config in config.get("scrape_configs", []): if scrape_config.get("loki_push_api"): - scrape_config["loki_push_api"]["server"][ - "http_tls_config" - ] = self.tls_config - scrape_config["loki_push_api"]["server"][ - "grpc_tls_config" - ] = self.tls_config + scrape_config["loki_push_api"]["server"]["http_tls_config"] = ( + self.tls_config + ) + scrape_config["loki_push_api"]["server"]["grpc_tls_config"] = ( + self.tls_config + ) configs.extend(self._additional_log_configs) # type: ignore return ( diff --git a/src/snap_management.py b/src/snap_management.py index 40b5d49..9cf7201 100644 --- a/src/snap_management.py +++ b/src/snap_management.py @@ -10,9 +10,9 @@ Modified from https://github.com/canonical/k8s-operator/blob/main/charms/worker/k8s/src/snap.py """ - import logging import platform +import subprocess import charms.operator_libs_linux.v2.snap as snap_lib @@ -26,6 +26,8 @@ # (confinement, arch): revision ("strict", "amd64"): 51, # 0.40.4 ("strict", "arm64"): 52, # 0.40.4 + ("classic", "amd64"): 82, # 0.40.4 + ("classic", "arm64"): 83, # 0.40.4 } @@ -63,7 +65,20 @@ def _install_snap( f"Ensuring {name} snap is installed at revision={revision}" f" with classic confinement={classic}" ) - snap.ensure(state=snap_lib.SnapState.Present, revision=revision, classic=classic) + # snap.ensure(state=snap_lib.SnapState.Present, revision=revision, classic=classic) + # Currently, snap.ensure does not properly use the classic flag. Use the commented line above + # instead of the below code once the issue is resolved. + # https://github.com/canonical/operator-libs-linux/issues/129 + if snap.present: + if snap.revision != revision: + cmd = ["snap", "refresh", "grafana-agent", f'--revision="{revision}"'] + if classic: + cmd.append("--classic") + subprocess.run(cmd) + snap.start(enable=True) + else: + snap.ensure(state=snap_lib.SnapState.Present, revision=revision, classic=classic) + snap.hold() diff --git a/tests/integration/test_classic_config_flag.py b/tests/integration/test_classic_config_flag.py new file mode 100644 index 0000000..507f712 --- /dev/null +++ b/tests/integration/test_classic_config_flag.py @@ -0,0 +1,94 @@ +# Copyright 2023 Canonical Ltd. +# See LICENSE file for licensing details. + +import logging +from types import SimpleNamespace +from typing import List + +import pytest +from juju.errors import JujuError +from pytest_operator.plugin import OpsTest + +agent = SimpleNamespace(name="agent") +principal = SimpleNamespace(charm="ubuntu", name="principal") + +logger = logging.getLogger(__name__) + + +async def ssh(ops_test, app_name: str, command: str) -> List[str]: + """Run a command in all units of the given apps and return all the outputs.""" + unit = ops_test.model.applications[app_name].units[0] + try: + return await unit.ssh(command) + except JujuError as e: + pytest.fail(f"Failed to run ssh command '{command}' in {app_name}: {e.message}") + + +@pytest.mark.abort_on_fail +async def test_build_and_deploy(ops_test: OpsTest, grafana_agent_charm): + # Principal + await ops_test.model.deploy(principal.charm, application_name=principal.name, series="jammy") + + # Workaround: `charmcraft pack` produces two files, but `ops_test.build_charm` returns the + # first one. Since primary is jammy, we know we need to deploy the jammy grafana agent charm, + # otherwise we'd get an error such as: + # ImportError: libssl.so.1.1: cannot open shared object file: No such file or directory + jammy_charm_path = grafana_agent_charm.parent / "grafana-agent_ubuntu-22.04-amd64.charm" + + # Subordinate + await ops_test.model.deploy( + jammy_charm_path, application_name=agent.name, num_units=0, series="jammy" + ) + + # Placeholder for o11y relations (otherwise grafana agent charm is in blocked status) + await ops_test.model.deploy( + "grafana-cloud-integrator", + application_name="gci", + num_units=1, + series="focal", + channel="edge", + ) + + await ops_test.model.integrate("agent:juju-info", principal.name) + await ops_test.model.integrate("agent:grafana-cloud-config", "gci") + await ops_test.model.wait_for_idle(apps=[principal.name, agent.name], status="active") + + +@pytest.mark.abort_on_fail +async def test_classic_by_default(ops_test: OpsTest): + # WHEN the charm is related to a principal over `juju-info` + + # THEN all units of the principal have the charm in 'enabled/active' state + # $ juju ssh agent/0 snap services grafana-agent + # Service Startup Current Notes + # grafana-agent.grafana-agent enabled active - + out = await ssh(ops_test, agent.name, "snap info --verbose grafana-agent | grep confinement") + assert out.split()[1] == "classic" + + +@pytest.mark.abort_on_fail +async def test_switch_to_strict(ops_test: OpsTest): + # WHEN the charm is related to a principal over `juju-info` + + # THEN all units of the principal have the charm in 'enabled/active' state + # $ juju ssh agent/0 snap services grafana-agent + # Service Startup Current Notes + # grafana-agent.grafana-agent enabled active - + await ops_test.model.applications[agent.name].set_config({"classic_snap": "false"}) + await ops_test.model.wait_for_idle(apps=[principal.name, agent.name], status="active") + out = await ssh(ops_test, agent.name, "snap info --verbose grafana-agent | grep confinement") + assert out.split()[1] == "strict" + + +@pytest.mark.abort_on_fail +async def test_switch_to_classic(ops_test: OpsTest): + # WHEN the charm is related to a principal over `juju-info` + + # THEN all units of the principal have the charm in 'enabled/active' state + # $ juju ssh agent/0 snap services grafana-agent + # Service Startup Current Notes + # grafana-agent.grafana-agent enabled active - + await ops_test.model.applications[agent.name].set_config({"classic_snap": "true"}) + await ops_test.model.wait_for_idle(apps=[principal.name, agent.name], status="active") + out = await ssh(ops_test, agent.name, "snap info --verbose grafana-agent | grep confinement") + assert out.split()[1] == "classic" diff --git a/tests/integration/test_juju_info_cos_agent.py b/tests/integration/test_juju_info_cos_agent.py index 9a2c2b2..766574d 100644 --- a/tests/integration/test_juju_info_cos_agent.py +++ b/tests/integration/test_juju_info_cos_agent.py @@ -80,10 +80,10 @@ async def test_build_and_deploy(ops_test: OpsTest, grafana_agent_charm): @pytest.mark.abort_on_fail async def test_service(ops_test: OpsTest): # WHEN the charm is related to a principal over `juju-info` - await ops_test.model.add_relation("agent:juju-info", principal.name) - await ops_test.model.add_relation("hwo:general-info", principal.name) - await ops_test.model.add_relation("hwo:cos-agent", "agent:cos-agent") - await ops_test.model.add_relation("agent:grafana-cloud-config", "gci") + await ops_test.model.integrate("agent:juju-info", principal.name) + await ops_test.model.integrate("hwo:general-info", principal.name) + await ops_test.model.integrate("hwo:cos-agent", "agent:cos-agent") + await ops_test.model.integrate("agent:grafana-cloud-config", "gci") await ops_test.model.wait_for_idle(apps=[principal.name, agent.name], status="active") # THEN all units of the principal have the charm in 'enabled/active' state diff --git a/tests/scenario/test_machine_charm/conftest.py b/tests/scenario/test_machine_charm/conftest.py index 3072bbf..78169f3 100644 --- a/tests/scenario/test_machine_charm/conftest.py +++ b/tests/scenario/test_machine_charm/conftest.py @@ -1,6 +1,6 @@ # Copyright 2022 Canonical Ltd. # See LICENSE file for licensing details. -from unittest.mock import patch +from unittest.mock import PropertyMock, patch import pytest @@ -14,3 +14,28 @@ def placeholder_cfg_path(tmp_path): def mock_config_path(placeholder_cfg_path): with patch("grafana_agent.CONFIG_PATH", placeholder_cfg_path): yield + + +@pytest.fixture(autouse=True) +def mock_snap(): + """Mock the charm's snap property so we don't access the host.""" + with patch("charm.GrafanaAgentMachineCharm.snap", new_callable=PropertyMock): + yield + + +@pytest.fixture(autouse=True) +def mock_refresh(): + """Mock the refresh call so we don't access the host.""" + with patch("snap_management._install_snap", new_callable=PropertyMock): + yield + + +CONFIG_MATRIX = [ + {"classic_snap": True}, + {"classic_snap": False}, +] + + +@pytest.fixture(params=CONFIG_MATRIX) +def charm_config(request): + return request.param diff --git a/tests/scenario/test_machine_charm/test_alert_labels.py b/tests/scenario/test_machine_charm/test_alert_labels.py index d52eb4e..945722a 100644 --- a/tests/scenario/test_machine_charm/test_alert_labels.py +++ b/tests/scenario/test_machine_charm/test_alert_labels.py @@ -2,13 +2,11 @@ # See LICENSE file for licensing details. import json -from unittest.mock import PropertyMock, patch import pytest from scenario import Context, PeerRelation, Relation, State, SubordinateRelation import charm -from tests.scenario.helpers import get_charm_meta @pytest.fixture(autouse=True) @@ -17,14 +15,7 @@ def use_mock_config_path(mock_config_path): yield -@pytest.fixture(autouse=True) -def mock_snap(): - """Mock the charm's snap property so we don't access the host.""" - with patch("charm.GrafanaAgentMachineCharm.snap", new_callable=PropertyMock): - yield - - -def test_metrics_alert_rule_labels(vroot): +def test_metrics_alert_rule_labels(vroot, charm_config): """Check that metrics alert rules are labeled with principal topology.""" cos_agent_primary_data = { "config": json.dumps( @@ -108,7 +99,6 @@ def test_metrics_alert_rule_labels(vroot): context = Context( charm_type=charm.GrafanaAgentMachineCharm, - meta=get_charm_meta(charm.GrafanaAgentMachineCharm), charm_root=vroot, ) state = State( @@ -119,12 +109,11 @@ def test_metrics_alert_rule_labels(vroot): remote_write_relation, PeerRelation("peers"), ], + config=charm_config, ) state_0 = context.run(event=cos_agent_primary_relation.changed_event, state=state) - (vroot / "charmcraft.yaml").unlink(missing_ok=True) state_1 = context.run(event=cos_agent_subordinate_relation.changed_event, state=state_0) - (vroot / "charmcraft.yaml").unlink(missing_ok=True) state_2 = context.run(event=remote_write_relation.joined_event, state=state_1) alert_rules = json.loads(state_2.relations[2].local_app_data["alert_rules"]) diff --git a/tests/scenario/test_machine_charm/test_multiple_subordinates.py b/tests/scenario/test_machine_charm/test_multiple_subordinates.py index c8b4413..cf1e0c5 100644 --- a/tests/scenario/test_machine_charm/test_multiple_subordinates.py +++ b/tests/scenario/test_machine_charm/test_multiple_subordinates.py @@ -2,13 +2,11 @@ # See LICENSE file for licensing details. import json -from unittest.mock import PropertyMock, patch import pytest from scenario import Context, PeerRelation, State, SubordinateRelation import charm -from tests.scenario.helpers import get_charm_meta @pytest.fixture(autouse=True) @@ -17,14 +15,7 @@ def use_mock_config_path(mock_config_path): yield -@pytest.fixture(autouse=True) -def mock_snap(): - """Mock the charm's snap property so we don't access the host.""" - with patch("charm.GrafanaAgentMachineCharm.snap", new_callable=PropertyMock): - yield - - -def test_juju_info_and_cos_agent(vroot): +def test_juju_info_and_cos_agent(vroot, charm_config): def post_event(charm: charm.GrafanaAgentMachineCharm): assert len(charm._cos.dashboards) == 1 assert len(charm._cos.snap_log_endpoints) == 1 @@ -56,7 +47,6 @@ def post_event(charm: charm.GrafanaAgentMachineCharm): context = Context( charm_type=charm.GrafanaAgentMachineCharm, - meta=get_charm_meta(charm.GrafanaAgentMachineCharm), charm_root=vroot, ) state = State( @@ -64,12 +54,13 @@ def post_event(charm: charm.GrafanaAgentMachineCharm): cos_agent_relation, SubordinateRelation("juju-info", remote_app_name="remote-juju-info"), PeerRelation("peers"), - ] + ], + config=charm_config, ) context.run(event=cos_agent_relation.changed_event, state=state, post_event=post_event) -def test_two_cos_agent_relations(vroot): +def test_two_cos_agent_relations(vroot, charm_config): def post_event(charm: charm.GrafanaAgentMachineCharm): assert len(charm._cos.dashboards) == 2 assert len(charm._cos.snap_log_endpoints) == 2 @@ -122,7 +113,6 @@ def post_event(charm: charm.GrafanaAgentMachineCharm): context = Context( charm_type=charm.GrafanaAgentMachineCharm, - meta=get_charm_meta(charm.GrafanaAgentMachineCharm), charm_root=vroot, ) state = State( @@ -130,7 +120,8 @@ def post_event(charm: charm.GrafanaAgentMachineCharm): cos_agent_primary_relation, cos_agent_subordinate_relation, PeerRelation("peers"), - ] + ], + config=charm_config, ) out_state = context.run(event=cos_agent_primary_relation.changed_event, state=state) vroot.clean() diff --git a/tests/scenario/test_machine_charm/test_relation_priority.py b/tests/scenario/test_machine_charm/test_relation_priority.py index 0d08286..bb1907b 100644 --- a/tests/scenario/test_machine_charm/test_relation_priority.py +++ b/tests/scenario/test_machine_charm/test_relation_priority.py @@ -9,14 +9,12 @@ from scenario import Context, PeerRelation, State, SubordinateRelation import charm -from tests.scenario.helpers import get_charm_meta from tests.scenario.test_machine_charm.helpers import set_run_out def trigger(evt: str, state: State, vroot: Path = None, **kwargs): context = Context( charm_type=charm.GrafanaAgentMachineCharm, - meta=get_charm_meta(charm.GrafanaAgentMachineCharm), charm_root=vroot, ) return context.run(event=evt, state=state, **kwargs) @@ -34,7 +32,7 @@ def patch_all(placeholder_cfg_path): @patch("charm.subprocess.run") -def test_no_relations(mock_run, vroot): +def test_no_relations(mock_run, vroot, charm_config): def post_event(charm: charm.GrafanaAgentMachineCharm): assert not charm._cos.dashboards assert not charm._cos.logs_alerts @@ -43,11 +41,11 @@ def post_event(charm: charm.GrafanaAgentMachineCharm): assert not charm._cos.snap_log_endpoints set_run_out(mock_run, 0) - trigger("start", State(), post_event=post_event, vroot=vroot) + trigger("start", State(config=charm_config), post_event=post_event, vroot=vroot) @patch("charm.subprocess.run") -def test_juju_info_relation(mock_run, vroot): +def test_juju_info_relation(mock_run, vroot, charm_config): def post_event(charm: charm.GrafanaAgentMachineCharm): assert not charm._cos.dashboards assert not charm._cos.logs_alerts @@ -63,7 +61,8 @@ def post_event(charm: charm.GrafanaAgentMachineCharm): SubordinateRelation( "juju-info", remote_unit_data={"config": json.dumps({"subordinate": True})} ) - ] + ], + config=charm_config, ), post_event=post_event, vroot=vroot, @@ -71,7 +70,7 @@ def post_event(charm: charm.GrafanaAgentMachineCharm): @patch("charm.subprocess.run") -def test_cos_machine_relation(mock_run, vroot): +def test_cos_machine_relation(mock_run, vroot, charm_config): def post_event(charm: charm.GrafanaAgentMachineCharm): assert charm._cos.dashboards assert charm._cos.snap_log_endpoints @@ -120,7 +119,8 @@ def post_event(charm: charm.GrafanaAgentMachineCharm): remote_unit_data=cos_agent_data, ), PeerRelation("peers", peers_data={1: peer_data}), - ] + ], + config=charm_config, ), post_event=post_event, vroot=vroot, @@ -128,7 +128,7 @@ def post_event(charm: charm.GrafanaAgentMachineCharm): @patch("charm.subprocess.run") -def test_both_relations(mock_run, vroot): +def test_both_relations(mock_run, vroot, charm_config): def post_event(charm: charm.GrafanaAgentMachineCharm): assert charm._cos.dashboards assert charm._cos.snap_log_endpoints @@ -170,7 +170,6 @@ def post_event(charm: charm.GrafanaAgentMachineCharm): context = Context( charm_type=charm.GrafanaAgentMachineCharm, - meta=get_charm_meta(charm.GrafanaAgentMachineCharm), charm_root=vroot, ) state = State( @@ -182,6 +181,7 @@ def post_event(charm: charm.GrafanaAgentMachineCharm): ), SubordinateRelation("juju-info", remote_app_name="remote-juju-info"), PeerRelation("peers", peers_data={1: peer_data}), - ] + ], + config=charm_config, ) context.run(event="start", state=state, post_event=post_event) diff --git a/tests/scenario/test_machine_charm/test_scrape_configs.py b/tests/scenario/test_machine_charm/test_scrape_configs.py index 383d661..075ab64 100644 --- a/tests/scenario/test_machine_charm/test_scrape_configs.py +++ b/tests/scenario/test_machine_charm/test_scrape_configs.py @@ -30,7 +30,7 @@ def patch_all(placeholder_cfg_path): @pytest.mark.skip(reason="can't parse a custom fstab file") -def test_snap_endpoints(placeholder_cfg_path): +def test_snap_endpoints(placeholder_cfg_path, charm_config): written_path, written_text = "", "" def mock_write(_, path, text): @@ -71,6 +71,7 @@ def mock_write(_, path, text): state = State( relations=[cos_relation, loki_relation, PeerRelation("peers")], model=Model(name="my-model", uuid=my_uuid), + config=charm_config, ) ctx = Context( diff --git a/tests/scenario/test_machine_charm/test_tracing_configuration.py b/tests/scenario/test_machine_charm/test_tracing_configuration.py index b0c11f4..00e4e11 100644 --- a/tests/scenario/test_machine_charm/test_tracing_configuration.py +++ b/tests/scenario/test_machine_charm/test_tracing_configuration.py @@ -13,13 +13,15 @@ def test_cos_agent_receiver_protocols_match_with_tracing(): @pytest.mark.parametrize("protocol", get_args(TracingReceiverProtocol)) -def test_always_enable_config_variables_are_generated_for_tracing_protocols(protocol, vroot): +def test_always_enable_config_variables_are_generated_for_tracing_protocols( + protocol, vroot, mock_config_path, charm_config +): context = Context( charm_type=GrafanaAgentMachineCharm, charm_root=vroot, ) state = State( - config={f"always_enable_{protocol}": True}, + config={f"always_enable_{protocol}": True, **charm_config}, leader=True, relations=[], ) diff --git a/tests/unit/test_cloud_integration.py b/tests/unit/test_cloud_integration.py index 06030ff..8231c79 100644 --- a/tests/unit/test_cloud_integration.py +++ b/tests/unit/test_cloud_integration.py @@ -30,6 +30,10 @@ def setUp(self, *unused): self.mock_install = patcher.start() self.addCleanup(patcher.stop) + patcher = patch.object(GrafanaAgentCharm, "_verify_snap_track") + self.mock_verify_snap_track = patcher.start() + self.addCleanup(patcher.stop) + def test_prometheus_remote_write_config_with_grafana_cloud_integrator(self): """Asserts that the prometheus remote write config is written correctly for leaders and non-leaders.""" for leader in (True, False): diff --git a/tests/unit/test_relation_status.py b/tests/unit/test_relation_status.py index 4141303..c200cdd 100644 --- a/tests/unit/test_relation_status.py +++ b/tests/unit/test_relation_status.py @@ -31,6 +31,10 @@ def setUp(self, *unused): self.mock_install = patcher.start() self.addCleanup(patcher.stop) + patcher = patch.object(GrafanaAgentCharm, "_verify_snap_track") + self.mock_verify_snap_track = patcher.start() + self.addCleanup(patcher.stop) + self.harness = Harness(GrafanaAgentCharm) self.harness.set_model_name(self.__class__.__name__) diff --git a/tests/unit/test_update_status.py b/tests/unit/test_update_status.py index 2d6c885..dfa8add 100644 --- a/tests/unit/test_update_status.py +++ b/tests/unit/test_update_status.py @@ -30,6 +30,10 @@ def setUp(self, *unused): self.mock_install = patcher.start() self.addCleanup(patcher.stop) + patcher = patch.object(GrafanaAgentCharm, "_verify_snap_track") + self.mock_verify_snap_track = patcher.start() + self.addCleanup(patcher.stop) + self.harness = Harness(GrafanaAgentCharm) self.harness.set_model_name(self.__class__.__name__) diff --git a/tox.ini b/tox.ini index 567f8c4..cc6de4b 100644 --- a/tox.ini +++ b/tox.ini @@ -18,35 +18,24 @@ setenv = PYTHONBREAKPOINT=ipdb.set_trace PY_COLORS=1 skip_install=True -#passenv = -# PYTHONPATH -# HOME -# PATH -# CHARM_BUILD_DIR -# MODEL_SETTINGS -# HTTP_PROXY -# HTTPS_PROXY -# NO_PROXY [testenv:fmt] description = Apply coding style standards to code deps = - black ruff commands = ruff check --fix {[vars]all_path} - black {[vars]all_path} + ruff format {[vars]all_path} [testenv:lint] description = Check code against coding style standards deps = - black ruff codespell commands = codespell . --skip .git --skip .tox --skip build --skip lib --skip venv --skip .mypy_cache --skip *.svg ruff check {[vars]all_path} - black --check --diff {[vars]all_path} + ruff format --check {[vars]all_path} [testenv:static-{charm,lib}] description = Run static analysis checks @@ -81,7 +70,7 @@ deps = -r{toxinidir}/requirements.txt pytest cosl - ops-scenario>=6.1.5 + ops-scenario~=6.1 commands = pytest -vv --tb native --log-cli-level=INFO -s {posargs} {[vars]tst_path}/scenario/test_machine_charm