Merge ~jfguedez/juju-lint:feature/add-json into juju-lint:master

[Jose Guedez]

Adds unit tests for the linter, as well as json output for the linting errors.

Adds unit tests for the linter, as well as json output for the linting errors:

    {
      "description": "Checks for mandatory Ops subordinates",
      "id": "ops-subordinate-missing",
      "message": "Subordinate 'hw-health' is missing for application(s): 'ubuntu'",
      "principals": "ubuntu",
      "subordinate": "hw-health",
      "tags": [
        "missing",
        "ops",
        "charm",
        "mandatory",
        "subordinate"
      ]
    },

collected 20 items

tests/test_cli.py::test_pytest PASSED [ 5%]
tests/test_cli.py::test_cli_fixture PASSED [ 10%]
tests/test_jujulint.py::test_flatten_list PASSED [ 15%]
tests/test_jujulint.py::test_map_charms PASSED [ 20%]
tests/test_jujulint.py::TestLinter::test_minimal_rules PASSED [ 25%]
tests/test_jujulint.py::TestLinter::test_charm_identification PASSED [ 30%]
tests/test_jujulint.py::TestLinter::test_juju_status_unexpected PASSED [ 35%]
tests/test_jujulint.py::TestLinter::test_AZ_parsing PASSED [ 40%]
tests/test_jujulint.py::TestLinter::test_AZ_balancing PASSED [ 45%]
tests/test_jujulint.py::TestLinter::test_ops_charm_missing PASSED [ 50%]
tests/test_jujulint.py::TestLinter::test_unrecognised_charm PASSED [ 55%]
tests/test_jujulint.py::TestLinter::test_ops_subordinate_missing PASSED [ 60%]
tests/test_jujulint.py::TestLinter::test_subordinate_extraneous PASSED [ 65%]
tests/test_jujulint.py::TestLinter::test_subordinate_duplicates PASSED [ 70%]
tests/test_jujulint.py::TestLinter::test_openstack_ops_charm_missing PASSED [ 75%]
tests/test_jujulint.py::TestLinter::test_openstack_charm_missing PASSED [ 80%]
tests/test_jujulint.py::TestLinter::test_config_eq PASSED [ 85%]
tests/test_jujulint.py::TestLinter::test_config_gte PASSED [ 90%]
tests/test_jujulint.py::TestLinter::test_config_isset_false PASSED [ 95%]
tests/test_jujulint.py::TestLinter::test_config_isset_true PASSED [100%]

===================================================================== 20 passed in 0.20s =====================================================================

[🤖 Canonical IS Merge Bot (canonical-is-mergebot)]

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

[🤖 Canonical IS Merge Bot (canonical-is-mergebot)]

Unable to determine commit message from repository - please click "Set commit message" and enter the commit message manually.

[Linda Guo (lihuiguo)]

more unit tests were added. LGTM

review: Approve

[🤖 Canonical IS Merge Bot (canonical-is-mergebot)]

Change has no commit message, setting status to needs review.

[🤖 Canonical IS Merge Bot (canonical-is-mergebot)]

Change successfully merged at revision 76ca869ab423f56240cb3c24f885a8c2eb153bcb

diff --git a/jujulint/cli.py b/jujulint/cli.py
index 8892420..aafe9a5 100755
--- a/jujulint/cli.py
+++ b/jujulint/cli.py
@@ -21,6 +21,7 @@ from jujulint.config import Config
 from jujulint.lint import Linter
 from jujulint.logging import Logger
 from jujulint.openstack import OpenStack
+import logging
 import os.path
 import pkg_resources
 import yaml
@@ -36,6 +37,11 @@ class Cli:
         """Create new CLI and configure runtime environment."""
         self.config = Config()
         self.logger = Logger(self.config["logging"]["loglevel"].get())
+        self.output_format = self.config["format"].get()
+
+        # disable logging for non-text output formats
+        if self.output_format != "text":
+            logging.disable(level=logging.CRITICAL)
 
         # get the version of the current package if available
         # this will fail if not running from an installed package
@@ -79,7 +85,12 @@ class Cli:
     def audit_file(self, filename, cloud_type=None):
         """Directly audit a YAML file."""
         self.logger.debug("Starting audit of file {}".format(filename))
-        linter = Linter(filename, self.lint_rules, cloud_type=cloud_type,)
+        linter = Linter(
+            filename,
+            self.lint_rules,
+            cloud_type=cloud_type,
+            output_format=self.output_format,
+        )
         linter.read_rules()
         self.logger.info("[{}] Linting manual file...".format(filename))
         linter.lint_yaml_file(filename)
diff --git a/jujulint/config.py b/jujulint/config.py
index 05b7883..1173187 100644
--- a/jujulint/config.py
+++ b/jujulint/config.py
@@ -96,6 +96,13 @@ class Config(Configuration):
             help="File to log to in addition to stdout",
             dest="logging.file",
         )
+        self.parser.add_argument(
+            "--format",
+            "-F",
+            choices=["text", "json"],
+            default="text",
+            help="Format for output",
+        )
 
         args = self.parser.parse_args()
         self.set_args(args, dots=True)
diff --git a/jujulint/lint.py b/jujulint/lint.py
index ce49398..4e2f70a 100755
--- a/jujulint/lint.py
+++ b/jujulint/lint.py
@@ -19,6 +19,7 @@
 # along with this program. If not, see <http://www.gnu.org/licenses/>.
 """Lint operations and rule processing engine."""
 import collections
+import json
 import os.path
 import pprint
 import re
@@ -65,6 +66,7 @@ class Linter:
         model_name="manual",
         overrides=None,
         cloud_type=None,
+        output_format="text",
     ):
         """Instantiate linter."""
         self.logger = Logger()
@@ -77,6 +79,19 @@ class Linter:
         self.controller_name = controller_name
         self.model_name = model_name
 
+        # output
+        self.output_format = output_format
+        self.output_collector = {
+            "name": name,
+            "controller": controller_name,
+            "model": model_name,
+            "rules": filename,
+            "errors": [],
+        }
+
+        # collect errors only for non-text output (e.g. json)
+        self.collect_errors = True if self.output_format != "text" else False
+
     def read_rules(self):
         """Read and parse rules from YAML, optionally processing provided overrides."""
         if os.path.isfile(self.filename):
@@ -138,6 +153,7 @@ class Linter:
                 if sub in self.model.subs_on_machines[machine]:
                     self.model.duelling_subs.setdefault(sub, set())
                     self.model.duelling_subs[sub].add(machine)
+                self.model.subs_on_machines[machine].add(sub)
             self.model.subs_on_machines[machine] = (
                 set(subordinates) | self.model.subs_on_machines[machine]
             )
@@ -146,7 +162,7 @@ class Linter:
 
         return
 
-    def atoi(val):
+    def atoi(self, val):
         """Deal with complex number representations as strings, returning a number."""
         if type(val) != str:
             return val
@@ -185,16 +201,21 @@ class Linter:
                     )
                 )
                 return True
-            self.logger.error(
-                "[{}] [{}/{}] (FAIL) Application {} has config for {} which is less than {}: {}.".format(
-                    self.cloud_name,
-                    self.controller_name,
-                    self.model_name,
-                    name,
-                    rule,
-                    check_value,
-                    config[rule],
-                )
+
+            actual_value = config[rule]
+            self.handle_error(
+                {
+                    "id": "config-gte-check",
+                    "tags": ["config", "gte"],
+                    "description": "Checks for config condition 'gte'",
+                    "application": name,
+                    "rule": rule,
+                    "expected_value": check_value,
+                    "actual_value": actual_value,
+                    "message": "Application {} has config for {} which is less than {}: {}.".format(
+                        name, rule, check_value, actual_value
+                    ),
+                }
             )
             return False
         self.logger.warn(
@@ -224,15 +245,19 @@ class Linter:
                     )
                 )
                 return True
-            self.logger.error(
-                "[{}] [{}/{}] (FAIL) Application {} has manual config for {}: {}.".format(
-                    self.cloud_name,
-                    self.controller_name,
-                    self.model_name,
-                    name,
-                    rule,
-                    config[rule],
-                )
+            actual_value = config[rule]
+            self.handle_error(
+                {
+                    "id": "config-isset-check-false",
+                    "tags": ["config", "isset"],
+                    "description": "Checks for config condition 'isset'",
+                    "application": name,
+                    "rule": rule,
+                    "actual_value": actual_value,
+                    "message": "Application {} has manual config for {}: {}.".format(
+                        name, rule, actual_value
+                    ),
+                }
             )
             return False
         if check_value is False:
@@ -246,14 +271,17 @@ class Linter:
                 )
             )
             return True
-        self.logger.error(
-            "[{}] [{}/{}] (FAIL) Application {} has no manual config for {}.".format(
-                self.cloud_name,
-                self.controller_name,
-                self.model_name,
-                name,
-                rule,
-            )
+        self.handle_error(
+            {
+                "id": "config-isset-check-true",
+                "tags": ["config", "isset"],
+                "description": "Checks for config condition 'isset' true",
+                "application": name,
+                "rule": rule,
+                "message": "Application {} has no manual config for {}.".format(
+                    name, rule
+                ),
+            }
         )
         return False
 
@@ -278,16 +306,20 @@ class Linter:
                     )
                 )
                 return True
-            self.logger.error(
-                "[{}] [{}/{}] Application {} has incorrect setting for {}: Expected {}, got {}.".format(
-                    self.cloud_name,
-                    self.controller_name,
-                    self.model_name,
-                    name,
-                    rule,
-                    check_value,
-                    config[rule],
-                )
+            actual_value = config[rule]
+            self.handle_error(
+                {
+                    "id": "config-eq-check",
+                    "tags": ["config", "eq"],
+                    "description": "Checks for config condition 'eq'",
+                    "application": name,
+                    "rule": rule,
+                    "expected_value": check_value,
+                    "actual_value": actual_value,
+                    "message": "Application {} has incorrect setting for {}: Expected {}, got {}.".format(
+                        name, rule, check_value, actual_value
+                    ),
+                }
             )
             return False
         else:
@@ -317,7 +349,7 @@ class Linter:
                 elif check_op == "eq":
                     self.eq(name, check_value, rule, config)
                 elif check_op == "gte":
-                    self.eq(name, check_value, rule, config)
+                    self.gte(name, check_value, rule, config)
                 else:
                     self.logger.warn(
                         "[{}] [{}/{}] Application {} has unknown check operation for {}: {}.".format(
@@ -331,7 +363,7 @@ class Linter:
                     )
 
     def check_configuration(self, applications):
-        """Check applicaton configs in the model."""
+        """Check application configs in the model."""
         for application in applications.keys():
             # look for config rules for this application
             lint_rules = []
@@ -610,113 +642,154 @@ class Linter:
         """Check we recognise the charms which are in the model."""
         for charm in self.model.charms:
             if charm not in self.lint_rules["known charms"]:
-                self.logger.error(
-                    "[{}] Charm '{}' in model {} on controller {} not recognised".format(
-                        self.cloud_name, charm, self.model_name, self.controller_name
-                    )
+                self.handle_error(
+                    {
+                        "id": "unrecognised-charm",
+                        "tags": ["charm", "unrecognised"],
+                        "description": "An unrecognised charm is present in the model",
+                        "charm": charm,
+                        "message": "Charm '{}' not recognised".format(charm),
+                    }
                 )
         # Then look for charms we require
         for charm in self.lint_rules["operations mandatory"]:
             if charm not in self.model.charms:
-                self.logger.error(
-                    "[{}] Ops charm '{}' in model {} on controller {} not found".format(
-                        self.cloud_name, charm, self.model_name, self.controller_name
-                    )
+                self.handle_error(
+                    {
+                        "id": "ops-charm-missing",
+                        "tags": ["missing", "ops", "charm", "mandatory", "principal"],
+                        "description": "An Ops charm is missing",
+                        "charm": charm,
+                        "message": "Ops charm '{}' is missing".format(charm),
+                    }
                 )
         if self.cloud_type == "openstack":
             for charm in self.lint_rules["openstack mandatory"]:
                 if charm not in self.model.charms:
-                    self.logger.error(
-                        "[{}] OpenStack charm '{}' in model {} on controller {} not found".format(
-                            self.cloud_name,
-                            charm,
-                            self.model_name,
-                            self.controller_name,
-                        )
+                    self.handle_error(
+                        {
+                            "id": "openstack-charm-missing",
+                            "tags": [
+                                "missing",
+                                "openstack",
+                                "charm",
+                                "mandatory",
+                                "principal",
+                            ],
+                            "description": "An Openstack charm is missing",
+                            "charm": charm,
+                            "message": "Openstack charm '{}' is missing".format(charm),
+                        }
                     )
             for charm in self.lint_rules["operations openstack mandatory"]:
                 if charm not in self.model.charms:
-                    self.logger.error(
-                        "[{}] Ops charm '{}' in OpenStack model {} on controller {} not found".format(
-                            self.cloud_name,
-                            charm,
-                            self.model_name,
-                            self.controller_name,
-                        )
+                    self.handle_error(
+                        {
+                            "id": "openstack-ops-charm-missing",
+                            "tags": [
+                                "missing",
+                                "openstack",
+                                "ops",
+                                "charm",
+                                "mandatory",
+                                "principal",
+                            ],
+                            "description": "An Openstack ops charm is missing",
+                            "charm": charm,
+                            "message": "Openstack ops charm '{}' is missing".format(
+                                charm
+                            ),
+                        }
                     )
         elif self.cloud_type == "kubernetes":
             for charm in self.lint_rules["kubernetes mandatory"]:
                 if charm not in self.model.charms:
-                    self.logger.error(
-                        "[{}] [{}/{}] Kubernetes charm '{}' not found".format(
-                            self.cloud_name,
-                            self.controller_name,
-                            self.model_name,
-                            charm,
-                        )
+                    self.handle_error(
+                        {
+                            "id": "kubernetes-charm-missing",
+                            "tags": [
+                                "missing",
+                                "kubernetes",
+                                "charm",
+                                "mandatory",
+                                "principal",
+                            ],
+                            "description": "An Kubernetes charm is missing",
+                            "charm": charm,
+                            "message": "Kubernetes charm '{}' is missing".format(charm),
+                        }
                     )
 
     def results(self):
         """Provide results of the linting process."""
         if self.model.missing_subs:
-            self.logger.error("The following subordinates couldn't be found:")
             for sub in self.model.missing_subs:
-                self.logger.error(
-                    "[{}] [{}/{}] -> {} [{}]".format(
-                        self.cloud_name,
-                        self.controller_name,
-                        self.model_name,
-                        sub,
-                        ", ".join(sorted(self.model.missing_subs[sub])),
-                    )
+                principals = ", ".join(sorted(self.model.missing_subs[sub]))
+                self.handle_error(
+                    {
+                        "id": "ops-subordinate-missing",
+                        "tags": ["missing", "ops", "charm", "mandatory", "subordinate"],
+                        "description": "Checks for mandatory Ops subordinates",
+                        "principals": principals,
+                        "subordinate": sub,
+                        "message": "Subordinate '{}' is missing for application(s): '{}'".format(
+                            sub, principals
+                        ),
+                    }
                 )
+
         if self.model.extraneous_subs:
-            self.logger.error("following subordinates where found unexpectedly:")
             for sub in self.model.extraneous_subs:
-                self.logger.error(
-                    "[{}] [{}/{}] -> {} [{}]".format(
-                        self.cloud_name,
-                        self.controller_name,
-                        self.model_name,
-                        sub,
-                        ", ".join(sorted(self.model.extraneous_subs[sub])),
-                    )
+                principals = ", ".join(sorted(self.model.extraneous_subs[sub]))
+                self.handle_error(
+                    {
+                        "id": "subordinate-extraneous",
+                        "tags": ["extraneous", "charm", "subordinate"],
+                        "description": "Checks for extraneous subordinates in containers",
+                        "principals": principals,
+                        "subordinate": sub,
+                        "message": "Application(s) '{}' has extraneous subordinate '{}'".format(
+                            principals, sub
+                        ),
+                    }
                 )
         if self.model.duelling_subs:
-            self.logger.error(
-                "[{}] [{}/{}] following subordinates where found on machines more than once:".format(
-                    self.cloud_name,
-                    self.controller_name,
-                    self.model_name,
-                )
-            )
             for sub in self.model.duelling_subs:
-                self.logger.error(
-                    "[{}] [{}/{}] -> {} [{}]".format(
-                        self.cloud_name,
-                        self.controller_name,
-                        self.model_name,
-                        sub,
-                        ", ".join(sorted(self.model.duelling_subs[sub])),
-                    )
+                machines = ", ".join(sorted(self.model.duelling_subs[sub]))
+                self.handle_error(
+                    {
+                        "id": "subordinate-duplicate",
+                        "tags": ["duplicate", "charm", "subordinate"],
+                        "description": "Checks for duplicate subordinates in a machine",
+                        "machines": machines,
+                        "subordinate": sub,
+                        "message": "Subordinate '{}' is duplicated on machines: '{}'".format(
+                            sub,
+                            machines,
+                        ),
+                    }
                 )
         if self.model.az_unbalanced_apps:
-            self.logger.error("The following apps are unbalanced across AZs: ")
             for app in self.model.az_unbalanced_apps:
                 (num_units, az_counter) = self.model.az_unbalanced_apps[app]
                 az_map = ", ".join(
-                    ["{}: {}".format(az, az_counter[az]) for az in az_counter]
+                    ["{}: {}".format(az, az_counter[az]) for az in sorted(az_counter)]
                 )
-                self.logger.error(
-                    "[{}] [{}/{}] -> {}: {} units, deployed as: {}".format(
-                        self.cloud_name,
-                        self.controller_name,
-                        self.model_name,
-                        app,
-                        num_units,
-                        az_map,
-                    )
+                self.handle_error(
+                    {
+                        "id": "AZ-unbalance",
+                        "tags": ["AZ"],
+                        "description": "Checks for application balance across AZs",
+                        "application": app,
+                        "num_units": num_units,
+                        "az_map": az_map,
+                        "message": "Application '{}' is unbalanced across AZs: {} units, deployed as: {}".format(
+                            app, num_units, az_map
+                        ),
+                    }
                 )
+        if self.output_format == "json":
+            print(json.dumps(self.output_collector, indent=2, sort_keys=True))
 
     def map_charms(self, applications):
         """Process applications in the model, validating and normalising the names."""
@@ -726,10 +799,16 @@ class Linter:
                 self.model.charms.add(charm_name)
                 self.model.app_to_charm[app] = charm_name
             else:
-                self.logger.error(
-                    "[{}] [{}/{}] Could not detect which charm is used for application {}".format(
-                        self.cloud_name, self.controller_name, self.model_name, app
-                    )
+                self.handle_error(
+                    {
+                        "id": "charm-not-mapped",
+                        "tags": ["charm", "mapped", "parsing"],
+                        "description": "Detect the charm used by an application",
+                        "application": app,
+                        "message": "Could not detect which charm is used for application {}".format(
+                            app
+                        ),
+                    }
                 )
 
     def map_machines_to_az(self, machines):
@@ -760,20 +839,25 @@ class Linter:
 
     def check_status(self, what, status, expected):
         """Lint the status of a unit."""
+        current_status = status.get("current")
         if isinstance(expected, str):
             expected = [expected]
-        if status.get("current") not in expected:
-            self.logger.error(
-                "[{}] [{}/{}] {} has status '{}' (since: {}, message: {}); (We expected: {})".format(
-                    self.cloud_name,
-                    self.controller_name,
-                    self.model_name,
-                    what,
-                    status.get("current"),
-                    status.get("since"),
-                    status.get("message"),
-                    expected,
-                )
+        if current_status not in expected:
+            status_since = status.get("since")
+            status_msg = status.get("message")
+            self.handle_error(
+                {
+                    "id": "status-unexpected",
+                    "tags": ["status"],
+                    "description": "Checks for unexpected status in juju and workload",
+                    "what": what,
+                    "status_current": current_status,
+                    "status_since": status_since,
+                    "status_msg": status_msg,
+                    "message": "{} has status '{}' (since: {}, message: {}); (We expected: {})".format(
+                        what, current_status, status_since, status_msg, expected
+                    ),
+                }
             )
 
     def check_status_pair(self, name, status_type, data_d):
@@ -866,10 +950,16 @@ class Linter:
             azs.add(self.model.machines_to_az[machine])
         num_azs = len(azs)
         if num_azs != 3:
-            self.logger.error(
-                "[{}] [{}/{}] Found {} AZs (not 3); and I don't currently know how to lint that.".format(
-                    self.cloud_name, self.controller_name, self.model_name, num_azs
-                )
+            self.handle_error(
+                {
+                    "id": "AZ-invalid-number",
+                    "tags": ["AZ"],
+                    "description": "Checks for a valid number or AZs (currently 3)",
+                    "num_azs": num_azs,
+                    "message": "Invalid number of AZs: '{}', expecting 3".format(
+                        num_azs
+                    ),
+                }
             )
             return
 
@@ -962,3 +1052,18 @@ class Linter:
                     self.model_name,
                 )
             )
+
+    def collect(self, error):
+        """Collect an error and add it to the collector."""
+        self.output_collector["errors"].append(error)
+
+    def handle_error(self, error):
+        """Collect an error and add it to the collector."""
+
+        self.logger.error(
+            "[{}] [{}/{}] {}.".format(
+                self.cloud_name, self.controller_name, self.model_name, error["message"]
+            )
+        )
+        if self.collect_errors:
+            self.collect(error)
diff --git a/tests/conftest.py b/tests/conftest.py
index ef72390..ba9ed77 100644
--- a/tests/conftest.py
+++ b/tests/conftest.py
@@ -29,9 +29,12 @@ def mocked_pkg_resources(monkeypatch):
 @pytest.fixture
 def cli(monkeypatch):
     """Provide a test instance of the CLI class."""
-    monkeypatch.setattr(sys, "argv", ["juju-lint", "-c", "contrib/canonical-rules.yaml"])
+    monkeypatch.setattr(
+        sys, "argv", ["juju-lint", "-c", "contrib/canonical-rules.yaml"]
+    )
 
     from jujulint.cli import Cli
+
     cli = Cli()
 
     return cli
@@ -48,14 +51,70 @@ def utils():
 @pytest.fixture
 def parser(monkeypatch):
     """Mock the configuration parser."""
-    monkeypatch.setattr('jujulint.config.ArgumentParser', mock.Mock())
+    monkeypatch.setattr("jujulint.config.ArgumentParser", mock.Mock())
 
 
 @pytest.fixture
-def lint(parser):
+def linter(parser):
     """Provide test fixture for the linter class."""
     from jujulint.lint import Linter
 
-    linter = Linter('mockcloud', 'mockrules.yaml')
+    rules = {
+        "known charms": ["ntp", "ubuntu"],
+        "operations mandatory": ["ubuntu"],
+        "subordinates": {
+            "ntp": {"where": "all"},
+        },
+    }
+
+    linter = Linter("mockcloud", "mockrules.yaml")
+    linter.lint_rules = rules
+    linter.collect_errors = True
 
     return linter
+
+
[e-mail]
+def juju_status():
+    """Provide a base juju status for testing."""
+    return {
+        "applications": {
+            "ubuntu": {
+                "application-status": {"current": "active"},
+                "charm": "cs:ubuntu-18",
+                "charm-name": "ubuntu",
+                "relations": {"juju-info": ["ntp"]},
+                "units": {
+                    "ubuntu/0": {
+                        "juju-status": {"current": "idle"},
+                        "machine": "0",
+                        "subordinates": {
+                            "ntp/0": {
+                                "juju-status": {"current": "idle"},
+                                "workload-status": {"current": "active"},
+                            }
+                        },
+                        "workload-status": {"current": "active"},
+                    }
+                },
+            },
+        },
+        "machines": {
+            "0": {
+                "hardware": "availability-zone=rack-1",
+                "juju-status": {"current": "started"},
+                "machine-status": {"current": "running"},
+                "modification-status": {"current": "applied"},
+            },
+            "1": {
+                "hardware": "availability-zone=rack-2",
+                "juju-status": {"current": "started"},
+                "machine-status": {"current": "running"},
+            },
+            "2": {
+                "hardware": "availability-zone=rack-3",
+                "juju-status": {"current": "started"},
+                "machine-status": {"current": "running"},
+            },
+        },
+    }
diff --git a/tests/test_jujulint.py b/tests/test_jujulint.py
index 4c27cc1..7dacb7d 100644
--- a/tests/test_jujulint.py
+++ b/tests/test_jujulint.py
@@ -16,7 +16,7 @@ def test_flatten_list(utils):
     assert flattened_list == utils.flatten_list(unflattened_list)
 
 
-def test_map_charms(lint, utils):
+def test_map_charms(linter, utils):
     """Test the charm name validation code."""
     applications = {
         "test-app-1": {"charm": "cs:~USER/SERIES/TEST-CHARM12-123"},
@@ -26,11 +26,282 @@ def test_map_charms(lint, utils):
         "test-app-5": {"charm": "local:TEST-CHARM12"},
         "test-app-6": {"charm": "cs:~TEST-CHARMERS/TEST-CHARM12-123"},
     }
-    lint.map_charms(applications)
-    for charm in lint.model.charms:
+    linter.map_charms(applications)
+    for charm in linter.model.charms:
         assert "TEST-CHARM12" == charm
     applications = {
         "test-app1": {"charm": "cs:invalid-charm$"},
     }
     with pytest.raises(utils.InvalidCharmNameError):
-        lint.map_charms(applications)
+        linter.map_charms(applications)
+
+
+class TestLinter:
+    def test_minimal_rules(self, linter, juju_status):
+        """Test that the base rules/status have no errors."""
+        linter.do_lint(juju_status)
+        assert len(linter.output_collector["errors"]) == 0
+
+    def test_charm_identification(self, linter, juju_status):
+        """Test that applications are mapped to charms."""
+        juju_status["applications"]["ubuntu2"] = {
+            "application-status": {"current": "active"},
+            "charm-name": "ubuntu",
+            "relations": {"juju-info": ["ntp"]},
+            "units": {
+                "ubuntu2/0": {
+                    "juju-status": {"current": "idle"},
+                    "machine": "1",
+                    "subordinates": {
+                        "ntp/1": {
+                            "juju-status": {"current": "idle"},
+                            "workload-status": {"current": "error"},
+                        }
+                    },
+                    "workload-status": {"current": "active"},
+                }
+            },
+        }
+        linter.do_lint(juju_status)
+
+        errors = linter.output_collector["errors"]
+        assert len(errors) == 1
+        assert errors[0]["id"] == "charm-not-mapped"
+        assert errors[0]["application"] == "ubuntu2"
+
+    def test_juju_status_unexpected(self, linter, juju_status):
+        """Test that juju and workload status is expected."""
+        # inject invalid status to the application
+        juju_status["applications"]["ubuntu"]["units"]["ubuntu/0"][
+            "workload-status"
+        ].update(
+            {
+                "current": "error",
+                "since": "01 Apr 2021 05:14:13Z",
+                "message": 'hook failed: "install"',
+            }
+        )
+        linter.do_lint(juju_status)
+
+        errors = linter.output_collector["errors"]
+        assert len(errors) == 1
+        assert errors[0]["id"] == "status-unexpected"
+        assert errors[0]["status_current"] == "error"
+        assert errors[0]["status_since"] == "01 Apr 2021 05:14:13Z"
+        assert errors[0]["status_msg"] == 'hook failed: "install"'
+
+    def test_AZ_parsing(self, linter, juju_status):
+        """Test that the AZ parsing is working as expected."""
+        # duplicate a AZ name so we have 2 AZs instead of the expected 3
+        juju_status["machines"]["2"]["hardware"] = "availability-zone=rack-1"
+        linter.do_lint(juju_status)
+
+        errors = linter.output_collector["errors"]
+        assert len(errors) == 1
+        assert errors[0]["id"] == "AZ-invalid-number"
+        assert errors[0]["num_azs"] == 2
+
+    def test_AZ_balancing(self, linter, juju_status):
+        """Test that applications are balanced across AZs."""
+        # add an extra machine in an existing AZ
+        juju_status["machines"].update(
+            {
+                "3": {
+                    "hardware": "availability-zone=rack-3",
+                    "juju-status": {"current": "started"},
+                    "machine-status": {"current": "running"},
+                    "modification-status": {"current": "applied"},
+                }
+            }
+        )
+        # add two more ubuntu units, but unbalanced (ubuntu/0 is in rack-1)
+        juju_status["applications"]["ubuntu"]["units"].update(
+            {
+                "ubuntu/1": {
+                    "juju-status": {"current": "idle"},
+                    "machine": "2",
+                    "subordinates": {
+                        "ntp/1": {
+                            "juju-status": {"current": "idle"},
+                            "workload-status": {"current": "error"},
+                        }
+                    },
+                    "workload-status": {"current": "active"},
+                },
+                "ubuntu/2": {
+                    "juju-status": {"current": "idle"},
+                    "machine": "3",
+                    "subordinates": {
+                        "ntp/2": {
+                            "juju-status": {"current": "idle"},
+                            "workload-status": {"current": "error"},
+                        }
+                    },
+                    "workload-status": {"current": "active"},
+                },
+            }
+        )
+        linter.do_lint(juju_status)
+
+        errors = linter.output_collector["errors"]
+        assert len(errors) == 1
+        assert errors[0]["id"] == "AZ-unbalance"
+        assert errors[0]["application"] == "ubuntu"
+        assert errors[0]["num_units"] == 3
+        assert errors[0]["az_map"] == "rack-1: 1, rack-2: 0, rack-3: 2"
+
+    def test_ops_charm_missing(self, linter, juju_status):
+        """Test that missing ops mandatory charms are detected."""
+        # add a new mandatory ops charm
+        linter.lint_rules["operations mandatory"].append("telegraf")
+        linter.do_lint(juju_status)
+
+        errors = linter.output_collector["errors"]
+        assert len(errors) == 1
+        assert errors[0]["id"] == "ops-charm-missing"
+        assert errors[0]["charm"] == "telegraf"
+
+    def test_unrecognised_charm(self, linter, juju_status):
+        """Test that unrecognised charms are detected."""
+        # drop 'ubuntu' from the known charms
+        linter.lint_rules["known charms"] = ["ntp"]
+        linter.do_lint(juju_status)
+
+        errors = linter.output_collector["errors"]
+        assert len(errors) == 1
+        assert errors[0]["id"] == "unrecognised-charm"
+        assert errors[0]["charm"] == "ubuntu"
+
+    def test_ops_subordinate_missing(self, linter, juju_status):
+        """Test that missing ops subordinate charms are detected."""
+        # drop the subordinate unit
+        juju_status["applications"]["ubuntu"]["units"]["ubuntu/0"]["subordinates"] = {}
+        linter.do_lint(juju_status)
+
+        errors = linter.output_collector["errors"]
+        assert len(errors) == 1
+        assert errors[0]["id"] == "ops-subordinate-missing"
+        assert errors[0]["principals"] == "ubuntu"
+        assert errors[0]["subordinate"] == "ntp"
+
+    def test_subordinate_extraneous(self, linter, juju_status):
+        """Test that extraneous subordinate charms are detected."""
+        # this check triggers on subordinates on containers that should only
+        # be present in hosts
+        linter.lint_rules["subordinates"]["ntp"]["where"] = "host only"
+        juju_status["applications"]["ubuntu"]["units"]["ubuntu/0"][
+            "machine"
+        ] = "0/lxd/0"
+        linter.do_lint(juju_status)
+
+        errors = linter.output_collector["errors"]
+        assert len(errors) == 1
+        assert errors[0]["id"] == "subordinate-extraneous"
+        assert errors[0]["principals"] == "ubuntu"
+        assert errors[0]["subordinate"] == "ntp"
+
+    def test_subordinate_duplicates(self, linter, juju_status):
+        """Test that subordinate charms are not duplicated."""
+        ntp0 = juju_status["applications"]["ubuntu"]["units"]["ubuntu/0"][
+            "subordinates"
+        ]["ntp/0"]
+        juju_status["applications"]["ubuntu"]["units"]["ubuntu/0"]["subordinates"][
+            "ntp/1"
+        ] = ntp0
+        linter.do_lint(juju_status)
+
+        errors = linter.output_collector["errors"]
+        assert len(errors) == 1
+        assert errors[0]["id"] == "subordinate-duplicate"
+        assert errors[0]["machines"] == "0"
+        assert errors[0]["subordinate"] == "ntp"
+
+    def test_openstack_charm_missing(self, linter, juju_status):
+        """Test that missing openstack mandatory charms are detected."""
+        linter.cloud_type = "openstack"
+        linter.lint_rules["openstack mandatory"] = ["keystone"]
+        linter.lint_rules["operations openstack mandatory"] = ["ubuntu"]
+        linter.do_lint(juju_status)
+
+        errors = linter.output_collector["errors"]
+        assert len(errors) == 1
+        assert errors[0]["id"] == "openstack-charm-missing"
+        assert errors[0]["charm"] == "keystone"
+
+    def test_openstack_ops_charm_missing(self, linter, juju_status):
+        """Test that missing openstack mandatory ops charms are detected."""
+        linter.cloud_type = "openstack"
+        linter.lint_rules["openstack mandatory"] = ["ubuntu"]
+        linter.lint_rules["operations openstack mandatory"] = [
+            "openstack-service-checks"
+        ]
+        linter.do_lint(juju_status)
+
+        errors = linter.output_collector["errors"]
+        assert len(errors) == 1
+        assert errors[0]["id"] == "openstack-ops-charm-missing"
+        assert errors[0]["charm"] == "openstack-service-checks"
+
+    def test_openstack_charm_missing(self, linter, juju_status):
+        """Test that missing kubernetes mandatory charms are detected."""
+        linter.cloud_type = "kubernetes"
+        linter.lint_rules["kubernetes mandatory"] = ["kubernetes-master"]
+        linter.do_lint(juju_status)
+
+        errors = linter.output_collector["errors"]
+        assert len(errors) == 1
+        assert errors[0]["id"] == "kubernetes-charm-missing"
+        assert errors[0]["charm"] == "kubernetes-master"
+
+    def test_config_eq(self, linter, juju_status):
+        """Test the config condition 'eq'."""
+        linter.lint_rules["config"] = {"ubuntu": {"fake-opt": {"eq": False}}}
+        juju_status["applications"]["ubuntu"]["options"] = {"fake-opt": True}
+        linter.do_lint(juju_status)
+
+        errors = linter.output_collector["errors"]
+        assert len(errors) == 1
+        assert errors[0]["id"] == "config-eq-check"
+        assert errors[0]["application"] == "ubuntu"
+        assert errors[0]["rule"] == "fake-opt"
+        assert errors[0]["expected_value"] == False
+        assert errors[0]["actual_value"] == True
+
+    def test_config_gte(self, linter, juju_status):
+        """Test the config condition 'gte'."""
+        linter.lint_rules["config"] = {"ubuntu": {"fake-opt": {"gte": 3}}}
+        juju_status["applications"]["ubuntu"]["options"] = {"fake-opt": 0}
+        linter.do_lint(juju_status)
+
+        errors = linter.output_collector["errors"]
+        assert len(errors) == 1
+        assert errors[0]["id"] == "config-gte-check"
+        assert errors[0]["application"] == "ubuntu"
+        assert errors[0]["rule"] == "fake-opt"
+        assert errors[0]["expected_value"] == 3
+        assert errors[0]["actual_value"] == 0
+
+    def test_config_isset_false(self, linter, juju_status):
+        """Test the config condition 'isset' false."""
+        linter.lint_rules["config"] = {"ubuntu": {"fake-opt": {"isset": False}}}
+        juju_status["applications"]["ubuntu"]["options"] = {"fake-opt": 0}
+        linter.do_lint(juju_status)
+
+        errors = linter.output_collector["errors"]
+        assert len(errors) == 1
+        assert errors[0]["id"] == "config-isset-check-false"
+        assert errors[0]["application"] == "ubuntu"
+        assert errors[0]["rule"] == "fake-opt"
+        assert errors[0]["actual_value"] == 0
+
+    def test_config_isset_true(self, linter, juju_status):
+        """Test the config condition 'isset' true."""
+        linter.lint_rules["config"] = {"ubuntu": {"fake-opt": {"isset": True}}}
+        juju_status["applications"]["ubuntu"]["options"] = {}
+        linter.do_lint(juju_status)
+
+        errors = linter.output_collector["errors"]
+        assert len(errors) == 1
+        assert errors[0]["id"] == "config-isset-check-true"
+        assert errors[0]["application"] == "ubuntu"
+        assert errors[0]["rule"] == "fake-opt"