Merge ~jfguedez/juju-lint:bug/1855858 into juju-lint:master

[Jose Guedez]

Add the "allow-multiple" condition for subordinates

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

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

[Edin S (exsdev)]

review: Approve

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

Change successfully merged at revision 3b85de9d81bf29c7769febd0a60c7a41fc424a34

diff --git a/contrib/includes/base.yaml b/contrib/includes/base.yaml
index da558a3..bc39a46 100644
--- a/contrib/includes/base.yaml
+++ b/contrib/includes/base.yaml
@@ -29,6 +29,7 @@ subordinates:
   host-suffixes: [host, physical, guest]
   container-suffixes: [lxd, container]
   exceptions: [nagios]
+  allow-multiple: true  # There can be multiple nrpe units, see LP#1855858
  ntp:
   # You don't want NTP in a container dueling with ntp in the host
   where: host only
diff --git a/jujulint/lint.py b/jujulint/lint.py
index f680370..58ce1f1 100755
--- a/jujulint/lint.py
+++ b/jujulint/lint.py
@@ -150,8 +150,6 @@ class Linter:
         if "units" not in app_d:
             return
         for unit in app_d["units"]:
-            # juju_status = app_d["units"][unit]["juju-status"]
-            # workload_status = app_d["units"][unit]["workload-status"]
             if "subordinates" in app_d["units"][unit]:
                 subordinates = app_d["units"][unit]["subordinates"].keys()
                 subordinates = [i.split("/")[0] for i in subordinates]
@@ -162,8 +160,11 @@ class Linter:
             self.model.subs_on_machines.setdefault(machine, set())
             for sub in subordinates:
                 if sub in self.model.subs_on_machines[machine]:
-                    self.model.duelling_subs.setdefault(sub, set())
-                    self.model.duelling_subs[sub].add(machine)
+                    charm = self.model.app_to_charm[sub]
+                    allow_multiple = self.lint_rules['subordinates'][charm].get("allow-multiple")
+                    if not allow_multiple:
+                        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]
diff --git a/tests/test_jujulint.py b/tests/test_jujulint.py
index 2494c28..c74c550 100644
--- a/tests/test_jujulint.py
+++ b/tests/test_jujulint.py
@@ -280,6 +280,61 @@ class TestLinter:
         assert errors[0]["machines"] == "0"
         assert errors[0]["subordinate"] == "ntp"
 
+    def test_subordinate_duplicates_allow(self, linter, juju_status):
+        """
+        Test the subordinate option "allow-multiple".
+
+        Setting it to true will skip the duplicate check for subordinates
+        """
+        template_status = {
+            "juju-status": {"current": "idle"},
+            "workload-status": {"current": "active"},
+        }
+
+        # Drop the fixture ntp rule and add the nrpe rule with allowing duplicates
+        linter.lint_rules["subordinates"].pop("ntp")
+        linter.lint_rules["subordinates"]["nrpe"] = {
+            "where": "container aware",
+            "host-suffixes": "[host, physical, guest]",
+            "allow-multiple": True,
+        }
+
+        # Add a nrpe-host subordinate application
+        linter.lint_rules["known charms"].append("nrpe")
+        juju_status["applications"]["nrpe-host"] = {
+            "application-status": {"current": "active"},
+            "charm": "cs:nrpe-74",
+            "charm-name": "nrpe",
+            "relations": {"juju-info": ["ubuntu", "ubuntu2"]},
+        }
+
+        # Add a nrpe-host subordinate unit to the 'ubuntu' app
+        juju_status["applications"]["ubuntu"]["units"]["ubuntu/0"]["subordinates"] = {
+            "nrpe-host/0": template_status
+        }
+
+        # Add a second 'ubuntu' app with nrpe subordinate
+        juju_status["applications"]["ubuntu2"] = {
+            "application-status": {"current": "active"},
+            "charm": "cs:ubuntu-18",
+            "charm-name": "ubuntu",
+            "relations": {"juju-info": ["ntp", "nrpe-host"]},
+            "units": {
+                "ubuntu2/0": {
+                    "juju-status": {"current": "idle"},
+                    "machine": "0",
+                    "subordinates": {"nrpe-host/1": template_status},
+                    "workload-status": {"current": "active"},
+                }
+            },
+        }
+
+        linter.do_lint(juju_status)
+
+        # Since we allow duplicates there should be no errors
+        errors = linter.output_collector["errors"]
+        assert not errors
+
     def test_ops_subordinate_metal_only1(self, linter, juju_status):
         """
         Test that missing ops subordinate charms are detected.
@@ -305,7 +360,9 @@ class TestLinter:
         linter.lint_rules["subordinates"]["hw-health"] = {"where": "metal only"}
 
         # Turn machine "0" into a "VM"
-        juju_status["machines"]["0"]["hardware"] = "tags=virtual availability-zone=rack-1"
+        juju_status["machines"]["0"][
+            "hardware"
+        ] = "tags=virtual availability-zone=rack-1"
         linter.do_lint(juju_status)
 
         errors = linter.output_collector["errors"]