Merge ~vultaire/charm-nrpe:host-container-check-rework into charm-nrpe:master

Proposed by Paul Goins
Status: Merged
Approved by: James Troup
Approved revision: a01359b53f0c564adf766afdf054d1b969d83bc7
Merged at revision: 989b71a53179c762a299f9085ce4d788d207b14e
Proposed branch: ~vultaire/charm-nrpe:host-container-check-rework
Merge into: charm-nrpe:master
Diff against target: 356 lines (+167/-74)
6 files modified
hooks/nrpe_helpers.py (+79/-74)
tests/functional/tests/bundles/bionic.yaml (+6/-0)
tests/functional/tests/bundles/focal.yaml (+6/-0)
tests/functional/tests/bundles/xenial.yaml (+6/-0)
tests/functional/tests/nrpe_tests.py (+66/-0)
tests/functional/tests/tests.yaml (+4/-0)
Reviewer Review Type Date Requested Status
🤖 prod-jenkaas-bootstack (community) continuous-integration Approve
BootStack Reviewers Pending
BootStack Reviewers Pending
Review via email: mp+407276@code.launchpad.net

Commit message

Reworked some checks to only run on hosts

Some checks have limited to no meaning when run on containers. This
patch tries to change some of those checks so they're only enabled and
installed on host machines.

Do note that this does impact testing. Functional tests expect that
the main machines of the model are not containers, and a newly added
test for this functionality will fail if it is run on an LXD-based
Juju controller.

To post a comment you must log in.
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
🤖 Canonical IS Merge Bot (canonical-is-mergebot) wrote :

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

Revision history for this message
🤖 prod-jenkaas-bootstack (prod-jenkaas-bootstack) wrote :
review: 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 :
review: Approve (continuous-integration)
Revision history for this message
🤖 Canonical IS Merge Bot (canonical-is-mergebot) wrote :

Change successfully merged at revision 989b71a53179c762a299f9085ce4d788d207b14e

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/hooks/nrpe_helpers.py b/hooks/nrpe_helpers.py
2index b9588e7..dc46245 100644
3--- a/hooks/nrpe_helpers.py
4+++ b/hooks/nrpe_helpers.py
5@@ -400,12 +400,6 @@ class SubordinateCheckDefinitions(dict):
6 local_plugin_dir = "/usr/local/lib/nagios/plugins/"
7 checks = [
8 {
9- "description": "Root disk",
10- "cmd_name": "check_disk_root",
11- "cmd_exec": pkg_plugin_dir + "check_disk",
12- "cmd_params": disk_root_thresholds,
13- },
14- {
15 "description": "Number of Zombie processes",
16 "cmd_name": "check_zombie_procs",
17 "cmd_exec": pkg_plugin_dir + "check_procs",
18@@ -418,61 +412,72 @@ class SubordinateCheckDefinitions(dict):
19 "cmd_params": proc_thresholds,
20 },
21 {
22- "description": "System Load",
23- "cmd_name": "check_load",
24- "cmd_exec": pkg_plugin_dir + "check_load",
25- "cmd_params": load_thresholds,
26- },
27- {
28 "description": "Number of Users",
29 "cmd_name": "check_users",
30 "cmd_exec": pkg_plugin_dir + "check_users",
31 "cmd_params": hookenv.config("users"),
32 },
33 {
34- "description": "Swap Activity",
35- "cmd_name": "check_swap_activity",
36- "cmd_exec": local_plugin_dir + "check_swap_activity",
37- "cmd_params": hookenv.config("swap_activity"),
38- },
39- {
40- "description": "Memory",
41- "cmd_name": "check_mem",
42- "cmd_exec": local_plugin_dir + "check_mem.pl",
43- "cmd_params": hookenv.config("mem"),
44- },
45- {
46 "description": "Connnection tracking table",
47 "cmd_name": "check_conntrack",
48 "cmd_exec": local_plugin_dir + "check_conntrack.sh",
49 "cmd_params": hookenv.config("conntrack"),
50 },
51- {
52- "description": "XFS Errors",
53- "cmd_name": "check_xfs_errors",
54- "cmd_exec": local_plugin_dir + "check_xfs_errors.py",
55- "cmd_params": hookenv.config("xfs_errors"),
56- },
57 ]
58
59- if hookenv.config("swap").strip():
60- check_swap = {
61- "description": "Swap",
62- "cmd_name": "check_swap",
63- "cmd_exec": pkg_plugin_dir + "check_swap",
64- "cmd_params": hookenv.config("swap").strip(),
65- }
66- checks.append(check_swap)
67-
68 if not is_container():
69- arp_check = {
70- "description": "ARP cache entries",
71- "cmd_name": "check_arp_cache",
72- "cmd_exec": os.path.join(local_plugin_dir, "check_arp_cache.py"),
73- # Specify params here to enable the check, not required otherwise.
74- "cmd_params": "-w 60 -c 80",
75- }
76- checks.append(arp_check)
77+ checks.extend(
78+ [
79+ {
80+ "description": "Root disk",
81+ "cmd_name": "check_disk_root",
82+ "cmd_exec": pkg_plugin_dir + "check_disk",
83+ "cmd_params": disk_root_thresholds,
84+ },
85+ {
86+ "description": "System Load",
87+ "cmd_name": "check_load",
88+ "cmd_exec": pkg_plugin_dir + "check_load",
89+ "cmd_params": load_thresholds,
90+ },
91+ {
92+ "description": "Swap",
93+ "cmd_name": "check_swap",
94+ "cmd_exec": pkg_plugin_dir + "check_swap",
95+ "cmd_params": hookenv.config("swap").strip(),
96+ },
97+ # Note: check_swap_activity *must* be listed after check_swap, else
98+ # check_swap_activity will be removed during installation of
99+ # check_swap.
100+ {
101+ "description": "Swap Activity",
102+ "cmd_name": "check_swap_activity",
103+ "cmd_exec": local_plugin_dir + "check_swap_activity",
104+ "cmd_params": hookenv.config("swap_activity"),
105+ },
106+ {
107+ "description": "Memory",
108+ "cmd_name": "check_mem",
109+ "cmd_exec": local_plugin_dir + "check_mem.pl",
110+ "cmd_params": hookenv.config("mem"),
111+ },
112+ {
113+ "description": "XFS Errors",
114+ "cmd_name": "check_xfs_errors",
115+ "cmd_exec": local_plugin_dir + "check_xfs_errors.py",
116+ "cmd_params": hookenv.config("xfs_errors"),
117+ },
118+ {
119+ "description": "ARP cache entries",
120+ "cmd_name": "check_arp_cache",
121+ "cmd_exec": os.path.join(
122+ local_plugin_dir, "check_arp_cache.py"
123+ ),
124+ "cmd_params": "-w 60 -c 80",
125+ },
126+ ]
127+ )
128+
129 ro_filesystem_excludes = hookenv.config("ro_filesystem_excludes")
130 if ro_filesystem_excludes == "":
131 # specify cmd_params = '' to disable/remove the check from nrpe
132@@ -497,38 +502,38 @@ class SubordinateCheckDefinitions(dict):
133 }
134 checks.append(check_ro_filesystem)
135
136- if hookenv.config("lacp_bonds").strip():
137- for bond_iface in hookenv.config("lacp_bonds").strip().split():
138- if os.path.exists("/sys/class/net/{}".format(bond_iface)):
139- description = "LACP Check {}".format(bond_iface)
140- cmd_name = "check_lacp_{}".format(bond_iface)
141- cmd_exec = local_plugin_dir + "check_lacp_bond.py"
142- cmd_params = "-i {}".format(bond_iface)
143- lacp_check = {
144+ if hookenv.config("lacp_bonds").strip():
145+ for bond_iface in hookenv.config("lacp_bonds").strip().split():
146+ if os.path.exists("/sys/class/net/{}".format(bond_iface)):
147+ description = "LACP Check {}".format(bond_iface)
148+ cmd_name = "check_lacp_{}".format(bond_iface)
149+ cmd_exec = local_plugin_dir + "check_lacp_bond.py"
150+ cmd_params = "-i {}".format(bond_iface)
151+ lacp_check = {
152+ "description": description,
153+ "cmd_name": cmd_name,
154+ "cmd_exec": cmd_exec,
155+ "cmd_params": cmd_params,
156+ }
157+ checks.append(lacp_check)
158+
159+ if hookenv.config("netlinks"):
160+ ifaces = yaml.safe_load(hookenv.config("netlinks"))
161+ cmd_exec = local_plugin_dir + "check_netlinks.py"
162+ if hookenv.config("netlinks_skip_unfound_ifaces"):
163+ cmd_exec += " --skip-unfound-ifaces"
164+ d_ifaces = self.parse_netlinks(ifaces)
165+ for iface in d_ifaces:
166+ description = "Netlinks status ({})".format(iface)
167+ cmd_name = "check_netlinks_{}".format(iface)
168+ cmd_params = d_ifaces[iface]
169+ netlink_check = {
170 "description": description,
171 "cmd_name": cmd_name,
172 "cmd_exec": cmd_exec,
173 "cmd_params": cmd_params,
174 }
175- checks.append(lacp_check)
176-
177- if hookenv.config("netlinks"):
178- ifaces = yaml.safe_load(hookenv.config("netlinks"))
179- cmd_exec = local_plugin_dir + "check_netlinks.py"
180- if hookenv.config("netlinks_skip_unfound_ifaces"):
181- cmd_exec += " --skip-unfound-ifaces"
182- d_ifaces = self.parse_netlinks(ifaces)
183- for iface in d_ifaces:
184- description = "Netlinks status ({})".format(iface)
185- cmd_name = "check_netlinks_{}".format(iface)
186- cmd_params = d_ifaces[iface]
187- netlink_check = {
188- "description": description,
189- "cmd_name": cmd_name,
190- "cmd_exec": cmd_exec,
191- "cmd_params": cmd_params,
192- }
193- checks.append(netlink_check)
194+ checks.append(netlink_check)
195
196 self["checks"] = []
197 sub_postfix = str(hookenv.config("sub_postfix"))
198diff --git a/tests/functional/tests/bundles/bionic.yaml b/tests/functional/tests/bundles/bionic.yaml
199index 1f68afe..f83462a 100644
200--- a/tests/functional/tests/bundles/bionic.yaml
201+++ b/tests/functional/tests/bundles/bionic.yaml
202@@ -3,11 +3,17 @@ applications:
203 rabbitmq-server:
204 charm: cs:rabbitmq-server
205 num_units: 1
206+ container:
207+ charm: cs:ubuntu
208+ num_units: 1
209+ to: ["lxd:rabbitmq-server/0"]
210 nagios:
211 charm: cs:nagios
212 num_units: 1
213 relations:
214 - - rabbitmq-server:nrpe-external-master
215 - nrpe:nrpe-external-master
216+ - - container:juju-info
217+ - nrpe:general-info
218 - - nrpe:monitors
219 - nagios:monitors
220diff --git a/tests/functional/tests/bundles/focal.yaml b/tests/functional/tests/bundles/focal.yaml
221index 606e891..2689a08 100644
222--- a/tests/functional/tests/bundles/focal.yaml
223+++ b/tests/functional/tests/bundles/focal.yaml
224@@ -3,6 +3,10 @@ applications:
225 rabbitmq-server:
226 charm: cs:rabbitmq-server
227 num_units: 1
228+ container:
229+ charm: cs:ubuntu
230+ num_units: 1
231+ to: ["lxd:rabbitmq-server/0"]
232 nagios:
233 charm: cs:nagios
234 num_units: 1
235@@ -10,5 +14,7 @@ applications:
236 relations:
237 - - rabbitmq-server:nrpe-external-master
238 - nrpe:nrpe-external-master
239+ - - container:juju-info
240+ - nrpe:general-info
241 - - nrpe:monitors
242 - nagios:monitors
243diff --git a/tests/functional/tests/bundles/xenial.yaml b/tests/functional/tests/bundles/xenial.yaml
244index e6176e1..89ef991 100644
245--- a/tests/functional/tests/bundles/xenial.yaml
246+++ b/tests/functional/tests/bundles/xenial.yaml
247@@ -3,11 +3,17 @@ applications:
248 rabbitmq-server:
249 charm: cs:rabbitmq-server
250 num_units: 1
251+ container:
252+ charm: cs:ubuntu
253+ num_units: 1
254+ to: ["lxd:rabbitmq-server/0"]
255 nagios:
256 charm: cs:nagios
257 num_units: 1
258 relations:
259 - - rabbitmq-server:nrpe-external-master
260 - nrpe:nrpe-external-master
261+ - - container:juju-info
262+ - nrpe:general-info
263 - - nrpe:monitors
264 - nagios:monitors
265diff --git a/tests/functional/tests/nrpe_tests.py b/tests/functional/tests/nrpe_tests.py
266index de138ea..9f288b2 100644
267--- a/tests/functional/tests/nrpe_tests.py
268+++ b/tests/functional/tests/nrpe_tests.py
269@@ -1,5 +1,6 @@
270 """Zaza functional tests."""
271 import logging
272+import pprint
273 import unittest
274
275 import yaml
276@@ -212,3 +213,68 @@ class TestNrpe(TestBase):
277 raise model.CommandRunFailed(cmd, result)
278 content = result.get("Stdout")
279 self.assertTrue(line in content)
280+
281+ def test_06_container_checks(self):
282+ """Check that certain checks are enabled on hosts but disabled on containers."""
283+ # Enable appropriate config to enable various checks for testing whether they
284+ # get created on containers versus hosts.
285+ model.set_application_config(self.application_name, {
286+ "disk_root": "-u GB -w 25% -c 20% -K 5%",
287+ "zombies": "-w 3 -c 6 -s Z",
288+ "procs": "-k -w 250 -c 300",
289+ "load": "auto",
290+ "conntrack": "-w 80 -c 90",
291+ "users": "-w 20 -c 25",
292+ "swap": "-w 40% -c 25%",
293+ "swap_activity": "-i 5 -w 10240 -c 40960",
294+ "mem": "-C -h -u -w 85 -c 90",
295+ "lacp_bonds": "lo", # Enable a bogus lacp check on the loopback interface
296+ "netlinks": "- ens3 mtu:9000 speed:10000", # Copied from test_05_netlinks
297+ "xfs_errors": "5",
298+ })
299+ model.block_until_all_units_idle()
300+
301+ host_checks = self._get_unit_check_files("rabbitmq-server/0")
302+ container_checks = self._get_unit_check_files("container/0")
303+ expected_shared_checks = set([
304+ "check_conntrack.cfg", # I think this should be host-only, but am not sure.
305+ "check_total_procs.cfg",
306+ "check_users.cfg",
307+ "check_zombie_procs.cfg", # This also feels host-only to me; thoughts?
308+ ])
309+ expected_host_only_checks = set([
310+ "check_arp_cache.cfg",
311+ "check_disk_root.cfg",
312+ "check_lacp_lo.cfg",
313+ "check_load.cfg",
314+ "check_mem.cfg",
315+ "check_netlinks_ens3.cfg",
316+ "check_ro_filesystem.cfg",
317+ "check_swap.cfg",
318+ "check_swap_activity.cfg",
319+ "check_xfs_errors.cfg",
320+ ])
321+ self.assertTrue(expected_shared_checks.issubset(host_checks),
322+ self._get_set_comparison(expected_shared_checks,
323+ host_checks))
324+ self.assertTrue(expected_shared_checks.issubset(container_checks),
325+ self._get_set_comparison(expected_shared_checks,
326+ container_checks))
327+ self.assertTrue(expected_host_only_checks.issubset(host_checks),
328+ self._get_set_comparison(expected_host_only_checks,
329+ host_checks))
330+ self.assertTrue(expected_host_only_checks.isdisjoint(container_checks),
331+ self._get_set_comparison(expected_host_only_checks,
332+ container_checks))
333+
334+ def _get_unit_check_files(self, unit):
335+ cmdline = "ls /etc/nagios/nrpe.d/"
336+ result = model.run_on_unit(unit, cmdline)
337+ self.assertEqual(result["Code"], "0")
338+ return set(result["Stdout"].splitlines())
339+
340+ def _get_set_comparison(self, expected_checks, actual_checks):
341+ return pprint.pformat({
342+ 'Expected:': expected_checks,
343+ 'Actual:': actual_checks,
344+ })
345diff --git a/tests/functional/tests/tests.yaml b/tests/functional/tests/tests.yaml
346index 640c3d2..8034f51 100644
347--- a/tests/functional/tests/tests.yaml
348+++ b/tests/functional/tests/tests.yaml
349@@ -5,3 +5,7 @@ gate_bundles:
350 - focal
351 tests:
352 - tests.nrpe_tests.TestNrpe
353+target_deploy_status:
354+ container:
355+ workload-status: active
356+ workload-status-message-prefix: ""

Subscribers

People subscribed via source and target branches

to all changes: