Merge pull request #910 from cderici/forward-ports-from-2.9-to-3.x

#910

#### Description

This brings forward from `2.9` the following PRs: #901, #905, and #896 into `3.x` (the main branch).

#### QA Steps

There were a couple of conflicts that needed to be fixed, so QA will be necessary, make sure the following tests are passing:

```
tox -e integration -- tests/integration/test_application.py::test_app_charm_name
```
```
tox -e py3 -- -m wait_for_idle
```

```
tox -e integration -- -m wait_for_idle
```
```sh
tox -e integration -- tests/integration/test_model.py::test_deploy_bundle_local_charm_series_manifest
```

Author: jujubot

juju/application.py

@@ -567,11 +567,19 @@ async def run(self, command, timeout=None):
             units=[],
         )
 
+    @property
+    def charm_name(self):
+        """Get the charm name of this application
+
+        :return str: The name of the charm
+        """
+        return URL.parse(self.charm_url).name
+
     @property
     def charm_url(self):
-        """Get the charm url for a given application
+        """Get the charm url for this application
 
-        :return string: The charm url for an application
+        :return str: The charm url
         """
         return self.safe_data['charm-url']
 

juju/bundle.py

@@ -129,7 +129,10 @@ async def _handle_local_charms(self, bundle, bundle_dir):
             if not series:
                 metadata = utils.get_local_charm_metadata(charm_dir)
                 series = await get_charm_series(metadata, self.model)
-            if not self.model.connection().is_using_old_client and not series:
+            if not series:
+                base = utils.get_local_charm_base(None, charm_path, client.Base)
+                series = utils.base_channel_to_series(base.channel)
+            if not series:
                 raise JujuError(
                     "Couldn't determine series for charm at {}. "
                     "Add a 'series' key to the bundle.".format(charm_dir))

juju/model.py

@@ -2709,12 +2709,24 @@ def _raise_for_status(entities, status):
                     if raise_on_blocked and unit.workload_status == "blocked":
                         blocks.setdefault("Unit", []).append(unit.name)
                         continue
-                    waiting_for_a_particular_status = status and unit.workload_status != status
-                    if not waiting_for_a_particular_status and unit.agent_status == "idle":
-                        # We'll be here in two cases:
-                        # 1) We're not waiting for a particular status and the agent is "idle"
-                        # 2) We're waiting for a particular status and the workload is in that
-                        # status
+                    # TODO (cderici): we need two versions of wait_for_idle, one for waiting on
+                    #  individual units, another one for waiting for an application.
+                    #  The convoluted logic below is the result of trying to do both at the same
+                    #  time
+                    need_to_wait_more_for_a_particular_status = status and (unit.workload_status != status)
+                    app_is_in_desired_status = (not status) or (app_status == status)
+                    if not need_to_wait_more_for_a_particular_status and \
+                            unit.agent_status == "idle" and \
+                            (wait_for_at_least_units or app_is_in_desired_status):
+                        # A unit is ready if either:
+                        # 1) Don't need to wait more for a particular status and the agent is "idle"
+                        # 2) We're looking for a particular status and the unit's workload,
+                        # as well as the application, is in that status. If the user wants to
+                        # see only a particular number of units in that state -- i.e. a subset of
+                        # the units is needed, then we don't care about the application status
+                        # (because e.g. app can be in 'waiting' while unit.0 is 'active' and unit.1
+                        # is 'waiting')
+
                         # Either way, the unit is ready, start measuring the time period that
                         # it needs to stay in that state (i.e. idle_period)
                         units_ready.add(unit.name)

tests/integration/bundle/local-manifest.yaml

@@ -0,0 +1,4 @@
+applications:
+  test1:
+    charm: "../charm-manifest"
+    num_units: 1

tests/integration/charm-manifest/config.yaml

@@ -0,0 +1,4 @@
+options:
+  status:
+    type: string
+    default: "active"

tests/integration/charm-manifest/dispatch

@@ -0,0 +1,13 @@
+#!/bin/bash
+
+status="$(config-get status)"
+
+if [[ "$status" == "error" ]]; then
+    if [[ -e .errored ]]; then
+        status="active"
+    else
+        touch .errored
+        exit 1
+    fi
+fi
+status-set "$status"

tests/integration/charm-manifest/manifest.yaml

@@ -0,0 +1,13 @@
+analysis:
+  attributes:
+  - name: language
+    result: python
+  - name: framework
+    result: operator
+bases:
+- architectures:
+  - amd64
+  channel: '20.04'
+  name: ubuntu
+charmcraft-started-at: '2021-08-20T08:09:00.639806Z'
+charmcraft-version: 1.2.1

tests/integration/charm-manifest/metadata.yaml

@@ -0,0 +1,4 @@
+name: charm
+summary: "test"
+description: "test"
+maintainers: ["test"]

tests/integration/test_application.py

@@ -297,3 +297,13 @@ async def test_app_remove_wait_flag(event_loop):
 
         await model.remove_application(app.name, block_until_done=True)
         assert a_name not in model.applications
+
+
+@base.bootstrapped
+@pytest.mark.asyncio
+async def test_app_charm_name(event_loop):
+    async with base.CleanModel() as model:
+        app = await model.deploy('ubuntu')
+        await model.wait_for_idle(status="active")
+        assert 'ubuntu' in app.charm_url
+        assert 'ubuntu' == app.charm_name

tests/integration/test_model.py

@@ -178,6 +178,20 @@ async def test_deploy_bundle_local_charms(event_loop):
         assert model.units['test2/0'].workload_status == 'active'
 
 
+@base.bootstrapped
+@pytest.mark.asyncio
+@pytest.mark.bundle
+async def test_deploy_bundle_local_charm_series_manifest(event_loop):
+    bundle_path = INTEGRATION_TEST_DIR / 'bundle' / 'local-manifest.yaml'
+
+    async with base.CleanModel() as model:
+        await model.deploy(bundle_path)
+        await wait_for_bundle(model, bundle_path)
+        assert set(model.units.keys()) == set(['test1/0'])
+        assert model.units['test1/0'].agent_status == 'idle'
+        assert model.units['test1/0'].workload_status == 'active'
+
+
 @base.bootstrapped
 @pytest.mark.asyncio
 @pytest.mark.bundle
@@ -848,12 +862,12 @@ async def test_get_machines(event_loop):
 
 @base.bootstrapped
 @pytest.mark.asyncio
+@pytest.mark.wait_for_idle
 async def test_wait_for_idle_without_units(event_loop):
     async with base.CleanModel() as model:
         await model.deploy(
             'ubuntu',
             application_name='ubuntu',
-            series='bionic',
             channel='stable',
             num_units=0,
         )
@@ -863,12 +877,12 @@ async def test_wait_for_idle_without_units(event_loop):
 
 @base.bootstrapped
 @pytest.mark.asyncio
+@pytest.mark.wait_for_idle
 async def test_wait_for_idle_with_not_enough_units(event_loop):
     async with base.CleanModel() as model:
         await model.deploy(
             'ubuntu',
             application_name='ubuntu',
-            series='bionic',
             channel='stable',
             num_units=2,
         )
@@ -878,6 +892,7 @@ async def test_wait_for_idle_with_not_enough_units(event_loop):
 
 @base.bootstrapped
 @pytest.mark.asyncio
+@pytest.mark.wait_for_idle
 async def test_wait_for_idle_more_units_than_needed(event_loop):
     async with base.CleanModel() as model:
         charm_path = TESTS_DIR / 'charm'
@@ -900,13 +915,13 @@ async def test_wait_for_idle_more_units_than_needed(event_loop):
 
 @base.bootstrapped
 @pytest.mark.asyncio
+@pytest.mark.wait_for_idle
 async def test_wait_for_idle_with_enough_units(event_loop):
     pytest.skip("This is testing juju functionality")
     async with base.CleanModel() as model:
         await model.deploy(
             'ubuntu',
             application_name='ubuntu',
-            series='jammy',
             channel='stable',
             num_units=3,
         )
@@ -915,13 +930,13 @@ async def test_wait_for_idle_with_enough_units(event_loop):
 
 @base.bootstrapped
 @pytest.mark.asyncio
+@pytest.mark.wait_for_idle
 async def test_wait_for_idle_with_exact_units(event_loop):
     pytest.skip("This is testing juju functionality")
     async with base.CleanModel() as model:
         await model.deploy(
             'ubuntu',
             application_name='ubuntu',
-            series='jammy',
             channel='stable',
             num_units=2,
         )
@@ -930,6 +945,7 @@ async def test_wait_for_idle_with_exact_units(event_loop):
 
 @base.bootstrapped
 @pytest.mark.asyncio
+@pytest.mark.wait_for_idle
 async def test_wait_for_idle_with_exact_units_scale_down(event_loop):
     """Deploys 3 units, waits for them to be idle, then removes 2 of them,
     then waits for exactly 1 unit to be left.