sts-sponsors team mailing list archive

[Thorsten Merten <mp+436109@xxxxxxxxxxxxxxxxxx>]

Review: Approve

Some minor ideas below. I am really excited to see this running on CI :)

Diff comments:

> diff --git a/systemtests/env_builder/test_basic.py b/systemtests/env_builder/test_basic.py
> index 7ecfbd4..7405a05 100644
> --- a/systemtests/env_builder/test_basic.py
> +++ b/systemtests/env_builder/test_basic.py
> @@ -17,17 +22,59 @@ from systemtests.utils import (
>      retries,
>      wait_for_machine,
>      wait_for_new_machine,
> -    wait_for_ready_controllers,
>  )
>  
>  if TYPE_CHECKING:
>      from logging import Logger
>  
> -    from systemtests.api import AuthenticatedAPIClient, UnauthenticatedMAASAPIClient
>      from systemtests.machine_config import MachineConfig
>      from systemtests.region import MAASRegion
>  
>  
> +@retry(tries=3)
> +def _ensure_machine_enlisted(
> +    maas_api_client: AuthenticatedAPIClient,
> +    mac_address: str,
> +    instance: Instance,
> +) -> Machine:
> +    instance_log = instance.logger.getChild(instance.name)
> +    # Find the VM in MAAS by MAC
> +    maybe_machine = maas_api_client.list_machines(mac_address=mac_address)
> +    if maybe_machine:

maybe_machine? I can't come up with an awesome name, but are we not using plural for arrays (independently of the number of items in it)?

> +        # Yay, it exists
> +        return maybe_machine[0]
> +
> +    # Machine not registered, let's boot it up
> +    @retry(tries=5, delay=5, backoff=1.2, logger=instance_log)
> +    def _boot_vm(vm: Instance) -> None:
> +        status = instance.status()
> +        if status == "RUNNING":

I am confused how this goes with the "let's boot it up" part of the comment above?

> +            instance_log.debug("already running, restarting")
> +            instance.restart()
> +        elif status == "STOPPED":
> +            instance_log.debug("is stopped, starting")
> +            try:
> +                instance.start()
> +            except CalledProcessError:
> +                debug_lxd_vm(instance.name, instance_log)
> +                raise
> +        else:
> +            assert False, f"Don't know how to handle lxd_vm status: {status}"

raise AssertionError("...")?

> +
> +    _boot_vm(instance)
> +    try:
> +        vm_status = instance.status()
> +    except ValueError:
> +        vm_status = "not available"
> +    instance_log.debug(f"is {vm_status}")
> +
> +    machine = wait_for_new_machine(
> +        maas_api_client, mac_address, instance.name, timeout=(5 * 60, 20)
> +    )
> +    instance_log.debug(f"found machine {machine['hostname']}")
> +    return machine
> +
> +
>  class TestSetup:
>      @pytest.mark.skip_if_installed_from_snap("Prometheus is installed in the snap")
>      def test_setup_prometheus(


-- 
https://code.launchpad.net/~adam-collard/maas-ci/+git/system-tests/+merge/436109
Your team MAAS Committers is subscribed to branch ~maas-committers/maas-ci/+git/system-tests:master.