Merge pull request #691 from cderici/more-test-fixes-for-3.0-compat

#691

#### Description

This PR completes the fixes for 3.0 compatibility. With this, `pylibjuju` should be able to work with `juju 3.0`.

Includes 8 commits. The summary of the changes is:

* `ClientFacade` -> `CharmsFacade` for upgrades
* Correct facade functions for setting and unsetting application config.
* Added a call to charmhub api to check for subordinate
* Changes to use the correct facade functions with correct input/result parameters
* Small fixes for tests

#### QA Steps

All the integration tests should pass on a `juju 3.0` controller.
So bootstrap from juju's `develop` branch (or `snap install juju --channel=latest/beta`) and run the following in libjuju:

```sh
make test
```

#### Notes & Discussion

* I observe in my local runs that some tests get stuck, running them individually makes them pass, there might be intermittent issues, but this is not related to 3.0 compatibility, a further investigation about that might be done later on.
* Again, this targets the `juju-3.0-compatibility` branch. Next step on that is to make sure that the `pylibjuju` can work with both `2.9` and `3.0` facades at the same time.

Author: jujubot

juju/application.py

@@ -431,10 +431,9 @@ async def get_actions(self, schema=False):
         :return dict: The charms actions, empty dict if none are defined.
         """
         actions = {}
-        entity = [{"tag": self.tag}]
+        entity = {"tag": self.tag}
         action_facade = client.ActionFacade.from_connection(self.connection)
-        results = (
-            await action_facade.ApplicationsCharmsActions(entities=entity)).results
+        results = (await action_facade.ApplicationsCharmsActions(entities=[entity])).results
         for result in results:
             if result.application_tag == self.tag and result.actions:
                 actions = result.actions
@@ -563,7 +562,10 @@ async def set_config(self, config):
             else:
                 raise JujuApplicationConfigError(config, [k, v])
 
-        await app_facade.Set(application=self.name, options=str_config)
+        return await app_facade.SetConfigs(args=[{
+            "application": self.name,
+            "config": str_config,
+        }])
 
     async def reset_config(self, to_default):
         """
@@ -577,7 +579,10 @@ async def reset_config(self, to_default):
         log.debug(
             'Restoring default config for %s: %s', self.name, to_default)
 
-        return await app_facade.Unset(application=self.name, options=to_default)
+        return await app_facade.UnsetApplicationsConfig(args=[{
+            "application": self.name,
+            "options": to_default,
+        }])
 
     async def set_constraints(self, constraints):
         """Set machine constraints for this application.
@@ -619,7 +624,7 @@ async def refresh(
         if switch is not None and revision is not None:
             raise ValueError("switch and revision are mutually exclusive")
 
-        client_facade = client.ClientFacade.from_connection(self.connection)
+        charms_facade = client.CharmsFacade.from_connection(self.connection)
         resources_facade = client.ResourcesFacade.from_connection(
             self.connection)
         app_facade = self._facade()
@@ -644,11 +649,15 @@ async def refresh(
         if charm_url == self.data['charm-url']:
             raise JujuError('already running charm "%s"' % charm_url)
 
+        # TODO (caner) : this needs to be revisited and updated with the charmhub stuff
+        origin = client.CharmOrigin(source="charm-store",
+                                    risk=channel,
+                                    )
         # Update charm
-        await client_facade.AddCharm(
+        await charms_facade.AddCharm(
             url=charm_url,
             force=force,
-            channel=channel
+            charm_origin=origin,
         )
 
         # Update resources

juju/bundle.py

@@ -619,7 +619,7 @@ async def run(self, context):
                 self.application, charm, overrides=self.resources)
         elif Schema.CHARM_HUB.matches(url.schema):
             c_hub = charmhub.CharmHub(context.model)
-            id_ = c_hub.get_charm_id(url.name)
+            id_, _ = c_hub.get_charm_id(url.name)
             origin.id_ = id_
             resources = await context.model._add_charmhub_resources(
                 self.application, charm, origin, overrides=self.resources)

juju/charmhub.py

@@ -17,7 +17,20 @@ def get_charm_id(self, charm_name):
         if not _response.status_code == 200:
             raise JujuError("Got {} from {}".format(_response.status_code, url))
         response = json.loads(_response.text)
-        return response['id']
+        return response['id'], response['name']
+
+    def is_subordinate(self, charm_name):
+        conn, headers, path_prefix = self.model.connection().https_connection()
+
+        url = "http://api.snapcraft.io/v2/charms/info/{}?fields=default-release.revision.subordinate".format(charm_name)
+        _response = requests.get(url)
+        if not _response.status_code == 200:
+            raise JujuError("Got {} from {}".format(_response.status_code, url))
+        response = json.loads(_response.text)
+        return 'subordinate' in response['default-release']['revision']
+
+    # TODO (caner) : we should be able to recreate the channel-map through the
+    #  api call without needing the CharmHub facade
 
     async def info(self, name, channel=None):
         """info displays detailed information about a CharmHub charm. The charm

juju/controller.py

@@ -807,7 +807,7 @@ async def get_consume_details(self, endpoint):
         model to consume the specified offers represented by the urls.
         """
         facade = client.ApplicationOffersFacade.from_connection(self.connection())
-        offers = await facade.GetConsumeDetails(offer_urls=[endpoint])
+        offers = await facade.GetConsumeDetails(offer_urls=client.OfferURLs(offer_urls=[endpoint]))
         if len(offers.results) != 1:
             raise JujuAPIError("expected to find one result")
         result = offers.results[0]

juju/model.py

@@ -1636,14 +1636,7 @@ async def deploy(
                     resources = await self._add_charmhub_resources(res.app_name,
                                                                    identifier,
                                                                    add_charm_res.charm_origin)
-                    # TODO (cderici) : temporarily disable subordinate check for now
-                    # 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 self.charmhub.is_subordinate(url.name):
                         if num_units > 1:
                             raise JujuError("cannot use num_units with subordinate application")
                         num_units = 0

juju/unit.py

@@ -145,7 +145,7 @@ async def run(self, command, timeout=None):
             timeout=timeout,
             units=[self.name],
         )
-        return await self.model.wait_for_action(res.results[0].action.tag)
+        return await self.model.wait_for_action(res.actions[0].action.tag)
 
     async def run_action(self, action_name, **params):
         """Run an action on this unit.

tests/integration/test_application.py

@@ -3,6 +3,7 @@
 import pytest
 
 from .. import base
+from juju import jasyncio
 
 MB = 1
 
@@ -40,6 +41,8 @@ async def test_action(event_loop):
         constraints = await ubuntu_app.get_constraints()
         assert constraints['mem'] == 512 * MB
 
+        await jasyncio.sleep(5)
+
         # check action definitions
         actions = await ubuntu_app.get_actions()
         assert 'backup' in actions.keys()

tests/integration/test_charmhub.py

@@ -9,13 +9,13 @@
 @pytest.mark.asyncio
 async def test_info(event_loop):
     async with base.CleanModel() as model:
-        result = await model.charmhub.info("hello-juju")
-
-        assert result.result.name == "hello-juju"
+        _, name = model.charmhub.get_charm_id("hello-juju")
+        assert name == "hello-juju"
 
 
 @base.bootstrapped
 @pytest.mark.asyncio
+@pytest.mark.skip('CharmHub facade no longer exists')
 async def test_info_with_channel(event_loop):
     async with base.CleanModel() as model:
         result = await model.charmhub.info("hello-juju", "latest/stable")
@@ -26,6 +26,7 @@ async def test_info_with_channel(event_loop):
 
 @base.bootstrapped
 @pytest.mark.asyncio
+@pytest.mark.skip('CharmHub facade no longer exists')
 async def test_info_not_found(event_loop):
     async with base.CleanModel() as model:
         try:
@@ -38,6 +39,7 @@ async def test_info_not_found(event_loop):
 
 @base.bootstrapped
 @pytest.mark.asyncio
+@pytest.mark.skip('CharmHub facade no longer exists')
 async def test_find(event_loop):
     async with base.CleanModel() as model:
         result = await model.charmhub.find("kube")
@@ -50,6 +52,7 @@ async def test_find(event_loop):
 
 @base.bootstrapped
 @pytest.mark.asyncio
+@pytest.mark.skip('CharmHub facade no longer exists')
 async def test_find_bundles(event_loop):
     async with base.CleanModel() as model:
         result = await model.charmhub.find("kube", charm_type="bundle")
@@ -62,6 +65,7 @@ async def test_find_bundles(event_loop):
 
 @base.bootstrapped
 @pytest.mark.asyncio
+@pytest.mark.skip('CharmHub facade no longer exists')
 async def test_find_all(event_loop):
     async with base.CleanModel() as model:
         result = await model.charmhub.find("")

tests/integration/test_errors.py

@@ -54,7 +54,7 @@ async def test_juju_error_in_results_list(event_loop):
 @pytest.mark.asyncio
 async def test_juju_error_in_result(event_loop):
     '''
-    Verify that we raise a JujuError when appropraite when we are
+    Verify that we raise a JujuError when appropriate when we are
     looking at a single result coming back.
 
     '''
@@ -65,4 +65,4 @@ async def test_juju_error_in_result(event_loop):
         app_facade = client.ApplicationFacade.from_connection(model.connection())
 
         with pytest.raises(JujuError):
-            return await app_facade.GetCharmURL(application='foo')
+            return await app_facade.GetCharmURLOrigin(application='foo')

tests/integration/test_unit.py

@@ -81,7 +81,7 @@ async def test_run(event_loop):
         for unit in app.units:
             action = await unit.run('unit-get public-address')
             assert isinstance(action, Action)
-            assert 'Stdout' in action.results
+            assert action.status == 'completed'
             break
 
         for unit in app.units: