Review: Needs Fixing
Diff comments:
> diff --git a/charms/focal/autopkgtest-cloud-worker/lib/systemd.py
> b/charms/focal/autopkgtest-cloud-worker/lib/systemd.py
> index 0f1cc34..95ed829 100644
> --- a/charms/focal/autopkgtest-cloud-worker/lib/systemd.py
> +++ b/charms/focal/autopkgtest-cloud-worker/lib/systemd.py
> @@ -250,6 +250,24 @@ def update_lxd_dropins(arch, ip, n):
> reload()
>
>
> +def remove_old_lxd_dropins(target_lxd_config):
> + (
> + _,
> + lxd_unit_object_paths,
> + _,
> + ) = get_units()
> + all_lxd_arches = set(
> + list(lxd_unit_object_paths.keys()) + list(target_lxd_config.keys())
> + )
> + for arch in all_lxd_arches:
> + ips = target_lxd_config[arch].keys()
> + for lxd_path in lxd_unit_object_paths:
> + if "autopkgtest@" in lxd_path and [
> + ip for ip in ips if ip not in lxd_path
I'm not entirely sure, but I think this test will always succeed, as long as
you have at least two different IPs, since at least one of them will **not** be
in `lxd_path`, thus will end up in the list you're building, making that list
to evaluate to True.
I think a test that would work would rather have a form like `not any([ip in
lxd_path for ip in ips])`.
> + ]:
> + os.remove(lxd_path)
Adding a log here would certainly help make sure the condition above is
correct, and would help investigate if something weird happens.
> +
> +
> def enable_timer(region, arch, releases):
> unit_names = [
> "build-adt-image@{}-{}-{}.timer".format(release, region, arch)
--
https://code.launchpad.net/~andersson123/autopkgtest-cloud/+git/autopkgtest-cloud/+merge/456773
Your team Canonical's Ubuntu QA is requested to review the proposed merge of
~andersson123/autopkgtest-cloud:lxd-cleanup-srv-files into
autopkgtest-cloud:master.
--
Mailing list: https://launchpad.net/~canonical-ubuntu-qa
Post to : canonical-ubuntu-qa@lists.launchpad.net
Unsubscribe : https://launchpad.net/~canonical-ubuntu-qa
More help : https://help.launchpad.net/ListHelp
