juju_utils: fix assumptions about 'subordinate-to'

Few places in the code made assumption that for subordinate charms,
the 'subordinate-to' list would contain only principal charms. However
that is not the case. If a suboridnate charm is related to other
subordinate charms, they show up in each other's 'subordinate-to' lists.

This was especially problematic in the 'get_machines_for_application'
function which would end up in an infinite loop.

Signed-off-by: Martin Kalcok <martin.kalcok@canonical.com>

Author: mkalcok

unit_tests/test_zaza_model.py

@@ -245,25 +245,52 @@ def fail_on_use():
         self.machine_data = {self.key: self.key_data}
         self.unit = "app/1"
         self.application = "app"
+
         self.subordinate_application = "subordinate_application"
         self.subordinate_application_data = {
             "subordinate-to": [self.application],
             "units": None}
         self.subordinate_unit = "subordinate_application/1"
         self.subordinate_unit_data = {
             "workload-status": {"status": "active"}}
+
+        # Second subordinate app is related both to the principal application
+        # and to the first subordinate application
+        self.second_subordinate_application = "second_subordinate_application"
+        self.second_subordinate_application_data = {
+            "subordinate-to": [
+                self.subordinate_application,
+                self.application
+            ],
+            "units": None}
+        self.second_subordinate_unit = "second_subordinate_application/1"
+        self.second_subordinate_unit_data = {
+            "workload-status": {"status": "active"}}
+        # Update the suboridnate list on the first subordinate as well
+        self.subordinate_application_data["subordinate-to"].insert(
+            0,
+            self.second_subordinate_application,
+        )
+
         self.unit_data = {
             "workload-status": {"status": "active"},
             "machine": self.machine,
             "subordinates": {
-                self.subordinate_unit: self.subordinate_unit_data}}
+                self.subordinate_unit: self.subordinate_unit_data,
+                self.second_subordinate_unit:
+                    self.second_subordinate_unit_data,
+            }
+        }
         self.application_data = {"units": {
             self.unit1.name: self.subordinate_unit_data,
             self.unit: self.unit_data}}
         self.juju_status = mock.MagicMock()
         self.juju_status.applications = {
             self.application: self.application_data,
-            self.subordinate_application: self.subordinate_application_data}
+            self.subordinate_application: self.subordinate_application_data,
+            self.second_subordinate_application:
+                self.second_subordinate_application_data,
+        }
         self.juju_status.machines = self.machine_data
 
         async def _connect_model(model_name):

unit_tests/utilities/test_zaza_utilities_juju.py

@@ -78,18 +78,39 @@ def fail_on_use():
         self.unit2_mock.data = {'machine-id': self.machine2}
 
         self.application = "app"
+
         self.subordinate_application = "subordinate_application"
         self.subordinate_application_unit = "subordinate_application/0"
         self.subordinate_application_data = {
             "subordinate-to": [self.application]}
+
+        # Second subordinate app is related both to the principal application
+        # and to the first subordinate application
+        self.second_subordinate_application = "second_subordinate"
+        self.second_subordinate_application_unit = "second_subordinate/0"
+        self.second_subordinate_application_data = {
+            "subordinate-to": [self.subordinate_application, self.application]}
+        # Update the suboridnate list on the first subordinate as well
+        self.subordinate_application_data['subordinate-to'].insert(
+            0,
+            self.second_subordinate_application
+        )
+
         self.application_data = {
             "units": {self.unit1: self.unit1_data},
-            "subordinates": {self.subordinate_application_unit: {}}}
+            "subordinates": {
+                self.subordinate_application_unit: {},
+                self.second_subordinate_application_unit: {},
+            }
+        }
         self.juju_status = mock.MagicMock()
         self.juju_status.name = "juju_status_object"
         self.juju_status.applications = {
             self.application: self.application_data,
-            self.subordinate_application: self.subordinate_application_data}
+            self.subordinate_application: self.subordinate_application_data,
+            self.second_subordinate_application:
+                self.second_subordinate_application_data,
+        }
         self.juju_status.machines = {
             self.machine0: self.machine0_mock,
             self.machine1: self.machine1_mock,

zaza/model.py

@@ -40,6 +40,7 @@
 
 from zaza import sync_wrapper
 import zaza.utilities.generic as generic_utils
+import zaza.utilities.juju as juju_utils
 import zaza.utilities.exceptions as zaza_exceptions
 from zaza.utilities import deprecate
 
@@ -2570,8 +2571,14 @@ async def _unit_status():
             # unit
             lead_app_name = subordinate_principal
             if not subordinate_principal:
-                lead_app_name = model_status.applications[app][
-                    'subordinate-to'][0]
+                for app_ in model_status.applications[app]['subordinate-to']:
+                    app_status = model_status.applications[app_]
+                    if not juju_utils.is_subordinate_application(app_,
+                                                                 app_status,
+                                                                 model_name):
+                        lead_app_name = app_
+                        break
+
             units = model_status.applications[lead_app_name]['units']
             for unit in units.values():
                 try:
@@ -2582,7 +2589,8 @@ async def _unit_status():
                     pass
             else:  # pragma: no cover
                 raise ValueError('{} does not exist as a subordinate under a '
-                                 'principal'.format(unit_name))
+                                 'principal to {}'.format(unit_name,
+                                                          lead_app_name))
         if negate_match:
             return v != status
         else:

zaza/utilities/juju.py

@@ -122,7 +122,12 @@ def get_principle_applications(application_name, application_status=None,
     status = application_status or get_application_status(
         application_name,
         model_name=model_name)
-    return status.get("subordinate-to")
+    # A subodrinate application can be related to other
+    # subordinate applications, such that they both show up in
+    # each other's 'subordinate-to' list. We need to filter them out
+    # from the list that we are returning.
+    return [app for app in status.get("subordinate-to")
+            if not is_subordinate_application(app, model_name=model_name)]
 
 
 def get_machines_for_application(application, model_name=None):
@@ -142,8 +147,15 @@ def get_machines_for_application(application, model_name=None):
     # libjuju juju status no longer has units for subordinate charms
     # Use the application it is subordinate-to to find machines
     if is_subordinate_application(application, model_name=model_name):
+        principal = get_principle_applications(application,
+                                               model_name=model_name)
+        if not principal:
+            logging.warn(f"Suboridnate application {application} is not"
+                         " related to any principal application.")
+            return
+
         yield from get_machines_for_application(
-            status.get("subordinate-to")[0],
+            principal[0],
             model_name=model_name)
     else:
         for unit in status.get("units").keys():