Merge ~rgildein/charm-hw-health:bug1983398 into charm-hw-health:master

[Robert Gildein]

Update pytest fixtures and fix deployment

- update to pytest_asyncio.fixture
- fix deploy_app

fixes: #1983398

Update pytest fixtures and fix deployment

- update to pytest_asyncio.fixture
- fix deploy_app

result from functional tests: https://pastebin.ubuntu.com/p/hCnHZb4j6s/

[🤖 Canonical IS Merge Bot (canonical-is-mergebot)]

This merge proposal is being monitored by mergebot. Change the status to Approved to merge.

[Eric Chen (eric-chen)]

LGTM

review: Approve

[Robert Gildein (rgildein)]

I forgot to run lint and unit tests [1].

---
[1]: https://pastebin.ubuntu.com/p/jGrQtDQ5Zn/

[🤖 Canonical IS Merge Bot (canonical-is-mergebot)]

Change successfully merged at revision 5f6c95198de83f732655b7b204855dc5ff183b97

diff --git a/src/README.md b/src/README.md
index c3d71d1..2889eed 100644
--- a/src/README.md
+++ b/src/README.md
@@ -116,7 +116,7 @@ For example:
 [megacli](https://docs.broadcom.com/docs/12351586)/
 [sas3ircu](https://docs.broadcom.com/docs/SAS3IRCU_P16.zip)/
 [sas2ircu](https://docs.broadcom.com/docs/12351735)/
-[perccli] (https://dl.dell.com/FOLDER07576010M/1/PERCCLI_7.1623.00_A11_Linux.tar.gz)
+[perccli](https://dl.dell.com/FOLDER07576010M/1/PERCCLI_7.1623.00_A11_Linux.tar.gz)
 
 You will then have to extract, rename, and compress the binaries to obtain the
 following structure:
diff --git a/src/tests/functional/conftest.py b/src/tests/functional/conftest.py
index c7c856c..b0a1353 100644
--- a/src/tests/functional/conftest.py
+++ b/src/tests/functional/conftest.py
@@ -19,6 +19,9 @@ from juju.errors import JujuError
 
 import pytest
 
+import pytest_asyncio
+
+
 STAT_CMD = """python3 - <<EOF
 import json
 import os
@@ -48,7 +51,7 @@ def event_loop():
     asyncio.set_event_loop(None)
 
 
[e-mail](scope="module")
+@pytest_asyncio.fixture(scope="module")
 async def controller():
     """Connect to the current controller."""
     _controller = Controller()
@@ -57,7 +60,7 @@ async def controller():
     await _controller.disconnect()
 
 
[e-mail](scope="module")
+@pytest_asyncio.fixture(scope="module")
 async def model(controller):  # pylint: disable=redefined-outer-name
     """Create model which lives only for the duration of the test."""
     model_name = "functest-{}".format(uuid.uuid4())
@@ -70,7 +73,7 @@ async def model(controller):  # pylint: disable=redefined-outer-name
             await asyncio.sleep(1)
 
 
[e-mail]()
+@pytest_asyncio.fixture()
 async def get_app(model):  # pylint: disable=redefined-outer-name
     """Return the application requested."""
 
@@ -83,7 +86,7 @@ async def get_app(model):  # pylint: disable=redefined-outer-name
     return _get_app
 
 
[e-mail]()
+@pytest_asyncio.fixture()
 async def get_unit(model):  # pylint: disable=redefined-outer-name
     """Return the requested <app_name>/<unit_number> unit."""
 
@@ -97,7 +100,7 @@ async def get_unit(model):  # pylint: disable=redefined-outer-name
     return _get_unit
 
 
[e-mail]()
+@pytest_asyncio.fixture()
 async def get_entity(get_unit, get_app):  # pylint: disable=redefined-outer-name
     """Return a unit or an application."""
 
@@ -113,7 +116,7 @@ async def get_entity(get_unit, get_app):  # pylint: disable=redefined-outer-name
     return _get_entity
 
 
[e-mail]
+@pytest_asyncio.fixture
 async def run_command(get_unit):  # pylint: disable=redefined-outer-name
     """Run a command on a unit.
 
@@ -129,7 +132,7 @@ async def run_command(get_unit):  # pylint: disable=redefined-outer-name
     return _run_command
 
 
[e-mail]
+@pytest_asyncio.fixture
 async def file_stat(run_command):  # pylint: disable=redefined-outer-name
     """Run stat on a file.
 
@@ -152,7 +155,7 @@ async def file_stat(run_command):  # pylint: disable=redefined-outer-name
     return _file_stat
 
 
[e-mail]
+@pytest_asyncio.fixture
 async def file_contents(run_command):  # pylint: disable=redefined-outer-name
     """Return the contents of a file.
 
@@ -168,7 +171,7 @@ async def file_contents(run_command):  # pylint: disable=redefined-outer-name
     return _file_contents
 
 
[e-mail]
+@pytest_asyncio.fixture
 async def reconfigure_app(get_app, model):  # pylint: disable=redefined-outer-name
     """Apply a different config to the requested app."""
 
diff --git a/src/tests/functional/requirements.txt b/src/tests/functional/requirements.txt
index a10cbae..457c0ae 100644
--- a/src/tests/functional/requirements.txt
+++ b/src/tests/functional/requirements.txt
@@ -1,8 +1,7 @@
-flake8
-juju
+juju < 3.0.0
 mock
-pytest<6
-pytest-asyncio<0.17.*
+pytest
+pytest-asyncio
 requests
 charmhelpers
 charms.reactive
diff --git a/src/tests/functional/test_hwhealth.py b/src/tests/functional/test_hwhealth.py
index 936d9c8..c97cec7 100644
--- a/src/tests/functional/test_hwhealth.py
+++ b/src/tests/functional/test_hwhealth.py
@@ -12,6 +12,8 @@ from hwhealth import tools  # noqa: E402
 
 import pytest  # noqa: E402
 
+import pytest_asyncio  # noqa: E402
+
 
 # Treat all tests as coroutines
 pytestmark = pytest.mark.asyncio
@@ -68,7 +70,7 @@ async def update_hwhealth_res(model, app_name, res_filename):
 ###################
 
 
[e-mail](scope="module", params=SERIES)
+@pytest_asyncio.fixture(scope="module", params=SERIES)
 async def deploy_app(request, model):
     """Deploy the hw-health charm as a subordinate of ubuntu."""
     # TODO: this might look nicer if we deployed a bundle instead. It could be
@@ -78,25 +80,34 @@ async def deploy_app(request, model):
     hw_health_app_name = "hw-health-{}".format(release)
     hw_health_checksum_app_name = "hw-health-checksum-{}".format(release)
 
-    for principal_app in ["ubuntu", "nagios"]:
+    if release == "focal":
+        # NOTE(aluria): nagios was not available in focal
+        # On the other hand, bionic testing would create nagios-bionic
+        # hence nagios-bionic2
+        relname = "bionic2"
+        series = "bionic"
+    else:
         relname = series = release
-        if principal_app == "nagios" and release == "focal":
-            # NOTE(aluria): cs:nagios was not available in focal
-            # On the other hand, bionic testing would create nagios-bionic
-            # hence nagios-bionic2
-            relname = "bionic2"
-            series = "bionic"
-        await model.deploy(
-            principal_app,
-            application_name="{}-{}".format(principal_app, relname),
-            series=series,
-            channel=channel,
-        )
+
+    await model.deploy(
+        "nagios",
+        application_name="{}-{}".format("nagios", relname),
+        series=series,
+        channel=channel,
+    )
+    await model.deploy(
+        "ubuntu",
+        application_name="{}-{}".format("ubuntu", release),
+        series=release,
+        channel=channel,
+        to="lxd",  # to ensure that hw-health would be deployed on LXD container
+    )
     await model.deploy(
         "ubuntu",
         application_name="ubuntu-checksum-{}".format(release),
         series=release,
         channel=channel,
+        to="lxd",  # to ensure that hw-health would be deployed on LXD container
     )
     nrpe_app = await model.deploy(
         "nrpe",
@@ -105,10 +116,12 @@ async def deploy_app(request, model):
         num_units=0,
         channel=channel,
     )
-    for ubuntu_unit in ["ubuntu", "ubuntu-checksum"]:
-        await nrpe_app.add_relation(
-            "general-info", "{}-{}:juju-info".format(ubuntu_unit, release)
-        )
+    await nrpe_app.add_relation(
+        "general-info", "{}-{}:juju-info".format("ubuntu", release)
+    )
+    await nrpe_app.add_relation(
+        "general-info", "{}-{}:juju-info".format("ubuntu-checksum", release)
+    )
     await nrpe_app.add_relation("monitors", "nagios-{}:monitors".format(relname))
 
     # Attaching resources is not implemented yet in libjuju
@@ -116,14 +129,12 @@ async def deploy_app(request, model):
     await deploy_hwhealth_res(model, hw_health_app_name, "tools.zip")
     await deploy_hwhealth_res(model, hw_health_checksum_app_name, "tools-checksum.zip")
 
-    # This is pretty horrible, but we can't deploy via libjuju
-    while True:
-        try:
-            hw_health_app = model.applications[hw_health_app_name]
-            hw_health_checksum_app = model.applications[hw_health_checksum_app_name]
-            break
-        except KeyError:
-            await asyncio.sleep(5)
+    other_apps = [
+        app for app in model.applications if not app.startswith == "hw-health"
+    ]
+    await model.wait_for_idle(apps=other_apps, timeout=DEF_TIMEOUT)
+    hw_health_app = model.applications[hw_health_app_name]
+    hw_health_checksum_app = model.applications[hw_health_checksum_app_name]
 
     await hw_health_app.add_relation(
         "general-info", "ubuntu-{}:juju-info".format(release)
@@ -139,19 +150,15 @@ async def deploy_app(request, model):
         "nrpe-external-master", "{}:nrpe-external-master".format(nrpe_app.name)
     )
 
-    # The app will initially be in blocked state because it's running in a
-    # container
-    await model.block_until(
-        lambda: (
-            hw_health_app.status == "blocked"
-            and hw_health_checksum_app.status == "blocked"  # noqa W503
-        ),
+    await model.wait_for_idle(
+        apps=[hw_health_app_name, hw_health_checksum_app_name],
+        status="blocked",
         timeout=DEF_TIMEOUT,
     )
     yield hw_health_app
 
 
[e-mail](scope="module")
+@pytest_asyncio.fixture(scope="module")
 async def deployed_unit(deploy_app):
     """Return the hw-health unit we've deployed."""
     return deploy_app.units[0]
@@ -163,7 +170,7 @@ def _monkey_config(k=None):
     return {"manufacturer": "test"}.get(k)
 
 
[e-mail](scope="function")
+@pytest_asyncio.fixture(scope="function")
 async def toolset(monkeypatch):
     # All tool classes know which files should be installed and how, so we can
     # use them to read the expected stat results. Monkeypatching is however
@@ -193,15 +200,11 @@ async def test_forced_deploy(deploy_app, model, run_command):
     # Create a fake NVMe device for the cronjob to be configured
     create_fake_nvme_cmd = "/bin/bash -c 'touch /dev/nvme0'"
     series = deploy_app.name.split("-")[-1]
-    for unit in model.units.values():
-        if unit.entity_id.startswith("ubuntu-{}".format(series)):
-            ubuntu_unit = unit
-            await model.block_until(
-                lambda: ubuntu_unit.workload_status == "active", timeout=DEF_TIMEOUT
-            )
-            await run_command(create_fake_nvme_cmd, ubuntu_unit)
-            break
-
+    ubuntu_unit = model.applications["ubuntu-{}".format(series)].units[0]
+    await model.block_until(
+        lambda: ubuntu_unit.workload_status == "active", timeout=DEF_TIMEOUT
+    )
+    await run_command(create_fake_nvme_cmd, ubuntu_unit)
     await deploy_app.set_config({"manufacturer": "test"})
     await model.block_until(lambda: deploy_app.status == "active", timeout=DEF_TIMEOUT)
     assert deploy_app.status == "active"