Merge ~elmo/juju-lint:fixes into juju-lint:master

[James Troup]

Fix config checking, promote hw-health to mandatory + more

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

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

[James Hebden (ec0)]

Looks good to me, +1

review: Approve

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

Change successfully merged at revision 39fd901463f08610b01945c6eebd3ab37d265ca0

diff --git a/contrib/canonical-rules.yaml b/contrib/canonical-rules.yaml
index 9555a15..0961671 100644
--- a/contrib/canonical-rules.yaml
+++ b/contrib/canonical-rules.yaml
@@ -81,6 +81,10 @@ subordinates:
   where: host only # You don't want NTP in a container duelling with ntp in the host
  thruk-agent:
   where: on nagios
+ hw-health:
+  where: host only
+ logrotate:
+  where: all
 
 operations mandatory: &operations-mandatory-charms
  - elasticsearch
@@ -92,7 +96,6 @@ operations mandatory: &operations-mandatory-charms
 
 operations optional: &operations-optional-charms
  - infra-node
- - hw-health
  - cloudstats
 
 operations openstack mandatory: &operations-openstack-mandatory-charms
@@ -115,6 +118,8 @@ operations subordinates: &operations-mandatory-subs
  - ntp
  - telegraf
  - thruk-agent
+ - hw-health
+ - logrotated
 
 operations optional subordinates: &operations-optional-subs
  - policy-routing
@@ -196,6 +201,12 @@ masakari-charms: &masakari-charms
  - masakari-monitors
  - pacemaker-remote
 
+trilio-charms: &trilio-charms
+ - trilio-dm-api
+ - trilio-horizon-plugin
+ - trilio-data-mover
+ - trilio-wlm
+
 openstack charms: &openstack-charms
  - *openstack-mandatory-charms
  - *openstack-mandatory-deps
@@ -204,6 +215,7 @@ openstack charms: &openstack-charms
  - *cisco-aci-charms
  - *ovn-charms
  - *masakari-charms
+ - *trilio-charms
 
 known charms:
  - ubuntu
diff --git a/jujulint/lint.py b/jujulint/lint.py
index 26f2c81..70f5ae6 100755
--- a/jujulint/lint.py
+++ b/jujulint/lint.py
@@ -28,7 +28,7 @@ import yaml
 from attr import attrs, attrib
 import attr
 
-from jujulint.util import flatten_list, is_container
+from jujulint.util import flatten_list, is_container, extract_charm_name
 from jujulint.logging import Logger
 
 # TODO:
@@ -37,12 +37,6 @@ from jujulint.logging import Logger
 #  - info mode, e.g. num of machines, version (e.g. look at ceph), architecture
 
 
-class InvalidCharmNameError(Exception):
-    """Represents an invalid charm name being processed."""
-
-    pass
-
-
 @attrs
 class ModelInfo(object):
     """Represent information obtained from juju status data."""
@@ -177,7 +171,7 @@ class Linter:
             current = self.atoi(config[rule])
             expected = self.atoi(check_value)
             if current >= expected:
-                self.logger.info(
+                self.logger.debug(
                     "[{}] [{}/{}] (PASS) Application {} has config for {} which is >= {}: {}.".format(
                         self.cloud_name,
                         self.controller_name,
@@ -217,7 +211,7 @@ class Linter:
         """Check if value is set per rule constraints."""
         if rule in config:
             if check_value is True:
-                self.logger.info(
+                self.logger.debug(
                     "[{}] [{}/{}] (PASS) Application {} correctly has manual config for {}: {}.".format(
                         self.cloud_name,
                         self.controller_name,
@@ -240,7 +234,7 @@ class Linter:
             )
             return False
         if check_value is False:
-            self.logger.info(
+            self.logger.debug(
                 "[{}] [{}/{}] (PASS) Application {} is correctly using default config for {}.".format(
                     self.cloud_name, self.controller_name, self.model_name, name, rule,
                 )
@@ -262,7 +256,7 @@ class Linter:
             except re.error:
                 match = check_value == config[rule]
             if match:
-                self.logger.info(
+                self.logger.debug(
                     "[{}] [{}/{}] Application {} has correct setting for {}: Expected {}, got {}.".format(
                         self.cloud_name,
                         self.controller_name,
@@ -285,6 +279,18 @@ class Linter:
                     config[rule],
                 )
             )
+            return False
+        else:
+            self.logger.warn(
+                "[{}] [{}/{}] When checking if application {} has no config for {}, can't determine if == {}.".format(
+                    self.cloud_name,
+                    self.controller_name,
+                    self.model_name,
+                    name,
+                    rule,
+                    check_value,
+                )
+            )
 
     def check_config(self, name, config, rules):
         """Check application against provided rules."""
@@ -319,28 +325,38 @@ class Linter:
         for application in applications.keys():
             # look for config rules for this application
             lint_rules = []
-            if "charm-name" in applications[application]:
-                charm_name = applications[application]["charm-name"]
-                if "config" in self.lint_rules:
-                    if charm_name in self.lint_rules["config"]:
-                        lint_rules = self.lint_rules["config"][charm_name].items()
-
-                if self.cloud_type == "openstack":
-                    # process openstack config rules
-                    if "openstack config" in self.lint_rules:
-                        if charm_name in self.lint_rules["openstack config"]:
-                            lint_rules.extend(
-                                self.lint_rules["openstack config"][charm_name].items()
-                            )
+            if "charm" not in applications[application]:
+                self.logger.warn(
+                    "[{}] [{}/{}] Application {} has no charm.".format(
+                        self.cloud_name,
+                        self.controller_name,
+                        self.model_name,
+                        application,
+                    )
+                )
+                continue
 
-                if lint_rules:
-                    if "options" in applications[application]:
-                        self.check_config(
-                            application,
-                            applications[application]["options"],
-                            lint_rules,
+            charm_name = extract_charm_name(applications[application]["charm"])
+            if "config" in self.lint_rules:
+                if charm_name in self.lint_rules["config"]:
+                    lint_rules = self.lint_rules["config"][charm_name].items()
+
+            if self.cloud_type == "openstack":
+                # process openstack config rules
+                if "openstack config" in self.lint_rules:
+                    if charm_name in self.lint_rules["openstack config"]:
+                        lint_rules.extend(
+                            self.lint_rules["openstack config"][charm_name].items()
                         )
 
+            if lint_rules:
+                if "options" in applications[application]:
+                    self.check_config(
+                        application,
+                        applications[application]["options"],
+                        lint_rules,
+                    )
+
     def check_subs(self):
         """Check the subordinates in the model."""
         all_or_nothing = set()
@@ -675,17 +691,9 @@ class Linter:
         """Process applications in the model, validating and normalising the names."""
         for app in applications:
             if "charm" in applications[app]:
-                charm = applications[app]["charm"]
-                match = re.match(
-                    r"^(?:\w+:)?(?:~[\w-]+/)?(?:\w+/)?([a-zA-Z0-9-]+?)(?:-\d+)?$", charm
-                )
-                if not match:
-                    raise InvalidCharmNameError(
-                        "charm name '{}' is invalid".format(charm)
-                    )
-                charm = match.group(1)
-                self.model.charms.add(charm)
-                self.model.app_to_charm[app] = charm
+                charm_name = extract_charm_name(applications[app]["charm"])
+                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(
@@ -895,23 +903,24 @@ class Linter:
             self.check_subs()
             self.check_charms()
 
-            if parsed_yaml.get("machines"):
+            if "relations" not in parsed_yaml:
                 self.map_machines_to_az(parsed_yaml["machines"])
                 self.check_azs(parsed_yaml[applications])
                 self.check_statuses(parsed_yaml, applications)
             else:
                 self.logger.warn(
                     (
-                        "[{}] [{}/{}] No machine status present in model."
-                        "possibly a bundle without status, skipping AZ checks"
+                        "[{}] [{}/{}] Relations data found; assuming a bundle and "
+                        "skipping AZ and status checks."
                     ).format(
                         self.cloud_name, self.model_name, self.controller_name,
                     )
                 )
 
             self.results()
-        self.logger.warn(
-            "[{}] [{}/{}] Model contains no applications, skipping.".format(
-                self.cloud_name, self.controller_name, self.model_name,
+        else:
+            self.logger.warn(
+                "[{}] [{}/{}] Model contains no applications, skipping.".format(
+                    self.cloud_name, self.controller_name, self.model_name,
+                )
             )
-        )
diff --git a/jujulint/util.py b/jujulint/util.py
index 2b7582a..f376640 100644
--- a/jujulint/util.py
+++ b/jujulint/util.py
@@ -18,6 +18,14 @@
 # along with this program. If not, see <http://www.gnu.org/licenses/>.
 """Utility library for all helpful functions this project uses."""
 
+import re
+
+
+class InvalidCharmNameError(Exception):
+    """Represents an invalid charm name being processed."""
+
+    pass
+
 
 def flatten_list(lumpy_list):
     """Flatten a list potentially containing other lists."""
@@ -36,3 +44,14 @@ def is_container(machine):
         return True
     else:
         return False
+
+
+def extract_charm_name(charm):
+    match = re.match(
+        r"^(?:\w+:)?(?:~[\w-]+/)?(?:\w+/)?([a-zA-Z0-9-]+?)(?:-\d+)?$", charm
+    )
+    if not match:
+        raise InvalidCharmNameError(
+            "charm name '{}' is invalid".format(charm)
+        )
+    return match.group(1)