Merge pull request #879 from cderici/utilities-for-subordinate-units

#879

#### Description

This is in response to an [LP bug](https://bugs.launchpad.net/juju/+bug/1987332), which is about getting the subordinate units using libjuju. [The way that `zaza` was doing it](https://github.com/openstack-charmers/zaza/blob/b2f2b46e7903cbd601917c7e4e40d2e00ba4d03d/zaza/utilities/juju.py#LL449C57-L449C57) is incorrect, in that it uses the `Charm` field in the subordinate units to get the charm_url. This is an internal field and shouldn't be used externally. Furthermore the naming of this field is somewhat misleading, as [it's really the `upgrading-from` field](https://github.com/juju/juju/blob/63900364b3f56c273ed3842f77b40b710b2c87a4/cmd/juju/status/formatted.go#LL230C101-L230C101) that's being set only during an upgrade -to indicate the previous charm url prior to the upgrade- and it is to be deleted/unset right after the upgrade is done. This is why getting the charm-url from this field is not correct. 

This change adds some useful utilities (some fields and some methods) to interact with the subordinate units, in particular, we're adding:

- `ModelState.subordinate_units` and `model.subordinate_units`
- `Application.subordinate_units`
- `Unit.is_subordinate`, `Unit.principal_unit` and `Unit.get_subordinates()`

#### QA Steps

This also adds the test `tests/integration/test_unit.py::test_subordinate_units`, so the following should be working:

```
tox -e integration -- tests/integration/test_unit.py::test_subordinate_units
```

#### Notes & Discussion

Marked to be forward-ported into the main branch.

Author: jujubot

juju/application.py

@@ -64,6 +64,11 @@ def units(self):
             if unit.application == self.name
         ]
 
+    @property
+    def subordinate_units(self):
+        """Returns the subordinate units of this application"""
+        return [u for u in self.units if u.is_subordinate]
+
     @property
     def relations(self):
         return [rel for rel in self.model.relations if rel.matches(self.name)]

juju/model.py

@@ -171,6 +171,11 @@ def units(self):
         """
         return self._live_entity_map('unit')
 
+    @property
+    def subordinate_units(self):
+        """Return a map of unit-id:Unit for all subordinate units"""
+        return {u_name: u for u_name, u in self.units.items() if u.is_subordinate}
+
     @property
     def relations(self):
         """Return a map of relation-id:Relation for all relations currently in
@@ -969,6 +974,14 @@ def units(self):
         """
         return self.state.units
 
+    @property
+    def subordinate_units(self):
+        """Return a map of unit-id:Unit for all subordiante units currently in
+        the model.
+
+        """
+        return self.state.subordinate_units
+
     @property
     def relations(self):
         """Return a list of all Relations currently in the model.

juju/unit.py

@@ -26,6 +26,20 @@ def agent_status_since(self):
         """
         return pyrfc3339.parse(self.safe_data['agent-status']['since'])
 
+    @property
+    def is_subordinate(self):
+        """True if the unit is subordinate of another unit
+
+        """
+        return self.safe_data['subordinate']
+
+    @property
+    def principal_unit(self):
+        """Returns the name of the unit of which this unit is a subordinate to.
+        Returns '' for principal units themselves.
+        """
+        return self.safe_data['principal']
+
     @property
     def agent_status_message(self):
         """Get the agent status message.
@@ -77,6 +91,14 @@ def public_address(self):
     def tag(self):
         return tag.unit(self.name)
 
+    def get_subordinates(self):
+        """Returns the unit objects that are subordinates to this unit
+
+        :return [Unit]
+        """
+        return [u for u_name, u in self.model.units.items() if u.is_subordinate and
+                u.principal_unit == self.name]
+
     async def destroy(self):
         """Destroy this unit.
 

tests/integration/test_unit.py

@@ -214,3 +214,33 @@ async def test_resolve_local(event_loop):
             # Errored units won't get cleaned up unless we force them.
             await asyncio.gather(*(machine.destroy(force=True)
                                    for machine in model.machines.values()))
+
+
+@base.bootstrapped
+@pytest.mark.asyncio
+async def test_subordinate_units(event_loop):
+    async with base.CleanModel() as model:
+        u_app = await model.deploy('ubuntu')
+        n_app = await model.deploy('ntp')
+        await model.relate('ubuntu', 'ntp')
+        await model.wait_for_idle()
+
+        # model subordinates
+        model_subs = model.subordinate_units
+        assert len(model_subs) == 1
+        assert 'ntp/0' in model_subs
+        assert 'ubuntu/0' not in model_subs
+
+        n_unit = model_subs['ntp/0']
+        u_unit = u_app.units[0]
+
+        # application subordinates
+        app_sub_names = [u.name for u in n_app.subordinate_units]
+        assert n_unit.name in app_sub_names
+        assert u_unit.name not in app_sub_names
+
+        assert n_unit.is_subordinate
+        assert not u_unit.is_subordinate
+        assert n_unit.principal_unit == 'ubuntu/0'
+        assert u_unit.principal_unit == ''
+        assert [u.name for u in u_unit.get_subordinates()] == [n_unit.name]