Merge ~barryprice/charm-telegraf:split_prometheus_client_config into charm-telegraf:master

[Barry Price]

Split prometheus_client output config out of the main config file. Allow a prometheus-client relation to prometheus even if prometheus_output_port is set (and use that port in the relation).

[Laurent Sesquès (sajoupa)]

passes make test

[🤖 Canonical IS Merge Bot (canonical-is-mergebot)]

This merge proposal is being monitored by mergebot. Change the status to Approved to merge.

[🤖 Canonical IS Merge Bot (canonical-is-mergebot)]

Unable to determine commit message from repository - please click "Set commit message" and enter the commit message manually.

[🤖 Canonical IS Merge Bot (canonical-is-mergebot)]

This merge proposal is being monitored by mergebot. Change the status to Approved to merge.

[Haw Loeung (hloeung)]

LGTM

review: Approve (+1)

[🤖 Canonical IS Merge Bot (canonical-is-mergebot)]

Change successfully merged at revision 681fc188e8d79ae8420393be2b00d62d76099f7b

diff --git a/src/reactive/telegraf.py b/src/reactive/telegraf.py
index d8fd268..82cf10e 100644
--- a/src/reactive/telegraf.py
+++ b/src/reactive/telegraf.py
@@ -183,7 +183,14 @@ def list_config_files():
     current_states = get_states()
 
     for plugin in list_supported_plugins():
-        if "plugins.{}.configured".format(plugin) in current_states.keys():
+        # The prometheus_client plugin can be configured either from a relation or
+        # from the juju config
+        if ("plugins.{}.configured".format(plugin) in current_states.keys()) or (
+            "{}.configured".format(plugin) in current_states.keys()
+        ):
+            # The "prometheus-client" relation sets the "prometheus_client" plugin
+            if plugin == "prometheus-client":
+                plugin = "prometheus_client"
             config_path = "{}/{}.conf".format(get_configs_dir(), plugin)
             config_files.append(config_path)
 
@@ -527,9 +534,6 @@ def configure_telegraf():  # noqa: C901
         )
         context["logfile"] = ""
 
-    if get_prometheus_port():
-        context["prometheus_output_port"] = get_prometheus_port()
-        check_prometheus_port("prometheus_output", get_prometheus_port())
     context["extra_options"] = get_extra_options()
 
     if get_socket_listener_port():
@@ -645,6 +649,7 @@ def install_telegraf():
 
 @when("telegraf.installed")
 @when("apt.installed.telegraf")
+@when("plugins.prometheus-client.configured")
 @when_not("telegraf.configured")
 @when_not("telegraf.apt.configured")
 def configure_telegraf_deb():
@@ -653,6 +658,7 @@ def configure_telegraf_deb():
 
 @when("telegraf.installed")
 @when("snap.installed.telegraf")
+@when("plugins.prometheus-client.configured")
 @when_not("telegraf.configured")
 @when_not("telegraf.snap.configured")
 def configure_telegraf_snap():
@@ -677,6 +683,7 @@ def handle_config_changes():
     if config.changed("extra_options"):
         for plugin in list_supported_plugins():
             clear_flag("plugins.{}.configured".format(plugin))
+        clear_flag("prometheus-client.relation.configured")
     # if something else changed, let's reconfigure telegraf itself just in case
 
     if config.changed("extra_plugins"):
@@ -689,6 +696,12 @@ def handle_config_changes():
     ):
         clear_flag("telegraf.installed")
         clear_flag("extra_plugins.configured")
+        clear_flag("plugins.prometheus-client.configured")
+        clear_flag("prometheus-client.relation.configured")
+
+    if config.changed("prometheus_output_port"):
+        clear_flag("plugins.prometheus-client.configured")
+        clear_flag("prometheus-client.relation.configured")
     clear_flag("telegraf.configured")
     clear_flag("telegraf.apt.configured")
     clear_flag("telegraf.snap.configured")
@@ -1207,59 +1220,108 @@ def influxdb_api_output(influxdb):
     set_flag("telegraf.needs_reload")
 
 
+def generate_prometheus_output_config(prometheus_output_port):
+    # If extra_options are set for prometheus_client, let's integrate them
+    extra_options = get_extra_options()
+    options = extra_options["outputs"].get("prometheus_client", {})
+    listen = options.pop("listen", None)
+    if not listen:
+        listen = ":{}".format(prometheus_output_port)
+    elif int(listen.split(":", 1)[1]) != prometheus_output_port:
+        hookenv.log(
+            """prometheus_output_port is {}, but extra_options would set it
+                       to {}. Choosing {} from prometheus_output_port.""".format(
+                prometheus_output_port,
+                int(listen.split(":", 1)[1]),
+                prometheus_output_port,
+            ),
+            level=hookenv.WARNING,
+        )
+        listen = "{}:{}".format(listen.split(":", 1)[0], prometheus_output_port)
+
+    return {
+        "listen": listen,
+        "extra_options": render_extra_options(
+            "outputs", "prometheus_client", extra_options=extra_options
+        ),
+    }
+
+
+def render_prometheus_client_config(port):
+    config_path = "{}/{}.conf".format(get_configs_dir(), "prometheus_client")
+    hookenv.log(
+        "Updating {} plugin config file. Port is {}".format("prometheus_client", port),
+        level=hookenv.INFO,
+    )
+    context = generate_prometheus_output_config(port)
+    render(
+        source="prometheus_client.tmpl",
+        templates_dir=get_templates_dir(),
+        target=config_path,
+        context=context,
+    )
+
+
 @when("prometheus-client.available")
-@when("telegraf.installed")
-def prometheus_client(prometheus):
-    template = """
-[[outputs.prometheus_client]]
-  listen = "{{ listen }}"
-"""
+@when_not("prometheus-client.relation.configured")
+def configure_prometheus_client_with_relation(prometheus):
+    hookenv.log(
+        "Configuring prometheus_client output plugin, with prometheus-client relation",
+        level=hookenv.DEBUG,
+    )
+    port = get_prometheus_port() or "9126"
+    # We'll iterate through the prometheus-client relation counterparts,
+    # inform them of our address so that they scrape it, and get their egress subnets
+    # so that we can allow them
+    remote_egress_subnets = []
     for relation_id in hookenv.relation_ids("prometheus-client"):
         # if juju 2.x+ then we'll attempt to get the network space address
         try:
-            hookenv.log("Network Info")
+            hookenv.log("Getting local network info", level=hookenv.DEBUG)
             network_info = hookenv.network_get(
                 "prometheus-client", relation_id=relation_id
             )
-            hookenv.log(network_info)
+            hookenv.log(network_info, level=hookenv.DEBUG)
             if "ingress-addresses" in network_info:
                 ip_addr = network_info.get("ingress-addresses")[0]
             else:
                 ip_addr = hookenv.network_get_primary_address("prometheus-client")
+            for unit in hookenv.related_units(relation_id):
+                hookenv.log(
+                    "Getting remote egress subnet for relation {} - {}".format(
+                        unit, relation_id
+                    ),
+                    level=hookenv.DEBUG,
+                )
+                remote_egress_subnets.append(
+                    hookenv.relation_get("egress-subnets", unit, relation_id)
+                )
         except NotImplementedError:
             # if that fails, just let prometheus.configure(...) do it's default
             ip_addr = None
-        if get_prometheus_port():
-            hookenv.log("Prometheus configured globally, skipping plugin setup")
-            prometheus.configure(
-                get_prometheus_port(), hostname=ip_addr, private_address=ip_addr
-            )
-            set_flag("prometheus-client.configured")
-            # bail out, nothing more need to be configured here
-            return
-        port = 9126
-        extra_options = get_extra_options()
-        options = extra_options["outputs"].get("prometheus-client", {})
-        listen = options.pop("listen", None)
-        if listen is not None:
-            hookenv.log(
-                "Configuring prometheus_client plugin to listen on: '{}'".format(listen)
-            )
-            port = int(listen.split(":", 1)[1])
-        else:
-            listen = ":{}".format(port)
-        check_prometheus_port("prometheus_output", port)
         prometheus.configure(port, hostname=ip_addr, private_address=ip_addr)
-        config_path = "{}/{}.conf".format(get_configs_dir(), "prometheus-client")
-        hookenv.log("Updating {} plugin config file".format("prometheus-client"))
-        context = {"listen": listen}
-        content = render_template(template, context) + render_extra_options(
-            "outputs", "prometheus_client", extra_options=extra_options
-        )
-        host.write_file(config_path, content.encode("utf-8"))
+    check_prometheus_port("prometheus_output", port)
+    render_prometheus_client_config(port)
+    set_flag("plugins.prometheus-client.configured")
+    set_flag("prometheus-client.relation.configured")
+    set_flag("telegraf.needs_reload")
+
+
+@when_not("prometheus-client.available")
+@when_not("plugins.prometheus-client.configured")
+def configure_prometheus_client():
+    hookenv.log("Configuring prometheus_client output plugin", level=hookenv.DEBUG)
+    if get_prometheus_port():
+        port = get_prometheus_port()
+    else:
+        # No relation to prometheus, no port configured: do not configure the plugin
         set_flag("plugins.prometheus-client.configured")
-        set_flag("telegraf.needs_reload")
-        set_flag("prometheus-client.configured")
+        return
+    check_prometheus_port("prometheus_output", port)
+    render_prometheus_client_config(port)
+    set_flag("plugins.prometheus-client.configured")
+    set_flag("telegraf.needs_reload")
+    clear_flag("prometheus-client.relation.configured")
 
 
 def convert_days(time_string):
@@ -1317,17 +1379,15 @@ def render_prometheus_rules(prometheus_rules):
 
 
 @when_not("prometheus-client.available")
-@when("plugins.prometheus-client.configured")
+@when("prometheus-client.relation.configured")
 def prometheus_client_departed():
     hookenv.log("prometheus-client relation not available")
-    config_path = "{}/{}.conf".format(get_configs_dir(), "prometheus-client")
+    config_path = "{}/{}.conf".format(get_configs_dir(), "prometheus_client")
     rels = hookenv.relations_of_type("prometheus-client")
     if not rels and os.path.exists(config_path):
         hookenv.log("Deleting {} plugin config file".format("prometheus-client"))
         os.unlink(config_path)
         clear_flag("plugins.prometheus-client.configured")
-        set_flag("telegraf.needs_reload")
-    clear_flag("prometheus-client.configured")
 
 
 @when(
@@ -1406,6 +1466,8 @@ def grafana_dashboard_import_failed():
 
 
 @when("telegraf.needs_reload")
+@when("telegraf.installed")
+@when("telegraf.configured")
 def start_or_restart():
     states = sorted(
         [
diff --git a/src/templates/prometheus_client.tmpl b/src/templates/prometheus_client.tmpl
new file mode 100644
index 0000000..add2099
--- /dev/null
+++ b/src/templates/prometheus_client.tmpl
@@ -0,0 +1,3 @@
+[[outputs.prometheus_client]]
+  listen = "{{ listen }}"
+{%- if extra_options %}{{ extra_options }}{%- endif %}
diff --git a/src/templates/telegraf.conf.tmpl b/src/templates/telegraf.conf.tmpl
index af59ed5..966b4a9 100644
--- a/src/templates/telegraf.conf.tmpl
+++ b/src/templates/telegraf.conf.tmpl
@@ -80,12 +80,6 @@
 
 {{ outputs }}
 
-{% if prometheus_output_port %}
-[[outputs.prometheus_client]]
-  listen = ":{{ prometheus_output_port }}"
-  {%- if extra_options['outputs']['prometheus_client'] %}{{ render_options('prometheus_client', 'outputs', extra_options['outputs']) }}{%- endif %}
-{%- endif %}
-
 ###############################################################################
 #                                  INPUTS                                     #
 ###############################################################################
diff --git a/src/tests/unit/test_telegraf.py b/src/tests/unit/test_telegraf.py
index 62a5595..d1ecd6b 100644
--- a/src/tests/unit/test_telegraf.py
+++ b/src/tests/unit/test_telegraf.py
@@ -809,12 +809,10 @@ def test_prometheus_global(monkeypatch, config):
     monkeypatch.setattr(telegraf.hookenv, "open_port", lambda p: open_ports.add(p))
     monkeypatch.setattr(telegraf.hookenv, "close_port", lambda p: open_ports.remove(p))
     config["prometheus_output_port"] = "default"
-    telegraf.configure_telegraf()
-    expected = """
-[[outputs.prometheus_client]]
-  listen = ":9103"
-"""
-    config_file = base_dir().join("telegraf.conf")
+    telegraf.configure_prometheus_client()
+    expected = '''[[outputs.prometheus_client]]
+  listen = ":9103"'''
+    config_file = base_dir().join("telegraf.d", "prometheus_client.conf")
     assert expected in config_file.read()
 
 
@@ -833,15 +831,13 @@ outputs:
       cpu: ["cpu0"]
 
 """
-    telegraf.configure_telegraf()
-    expected = """
-[[outputs.prometheus_client]]
+    telegraf.configure_prometheus_client()
+    expected = """[[outputs.prometheus_client]]
   listen = ":9103"
   namedrop = ["aerospike*"]
   [outputs.prometheus_client.tagpass]
-    cpu = ["cpu0"]
-"""
-    config_file = base_dir().join("telegraf.conf")
+    cpu = ["cpu0"]"""
+    config_file = os.path.join(base_dir(), "telegraf.d", "prometheus_client.conf")
     assert expected in config_file.read()
 
 
@@ -1132,11 +1128,13 @@ def test_influxdb_api_output(monkeypatch, config):
 
 
 def test_prometheus_client_output(mocker, monkeypatch, config):
-    config["prometheus_output_port"] = ""  # Not enabled globally
+    config["prometheus_output_port"] = ""  # Not configured globally
     monkeypatch.setattr(telegraf.hookenv, "open_port", lambda p: None)
 
     relation_ids = ["prometheus-client:0"]
+    related_units = ["prometheus/0"]
     monkeypatch.setattr(telegraf.hookenv, "relation_ids", lambda r: relation_ids)
+    monkeypatch.setattr(telegraf.hookenv, "related_units", lambda r: related_units)
 
     network_get_primary_address = mocker.patch.object(
         telegraf.hookenv, "network_get_primary_address", return_value="foo"
@@ -1145,32 +1143,33 @@ def test_prometheus_client_output(mocker, monkeypatch, config):
     mocker.patch.object(
         telegraf.hookenv, "network_get", return_value={"ingress_addresses": ["1.2.3.4"]}
     )
+    mocker.patch.object(telegraf.hookenv, "relation_get", return_value="5.6.7.8/32")
 
     interface = mocker.Mock(spec=RelationBase)
     interface.configure = mocker.Mock()
-    telegraf.prometheus_client(interface)
+    telegraf.configure_prometheus_client_with_relation(interface)
     expected = """
     [[outputs.prometheus_client]]
   listen = ":9126"
 """
     assert (
-        configs_dir().join("prometheus-client.conf").read().strip() == expected.strip()
+        configs_dir().join("prometheus_client.conf").read().strip() == expected.strip()
     )
     network_get_primary_address.assert_called_once_with("prometheus-client")
     interface.configure.assert_called_once_with(
-        9126, hostname="foo", private_address="foo"
+        "9126", hostname="foo", private_address="foo"
     )
 
 
 def test_prometheus_client_output_departed(mocker, monkeypatch, config):
-    configs_dir().join("prometheus-client.conf").write("empty")
+    configs_dir().join("prometheus_client.conf").write("empty")
     relations = [1]
     monkeypatch.setattr(telegraf.hookenv, "relations_of_type", lambda n: relations)
     telegraf.prometheus_client_departed()
-    assert configs_dir().join("prometheus-client.conf").exists()
+    assert configs_dir().join("prometheus_client.conf").exists()
     relations.pop()
     telegraf.prometheus_client_departed()
-    assert not configs_dir().join("prometheus-client.conf").exists()
+    assert not configs_dir().join("prometheus_client.conf").exists()
 
 
 # Integration tests (kind of)
@@ -1264,15 +1263,16 @@ def test_restart_on_output_plugin_relation_departed(mocker, monkeypatch, config)
     )
     config["prometheus_output_port"] = ""
     bus.discover()
+    set_flag("telegraf.installed")
     set_flag("snap.telegraf.installed")
     set_flag("telegraf.configured")
     interface = mocker.Mock(spec=RelationBase)
     interface.configure = mocker.Mock()
-    telegraf.prometheus_client(interface)
-    assert configs_dir().join("prometheus-client.conf").exists()
+    telegraf.configure_prometheus_client_with_relation(interface)
+    assert configs_dir().join("prometheus_client.conf").exists()
     # dispatch, file should be gone and telegraf restarted.
     bus.dispatch()
-    assert not configs_dir().join("prometheus-client.conf").exists()
+    assert not configs_dir().join("prometheus_client.conf").exists()
     service_restart.assert_called_with(telegraf.DEB_SERVICE)