diff --git a/config.yaml b/config.yaml index 728e1c466..4a437b38d 100644 --- a/config.yaml +++ b/config.yaml @@ -48,3 +48,13 @@ options: Ref: https://grafana.com/docs/loki/latest/operations/storage/retention/ type: int default: 0 + reporting-enabled: + description: | + When disabled, Loki will be configured to not send anonymous usage statistics to stats.grafana.org. + It is very helpful to the Grafana project, so please leave this enabled. + + When enabled, Loki will use its default values for analytics. + + Ref: https://grafana.com/docs/loki/latest/configure/#analytics + type: boolean + default: true diff --git a/requirements.txt b/requirements.txt index c2f0fc292..efc80ea7b 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,6 +1,6 @@ cosl>=0.0.12 # pinned to 2.16 as 2.17 breaks our unittests -ops == 2.16 +ops kubernetes requests lightkube diff --git a/src/charm.py b/src/charm.py index c800c736b..3876c8f0b 100755 --- a/src/charm.py +++ b/src/charm.py @@ -500,6 +500,7 @@ def _configure(self): # noqa: C901 retention_period=int(self.config["retention-period"]), http_tls=self._tls_ready, tsdb_versions_migration_dates=self._tsdb_versions_migration_dates, + reporting_enabled=bool(self.config["reporting-enabled"]), ).build() # Add a layer so we can check if the service is running diff --git a/src/config_builder.py b/src/config_builder.py index ff06df8d6..dc6d9299b 100644 --- a/src/config_builder.py +++ b/src/config_builder.py @@ -55,6 +55,7 @@ def __init__( retention_period: int, http_tls: bool = False, tsdb_versions_migration_dates: Optional[List[Dict[str, str]]] = None, + reporting_enabled: bool, ): """Init method.""" self.instance_addr = instance_addr @@ -65,10 +66,11 @@ def __init__( self.http_tls = http_tls self.retention_period = retention_period self.tsdb_versions_migration_dates = tsdb_versions_migration_dates or [] + self.reporting_enabled = reporting_enabled def build(self) -> dict: """Build Loki config dictionary.""" - return { + loki_config = { "target": self._target, "auth_enabled": self._auth_enabled, "common": self._common, @@ -85,6 +87,12 @@ def build(self) -> dict: "compactor": self._compactor, } + # Overwrite the default only if reporting is not enabled + if not self.reporting_enabled: + loki_config["analytics"] = self._analytics + + return loki_config + @property def _common(self) -> dict: return { @@ -250,3 +258,11 @@ def _compactor(self) -> dict: "working_directory": COMPACTOR_DIR, "shared_store": "filesystem", } + + @property + def _analytics(self) -> dict: + # Ref: https://grafana.com/docs/loki/latest/configure/#analytics + return { + # Enable anonymous usage reporting. + "reporting_enabled": self.reporting_enabled, + } diff --git a/tests/scenario/conftest.py b/tests/scenario/conftest.py index f962da138..e5757d8db 100644 --- a/tests/scenario/conftest.py +++ b/tests/scenario/conftest.py @@ -1,7 +1,7 @@ from unittest.mock import PropertyMock, patch import pytest -import scenario +from ops.testing import Context from charm import LokiOperatorCharm @@ -25,4 +25,4 @@ def loki_charm(): @pytest.fixture def context(loki_charm): - return scenario.Context(loki_charm) + return Context(loki_charm) diff --git a/tests/scenario/test_charm_logging.py b/tests/scenario/test_charm_logging.py index 33a4b740d..c07b14b9b 100644 --- a/tests/scenario/test_charm_logging.py +++ b/tests/scenario/test_charm_logging.py @@ -3,7 +3,7 @@ import ops.pebble import pytest -import scenario +from ops.testing import Container, Exec, State @pytest.fixture @@ -13,19 +13,19 @@ def loki_emitter(): def test_no_endpoints_on_loki_not_ready(context, loki_emitter): - state = scenario.State( + state = State( containers=[ - scenario.Container( + Container( "loki", can_connect=True, layers={"loki": ops.pebble.Layer({"services": {"loki": {}}})}, - service_status={"loki": ops.pebble.ServiceStatus.INACTIVE}, - exec_mock={("update-ca-certificates", "--fresh"): scenario.ExecOutput()}, + service_statuses={"loki": ops.pebble.ServiceStatus.INACTIVE}, + execs={Exec(["update-ca-certificates", "--fresh"], return_code=0)}, ) ] ) - with context.manager("update-status", state) as mgr: + with context(context.on.update_status(), state) as mgr: charm = mgr.charm assert charm._charm_logging_endpoints == [] logging.getLogger("foo").debug("bar") @@ -34,19 +34,19 @@ def test_no_endpoints_on_loki_not_ready(context, loki_emitter): def test_endpoints_on_loki_ready(context, loki_emitter): - state = scenario.State( + state = State( containers=[ - scenario.Container( + Container( "loki", can_connect=True, layers={"loki": ops.pebble.Layer({"services": {"loki": {}}})}, - service_status={"loki": ops.pebble.ServiceStatus.ACTIVE}, - exec_mock={("update-ca-certificates", "--fresh"): scenario.ExecOutput()}, + service_statuses={"loki": ops.pebble.ServiceStatus.ACTIVE}, + execs={Exec(["update-ca-certificates", "--fresh"], return_code=0)}, ) ] ) - with context.manager("update-status", state) as mgr: + with context(context.on.update_status(), state) as mgr: charm = mgr.charm assert charm._charm_logging_endpoints == ["http://localhost:3100/loki/api/v1/push"] logging.getLogger("foo").debug("bar") @@ -62,19 +62,19 @@ def test_endpoints_on_loki_ready(context, loki_emitter): @patch("charm.LokiOperatorCharm._charm_logging_ca_cert", new_callable=lambda *_: True) def test_endpoints_on_loki_ready_tls(_, context, loki_emitter): - state = scenario.State( + state = State( containers=[ - scenario.Container( + Container( "loki", can_connect=True, layers={"loki": ops.pebble.Layer({"services": {"loki": {}}})}, - service_status={"loki": ops.pebble.ServiceStatus.ACTIVE}, - exec_mock={("update-ca-certificates", "--fresh"): scenario.ExecOutput()}, + service_statuses={"loki": ops.pebble.ServiceStatus.ACTIVE}, + execs={Exec(["update-ca-certificates", "--fresh"], return_code=0)}, ) ] ) - with context.manager("update-status", state) as mgr: + with context(context.on.update_status(), state) as mgr: charm = mgr.charm assert charm._charm_logging_endpoints == ["https://localhost:3100/loki/api/v1/push"] logging.getLogger("foo").debug("bar") diff --git a/tests/scenario/test_config_reporting_enabled.py b/tests/scenario/test_config_reporting_enabled.py new file mode 100644 index 000000000..379d02148 --- /dev/null +++ b/tests/scenario/test_config_reporting_enabled.py @@ -0,0 +1,59 @@ +import yaml +from ops.testing import Container, Exec, State, pebble + +containers = [ + Container( + name="loki", + can_connect=True, + layers={ + "loki": pebble.Layer( + { + "services": { + "loki": {"startup": "enabled"}, + }, + } + ), + }, + execs={Exec(["update-ca-certificates", "--fresh"], return_code=0)}, + ), + Container(name="node-exporter", can_connect=True), +] + + +def test_reporting_enabled(context): + # GIVEN the "reporting_enabled" config option is set to True + state = State(leader=True, config={"reporting-enabled": True}, containers=containers) + + # WHEN config-changed fires + out = context.run(context.on.config_changed(), state) + + # THEN the config file is written WITHOUT the [analytics] section being rendered + simulated_pebble_filesystem = out.get_container("loki").get_filesystem(context) + grafana_config_path = simulated_pebble_filesystem / "etc/loki/loki-local-config.yaml" + + with open(grafana_config_path, "r") as file: + config = yaml.safe_load(file) + + assert "analytics" not in config + + +def test_reporting_disabled(context): + # GIVEN the "reporting_enabled" config option is set to False + state = State(leader=True, config={"reporting-enabled": False}, containers=containers) + + # WHEN config-changed fires + out = context.run(context.on.config_changed(), state) + + # THEN the config file is written WITH the [analytics] section being rendered + simulated_pebble_filesystem = out.get_container("loki").get_filesystem(context) + grafana_config_path = simulated_pebble_filesystem / "etc/loki/loki-local-config.yaml" + + with open(grafana_config_path, "r") as file: + config = yaml.safe_load(file) + + assert "analytics" in config + assert not config["analytics"].get("reporting_enabled") + + # AND the "loki" service is restarted + # TODO Does it make sense to check this if the charm under test's lifetime is only for the config-changed? + # TODO How to assert this? diff --git a/tests/unit/test_charm.py b/tests/unit/test_charm.py index 187947b54..b0cb48ecb 100644 --- a/tests/unit/test_charm.py +++ b/tests/unit/test_charm.py @@ -459,17 +459,7 @@ def setUp(self, *_): def tearDown(self): self.mock_request.stop() - @patch("ops.testing._TestingModelBackend.network_get") - def _add_alerting_relation(self, mock_unit_ip): - fake_network = { - "bind-addresses": [ - { - "interface-name": "eth0", - "addresses": [{"hostname": "loki-0", "value": "10.1.2.3"}], - } - ] - } - mock_unit_ip.return_value = fake_network + def _add_alerting_relation(self): rel_id = self.harness.add_relation("logging", "tester") self.harness.add_relation_unit(rel_id, "tester/0") diff --git a/tests/unit/test_provider.py b/tests/unit/test_provider.py index 833ba3999..4c6b00458 100644 --- a/tests/unit/test_provider.py +++ b/tests/unit/test_provider.py @@ -4,7 +4,6 @@ import json import textwrap import unittest -from unittest.mock import patch from charms.loki_k8s.v0.loki_push_api import LokiPushApiProvider from ops.charm import CharmBase @@ -122,17 +121,7 @@ def test_relation_data(self): expected_value = {"url": endpoint} self.assertEqual(expected_value, self.harness.charm.loki_provider._endpoint(url)) - @patch("ops.testing._TestingModelBackend.network_get") - def test__on_logging_relation_changed(self, mock_unit_ip): - fake_network = { - "bind-addresses": [ - { - "interface-name": "eth0", - "addresses": [{"hostname": "loki-0", "value": "10.1.2.3"}], - } - ] - } - mock_unit_ip.return_value = fake_network + def test__on_logging_relation_changed(self): rel_id = self.harness.add_relation("logging", "promtail") self.harness.add_relation_unit(rel_id, "promtail/0") @@ -141,17 +130,7 @@ def test__on_logging_relation_changed(self, mock_unit_ip): ) self.assertEqual(len(self.harness.charm._stored.events), 1) - @patch("ops.testing._TestingModelBackend.network_get") - def test__on_logging_relation_created_and_broken(self, mock_unit_ip): - fake_network = { - "bind-addresses": [ - { - "interface-name": "eth0", - "addresses": [{"hostname": "loki-0", "value": "10.1.2.3"}], - } - ] - } - mock_unit_ip.return_value = fake_network + def test__on_logging_relation_created_and_broken(self): rel_id = self.harness.add_relation("logging", "promtail") self.harness.add_relation_unit(rel_id, "promtail/0") @@ -163,17 +142,7 @@ def test__on_logging_relation_created_and_broken(self, mock_unit_ip): self.harness.remove_relation(rel_id) self.assertEqual(len(self.harness.charm._stored.events), 3) - @patch("ops.testing._TestingModelBackend.network_get") - def test_alerts(self, mock_unit_ip): - fake_network = { - "bind-addresses": [ - { - "interface-name": "eth0", - "addresses": [{"hostname": "loki-0", "value": "10.1.2.3"}], - } - ] - } - mock_unit_ip.return_value = fake_network + def test_alerts(self): rel_id = self.harness.add_relation("logging", "consumer") self.harness.update_relation_data( rel_id, diff --git a/tox.ini b/tox.ini index 6e6ac15c4..be9798af3 100644 --- a/tox.ini +++ b/tox.ini @@ -82,7 +82,7 @@ commands = description = Scenario tests deps = pytest - ops-scenario==6.1.7 + ops[testing] -r{toxinidir}/requirements.txt commands = pytest -v --tb native {[vars]tst_path}/scenario --log-cli-level=INFO -s {posargs}