Merge ~sajoupa/charm-telegraf:split_prometheus_client_config into charm-telegraf:master
- Git
- lp:~sajoupa/charm-telegraf
- split_prometheus_client_config
- Merge into master
Status: | Superseded |
---|---|
Proposed branch: | ~sajoupa/charm-telegraf:split_prometheus_client_config |
Merge into: | charm-telegraf:master |
Diff against target: |
387 lines (+137/-73) 4 files modified
src/reactive/telegraf.py (+112/-45) src/templates/prometheus_client.tmpl (+3/-0) src/templates/telegraf.conf.tmpl (+0/-6) src/tests/unit/test_telegraf.py (+22/-22) |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Canonical IS Reviewers | Pending | ||
BootStack Reviewers | Pending | ||
Review via email: mp+397010@code.launchpad.net |
This proposal has been superseded by a proposal from 2021-02-18.
Commit message
Split prometheus_client output config out of the main config file. Allow a prometheus-client relation to prometheus even if prometheus_
Description of the change
Laurent Sesquès (sajoupa) wrote : | # |
🤖 Canonical IS Merge Bot (canonical-is-mergebot) wrote : | # |
This merge proposal is being monitored by mergebot. Change the status to Approved to merge.
🤖 Canonical IS Merge Bot (canonical-is-mergebot) wrote : | # |
Unable to determine commit message from repository - please click "Set commit message" and enter the commit message manually.
Haw Loeung (hloeung) : | # |
Unmerged commits
- 3c4a87e... by Laurent Sesquès
-
small fix: move flag changes to the right if stanza for config.changed
- f550c15... by Laurent Sesquès
-
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).
Preview Diff
1 | diff --git a/src/reactive/telegraf.py b/src/reactive/telegraf.py |
2 | index c139276..e2c0874 100644 |
3 | --- a/src/reactive/telegraf.py |
4 | +++ b/src/reactive/telegraf.py |
5 | @@ -159,7 +159,14 @@ def list_config_files(): |
6 | current_states = get_states() |
7 | |
8 | for plugin in list_supported_plugins(): |
9 | - if "plugins.{}.configured".format(plugin) in current_states.keys(): |
10 | + # The prometheus_client plugin can be configured either from a relation or |
11 | + # from the juju config |
12 | + if ("plugins.{}.configured".format(plugin) in current_states.keys()) or ( |
13 | + "{}.configured".format(plugin) in current_states.keys() |
14 | + ): |
15 | + # The "prometheus-client" relation sets the "prometheus_client" plugin |
16 | + if plugin == "prometheus-client": |
17 | + plugin = "prometheus_client" |
18 | config_path = "{}/{}.conf".format(get_configs_dir(), plugin) |
19 | config_files.append(config_path) |
20 | |
21 | @@ -503,9 +510,6 @@ def configure_telegraf(): # noqa: C901 |
22 | ) |
23 | context["logfile"] = "" |
24 | |
25 | - if get_prometheus_port(): |
26 | - context["prometheus_output_port"] = get_prometheus_port() |
27 | - check_prometheus_port("prometheus_output", get_prometheus_port()) |
28 | context["extra_options"] = get_extra_options() |
29 | |
30 | if get_socket_listener_port(): |
31 | @@ -621,6 +625,7 @@ def install_telegraf(): |
32 | |
33 | @when("telegraf.installed") |
34 | @when("apt.installed.telegraf") |
35 | +@when("plugins.prometheus-client.configured") |
36 | @when_not("telegraf.configured") |
37 | @when_not("telegraf.apt.configured") |
38 | def configure_telegraf_deb(): |
39 | @@ -629,6 +634,7 @@ def configure_telegraf_deb(): |
40 | |
41 | @when("telegraf.installed") |
42 | @when("snap.installed.telegraf") |
43 | +@when("plugins.prometheus-client.configured") |
44 | @when_not("telegraf.configured") |
45 | @when_not("telegraf.snap.configured") |
46 | def configure_telegraf_snap(): |
47 | @@ -653,6 +659,7 @@ def handle_config_changes(): |
48 | if config.changed("extra_options"): |
49 | for plugin in list_supported_plugins(): |
50 | clear_flag("plugins.{}.configured".format(plugin)) |
51 | + clear_flag("prometheus-client.relation.configured") |
52 | # if something else changed, let's reconfigure telegraf itself just in case |
53 | |
54 | if config.changed("extra_plugins"): |
55 | @@ -665,6 +672,12 @@ def handle_config_changes(): |
56 | ): |
57 | clear_flag("telegraf.installed") |
58 | clear_flag("extra_plugins.configured") |
59 | + clear_flag("plugins.prometheus-client.configured") |
60 | + clear_flag("prometheus-client.relation.configured") |
61 | + |
62 | + if config.changed("prometheus_output_port"): |
63 | + clear_flag("plugins.prometheus-client.configured") |
64 | + clear_flag("prometheus-client.relation.configured") |
65 | clear_flag("telegraf.configured") |
66 | clear_flag("telegraf.apt.configured") |
67 | clear_flag("telegraf.snap.configured") |
68 | @@ -1183,59 +1196,113 @@ def influxdb_api_output(influxdb): |
69 | set_flag("telegraf.needs_reload") |
70 | |
71 | |
72 | +def generate_prometheus_output_config(prometheus_output_port): |
73 | + # If extra_options are set for prometheus_client, let's integrate them |
74 | + extra_options = get_extra_options() |
75 | + options = extra_options["outputs"].get("prometheus_client", {}) |
76 | + listen = options.pop("listen", None) |
77 | + if not listen: |
78 | + listen = ":{}".format(prometheus_output_port) |
79 | + elif int(listen.split(":", 1)[1]) != prometheus_output_port: |
80 | + hookenv.log( |
81 | + """prometheus_output_port is {}, but extra_options would set it |
82 | + to {}. Choosing {} from prometheus_output_port.""".format( |
83 | + prometheus_output_port, |
84 | + int(listen.split(":", 1)[1]), |
85 | + prometheus_output_port, |
86 | + ), |
87 | + level=hookenv.WARNING, |
88 | + ) |
89 | + listen = "{}:{}".format(listen.split(":", 1)[0], prometheus_output_port) |
90 | + |
91 | + return { |
92 | + "listen": listen, |
93 | + "extra_options": render_extra_options( |
94 | + "outputs", "prometheus_client", extra_options=extra_options |
95 | + ), |
96 | + } |
97 | + |
98 | + |
99 | +def render_prometheus_client_config(port): |
100 | + config_path = "{}/{}.conf".format(get_configs_dir(), "prometheus_client") |
101 | + hookenv.log( |
102 | + "Updating {} plugin config file. Port is {}".format("prometheus_client", port), |
103 | + level=hookenv.INFO, |
104 | + ) |
105 | + context = generate_prometheus_output_config(port) |
106 | + render( |
107 | + source="prometheus_client.tmpl", |
108 | + templates_dir=get_templates_dir(), |
109 | + target=config_path, |
110 | + context=context, |
111 | + ) |
112 | + |
113 | + |
114 | @when("prometheus-client.available") |
115 | -@when("telegraf.installed") |
116 | -def prometheus_client(prometheus): |
117 | - template = """ |
118 | -[[outputs.prometheus_client]] |
119 | - listen = "{{ listen }}" |
120 | -""" |
121 | +@when_not("prometheus-client.relation.configured") |
122 | +def configure_prometheus_client_with_relation(prometheus): |
123 | + hookenv.log( |
124 | + "Configuring prometheus_client output plugin, with prometheus-client relation", |
125 | + level=hookenv.DEBUG, |
126 | + ) |
127 | + if get_prometheus_port(): |
128 | + port = get_prometheus_port() |
129 | + else: |
130 | + # We have a relation with prometheus, but removed the port number config. |
131 | + # Default to 9126. |
132 | + port = "9126" |
133 | + # We'll iterate through the prometheus-client relation counterparts, |
134 | + # inform them of our address so that they scrape it, and get their egress subnets |
135 | + # so that we can allow them |
136 | + remote_egress_subnets = [] |
137 | for relation_id in hookenv.relation_ids("prometheus-client"): |
138 | # if juju 2.x+ then we'll attempt to get the network space address |
139 | try: |
140 | - hookenv.log("Network Info") |
141 | + hookenv.log("Getting local network info", level=hookenv.DEBUG) |
142 | network_info = hookenv.network_get( |
143 | "prometheus-client", relation_id=relation_id |
144 | ) |
145 | - hookenv.log(network_info) |
146 | + hookenv.log(network_info, level=hookenv.DEBUG) |
147 | if "ingress-addresses" in network_info: |
148 | ip_addr = network_info.get("ingress-addresses")[0] |
149 | else: |
150 | ip_addr = hookenv.network_get_primary_address("prometheus-client") |
151 | + for unit in hookenv.related_units(relation_id): |
152 | + hookenv.log( |
153 | + "Getting remote egress subnet for relation {} - {}".format( |
154 | + unit, relation_id |
155 | + ), |
156 | + level=hookenv.DEBUG, |
157 | + ) |
158 | + remote_egress_subnets.append( |
159 | + hookenv.relation_get("egress-subnets", unit, relation_id) |
160 | + ) |
161 | except NotImplementedError: |
162 | # if that fails, just let prometheus.configure(...) do it's default |
163 | ip_addr = None |
164 | - if get_prometheus_port(): |
165 | - hookenv.log("Prometheus configured globally, skipping plugin setup") |
166 | - prometheus.configure( |
167 | - get_prometheus_port(), hostname=ip_addr, private_address=ip_addr |
168 | - ) |
169 | - set_flag("prometheus-client.configured") |
170 | - # bail out, nothing more need to be configured here |
171 | - return |
172 | - port = 9126 |
173 | - extra_options = get_extra_options() |
174 | - options = extra_options["outputs"].get("prometheus-client", {}) |
175 | - listen = options.pop("listen", None) |
176 | - if listen is not None: |
177 | - hookenv.log( |
178 | - "Configuring prometheus_client plugin to listen on: '{}'".format(listen) |
179 | - ) |
180 | - port = int(listen.split(":", 1)[1]) |
181 | - else: |
182 | - listen = ":{}".format(port) |
183 | - check_prometheus_port("prometheus_output", port) |
184 | prometheus.configure(port, hostname=ip_addr, private_address=ip_addr) |
185 | - config_path = "{}/{}.conf".format(get_configs_dir(), "prometheus-client") |
186 | - hookenv.log("Updating {} plugin config file".format("prometheus-client")) |
187 | - context = {"listen": listen} |
188 | - content = render_template(template, context) + render_extra_options( |
189 | - "outputs", "prometheus_client", extra_options=extra_options |
190 | - ) |
191 | - host.write_file(config_path, content.encode("utf-8")) |
192 | + check_prometheus_port("prometheus_output", port) |
193 | + render_prometheus_client_config(port) |
194 | + set_flag("plugins.prometheus-client.configured") |
195 | + set_flag("prometheus-client.relation.configured") |
196 | + set_flag("telegraf.needs_reload") |
197 | + |
198 | + |
199 | +@when_not("prometheus-client.available") |
200 | +@when_not("plugins.prometheus-client.configured") |
201 | +def configure_prometheus_client(): |
202 | + hookenv.log("Configuring prometheus_client output plugin", level=hookenv.DEBUG) |
203 | + if get_prometheus_port(): |
204 | + port = get_prometheus_port() |
205 | + else: |
206 | + # No relation to prometheus, no port configured: do not configure the plugin |
207 | set_flag("plugins.prometheus-client.configured") |
208 | - set_flag("telegraf.needs_reload") |
209 | - set_flag("prometheus-client.configured") |
210 | + return |
211 | + check_prometheus_port("prometheus_output", port) |
212 | + render_prometheus_client_config(port) |
213 | + set_flag("plugins.prometheus-client.configured") |
214 | + set_flag("telegraf.needs_reload") |
215 | + clear_flag("prometheus-client.relation.configured") |
216 | |
217 | |
218 | def convert_days(time_string): |
219 | @@ -1293,17 +1360,15 @@ def render_prometheus_rules(prometheus_rules): |
220 | |
221 | |
222 | @when_not("prometheus-client.available") |
223 | -@when("plugins.prometheus-client.configured") |
224 | +@when("prometheus-client.relation.configured") |
225 | def prometheus_client_departed(): |
226 | hookenv.log("prometheus-client relation not available") |
227 | - config_path = "{}/{}.conf".format(get_configs_dir(), "prometheus-client") |
228 | + config_path = "{}/{}.conf".format(get_configs_dir(), "prometheus_client") |
229 | rels = hookenv.relations_of_type("prometheus-client") |
230 | if not rels and os.path.exists(config_path): |
231 | hookenv.log("Deleting {} plugin config file".format("prometheus-client")) |
232 | os.unlink(config_path) |
233 | clear_flag("plugins.prometheus-client.configured") |
234 | - set_flag("telegraf.needs_reload") |
235 | - clear_flag("prometheus-client.configured") |
236 | |
237 | |
238 | @when( |
239 | @@ -1382,6 +1447,8 @@ def grafana_dashboard_import_failed(): |
240 | |
241 | |
242 | @when("telegraf.needs_reload") |
243 | +@when("telegraf.installed") |
244 | +@when("telegraf.configured") |
245 | def start_or_restart(): |
246 | states = sorted( |
247 | [ |
248 | diff --git a/src/templates/prometheus_client.tmpl b/src/templates/prometheus_client.tmpl |
249 | new file mode 100644 |
250 | index 0000000..add2099 |
251 | --- /dev/null |
252 | +++ b/src/templates/prometheus_client.tmpl |
253 | @@ -0,0 +1,3 @@ |
254 | +[[outputs.prometheus_client]] |
255 | + listen = "{{ listen }}" |
256 | +{%- if extra_options %}{{ extra_options }}{%- endif %} |
257 | diff --git a/src/templates/telegraf.conf.tmpl b/src/templates/telegraf.conf.tmpl |
258 | index af59ed5..966b4a9 100644 |
259 | --- a/src/templates/telegraf.conf.tmpl |
260 | +++ b/src/templates/telegraf.conf.tmpl |
261 | @@ -80,12 +80,6 @@ |
262 | |
263 | {{ outputs }} |
264 | |
265 | -{% if prometheus_output_port %} |
266 | -[[outputs.prometheus_client]] |
267 | - listen = ":{{ prometheus_output_port }}" |
268 | - {%- if extra_options['outputs']['prometheus_client'] %}{{ render_options('prometheus_client', 'outputs', extra_options['outputs']) }}{%- endif %} |
269 | -{%- endif %} |
270 | - |
271 | ############################################################################### |
272 | # INPUTS # |
273 | ############################################################################### |
274 | diff --git a/src/tests/unit/test_telegraf.py b/src/tests/unit/test_telegraf.py |
275 | index 2cfbdc8..09e1575 100644 |
276 | --- a/src/tests/unit/test_telegraf.py |
277 | +++ b/src/tests/unit/test_telegraf.py |
278 | @@ -753,12 +753,10 @@ def test_prometheus_global(monkeypatch, config): |
279 | monkeypatch.setattr(telegraf.hookenv, "open_port", lambda p: open_ports.add(p)) |
280 | monkeypatch.setattr(telegraf.hookenv, "close_port", lambda p: open_ports.remove(p)) |
281 | config["prometheus_output_port"] = "default" |
282 | - telegraf.configure_telegraf() |
283 | - expected = """ |
284 | -[[outputs.prometheus_client]] |
285 | - listen = ":9103" |
286 | -""" |
287 | - config_file = base_dir().join("telegraf.conf") |
288 | + telegraf.configure_prometheus_client() |
289 | + expected = '''[[outputs.prometheus_client]] |
290 | + listen = ":9103"''' |
291 | + config_file = base_dir().join("telegraf.d", "prometheus_client.conf") |
292 | assert expected in config_file.read() |
293 | |
294 | |
295 | @@ -777,15 +775,13 @@ outputs: |
296 | cpu: ["cpu0"] |
297 | |
298 | """ |
299 | - telegraf.configure_telegraf() |
300 | - expected = """ |
301 | -[[outputs.prometheus_client]] |
302 | + telegraf.configure_prometheus_client() |
303 | + expected = """[[outputs.prometheus_client]] |
304 | listen = ":9103" |
305 | namedrop = ["aerospike*"] |
306 | [outputs.prometheus_client.tagpass] |
307 | - cpu = ["cpu0"] |
308 | -""" |
309 | - config_file = base_dir().join("telegraf.conf") |
310 | + cpu = ["cpu0"]""" |
311 | + config_file = base_dir().join("telegraf.d", "prometheus_client.conf") |
312 | assert expected in config_file.read() |
313 | |
314 | |
315 | @@ -1076,11 +1072,13 @@ def test_influxdb_api_output(monkeypatch, config): |
316 | |
317 | |
318 | def test_prometheus_client_output(mocker, monkeypatch, config): |
319 | - config["prometheus_output_port"] = "" # Not enabled globally |
320 | + config["prometheus_output_port"] = "" # Not configured globally |
321 | monkeypatch.setattr(telegraf.hookenv, "open_port", lambda p: None) |
322 | |
323 | relation_ids = ["prometheus-client:0"] |
324 | + related_units = ["prometheus/0"] |
325 | monkeypatch.setattr(telegraf.hookenv, "relation_ids", lambda r: relation_ids) |
326 | + monkeypatch.setattr(telegraf.hookenv, "related_units", lambda r: related_units) |
327 | |
328 | network_get_primary_address = mocker.patch.object( |
329 | telegraf.hookenv, "network_get_primary_address", return_value="foo" |
330 | @@ -1089,32 +1087,33 @@ def test_prometheus_client_output(mocker, monkeypatch, config): |
331 | mocker.patch.object( |
332 | telegraf.hookenv, "network_get", return_value={"ingress_addresses": ["1.2.3.4"]} |
333 | ) |
334 | + mocker.patch.object(telegraf.hookenv, "relation_get", return_value="5.6.7.8/32") |
335 | |
336 | interface = mocker.Mock(spec=RelationBase) |
337 | interface.configure = mocker.Mock() |
338 | - telegraf.prometheus_client(interface) |
339 | + telegraf.configure_prometheus_client_with_relation(interface) |
340 | expected = """ |
341 | [[outputs.prometheus_client]] |
342 | listen = ":9126" |
343 | """ |
344 | assert ( |
345 | - configs_dir().join("prometheus-client.conf").read().strip() == expected.strip() |
346 | + configs_dir().join("prometheus_client.conf").read().strip() == expected.strip() |
347 | ) |
348 | network_get_primary_address.assert_called_once_with("prometheus-client") |
349 | interface.configure.assert_called_once_with( |
350 | - 9126, hostname="foo", private_address="foo" |
351 | + "9126", hostname="foo", private_address="foo" |
352 | ) |
353 | |
354 | |
355 | def test_prometheus_client_output_departed(mocker, monkeypatch, config): |
356 | - configs_dir().join("prometheus-client.conf").write("empty") |
357 | + configs_dir().join("prometheus_client.conf").write("empty") |
358 | relations = [1] |
359 | monkeypatch.setattr(telegraf.hookenv, "relations_of_type", lambda n: relations) |
360 | telegraf.prometheus_client_departed() |
361 | - assert configs_dir().join("prometheus-client.conf").exists() |
362 | + assert configs_dir().join("prometheus_client.conf").exists() |
363 | relations.pop() |
364 | telegraf.prometheus_client_departed() |
365 | - assert not configs_dir().join("prometheus-client.conf").exists() |
366 | + assert not configs_dir().join("prometheus_client.conf").exists() |
367 | |
368 | |
369 | # Integration tests (kind of) |
370 | @@ -1208,15 +1207,16 @@ def test_restart_on_output_plugin_relation_departed(mocker, monkeypatch, config) |
371 | ) |
372 | config["prometheus_output_port"] = "" |
373 | bus.discover() |
374 | + set_flag("telegraf.installed") |
375 | set_flag("snap.telegraf.installed") |
376 | set_flag("telegraf.configured") |
377 | interface = mocker.Mock(spec=RelationBase) |
378 | interface.configure = mocker.Mock() |
379 | - telegraf.prometheus_client(interface) |
380 | - assert configs_dir().join("prometheus-client.conf").exists() |
381 | + telegraf.configure_prometheus_client_with_relation(interface) |
382 | + assert configs_dir().join("prometheus_client.conf").exists() |
383 | # dispatch, file should be gone and telegraf restarted. |
384 | bus.dispatch() |
385 | - assert not configs_dir().join("prometheus-client.conf").exists() |
386 | + assert not configs_dir().join("prometheus_client.conf").exists() |
387 | service_restart.assert_called_with(telegraf.DEB_SERVICE) |
388 | |
389 |
passes make test