Merge ~xavpaice/charm-grafana:bug/1873105 into charm-grafana:master
- Git
- lp:~xavpaice/charm-grafana
- bug/1873105
- Merge into master
Status: | Merged | ||||
---|---|---|---|---|---|
Approved by: | Joe Guo | ||||
Approved revision: | 59f8c55d0d5cca497a0b8d7b64128cdf288ab883 | ||||
Merged at revision: | 2078b8c32cf6c26b0ad3e4444b69cf553039757c | ||||
Proposed branch: | ~xavpaice/charm-grafana:bug/1873105 | ||||
Merge into: | charm-grafana:master | ||||
Diff against target: |
517 lines (+86/-113) 8 files modified
.gitignore (+1/-0) Makefile (+1/-1) lib/charms/layer/grafana.py (+3/-8) reactive/grafana.py (+38/-66) tests/functional/tests/test_grafana.py (+10/-30) tests/unit/requirements.txt (+3/-0) tests/unit/test_grafana.py (+27/-4) tox.ini (+3/-4) |
||||
Related bugs: |
|
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Joe Guo (community) | Approve | ||
Paul Goins | Approve | ||
Xav Paice (community) | Needs Resubmitting | ||
Alvaro Uria | Pending | ||
Peter Sabaini | Pending | ||
Review via email: mp+388019@code.launchpad.net |
This proposal supersedes a proposal from 2020-04-16.
Commit message
Replace the process monitoring with functional
Description of the change
🤖 Canonical IS Merge Bot (canonical-is-mergebot) wrote : Posted in a previous version of this proposal | # |
Peter Sabaini (peter-sabaini) wrote : Posted in a previous version of this proposal | # |
Two minor comments inline, otherwise lgtm, cheers
Alvaro Uria (aluria) wrote : Posted in a previous version of this proposal | # |
Hi, there are a few things to be reviewed:
* Makefile does not have a unittest target (probably because unit tests didn't exist). However, tox.ini does have a "unit" env. I think the Makefile misses the target, running "tox -e unit"
* Functional tests miss nagios-{{series}} and nrpe-{{series}} in the overlay.yaml.j2 template. It would be good to test when grafana gets related to nrpe, by checking that the check_grafana_
* Functional tests don't test a deployment of cs:grafana + juju upgrade-charm to the local version. I spotted a bug that can be seen at [1]. "nrpe-external-
On this 3rd test (upgrade-charm) it would be could to verify that the old checks (check_
Alvaro Uria (aluria) wrote : Posted in a previous version of this proposal | # |
BTW, when I tried to workaround the issue, by manually setting the nrpe-external-
Xav Paice (xavpaice) wrote : Posted in a previous version of this proposal | # |
Change needs a rebase, and rework to the tests. WIP.
🤖 Canonical IS Merge Bot (canonical-is-mergebot) wrote : | # |
This merge proposal is being monitored by mergebot. Change the status to Approved to merge.
Joe Guo (guoqiao) wrote : | # |
made 2 trivial comments inline.
Paul Goins (vultaire) wrote : | # |
Replying to previous comments:
* Re: unit tests: presently "make unittest" calls "tox -e unit"; this may be because of the rebase that it does work now. That being said, the unit tests fail; one of them needs to be repaired.
* Re: functional tests:
i. NRPE-related tests exist; I'm guessing this is due to the rebase.
ii. I still don't see any upgrade-
iii. Uncertain re: check_grafana-
At a minimum, I think the unit tests should be fixed. I'm not 100% sure whether the upgrade-charm tests should be addressed now or perhaps after the next charm series release.
Xav Paice (xavpaice) wrote : | # |
Unit tests fixed, plus one or to minor lints.
"ii. I still don't see any upgrade-
Testing for upgrade-charm is way out of scope for this change, we don't run that test for a single one of the LMA charm stack right now - though it would be an excellent addition and make life a heck of a lot easier during the release process. I don't think we can reasonably block this change because of something so far out of scope.
Paul Goins (vultaire) wrote : | # |
LGTM (and agreed with scope sentiment)
Xav Paice (xavpaice) wrote : | # |
info: When running lint on this, Black wants to change a number of items, but I've not included that linting in this change since we're going to do a separate commit for lint changes.
🤖 Canonical IS Merge Bot (canonical-is-mergebot) wrote : | # |
Change successfully merged at revision 2078b8c32cf6c26
Preview Diff
1 | diff --git a/.gitignore b/.gitignore |
2 | index 32e2995..34da750 100644 |
3 | --- a/.gitignore |
4 | +++ b/.gitignore |
5 | @@ -20,3 +20,4 @@ repo-info |
6 | |
7 | # reports |
8 | report/* |
9 | +.unit-state.db |
10 | diff --git a/Makefile b/Makefile |
11 | index 60f593c..1334496 100644 |
12 | --- a/Makefile |
13 | +++ b/Makefile |
14 | @@ -21,7 +21,7 @@ lint: |
15 | @echo "Running flake8" |
16 | @tox -e lint |
17 | |
18 | -test: lint |
19 | +unittest: |
20 | @echo "Running unit tests..." |
21 | @tox -e unit |
22 | |
23 | diff --git a/lib/charms/layer/grafana.py b/lib/charms/layer/grafana.py |
24 | index 9853dc2..cb7fd2c 100644 |
25 | --- a/lib/charms/layer/grafana.py |
26 | +++ b/lib/charms/layer/grafana.py |
27 | @@ -32,9 +32,7 @@ SNAP_CODENAME_MAP = { |
28 | |
29 | def get_admin_password(): |
30 | """Get admin password from config, fall back to kv.""" |
31 | - return config("admin_password") or unitdata.kv().get( |
32 | - "grafana.admin_password" |
33 | - ) |
34 | + return config("admin_password") or unitdata.kv().get("grafana.admin_password") |
35 | |
36 | |
37 | def import_dashboard(dashboard, name=None): |
38 | @@ -132,9 +130,7 @@ def check_snap_channel(snap_name, channel): |
39 | UNCHANGED: Channel unchanged |
40 | DOWNGRADE: Would result in downgrade of major version |
41 | """ |
42 | - installed_ver = parse_snap_major_version( |
43 | - snap.get_installed_channel(snap_name) |
44 | - ) |
45 | + installed_ver = parse_snap_major_version(snap.get_installed_channel(snap_name)) |
46 | new_ver = parse_snap_major_version(channel) |
47 | is_refresh_available = snap.is_refresh_available(snap_name) |
48 | if new_ver == -2: |
49 | @@ -163,8 +159,7 @@ def check_snap_channel(snap_name, channel): |
50 | return ChangeStatus.UNCHANGED |
51 | if installed_ver < new_ver: |
52 | msg = ( |
53 | - "PACKAGE UPGRADE REQUIRES MANUAL INTERVENTION: see {} for " |
54 | - "more info" |
55 | + "PACKAGE UPGRADE REQUIRES MANUAL INTERVENTION: see {} for more info" |
56 | ).format(VERSION_CHANGE_URL) |
57 | status_set("blocked", msg) |
58 | log(msg) |
59 | diff --git a/reactive/grafana.py b/reactive/grafana.py |
60 | index 17dfad1..4434b17 100644 |
61 | --- a/reactive/grafana.py |
62 | +++ b/reactive/grafana.py |
63 | @@ -97,6 +97,7 @@ from charms.reactive import ( |
64 | remove_state, |
65 | set_state, |
66 | when, |
67 | + when_any, |
68 | when_not, |
69 | ) |
70 | from charms.reactive.helpers import ( |
71 | @@ -184,8 +185,7 @@ def install_packages(): |
72 | |
73 | if config.changed("install_file") and config.get("install_file"): |
74 | hookenv.status_set( |
75 | - "maintenance", |
76 | - "Installing deb pkgs since install_file changed", |
77 | + "maintenance", "Installing deb pkgs since install_file changed", |
78 | ) |
79 | |
80 | url = config.get("install_file") |
81 | @@ -249,9 +249,7 @@ def install_plugins(): |
82 | |
83 | if os.path.exists(plugins_dir): |
84 | hookenv.log("Cleaning up currently installed plugins") |
85 | - hookenv.status_set( |
86 | - "maintenance", "Cleaning up currently installed plugins" |
87 | - ) |
88 | + hookenv.status_set("maintenance", "Cleaning up currently installed plugins") |
89 | for entry in os.listdir(plugins_dir): |
90 | entry_path = os.path.join(plugins_dir, entry) |
91 | if os.path.isfile(entry_path): |
92 | @@ -283,12 +281,12 @@ def install_plugins(): |
93 | @when_not("grafana.change-block") |
94 | def upgrade_charm(): |
95 | hookenv.status_set( |
96 | - "maintenance", |
97 | - "Forcing package update and reconfiguration on upgrade-charm", |
98 | + "maintenance", "Forcing package update and reconfiguration on upgrade-charm", |
99 | ) |
100 | remove_state("grafana.installed") |
101 | remove_state("grafana.backup.configured") |
102 | remove_state("grafana.configured") |
103 | + remove_state("grafana.nagios-setup.completed") |
104 | |
105 | |
106 | @hook("config-changed") |
107 | @@ -314,9 +312,7 @@ def config_changed(): |
108 | else: |
109 | # Install from requested channel |
110 | snap.install( |
111 | - SNAP_NAME, |
112 | - channel=config["snap_channel"], |
113 | - force_dangerous=False, |
114 | + SNAP_NAME, channel=config["snap_channel"], force_dangerous=False, |
115 | ) |
116 | remove_state("grafana.configured") |
117 | if config.changed("dashboards_backup_schedule") or config.changed( |
118 | @@ -327,6 +323,7 @@ def config_changed(): |
119 | remove_state("grafana.admin_password.set") |
120 | if config.changed("install_file") and config.get("install_file"): |
121 | remove_state("grafana.installed") |
122 | + remove_state("grafana.nagios-setup.completed") |
123 | |
124 | |
125 | def check_ports(new_port): |
126 | @@ -401,15 +398,11 @@ def add_backup_api_keys(): |
127 | "SELECT id FROM api_key WHERE org_id=? AND name=?", [org_id, name], |
128 | ): |
129 | hookenv.log( |
130 | - "API key {} in org {} already exists, skipping".format( |
131 | - name, org_id |
132 | - ) |
133 | + "API key {} in org {} already exists, skipping".format(name, org_id) |
134 | ) |
135 | continue |
136 | j = {"n": name, "k": passwd, "id": org_id} |
137 | - encoded = base64.b64encode(json.dumps(j).encode("ascii")).decode( |
138 | - "ascii" |
139 | - ) |
140 | + encoded = base64.b64encode(json.dumps(j).encode("ascii")).decode("ascii") |
141 | stmt = ( |
142 | "INSERT INTO api_key (org_id, name, key, role, created, updated)" |
143 | + ' VALUES (?,?,?,"Viewer",?,?)' |
144 | @@ -473,9 +466,7 @@ def setup_backup_shedule(): |
145 | |
146 | config = hookenv.config() |
147 | if config.get("dashboards_backup_schedule", False): |
148 | - hookenv.status_set( |
149 | - "maintenance", "Configuring grafana dashboard backup" |
150 | - ) |
151 | + hookenv.status_set("maintenance", "Configuring grafana dashboard backup") |
152 | hookenv.log("Setting up dashboards backup job...") |
153 | host.rsync( |
154 | "files/dashboards_backup", "/usr/local/bin/dashboards_backup", |
155 | @@ -494,9 +485,7 @@ def setup_backup_shedule(): |
156 | group="root", |
157 | perms=0o640, |
158 | ) |
159 | - hookenv.status_set( |
160 | - "active", "Completed configuring grafana dashboard backup" |
161 | - ) |
162 | + hookenv.status_set("active", "Completed configuring grafana dashboard backup") |
163 | set_state("grafana.backup.configured") |
164 | |
165 | |
166 | @@ -518,9 +507,7 @@ def restart_grafana(): |
167 | hookenv.status_set("maintenance", msg) |
168 | hookenv.log(msg) |
169 | host.service_start(svcname) |
170 | - elif any_file_changed([grafana_ini]) or is_state( |
171 | - "config.changed.install_plugins" |
172 | - ): |
173 | + elif any_file_changed([grafana_ini]) or is_state("config.changed.install_plugins"): |
174 | msg = "Restarting {}".format(svcname) |
175 | hookenv.log(msg) |
176 | hookenv.status_set("maintenance", msg) |
177 | @@ -531,30 +518,33 @@ def restart_grafana(): |
178 | |
179 | |
180 | @when("grafana.started") |
181 | -@when("nrpe-external-master.available") |
182 | +@when_any("nrpe-external-master.available", "nrpe-external-master.connected") |
183 | @when_not("grafana.change-block") |
184 | -def update_nrpe_config(svc): |
185 | - kv = unitdata.kv() |
186 | - source = kv.get("install_method") |
187 | - if source in ("snap", "apt"): |
188 | - svcname = SVCNAME[source] |
189 | - else: |
190 | - hookenv.status_set("blocked", "Unsupported install_method") |
191 | - return |
192 | +@when_not("grafana.nagios-setup.completed") |
193 | +def update_nrpe_config(): |
194 | # python-dbus is used by check_upstart_job |
195 | fetch.apt_install("python-dbus", fatal=True) |
196 | hostname = nrpe.get_nagios_hostname() |
197 | - current_unit = nrpe.get_nagios_unit_name() |
198 | nrpe_setup = nrpe.NRPE(hostname=hostname) |
199 | - nrpe.add_init_service_checks(nrpe_setup, [svcname], current_unit) |
200 | + config = hookenv.config() |
201 | + |
202 | + # write the http check |
203 | + nrpe_setup.add_check( |
204 | + "grafana_http", |
205 | + "Grafana HTTP check", |
206 | + "check_http -I 127.0.0.1 -p {} -u /login".format(config["port"]), |
207 | + ) |
208 | nrpe_setup.write() |
209 | + set_state("grafana.nagios-setup.completed") |
210 | |
211 | |
212 | -@when_not("nrpe-external-master.available") |
213 | +@when_not("nrpe-external-master.connected") |
214 | def wipe_nrpe_checks(): |
215 | checks = [ |
216 | "/etc/nagios/nrpe.d/check_grafana-server.cfg", |
217 | "/etc/nagios/nrpe.d/check_grafana.cfg", |
218 | + "/etc/nagios/nrpe.d/check_grafana_http.cfg", |
219 | + "/etc/nagios/nrpe.d/check_snap.grafana.grafana.cfg", |
220 | "/var/lib/nagios/export/service__*_grafana-server.cfg", |
221 | "/var/lib/nagios/export/service__*_grafana.cfg", |
222 | ] |
223 | @@ -562,6 +552,7 @@ def wipe_nrpe_checks(): |
224 | for f in glob.glob(check): |
225 | if os.path.isfile(f): |
226 | os.unlink(f) |
227 | + remove_state("grafana.nagios-setup.completed") |
228 | |
229 | |
230 | @when("grafana.started") |
231 | @@ -602,8 +593,7 @@ def sources_gone(relation): |
232 | conn = sqlite3.connect("{}/grafana.db".format(data_dir), timeout=30) |
233 | cur = conn.cursor() |
234 | cur.execute( |
235 | - "DELETE FROM DATA_SOURCE WHERE type=? AND url=?", |
236 | - (ds["type"], ds["url"]), |
237 | + "DELETE FROM DATA_SOURCE WHERE type=? AND url=?", (ds["type"], ds["url"]), |
238 | ) |
239 | conn.commit() |
240 | cur.close() |
241 | @@ -668,16 +658,12 @@ def check_datasource(ds): |
242 | return |
243 | conn = sqlite3.connect("{}/grafana.db".format(data_dir), timeout=30) |
244 | cur = conn.cursor() |
245 | - query = cur.execute( |
246 | - "SELECT id, type, name, url, is_default FROM DATA_SOURCE" |
247 | - ) |
248 | + query = cur.execute("SELECT id, type, name, url, is_default FROM DATA_SOURCE") |
249 | rows = query.fetchall() |
250 | ds_name = "{} - {}".format(ds["service_name"], ds["description"]) |
251 | for row in rows: |
252 | if row[1] == ds["type"] and row[3] == ds["url"]: |
253 | - hookenv.log( |
254 | - "Datasource already exist, updating: {}".format(ds_name) |
255 | - ) |
256 | + hookenv.log("Datasource already exist, updating: {}".format(ds_name)) |
257 | stmt, values = generate_query(ds, row[4], id=row[0]) |
258 | print(stmt, values) |
259 | cur.execute(stmt, values) |
260 | @@ -705,16 +691,12 @@ def render_custom(source, context, **parameters): |
261 | template_folder = os.path.join( |
262 | hookenv.charm_dir(), "templates/dashboards/prometheus" |
263 | ) |
264 | - environment = Environment( |
265 | - loader=FileSystemLoader(template_folder), **parameters |
266 | - ) |
267 | + environment = Environment(loader=FileSystemLoader(template_folder), **parameters) |
268 | try: |
269 | template = environment.get_template(source) |
270 | except exceptions.TemplateNotFound as e: |
271 | hookenv.log( |
272 | - "Could not load template {} from {}".format( |
273 | - source, template_folder |
274 | - ) |
275 | + "Could not load template {} from {}".format(source, template_folder) |
276 | ) |
277 | raise e |
278 | return template.render(context) |
279 | @@ -817,9 +799,7 @@ def check_and_add_dashboard( |
280 | dashboard_changed = diff(new, curr) |
281 | if not dashboard_changed: |
282 | hookenv.log( |
283 | - "Skipping Dashboard Template: already up to date: {}".format( |
284 | - filename |
285 | - ) |
286 | + "Skipping Dashboard Template: already up to date: {}".format(filename) |
287 | ) |
288 | return |
289 | |
290 | @@ -835,15 +815,11 @@ def check_and_add_dashboard( |
291 | |
292 | hookenv.log("Using Dashboard Template: {}".format(filename)) |
293 | post_req = "http://127.0.0.1:{}/api/dashboards/db".format(port) |
294 | - r = requests.post( |
295 | - post_req, json=dashboard_json, auth=("admin", gf_adminpasswd) |
296 | - ) |
297 | + r = requests.post(post_req, json=dashboard_json, auth=("admin", gf_adminpasswd)) |
298 | |
299 | if r.status_code != 200: |
300 | hookenv.log( |
301 | - "Posting template {} failed with error: {}".format( |
302 | - filename, r.text |
303 | - ), |
304 | + "Posting template {} failed with error: {}".format(filename, r.text), |
305 | hookenv.ERROR, |
306 | ) |
307 | |
308 | @@ -862,9 +838,7 @@ def generate_prometheus_dashboards(gf_adminpasswd, ds): |
309 | config = hookenv.config() |
310 | |
311 | # https://prometheus.io/docs/prometheus/latest/querying/api/#querying-label-values |
312 | - response = requests.get( |
313 | - "{}/api/v1/label/__name__/values".format(ds["url"]) |
314 | - ) |
315 | + response = requests.get("{}/api/v1/label/__name__/values".format(ds["url"])) |
316 | if response.status_code != 200: |
317 | hookenv.log( |
318 | "Could not reach prometheus API status code: " |
319 | @@ -1105,9 +1079,7 @@ def check_adminuser(): |
320 | conn.commit() |
321 | hookenv.log("[*] admin password updated on database") |
322 | else: |
323 | - hookenv.log( |
324 | - "Could not update user table: hpwgen func failed" |
325 | - ) |
326 | + hookenv.log("Could not update user table: hpwgen func failed") |
327 | break |
328 | conn.close() |
329 | except sqlite3.OperationalError as e: |
330 | diff --git a/tests/functional/tests/test_grafana.py b/tests/functional/tests/test_grafana.py |
331 | index 31d3b4d..06a1942 100644 |
332 | --- a/tests/functional/tests/test_grafana.py |
333 | +++ b/tests/functional/tests/test_grafana.py |
334 | @@ -27,9 +27,7 @@ class BaseGrafanaTest(unittest.TestCase): |
335 | cls.lead_unit_name = model.get_lead_unit_name( |
336 | cls.application_name, model_name=cls.model_name |
337 | ) |
338 | - cls.units = model.get_units( |
339 | - cls.application_name, model_name=cls.model_name |
340 | - ) |
341 | + cls.units = model.get_units(cls.application_name, model_name=cls.model_name) |
342 | cls.grafana_ip = model.get_app_ips(cls.application_name)[0] |
343 | |
344 | def get_unit_status(self, unit): |
345 | @@ -84,9 +82,7 @@ class CharmOperationTest(BaseGrafanaTest): |
346 | We'll retry until the TEST_TIMEOUT. |
347 | """ |
348 | test_command = "{} -I 127.0.0.1 -p {} -u {}".format( |
349 | - "/usr/lib/nagios/plugins/check_http", |
350 | - DEFAULT_API_PORT, |
351 | - DEFAULT_API_URL, |
352 | + "/usr/lib/nagios/plugins/check_http", DEFAULT_API_PORT, DEFAULT_API_URL, |
353 | ) |
354 | timeout = time.time() + TEST_TIMEOUT |
355 | while time.time() < timeout: |
356 | @@ -94,9 +90,7 @@ class CharmOperationTest(BaseGrafanaTest): |
357 | if response["Code"] == "0": |
358 | return |
359 | logging.warning( |
360 | - "Unexpected check_http response: {}. Retrying in 30s.".format( |
361 | - response |
362 | - ) |
363 | + "Unexpected check_http response: {}. Retrying in 30s.".format(response) |
364 | ) |
365 | time.sleep(30) |
366 | |
367 | @@ -104,9 +98,7 @@ class CharmOperationTest(BaseGrafanaTest): |
368 | self.fail( |
369 | "http port didn't respond to the command \n" |
370 | "'{test_command}' as expected.\n" |
371 | - "Result: {result}".format( |
372 | - test_command=test_command, result=response |
373 | - ) |
374 | + "Result: {result}".format(test_command=test_command, result=response) |
375 | ) |
376 | |
377 | def test_02_nrpe_http_check(self): |
378 | @@ -114,21 +106,11 @@ class CharmOperationTest(BaseGrafanaTest): |
379 | logging.debug( |
380 | "Verify the nrpe check is created and has the required content..." |
381 | ) |
382 | - # Check if this is snap or apt |
383 | - cmd = "config-get install_method" |
384 | - install_method = model.run_on_unit(self.lead_unit_name, cmd) |
385 | - if "apt" in install_method["Stdout"]: |
386 | - expected_nrpe_check = ( |
387 | - "command[check_grafana-server]=/usr/local/lib/nagios/" |
388 | - "plugins/check_systemd.py grafana-server" |
389 | - ) |
390 | - cmd = "cat /etc/nagios/nrpe.d/check_grafana-server.cfg" |
391 | - else: # must be snap |
392 | - expected_nrpe_check = ( |
393 | - "command[check_snap.grafana.grafana]=/usr/local/lib/" |
394 | - "nagios/plugins/check_systemd.py snap.grafana.grafana" |
395 | - ) |
396 | - cmd = "cat /etc/nagios/nrpe.d/check_snap.grafana.grafana.cfg" |
397 | + expected_nrpe_check = ( |
398 | + "command[check_grafana_http]=/usr/lib/nagios/plugins/check_http " |
399 | + "-I 127.0.0.1 -p 3000 -u /login" |
400 | + ) |
401 | + cmd = "cat /etc/nagios/nrpe.d/check_grafana_http.cfg" |
402 | result = model.run_on_unit(self.lead_unit_name, cmd) |
403 | code = result.get("Code") |
404 | if code != "0": |
405 | @@ -232,9 +214,7 @@ class SnappedGrafanaTest(BaseGrafanaTest): |
406 | model.block_until_unit_wl_status(self.lead_unit_name, "blocked") |
407 | model.block_until_all_units_idle() |
408 | status_message = self.get_unit_status(self.lead_unit_name) |
409 | - self.assertTrue( |
410 | - "PACKAGE DOWNGRADES ARE NOT SUPPORTED" in status_message |
411 | - ) |
412 | + self.assertTrue("PACKAGE DOWNGRADES ARE NOT SUPPORTED" in status_message) |
413 | model.set_application_config( # unblock unit again |
414 | self.application_name, {"snap_channel": "7/stable"} |
415 | ) |
416 | diff --git a/tests/unit/requirements.txt b/tests/unit/requirements.txt |
417 | index 29f9318..cb975bd 100644 |
418 | --- a/tests/unit/requirements.txt |
419 | +++ b/tests/unit/requirements.txt |
420 | @@ -4,3 +4,6 @@ mock |
421 | pytest |
422 | pytest-cov |
423 | requests |
424 | +jsondiff |
425 | +pbkdf2 |
426 | +pycrypto |
427 | diff --git a/tests/unit/test_grafana.py b/tests/unit/test_grafana.py |
428 | index 57e730d..0c8a658 100644 |
429 | --- a/tests/unit/test_grafana.py |
430 | +++ b/tests/unit/test_grafana.py |
431 | @@ -1,18 +1,18 @@ |
432 | -from os.path import isfile |
433 | -from unittest import mock |
434 | import sys |
435 | import unittest |
436 | - |
437 | +from os.path import isfile |
438 | +from unittest import mock |
439 | +from unittest.mock import call |
440 | |
441 | sys.modules["charms.layer.snap"] = mock.Mock() |
442 | |
443 | - |
444 | from charms.layer.grafana import ( # noqa E402 |
445 | download_file, |
446 | get_cmd_output, |
447 | get_deb_package_version, |
448 | get_installed_package_version, |
449 | ) |
450 | +from reactive import grafana # noqa E402 |
451 | |
452 | |
453 | class GrafanaTestCase(unittest.TestCase): |
454 | @@ -45,3 +45,26 @@ class GrafanaTestCase(unittest.TestCase): |
455 | self.assertTrue(isfile(path)) |
456 | version = get_deb_package_version(path) |
457 | self.assertEqual(version, "2.10-2") |
458 | + |
459 | + @mock.patch("charmhelpers.fetch.apt_install") |
460 | + @mock.patch("charmhelpers.contrib.charmsupport.nrpe.NRPE") |
461 | + @mock.patch("charmhelpers.contrib.charmsupport.nrpe.get_nagios_hostname") |
462 | + @mock.patch("charmhelpers.core.hookenv.config") |
463 | + def test_update_nrpe_config( |
464 | + self, mock_hookenv_config, mock_get_nagios_hostname, mock_nrpe, *args |
465 | + ): |
466 | + sys.modules["charms.layer"] = mock.MagicMock() |
467 | + sys.modules["charms.layer.grafana"] = mock.MagicMock() |
468 | + mock_hookenv_config.return_value = {"port": 1234} |
469 | + mock_get_nagios_hostname.return_value = "testunit" |
470 | + grafana.update_nrpe_config() |
471 | + calls = [ |
472 | + call(hostname="testunit"), |
473 | + call().add_check( |
474 | + "grafana_http", |
475 | + "Grafana HTTP check", |
476 | + "check_http -I 127.0.0.1 -p 1234 -u /login", |
477 | + ), |
478 | + call().write(), |
479 | + ] |
480 | + mock_nrpe.assert_has_calls(calls) |
481 | diff --git a/tox.ini b/tox.ini |
482 | index 486c489..c62ec88 100644 |
483 | --- a/tox.ini |
484 | +++ b/tox.ini |
485 | @@ -6,7 +6,7 @@ skip_missing_interpreters = True |
486 | [testenv] |
487 | basepython = python3 |
488 | setenv = |
489 | - PYTHONPATH = . |
490 | + PYTHONPATH = .:lib |
491 | passenv = |
492 | HOME |
493 | CHARM_BUILD_DIR |
494 | @@ -24,7 +24,6 @@ commands = pytest -v --ignore {toxinidir}/tests/functional \ |
495 | --cov-report=html:report/html |
496 | deps = -r{toxinidir}/tests/unit/requirements.txt |
497 | -r{toxinidir}/requirements.txt |
498 | -setenv = PYTHONPATH={toxinidir}/lib |
499 | |
500 | [testenv:func] |
501 | changedir = {toxinidir}/tests/functional |
502 | @@ -39,7 +38,7 @@ deps = -r{toxinidir}/tests/functional/requirements.txt |
503 | |
504 | [testenv:lint] |
505 | commands = |
506 | - black --check --line-length 79 reactive lib tests |
507 | + black --check --line-length 88 reactive lib tests |
508 | flake8 |
509 | deps = |
510 | black |
511 | @@ -75,5 +74,5 @@ exclude = |
512 | # I100: Import statements are in the wrong order |
513 | |
514 | ignore = H405,D100,D101,D102,D103,D104,D105,D107,D200,D202,D203,D204,D205,D208,D400,D401,I100,I201,W503,W504 |
515 | -max-line-length = 79 |
516 | +max-line-length = 88 |
517 | max-complexity = 10 |
This merge proposal is being monitored by mergebot. Change the status to Approved to merge.