Merge pull request #890 from cderici/wait-for-at-least-units-param-re…

…name

#890

#### Description

This is to clear a bit the semantics of `wait_for_units` parameter in `model.wait_for_idle`, which is to wait for a certain amount of units across all the given applications and return when those are available, regardless of the state of the other units (they can be in an error state or whatever just don't care and return)

Fixes #886


#### QA Steps

We got a couple of tests specific to `wait_for_idle` and `wait_for_units`, so all of them should pass, some important ones are the following:

```
tox -e py3 -- tests/unit/test_model.py::TestModelWaitForIdle
```

```
tox -e integration -- tests/integration/test_model.py::test_wait_for_idle_with_enough_units
```

```
tox -e integration -- tests/integration/test_model.py::test_wait_for_idle_with_exact_units
```

```
tox -e integration -- tests/integration/test_model.py::test_wait_for_idle_with_exact_units_scale_down
```

```
tox -e integration -- tests/integration/test_model.py::test_wait_for_idle_with_exact_units_scale_down_zero
```

juju/model.py

@@ -2574,55 +2574,56 @@ async def _get_source_api(self, url, controller_name=None):
 
     async def wait_for_idle(self, apps=None, raise_on_error=True, raise_on_blocked=False,
                             wait_for_active=False, timeout=10 * 60, idle_period=15, check_freq=0.5,
-                            status=None, wait_for_units=None, wait_for_exact_units=None):
+                            status=None, wait_for_at_least_units=None, wait_for_exact_units=None):
         """Wait for applications in the model to settle into an idle state.
 
-        :param apps (list[str]): Optional list of specific app names to wait on.
+        :param List[str] apps: Optional list of specific app names to wait on.
             If given, all apps must be present in the model and idle, while other
             apps in the model can still be busy. If not given, all apps currently
             in the model must be idle.
 
-        :param raise_on_error (bool): If True, then any unit or app going into
+        :param bool raise_on_error: If True, then any unit or app going into
             "error" status immediately raises either a JujuAppError or a JujuUnitError.
             Note that machine or agent failures will always raise an exception (either
             JujuMachineError or JujuAgentError), regardless of this param. The default
             is True.
 
-        :param raise_on_blocked (bool): If True, then any unit or app going into
+        :param bool raise_on_blocked: If True, then any unit or app going into
             "blocked" status immediately raises either a JujuAppError or a JujuUnitError.
-            The defaul tis False.
+            The default is False.
 
-        :param wait_for_active (bool): If True, then also wait for all unit workload
+        :param bool wait_for_active: If True, then also wait for all unit workload
             statuses to be "active" as well. The default is False.
 
-        :param timeout (float): How long to wait, in seconds, for the bundle settles
+        :param float timeout: How long to wait, in seconds, for the bundle settles
             before raising an asyncio.TimeoutError. If None, will wait forever.
             The default is 10 minutes.
 
-        :param idle_period (float): How long, in seconds, the agent statuses of all
+        :param float idle_period: How long, in seconds, the agent statuses of all
             units of all apps need to be `idle`. This delay is used to ensure that
             any pending hooks have a chance to start to avoid false positives.
             The default is 15 seconds.
 
-        :param check_freq (float): How frequently, in seconds, to check the model.
+        :param float check_freq: How frequently, in seconds, to check the model.
             The default is every half-second.
 
-        :param status (str): The status to wait for. If None, not waiting.
+        :param str status: The status to wait for. If None, not waiting.
             The default is None (not waiting for any status).
 
-        :param wait_for_units (int): The least number of units to be expected before
-            going into the idle state.
+        :param int wait_for_at_least_units: The least number of units to go into the idle
+        state. wait_for_idle will return after that many units are available (across all the
+        given applications).
             The default is 1 unit.
 
-        :param wait_for_exact_units (int): The exact number of units to be expected before
+        :param int wait_for_exact_units: The exact number of units to be expected before
             going into the idle state. (e.g. useful for scaling down).
             When set, takes precedence over the `wait_for_units` parameter.
         """
         if wait_for_active:
             warnings.warn("wait_for_active is deprecated; use status", DeprecationWarning)
             status = "active"
 
-        _wait_for_units = wait_for_units if wait_for_units is not None else 1
+        _wait_for_units = wait_for_at_least_units if wait_for_at_least_units is not None else 1
 
         timeout = timedelta(seconds=timeout) if timeout is not None else None
         idle_period = timedelta(seconds=idle_period)
@@ -2689,10 +2690,11 @@ def _raise_for_status(entities, status):
                     busy.append(app.name + " (not enough units yet - %s/%s)" %
                                 (len(app.units), _wait_for_units))
                     continue
-                # User wants to see a certain # of units, and we have enough
-                elif wait_for_units and len(units_ready) >= _wait_for_units:
+                # User is waiting for at least a certain # of units, and we have enough
+                elif wait_for_at_least_units and len(units_ready) >= _wait_for_units:
                     # So no need to keep looking, we have the desired number of units ready to go,
-                    # exit the loop. Don't return, though, we might still have some errors to raise
+                    # exit the loop. Don't just return here, though, we might still have some
+                    # errors to raise at the end
                     break
                 for unit in app.units:
                     if unit.machine is not None and unit.machine.status == "error":
@@ -2711,7 +2713,8 @@ def _raise_for_status(entities, 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
+                        # 2) We're waiting for a particular status and the workload is in that
+                        # status
                         # 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/test_model.py

@@ -865,7 +865,7 @@ async def test_wait_for_idle_with_not_enough_units(event_loop):
             num_units=2,
         )
         with pytest.raises(jasyncio.TimeoutError):
-            await model.wait_for_idle(timeout=5 * 60, wait_for_units=3)
+            await model.wait_for_idle(timeout=5 * 60, wait_for_at_least_units=3)
 
 
 @base.bootstrapped
@@ -885,9 +885,9 @@ async def test_wait_for_idle_more_units_than_needed(event_loop):
             num_units=1,
         )
 
-        # because the wait_for_units=1, wait_for_idle should return without timing out
+        # because the wait_for_at_least_units=1, wait_for_idle should return without timing out
         # even though there are two more units that aren't active/idle
-        await model.wait_for_idle(timeout=5 * 60, wait_for_units=1, status='active')
+        await model.wait_for_idle(timeout=5 * 60, wait_for_at_least_units=1, status='active')
 
 
 @base.bootstrapped
@@ -902,7 +902,7 @@ async def test_wait_for_idle_with_enough_units(event_loop):
             channel='stable',
             num_units=3,
         )
-        await model.wait_for_idle(timeout=5 * 60, wait_for_units=3)
+        await model.wait_for_idle(timeout=5 * 60, wait_for_at_least_units=3)
 
 
 @base.bootstrapped