Merge ~gabrielcocenza/juju-lint:bug/1893272 into juju-lint:master
- Git
- lp:~gabrielcocenza/juju-lint
- bug/1893272
- Merge into master
Status: | Merged | ||||
---|---|---|---|---|---|
Approved by: | Martin Kalcok | ||||
Approved revision: | 5dc4c256dedc2d5596d00f1c6be6453df5a572a9 | ||||
Merged at revision: | 9160dae8189345db8494835da2465d9acf8b9d8f | ||||
Proposed branch: | ~gabrielcocenza/juju-lint:bug/1893272 | ||||
Merge into: | juju-lint:master | ||||
Prerequisite: | ~gabrielcocenza/juju-lint:bug/1965762-B | ||||
Diff against target: |
1965 lines (+1241/-287) 10 files modified
CONTRIBUTING.md (+2/-2) contrib/includes/base.yaml (+15/-0) jujulint/lint.py (+20/-9) jujulint/model_input.py (+331/-0) jujulint/relations.py (+73/-110) setup.py (+1/-1) tests/unit/conftest.py (+224/-67) tests/unit/test_input.py (+233/-0) tests/unit/test_jujulint.py (+72/-15) tests/unit/test_relations.py (+270/-83) |
||||
Related bugs: |
|
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
🤖 prod-jenkaas-bootstack | continuous-integration | Approve | |
Martin Kalcok (community) | Approve | ||
Mert Kirpici | Pending | ||
BootStack Reviewers | Pending | ||
Eric Chen | Pending | ||
Review via email: mp+429919@code.launchpad.net |
Commit message
added new relation rule check for ubiquitous
- ubiquitous relation rule to ensure every machine has a charm unit on it.
- added ubiquitous relation rule to nrpe, telegraf, filebeat, landscape-client and ubuntu-advantage.
- added a new model input module that is responsible to differentiate the
type of input (Juju Status, Juju Bundle). The input object encapsulate
all mapping and queries necessary against a file.
Closes-Bug: #1893272
Description of the change
This change is mainly focused on checking charms that need to be deployed on every machine.
The next step is differentiate container, virtual and physical machines and add adapt to check ntp, hw-health, lldpd just on the physical ones.
Gabriel Cocenza (gabrielcocenza) : | # |
🤖 Canonical IS Merge Bot (canonical-is-mergebot) wrote : | # |
Gabriel Cocenza (gabrielcocenza) wrote : | # |
logs of unit, lint and functional test [0]
Martin Kalcok (martin-kalcok) wrote : | # |
Thanks for the improvements. Overall LGTM, but I do have some comments (see inline)
Gabriel Cocenza (gabrielcocenza) : | # |
Gabriel Cocenza (gabrielcocenza) : | # |
Martin Kalcok (martin-kalcok) : | # |
Martin Kalcok (martin-kalcok) wrote : | # |
LGTM, thanks.
🤖 prod-jenkaas-bootstack (prod-jenkaas-bootstack) wrote : | # |
PASSED: Continuous integration, rev:5dc4c256ded
https:/
Executed test runs:
SUCCESS: https:/
None: https:/
Click here to trigger a rebuild:
https:/
🤖 Canonical IS Merge Bot (canonical-is-mergebot) wrote : | # |
Change successfully merged at revision 9160dae8189345d
Preview Diff
1 | diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md |
2 | index ef0a32c..2baa385 100644 |
3 | --- a/CONTRIBUTING.md |
4 | +++ b/CONTRIBUTING.md |
5 | @@ -55,9 +55,9 @@ make clean # clean the snapcraft lxd containers and the snap files cre |
6 | ``` |
7 | ### Functional Tests |
8 | |
9 | -`make functional` will build the snap, rename it, install it locally and run the tests against the installed snap package. Since this action involves installing a snap package, passwordless `sudo` privileges are needed. |
10 | +`make functional` will build the snap, rename it, install it locally and run the tests against the installed snap package. Since this action involves installing a snap package, passwordless `sudo` privileges are needed. |
11 | However for development purposes, the testing infrastructure also allows for installing `juju-lint` as a python package to a tox environment |
12 | -and running tests against it. |
13 | +and running tests against it. |
14 | In order to invoke: |
15 | - development smoke suite, which deselects tests running against a live lxd cloud |
16 | - `$ tox -e func-smoke` |
17 | diff --git a/contrib/includes/base.yaml b/contrib/includes/base.yaml |
18 | index 2ab4712..8e0a2ce 100644 |
19 | --- a/contrib/includes/base.yaml |
20 | +++ b/contrib/includes/base.yaml |
21 | @@ -46,7 +46,22 @@ subordinates: |
22 | |
23 | # openstack and k8s should check nrpe relations. See LP#1965762 |
24 | relations base check: &relations-base-check |
25 | + # NOTE(gabrielcocenza) filebeat, telegraf, nrpe are necessary until switch to the COS |
26 | - charm: nrpe |
27 | check: [ |
28 | ["*:nrpe-external-master", "nrpe:nrpe-external-master"] |
29 | ] |
30 | + # every machine has nrpe unit on it. See LP#1893272 |
31 | + ubiquitous: true |
32 | + |
33 | + - charm: telegraf |
34 | + ubiquitous: true |
35 | + |
36 | + - charm: filebeat |
37 | + ubiquitous: true |
38 | + |
39 | + - charm: landscape-client |
40 | + ubiquitous: true |
41 | + |
42 | + - charm: ubuntu-advantage |
43 | + ubiquitous: true |
44 | diff --git a/jujulint/lint.py b/jujulint/lint.py |
45 | index 937733e..8bbcff4 100755 |
46 | --- a/jujulint/lint.py |
47 | +++ b/jujulint/lint.py |
48 | @@ -36,6 +36,7 @@ from dateutil import relativedelta |
49 | import jujulint.util as utils |
50 | from jujulint.check_spaces import Relation, find_space_mismatches |
51 | from jujulint.logging import Logger |
52 | +from jujulint.model_input import input_handler |
53 | from jujulint.relations import RelationError, RelationsRulesBootStrap |
54 | |
55 | VALID_CONFIG_CHECKS = ("isset", "eq", "neq", "gte", "search") |
56 | @@ -613,20 +614,19 @@ class Linter: |
57 | if not self.model.extraneous_subs[sub]: |
58 | del self.model.extraneous_subs[sub] |
59 | |
60 | - def check_relations(self, applications): |
61 | + def check_relations(self, input_file): |
62 | """Check the relations in the rules file. |
63 | |
64 | - :param applications: applications present in the model |
65 | - :type applications: Dict |
66 | + :param input_file: mapped content of the input file. |
67 | + :type input_file: Union[JujuBundleFile, JujuStatusFile] |
68 | """ |
69 | if "relations" not in self.lint_rules: |
70 | self._log_with_header("No relation rules found. Skipping relation checks") |
71 | return |
72 | try: |
73 | relations_rules = RelationsRulesBootStrap( |
74 | - charm_to_app=self.model.charm_to_app, |
75 | relations_rules=self.lint_rules["relations"], |
76 | - applications=applications, |
77 | + input_file=input_file, |
78 | ).check() |
79 | except RelationError as e: |
80 | relations_rules = [] |
81 | @@ -656,6 +656,18 @@ class Linter: |
82 | } |
83 | ) |
84 | |
85 | + if rule.missing_machines: |
86 | + self.handle_error( |
87 | + { |
88 | + "id": "missing-machine", |
89 | + "tags": ["missing", "machine"], |
90 | + "message": "Charm '{}' missing on machines: {}".format( |
91 | + rule.charm, |
92 | + rule.missing_machines, |
93 | + ), |
94 | + } |
95 | + ) |
96 | + |
97 | def check_charms_ops_mandatory(self, charm): |
98 | """ |
99 | Check if a mandatory ops charms is present in the model. |
100 | @@ -1228,9 +1240,8 @@ class Linter: |
101 | def do_lint(self, parsed_yaml): # pragma: no cover |
102 | """Lint parsed YAML.""" |
103 | # Handle Juju 2 vs Juju 1 |
104 | - applications = "applications" |
105 | - if applications not in parsed_yaml: |
106 | - applications = "services" |
107 | + applications = "applications" if "applications" in parsed_yaml else "services" |
108 | + input_file = input_handler(parsed_yaml, applications) |
109 | |
110 | if applications in parsed_yaml: |
111 | |
112 | @@ -1251,7 +1262,7 @@ class Linter: |
113 | self.process_subordinates(parsed_yaml[applications][app], app) |
114 | |
115 | self.check_subs(parsed_yaml["machines"]) |
116 | - self.check_relations(parsed_yaml[applications]) |
117 | + self.check_relations(input_file) |
118 | self.check_charms() |
119 | |
120 | if "relations" in parsed_yaml: |
121 | diff --git a/jujulint/model_input.py b/jujulint/model_input.py |
122 | new file mode 100644 |
123 | index 0000000..a14551c |
124 | --- /dev/null |
125 | +++ b/jujulint/model_input.py |
126 | @@ -0,0 +1,331 @@ |
127 | +#! /usr/bin/env python3 |
128 | +from __future__ import annotations # type annotations are lazy-evaluated |
129 | + |
130 | +"""Treat input formats to lint.""" |
131 | + |
132 | +from collections import defaultdict |
133 | +from dataclasses import dataclass, field |
134 | +from functools import partialmethod |
135 | +from logging import getLogger |
136 | +from typing import Any, Dict, List, Set, Tuple, Union |
137 | + |
138 | +from jujulint.util import extract_charm_name |
139 | + |
140 | +LOGGER = getLogger(__name__) |
141 | + |
142 | + |
143 | +@dataclass |
144 | +class BaseFile: |
145 | + """BaseFile to define common properties and methods.""" |
146 | + |
147 | + applications_data: Dict |
148 | + machines_data: Dict |
149 | + charms: Set = field(default_factory=set) |
150 | + machines: Set = field(default_factory=set) |
151 | + app_to_charm: Dict = field(default_factory=dict) |
152 | + charm_to_app: defaultdict[set] = field(default_factory=lambda: defaultdict(set)) |
153 | + apps_to_machines: defaultdict[set] = field(default_factory=lambda: defaultdict(set)) |
154 | + |
155 | + def __post_init__(self): |
156 | + """Dunder method to map file after instantiating.""" |
157 | + self.map_file() |
158 | + |
159 | + @property |
160 | + def applications(self) -> Set: |
161 | + """Applications present in the file.""" |
162 | + return set(self.applications_data.keys()) |
163 | + |
164 | + @staticmethod |
165 | + def split_relation(relation: List[List[str]]) -> Tuple[str, str]: |
166 | + """Split relations into apps and endpoints. |
167 | + |
168 | + :param relation: Relation having the following format: |
169 | + [[<app_1>:<endpoint_1>], [<app_2>:<endpoint_2>]] |
170 | + :type relation: List[List[str]] |
171 | + :return: Relation and endpoint. |
172 | + :rtype: Tuple[str, str] |
173 | + """ |
174 | + return *relation[0].split(":"), *relation[1].split(":") |
175 | + |
176 | + def map_file(self) -> None: |
177 | + """Process the input file.""" |
178 | + for app in self.applications: |
179 | + if "charm" in self.applications_data[app]: |
180 | + charm_name = extract_charm_name(self.applications_data[app]["charm"]) |
181 | + self.charms.add(charm_name) |
182 | + self.app_to_charm[app] = charm_name |
183 | + self.charm_to_app[charm_name].add(app) |
184 | + self.map_machines() |
185 | + self.map_apps_to_machines() |
186 | + |
187 | + def check_app_endpoint_existence( |
188 | + self, app_endpoint: str, charm: str, endpoints_key: str |
189 | + ) -> Tuple[str, str]: |
190 | + """Check if app and endpoint exist on the object to lint. |
191 | + |
192 | + :param app_endpoint: app and endpoint separated by ":" with the following format: |
193 | + <app>:<endpoint>. When app is equal to "*", it's considered as ALL possible apps |
194 | + :type app_endpoint: str |
195 | + :param charm: charm to not check itself. |
196 | + :type charm: str |
197 | + :param endpoints_key: dictionary key to access endpoints. |
198 | + :type endpoints_key: str |
199 | + :return: application and endpoint |
200 | + :rtype: Tuple[str, str] |
201 | + """ |
202 | + app, endpoint = app_endpoint.split(":") |
203 | + # app == "*" means all apps |
204 | + # a charm from relation rule can have different app names. |
205 | + if app != "*" and app != charm: |
206 | + if app not in self.applications: |
207 | + LOGGER.warning(f"{app} not found on applications.") |
208 | + return "", "" |
209 | + |
210 | + # NOTE(gabrielcocenza) it's not always that a bundle will contain all endpoints under "bindings". |
211 | + # See LP#1949883 and LP#1990017 |
212 | + # juju-info is represented by "" on endpoint-bindings |
213 | + if endpoint != "juju-info" and endpoint not in self.applications_data[ |
214 | + app |
215 | + ].get(endpoints_key, {}): |
216 | + LOGGER.warning(f"endpoint: {endpoint} not found on {app}") |
217 | + return "", "" |
218 | + return app, endpoint |
219 | + |
220 | + def filter_by_app_and_endpoint( |
221 | + self, charm: str, app: str, endpoint: str, endpoints_key: str |
222 | + ) -> Set: |
223 | + """Filter applications by the presence of an endpoint. |
224 | + |
225 | + :param charm: Charm to not filter itself. |
226 | + :type charm: str |
227 | + :param app: Application to be filtered. When app is equal to "*", filters ALL apps |
228 | + that have the endpoint passed. |
229 | + :type app: str |
230 | + :param endpoint: Endpoint of an application. |
231 | + :type endpoint: str |
232 | + :param endpoints_key: dictionary key to access endpoint. |
233 | + :type endpoints_key: str |
234 | + :return: Applications that matches with the endpoint passed. |
235 | + :rtype: Set |
236 | + """ |
237 | + # when app == "*", filters all apps that have the endpoint passed. |
238 | + if app == "*": |
239 | + # remove all possible app names to not check itself. |
240 | + apps_to_check = self.applications - self.charm_to_app[charm] |
241 | + return { |
242 | + app |
243 | + for app in apps_to_check |
244 | + if endpoint |
245 | + in self.applications_data.get(app, {}).get(endpoints_key, {}) |
246 | + } |
247 | + return ( |
248 | + set([app]) |
249 | + if endpoint in self.applications_data.get(app, {}).get(endpoints_key, {}) |
250 | + else set() |
251 | + ) |
252 | + |
253 | + def map_machines(self): |
254 | + """Map machines method to be implemented. |
255 | + |
256 | + :raises NotImplementedError: Raise if not implemented on child classes. |
257 | + """ |
258 | + raise NotImplementedError(f"{self.__class__.__name__} missing: map_machines") |
259 | + |
260 | + def map_apps_to_machines(self): |
261 | + """Map apps to machines method to be implemented. |
262 | + |
263 | + :raises NotImplementedError: Raise if not implemented on child classes. |
264 | + """ |
265 | + raise NotImplementedError( |
266 | + f"{self.__class__.__name__} missing: map_apps_to_machines" |
267 | + ) |
268 | + |
269 | + def filter_by_relation(self, apps: Set, endpoint: str) -> Set: |
270 | + """Filter apps by relation to be implemented. |
271 | + |
272 | + :raises NotImplementedError: Raise if not implemented on child classes. |
273 | + """ |
274 | + raise NotImplementedError( |
275 | + f"{self.__class__.__name__} missing: filter_by_relation" |
276 | + ) |
277 | + |
278 | + def sorted_machines(self, machine: str): |
279 | + """Sort machines. |
280 | + |
281 | + :param machine: machine id. |
282 | + :type machine: str |
283 | + :raises NotImplementedError: Raise if not implemented on child classes. |
284 | + """ |
285 | + raise NotImplementedError(f"{self.__class__.__name__} missing: sorted_machines") |
286 | + |
287 | + |
288 | +@dataclass |
289 | +class JujuStatusFile(BaseFile): |
290 | + """Juju status file input representation.""" |
291 | + |
292 | + def map_machines(self) -> None: |
293 | + """Map machines passed on the file.""" |
294 | + self.machines.update(self.machines_data.keys()) |
295 | + for machine in self.machines_data: |
296 | + self.machines.update(self.machines_data[machine].get("containers", [])) |
297 | + |
298 | + def map_apps_to_machines(self) -> None: |
299 | + """Map applications on machines.""" |
300 | + for app in self.applications_data: |
301 | + units = self.applications_data[app].get("units", {}) |
302 | + for unit in units: |
303 | + machine = units[unit].get("machine") |
304 | + self.apps_to_machines[app].add(machine) |
305 | + subordinates = units[unit].get("subordinates", {}) |
306 | + for sub in subordinates: |
307 | + self.apps_to_machines[sub.split("/")[0]].add(machine) |
308 | + |
309 | + @staticmethod |
310 | + def sorted_machines(machine: str) -> Tuple[int, str, int]: |
311 | + """Sort machines by number and/or containers. |
312 | + |
313 | + :param machine: name of the machine |
314 | + E.g of expected input: "1", "1/lxd/3" |
315 | + :type machine: str |
316 | + :return: tuple with sort keys |
317 | + :rtype: Tuple[int, str, int] |
318 | + """ |
319 | + # way to be sure that a machine have at least 3 key values to sort |
320 | + key_1, key_2, key_3, *_ = machine.split("/") + ["", 0] |
321 | + return int(key_1), key_2, int(key_3) |
322 | + |
323 | + # overwrite parent method passing "endpoint-bindings" as endpoints_key |
324 | + filter_by_app_and_endpoint = partialmethod( |
325 | + BaseFile.filter_by_app_and_endpoint, endpoints_key="endpoint-bindings" |
326 | + ) |
327 | + check_app_endpoint_existence = partialmethod( |
328 | + BaseFile.check_app_endpoint_existence, endpoints_key="endpoint-bindings" |
329 | + ) |
330 | + |
331 | + def filter_by_relation(self, apps: Set, endpoint: str) -> Set: |
332 | + """Filter applications that relate with an endpoint. |
333 | + |
334 | + :param apps: Applications to filter relations using the endpoint. |
335 | + :type apps: Set |
336 | + :param endpoint: endpoint of the applications. |
337 | + :type endpoint: str |
338 | + :return: Applications that has a relation with the apps using the endpoint. |
339 | + :rtype: Set |
340 | + """ |
341 | + apps_related = set() |
342 | + for app in apps: |
343 | + relations = self.applications_data.get(app, {}).get("relations", {}) |
344 | + apps_related.update(relations.get(endpoint, [])) |
345 | + return apps_related |
346 | + |
347 | + |
348 | +@dataclass |
349 | +class JujuBundleFile(BaseFile): |
350 | + """Juju bundle file input representation.""" |
351 | + |
352 | + relations_data: List = field(default_factory=list) |
353 | + |
354 | + def map_machines(self) -> None: |
355 | + """Map machines passed on the file.""" |
356 | + self.machines.update(self.machines_data.keys()) |
357 | + for app in self.applications_data: |
358 | + deploy_to_machines = self.applications_data[app].get("to", []) |
359 | + # openstack bundles can have corner cases e.g: to: - designate-bind/0 |
360 | + # in this case the application is deployed where designate-bind/0 is located |
361 | + # See https://launchpad.net/bugs/1965256 |
362 | + machines = [machine for machine in deploy_to_machines if "/" not in machine] |
363 | + self.machines.update(machines) |
364 | + |
365 | + def map_apps_to_machines(self) -> None: |
366 | + """Map applications on machines.""" |
367 | + for app in self.applications_data: |
368 | + machines = self.applications_data[app].get("to", []) |
369 | + self.apps_to_machines[app].update(machines) |
370 | + # NOTE(gabrielcocenza) subordinates won't have the 'to' field because |
371 | + # they are deployed thru relations. |
372 | + subordinates = { |
373 | + sub for sub, machines in self.apps_to_machines.items() if machines == set() |
374 | + } |
375 | + for relation in self.relations_data: |
376 | + app_1, endpoint_1, app_2, endpoint_2 = self.split_relation(relation) |
377 | + # update with the machines of the application that the subordinate charm relate. |
378 | + if app_1 in subordinates: |
379 | + self.apps_to_machines[app_1].update(self.apps_to_machines[app_2]) |
380 | + elif app_2 in subordinates: |
381 | + self.apps_to_machines[app_2].update(self.apps_to_machines[app_1]) |
382 | + |
383 | + @staticmethod |
384 | + def sorted_machines(machine: str) -> Tuple[int, str]: |
385 | + """Sort machines by number and/or containers. |
386 | + |
387 | + :param machine: name of the machine |
388 | + E.g of expected input: "1", "lxd:1" |
389 | + :type machine: str |
390 | + :return: tuple with sort keys |
391 | + :rtype: Tuple[int, str] |
392 | + """ |
393 | + # way to be sure that a machine have at least 2 key values to sort |
394 | + key_1, key_2, *_ = machine.split(":") + [""] |
395 | + |
396 | + if key_1.isdigit(): |
397 | + # not container machines comes first. |
398 | + return int(key_1), "0" |
399 | + # in a container from a bundle, the machine number it's in the end. E.g: lxd:1 |
400 | + else: |
401 | + return int(key_2), key_1 |
402 | + |
403 | + # overwrite parent method passing "bindings" as endpoints_key |
404 | + filter_by_app_and_endpoint = partialmethod( |
405 | + BaseFile.filter_by_app_and_endpoint, endpoints_key="bindings" |
406 | + ) |
407 | + check_app_endpoint_existence = partialmethod( |
408 | + BaseFile.check_app_endpoint_existence, endpoints_key="bindings" |
409 | + ) |
410 | + |
411 | + def filter_by_relation(self, apps: Set, endpoint: str) -> Set: |
412 | + """Filter applications that relate with an endpoint. |
413 | + |
414 | + :param apps: Applications to filter relations using the endpoint. |
415 | + :type apps: Set |
416 | + :param endpoint: endpoint of the applications. |
417 | + :type endpoint: str |
418 | + :return: Applications that has a relation with the apps using the endpoint. |
419 | + :rtype: Set |
420 | + """ |
421 | + apps_related = set() |
422 | + for relation in self.relations_data: |
423 | + for app in apps: |
424 | + app_ep = f"{app}:{endpoint}" |
425 | + app_1_ep_1, app_2_ep_2 = relation |
426 | + if app_1_ep_1 == app_ep: |
427 | + apps_related.add(app_2_ep_2.split(":")[0]) |
428 | + elif app_2_ep_2 == app_ep: |
429 | + apps_related.add(app_1_ep_1.split(":")[0]) |
430 | + return apps_related |
431 | + |
432 | + |
433 | +def input_handler( |
434 | + parsed_yaml: Dict[str, Any], applications_key: str |
435 | +) -> Union[JujuStatusFile, JujuBundleFile]: |
436 | + """Input handler to set right methods and fields. |
437 | + |
438 | + :param parsed_yaml: input file that came from juju status or bundle. |
439 | + :type parsed_yaml: Dict[str, Any] |
440 | + :param applications_key: key to access applications or services (Juju v1) |
441 | + :type applications_key: str |
442 | + :return: Data Class from juju status or bundle. |
443 | + :rtype: Union[JujuStatus, JujuBundle] |
444 | + """ |
445 | + # relations key field is present just on bundles |
446 | + if applications_key in parsed_yaml: |
447 | + if "relations" in parsed_yaml: |
448 | + return JujuBundleFile( |
449 | + applications_data=parsed_yaml[applications_key], |
450 | + machines_data=parsed_yaml["machines"], |
451 | + relations_data=parsed_yaml["relations"], |
452 | + ) |
453 | + else: |
454 | + return JujuStatusFile( |
455 | + applications_data=parsed_yaml[applications_key], |
456 | + machines_data=parsed_yaml["machines"], |
457 | + ) |
458 | diff --git a/jujulint/relations.py b/jujulint/relations.py |
459 | index b6a39d2..6b85a6e 100644 |
460 | --- a/jujulint/relations.py |
461 | +++ b/jujulint/relations.py |
462 | @@ -1,7 +1,9 @@ |
463 | #!/usr/bin/python3 |
464 | """Checks relations between applications.""" |
465 | from logging import getLogger |
466 | -from typing import Any, Dict, List, Set, Tuple |
467 | +from typing import Any, Dict, List, Set, Union |
468 | + |
469 | +from jujulint.model_input import JujuBundleFile, JujuStatusFile |
470 | |
471 | LOGGER = getLogger(__name__) |
472 | |
473 | @@ -23,46 +25,47 @@ class RelationRule: |
474 | |
475 | def __init__( |
476 | self, |
477 | - charm_to_app: Set, |
478 | - applications: Dict[str, Any], |
479 | + input_file: Union[JujuBundleFile, JujuStatusFile], |
480 | charm: str, |
481 | relations: List[List[str]], |
482 | not_exist: List[List[str]], |
483 | exception: Set, |
484 | + ubiquitous: bool, |
485 | ): |
486 | """Relation Rule object. |
487 | |
488 | - :param charm_to_app: A charm can have more than one application name |
489 | - e.g:(nrpe-host and nrpe-container). This parameters contains all |
490 | - applications name of a given charm. |
491 | - :type charm_to_app: Set |
492 | - :param applications: All applications of the file passed to lint. |
493 | - :type applications: Dict[str, Any] |
494 | + :param input_file: mapped content of the input file. |
495 | + :type input_file: Union[JujuBundleFile, JujuStatusFile] |
496 | :param charm: Name of the charm to check the relations rules. |
497 | :type charm: str |
498 | :param relations: Relations that should be checked of a given charm |
499 | having the following format: |
500 | - [[<application>:<application_endpoint>, <application>:<application_endpoint>]] |
501 | + [[<app_1>:<endpoint_1>], [<app_2>:<endpoint_2>]] |
502 | If <application> == "*" it will check the relation using the endpoint |
503 | on all applications form the file passed. |
504 | :type relations: List[List[str]] |
505 | :param not_exist: Relation that should not exist in the presence of the |
506 | relations passed to be checked. This parameter has the following format: |
507 | - [[<application>:<application_endpoint>, <application>:<application_endpoint>]] |
508 | + [[<app_1>:<endpoint_1>], [<app_2>:<endpoint_2>]] |
509 | :type not_exist: List[List[str]], |
510 | :param exception: Set of applications that the rule doesn't apply to. |
511 | :type exception: Set |
512 | + :param ubiquitous: Check if charm is present on all machines. |
513 | + :type ubiquitous: bool |
514 | """ |
515 | - self.charm_to_app = charm_to_app |
516 | - self.applications = applications |
517 | + self.input_file = input_file |
518 | self.charm = charm |
519 | self.relations = relations |
520 | self.not_exist = not_exist |
521 | self.exception = exception |
522 | + self.ubiquitous = ubiquitous |
523 | # remove all possible app names from the subordinate app to not check itself |
524 | - self.apps_to_check = set(self.applications.keys()) - self.charm_to_app |
525 | + # self.apps_to_check = ( |
526 | + # self.input_file.applications - self.input_file.charm_to_app[self.charm] |
527 | + # ) |
528 | self.missing_relations = dict() |
529 | self.not_exist_error = list() |
530 | + self.missing_machines = set() |
531 | |
532 | @property |
533 | def relations(self) -> List[List[str]]: |
534 | @@ -89,16 +92,26 @@ class RelationRule: |
535 | return |
536 | for relation_rule in raw_relations_rules: |
537 | try: |
538 | - app_0, endpoint_0 = self.check_app_endpoint_existence(relation_rule[0]) |
539 | - app_1, endpoint_1 = self.check_app_endpoint_existence(relation_rule[1]) |
540 | + app_0, endpoint_0 = self.input_file.check_app_endpoint_existence( |
541 | + relation_rule[0], self.charm |
542 | + ) |
543 | + app_1, endpoint_1 = self.input_file.check_app_endpoint_existence( |
544 | + relation_rule[1], self.charm |
545 | + ) |
546 | if not all([app_0, endpoint_0, app_1, endpoint_1]): |
547 | # means that app or endpoint was not found |
548 | return |
549 | - if app_0 == self.charm: |
550 | + if ( |
551 | + app_0 == self.charm |
552 | + or app_0 in self.input_file.charm_to_app[self.charm] |
553 | + ): |
554 | self.endpoint = endpoint_0 |
555 | app_to_check = app_1 |
556 | endpoint_to_check = endpoint_1 |
557 | - elif app_1 == self.charm: |
558 | + elif ( |
559 | + app_1 == self.charm |
560 | + or app_1 in self.input_file.charm_to_app[self.charm] |
561 | + ): |
562 | self.endpoint = endpoint_1 |
563 | app_to_check = app_0 |
564 | endpoint_to_check = endpoint_0 |
565 | @@ -111,8 +124,10 @@ class RelationRule: |
566 | return |
567 | |
568 | # check if all apps variations has the endpoint |
569 | - for app in self.charm_to_app: |
570 | - self.check_app_endpoint_existence(f"{app}:{self.endpoint}") |
571 | + for app in self.input_file.charm_to_app[self.charm]: |
572 | + self.input_file.check_app_endpoint_existence( |
573 | + f"{app}:{self.endpoint}", self.charm |
574 | + ) |
575 | self._relations.append([app_to_check, endpoint_to_check]) |
576 | except (IndexError, ValueError) as e: |
577 | raise RelationError(f"Relations rules has an unexpected format: {e}") |
578 | @@ -127,71 +142,32 @@ class RelationRule: |
579 | |
580 | def check(self) -> None: |
581 | """Apply the relations rules check.""" |
582 | - self.relation_exist_check() |
583 | - self.relation_not_exist_check() |
584 | - |
585 | - def filter_by_app_and_endpoint(self, app: str, endpoint: str) -> Set: |
586 | - """Filter applications by the presence of an endpoint. |
587 | - |
588 | - :param app: Application to be filtered. |
589 | - :type app: str |
590 | - :param endpoint: Endpoint of a application. |
591 | - :type endpoint: str |
592 | - :return: Applications that matched with the endpoint passed. |
593 | - :rtype: Set |
594 | - """ |
595 | - # NOTE(gabrielcocenza) this function just works with fields from juju status. |
596 | - # when app == "*", filters all apps that have the endpoint passed. |
597 | - if app == "*": |
598 | - return { |
599 | - app |
600 | - for app in self.apps_to_check |
601 | - if endpoint |
602 | - in self.applications.get(app, {}).get("endpoint-bindings", {}) |
603 | - } |
604 | - return ( |
605 | - set([app]) |
606 | - if endpoint in self.applications.get(app, {}).get("endpoint-bindings", {}) |
607 | - else set() |
608 | - ) |
609 | - |
610 | - def filter_by_relation(self, apps: Set, endpoint: str) -> Set: |
611 | - """Filter applications that relate with an endpoint. |
612 | - |
613 | - :param apps: Applications to filter relations using the endpoint. |
614 | - :type apps: Set |
615 | - :param endpoint: endpoint of the applications. |
616 | - :type endpoint: str |
617 | - :return: Applications that has a relation with the apps using the endpoint. |
618 | - :rtype: Set |
619 | - """ |
620 | - # NOTE(gabrielcocenza) this function just works with fields from juju status. |
621 | - apps_related = set() |
622 | - for app in apps: |
623 | - relations = self.applications.get(app, {}).get("relations", {}) |
624 | - apps_related.update(relations.get(endpoint, [])) |
625 | - return apps_related |
626 | + try: |
627 | + self.missing_machines = self.ubiquitous_check() |
628 | + self.relation_exist_check() |
629 | + self.relation_not_exist_check() |
630 | + except NotImplementedError as e: |
631 | + LOGGER.debug(e) |
632 | |
633 | def relation_exist_check(self) -> None: |
634 | """Check if app(s) are relating with an endpoint.""" |
635 | for relation in self.relations: |
636 | app_to_check, endpoint_to_check = relation |
637 | # applications in the bundle that have the endpoint to relate |
638 | - apps_with_endpoint_to_check = self.filter_by_app_and_endpoint( |
639 | + apps_with_endpoint_to_check = self.input_file.filter_by_app_and_endpoint( |
640 | + self.charm, |
641 | app_to_check, |
642 | endpoint_to_check, |
643 | ) |
644 | # applications that are relating using the endpoint from the relation rule |
645 | - apps_related_with_relation_rule = self.filter_by_relation( |
646 | - self.charm_to_app, |
647 | + apps_related_with_relation_rule = self.input_file.filter_by_relation( |
648 | + self.input_file.charm_to_app[self.charm], |
649 | self.endpoint, |
650 | ) |
651 | self.missing_relations[f"{self.charm}:{self.endpoint}"] = sorted( |
652 | - list( |
653 | - apps_with_endpoint_to_check |
654 | - - apps_related_with_relation_rule |
655 | - - self.exception |
656 | - ) |
657 | + apps_with_endpoint_to_check |
658 | + - apps_related_with_relation_rule |
659 | + - self.exception |
660 | ) |
661 | |
662 | def relation_not_exist_check(self) -> None: |
663 | @@ -204,7 +180,9 @@ class RelationRule: |
664 | app_endpoint_splitted = [] |
665 | try: |
666 | for app_endpoint in relation: |
667 | - app, endpoint = self.check_app_endpoint_existence(app_endpoint) |
668 | + app, endpoint = self.input_file.check_app_endpoint_existence( |
669 | + app_endpoint, self.charm |
670 | + ) |
671 | app_endpoint_splitted.extend([app, endpoint]) |
672 | ( |
673 | app_to_check_0, |
674 | @@ -212,7 +190,7 @@ class RelationRule: |
675 | app_to_check_1, |
676 | _, |
677 | ) = app_endpoint_splitted |
678 | - relations_app_endpoint_0 = self.filter_by_relation( |
679 | + relations_app_endpoint_0 = self.input_file.filter_by_relation( |
680 | {app_to_check_0}, endpoint_to_check_0 |
681 | ) |
682 | if app_to_check_1 in relations_app_endpoint_0: |
683 | @@ -221,32 +199,23 @@ class RelationRule: |
684 | except (IndexError, ValueError) as e: |
685 | raise RelationError(f"Problem during check_relation_not_exist: {e}") |
686 | |
687 | - def check_app_endpoint_existence(self, app_endpoint: str) -> Tuple[str, str]: |
688 | - """Check if app and endpoint exist on the object to lint. |
689 | + def ubiquitous_check(self) -> List[str]: |
690 | + """Check if charm from relation rule is present on all machines. |
691 | |
692 | - :param app_endpoint: app and endpoint separated by ":" with the following format: |
693 | - <application>:<application_endpoint> |
694 | - :type app_endpoint: str |
695 | - :return: application and endpoint |
696 | - :rtype: Tuple[str, str] |
697 | + :return: Sorted list of machines missing the charm. If is present on |
698 | + all machines, returns an empty list. |
699 | + :rtype: List[str] |
700 | """ |
701 | - app, endpoint = app_endpoint.split(":") |
702 | - # app == "*" means all apps |
703 | - # a charm from relation rule can have different app names. |
704 | - if app != "*" and app != self.charm: |
705 | - if app not in self.applications.keys(): |
706 | - LOGGER.warning(f"{app} not found on applications to check relations") |
707 | - return "", "" |
708 | - |
709 | - # juju-info is represented by "" on endpoint-bindings |
710 | - if endpoint != "juju-info" and endpoint not in self.applications[app].get( |
711 | - "endpoint-bindings", {} |
712 | - ): |
713 | - LOGGER.warning( |
714 | - f"{app} don't have the endpoint: {endpoint} to check relations" |
715 | - ) |
716 | - return "", "" |
717 | - return app, endpoint |
718 | + if self.ubiquitous: |
719 | + machines_with_charm = set() |
720 | + for app in self.input_file.charm_to_app[self.charm]: |
721 | + machines_with_charm.update(self.input_file.apps_to_machines[app]) |
722 | + |
723 | + return sorted( |
724 | + self.input_file.machines - machines_with_charm, |
725 | + key=self.input_file.sorted_machines, |
726 | + ) |
727 | + return [] |
728 | |
729 | |
730 | class RelationsRulesBootStrap: |
731 | @@ -254,24 +223,18 @@ class RelationsRulesBootStrap: |
732 | |
733 | def __init__( |
734 | self, |
735 | - charm_to_app: Dict[str, Set], |
736 | relations_rules: List[Dict[str, Any]], |
737 | - applications: Dict[str, Any], |
738 | + input_file: Union[JujuBundleFile, JujuStatusFile], |
739 | ): |
740 | """Relations rules bootStrap object. |
741 | |
742 | - :param charm_to_app: A charm can have more than one application name |
743 | - e.g:(nrpe-host and nrpe-container). This parameters contains all |
744 | - applications name of a given charm. |
745 | - :type charm_to_app: Dict[str, Set] |
746 | :param relations_rules: Relations rules from the rule file. |
747 | :type relations_rules: List[Dict[str, Any]] |
748 | - :param applications: All applications of the file passed to lint. |
749 | - :type applications: Dict[str, Any] |
750 | + :param input_file: mapped content of the input file. |
751 | + :type input_file: Union[JujuBundleFile, JujuStatusFile] |
752 | """ |
753 | - self.charm_to_app = charm_to_app |
754 | self.relations_rules = relations_rules |
755 | - self.applications = applications |
756 | + self.input_file = input_file |
757 | |
758 | def check(self) -> List[RelationRule]: |
759 | """Check all RelationRule objects. |
760 | @@ -281,12 +244,12 @@ class RelationsRulesBootStrap: |
761 | """ |
762 | relations_rules = [ |
763 | RelationRule( |
764 | - charm_to_app=self.charm_to_app.get(rule.get("charm", ""), set()), |
765 | - applications=self.applications, |
766 | + input_file=self.input_file, |
767 | charm=rule.get("charm"), |
768 | relations=rule.get("check", [[]]), |
769 | not_exist=rule.get("not-exist", [[]]), |
770 | exception=set(rule.get("exception", set())), |
771 | + ubiquitous=rule.get("ubiquitous", False), |
772 | ) |
773 | for rule in self.relations_rules |
774 | ] |
775 | diff --git a/setup.py b/setup.py |
776 | index 9314d60..9bd497c 100644 |
777 | --- a/setup.py |
778 | +++ b/setup.py |
779 | @@ -41,7 +41,7 @@ setuptools.setup( |
780 | "Environment :: Plugins", |
781 | "Intended Audience :: System Administrators", |
782 | ], |
783 | - python_requires=">=3.4", |
784 | + python_requires=">=3.7", |
785 | packages=["jujulint"], |
786 | entry_points={"console_scripts": ["juju-lint=jujulint.cli:main"]}, |
787 | setup_requires=["setuptools_scm"], |
788 | diff --git a/tests/unit/conftest.py b/tests/unit/conftest.py |
789 | index 00500ea..b49a4b2 100644 |
790 | --- a/tests/unit/conftest.py |
791 | +++ b/tests/unit/conftest.py |
792 | @@ -16,12 +16,13 @@ from unittest.mock import MagicMock |
793 | import mock |
794 | import pytest |
795 | |
796 | +from jujulint import cloud # noqa: E402 |
797 | +from jujulint.model_input import JujuBundleFile, JujuStatusFile |
798 | + |
799 | # bring in top level library to path |
800 | test_path = os.path.dirname(os.path.abspath(__file__)) |
801 | sys.path.insert(0, test_path + "/../") |
802 | |
803 | -from jujulint import cloud # noqa: E402 |
804 | - |
805 | |
806 | @pytest.fixture |
807 | def mocked_pkg_resources(monkeypatch): |
808 | @@ -130,6 +131,7 @@ def juju_status(): |
809 | "charm": "cs:ntp-47", |
810 | "charm-name": "ntp", |
811 | "relations": {"juju-info": ["ubuntu"]}, |
812 | + "subordinate-to": ["ubuntu"], |
813 | "endpoint-bindings": { |
814 | "": "external-space", |
815 | "certificates": "external-space", |
816 | @@ -240,82 +242,237 @@ def rules_files(): |
817 | |
818 | |
819 | @pytest.fixture |
820 | -def juju_status_relation(): |
821 | +def input_files(parsed_yaml_status, parsed_yaml_bundle): |
822 | + return { |
823 | + "juju-status": JujuStatusFile( |
824 | + applications_data=parsed_yaml_status["applications"], |
825 | + machines_data=parsed_yaml_status["machines"], |
826 | + ), |
827 | + "juju-bundle": JujuBundleFile( |
828 | + applications_data=parsed_yaml_bundle["applications"], |
829 | + machines_data=parsed_yaml_bundle["machines"], |
830 | + relations_data=parsed_yaml_bundle["relations"], |
831 | + ), |
832 | + } |
833 | + |
834 | + |
835 | +@pytest.fixture |
836 | +def parsed_yaml_status(): |
837 | """Representation of juju status input to test relations checks.""" |
838 | return { |
839 | - "nrpe-container": { |
840 | - "charm": "cs:nrpe-61", |
841 | - "charm-name": "nrpe", |
842 | - "relations": { |
843 | - "monitors": ["nagios"], |
844 | - "nrpe-external-master": [ |
845 | - "keystone", |
846 | - ], |
847 | + "applications": { |
848 | + "nrpe-container": { |
849 | + "charm": "cs:nrpe-61", |
850 | + "charm-name": "nrpe", |
851 | + "relations": { |
852 | + "nrpe-external-master": [ |
853 | + "keystone", |
854 | + ], |
855 | + }, |
856 | + "endpoint-bindings": { |
857 | + "general-info": "", |
858 | + "local-monitors": "", |
859 | + "monitors": "oam-space", |
860 | + "nrpe": "", |
861 | + "nrpe-external-master": "", |
862 | + }, |
863 | + "subordinate-to": ["keystone"], |
864 | }, |
865 | - "endpoint-bindings": { |
866 | - "general-info": "", |
867 | - "local-monitors": "", |
868 | - "monitors": "oam-space", |
869 | - "nrpe": "", |
870 | - "nrpe-external-master": "", |
871 | + "nrpe-host": { |
872 | + "charm": "cs:nrpe-67", |
873 | + "charm-name": "nrpe", |
874 | + "relations": { |
875 | + "nrpe-external-master": [ |
876 | + "elasticsearch", |
877 | + ], |
878 | + "general-info": ["ubuntu"], |
879 | + }, |
880 | + "endpoint-bindings": { |
881 | + "general-info": "", |
882 | + "local-monitors": "", |
883 | + "monitors": "oam-space", |
884 | + "nrpe": "", |
885 | + "nrpe-external-master": "", |
886 | + }, |
887 | + "subordinate-to": ["elasticsearch", "ubuntu"], |
888 | }, |
889 | - }, |
890 | - "nrpe-host": { |
891 | - "charm": "cs:nrpe-67", |
892 | - "charm-name": "nrpe", |
893 | - "relations": { |
894 | - "monitors": ["nagios"], |
895 | - "nrpe-external-master": [ |
896 | - "elasticsearch", |
897 | - ], |
898 | + "ubuntu": { |
899 | + "application-status": {"current": "active"}, |
900 | + "charm": "cs:ubuntu-18", |
901 | + "charm-name": "ubuntu", |
902 | + "relations": {"juju-info": ["nrpe-host"]}, |
903 | + "endpoint-bindings": { |
904 | + "": "external-space", |
905 | + "certificates": "external-space", |
906 | + }, |
907 | + "units": { |
908 | + "ubuntu/0": { |
909 | + "machine": "1", |
910 | + "workload-status": {"current": "active"}, |
911 | + "subordinates": { |
912 | + "nrpe-host/0": { |
913 | + "workload-status": { |
914 | + "current": "active", |
915 | + } |
916 | + } |
917 | + }, |
918 | + } |
919 | + }, |
920 | }, |
921 | - "endpoint-bindings": { |
922 | - "general-info": "", |
923 | - "local-monitors": "", |
924 | - "monitors": "oam-space", |
925 | - "nrpe": "", |
926 | - "nrpe-external-master": "", |
927 | + "keystone": { |
928 | + "charm": "cs:keystone-309", |
929 | + "charm-name": "keystone", |
930 | + "relations": { |
931 | + "nrpe-external-master": ["nrpe-container"], |
932 | + }, |
933 | + "endpoint-bindings": { |
934 | + "": "oam-space", |
935 | + "admin": "external-space", |
936 | + "certificates": "oam-space", |
937 | + "cluster": "oam-space", |
938 | + "domain-backend": "oam-space", |
939 | + "ha": "oam-space", |
940 | + "identity-admin": "oam-space", |
941 | + "identity-credentials": "oam-space", |
942 | + "identity-notifications": "oam-space", |
943 | + "identity-service": "oam-space", |
944 | + "internal": "internal-space", |
945 | + "keystone-fid-service-provider": "oam-space", |
946 | + "keystone-middleware": "oam-space", |
947 | + "nrpe-external-master": "oam-space", |
948 | + "public": "external-space", |
949 | + "shared-db": "internal-space", |
950 | + "websso-trusted-dashboard": "oam-space", |
951 | + }, |
952 | + "units": { |
953 | + "keystone/0": { |
954 | + "machine": "1/lxd/0", |
955 | + "subordinates": { |
956 | + "nrpe-container/0": { |
957 | + "workload-status": { |
958 | + "current": "active", |
959 | + } |
960 | + } |
961 | + }, |
962 | + } |
963 | + }, |
964 | + }, |
965 | + "elasticsearch": { |
966 | + "charm": "cs:elasticsearch-39", |
967 | + "charm-name": "elasticsearch", |
968 | + "relations": { |
969 | + "nrpe-external-master": ["nrpe-host"], |
970 | + }, |
971 | + "endpoint-bindings": { |
972 | + "": "oam-space", |
973 | + "client": "oam-space", |
974 | + "data": "oam-space", |
975 | + "logs": "oam-space", |
976 | + "nrpe-external-master": "oam-space", |
977 | + "peer": "oam-space", |
978 | + }, |
979 | + "units": { |
980 | + "elasticsearch/0": { |
981 | + "machine": "0", |
982 | + "subordinates": { |
983 | + "nrpe-host/0": { |
984 | + "workload-status": { |
985 | + "current": "active", |
986 | + } |
987 | + } |
988 | + }, |
989 | + } |
990 | + }, |
991 | }, |
992 | }, |
993 | - "keystone": { |
994 | - "charm": "cs:keystone-309", |
995 | - "charm-name": "keystone", |
996 | - "relations": { |
997 | - "nrpe-external-master": ["nrpe-container"], |
998 | + "machines": { |
999 | + "0": { |
1000 | + "series": "focal", |
1001 | }, |
1002 | - "endpoint-bindings": { |
1003 | - "": "oam-space", |
1004 | - "admin": "external-space", |
1005 | - "certificates": "oam-space", |
1006 | - "cluster": "oam-space", |
1007 | - "domain-backend": "oam-space", |
1008 | - "ha": "oam-space", |
1009 | - "identity-admin": "oam-space", |
1010 | - "identity-credentials": "oam-space", |
1011 | - "identity-notifications": "oam-space", |
1012 | - "identity-service": "oam-space", |
1013 | - "internal": "internal-space", |
1014 | - "keystone-fid-service-provider": "oam-space", |
1015 | - "keystone-middleware": "oam-space", |
1016 | - "nrpe-external-master": "oam-space", |
1017 | - "public": "external-space", |
1018 | - "shared-db": "internal-space", |
1019 | - "websso-trusted-dashboard": "oam-space", |
1020 | + "1": { |
1021 | + "series": "focal", |
1022 | + "containers": { |
1023 | + "1/lxd/0": { |
1024 | + "series": "focal", |
1025 | + } |
1026 | + }, |
1027 | }, |
1028 | }, |
1029 | - "elasticsearch": { |
1030 | - "charm": "cs:elasticsearch-39", |
1031 | - "charm-name": "elasticsearch", |
1032 | - "relations": { |
1033 | - "nrpe-external-master": ["nrpe-host"], |
1034 | + } |
1035 | + |
1036 | + |
1037 | +@pytest.fixture |
1038 | +def parsed_yaml_bundle(): |
1039 | + """Representation of juju bundle input to test relations checks.""" |
1040 | + return { |
1041 | + "series": "focal", |
1042 | + "applications": { |
1043 | + "elasticsearch": { |
1044 | + "bindings": { |
1045 | + "nrpe-external-master": "internal-space", |
1046 | + }, |
1047 | + "charm": "elasticsearch", |
1048 | + "channel": "stable", |
1049 | + "revision": 59, |
1050 | + "num_units": 1, |
1051 | + "to": ["0"], |
1052 | + "constraints": "arch=amd64 mem=4096", |
1053 | }, |
1054 | - "endpoint-bindings": { |
1055 | - "": "oam-space", |
1056 | - "client": "oam-space", |
1057 | - "data": "oam-space", |
1058 | - "logs": "oam-space", |
1059 | - "nrpe-external-master": "oam-space", |
1060 | - "peer": "oam-space", |
1061 | + "keystone": { |
1062 | + "bindings": { |
1063 | + "nrpe-external-master": "internal-space", |
1064 | + "public": "internal-space", |
1065 | + }, |
1066 | + "charm": "keystone", |
1067 | + "channel": "stable", |
1068 | + "revision": 539, |
1069 | + "resources": {"policyd-override": 0}, |
1070 | + "num_units": 1, |
1071 | + "to": ["lxd:1"], |
1072 | + "constraints": "arch=amd64", |
1073 | + }, |
1074 | + "nrpe-container": { |
1075 | + "bindings": { |
1076 | + "nrpe-external-master": "internal-space", |
1077 | + "local-monitors": "", |
1078 | + }, |
1079 | + "charm": "nrpe", |
1080 | + "channel": "stable", |
1081 | + "revision": 94, |
1082 | + }, |
1083 | + "nrpe-host": { |
1084 | + "bindings": { |
1085 | + "nrpe-external-master": "internal-space", |
1086 | + "local-monitors": "", |
1087 | + }, |
1088 | + "charm": "nrpe", |
1089 | + "channel": "stable", |
1090 | + "revision": 94, |
1091 | + }, |
1092 | + "ubuntu": { |
1093 | + "bindings": {"": "internal-space", "certificates": "external-space"}, |
1094 | + "charm": "ubuntu", |
1095 | + "channel": "stable", |
1096 | + "revision": 21, |
1097 | + "num_units": 1, |
1098 | + "to": ["1"], |
1099 | + "options": {"hostname": ""}, |
1100 | + "constraints": "arch=amd64 mem=4096", |
1101 | }, |
1102 | }, |
1103 | + "machines": { |
1104 | + "0": {"constraints": "arch=amd64 mem=4096"}, |
1105 | + "1": {"constraints": "arch=amd64 mem=4096"}, |
1106 | + }, |
1107 | + "relations": [ |
1108 | + [ |
1109 | + "nrpe-container:nrpe-external-master", |
1110 | + "keystone:nrpe-external-master", |
1111 | + ], |
1112 | + ["nrpe-host:general-info", "ubuntu:juju-info"], |
1113 | + [ |
1114 | + "elasticsearch:nrpe-external-master", |
1115 | + "nrpe-host:nrpe-external-master", |
1116 | + ], |
1117 | + ], |
1118 | } |
1119 | diff --git a/tests/unit/test_input.py b/tests/unit/test_input.py |
1120 | new file mode 100644 |
1121 | index 0000000..682b664 |
1122 | --- /dev/null |
1123 | +++ b/tests/unit/test_input.py |
1124 | @@ -0,0 +1,233 @@ |
1125 | +from dataclasses import dataclass |
1126 | + |
1127 | +import pytest |
1128 | + |
1129 | +from jujulint import model_input |
1130 | + |
1131 | + |
1132 | +@pytest.mark.parametrize("input_file_type", ["juju-status", "juju-bundle"]) |
1133 | +def test_file_inputs(input_files, input_file_type): |
1134 | + """Test that files are mapping properties as expected.""" |
1135 | + input_file = input_files[input_file_type] |
1136 | + expected_output = { |
1137 | + "applications": { |
1138 | + "elasticsearch", |
1139 | + "ubuntu", |
1140 | + "keystone", |
1141 | + "nrpe-container", |
1142 | + "nrpe-host", |
1143 | + }, |
1144 | + "machines": { |
1145 | + "juju-status": {"0", "1", "1/lxd/0"}, |
1146 | + "juju-bundle": {"0", "1", "lxd:1"}, |
1147 | + }, |
1148 | + "charms": {"elasticsearch", "ubuntu", "nrpe", "keystone"}, |
1149 | + "app_to_charm": { |
1150 | + "elasticsearch": "elasticsearch", |
1151 | + "ubuntu": "ubuntu", |
1152 | + "keystone": "keystone", |
1153 | + "nrpe-host": "nrpe", |
1154 | + "nrpe-container": "nrpe", |
1155 | + }, |
1156 | + "charm_to_app": { |
1157 | + "nrpe": {"nrpe-container", "nrpe-host"}, |
1158 | + "ubuntu": {"ubuntu"}, |
1159 | + "elasticsearch": {"elasticsearch"}, |
1160 | + "keystone": {"keystone"}, |
1161 | + }, |
1162 | + "apps_to_machines": { |
1163 | + "juju-status": { |
1164 | + "nrpe-container": {"1/lxd/0"}, |
1165 | + "nrpe-host": {"0", "1"}, |
1166 | + "ubuntu": {"1"}, |
1167 | + "elasticsearch": {"0"}, |
1168 | + "keystone": {"1/lxd/0"}, |
1169 | + }, |
1170 | + "juju-bundle": { |
1171 | + "nrpe-container": {"lxd:1"}, |
1172 | + "nrpe-host": {"0", "1"}, |
1173 | + "ubuntu": {"1"}, |
1174 | + "elasticsearch": {"0"}, |
1175 | + "keystone": {"lxd:1"}, |
1176 | + }, |
1177 | + }, |
1178 | + } |
1179 | + assert input_file.applications == expected_output["applications"] |
1180 | + assert input_file.machines == expected_output["machines"][input_file_type] |
1181 | + assert ( |
1182 | + input_file.apps_to_machines |
1183 | + == expected_output["apps_to_machines"][input_file_type] |
1184 | + ) |
1185 | + assert input_file.charms == expected_output["charms"] |
1186 | + assert input_file.app_to_charm == expected_output["app_to_charm"] |
1187 | + assert input_file.charm_to_app == expected_output["charm_to_app"] |
1188 | + |
1189 | + |
1190 | +@pytest.mark.parametrize( |
1191 | + "app_endpoint, app_error, endpoint_error, input_file_type, expected_output", |
1192 | + [ |
1193 | + # app doesn't exist |
1194 | + ("foo:juju-info", True, False, "juju-status", ("", "")), |
1195 | + ("foo:juju-info", True, False, "juju-bundle", ("", "")), |
1196 | + # endpoint doesn't exist |
1197 | + ("keystone:bar", False, True, "juju-status", ("", "")), |
1198 | + ("keystone:bar", False, True, "juju-bundle", ("", "")), |
1199 | + # app and endpoint exist |
1200 | + ( |
1201 | + "keystone:nrpe-external-master", |
1202 | + False, |
1203 | + False, |
1204 | + "juju-status", |
1205 | + ("keystone", "nrpe-external-master"), |
1206 | + ), |
1207 | + ( |
1208 | + "keystone:nrpe-external-master", |
1209 | + False, |
1210 | + False, |
1211 | + "juju-bundle", |
1212 | + ("keystone", "nrpe-external-master"), |
1213 | + ), |
1214 | + ], |
1215 | +) |
1216 | +def test_check_app_endpoint_existence( |
1217 | + app_endpoint, |
1218 | + app_error, |
1219 | + endpoint_error, |
1220 | + mocker, |
1221 | + input_files, |
1222 | + input_file_type, |
1223 | + expected_output, |
1224 | +): |
1225 | + """Test the expected check_app_endpoint_existence method behavior.""" |
1226 | + input_file = input_files[input_file_type] |
1227 | + logger_mock = mocker.patch.object(model_input, "LOGGER") |
1228 | + app, endpoint = app_endpoint.split(":") |
1229 | + expected_msg = "" |
1230 | + if app_error: |
1231 | + expected_msg = f"{app} not found on applications." |
1232 | + elif endpoint_error: |
1233 | + expected_msg = f"endpoint: {endpoint} not found on {app}" |
1234 | + |
1235 | + assert ( |
1236 | + input_file.check_app_endpoint_existence(app_endpoint, "nrpe") == expected_output |
1237 | + ) |
1238 | + if expected_msg: |
1239 | + logger_mock.warning.assert_has_calls([mocker.call(expected_msg)]) |
1240 | + |
1241 | + |
1242 | +@pytest.mark.parametrize( |
1243 | + "input_file_type, charm, app, endpoint, expected_output", |
1244 | + [ |
1245 | + ( |
1246 | + "juju-status", |
1247 | + "nrpe", |
1248 | + "*", |
1249 | + "nrpe-external-master", |
1250 | + {"keystone", "elasticsearch"}, |
1251 | + ), # all apps with nrpe-external-master |
1252 | + ( |
1253 | + "juju-bundle", |
1254 | + "nrpe", |
1255 | + "*", |
1256 | + "nrpe-external-master", |
1257 | + {"keystone", "elasticsearch"}, |
1258 | + ), # all apps with nrpe-external-master |
1259 | + ( |
1260 | + "juju-status", |
1261 | + "nrpe", |
1262 | + "keystone", |
1263 | + "nrpe-external-master", |
1264 | + {"keystone"}, |
1265 | + ), # check if keystone has nrpe-external-master |
1266 | + ( |
1267 | + "juju-bundle", |
1268 | + "nrpe", |
1269 | + "keystone", |
1270 | + "nrpe-external-master", |
1271 | + {"keystone"}, |
1272 | + ), # check if keystone has nrpe-external-master |
1273 | + ( |
1274 | + "juju-status", |
1275 | + "nrpe", |
1276 | + "ubuntu", |
1277 | + "nrpe-external-master", |
1278 | + set(), |
1279 | + ), # check if ubuntu has nrpe-external-master |
1280 | + ( |
1281 | + "juju-bundle", |
1282 | + "nrpe", |
1283 | + "ubuntu", |
1284 | + "nrpe-external-master", |
1285 | + set(), |
1286 | + ), # check if ubuntu has nrpe-external-master |
1287 | + ], |
1288 | +) |
1289 | +def test_filter_by_app_and_endpoint( |
1290 | + input_files, input_file_type, charm, app, endpoint, expected_output |
1291 | +): |
1292 | + """Test filter_by_app_and_endpoint method behave as expected.""" |
1293 | + input_file = input_files[input_file_type] |
1294 | + assert ( |
1295 | + input_file.filter_by_app_and_endpoint(charm, app, endpoint) == expected_output |
1296 | + ) |
1297 | + |
1298 | + |
1299 | +@pytest.mark.parametrize( |
1300 | + "input_file_type, endpoint, expected_output", |
1301 | + [ |
1302 | + ("juju-status", "nrpe-external-master", {"keystone", "elasticsearch"}), |
1303 | + ("juju-bundle", "nrpe-external-master", {"keystone", "elasticsearch"}), |
1304 | + ("juju-status", "general-info", {"ubuntu"}), |
1305 | + ("juju-bundle", "general-info", {"ubuntu"}), |
1306 | + ("juju-status", "monitors", set()), |
1307 | + ("juju-bundle", "monitors", set()), |
1308 | + ], |
1309 | +) |
1310 | +def test_filter_by_relation(input_file_type, endpoint, expected_output, input_files): |
1311 | + """Test filter_by_relation method behave as expected.""" |
1312 | + input_file = input_files[input_file_type] |
1313 | + assert ( |
1314 | + input_file.filter_by_relation(input_file.charm_to_app["nrpe"], endpoint) |
1315 | + == expected_output |
1316 | + ) |
1317 | + |
1318 | + |
1319 | +@pytest.mark.parametrize( |
1320 | + "parsed_yaml, expected_output", |
1321 | + [ |
1322 | + ("parsed_yaml_status", model_input.JujuStatusFile), |
1323 | + ("parsed_yaml_bundle", model_input.JujuBundleFile), |
1324 | + ], |
1325 | +) |
1326 | +def test_input_handler(parsed_yaml, expected_output, request): |
1327 | + """Input handler return expected objects depending on the input.""" |
1328 | + input_file = request.getfixturevalue(parsed_yaml) |
1329 | + assert isinstance( |
1330 | + model_input.input_handler(input_file, "applications"), expected_output |
1331 | + ) |
1332 | + |
1333 | + |
1334 | +def test_raise_not_implemented_methods(parsed_yaml_status): |
1335 | + # declare a new input class |
1336 | + @dataclass |
1337 | + class MyNewInput(model_input.BaseFile): |
1338 | + # overwrite parent method to map file |
1339 | + def __post_init__(self): |
1340 | + return 0 |
1341 | + |
1342 | + new_input = MyNewInput( |
1343 | + applications_data=parsed_yaml_status["applications"], |
1344 | + machines_data=parsed_yaml_status["machines"], |
1345 | + ) |
1346 | + |
1347 | + with pytest.raises(NotImplementedError): |
1348 | + new_input.map_machines() |
1349 | + |
1350 | + with pytest.raises(NotImplementedError): |
1351 | + new_input.map_apps_to_machines() |
1352 | + |
1353 | + with pytest.raises(NotImplementedError): |
1354 | + new_input.filter_by_relation({"nrpe"}, "nrpe-external-master") |
1355 | + |
1356 | + with pytest.raises(NotImplementedError): |
1357 | + new_input.sorted_machines("0") |
1358 | diff --git a/tests/unit/test_jujulint.py b/tests/unit/test_jujulint.py |
1359 | index 136044b..264b15e 100644 |
1360 | --- a/tests/unit/test_jujulint.py |
1361 | +++ b/tests/unit/test_jujulint.py |
1362 | @@ -1233,22 +1233,35 @@ applications: |
1363 | """Test conversion of string values (e.g. 2M (Megabytes)) to integers.""" |
1364 | assert linter.atoi(input_str) == expected_int |
1365 | |
1366 | - def test_check_relations_no_rules(self, linter, juju_status, mocker): |
1367 | + @pytest.mark.parametrize("input_file_type", ["juju-status", "juju-bundle"]) |
1368 | + def test_check_relations_no_rules( |
1369 | + self, linter, input_files, mocker, input_file_type |
1370 | + ): |
1371 | """Warn message if rule file doesn't pass relations to check.""" |
1372 | mock_log = mocker.patch("jujulint.lint.Linter._log_with_header") |
1373 | - linter.check_relations(juju_status["applications"]) |
1374 | + linter.check_relations(input_files[input_file_type]) |
1375 | mock_log.assert_called_with("No relation rules found. Skipping relation checks") |
1376 | |
1377 | - def test_check_relations(self, linter, juju_status, mocker): |
1378 | + @pytest.mark.parametrize("input_file_type", ["juju-status", "juju-bundle"]) |
1379 | + def test_check_relations(self, linter, input_files, mocker, input_file_type): |
1380 | """Ensure that check_relation pass.""" |
1381 | mock_handle_error = mocker.patch("jujulint.lint.Linter.handle_error") |
1382 | linter.lint_rules["relations"] = [ |
1383 | - {"charm": "ntp", "check": [["ntp:juju-info", "ubuntu:juju-info"]]} |
1384 | + {"charm": "nrpe", "check": [["nrpe:juju-info", "ubuntu:juju-info"]]} |
1385 | ] |
1386 | - linter.check_relations(juju_status["applications"]) |
1387 | + linter.check_relations(input_files[input_file_type]) |
1388 | mock_handle_error.assert_not_called() |
1389 | |
1390 | - def test_check_relations_exception_handling(self, linter, juju_status, mocker): |
1391 | + @pytest.mark.parametrize( |
1392 | + "input_file_type", |
1393 | + [ |
1394 | + "juju-status", |
1395 | + "juju-bundle", |
1396 | + ], |
1397 | + ) |
1398 | + def test_check_relations_exception_handling( |
1399 | + self, linter, juju_status, mocker, input_file_type, input_files |
1400 | + ): |
1401 | """Ensure that handle error if relation rules are in wrong format.""" |
1402 | mock_log = mocker.patch("jujulint.lint.Linter._log_with_header") |
1403 | mock_handle_error = mocker.patch("jujulint.lint.Linter.handle_error") |
1404 | @@ -1260,44 +1273,88 @@ applications: |
1405 | "values to unpack (expected 2, got 1)" |
1406 | ) |
1407 | expected_exception = relations.RelationError(expected_msg) |
1408 | - linter.check_relations(juju_status["applications"]) |
1409 | + |
1410 | + linter.check_relations(input_files[input_file_type]) |
1411 | mock_handle_error.assert_not_called() |
1412 | mock_log.assert_has_calls( |
1413 | [mocker.call(expected_exception.message, level=logging.ERROR)] |
1414 | ) |
1415 | |
1416 | - def test_check_relations_missing_relations(self, linter, juju_status, mocker): |
1417 | + @pytest.mark.parametrize("input_file_type", ["juju-status", "juju-bundle"]) |
1418 | + def test_check_relations_missing_relations( |
1419 | + self, linter, juju_status, mocker, input_file_type, input_files |
1420 | + ): |
1421 | """Ensure that check_relation handle missing relations.""" |
1422 | mock_handle_error = mocker.patch("jujulint.lint.Linter.handle_error") |
1423 | # add a relation rule that doesn't happen in the model |
1424 | linter.lint_rules["relations"] = [ |
1425 | - {"charm": "ntp", "check": [["ntp:certificates", "ubuntu:certificates"]]} |
1426 | + { |
1427 | + "charm": "nrpe", |
1428 | + "check": [["nrpe-host:local-monitors", "ubuntu:certificates"]], |
1429 | + } |
1430 | ] |
1431 | - linter.check_relations(juju_status["applications"]) |
1432 | + linter.check_relations(input_files[input_file_type]) |
1433 | mock_handle_error.assert_called_with( |
1434 | { |
1435 | "id": "missing-relations", |
1436 | "tags": ["relation", "missing"], |
1437 | "message": "Endpoint '{}' is missing relations with: {}".format( |
1438 | - "ntp:certificates", ["ubuntu"] |
1439 | + "nrpe:local-monitors", ["ubuntu"] |
1440 | ), |
1441 | } |
1442 | ) |
1443 | |
1444 | - def test_check_relations_exist(self, linter, juju_status, mocker): |
1445 | + @pytest.mark.parametrize("input_file_type", ["juju-status", "juju-bundle"]) |
1446 | + def test_check_relations_exist(self, linter, input_files, mocker, input_file_type): |
1447 | """Ensure that check_relation handle not exist error.""" |
1448 | mock_handle_error = mocker.patch("jujulint.lint.Linter.handle_error") |
1449 | + not_exist_relation = [ |
1450 | + "nrpe-host:nrpe-external-master", |
1451 | + "elasticsearch:nrpe-external-master", |
1452 | + ] |
1453 | # add a relation rule that happen in the model |
1454 | linter.lint_rules["relations"] = [ |
1455 | - {"charm": "ntp", "not-exist": [["ntp:juju-info", "ubuntu:juju-info"]]} |
1456 | + {"charm": "nrpe", "not-exist": [not_exist_relation]} |
1457 | ] |
1458 | - linter.check_relations(juju_status["applications"]) |
1459 | + linter.check_relations(input_files[input_file_type]) |
1460 | mock_handle_error.assert_called_with( |
1461 | { |
1462 | "id": "relation-exist", |
1463 | "tags": ["relation", "exist"], |
1464 | "message": "Relation(s) {} should not exist.".format( |
1465 | - ["ntp:juju-info", "ubuntu:juju-info"] |
1466 | + not_exist_relation |
1467 | + ), |
1468 | + } |
1469 | + ) |
1470 | + |
1471 | + @pytest.mark.parametrize("input_file_type", ["juju-status", "juju-bundle"]) |
1472 | + def test_check_relations_missing_machine( |
1473 | + self, linter, input_files, mocker, input_file_type |
1474 | + ): |
1475 | + """Ensure that check_relation handle missing machines when ubiquitous.""" |
1476 | + new_machines = {"3": {"series": "focal"}, "2": {"series": "bionic"}} |
1477 | + input_file = input_files[input_file_type] |
1478 | + # add two new machines |
1479 | + input_file.machines_data.update(new_machines) |
1480 | + # map file again |
1481 | + input_file.map_file() |
1482 | + mock_handle_error = mocker.patch("jujulint.lint.Linter.handle_error") |
1483 | + linter.lint_rules["relations"] = [ |
1484 | + { |
1485 | + "charm": "nrpe", |
1486 | + "ubiquitous": True, |
1487 | + } |
1488 | + ] |
1489 | + |
1490 | + expected_missing_machines = ["2", "3"] |
1491 | + linter.check_relations(input_file) |
1492 | + mock_handle_error.assert_called_with( |
1493 | + { |
1494 | + "id": "missing-machine", |
1495 | + "tags": ["missing", "machine"], |
1496 | + "message": "Charm '{}' missing on machines: {}".format( |
1497 | + "nrpe", |
1498 | + expected_missing_machines, |
1499 | ), |
1500 | } |
1501 | ) |
1502 | diff --git a/tests/unit/test_relations.py b/tests/unit/test_relations.py |
1503 | index 9ec5d72..f13d544 100644 |
1504 | --- a/tests/unit/test_relations.py |
1505 | +++ b/tests/unit/test_relations.py |
1506 | @@ -11,127 +11,191 @@ RELATIONS = [["*:nrpe-external-master", "nrpe:nrpe-external-master"]] |
1507 | |
1508 | |
1509 | @pytest.mark.parametrize( |
1510 | - "correct_relation", |
1511 | + "correct_relation, input_file_type", |
1512 | [ |
1513 | - RELATIONS, |
1514 | - [ |
1515 | - ["nrpe:nrpe-external-master", "*:nrpe-external-master"] |
1516 | - ], # inverting sequence doesn't change the endpoint |
1517 | - [ |
1518 | - ["nrpe:nrpe-external-master", "keystone:nrpe-external-master"] |
1519 | - ], # able to find specific app relation |
1520 | + (RELATIONS, "juju-status"), |
1521 | + (RELATIONS, "juju-bundle"), |
1522 | + ( |
1523 | + [["nrpe:nrpe-external-master", "*:nrpe-external-master"]], |
1524 | + "juju-status", |
1525 | + ), # inverting sequence doesn't change the endpoint |
1526 | + ( |
1527 | + [["nrpe:nrpe-external-master", "*:nrpe-external-master"]], |
1528 | + "juju-bundle", |
1529 | + ), # inverting sequence doesn't change the endpoint |
1530 | + ( |
1531 | + [["nrpe:nrpe-external-master", "keystone:nrpe-external-master"]], |
1532 | + "juju-status", |
1533 | + ), # able to find specific app relation |
1534 | + ( |
1535 | + [["nrpe:nrpe-external-master", "keystone:nrpe-external-master"]], |
1536 | + "juju-bundle", |
1537 | + ), # able to find specific app relation |
1538 | ], |
1539 | ) |
1540 | -def test_relation_rule_valid(correct_relation, juju_status_relation): |
1541 | +def test_relation_rule_valid(correct_relation, input_file_type, input_files): |
1542 | """Missing rules have empty set for endpoints with expected relations.""" |
1543 | relation_rule = relations.RelationRule( |
1544 | - charm_to_app=CHARM_TO_APP, |
1545 | - applications=juju_status_relation, |
1546 | + input_file=input_files[input_file_type], |
1547 | charm=CHARM, |
1548 | relations=correct_relation, |
1549 | not_exist=[[]], |
1550 | exception=set(), |
1551 | + ubiquitous=True, |
1552 | ) |
1553 | relation_rule.check() |
1554 | assert relation_rule.missing_relations == {"nrpe:nrpe-external-master": list()} |
1555 | assert relation_rule.not_exist_error == list() |
1556 | + assert relation_rule.missing_machines == list() |
1557 | assert relation_rule.__repr__() == "RelationRule(nrpe -> nrpe-external-master)" |
1558 | assert relation_rule.endpoint == "nrpe-external-master" |
1559 | |
1560 | |
1561 | -def test_relation_not_exist(juju_status_relation): |
1562 | +@pytest.mark.parametrize("input_file_type", ["juju-status", "juju-bundle"]) |
1563 | +def test_relation_not_exist(input_file_type, input_files): |
1564 | """Ensure that finds a relation that shouldn't happen.""" |
1565 | - juju_status_relation["foo-charm"] = { |
1566 | - "charm": "cs:foo-charm-7", |
1567 | - "charm-name": "foo-charm", |
1568 | - "relations": { |
1569 | - "foo-endpoint": ["keystone"], |
1570 | - }, |
1571 | - "endpoint-bindings": { |
1572 | - "": "oam-space", |
1573 | - "foo-endpoint": "oam-space", |
1574 | - }, |
1575 | - } |
1576 | - juju_status_relation["keystone"]["relations"]["foo-endpoint"] = ["foo-charm"] |
1577 | - juju_status_relation["keystone"]["endpoint-bindings"]["foo-endpoint"] = "oam-space" |
1578 | + wrong_relation = ["keystone:foo-endpoint", "foo-charm:foo-endpoint"] |
1579 | + input_file = input_files[input_file_type] |
1580 | + if input_file_type == "juju-status": |
1581 | + input_file.applications_data["foo-charm"] = { |
1582 | + "charm": "cs:foo-charm-7", |
1583 | + "charm-name": "foo-charm", |
1584 | + "relations": { |
1585 | + "foo-endpoint": ["keystone"], |
1586 | + }, |
1587 | + "endpoint-bindings": { |
1588 | + "": "oam-space", |
1589 | + "foo-endpoint": "oam-space", |
1590 | + }, |
1591 | + } |
1592 | + input_file.applications_data["keystone"]["relations"]["foo-endpoint"] = [ |
1593 | + "foo-charm" |
1594 | + ] |
1595 | + input_file.applications_data["keystone"]["endpoint-bindings"][ |
1596 | + "foo-endpoint" |
1597 | + ] = "oam-space" |
1598 | + |
1599 | + elif input_file_type == "juju-bundle": |
1600 | + input_file.applications_data["foo-charm"] = { |
1601 | + "charm": "cs:foo-charm-7", |
1602 | + "charm-name": "foo-charm", |
1603 | + "bindings": { |
1604 | + "": "oam-space", |
1605 | + "foo-endpoint": "oam-space", |
1606 | + }, |
1607 | + } |
1608 | + input_file.applications_data["keystone"]["bindings"][ |
1609 | + "foo-endpoint" |
1610 | + ] = "oam-space" |
1611 | + input_file.relations_data.append(wrong_relation) |
1612 | + |
1613 | relation_rule = relations.RelationRule( |
1614 | - charm_to_app=CHARM_TO_APP, |
1615 | - applications=juju_status_relation, |
1616 | + input_file=input_file, |
1617 | charm=CHARM, |
1618 | relations=RELATIONS, |
1619 | - not_exist=[["keystone:foo-endpoint", "foo-charm:foo-endpoint"]], |
1620 | + not_exist=[wrong_relation], |
1621 | exception=set(), |
1622 | + ubiquitous=True, |
1623 | ) |
1624 | relation_rule.check() |
1625 | - assert relation_rule.not_exist_error == [ |
1626 | - ["keystone:foo-endpoint", "foo-charm:foo-endpoint"] |
1627 | - ] |
1628 | + assert relation_rule.not_exist_error == [wrong_relation] |
1629 | |
1630 | |
1631 | -def test_relation_not_exist_raise(juju_status_relation): |
1632 | +@pytest.mark.parametrize("input_file_type", ["juju-status", "juju-bundle"]) |
1633 | +def test_relation_not_exist_raise(input_file_type, input_files): |
1634 | """Test that raise exception when not_exist has wrong format.""" |
1635 | - juju_status_relation["foo-charm"] = { |
1636 | - "charm": "cs:foo-charm-7", |
1637 | - "charm-name": "foo-charm", |
1638 | - "relations": { |
1639 | - "bar-endpoint": ["keystone"], |
1640 | - }, |
1641 | - "endpoint-bindings": { |
1642 | - "": "oam-space", |
1643 | - "bar-endpoint": "oam-space", |
1644 | - }, |
1645 | - } |
1646 | + input_file = input_files[input_file_type] |
1647 | + |
1648 | + if input_file_type == "juju-status": |
1649 | + input_file.applications_data["foo-charm"] = { |
1650 | + "charm": "cs:foo-charm-7", |
1651 | + "charm-name": "foo-charm", |
1652 | + "relations": { |
1653 | + "bar-endpoint": ["keystone"], |
1654 | + }, |
1655 | + "endpoint-bindings": { |
1656 | + "": "oam-space", |
1657 | + "bar-endpoint": "oam-space", |
1658 | + }, |
1659 | + } |
1660 | + |
1661 | + elif input_file_type == "juju-bundle": |
1662 | + input_file.applications_data["foo-charm"] = { |
1663 | + "charm": "cs:foo-charm-7", |
1664 | + "charm-name": "foo-charm", |
1665 | + "bindings": { |
1666 | + "": "oam-space", |
1667 | + "bar-endpoint": "oam-space", |
1668 | + }, |
1669 | + } |
1670 | |
1671 | with pytest.raises(relations.RelationError): |
1672 | relation_rule = relations.RelationRule( |
1673 | - charm_to_app=CHARM_TO_APP, |
1674 | - applications=juju_status_relation, |
1675 | + input_file=input_file, |
1676 | charm=CHARM, |
1677 | relations=RELATIONS, |
1678 | not_exist=[["keystone", "foo-charm:foo-endpoint"]], |
1679 | exception=set(), |
1680 | + ubiquitous=True, |
1681 | ) |
1682 | relation_rule.check() |
1683 | |
1684 | |
1685 | @pytest.mark.parametrize( |
1686 | - "expected_missing, exception", |
1687 | + "expected_missing, exception, input_file_type", |
1688 | [ |
1689 | - ({"nrpe:nrpe-external-master": ["foo-charm"]}, set()), |
1690 | - ({"nrpe:nrpe-external-master": list()}, {"foo-charm"}), |
1691 | + ({"nrpe:nrpe-external-master": ["foo-charm"]}, set(), "juju-status"), |
1692 | + ({"nrpe:nrpe-external-master": list()}, {"foo-charm"}, "juju-bundle"), |
1693 | ], |
1694 | ) |
1695 | def test_missing_relation_and_exception( |
1696 | - expected_missing, exception, juju_status_relation |
1697 | + expected_missing, exception, input_files, input_file_type |
1698 | ): |
1699 | - """Assert that exception is able to remove apps missing the relation.""" |
1700 | + """Assert that exception rule field is able to remove apps missing the relation.""" |
1701 | # add a charm in apps that has the endpoint nrpe-external-master, |
1702 | # but it's not relating with nrpe. |
1703 | - juju_status_relation["foo-charm"] = { |
1704 | - "charm": "cs:foo-charm-7", |
1705 | - "charm-name": "foo-charm", |
1706 | - "relations": { |
1707 | - "foo-endpoint": ["bar-charm"], |
1708 | - }, |
1709 | - "endpoint-bindings": { |
1710 | - "": "oam-space", |
1711 | - "nrpe-external-master": "oam-space", |
1712 | - }, |
1713 | - } |
1714 | + input_file = input_files[input_file_type] |
1715 | + if input_file_type == "juju-status": |
1716 | + input_file.applications_data["foo-charm"] = { |
1717 | + "charm": "cs:foo-charm-7", |
1718 | + "charm-name": "foo-charm", |
1719 | + "relations": { |
1720 | + "foo-endpoint": ["bar-charm"], |
1721 | + }, |
1722 | + "endpoint-bindings": { |
1723 | + "": "oam-space", |
1724 | + "nrpe-external-master": "oam-space", |
1725 | + }, |
1726 | + } |
1727 | + elif input_file_type == "juju-bundle": |
1728 | + input_file.applications_data["foo-charm"] = { |
1729 | + "charm": "cs:foo-charm-7", |
1730 | + "charm-name": "foo-charm", |
1731 | + "bindings": { |
1732 | + "": "oam-space", |
1733 | + "nrpe-external-master": "oam-space", |
1734 | + }, |
1735 | + } |
1736 | + input_file.relations_data.append( |
1737 | + ["foo-charm:nrpe-external-master", "nrpe-host:nrpe-external-master"] |
1738 | + ) |
1739 | + |
1740 | relation_rule = relations.RelationRule( |
1741 | - charm_to_app=CHARM_TO_APP, |
1742 | - applications=juju_status_relation, |
1743 | + input_file=input_file, |
1744 | charm=CHARM, |
1745 | relations=RELATIONS, |
1746 | not_exist=[[]], |
1747 | exception=exception, |
1748 | + ubiquitous=True, |
1749 | ) |
1750 | relation_rule.check() |
1751 | assert relation_rule.missing_relations == expected_missing |
1752 | |
1753 | |
1754 | -def test_relation_rule_unknown_charm(mocker, juju_status_relation): |
1755 | +@pytest.mark.parametrize("input_file_type", ["juju-status", "juju-bundle"]) |
1756 | +def test_relation_rule_unknown_charm(mocker, input_files, input_file_type): |
1757 | """Empty relation for a unknown charm in rules and gives warning message.""" |
1758 | + input_file = input_files[input_file_type] |
1759 | charm = "foo_charm" |
1760 | warning_msg = ( |
1761 | "Relations rules has an unexpected format. " |
1762 | @@ -139,51 +203,175 @@ def test_relation_rule_unknown_charm(mocker, juju_status_relation): |
1763 | ) |
1764 | logger_mock = mocker.patch.object(relations, "LOGGER") |
1765 | relation_rule = relations.RelationRule( |
1766 | - charm_to_app=CHARM_TO_APP, |
1767 | - applications=juju_status_relation, |
1768 | + input_file=input_file, |
1769 | charm="foo_charm", |
1770 | relations=[["*:public", "keystone:public"]], |
1771 | not_exist=[[]], |
1772 | exception=set(), |
1773 | + ubiquitous=False, |
1774 | ) |
1775 | assert relation_rule.relations == [] |
1776 | logger_mock.warning.assert_has_calls([mocker.call(warning_msg)]) |
1777 | |
1778 | |
1779 | @pytest.mark.parametrize( |
1780 | - "fake_relations, app_error, endpoint_error", |
1781 | + "fake_relations, app_error, endpoint_error, input_file_type", |
1782 | [ |
1783 | - ([["foo:juju-info", "bar:juju-info"]], True, False), # app doesn't exist |
1784 | - ([["keystone:bar", "nrpe-host:foo"]], False, True), # endpoint doesn't exist |
1785 | + ( |
1786 | + [["foo:juju-info", "bar:juju-info"]], |
1787 | + True, |
1788 | + False, |
1789 | + "juju-status", |
1790 | + ), # app doesn't exist |
1791 | + ( |
1792 | + [["foo:juju-info", "bar:juju-info"]], |
1793 | + True, |
1794 | + False, |
1795 | + "juju-bundle", |
1796 | + ), # app doesn't exist |
1797 | + ( |
1798 | + [["keystone:bar", "nrpe-host:foo"]], |
1799 | + False, |
1800 | + True, |
1801 | + "juju-status", |
1802 | + ), # endpoint doesn't exist |
1803 | + ( |
1804 | + [["keystone:bar", "nrpe-host:foo"]], |
1805 | + False, |
1806 | + True, |
1807 | + "juju-bundle", |
1808 | + ), # endpoint doesn't exist |
1809 | ], |
1810 | ) |
1811 | def test_relation_rule_unknown_app_endpoint( |
1812 | - fake_relations, app_error, endpoint_error, mocker, juju_status_relation |
1813 | + fake_relations, app_error, endpoint_error, input_files, input_file_type |
1814 | ): |
1815 | """Ensure warning message and empty relations if app or endpoint is unknown.""" |
1816 | - logger_mock = mocker.patch.object(relations, "LOGGER") |
1817 | - app, endpoint = fake_relations[0][0].split(":") |
1818 | - if app_error: |
1819 | - expected_msg = f"{app} not found on applications to check relations" |
1820 | - elif endpoint_error: |
1821 | - expected_msg = f"{app} don't have the endpoint: {endpoint} to check relations" |
1822 | + input_file = input_files[input_file_type] |
1823 | |
1824 | relations_rule = relations.RelationRule( |
1825 | - charm_to_app=CHARM_TO_APP, |
1826 | - applications=juju_status_relation, |
1827 | + input_file=input_file, |
1828 | charm=CHARM, |
1829 | relations=fake_relations, |
1830 | not_exist=[[]], |
1831 | exception=set(), |
1832 | + ubiquitous=False, |
1833 | ) |
1834 | # assert that relations is empty |
1835 | assert relations_rule.relations == [] |
1836 | - logger_mock.warning.assert_has_calls([mocker.call(expected_msg)]) |
1837 | |
1838 | |
1839 | -def test_relations_rules_bootstrap(juju_status_relation): |
1840 | +@pytest.mark.parametrize( |
1841 | + "machines, missing_machines, relations_to_check, input_file_type", |
1842 | + # adding new machines that nrpe is not relating |
1843 | + [ |
1844 | + ( |
1845 | + {"3": {"series": "focal"}, "2": {"series": "bionic"}}, |
1846 | + ["2", "3"], |
1847 | + RELATIONS, |
1848 | + "juju-status", |
1849 | + ), |
1850 | + ( |
1851 | + {"3": {"series": "focal"}, "2": {"series": "bionic"}}, |
1852 | + ["2", "3"], |
1853 | + RELATIONS, |
1854 | + "juju-bundle", |
1855 | + ), |
1856 | + # empty relations is able to run ubiquitous check |
1857 | + ( |
1858 | + {"3": {"series": "focal"}, "2": {"series": "bionic"}}, |
1859 | + ["2", "3"], |
1860 | + [[]], |
1861 | + "juju-status", |
1862 | + ), |
1863 | + ( |
1864 | + {"3": {"series": "focal"}, "2": {"series": "bionic"}}, |
1865 | + ["2", "3"], |
1866 | + [[]], |
1867 | + "juju-bundle", |
1868 | + ), |
1869 | + ( |
1870 | + { |
1871 | + "3": { |
1872 | + "series": "focal", |
1873 | + "containers": { |
1874 | + "3/lxd/0": {"series": "focal"}, |
1875 | + "3/lxd/10": {"series": "focal"}, |
1876 | + "3/lxd/1": {"series": "focal"}, |
1877 | + "3/lxd/5": {"series": "focal"}, |
1878 | + }, |
1879 | + } |
1880 | + }, |
1881 | + # result of missing machines is sorted |
1882 | + ["3", "3/lxd/0", "3/lxd/1", "3/lxd/5", "3/lxd/10"], |
1883 | + RELATIONS, |
1884 | + "juju-status", |
1885 | + ), |
1886 | + # bundles pass the machine to deploy the containers |
1887 | + ( |
1888 | + { |
1889 | + "3": { |
1890 | + "series": "focal", |
1891 | + "containers": ["lxd:0", "lxd:3"], |
1892 | + } |
1893 | + }, |
1894 | + # result of missing machines is sorted |
1895 | + ["lxd:0", "3", "lxd:3"], |
1896 | + RELATIONS, |
1897 | + "juju-bundle", |
1898 | + ), |
1899 | + ], |
1900 | +) |
1901 | +def test_ubiquitous_missing_machine( |
1902 | + input_files, machines, missing_machines, relations_to_check, input_file_type |
1903 | +): |
1904 | + """Test that find missing machines for an ubiquitous charm.""" |
1905 | + input_file = input_files[input_file_type] |
1906 | + if input_file_type == "juju-bundle": |
1907 | + for machine in machines: |
1908 | + containers = machines[machine].pop("containers", None) |
1909 | + if containers: |
1910 | + # pass new machines and containers to deploy keystone |
1911 | + input_file.applications_data["keystone"]["to"].extend(containers) |
1912 | + input_file.machines_data.update(machines) |
1913 | + # map machines again |
1914 | + input_file.map_file() |
1915 | + relation_rule = relations.RelationRule( |
1916 | + input_file=input_file, |
1917 | + charm=CHARM, |
1918 | + relations=relations_to_check, |
1919 | + not_exist=[[]], |
1920 | + exception=set(), |
1921 | + ubiquitous=True, |
1922 | + ) |
1923 | + relation_rule.check() |
1924 | + assert relation_rule.missing_machines == missing_machines |
1925 | + |
1926 | + |
1927 | +def test_relations_raise_not_implemented(input_files, mocker): |
1928 | + """Ensure that a new class that not implement mandatory methods raises error.""" |
1929 | + logger_mock = mocker.patch.object(relations, "LOGGER") |
1930 | + mocker.patch( |
1931 | + "jujulint.relations.RelationRule.relation_exist_check", |
1932 | + side_effect=NotImplementedError(), |
1933 | + ) |
1934 | + input_file = input_files["juju-status"] |
1935 | + relation_rule = relations.RelationRule( |
1936 | + input_file=input_file, |
1937 | + charm=CHARM, |
1938 | + relations=RELATIONS, |
1939 | + not_exist=[[]], |
1940 | + exception=set(), |
1941 | + ubiquitous=False, |
1942 | + ) |
1943 | + relation_rule.check() |
1944 | + logger_mock.debug.assert_called_once() |
1945 | + |
1946 | + |
1947 | +@pytest.mark.parametrize("input_file_type", ["juju-status", "juju-bundle"]) |
1948 | +def test_relations_rules_bootstrap(input_files, input_file_type): |
1949 | """Test RelationsRulesBootStrap object.""" |
1950 | - charm_to_app = {"nrpe": {"nrpe-host", "nrpe-container"}} |
1951 | + input_file = input_files[input_file_type] |
1952 | relations_rules = [ |
1953 | { |
1954 | "charm": "nrpe", |
1955 | @@ -197,9 +385,8 @@ def test_relations_rules_bootstrap(juju_status_relation): |
1956 | }, |
1957 | ] |
1958 | relations_rules = relations.RelationsRulesBootStrap( |
1959 | - charm_to_app=charm_to_app, |
1960 | relations_rules=relations_rules, |
1961 | - applications=juju_status_relation, |
1962 | + input_file=input_file, |
1963 | ).check() |
1964 | assert len(relations_rules) == 2 |
1965 | assert all( |
This merge proposal is being monitored by mergebot. Change the status to Approved to merge.