Merge pull request #648 from cderici/subordinate-charm-num-unit

#648

#### Description

Subordinate charms are by definition don't require additional units. So num-units should be 0 at deploy.

Normally in libjuju num_units defaults to 1, this change lets libjuju to check the metadata from charmhub to see if we're deploying a subordinate charm. If that's the case then we adjust the num-units to 0 to be able to deploy it without any problems from Juju's side.

Fixes #639 

#### QA Steps

```sh
tox -e integration -- tests/integration/test_charmhub.py::test_subordinate_charm_zero_units
```

#### Notes & Discussion

Author: jujubot

juju/model.py

@@ -1622,6 +1622,16 @@ async def deploy(
                     resources = await self._add_charmhub_resources(res.app_name,
                                                                    identifier,
                                                                    add_charm_res.charm_origin)
+                    charm_info = await self.charmhub.info(url.name)
+                    is_subordinate = False
+                    try:
+                        is_subordinate = charm_info.result.charm.subordinate
+                    except AttributeError:
+                        log.warning('CharmHub.Info : unable to retrieve the subordinate information')
+                    if is_subordinate:
+                        if num_units > 1:
+                            raise JujuError("cannot use num_units with subordinate application")
+                        num_units = 0
 
                 if Schema.CHARM_STORE.matches(url.schema):
                     resources = await self._add_store_resources(res.app_name,

tests/integration/test_charmhub.py

@@ -1,7 +1,8 @@
 import pytest
 
 from .. import base
-from juju.errors import JujuAPIError
+from juju.errors import JujuAPIError, JujuError
+from juju import jasyncio
 
 
 @base.bootstrapped
@@ -69,3 +70,30 @@ async def test_find_all(event_loop):
         for resp in result.result:
             assert resp.name != ""
             assert resp.type_ in ["charm", "bundle"]
+
+
+@base.bootstrapped
+@pytest.mark.asyncio
+async def test_subordinate_charm_zero_units(event_loop):
+    # normally in pylibjuju deploy num_units defaults to 1, we switch
+    # that to 0 behind the scenes if we see that the charmhub charm
+    # we're deploying is a subordinate charm
+    async with base.CleanModel() as model:
+
+        # rsyslog-forwarder-ha is a subordinate charm
+        app = await model.deploy('rsyslog-forwarder-ha')
+        await jasyncio.sleep(5)
+
+        assert len(app.units) == 0
+        await app.destroy()
+        await jasyncio.sleep(5)
+
+        # note that it'll error if the user tries to use num_units
+        with pytest.raises(JujuError):
+            await model.deploy('rsyslog-forwarder-ha', num_units=175)
+
+        # (full disclosure: it'll quitely switch to 0 if user enters
+        # num_units=1, instead of erroring)
+        app2 = await model.deploy('rsyslog-forwarder-ha', num_units=1)
+        await jasyncio.sleep(5)
+        assert len(app2.units) == 0

tests/integration/test_controller.py

@@ -149,7 +149,7 @@ async def test_list_models_user_access(event_loop):
         # testing all flag
         await user.grant(acl='superuser')
         models_all = await controller.list_models(username, all=True)
-        assert models1 == models_all
+        assert len(models_all) > len(models2)
 
 
 @base.bootstrapped