NRPE Charm

Merge ~xavpaice/charm-nrpe:fix_lint into charm-nrpe:master

  1. Git
  2. lp:~xavpaice/charm-nrpe
  3. fix_lint
  4. Merge into master
Proposed by Xav Paice
Status: Merged
Approved by: James Troup
Approved revision: 75853ba79adc0581d1178e9700ad4c6e3fb82a89
Merged at revision: f007f658e465935dbe7e2089b82e2b84733ae41f
Proposed branch: ~xavpaice/charm-nrpe:fix_lint
Merge into: charm-nrpe:master
Diff against target: 327 lines (+103/-82)
4 files modified
mod/charmhelpers (+1/-1)
tests/functional/tests/nrpe_tests.py (+89/-72)
tests/unit/test_nrpe_helpers.py (+10/-6)
tox.ini (+3/-3)
Reviewer Review Type Date Requested Status
🤖 prod-jenkaas-bootstack (community) continuous-integration Approve
BootStack Reviewers Pending
BootStack Reviewers Pending
Review via email: mp+414786@code.launchpad.net

Commit message

Lint fixes, add --keep-faulty-model

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 :

A CI job is currently in progress. A follow up comment will be added when it completes.

Revision history for this message
🤖 prod-jenkaas-bootstack (prod-jenkaas-bootstack) wrote :

FAILED: Continuous integration, rev:5b8759d76c588ebb36a6b12913c37e65ed21d1b9
https://jenkins.canonical.com/bootstack/job/lp-charm-nrpe-ci/59/
Executed test runs:
    FAILURE: https://jenkins.canonical.com/bootstack/job/lp-charm-test-functest/405/
    None: https://jenkins.canonical.com/bootstack/job/lp-update-mp/841/

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

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

FAILED: Continuous integration, rev:5b8759d76c588ebb36a6b12913c37e65ed21d1b9
https://jenkins.canonical.com/bootstack/job/lp-charm-nrpe-ci/60/
Executed test runs:
    FAILURE: https://jenkins.canonical.com/bootstack/job/lp-charm-test-functest/406/
    None: https://jenkins.canonical.com/bootstack/job/lp-update-mp/842/

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

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

PASSED: Continuous integration, rev:5b8759d76c588ebb36a6b12913c37e65ed21d1b9
https://jenkins.canonical.com/bootstack/job/lp-charm-nrpe-ci/61/
Executed test runs:
    SUCCESS: https://jenkins.canonical.com/bootstack/job/lp-charm-test-functest/411/
    None: https://jenkins.canonical.com/bootstack/job/lp-update-mp/847/

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

review: Approve (continuous-integration)
Revision history for this message
🤖 prod-jenkaas-bootstack (prod-jenkaas-bootstack) wrote :

A CI job is currently in progress. A follow up comment will be added when it completes.

Revision history for this message
🤖 prod-jenkaas-bootstack (prod-jenkaas-bootstack) wrote :

PASSED: Continuous integration, rev:75853ba79adc0581d1178e9700ad4c6e3fb82a89
https://jenkins.canonical.com/bootstack/job/lp-charm-nrpe-ci/62/
Executed test runs:
    SUCCESS: https://jenkins.canonical.com/bootstack/job/lp-charm-test-functest/417/
    None: https://jenkins.canonical.com/bootstack/job/lp-update-mp/854/

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

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

Change successfully merged at revision f007f658e465935dbe7e2089b82e2b84733ae41f

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/mod/charmhelpers b/mod/charmhelpers
2index 26efcd0..b811ffa 160000
3--- a/mod/charmhelpers
4+++ b/mod/charmhelpers
5@@ -1 +1 @@
6-Subproject commit 26efcd0da51d580f68ead2ca13c38f58766f8a14
7+Subproject commit b811ffada0a224260dfac348cb001bf2dc2d4d25
8diff --git a/tests/functional/tests/nrpe_tests.py b/tests/functional/tests/nrpe_tests.py
9index 9f288b2..1b626a6 100644
10--- a/tests/functional/tests/nrpe_tests.py
11+++ b/tests/functional/tests/nrpe_tests.py
12@@ -4,8 +4,7 @@ import pprint
13 import unittest
14
15 import yaml
16-
17-import zaza.model as model
18+import zaza.model as model # noqa I201
19
20
21 class TestBase(unittest.TestCase):
22@@ -33,20 +32,17 @@ class TestNrpe(TestBase):
23 )
24
25 nrpe_checks = {
26- "check_conntrack.cfg":
27- "command[check_conntrack]=/usr/local/lib/nagios/plugins/"
28- "check_conntrack.sh",
29- "check_disk_root.cfg":
30- "command[check_disk_root]=/usr/lib/nagios/plugins/check_disk",
31+ "check_conntrack.cfg": "command[check_conntrack]="
32+ "/usr/local/lib/nagios/plugins/check_conntrack.sh",
33+ "check_disk_root.cfg": "command[check_disk_root]="
34+ "/usr/lib/nagios/plugins/check_disk",
35 "check_load.cfg": "command[check_load]=/usr/lib/nagios/plugins/check_load",
36- "check_mem.cfg":
37- "command[check_mem]=/usr/local/lib/nagios/plugins/check_mem.pl",
38- "check_rabbitmq.cfg":
39- "command[check_rabbitmq]="
40- "/usr/local/lib/nagios/plugins/check_rabbitmq.py",
41- "check_swap_activity.cfg":
42- "command[check_swap_activity]="
43- "/usr/local/lib/nagios/plugins/check_swap_activity",
44+ "check_mem.cfg": "command[check_mem]="
45+ "/usr/local/lib/nagios/plugins/check_mem.pl",
46+ "check_rabbitmq.cfg": "command[check_rabbitmq]="
47+ "/usr/local/lib/nagios/plugins/check_rabbitmq.py",
48+ "check_swap_activity.cfg": "command[check_swap_activity]="
49+ "/usr/local/lib/nagios/plugins/check_swap_activity",
50 }
51
52 for nrpe_check in nrpe_checks:
53@@ -54,6 +50,7 @@ class TestNrpe(TestBase):
54 cmd = "cat /etc/nagios/nrpe.d/" + nrpe_check
55 result = model.run_on_unit(self.lead_unit_name, cmd)
56 code = result.get("Code")
57+
58 if code != "0":
59 logging.warning(
60 "Unable to find nrpe check {} at /etc/nagios/nrpe.d/".format(
61@@ -73,6 +70,7 @@ class TestNrpe(TestBase):
62 cmd = "cat /etc/nagios/nrpe.d/check_swap.cfg"
63 result = model.run_on_unit(self.lead_unit_name, cmd)
64 code = result.get("Code")
65+
66 if code != "0":
67 logging.warning(
68 "Unable to find nrpe check check_swap.cfg at /etc/nagios/nrpe.d/"
69@@ -131,12 +129,12 @@ class TestNrpe(TestBase):
70 model.block_until_all_units_idle()
71
72 local_nrpe_checks = {
73- "check_proc_jujud_user.cfg":
74- "command[check_proc_jujud_user]=/usr/lib/nagios/plugins/"
75- "check_procs -w 1 -c 1 -C jujud",
76- "check_proc_rsync_user.cfg":
77- "command[check_proc_rsync_user]=/usr/lib/nagios/plugins/"
78- "check_procs -w 1 -c 1 -C rsync",
79+ "check_proc_jujud_user.cfg": "command[check_proc_jujud_user]="
80+ "/usr/lib/nagios/plugins/"
81+ "check_procs -w 1 -c 1 -C jujud",
82+ "check_proc_rsync_user.cfg": "command[check_proc_rsync_user]="
83+ "/usr/lib/nagios/plugins/"
84+ "check_procs -w 1 -c 1 -C rsync",
85 }
86
87 for nrpe_check in local_nrpe_checks:
88@@ -144,6 +142,7 @@ class TestNrpe(TestBase):
89 cmd = "cat /etc/nagios/nrpe.d/" + nrpe_check
90 result = model.run_on_unit(self.lead_unit_name, cmd)
91 code = result.get("Code")
92+
93 if code != "0":
94 logging.warning(
95 "Unable to find nrpe check {} at /etc/nagios/nrpe.d/".format(
96@@ -155,10 +154,11 @@ class TestNrpe(TestBase):
97 self.assertTrue(local_nrpe_checks[nrpe_check] in content)
98
99 remote_nrpe_checks = {
100- "check_tcp_H_HOSTADDRESS__E_p22_s_SSH____eNone_w2_c10_t12_t10.cfg":
101- "/usr/lib/nagios/plugins/check_tcp -H $HOSTADDRESS$ "
102- "-E -p 22 -s 'SSH.*' -e None -w 2 -c 10 -t 12 -t 10"
103+ "check_tcp_H_HOSTADDRESS__E_p22_s_SSH____eNone_w2_c10_t12_t10.cfg": "/usr/"
104+ "lib/nagios/plugins/check_tcp -H $HOSTADDRESS$ "
105+ "-E -p 22 -s 'SSH.*' -e None -w 2 -c 10 -t 12 -t 10"
106 }
107+
108 for nrpe_check in remote_nrpe_checks:
109 logging.info(
110 "Checking content of '{}' nrpe command in nagios unit".format(
111@@ -171,6 +171,7 @@ class TestNrpe(TestBase):
112 )
113 result = model.run_on_unit(nagios_lead_unit_name, cmd)
114 code = result.get("Code")
115+
116 if code != "0":
117 logging.warning(
118 "Unable to find nrpe command {} at "
119@@ -187,6 +188,7 @@ class TestNrpe(TestBase):
120 cmd = "cat /etc/nagios/nrpe.cfg"
121 result = model.run_on_unit(self.lead_unit_name, cmd)
122 code = result.get("Code")
123+
124 if code != "0":
125 logging.warning("Unable to find nrpe config file at /etc/nagios/nrpe.cfg")
126 raise model.CommandRunFailed(cmd, result)
127@@ -205,6 +207,7 @@ class TestNrpe(TestBase):
128 )
129 result = model.run_on_unit(self.lead_unit_name, cmd)
130 code = result.get("Code")
131+
132 if code != "0":
133 logging.warning(
134 "Unable to find nrpe check at "
135@@ -218,63 +221,77 @@ class TestNrpe(TestBase):
136 """Check that certain checks are enabled on hosts but disabled on containers."""
137 # Enable appropriate config to enable various checks for testing whether they
138 # get created on containers versus hosts.
139- model.set_application_config(self.application_name, {
140- "disk_root": "-u GB -w 25% -c 20% -K 5%",
141- "zombies": "-w 3 -c 6 -s Z",
142- "procs": "-k -w 250 -c 300",
143- "load": "auto",
144- "conntrack": "-w 80 -c 90",
145- "users": "-w 20 -c 25",
146- "swap": "-w 40% -c 25%",
147- "swap_activity": "-i 5 -w 10240 -c 40960",
148- "mem": "-C -h -u -w 85 -c 90",
149- "lacp_bonds": "lo", # Enable a bogus lacp check on the loopback interface
150- "netlinks": "- ens3 mtu:9000 speed:10000", # Copied from test_05_netlinks
151- "xfs_errors": "5",
152- })
153+ model.set_application_config(
154+ self.application_name,
155+ {
156+ "disk_root": "-u GB -w 25% -c 20% -K 5%",
157+ "zombies": "-w 3 -c 6 -s Z",
158+ "procs": "-k -w 250 -c 300",
159+ "load": "auto",
160+ "conntrack": "-w 80 -c 90",
161+ "users": "-w 20 -c 25",
162+ "swap": "-w 40% -c 25%",
163+ "swap_activity": "-i 5 -w 10240 -c 40960",
164+ "mem": "-C -h -u -w 85 -c 90",
165+ "lacp_bonds": "lo", # Enable a bogus lacp check on the loopback iface
166+ "netlinks": "- ens3 mtu:9000 speed:10000",
167+ "xfs_errors": "5",
168+ },
169+ )
170 model.block_until_all_units_idle()
171
172 host_checks = self._get_unit_check_files("rabbitmq-server/0")
173 container_checks = self._get_unit_check_files("container/0")
174- expected_shared_checks = set([
175- "check_conntrack.cfg", # I think this should be host-only, but am not sure.
176- "check_total_procs.cfg",
177- "check_users.cfg",
178- "check_zombie_procs.cfg", # This also feels host-only to me; thoughts?
179- ])
180- expected_host_only_checks = set([
181- "check_arp_cache.cfg",
182- "check_disk_root.cfg",
183- "check_lacp_lo.cfg",
184- "check_load.cfg",
185- "check_mem.cfg",
186- "check_netlinks_ens3.cfg",
187- "check_ro_filesystem.cfg",
188- "check_swap.cfg",
189- "check_swap_activity.cfg",
190- "check_xfs_errors.cfg",
191- ])
192- self.assertTrue(expected_shared_checks.issubset(host_checks),
193- self._get_set_comparison(expected_shared_checks,
194- host_checks))
195- self.assertTrue(expected_shared_checks.issubset(container_checks),
196- self._get_set_comparison(expected_shared_checks,
197- container_checks))
198- self.assertTrue(expected_host_only_checks.issubset(host_checks),
199- self._get_set_comparison(expected_host_only_checks,
200- host_checks))
201- self.assertTrue(expected_host_only_checks.isdisjoint(container_checks),
202- self._get_set_comparison(expected_host_only_checks,
203- container_checks))
204+ expected_shared_checks = set(
205+ [
206+ "check_conntrack.cfg", # this should be host-only
207+ "check_total_procs.cfg",
208+ "check_users.cfg",
209+ "check_zombie_procs.cfg", # This also feels host-only
210+ ]
211+ )
212+ expected_host_only_checks = set(
213+ [
214+ "check_arp_cache.cfg",
215+ "check_disk_root.cfg",
216+ "check_lacp_lo.cfg",
217+ "check_load.cfg",
218+ "check_mem.cfg",
219+ "check_netlinks_ens3.cfg",
220+ "check_ro_filesystem.cfg",
221+ "check_swap.cfg",
222+ "check_swap_activity.cfg",
223+ "check_xfs_errors.cfg",
224+ ]
225+ )
226+ self.assertTrue(
227+ expected_shared_checks.issubset(host_checks),
228+ self._get_set_comparison(expected_shared_checks, host_checks),
229+ )
230+ self.assertTrue(
231+ expected_shared_checks.issubset(container_checks),
232+ self._get_set_comparison(expected_shared_checks, container_checks),
233+ )
234+ self.assertTrue(
235+ expected_host_only_checks.issubset(host_checks),
236+ self._get_set_comparison(expected_host_only_checks, host_checks),
237+ )
238+ self.assertTrue(
239+ expected_host_only_checks.isdisjoint(container_checks),
240+ self._get_set_comparison(expected_host_only_checks, container_checks),
241+ )
242
243 def _get_unit_check_files(self, unit):
244 cmdline = "ls /etc/nagios/nrpe.d/"
245 result = model.run_on_unit(unit, cmdline)
246 self.assertEqual(result["Code"], "0")
247+
248 return set(result["Stdout"].splitlines())
249
250 def _get_set_comparison(self, expected_checks, actual_checks):
251- return pprint.pformat({
252- 'Expected:': expected_checks,
253- 'Actual:': actual_checks,
254- })
255+ return pprint.pformat(
256+ {
257+ "Expected:": expected_checks,
258+ "Actual:": actual_checks,
259+ }
260+ )
261diff --git a/tests/unit/test_nrpe_helpers.py b/tests/unit/test_nrpe_helpers.py
262index 949eb92..4472602 100644
263--- a/tests/unit/test_nrpe_helpers.py
264+++ b/tests/unit/test_nrpe_helpers.py
265@@ -78,8 +78,9 @@ class TestIngressAddress(unittest.TestCase):
266 "egress-subnets": ["3.8.134.119/32"],
267 "ingress-addresses": ["3.8.134.119"],
268 }
269- self.assertEqual(nrpe_helpers.get_ingress_address("mockbinding"),
270- "172.31.29.247")
271+ self.assertEqual(
272+ nrpe_helpers.get_ingress_address("mockbinding"), "172.31.29.247"
273+ )
274
275 @mock.patch("nrpe_helpers.hookenv.config")
276 @mock.patch("nrpe_helpers.hookenv.network_get")
277@@ -105,8 +106,10 @@ class TestIngressAddress(unittest.TestCase):
278 "egress-subnets": ["3.8.134.119/32"],
279 "ingress-addresses": ["3.8.134.119"],
280 }
281- self.assertEqual(nrpe_helpers.get_ingress_address("mockbinding", external=True),
282- "172.31.29.247")
283+ self.assertEqual(
284+ nrpe_helpers.get_ingress_address("mockbinding", external=True),
285+ "172.31.29.247",
286+ )
287
288 @mock.patch("nrpe_helpers.hookenv.config")
289 @mock.patch("nrpe_helpers.hookenv.unit_get")
290@@ -114,5 +117,6 @@ class TestIngressAddress(unittest.TestCase):
291 """Prove we get a public IP address for Nagios relation."""
292 mock_config.return_value = "public"
293 mock_unit_get.return_value = "1.2.3.4"
294- self.assertEqual(nrpe_helpers.get_ingress_address("mockbinding", external=True),
295- "1.2.3.4")
296+ self.assertEqual(
297+ nrpe_helpers.get_ingress_address("mockbinding", external=True), "1.2.3.4"
298+ )
299diff --git a/tox.ini b/tox.ini
300index d179d89..e0c5f71 100644
301--- a/tox.ini
302+++ b/tox.ini
303@@ -35,7 +35,7 @@ passenv =
304 [testenv:lint]
305 commands =
306 flake8
307- black --check --exclude "/(\.eggs|\.git|\.tox|\.venv|\.build|dist|charmhelpers|mod|tests)/" .
308+ black --check --exclude "/(\.eggs|\.git|\.tox|\.venv|\.build|dist|charmhelpers|mod|files)/" .
309 deps =
310 black
311 flake8
312@@ -58,7 +58,7 @@ max-complexity = 14
313
314 [testenv:black]
315 commands =
316- black --exclude "/(\.eggs|\.git|\.tox|\.venv|\.build|dist|charmhelpers|mod|tests)/" .
317+ black --exclude "/(\.eggs|\.git|\.tox|\.venv|\.build|dist|charmhelpers|mod|files)/" .
318 deps =
319 black
320
321@@ -71,5 +71,5 @@ deps = -r{toxinidir}/tests/unit/requirements.txt
322
323 [testenv:func]
324 changedir = {toxinidir}/tests/functional
325-commands = functest-run-suite {posargs}
326+commands = functest-run-suite --keep-faulty-model {posargs}
327 deps = -r{toxinidir}/tests/functional/requirements.txt

Subscribers

People subscribed via source and target branches

to all changes: