Merge ~xavpaice/charm-nagios:lint-20.08 into ~nagios-charmers/charm-nagios:master

Proposed by Xav Paice
Status: Merged
Merged at revision: b27a20af88bd66c4c5d43b0903b30fa69fd6abd5
Proposed branch: ~xavpaice/charm-nagios:lint-20.08
Merge into: ~nagios-charmers/charm-nagios:master
Prerequisite: ~xavpaice/charm-nagios:blacken-20.08
Diff against target: 1205 lines (+185/-107)
9 files modified
hooks/common.py (+62/-11)
hooks/monitors_relation_changed.py (+25/-12)
hooks/upgrade_charm.py (+62/-25)
hooks/website_relation_joined.py (+3/-6)
tests/functional/conftest.py (+25/-19)
tests/functional/test_config.py (+6/-8)
tests/functional/test_deploy.py (+1/-0)
tests/unit/test_website_relation_joined.py (+1/-0)
tox.ini (+0/-26)
Reviewer Review Type Date Requested Status
Alvaro Uria (community) Approve
Celia Wang Approve
Review via email: mp+388634@code.launchpad.net

Commit message

Extra Linting completed for 20.08 charm release

To post a comment you must log in.
Revision history for this message
Celia Wang (ziyiwang) wrote :

LGTM

review: Approve
Revision history for this message
Alvaro Uria (aluria) wrote :

+1 this and the other 2 related MPs (blacken and makefile)

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/hooks/common.py b/hooks/common.py
2index 3bff942..b320880 100644
3--- a/hooks/common.py
4+++ b/hooks/common.py
5@@ -1,17 +1,17 @@
6-import subprocess
7-import socket
8 import os
9 import os.path
10 import re
11 import shutil
12+import socket
13+import subprocess
14 import tempfile
15
16 from charmhelpers.core.hookenv import (
17+ config,
18 log,
19 network_get,
20 network_get_primary_address,
21 unit_get,
22- config,
23 )
24
25 from pynag import Model
26@@ -28,16 +28,18 @@ PLUGIN_PATH = "/usr/lib/nagios/plugins"
27 Model.cfg_file = INPROGRESS_CFG
28 Model.pynag_directory = INPROGRESS_CONF_D
29
30-reduce_RE = re.compile(r"[\W_]")
31+REDUCE_RE = re.compile(r"[\W_]")
32
33
34 def check_ip(n):
35 try:
36 socket.inet_pton(socket.AF_INET, n)
37+
38 return True
39 except socket.error:
40 try:
41 socket.inet_pton(socket.AF_INET6, n)
42+
43 return True
44 except socket.error:
45 return False
46@@ -48,10 +50,12 @@ def get_local_ingress_address(binding="website"):
47 log("Getting hostname for binding %s" % binding)
48 try:
49 network_info = network_get(binding)
50+
51 if network_info is not None and "ingress-addresses" in network_info:
52 log("Using ingress-addresses")
53 hostname = network_info["ingress-addresses"][0]
54 log(hostname)
55+
56 return hostname
57 except NotImplementedError:
58 # We'll fallthrough to the Pre 2.3 code below.
59@@ -66,29 +70,36 @@ def get_local_ingress_address(binding="website"):
60 hostname = unit_get("private-address")
61 log("Using unit_get private address")
62 log(hostname)
63+
64 return hostname
65
66
67 def get_remote_relation_attr(remote_unit, attr_name, relation_id=None):
68 args = ["relation-get", attr_name, remote_unit]
69+
70 if relation_id is not None:
71 args.extend(["-r", relation_id])
72+
73 return subprocess.check_output(args).strip()
74
75
76 def get_ip_and_hostname(remote_unit, relation_id=None):
77 hostname = get_remote_relation_attr(remote_unit, "ingress-address", relation_id)
78+
79 if hostname is None or not len(hostname):
80 hostname = get_remote_relation_attr(remote_unit, "private-address", relation_id)
81
82 if hostname is None or not len(hostname):
83 log("relation-get failed")
84+
85 return 2
86+
87 if check_ip(hostname):
88 # Some providers don't provide hostnames, so use the remote unit name.
89 ip_address = hostname
90 else:
91 ip_address = socket.getaddrinfo(hostname, None)[0][4][0]
92+
93 return (ip_address, remote_unit.replace("/", "-"))
94
95
96@@ -98,11 +109,13 @@ def refresh_hostgroups(): # noqa:C901
97 hosts = [x["host_name"] for x in Model.Host.objects.all if x["host_name"]]
98
99 hgroups = {}
100+
101 for host in hosts:
102 try:
103 (service, unit_id) = host.rsplit("-", 1)
104 except ValueError:
105 continue
106+
107 if service in hgroups:
108 hgroups[service].append(host)
109 else:
110@@ -114,6 +127,7 @@ def refresh_hostgroups(): # noqa:C901
111
112 # Delete the ones not in hgroups
113 to_delete = set(auto_hgroups).difference(set(hgroups.keys()))
114+
115 for hgroup_name in to_delete:
116 try:
117 hgroup = Model.Hostgroup.objects.get_by_shortname(hgroup_name)
118@@ -138,7 +152,7 @@ def _make_check_command(args):
119 args = [str(arg) for arg in args]
120 # There is some worry of collision, but the uniqueness of the initial
121 # command should be enough.
122- signature = reduce_RE.sub("_", "".join([os.path.basename(arg) for arg in args]))
123+ signature = REDUCE_RE.sub("_", "".join([os.path.basename(arg) for arg in args]))
124 Model.Command.objects.reload_cache()
125 try:
126 cmd = Model.Command.objects.get_by_shortname(signature)
127@@ -147,6 +161,7 @@ def _make_check_command(args):
128 cmd.set_attribute("command_name", signature)
129 cmd.set_attribute("command_line", " ".join(args))
130 cmd.save()
131+
132 return signature
133
134
135@@ -163,19 +178,23 @@ def customize_http(service, name, extra):
136 path = extra.get("path", "/")
137 args = [port, path]
138 cmd_args = [plugin, "-p", '"$ARG1$"', "-u", '"$ARG2$"']
139+
140 if "status" in extra:
141 _extend_args(args, cmd_args, "-e", extra["status"])
142+
143 if "host" in extra:
144 _extend_args(args, cmd_args, "-H", extra["host"])
145 cmd_args.extend(("-I", "$HOSTADDRESS$"))
146 else:
147 cmd_args.extend(("-H", "$HOSTADDRESS$"))
148 check_timeout = config("check_timeout")
149+
150 if check_timeout is not None:
151 cmd_args.extend(("-t", check_timeout))
152 check_command = _make_check_command(cmd_args)
153 cmd = "%s!%s" % (check_command, "!".join([str(x) for x in args]))
154 service.set_attribute("check_command", cmd)
155+
156 return True
157
158
159@@ -183,16 +202,20 @@ def customize_mysql(service, name, extra):
160 plugin = os.path.join(PLUGIN_PATH, "check_mysql")
161 args = []
162 cmd_args = [plugin, "-H", "$HOSTADDRESS$"]
163+
164 if "user" in extra:
165 _extend_args(args, cmd_args, "-u", extra["user"])
166+
167 if "password" in extra:
168 _extend_args(args, cmd_args, "-p", extra["password"])
169 check_timeout = config("check_timeout")
170+
171 if check_timeout is not None:
172 cmd_args.extend(("-t", check_timeout))
173 check_command = _make_check_command(cmd_args)
174 cmd = "%s!%s" % (check_command, "!".join([str(x) for x in args]))
175 service.set_attribute("check_command", cmd)
176+
177 return True
178
179
180@@ -201,11 +224,13 @@ def customize_pgsql(service, name, extra):
181 args = []
182 cmd_args = [plugin, "-H", "$HOSTADDRESS$"]
183 check_timeout = config("check_timeout")
184+
185 if check_timeout is not None:
186 cmd_args.extend(("-t", check_timeout))
187 check_command = _make_check_command(cmd_args)
188 cmd = "%s!%s" % (check_command, "!".join([str(x) for x in args]))
189 service.set_attribute("check_command", cmd)
190+
191 return True
192
193
194@@ -213,6 +238,7 @@ def customize_nrpe(service, name, extra):
195 plugin = os.path.join(PLUGIN_PATH, "check_nrpe")
196 args = []
197 cmd_args = [plugin, "-H", "$HOSTADDRESS$"]
198+
199 if name in ("mem", "swap"):
200 cmd_args.extend(("-c", "check_%s" % name))
201 elif "command" in extra:
202@@ -220,61 +246,77 @@ def customize_nrpe(service, name, extra):
203 else:
204 cmd_args.extend(("-c", extra))
205 check_timeout = config("check_timeout")
206+
207 if check_timeout is not None:
208 cmd_args.extend(("-t", check_timeout))
209 check_command = _make_check_command(cmd_args)
210 cmd = "%s!%s" % (check_command, "!".join([str(x) for x in args]))
211 service.set_attribute("check_command", cmd)
212+
213 return True
214
215
216 def customize_rpc(service, name, extra):
217- """ Customize the check_rpc plugin to check things like nfs."""
218+ """Customize the check_rpc plugin to check things like nfs."""
219 plugin = os.path.join(PLUGIN_PATH, "check_rpc")
220 args = []
221 # /usr/lib/nagios/plugins/check_rpc -H <host> -C <rpc_command>
222 cmd_args = [plugin, "-H", "$HOSTADDRESS$"]
223+
224 if "rpc_command" in extra:
225 cmd_args.extend(("-C", extra["rpc_command"]))
226+
227 if "program_version" in extra:
228 cmd_args.extend(("-c", extra["program_version"]))
229
230 check_command = _make_check_command(cmd_args)
231 cmd = "%s!%s" % (check_command, "!".join([str(x) for x in args]))
232 service.set_attribute("check_command", cmd)
233+
234 return True
235
236
237 def customize_tcp(service, name, extra):
238- """ Customize tcp can be used to check things like memcached. """
239+ """Customize tcp can be used to check things like memcached."""
240 plugin = os.path.join(PLUGIN_PATH, "check_tcp")
241 args = []
242 # /usr/lib/nagios/plugins/check_tcp -H <host> -E
243 cmd_args = [plugin, "-H", "$HOSTADDRESS$", "-E"]
244+
245 if "port" in extra:
246 cmd_args.extend(("-p", extra["port"]))
247+
248 if "string" in extra:
249 cmd_args.extend(("-s", "'{}'".format(extra["string"])))
250+
251 if "expect" in extra:
252 cmd_args.extend(("-e", extra["expect"]))
253+
254 if "warning" in extra:
255 cmd_args.extend(("-w", extra["warning"]))
256+
257 if "critical" in extra:
258 cmd_args.extend(("-c", extra["critical"]))
259+
260 if "timeout" in extra:
261 cmd_args.extend(("-t", extra["timeout"]))
262 check_timeout = config("check_timeout")
263+
264 if check_timeout is not None:
265 cmd_args.extend(("-t", check_timeout))
266
267 check_command = _make_check_command(cmd_args)
268 cmd = "%s!%s" % (check_command, "!".join([str(x) for x in args]))
269 service.set_attribute("check_command", cmd)
270+
271 return True
272
273
274 def customize_service(service, family, name, extra):
275- """ The monitors.yaml names are mapped to methods that customize services. """
276+ """Map names to service methods.
277+
278+ The monitors.yaml names are mapped to methods that customize services.
279+ """
280 customs = {
281 "http": customize_http,
282 "mysql": customize_mysql,
283@@ -283,17 +325,19 @@ def customize_service(service, family, name, extra):
284 "rpc": customize_rpc,
285 "pgsql": customize_pgsql,
286 }
287+
288 if family in customs:
289 return customs[family](service, name, extra)
290+
291 return False
292
293
294 def update_localhost():
295- """ Update the localhost definition to use the ubuntu icons."""
296-
297+ """Update the localhost definition to use the ubuntu icons."""
298 Model.cfg_file = MAIN_NAGIOS_CFG
299 Model.pynag_directory = os.path.join(MAIN_NAGIOS_DIR, "conf.d")
300 hosts = Model.Host.objects.filter(host_name="localhost", object_type="host")
301+
302 for host in hosts:
303 host.icon_image = "base/ubuntu.png"
304 host.icon_image_alt = "Ubuntu Linux"
305@@ -318,6 +362,7 @@ def get_pynag_host(target_id, owner_unit=None, owner_relation=None):
306 host.save()
307 host = Model.Host.objects.get_by_shortname(target_id)
308 apply_host_policy(target_id, owner_unit, owner_relation)
309+
310 return host
311
312
313@@ -325,6 +370,7 @@ def get_pynag_service(target_id, service_name):
314 services = Model.Service.objects.filter(
315 host_name=target_id, service_description=service_name
316 )
317+
318 if len(services) == 0:
319 service = Model.Service()
320 service.set_filename(CHARM_CFG)
321@@ -333,6 +379,7 @@ def get_pynag_service(target_id, service_name):
322 service.set_attribute("use", "generic-service")
323 else:
324 service = services[0]
325+
326 return service
327
328
329@@ -369,6 +416,7 @@ def initialize_inprogress_config():
330 shutil.rmtree(INPROGRESS_DIR)
331 shutil.copytree(MAIN_NAGIOS_DIR, INPROGRESS_DIR)
332 _replace_in_config(MAIN_NAGIOS_DIR, INPROGRESS_DIR)
333+
334 if os.path.exists(CHARM_CFG):
335 os.unlink(CHARM_CFG)
336
337@@ -376,10 +424,13 @@ def initialize_inprogress_config():
338 def flush_inprogress_config():
339 if not os.path.exists(INPROGRESS_DIR):
340 return
341+
342 if os.path.exists(MAIN_NAGIOS_BAK):
343 shutil.rmtree(MAIN_NAGIOS_BAK)
344+
345 if os.path.exists(MAIN_NAGIOS_DIR):
346 shutil.move(MAIN_NAGIOS_DIR, MAIN_NAGIOS_BAK)
347 shutil.move(INPROGRESS_DIR, MAIN_NAGIOS_DIR)
348- # now that directory has been changed need to update the config file to reflect the real stuff..
349+ # now that directory has been changed need to update the config file to
350+ # reflect the real stuff..
351 _commit_in_config(INPROGRESS_DIR, MAIN_NAGIOS_DIR)
352diff --git a/hooks/monitors_relation_changed.py b/hooks/monitors_relation_changed.py
353index e3d5a5a..6df3303 100755
354--- a/hooks/monitors_relation_changed.py
355+++ b/hooks/monitors_relation_changed.py
356@@ -16,36 +16,32 @@
357 # You should have received a copy of the GNU General Public License
358 # along with this program. If not, see <http://www.gnu.org/licenses/>.
359
360-import sys
361 import os
362-import yaml
363 import re
364+import sys
365 from collections import defaultdict
366
367 from charmhelpers.core.hookenv import (
368- relation_get,
369+ DEBUG,
370 ingress_address,
371+ log,
372 related_units,
373+ relation_get,
374 relation_ids,
375- log,
376- DEBUG,
377 )
378
379 from common import (
380 customize_service,
381+ flush_inprogress_config,
382 get_pynag_host,
383 get_pynag_service,
384- refresh_hostgroups,
385 initialize_inprogress_config,
386- flush_inprogress_config,
387+ refresh_hostgroups,
388 )
389
390+import yaml
391
392-REQUIRED_REL_DATA_KEYS = [
393- "target-address",
394- "monitors",
395- "target-id",
396-]
397+REQUIRED_REL_DATA_KEYS = ["target-address", "monitors", "target-id"]
398
399
400 def _prepare_relation_data(unit, rid):
401@@ -56,6 +52,7 @@ def _prepare_relation_data(unit, rid):
402 unit, rid
403 )
404 log(msg, level=DEBUG)
405+
406 return {}
407
408 if rid.split(":")[0] == "nagios":
409@@ -76,6 +73,7 @@ def _prepare_relation_data(unit, rid):
410 key, unit, rid
411 )
412 log(msg, level=DEBUG)
413+
414 return {}
415
416 return relation_data
417@@ -83,10 +81,12 @@ def _prepare_relation_data(unit, rid):
418
419 def _collect_relation_data():
420 all_relations = defaultdict(dict)
421+
422 for relname in ["nagios", "monitors"]:
423 for relid in relation_ids(relname):
424 for unit in related_units(relid):
425 relation_data = _prepare_relation_data(unit=unit, rid=relid)
426+
427 if relation_data:
428 all_relations[relid][unit] = relation_data
429
430@@ -98,8 +98,10 @@ def main(argv): # noqa: C901
431 # and target-address' so the hook can be tested without being in a hook
432 # context.
433 #
434+
435 if len(argv) > 1:
436 relation_settings = {"monitors": open(argv[1]).read(), "target-id": argv[2]}
437+
438 if len(argv) > 3:
439 relation_settings["target-address"] = argv[3]
440 all_relations = {"monitors:99": {"testing/0": relation_settings}}
441@@ -108,11 +110,13 @@ def main(argv): # noqa: C901
442
443 # Hack to work around http://pad.lv/1025478
444 targets_with_addresses = set()
445+
446 for relid, units in all_relations.iteritems():
447 for unit, relation_settings in units.items():
448 if "target-id" in relation_settings:
449 targets_with_addresses.add(relation_settings["target-id"])
450 new_all_relations = {}
451+
452 for relid, units in all_relations.iteritems():
453 for unit, relation_settings in units.items():
454 if relation_settings["target-id"] in targets_with_addresses:
455@@ -124,11 +128,14 @@ def main(argv): # noqa: C901
456 initialize_inprogress_config()
457 # make a dict of machine ids to target-id hostnames
458 all_hosts = {}
459+
460 for relid, units in all_relations.items():
461 for unit, relation_settings in units.iteritems():
462 machine_id = relation_settings.get("machine_id", None)
463+
464 if machine_id:
465 all_hosts[machine_id] = relation_settings["target-id"]
466+
467 for relid, units in all_relations.items():
468 apply_relation_config(relid, units, all_hosts)
469 refresh_hostgroups()
470@@ -142,10 +149,13 @@ def apply_relation_config(relid, units, all_hosts): # noqa: C901
471 target_id = relation_settings["target-id"]
472 machine_id = relation_settings.get("machine_id", None)
473 parent_host = None
474+
475 if machine_id:
476 container_regex = re.compile(r"(\d+)/lx[cd]/\d+")
477+
478 if container_regex.search(machine_id):
479 parent_machine = container_regex.search(machine_id).group(1)
480+
481 if parent_machine in all_hosts:
482 parent_host = all_hosts[parent_machine]
483
484@@ -159,9 +169,11 @@ def apply_relation_config(relid, units, all_hosts): # noqa: C901
485
486 # Output nagios config
487 host = get_pynag_host(target_id)
488+
489 if not target_address:
490 raise Exception("No Target Address provied by NRPE service!")
491 host.set_attribute("address", target_address)
492+
493 if parent_host:
494 # We assume that we only want one parent and will overwrite any
495 # existing parents for this host.
496@@ -172,6 +184,7 @@ def apply_relation_config(relid, units, all_hosts): # noqa: C901
497 for mon_name, mon in mons.iteritems():
498 service_name = "%s-%s" % (target_id, mon_name)
499 service = get_pynag_service(target_id, service_name)
500+
501 if customize_service(service, mon_family, mon_name, mon):
502 service.save()
503 else:
504diff --git a/hooks/upgrade_charm.py b/hooks/upgrade_charm.py
505index 5091f9a..ca410ef 100755
506--- a/hooks/upgrade_charm.py
507+++ b/hooks/upgrade_charm.py
508@@ -3,25 +3,26 @@
509 # Rewritten from bash to python 3/2/2014 for charm helper inclusion
510 # of SSL-Everywhere!
511 import base64
512-from jinja2 import Template
513+import errno
514 import glob
515+import grp
516 import os
517-
518-# import re
519 import pwd
520-import grp
521-import string
522-import stat
523-import errno
524 import shutil
525+import stat
526+import string
527 import subprocess
528-import yaml
529+
530+from charmhelpers import fetch
531 from charmhelpers.contrib import ssl
532 from charmhelpers.core import hookenv, host
533-from charmhelpers import fetch
534
535 from common import update_localhost
536
537+from jinja2 import Template
538+
539+import yaml
540+
541 # Gather facts
542 legacy_relations = hookenv.config("legacy")
543 extra_config = hookenv.config("extraconfig")
544@@ -55,7 +56,7 @@ SSL_CONFIGURED = ssl_config in ["on", "only", "true"]
545
546
547 def warn_legacy_relations():
548- """Checks the charm relations for legacy relations.
549+ """Check the charm relations for legacy relations.
550
551 Inserts warnings into the log about legacy relations, as they will be removed
552 in the future
553@@ -70,7 +71,7 @@ def warn_legacy_relations():
554
555
556 def parse_extra_contacts(yaml_string):
557- """Parses a list of extra Nagios contacts from a YAML string.
558+ """Parse a list of extra Nagios contacts from a YAML string.
559
560 Does basic sanitization only
561 """
562@@ -82,6 +83,7 @@ def parse_extra_contacts(yaml_string):
563
564 try:
565 extra_contacts_raw = yaml.load(yaml_string, Loader=yaml.SafeLoader) or []
566+
567 if not isinstance(extra_contacts_raw, list):
568 raise ValueError("not a list")
569
570@@ -90,6 +92,7 @@ def parse_extra_contacts(yaml_string):
571 hookenv.log(
572 "Contact {} is missing fields.".format(contact), hookenv.WARNING
573 )
574+
575 continue
576
577 if set(contact["name"]) > set(valid_name_chars):
578@@ -97,10 +100,12 @@ def parse_extra_contacts(yaml_string):
579 "Contact name {} is illegal".format(contact["name"]),
580 hookenv.WARNING,
581 )
582+
583 continue
584
585 if "\n" in (contact["host"] + contact["service"]):
586 hookenv.log("Line breaks not allowed in commands", hookenv.WARNING)
587+
588 continue
589
590 contact["name"] = contact["name"].lower()
591@@ -111,6 +116,7 @@ def parse_extra_contacts(yaml_string):
592 hookenv.log(
593 'Invalid "extra_contacts" configuration: {}'.format(e), hookenv.WARNING
594 )
595+
596 if len(extra_contacts_raw) != len(extra_contacts):
597 hookenv.log(
598 "Invalid extra_contacts config, found {} contacts defined, "
599@@ -125,10 +131,12 @@ def parse_extra_contacts(yaml_string):
600 # proper nagios3 configuration file, otherwise remove the config
601 def write_extra_config():
602 # Be predjudice about this - remove the file always.
603+
604 if host.file_hash("/etc/nagios3/conf.d/extra.cfg") is not None:
605 os.remove("/etc/nagios3/conf.d/extra.cfg")
606 # If we have a config, then write it. the hook reconfiguration will
607 # handle the details
608+
609 if extra_config is not None:
610 host.write_file("/etc/nagios3/conf.d/extra.cfg", extra_config)
611
612@@ -150,6 +158,7 @@ def fixpath(path):
613 if os.path.isdir(path):
614 st = os.stat(path)
615 os.chmod(path, st.st_mode | stat.S_IXOTH)
616+
617 if path != "/":
618 fixpath(os.path.split(path)[0])
619
620@@ -163,6 +172,7 @@ def enable_livestatus_config():
621 # Make the directory and fix perms on it
622 hookenv.log("Fixing perms on livestatus_path")
623 livestatus_dir = os.path.dirname(livestatus_path)
624+
625 if not os.path.isdir(livestatus_dir):
626 hookenv.log("Making path for livestatus_dir")
627 mkdir_p(livestatus_dir)
628@@ -202,16 +212,16 @@ def enable_pagerduty_config():
629 }
630
631 with open("hooks/templates/pagerduty_nagios_cfg.tmpl", "r") as f:
632- templateDef = f.read()
633+ template_def = f.read()
634
635- t = Template(templateDef)
636+ t = Template(template_def)
637 with open(pagerduty_cfg, "w") as f:
638 f.write(t.render(template_values))
639
640 with open("hooks/templates/nagios-pagerduty-flush-cron.tmpl", "r") as f2:
641- templateDef = f2.read()
642+ template_def = f2.read()
643
644- t2 = Template(templateDef)
645+ t2 = Template(template_def)
646 with open(pagerduty_cron, "w") as f2:
647 f2.write(t2.render(template_values))
648
649@@ -219,6 +229,7 @@ def enable_pagerduty_config():
650 shutil.copy("files/pagerduty_nagios.pl", "/usr/local/bin/pagerduty_nagios.pl")
651
652 # Create the pagerduty queue dir
653+
654 if not os.path.isdir(pagerduty_path):
655 hookenv.log("Making path for pagerduty_path")
656 mkdir_p(pagerduty_path)
657@@ -228,14 +239,18 @@ def enable_pagerduty_config():
658 os.chown(pagerduty_path, uid, gid)
659 else:
660 # Clean up the files if we don't want pagerduty
661+
662 if os.path.isfile(pagerduty_cfg):
663 os.remove(pagerduty_cfg)
664+
665 if os.path.isfile(pagerduty_cron):
666 os.remove(pagerduty_cron)
667
668 # Update contacts for admin
669+
670 if enable_pagerduty:
671 # avoid duplicates
672+
673 if "pagerduty" not in contactgroup_members:
674 forced_contactgroup_members.append("pagerduty")
675
676@@ -249,6 +264,7 @@ def enable_traps_config():
677 if os.path.isfile(traps_cfg):
678 os.remove(traps_cfg)
679 hookenv.log("Send traps feature is disabled")
680+
681 return
682
683 hookenv.log("Send traps feature is enabled, target address is %s" % send_traps_to)
684@@ -259,9 +275,9 @@ def enable_traps_config():
685 template_values = {"send_traps_to": send_traps_to}
686
687 with open("hooks/templates/traps.tmpl", "r") as f:
688- templateDef = f.read()
689+ template_def = f.read()
690
691- t = Template(templateDef)
692+ t = Template(template_def)
693 with open(traps_cfg, "w") as f:
694 f.write(t.render(template_values))
695
696@@ -271,17 +287,20 @@ def update_contacts():
697 admin_members = ""
698 contacts = []
699 admin_email = list(filter(None, set(hookenv.config("admin_email").split(","))))
700+
701 if len(admin_email) == 0:
702 hookenv.log("admin_email is unset, this isn't valid config")
703 hookenv.status_set("blocked", "admin_email is not configured")
704 exit(1)
705 hookenv.status_set("active", "ready")
706+
707 if len(admin_email) == 1:
708 hookenv.log("Setting one admin email address '%s'" % admin_email[0])
709 contacts = [{"contact_name": "root", "alias": "Root", "email": admin_email[0]}]
710 elif len(admin_email) > 1:
711 hookenv.log("Setting %d admin email addresses" % len(admin_email))
712 contacts = []
713+
714 for email in admin_email:
715 contact_name = email.replace("@", "").replace(".", "").lower()
716 contact_alias = contact_name.capitalize()
717@@ -292,6 +311,7 @@ def update_contacts():
718 admin_members = ", ".join([c["contact_name"] for c in contacts])
719
720 resulting_members = contactgroup_members
721+
722 if admin_members:
723 # if multiple admin emails are passed ignore contactgroup_members
724 resulting_members = admin_members
725@@ -338,8 +358,10 @@ def update_contacts():
726
727 def ssl_configured():
728 allowed_options = ["on", "only"]
729+
730 if str(ssl_config).lower() in allowed_options:
731 return True
732+
733 return False
734
735
736@@ -359,8 +381,10 @@ chain_file = "/etc/ssl/certs/%s.csr" % (cert_domain)
737 def check_ssl_files():
738 key = os.path.exists(deploy_key_path)
739 cert = os.path.exists(deploy_cert_path)
740+
741 if key is False or cert is False:
742 return False
743+
744 return True
745
746
747@@ -370,9 +394,11 @@ def decode_ssl_keys():
748 hookenv.log("Writing key from config ssl_key: %s" % key_file)
749 with open(key_file, "w") as f:
750 f.write(str(base64.b64decode(hookenv.config("ssl_key"))))
751+
752 if hookenv.config("ssl_cert"):
753 with open(cert_file, "w") as f:
754 f.write(str(base64.b64decode(hookenv.config("ssl_cert"))))
755+
756 if hookenv.config("ssl_chain"):
757 with open(chain_file, "w") as f:
758 f.write(str(base64.b64decode(hookenv.config("ssl_cert"))))
759@@ -382,10 +408,13 @@ def enable_ssl():
760 # Set the basename of all ssl files
761
762 # Validate that we have configs, and generate a self signed certificate.
763+
764 if not hookenv.config("ssl_cert"):
765 # bail if keys already exist
766+
767 if os.path.exists(cert_file):
768 hookenv.log("Keys exist, not creating keys!", "WARNING")
769+
770 return
771 # Generate a self signed key using CharmHelpers
772 hookenv.log("Generating Self Signed Certificate", "INFO")
773@@ -405,6 +434,7 @@ def update_config():
774 local_host_name = "nagios"
775 principal_unitname = hookenv.principal_unit()
776 # Fallback to using "primary" if it exists.
777+
778 if principal_unitname:
779 local_host_name = principal_unitname
780 else:
781@@ -437,16 +467,16 @@ def update_config():
782 }
783
784 with open("hooks/templates/nagios-cfg.tmpl", "r") as f:
785- templateDef = f.read()
786+ template_def = f.read()
787
788- t = Template(templateDef)
789+ t = Template(template_def)
790 with open(nagios_cfg, "w") as f:
791 f.write(t.render(template_values))
792
793 with open("hooks/templates/localhost_nagios2.cfg.tmpl", "r") as f:
794- templateDef = f.read()
795+ template_def = f.read()
796
797- t = Template(templateDef)
798+ t = Template(template_def)
799 with open("/etc/nagios3/conf.d/localhost_nagios2.cfg", "w") as f:
800 f.write(t.render(template_values))
801
802@@ -456,9 +486,9 @@ def update_config():
803 def update_cgi_config():
804 template_values = {"nagiosadmin": nagiosadmin, "ro_password": ro_password}
805 with open("hooks/templates/nagios-cgi.tmpl", "r") as f:
806- templateDef = f.read()
807+ template_def = f.read()
808
809- t = Template(templateDef)
810+ t = Template(template_def)
811 with open(nagios_cgi_cfg, "w") as f:
812 f.write(t.render(template_values))
813
814@@ -472,12 +502,12 @@ def update_cgi_config():
815 # note: i tried to use cheetah, and it barfed, several times. It can go play
816 # in a fire. I'm jusing jinja2.
817 def update_apache():
818- """
819+ """Add SSL keys to default-ssl config.
820+
821 Nagios3 is deployed as a global apache application from the archive.
822 We'll get a little funky and add the SSL keys to the default-ssl config
823 which sets our keys, including the self-signed ones, as the host keyfiles.
824 """
825-
826 # Start by Setting the ports.conf
827
828 with open("hooks/templates/ports-cfg.jinja2", "r") as f:
829@@ -489,6 +519,7 @@ def update_apache():
830 f.write(t.render({"enable_http": HTTP_ENABLED}))
831
832 # Next setup the default-ssl.conf
833+
834 if os.path.exists(chain_file) and os.path.getsize(chain_file) > 0:
835 ssl_chain = chain_file
836 else:
837@@ -515,6 +546,7 @@ def update_apache():
838 # Configure the behavior of http sites
839 sites = glob.glob("/etc/apache2/sites-available/*.conf")
840 non_ssl = set(sites) - {ssl_conf}
841+
842 for each in non_ssl:
843 site = os.path.basename(each).rsplit(".", 1)[0]
844 Apache2Site(site).action(enabled=HTTP_ENABLED)
845@@ -549,6 +581,7 @@ class Apache2Site:
846 def _enable(self):
847 hookenv.log("Apache2Site: Enabling %s..." % self.site, "INFO")
848 self._call(["a2ensite", self.site])
849+
850 if self.port == 443:
851 self._call(["a2enmod", "ssl"])
852 hookenv.open_port(self.port)
853@@ -562,6 +595,7 @@ class Apache2Site:
854 def update_password(account, password):
855 """Update the charm and Apache's record of the password for the supplied account."""
856 account_file = "".join(["/var/lib/juju/nagios.", account, ".passwd"])
857+
858 if password:
859 with open(account_file, "w") as f:
860 f.write(password)
861@@ -580,6 +614,7 @@ write_extra_config()
862 # enable_traps_config and enable_pagerduty_config modify forced_contactgroup_members
863 # they need to run before update_contacts that will consume that global var.
864 enable_traps_config()
865+
866 if ssl_configured():
867 enable_ssl()
868 enable_pagerduty_config()
869@@ -591,8 +626,10 @@ update_localhost()
870 update_cgi_config()
871 update_contacts()
872 update_password("nagiosro", ro_password)
873+
874 if password:
875 update_password(nagiosadmin, password)
876+
877 if nagiosadmin != "nagiosadmin":
878 update_password("nagiosadmin", False)
879
880diff --git a/hooks/website_relation_joined.py b/hooks/website_relation_joined.py
881index 706a561..126e7cd 100755
882--- a/hooks/website_relation_joined.py
883+++ b/hooks/website_relation_joined.py
884@@ -15,18 +15,15 @@
885 # You should have received a copy of the GNU General Public License
886 # along with this program. If not, see <http://www.gnu.org/licenses/>.
887
888-import common
889+from charmhelpers.core.hookenv import config, log, relation_set
890
891-from charmhelpers.core.hookenv import (
892- config,
893- log,
894- relation_set,
895-)
896+import common
897
898
899 def main():
900 relation_data = {"hostname": common.get_local_ingress_address()}
901 sslcfg = config()["ssl"]
902+
903 if sslcfg == "only":
904 relation_data["port"] = 443
905 else:
906diff --git a/tests/functional/conftest.py b/tests/functional/conftest.py
907index a31034c..7ab5b1a 100644
908--- a/tests/functional/conftest.py
909+++ b/tests/functional/conftest.py
910@@ -42,9 +42,11 @@ async def model(controller):
911 model = await controller.add_model(model_name)
912 yield model
913 await model.disconnect()
914+
915 if os.getenv("PYTEST_KEEP_MODEL"):
916 return
917 await controller.destroy_model(model_name)
918+
919 while model_name in await controller.list_models():
920 await asyncio.sleep(1)
921
922@@ -60,7 +62,7 @@ async def current_model():
923
924 @pytest.fixture
925 async def get_app(model):
926- """Return the application requested."""
927+ """Return the application requested.""" # noqa D202
928
929 async def _get_app(name):
930 try:
931@@ -73,11 +75,12 @@ async def get_app(model):
932
933 @pytest.fixture
934 async def get_unit(model):
935- """Return the requested <app_name>/<unit_number> unit."""
936+ """Return the requested <app_name>/<unit_number> unit.""" # noqa D202
937
938 async def _get_unit(name):
939 try:
940 (app_name, unit_number) = name.split("/")
941+
942 return model.applications[app_name].units[unit_number]
943 except (KeyError, ValueError):
944 raise JujuError("Cannot find unit {}".format(name))
945@@ -87,7 +90,7 @@ async def get_unit(model):
946
947 @pytest.fixture
948 async def get_entity(model, get_unit, get_app):
949- """Return a unit or an application."""
950+ """Return a unit or an application.""" # noqa D202
951
952 async def _get_entity(name):
953 try:
954@@ -103,7 +106,7 @@ async def get_entity(model, get_unit, get_app):
955
956 @pytest.fixture
957 async def run_command(get_unit):
958- """Run a command on a unit."""
959+ """Run a command on a unit.""" # noqa D202
960
961 async def _run_command(cmd, target):
962 """
963@@ -114,6 +117,7 @@ async def run_command(get_unit):
964 """
965 unit = target if type(target) is juju.unit.Unit else await get_unit(target)
966 action = await unit.run(cmd)
967+
968 return action.results
969
970 return _run_command
971@@ -121,16 +125,16 @@ async def run_command(get_unit):
972
973 @pytest.fixture
974 async def file_stat(run_command):
975- """
976- Run stat on a file.
977+ """Run stat on a file.
978
979 :param path: File path
980 :param target: Unit object or unit name string
981- """
982+ """ # noqa D202
983
984 async def _file_stat(path, target):
985 cmd = STAT_FILE % path
986 results = await run_command(cmd, target)
987+
988 return json.loads(results["Stdout"])
989
990 return _file_stat
991@@ -138,16 +142,17 @@ async def file_stat(run_command):
992
993 @pytest.fixture
994 async def file_contents(run_command):
995- """Return the contents of a file."""
996+ """Return the contents of a file.""" # noqa D202
997
998 async def _file_contents(path, target):
999 """Return the contents of a file.
1000
1001- :param path: File path
1002- :param target: Unit object or unit name string
1003+ :param path: File path
1004+ :param target: Unit object or unit name string
1005 """
1006 cmd = "cat {}".format(path)
1007 results = await run_command(cmd, target)
1008+
1009 return results["Stdout"]
1010
1011 return _file_contents
1012@@ -155,7 +160,7 @@ async def file_contents(run_command):
1013
1014 @pytest.fixture
1015 async def reconfigure_app(get_app, model):
1016- """Apply a different config to the requested app."""
1017+ """Apply a different config to the requested app.""" # noqa D202
1018
1019 async def _reconfigure_app(cfg, target):
1020 application = (
1021@@ -172,7 +177,7 @@ async def reconfigure_app(get_app, model):
1022
1023 @pytest.fixture
1024 async def create_group(run_command):
1025- """Create the UNIX group specified."""
1026+ """Create the UNIX group specified.""" # noqa D202
1027
1028 async def _create_group(group_name, target):
1029 cmd = "sudo groupadd %s" % group_name
1030@@ -185,11 +190,7 @@ pytestmark = pytest.mark.asyncio
1031
1032 CHARM_BUILD_DIR = os.getenv("CHARM_BUILD_DIR", "..").rstrip("/")
1033
1034-SERIES = [
1035- "trusty",
1036- "xenial",
1037- "bionic",
1038-]
1039+SERIES = ["trusty", "xenial", "bionic"]
1040
1041
1042 ############
1043@@ -210,6 +211,7 @@ async def relatives(model, series):
1044 )
1045
1046 mysql = "mysql"
1047+
1048 if series != "trusty":
1049 mysql = "percona-cluster"
1050
1051@@ -224,7 +226,7 @@ async def relatives(model, series):
1052 )
1053 await model.block_until(
1054 lambda: mysql_app.units[0].workload_status == "active"
1055- and mysql_app.units[0].agent_status == "idle"
1056+ and mysql_app.units[0].agent_status == "idle" # noqa W503
1057 )
1058
1059 yield {
1060@@ -256,10 +258,11 @@ async def deploy_app(relatives, model, series):
1061 )
1062 await model.block_until(
1063 lambda: nagios_app.units[0].agent_status == "idle"
1064- and relatives["mysql"]["app"].units[0].agent_status == "idle"
1065+ and relatives["mysql"]["app"].units[0].agent_status == "idle" # noqa W503
1066 )
1067
1068 yield nagios_app
1069+
1070 if os.getenv("PYTEST_KEEP_MODEL"):
1071 return
1072
1073@@ -277,6 +280,7 @@ class Agent:
1074
1075 def is_active(self, status):
1076 u = self.u
1077+
1078 return u.agent_status == status and u.workload_status == "active"
1079
1080 async def block_until_or_timeout(self, lambda_f, **kwargs):
1081@@ -299,6 +303,7 @@ async def unit(model, deploy_app):
1082 """Return the unit we've deployed."""
1083 unit = Agent(deploy_app.units[0], deploy_app)
1084 await unit.block_until(lambda: unit.is_active("idle"))
1085+
1086 return unit
1087
1088
1089@@ -306,4 +311,5 @@ async def unit(model, deploy_app):
1090 async def auth(file_contents, unit):
1091 """Return the basic auth credentials."""
1092 nagiospwd = await file_contents("/var/lib/juju/nagios.passwd", unit.u)
1093+
1094 return "nagiosadmin", nagiospwd.strip()
1095diff --git a/tests/functional/test_config.py b/tests/functional/test_config.py
1096index 1b9310a..6bc36a6 100644
1097--- a/tests/functional/test_config.py
1098+++ b/tests/functional/test_config.py
1099@@ -1,5 +1,7 @@
1100 from async_generator import asynccontextmanager
1101+
1102 import pytest
1103+
1104 import requests
1105
1106 pytestmark = pytest.mark.asyncio
1107@@ -18,8 +20,7 @@ async def config(unit, item, test_value, post_test):
1108
1109 @pytest.fixture(params=["on", "only"])
1110 async def ssl(unit, request):
1111- """
1112- Enable SSL before a test, then disable after test
1113+ """Enable SSL before a test, then disable after test.
1114
1115 :param Agent unit: unit from the fixture
1116 :param request: test parameters
1117@@ -30,8 +31,7 @@ async def ssl(unit, request):
1118
1119 @pytest.fixture
1120 async def extra_config(unit):
1121- """
1122- Enable extraconfig for a test, and revert afterwards
1123+ """Enable extraconfig for a test, and revert afterwards.
1124
1125 :param Agent unit: unit from the fixture
1126 """
1127@@ -48,8 +48,7 @@ async def extra_config(unit):
1128
1129 @pytest.fixture
1130 async def livestatus_path(unit):
1131- """
1132- Enable livestatus before a test, then disable after test
1133+ """Enable livestatus before a test, then disable after test.
1134
1135 :param Agent unit: unit from the fixture
1136 """
1137@@ -60,8 +59,7 @@ async def livestatus_path(unit):
1138
1139 @pytest.fixture()
1140 async def enable_pagerduty(unit):
1141- """
1142- Enable enable_pagerduty before first test, then disable after last test
1143+ """Enable enable_pagerduty before first test, then disable after last test.
1144
1145 :param Agent unit: unit from the fixture
1146 """
1147diff --git a/tests/functional/test_deploy.py b/tests/functional/test_deploy.py
1148index 32503c2..efbafb7 100644
1149--- a/tests/functional/test_deploy.py
1150+++ b/tests/functional/test_deploy.py
1151@@ -1,4 +1,5 @@
1152 import pytest
1153+
1154 import requests
1155
1156 pytestmark = pytest.mark.asyncio
1157diff --git a/tests/unit/test_website_relation_joined.py b/tests/unit/test_website_relation_joined.py
1158index b4ca91a..7dff088 100644
1159--- a/tests/unit/test_website_relation_joined.py
1160+++ b/tests/unit/test_website_relation_joined.py
1161@@ -1,6 +1,7 @@
1162 import unittest.mock as mock
1163
1164 import pytest
1165+
1166 import website_relation_joined
1167
1168
1169diff --git a/tox.ini b/tox.ini
1170index 8f1b11b..17cb36f 100644
1171--- a/tox.ini
1172+++ b/tox.ini
1173@@ -48,33 +48,7 @@ ignore = # TODO remove most of these
1174 D101,
1175 D102,
1176 D103,
1177- D104,
1178 D107,
1179- D202,
1180- D205,
1181- D208,
1182- D210,
1183- D400,
1184- D401,
1185- I100,
1186- I101,
1187- I201,
1188- I202,
1189- E201,
1190- E202,
1191- E231,
1192- E121,
1193- E126,
1194- E131,
1195- D201,
1196- E302,
1197- E501,
1198- N806,
1199- N816,
1200- W503,
1201- W504
1202-
1203-
1204 max-line-length = 88
1205 max-complexity = 10
1206

Subscribers

People subscribed via source and target branches