Merge ~sudeephb/charm-telegraf:bug/2008436 into charm-telegraf:master

[Sudeep Bhandari]

Handle prometheus relation when tls not available

Fixes bug: #2008436

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

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

[🤖 prod-jenkaas-bootstack (prod-jenkaas-bootstack)]

FAILED: Continuous integration, rev:1cf027fc605ba6292cc537cbc926464042d8ec1f
https://jenkins.canonical.com/bootstack/job/lp-charm-telegraf-ci/1/
Executed test runs:
    FAILURE: https://jenkins.canonical.com/bootstack/job/lp-charm-test-functest/180/
    None: https://jenkins.canonical.com/bootstack/job/lp-update-mp/76/

Click here to trigger a rebuild:
https://jenkins.canonical.com/bootstack/job/lp-charm-telegraf-ci/1//rebuild

review: Needs Fixing (continuous-integration)

[🤖 prod-jenkaas-bootstack (prod-jenkaas-bootstack)]

FAILED: Continuous integration, rev:1cf027fc605ba6292cc537cbc926464042d8ec1f
https://jenkins.canonical.com/bootstack/job/lp-charm-telegraf-ci/2/
Executed test runs:
    FAILURE: https://jenkins.canonical.com/bootstack/job/lp-charm-test-functest/181/
    None: https://jenkins.canonical.com/bootstack/job/lp-update-mp/77/

Click here to trigger a rebuild:
https://jenkins.canonical.com/bootstack/job/lp-charm-telegraf-ci/2//rebuild

review: Needs Fixing (continuous-integration)

[🤖 prod-jenkaas-bootstack (prod-jenkaas-bootstack)]

PASSED: Continuous integration, rev:1cf027fc605ba6292cc537cbc926464042d8ec1f
https://jenkins.canonical.com/bootstack/job/lp-charm-telegraf-ci/3/
Executed test runs:
    SUCCESS: https://jenkins.canonical.com/bootstack/job/lp-charm-test-functest/185/
    None: https://jenkins.canonical.com/bootstack/job/lp-update-mp/81/

Click here to trigger a rebuild:
https://jenkins.canonical.com/bootstack/job/lp-charm-telegraf-ci/3//rebuild

review: Approve (continuous-integration)

[Martin Kalcok (martin-kalcok)]

I have one inline comment and I think it'd be best to add unit tests as well. At least:

  * one that would test that the situation from LP#2008436 is resolved
  * one that would test that change in certificates relation does not try to reconfigure prometheus relation if it does not exist.

review: Needs Fixing

[Eric Chen (eric-chen)]

Please check my inline question.

review: Needs Information

[🤖 prod-jenkaas-bootstack (prod-jenkaas-bootstack)]

PASSED: Continuous integration, rev:1cf027fc605ba6292cc537cbc926464042d8ec1f
https://jenkins.canonical.com/bootstack/job/lp-charm-telegraf-ci/4/
Executed test runs:
    SUCCESS: https://jenkins.canonical.com/bootstack/job/lp-charm-test-functest/186/
    None: https://jenkins.canonical.com/bootstack/job/lp-update-mp/82/

Click here to trigger a rebuild:
https://jenkins.canonical.com/bootstack/job/lp-charm-telegraf-ci/4//rebuild

review: Approve (continuous-integration)

[Eric Chen (eric-chen)]

inline message again

review: Needs Information

[Tianqi Xiao (txiao)]

Thanks for the PR. I have 1 comment in-line

review: Needs Fixing

[🤖 prod-jenkaas-bootstack (prod-jenkaas-bootstack)]

PASSED: Continuous integration, rev:608ce49ba3aa530a49013817d0f1584f30801a15
https://jenkins.canonical.com/bootstack/job/lp-charm-telegraf-ci/5/
Executed test runs:
    SUCCESS: https://jenkins.canonical.com/bootstack/job/lp-charm-test-functest/222/
    None: https://jenkins.canonical.com/bootstack/job/lp-update-mp/84/

Click here to trigger a rebuild:
https://jenkins.canonical.com/bootstack/job/lp-charm-telegraf-ci/5//rebuild

review: Approve (continuous-integration)

[Eric Chen (eric-chen)]

review: Approve

[Tianqi Xiao (txiao)]

Overall LGTM. Just one suggestion inline

[Robert Gildein (rgildein)]

LGTM

review: Approve

[🤖 prod-jenkaas-bootstack (prod-jenkaas-bootstack)]

PASSED: Continuous integration, rev:50146bdc3ecf70bf1f069ffa740a513e9da3676b
https://jenkins.canonical.com/bootstack/job/lp-charm-telegraf-ci/6/
Executed test runs:
    SUCCESS: https://jenkins.canonical.com/bootstack/job/lp-charm-test-functest/223/
    None: https://jenkins.canonical.com/bootstack/job/lp-update-mp/85/

Click here to trigger a rebuild:
https://jenkins.canonical.com/bootstack/job/lp-charm-telegraf-ci/6//rebuild

review: Approve (continuous-integration)

[Martin Kalcok (martin-kalcok)]

review: Approve

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

Change successfully merged at revision 58d7c9cc1bf6d181515b26b7c907b7748e855507

diff --git a/src/files/telegraf_exec_metrics.py b/src/files/telegraf_exec_metrics.py
index ec71bb9..a073239 100755
--- a/src/files/telegraf_exec_metrics.py
+++ b/src/files/telegraf_exec_metrics.py
@@ -258,7 +258,6 @@ class ZoneinfoFileMetric(FileMetric):
 
 
 class CmdMetric(FileMetric):
-
     cmd = []
 
     def get_cmd_output(self, cmd, is_json=False):
diff --git a/src/reactive/telegraf.py b/src/reactive/telegraf.py
index 898c8e9..b335f62 100644
--- a/src/reactive/telegraf.py
+++ b/src/reactive/telegraf.py
@@ -1702,7 +1702,6 @@ def generate_prometheus_output_config(
         options_ip_range = options.pop("ip_range", [])
         ip_range = options_ip_range + prometheus_ip_range
         if listen is not None:
-
             return {
                 "listen": listen,
                 "ip_range": ip_range,
@@ -1807,13 +1806,15 @@ def render_prometheus_client_config(port, address, ip_range):
     "certificates.available",
     "tls_client.certs.changed",
     "tls_client.server.certs.changed",
+    "endpoint.prometheus-client.changed",
 )
-@when("endpoint.prometheus-client.changed")
-def configure_prometheus_client_with_relation(prometheus):
+@when("prometheus-client.available")
+def configure_prometheus_client_with_relation(prometheus=None):
     hookenv.log(
         "Configuring prometheus_client output plugin, with prometheus-client relation",
         level=hookenv.DEBUG,
     )
+    prometheus = prometheus or endpoint_from_flag("prometheus-client.available")
     port, address = get_prometheus_bindaddress()
     hookenv.log("get_prometheus_bindaddress: {}:{}".format(address, str(port)))
     # We'll iterate through the prometheus-client relation counterparts,
@@ -2217,7 +2218,6 @@ def configure_tls():
 @when("endpoint.certificates.departed")
 @when_not("certificates.available")
 def cleanup_tls():
-
     if TLS_CERT.exists() and TLS_KEY.exists():
         TLS_CERT.unlink()
         TLS_KEY.unlink()
diff --git a/src/tests/unit/test_telegraf.py b/src/tests/unit/test_telegraf.py
index 37a98ca..5f59a91 100644
--- a/src/tests/unit/test_telegraf.py
+++ b/src/tests/unit/test_telegraf.py
@@ -202,7 +202,6 @@ def check_sysstat_config(original, expected):
 
 @pytest.mark.parametrize("install_method", ["deb", "snap"])
 def test_write_telegraf_file(mocker, install_method):
-
     if install_method == "deb":
         user = telegraf.DEB_OWNER
         group = telegraf.DEB_GROUP
@@ -1512,6 +1511,47 @@ def test_config_changed_apt(mocker, monkeypatch, config):
     service_restart.assert_called_once_with(telegraf.DEB_SERVICE)
 
 
[e-mail](
+    "flags, call_configure_prometheus_client_with_relation",
+    [
+        (["prometheus-client.available", "tls_client.server.certs.changed"], True),
+        (["prometheus-client.available", "endpoint.prometheus-client.changed"], True),
+        (["tls_client.certs.changed"], False),
+    ],
+)
+def test_prometheus_reconfiguration_when_certificate_changed(
+    flags, call_configure_prometheus_client_with_relation, mocker, monkeypatch
+):
+    """Verify that the function configure_prometheus_client_with_relation is invoked when expected."""  # noqa E501
+    mocker.patch("reactive.telegraf.host.service_restart")
+    monkeypatch.setattr(telegraf.host, "service_running", lambda n: True)
+    prometheus_interface = mocker.Mock()
+    prometheus_interface.configure = mocker.Mock()
+    endpoint_from_flag = mocker.patch("reactive.telegraf.endpoint_from_flag")
+    endpoint_from_flag.return_value = prometheus_interface
+    get_prometheus_bindaddress = mocker.patch(
+        "reactive.telegraf.get_prometheus_bindaddress"
+    )
+    get_prometheus_bindaddress.return_value = ("1.2.3.4", "9090")
+    mocker.patch("reactive.telegraf.check_prometheus_port")
+    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)
+    mocker.patch.object(
+        telegraf.hookenv, "network_get", return_value={"ingress_addresses": ["1.2.3.4"]}
+    )
+    get_prometheus_ip_range = mocker.patch("reactive.telegraf.get_prometheus_ip_range")
+    get_prometheus_ip_range.return_value = []
+    for flag in flags:
+        set_flag(flag)
+    bus.dispatch()
+    if call_configure_prometheus_client_with_relation:
+        prometheus_interface.configure.assert_called_once()
+    else:
+        prometheus_interface.configure.assert_not_called()
+
+
 def test_config_changed_extra_options(mocker, monkeypatch, config):
     service_restart = mocker.patch("reactive.telegraf.host.service_restart")
     monkeypatch.setattr(telegraf.host, "service_running", lambda n: True)