Merge ~samuelallan/charm-telegraf:check-running into charm-telegraf:master

[Samuel Allan]

Check if telegraf is running in update-status

This should help catch if something is unexpectedly crashing telegraf.

Closes-Bug: #2015517

Check if telegraf is running in update-status, so we catch if something is crashing telegraf.

[🤖 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:2a3f32953db34d8082db75eddfc416dd900c0508
https://jenkins.canonical.com/bootstack/job/lp-charm-telegraf-ci/9/
Executed test runs:
    FAILURE: https://jenkins.canonical.com/bootstack/job/lp-charm-test-functest/732/
    None: https://jenkins.canonical.com/bootstack/job/lp-update-mp/234/

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

review: Needs Fixing (continuous-integration)

[Robert Gildein (rgildein)]

Overall it's look good to me, but you need to fix linting + add
some unit tests. Thanks

review: Needs Fixing

[Samuel Allan (samuelallan)]

Thanks Robert, I've fixed the lints and added tests. :)

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

PASSED: Continuous integration, rev:00d8453fcbff61920aebcca55943050249fc0cc2
https://jenkins.canonical.com/bootstack/job/lp-charm-telegraf-ci/10/
Executed test runs:
    SUCCESS: https://jenkins.canonical.com/bootstack/job/lp-charm-test-functest/764/
    None: https://jenkins.canonical.com/bootstack/job/lp-update-mp/244/

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

review: Approve (continuous-integration)

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

PASSED: Continuous integration, rev:4434f819fcc22f4081f865967f23135d041ea0cf
https://jenkins.canonical.com/bootstack/job/lp-charm-telegraf-ci/11/
Executed test runs:
    SUCCESS: https://jenkins.canonical.com/bootstack/job/lp-charm-test-functest/766/
    None: https://jenkins.canonical.com/bootstack/job/lp-update-mp/245/

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

review: Approve (continuous-integration)

[Robert Gildein (rgildein)]

review: Approve

[Eric Chen (eric-chen)]

LGTM

review: Approve

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

Change successfully merged at revision 97303e509151b37fa6d099eddb106634e9eae81b

diff --git a/src/reactive/telegraf.py b/src/reactive/telegraf.py
index 6189fa3..eea17e6 100644
--- a/src/reactive/telegraf.py
+++ b/src/reactive/telegraf.py
@@ -47,6 +47,7 @@ from charms.reactive import (
     endpoint_from_flag,
     helpers,
     hook,
+    is_flag_set,
     set_flag,
     toggle_flag,
     when,
@@ -2088,24 +2089,28 @@ def start_or_restart():
         time.sleep(0.1)
 
     if host.service_running(service):
-        revision = ""
-        if os.path.exists("version"):
-            with open("version") as f:
-                line = f.readline().strip()
-            # We only want the first 8 characters, that's enough to tell
-            # which version of the charm we're using.
-            if len(line) > 8:
-                revision = " (source version/commit {}...)".format(line[:8])
-            else:
-                revision = " (source version/commit {})".format(line)
         hookenv.status_set(
-            "active", "Monitoring {}{}".format(get_remote_unit_name(), revision)
+            "active", "Monitoring {}{}".format(get_remote_unit_name(), get_revision())
         )
         clear_flag("telegraf.needs_reload")
     else:
         hookenv.status_set("blocked", "Telegraf failed to start. Check config.")
 
 
+def get_revision():
+    revision = ""
+    if os.path.exists("version"):
+        with open("version") as file:
+            line = file.readline().strip()
+        # We only want the first 8 characters, that's enough to tell
+        # which version of the charm we're using.
+        if len(line) > 8:
+            revision = " (source version/commit {}...)".format(line[:8])
+        else:
+            revision = " (source version/commit {})".format(line)
+    return revision
+
+
 def is_bcache():
     """Determine if this is a container.
 
@@ -2125,6 +2130,16 @@ def update_status():
         clear_flag("telegraf.apt.configured")
         clear_flag("telegraf.snap.configured")
 
+    if is_flag_set("telegraf.configured"):
+        service = get_service()
+        if not host.service_running(service):
+            hookenv.status_set("blocked", "Service not running: {}".format(service))
+        else:
+            hookenv.status_set(
+                "active",
+                "Monitoring {}{}".format(get_remote_unit_name(), get_revision()),
+            )
+
 
 @when("nrpe-external-master.available")
 @when("telegraf.installed")
diff --git a/src/tests/unit/test_telegraf.py b/src/tests/unit/test_telegraf.py
index 512f5c4..2ba2a0e 100644
--- a/src/tests/unit/test_telegraf.py
+++ b/src/tests/unit/test_telegraf.py
@@ -2111,3 +2111,31 @@ def test_cleanup_tls(mocker, monkeypatch):
     bus.dispatch()
     assert "plugins.prometheus-client.configured" in bus.get_states().keys()
     assert service_restart.called_once()
+
+
+def test_get_revision(mocker, monkeypatch):
+    monkeypatch.setattr(telegraf.os.path, "exists", lambda _: True)
+    mock_open = mocker.patch("reactive.telegraf.open")
+    mock_open.return_value.__enter__.return_value.readline.return_value = "abcdefghijkl"
+    assert telegraf.get_revision() == " (source version/commit abcdefgh...)"
+
+
+def test_get_revision_short(mocker, monkeypatch):
+    monkeypatch.setattr(telegraf.os.path, "exists", lambda _: True)
+    mock_open = mocker.patch("reactive.telegraf.open")
+    mock_open.return_value.__enter__.return_value.readline.return_value = "abc"
+    assert telegraf.get_revision() == " (source version/commit abc)"
+
+
+def test_update_status_telegraf_not_running(mocker, monkeypatch):
+    monkeypatch.setattr(host, "service_running", lambda _: False)
+    monkeypatch.setattr(
+        telegraf.charms.reactive.helpers, "data_changed", lambda _, __, hash_type: False
+    )
+    status_set = mocker.patch("reactive.telegraf.hookenv.status_set")
+    set_flag("telegraf.configured")
+    telegraf.update_status()
+    status_set.assert_called_once_with(
+        "blocked",
+        "Service not running: telegraf",
+    )