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

Proposed by Samuel Allan
Status: Merged
Approved by: Eric Chen
Approved revision: 4434f819fcc22f4081f865967f23135d041ea0cf
Merged at revision: 97303e509151b37fa6d099eddb106634e9eae81b
Proposed branch: ~samuelallan/charm-telegraf:check-running
Merge into: charm-telegraf:master
Diff against target: 105 lines (+54/-11)
2 files modified
src/reactive/telegraf.py (+26/-11)
src/tests/unit/test_telegraf.py (+28/-0)
Reviewer Review Type Date Requested Status
Eric Chen Approve
Robert Gildein Approve
prod-jenkaas-bootstack (community) continuous-integration Approve
Review via email: mp+441912@code.launchpad.net

Commit message

Check if telegraf is running in update-status

This should help catch if something is unexpectedly crashing telegraf.

Closes-Bug: #2015517

Description of the change

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

To post a comment you must log in.
Revision history for this message
Canonical IS Merge Bot (canonical-is-mergebot) wrote :

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

Revision history for this message
prod-jenkaas-bootstack (prod-jenkaas-bootstack) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Robert Gildein (rgildein) wrote :

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

review: Needs Fixing
Revision history for this message
Samuel Allan (samuelallan) wrote :

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

Revision history for this message
prod-jenkaas-bootstack (prod-jenkaas-bootstack) wrote :
review: Approve (continuous-integration)
Revision history for this message
prod-jenkaas-bootstack (prod-jenkaas-bootstack) wrote :
review: Approve (continuous-integration)
Revision history for this message
Robert Gildein (rgildein) :
review: Approve
Revision history for this message
Eric Chen (eric-chen) wrote :

LGTM

review: Approve
Revision history for this message
Canonical IS Merge Bot (canonical-is-mergebot) wrote :

Change successfully merged at revision 97303e509151b37fa6d099eddb106634e9eae81b

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/src/reactive/telegraf.py b/src/reactive/telegraf.py
2index 6189fa3..eea17e6 100644
3--- a/src/reactive/telegraf.py
4+++ b/src/reactive/telegraf.py
5@@ -47,6 +47,7 @@ from charms.reactive import (
6 endpoint_from_flag,
7 helpers,
8 hook,
9+ is_flag_set,
10 set_flag,
11 toggle_flag,
12 when,
13@@ -2088,24 +2089,28 @@ def start_or_restart():
14 time.sleep(0.1)
15
16 if host.service_running(service):
17- revision = ""
18- if os.path.exists("version"):
19- with open("version") as f:
20- line = f.readline().strip()
21- # We only want the first 8 characters, that's enough to tell
22- # which version of the charm we're using.
23- if len(line) > 8:
24- revision = " (source version/commit {}...)".format(line[:8])
25- else:
26- revision = " (source version/commit {})".format(line)
27 hookenv.status_set(
28- "active", "Monitoring {}{}".format(get_remote_unit_name(), revision)
29+ "active", "Monitoring {}{}".format(get_remote_unit_name(), get_revision())
30 )
31 clear_flag("telegraf.needs_reload")
32 else:
33 hookenv.status_set("blocked", "Telegraf failed to start. Check config.")
34
35
36+def get_revision():
37+ revision = ""
38+ if os.path.exists("version"):
39+ with open("version") as file:
40+ line = file.readline().strip()
41+ # We only want the first 8 characters, that's enough to tell
42+ # which version of the charm we're using.
43+ if len(line) > 8:
44+ revision = " (source version/commit {}...)".format(line[:8])
45+ else:
46+ revision = " (source version/commit {})".format(line)
47+ return revision
48+
49+
50 def is_bcache():
51 """Determine if this is a container.
52
53@@ -2125,6 +2130,16 @@ def update_status():
54 clear_flag("telegraf.apt.configured")
55 clear_flag("telegraf.snap.configured")
56
57+ if is_flag_set("telegraf.configured"):
58+ service = get_service()
59+ if not host.service_running(service):
60+ hookenv.status_set("blocked", "Service not running: {}".format(service))
61+ else:
62+ hookenv.status_set(
63+ "active",
64+ "Monitoring {}{}".format(get_remote_unit_name(), get_revision()),
65+ )
66+
67
68 @when("nrpe-external-master.available")
69 @when("telegraf.installed")
70diff --git a/src/tests/unit/test_telegraf.py b/src/tests/unit/test_telegraf.py
71index 512f5c4..2ba2a0e 100644
72--- a/src/tests/unit/test_telegraf.py
73+++ b/src/tests/unit/test_telegraf.py
74@@ -2111,3 +2111,31 @@ def test_cleanup_tls(mocker, monkeypatch):
75 bus.dispatch()
76 assert "plugins.prometheus-client.configured" in bus.get_states().keys()
77 assert service_restart.called_once()
78+
79+
80+def test_get_revision(mocker, monkeypatch):
81+ monkeypatch.setattr(telegraf.os.path, "exists", lambda _: True)
82+ mock_open = mocker.patch("reactive.telegraf.open")
83+ mock_open.return_value.__enter__.return_value.readline.return_value = "abcdefghijkl"
84+ assert telegraf.get_revision() == " (source version/commit abcdefgh...)"
85+
86+
87+def test_get_revision_short(mocker, monkeypatch):
88+ monkeypatch.setattr(telegraf.os.path, "exists", lambda _: True)
89+ mock_open = mocker.patch("reactive.telegraf.open")
90+ mock_open.return_value.__enter__.return_value.readline.return_value = "abc"
91+ assert telegraf.get_revision() == " (source version/commit abc)"
92+
93+
94+def test_update_status_telegraf_not_running(mocker, monkeypatch):
95+ monkeypatch.setattr(host, "service_running", lambda _: False)
96+ monkeypatch.setattr(
97+ telegraf.charms.reactive.helpers, "data_changed", lambda _, __, hash_type: False
98+ )
99+ status_set = mocker.patch("reactive.telegraf.hookenv.status_set")
100+ set_flag("telegraf.configured")
101+ telegraf.update_status()
102+ status_set.assert_called_once_with(
103+ "blocked",
104+ "Service not running: telegraf",
105+ )

Subscribers

People subscribed via source and target branches

to all changes: