From 4d29d7d96529f679fd483254c3c400421827b3ee Mon Sep 17 00:00:00 2001 From: Yann Dirson Date: Tue, 7 May 2024 15:59:23 +0200 Subject: [PATCH 01/24] host.main_sr: don't return "" as UUID on missing default-SR Signed-off-by: Yann Dirson --- lib/host.py | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/host.py b/lib/host.py index 712c20532..1c7b694d8 100644 --- a/lib/host.py +++ b/lib/host.py @@ -480,6 +480,7 @@ def main_sr(self): else: sr_uuid = self.pool.param_get('default-SR') assert sr_uuid, f"DEFAULT_SR='default' so there must be a default SR on the pool of host {self}" + assert sr_uuid != "" return sr_uuid def hostname(self): From 1450f8cc066cbf0dc9c94d6df6de9c023bee5b31 Mon Sep 17 00:00:00 2001 From: Yann Dirson Date: Mon, 13 May 2024 11:45:19 +0200 Subject: [PATCH 02/24] Host: rename main_sr to main_sr_uuid Other methods to get SRs return `SR` objects, this was inconsistent. Signed-off-by: Yann Dirson --- conftest.py | 2 +- lib/host.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/conftest.py b/conftest.py index 1e6c0bfcf..721f4d458 100644 --- a/conftest.py +++ b/conftest.py @@ -314,7 +314,7 @@ def imported_vm(host, vm_ref): name = vm_orig.name() logging.info(">> Reuse VM %s (%s) on host %s" % (vm_ref, name, host)) else: - vm_orig = host.import_vm(vm_ref, host.main_sr(), use_cache=CACHE_IMPORTED_VM) + vm_orig = host.import_vm(vm_ref, host.main_sr_uuid(), use_cache=CACHE_IMPORTED_VM) if CACHE_IMPORTED_VM: # Clone the VM before running tests, so that the original VM remains untouched diff --git a/lib/host.py b/lib/host.py index 1c7b694d8..15dce97ed 100644 --- a/lib/host.py +++ b/lib/host.py @@ -460,7 +460,7 @@ def local_vm_srs(self): srs.append(sr) return srs - def main_sr(self): + def main_sr_uuid(self): """ Main SR is either the default SR, or the first local SR, depending on data.py's DEFAULT_SR. """ try: from data import DEFAULT_SR From 7a987e403435354231f5e2247450adcd04029881 Mon Sep 17 00:00:00 2001 From: Yann Dirson Date: Tue, 14 May 2024 10:51:31 +0200 Subject: [PATCH 03/24] local_cmd: log command to be run, like ssh does Signed-off-by: Yann Dirson --- lib/commands.py | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/commands.py b/lib/commands.py index 841f52e38..71d69ca9b 100644 --- a/lib/commands.py +++ b/lib/commands.py @@ -205,6 +205,7 @@ def sftp(hostname_or_ip, cmds, check=True, suppress_fingerprint_warnings=True): def local_cmd(cmd, check=True, decode=True): """ Run a command locally on tester end. """ + logging.debug("[local] %s", (cmd,)) res = subprocess.run( cmd, stdout=subprocess.PIPE, From 84af37b5c51760d633d688cc07c52d938e65c6c5 Mon Sep 17 00:00:00 2001 From: Yann Dirson Date: Mon, 13 May 2024 14:58:47 +0200 Subject: [PATCH 04/24] VM: fix name of method dealing with CD insertion, provide a generic API "vm-cd-insert" does not mount anything, as the need for a subsequent "mount" in the only test using it shows. Signed-off-by: Yann Dirson --- lib/vm.py | 9 ++++++--- tests/guest-tools/unix/test_guest_tools_unix.py | 4 ++-- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/lib/vm.py b/lib/vm.py index f21c0d5c3..62d1f6c1f 100644 --- a/lib/vm.py +++ b/lib/vm.py @@ -334,10 +334,13 @@ def detect_package_manager(self): else: return PackageManagerEnum.UNKNOWN - def mount_guest_tools_iso(self): - self.host.xe('vm-cd-insert', {'uuid': self.uuid, 'cd-name': 'guest-tools.iso'}) + def insert_cd(self, vdi_name): + self.host.xe('vm-cd-insert', {'uuid': self.uuid, 'cd-name': vdi_name}) - def unmount_guest_tools_iso(self): + def insert_guest_tools_iso(self): + self.insert_cd('guest-tools.iso') + + def eject_cd(self): self.host.xe('vm-cd-eject', {'uuid': self.uuid}) # *** Common reusable test fragments diff --git a/tests/guest-tools/unix/test_guest_tools_unix.py b/tests/guest-tools/unix/test_guest_tools_unix.py index 51664777f..29e4ce601 100644 --- a/tests/guest-tools/unix/test_guest_tools_unix.py +++ b/tests/guest-tools/unix/test_guest_tools_unix.py @@ -62,7 +62,7 @@ def test_install(self, running_vm, state): # mount ISO logging.info("Mount guest tools ISO") - vm.mount_guest_tools_iso() + vm.insert_guest_tools_iso() tmp_mnt = vm.ssh(['mktemp', '-d']) time.sleep(2) # wait a small amount of time just to ensure the device is available vm.ssh(['mount', '-t', 'iso9660', '/dev/cdrom', tmp_mnt]) @@ -80,7 +80,7 @@ def test_install(self, running_vm, state): # unmount ISO logging.info("Unmount guest tools ISO") vm.ssh(['umount', tmp_mnt]) - vm.unmount_guest_tools_iso() + vm.eject_cd() # check that xe-daemon is running wait_for(lambda: vm.ssh_with_result(['pgrep', '-f', 'xe-daemon']).returncode == 0, From 15ce1bd3a08c4f775923318b38401bc7b71ffdb2 Mon Sep 17 00:00:00 2001 From: Yann Dirson Date: Mon, 13 May 2024 15:16:21 +0200 Subject: [PATCH 05/24] VM: add logging for CD insert/eject Signed-off-by: Yann Dirson --- lib/vm.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/vm.py b/lib/vm.py index 62d1f6c1f..80349fd2c 100644 --- a/lib/vm.py +++ b/lib/vm.py @@ -335,12 +335,14 @@ def detect_package_manager(self): return PackageManagerEnum.UNKNOWN def insert_cd(self, vdi_name): + logging.info("Insert CD %r in VM %s", vdi_name, self.uuid) self.host.xe('vm-cd-insert', {'uuid': self.uuid, 'cd-name': vdi_name}) def insert_guest_tools_iso(self): self.insert_cd('guest-tools.iso') def eject_cd(self): + logging.info("Ejecting CD from VM %s", self.uuid) self.host.xe('vm-cd-eject', {'uuid': self.uuid}) # *** Common reusable test fragments From 0d985f3e13eef79cb6334609a0a21e2f43213a72 Mon Sep 17 00:00:00 2001 From: Yann Dirson Date: Tue, 14 May 2024 17:51:38 +0200 Subject: [PATCH 06/24] _ssh: remove duplicate default values The entrypoint is ssh(), and the default values are already there. Signed-off-by: Yann Dirson --- lib/commands.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/commands.py b/lib/commands.py index 71d69ca9b..c5a54bc55 100644 --- a/lib/commands.py +++ b/lib/commands.py @@ -65,8 +65,8 @@ def _ellide_log_lines(log): OUPUT_LOGGER.addHandler(OUTPUT_HANDLER) OUTPUT_HANDLER.setFormatter(logging.Formatter('%(message)s')) -def _ssh(hostname_or_ip, cmd, check=True, simple_output=True, suppress_fingerprint_warnings=True, - background=False, target_os='linux', decode=True, options=[]): +def _ssh(hostname_or_ip, cmd, check, simple_output, suppress_fingerprint_warnings, + background, target_os, decode, options): opts = list(options) opts.append('-o "BatchMode yes"') if suppress_fingerprint_warnings: From e3dacc0850add0ba222994043d940168a06a428d Mon Sep 17 00:00:00 2001 From: Yann Dirson Date: Wed, 15 May 2024 11:06:07 +0200 Subject: [PATCH 07/24] Remove redundant logs on VM creation Since f9b53659ca2936ae8b5014b5a9b9dbada6e215a3 the VM ctor takes care of reporting this info. Signed-off-by: Yann Dirson --- lib/host.py | 1 - lib/vm.py | 1 - 2 files changed, 2 deletions(-) diff --git a/lib/host.py b/lib/host.py index 15dce97ed..73848a98f 100644 --- a/lib/host.py +++ b/lib/host.py @@ -227,7 +227,6 @@ def import_vm(self, uri, sr_uuid=None, use_cache=False): params['sr-uuid'] = sr_uuid logging.info(msg) vm_uuid = self.xe('vm-import', params) - logging.info("VM UUID: %s" % vm_uuid) vm_name = prefix_object_name(self.xe('vm-param-get', {'uuid': vm_uuid, 'param-name': 'name-label'})) vm = VM(vm_uuid, self) vm.param_set('name-label', vm_name) diff --git a/lib/vm.py b/lib/vm.py index 80349fd2c..c2f0b1655 100644 --- a/lib/vm.py +++ b/lib/vm.py @@ -481,7 +481,6 @@ def clone(self): name = self.name() + '_clone_for_tests' logging.info("Clone VM") uuid = self.host.xe('vm-clone', {'uuid': self.uuid, 'new-name-label': name}) - logging.info("New VM: %s (%s)" % (uuid, name)) return VM(uuid, self.host) def install_uefi_certs(self, auths): From 7f963e199d74ab3cac603fea34d046acb64aef58 Mon Sep 17 00:00:00 2001 From: Yann Dirson Date: Wed, 29 May 2024 14:04:35 +0200 Subject: [PATCH 08/24] host.import_vm: report when VM not found in cache Signed-off-by: Yann Dirson --- lib/host.py | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/host.py b/lib/host.py index 73848a98f..ea7944b17 100644 --- a/lib/host.py +++ b/lib/host.py @@ -215,6 +215,7 @@ def import_vm(self, uri, sr_uuid=None, use_cache=False): if not vm.vdi_uuids() or vm.get_sr().uuid == sr_uuid: logging.info(f"Reusing cached VM {vm.uuid} for {uri}") return vm + logging.info("Could not find a vm in cache with key %r", cache_key) params = {} msg = "Import VM %s" % uri From 820b38db0719efc596b1a6d6f2ea797e3ca5f7b9 Mon Sep 17 00:00:00 2001 From: Yann Dirson Date: Thu, 30 May 2024 11:16:05 +0200 Subject: [PATCH 09/24] host: split cached_vm identification from import_vm This will be useful to the plugin that allows not rerunning a cached dependency: it needs to probe the cache. Signed-off-by: Yann Dirson --- lib/host.py | 38 ++++++++++++++++++++++++-------------- 1 file changed, 24 insertions(+), 14 deletions(-) diff --git a/lib/host.py b/lib/host.py index ea7944b17..034b64398 100644 --- a/lib/host.py +++ b/lib/host.py @@ -200,22 +200,31 @@ def xo_server_reconnect(self): # is not enough to guarantee that the host object exists yet. wait_for(lambda: xo_object_exists(self.uuid), "Wait for XO to know about HOST %s" % self.uuid) + @staticmethod + def vm_cache_key(uri): + return f"[Cache for {uri}]" + + def cached_vm(self, uri, sr_uuid): + assert sr_uuid, "A SR UUID is necessary to use import cache" + cache_key = self.vm_cache_key(uri) + # Look for an existing cache VM + vm_uuids = safe_split(self.xe('vm-list', {'name-description': cache_key}, minimal=True), ',') + + for vm_uuid in vm_uuids: + vm = VM(vm_uuid, self) + # Make sure the VM is on the wanted SR. + # Assumption: if the first disk is on the SR, the VM is. + # If there's no VDI at all, then it is virtually on any SR. + if not vm.vdi_uuids() or vm.get_sr().uuid == sr_uuid: + logging.info(f"Reusing cached VM {vm.uuid} for {uri}") + return vm + logging.info("Could not find a VM in cache with key %r", cache_key) + def import_vm(self, uri, sr_uuid=None, use_cache=False): if use_cache: - assert sr_uuid, "A SR UUID is necessary to use import cache" - cache_key = f"[Cache for {uri}]" - # Look for an existing cache VM - vm_uuids = safe_split(self.xe('vm-list', {'name-description': cache_key}, minimal=True), ',') - - for vm_uuid in vm_uuids: - vm = VM(vm_uuid, self) - # Make sure the VM is on the wanted SR. - # Assumption: if the first disk is on the SR, the VM is. - # If there's no VDI at all, then it is virtually on any SR. - if not vm.vdi_uuids() or vm.get_sr().uuid == sr_uuid: - logging.info(f"Reusing cached VM {vm.uuid} for {uri}") - return vm - logging.info("Could not find a vm in cache with key %r", cache_key) + vm = self.cached_vm(uri, sr_uuid) + if vm: + return vm params = {} msg = "Import VM %s" % uri @@ -235,6 +244,7 @@ def import_vm(self, uri, sr_uuid=None, use_cache=False): for vif in vm.vifs(): vif.move(self.management_network()) if use_cache: + cache_key = self.vm_cache_key(uri) logging.info(f"Marking VM {vm.uuid} as cached") vm.param_set('name-description', cache_key) return vm From 16e44d499accfa84c54736fcf3b7f840d08dd2c3 Mon Sep 17 00:00:00 2001 From: Yann Dirson Date: Wed, 19 Jun 2024 13:49:13 +0200 Subject: [PATCH 10/24] requirements: cleanup cruft, avoid duplication in README Signed-off-by: Yann Dirson --- README.md | 7 +++---- requirements/base.txt | 11 ----------- requirements/dev.txt | 7 ------- 3 files changed, 3 insertions(+), 22 deletions(-) diff --git a/README.md b/README.md index 497771d89..ba61dad17 100644 --- a/README.md +++ b/README.md @@ -4,8 +4,9 @@ Note: this is a perpertual work in progress. If you encounter any obstacles or b ## Main requirements * python >= 3.5 -* pytest >= 5.4 (python3 version) -* xo-cli >= 0.17.0 installed, in the PATH, and registered to an instance of XO that will be used during the tests +* packages as listed in requirements/base.txt +* extra test-specific requirements are documented in the test file + "Requirements" header ### Quick install (python requirements) @@ -13,14 +14,12 @@ Install the python requirements using pip: ``` $ pip install -r requirements/base.txt - ``` Additionally, for dev dependencies (things like the linter / style checker): ``` $ pip install -r requirements/dev.txt - ``` ## Other requirements diff --git a/requirements/base.txt b/requirements/base.txt index 34fb262ec..facd2a48e 100644 --- a/requirements/base.txt +++ b/requirements/base.txt @@ -1,14 +1,3 @@ -attrs>=20.3.0 -cffi>=1.14.4 cryptography>=3.3.1 -importlib-metadata>=2.1.1 -more-itertools>=8.6.0 packaging>=20.7 -pathlib2>=2.3.5 -pluggy>=0.13.1 -py>=1.9.0 -pyparsing>=2.4.7 pytest>=5.4.0 -six>=1.15.0 -wcwidth>=0.2.5 -zipp>=1.2.0 diff --git a/requirements/dev.txt b/requirements/dev.txt index 0853008da..3b8d7bb15 100644 --- a/requirements/dev.txt +++ b/requirements/dev.txt @@ -1,13 +1,6 @@ # All requirements + those only used in development ansible>=5.0.1 -ansible-core>=2.12.1 -beautifulsoup4>=4.10.0 bs4>=0.0.1 -Jinja2>=3.0.3 -MarkupSafe>=2.0.1 pycodestyle>=2.6.0 -pycparser>=2.21 PyYAML>=6.0 -resolvelib>=0.5.4 -soupsieve>=2.3.1 -r base.txt From 7282e9462c77a2a29b33ef61ed15cc44b7904986 Mon Sep 17 00:00:00 2001 From: Yann Dirson Date: Wed, 19 Jun 2024 16:05:19 +0200 Subject: [PATCH 11/24] try_get_and_store_ip: raise boot timeout for nested tests When testing in a nested setup, 2 min is not enough to boot a Debian 12 to the point the XS agent has published to xenstore. 5 min should be enough for everyone. Signed-off-by: Yann Dirson --- conftest.py | 2 +- lib/vm.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/conftest.py b/conftest.py index 721f4d458..b7183b3b6 100644 --- a/conftest.py +++ b/conftest.py @@ -339,7 +339,7 @@ def running_vm(imported_vm): if not vm.is_running(): vm.start() wait_for(vm.is_running, '> Wait for VM running') - wait_for(vm.try_get_and_store_ip, "> Wait for VM IP") + wait_for(vm.try_get_and_store_ip, "> Wait for VM IP", timeout_secs=5 * 60) wait_for(vm.is_ssh_up, "> Wait for VM SSH up") return vm # no teardown diff --git a/lib/vm.py b/lib/vm.py index c2f0b1655..51752b65a 100644 --- a/lib/vm.py +++ b/lib/vm.py @@ -122,7 +122,7 @@ def wait_for_os_booted(self): # waiting for the IP: # - allows to make sure the OS actually started (on VMs that have the management agent) # - allows to store the IP for future use in the VM object - wait_for(self.try_get_and_store_ip, "Wait for VM IP") + wait_for(self.try_get_and_store_ip, "Wait for VM IP", timeout_secs=5 * 60) # now wait also for the management agent to have started wait_for(self.is_management_agent_up, "Wait for management agent up") From 95f7c1bdd14f85e07fb033baa0e0aac2ba97bf5b Mon Sep 17 00:00:00 2001 From: Yann Dirson Date: Wed, 19 Jun 2024 16:24:47 +0200 Subject: [PATCH 12/24] test_guest_tools_unix: make requirements more explicit Signed-off-by: Yann Dirson --- tests/guest-tools/unix/test_guest_tools_unix.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/guest-tools/unix/test_guest_tools_unix.py b/tests/guest-tools/unix/test_guest_tools_unix.py index 29e4ce601..a0235e0ed 100644 --- a/tests/guest-tools/unix/test_guest_tools_unix.py +++ b/tests/guest-tools/unix/test_guest_tools_unix.py @@ -9,6 +9,7 @@ # - hostA2: Second member of the pool. Can have any local SR. No need to specify it on CLI. # From --vm parameter # - A VM to import, supported by the Linux/install.sh script of the guest tools ISO +# (without this flag you get an alpine, and that is not suitable) class State: def __init__(self): From 6d4a5b1ead9c2831c78844210cf6578a6f2ab28c Mon Sep 17 00:00:00 2001 From: Yann Dirson Date: Wed, 19 Jun 2024 14:20:03 +0200 Subject: [PATCH 13/24] test_basic_without_ssh: split start/stop test from those needing a running VM. There is no other inter-test dependency in this file, this allows to stop using @pytest.mark.incremental here. Signed-off-by: Yann Dirson --- conftest.py | 10 +++++--- tests/misc/test_basic_without_ssh.py | 34 +++++++++++++--------------- 2 files changed, 23 insertions(+), 21 deletions(-) diff --git a/conftest.py b/conftest.py index b7183b3b6..a243a70f2 100644 --- a/conftest.py +++ b/conftest.py @@ -332,18 +332,22 @@ def imported_vm(host, vm_ref): vm.destroy(verify=True) @pytest.fixture(scope="module") -def running_vm(imported_vm): +def started_vm(imported_vm): vm = imported_vm - # may be already running if we skipped the import to use an existing VM if not vm.is_running(): vm.start() wait_for(vm.is_running, '> Wait for VM running') wait_for(vm.try_get_and_store_ip, "> Wait for VM IP", timeout_secs=5 * 60) - wait_for(vm.is_ssh_up, "> Wait for VM SSH up") return vm # no teardown +@pytest.fixture(scope="module") +def running_vm(started_vm): + vm = started_vm + wait_for(vm.is_ssh_up, "> Wait for VM SSH up") + return vm + @pytest.fixture(scope='module') def unix_vm(imported_vm): vm = imported_vm diff --git a/tests/misc/test_basic_without_ssh.py b/tests/misc/test_basic_without_ssh.py index 52587df77..1463cc2cc 100644 --- a/tests/misc/test_basic_without_ssh.py +++ b/tests/misc/test_basic_without_ssh.py @@ -16,8 +16,6 @@ # - when using an existing VM, the VM can be on any host of the pool, # the local SR or shared SR: the test will adapt itself. # Note however that an existing VM will be left on a different SR after the tests. -# -# This test suite is meant to be run entirely. There's no guarantee that cherry-picking tests will work. @pytest.fixture(scope='session') def existing_shared_sr(host): @@ -25,21 +23,25 @@ def existing_shared_sr(host): assert sr is not None, "A shared SR on the pool is required" return sr -@pytest.mark.incremental # tests depend on each other. If one test fails, don't execute the others @pytest.mark.multi_vms # run them on a variety of VMs @pytest.mark.big_vm # and also on a really big VM ideally -class TestBasicNoSSH: - def test_start(self, imported_vm): - vm = imported_vm - # if VM already running, stop it - if (vm.is_running()): - logging.info("VM already running, shutting it down first") - vm.shutdown(verify=True) - vm.start() - # this also tests the guest tools at the same time since they are used - # for retrieving the IP address and management agent status. - vm.wait_for_os_booted() +def test_vm_start_stop(imported_vm): + vm = imported_vm + # if VM already running, stop it + if (vm.is_running()): + logging.info("VM already running, shutting it down first") + vm.shutdown(verify=True) + vm.start() + # this also tests the guest tools at the same time since they are used + # for retrieving the IP address and management agent status. + vm.wait_for_os_booted() + + vm.shutdown(verify=True) +@pytest.mark.multi_vms # run them on a variety of VMs +@pytest.mark.big_vm # and also on a really big VM ideally +@pytest.mark.usefixtures("started_vm") +class TestBasicNoSSH: def test_pause(self, imported_vm): vm = imported_vm vm.pause(verify=True) @@ -104,7 +106,3 @@ def live_migrate(vm, dest_host, dest_sr, check_vdis=False): else: logging.info("* Preparing for live migration without storage motion *") live_migrate(vm, host1, existing_shared_sr) - - def test_shutdown(self, imported_vm): - vm = imported_vm - vm.shutdown(verify=True) From 4f79b05089236a74c5dd88c8f72ebf9a4cd2e679 Mon Sep 17 00:00:00 2001 From: Yann Dirson Date: Fri, 9 Aug 2024 10:51:25 +0200 Subject: [PATCH 14/24] test_basic_without_ssh: clarify requirements a bit (fixes #244) Signed-off-by: Yann Dirson --- tests/misc/test_basic_without_ssh.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tests/misc/test_basic_without_ssh.py b/tests/misc/test_basic_without_ssh.py index 1463cc2cc..de2915619 100644 --- a/tests/misc/test_basic_without_ssh.py +++ b/tests/misc/test_basic_without_ssh.py @@ -9,8 +9,9 @@ # because the VM may not have SSH installed, which is needed for more advanced scenarios. # # Requirements: -# - a two-host XCP-ng pool >= 8.1. -# - the pool must have 1 shared SR +# - XCP-ng >= 8.1. +# - a two-host pool with 1 shared SR (for test_live_migrate) +# - the pool must have `suspend-image-SR` set (for suspend and checkpoint) # - each host must have a local SR # - any VM with guest tools installed. No SSH required for this test suite. # - when using an existing VM, the VM can be on any host of the pool, From e445e031fad9a5c310e98a197d719631348d1d8a Mon Sep 17 00:00:00 2001 From: Yann Dirson Date: Wed, 19 Jun 2024 16:25:56 +0200 Subject: [PATCH 15/24] test_guest_tools_unix: use a fixture instead of expecting test_install Same as for test_basic_without_ssh, and this allows to stop using @pytest.mark.incremental here too. Signed-off-by: Yann Dirson --- tests/guest-tools/unix/test_guest_tools_unix.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/guest-tools/unix/test_guest_tools_unix.py b/tests/guest-tools/unix/test_guest_tools_unix.py index a0235e0ed..4bb82bc05 100644 --- a/tests/guest-tools/unix/test_guest_tools_unix.py +++ b/tests/guest-tools/unix/test_guest_tools_unix.py @@ -16,7 +16,6 @@ def __init__(self): self.tools_version = None self.vm_distro = None -@pytest.mark.incremental # tests depend on each other. If one test fails, don't execute the others @pytest.mark.multi_vms @pytest.mark.usefixtures("unix_vm") class TestGuestToolsUnix: @@ -34,7 +33,8 @@ def _check_os_info(self, vm, vm_distro): detected_distro = vm.distro() assert detected_distro == vm_distro - def test_install(self, running_vm, state): + @pytest.fixture(scope="class", autouse=True) + def vm_install(self, running_vm, state): vm = running_vm # skip test for some unixes From 14eeee636143b71fdddd7f67e9ac2bf6c9a896bf Mon Sep 17 00:00:00 2001 From: Yann Dirson Date: Wed, 19 Jun 2024 16:28:44 +0200 Subject: [PATCH 16/24] Drop "incremental" marker. Previous commits removed its two only uses, replaced by an autouse fixture. Other possible uses would be covered (with a more fine-grained approach) by pytest-dependency. This frees the global pytest_runtest_makereport hook for potential reuse. Signed-off-by: Yann Dirson --- conftest.py | 15 --------------- pytest.ini | 1 - 2 files changed, 16 deletions(-) diff --git a/conftest.py b/conftest.py index a243a70f2..7ca337b7f 100644 --- a/conftest.py +++ b/conftest.py @@ -19,21 +19,6 @@ # need to import them in the global conftest.py so that they are recognized as fixtures. from pkgfixtures import formatted_and_mounted_ext4_disk, sr_disk_wiped -# *** Support for incremental tests in test classes *** -# From https://stackoverflow.com/questions/12411431/how-to-skip-the-rest-of-tests-in-the-class-if-one-has-failed -def pytest_runtest_makereport(item, call): - if "incremental" in item.keywords: - if call.excinfo is not None: - parent = item.parent - parent._previousfailed = item - -def pytest_runtest_setup(item): - previousfailed = getattr(item.parent, "_previousfailed", None) - if previousfailed is not None: - pytest.skip("previous test failed (%s)" % previousfailed.name) - -# *** End of: Support for incremental tests *** - def pytest_addoption(parser): parser.addoption( "--hosts", diff --git a/pytest.ini b/pytest.ini index 1344531f3..ab6dc1e72 100644 --- a/pytest.ini +++ b/pytest.ini @@ -3,7 +3,6 @@ addopts = -ra --maxfail=1 -s markers = # *** Markers that change test behaviour *** default_vm: mark a test with a default VM in case no --vm parameter was given. - incremental: mark a class so that any test failure skips the rest of the tests in the class. # *** Markers used to select tests at collect stage *** From 980080ce85fc26f7db2d913fe50bd8b07e43af7d Mon Sep 17 00:00:00 2001 From: Yann Dirson Date: Wed, 19 Jun 2024 16:48:54 +0200 Subject: [PATCH 17/24] conftest: regroup all pytest hooks together, separated from fixtures Signed-off-by: Yann Dirson --- conftest.py | 72 ++++++++++++++++++++++++++++------------------------- 1 file changed, 38 insertions(+), 34 deletions(-) diff --git a/conftest.py b/conftest.py index 7ca337b7f..2d3c0b5a6 100644 --- a/conftest.py +++ b/conftest.py @@ -19,6 +19,8 @@ # need to import them in the global conftest.py so that they are recognized as fixtures. from pkgfixtures import formatted_and_mounted_ext4_disk, sr_disk_wiped +# pytest hooks + def pytest_addoption(parser): parser.addoption( "--hosts", @@ -70,6 +72,42 @@ def pytest_configure(config): global_config.ignore_ssh_banner = config.getoption('--ignore-ssh-banner') global_config.ssh_output_max_lines = int(config.getoption('--ssh-output-max-lines')) +def pytest_generate_tests(metafunc): + if "vm_ref" in metafunc.fixturenames: + vms = metafunc.config.getoption("vm") + if not vms: + vms = [None] # no --vm parameter does not mean skip the test, for us, it means use the default + metafunc.parametrize("vm_ref", vms, indirect=True, scope="module") + +def pytest_collection_modifyitems(items, config): + # Automatically mark tests based on fixtures they require. + # Check pytest.ini or pytest --markers for marker descriptions. + + markable_fixtures = [ + 'uefi_vm', + 'unix_vm', + 'windows_vm', + 'hostA2', + 'hostB1', + 'sr_disk', + 'sr_disk_4k' + ] + + for item in items: + fixturenames = getattr(item, 'fixturenames', ()) + for fixturename in markable_fixtures: + if fixturename in fixturenames: + item.add_marker(fixturename) + + if 'vm_ref' not in fixturenames: + item.add_marker('no_vm') + + if item.get_closest_marker('multi_vms'): + # multi_vms implies small_vm + item.add_marker('small_vm') + +# fixtures + def setup_host(hostname_or_ip): pool = Pool(hostname_or_ip) h = pool.master @@ -404,37 +442,3 @@ def second_network(pytestconfig, host): if network_uuid == host.management_network(): pytest.fail("--second-network must NOT be the management network") return network_uuid - -def pytest_generate_tests(metafunc): - if "vm_ref" in metafunc.fixturenames: - vms = metafunc.config.getoption("vm") - if not vms: - vms = [None] # no --vm parameter does not mean skip the test, for us, it means use the default - metafunc.parametrize("vm_ref", vms, indirect=True, scope="module") - -def pytest_collection_modifyitems(items, config): - # Automatically mark tests based on fixtures they require. - # Check pytest.ini or pytest --markers for marker descriptions. - - markable_fixtures = [ - 'uefi_vm', - 'unix_vm', - 'windows_vm', - 'hostA2', - 'hostB1', - 'sr_disk', - 'sr_disk_4k' - ] - - for item in items: - fixturenames = getattr(item, 'fixturenames', ()) - for fixturename in markable_fixtures: - if fixturename in fixturenames: - item.add_marker(fixturename) - - if 'vm_ref' not in fixturenames: - item.add_marker('no_vm') - - if item.get_closest_marker('multi_vms'): - # multi_vms implies small_vm - item.add_marker('small_vm') From c66ddcfa63d3e86f3265b3ff38f6ea780409d4eb Mon Sep 17 00:00:00 2001 From: Yann Dirson Date: Fri, 19 Jul 2024 14:46:40 +0200 Subject: [PATCH 18/24] CACHE_IMPORTED_VM: parse only once We will need this value in other places. Signed-off-by: Yann Dirson --- conftest.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/conftest.py b/conftest.py index 2d3c0b5a6..0a65879b7 100644 --- a/conftest.py +++ b/conftest.py @@ -19,6 +19,13 @@ # need to import them in the global conftest.py so that they are recognized as fixtures. from pkgfixtures import formatted_and_mounted_ext4_disk, sr_disk_wiped +# Do we cache VMs? +try: + from data import CACHE_IMPORTED_VM +except ImportError: + CACHE_IMPORTED_VM = False +assert CACHE_IMPORTED_VM in [True, False] + # pytest hooks def pytest_addoption(parser): @@ -325,13 +332,6 @@ def vm_ref(request): @pytest.fixture(scope="module") def imported_vm(host, vm_ref): - # Do we cache VMs? - try: - from data import CACHE_IMPORTED_VM - except ImportError: - CACHE_IMPORTED_VM = False - assert CACHE_IMPORTED_VM in [True, False] - if is_uuid(vm_ref): vm_orig = VM(vm_ref, host) name = vm_orig.name() From a41cdbda62c3fa2909506507592c25f47a780271 Mon Sep 17 00:00:00 2001 From: Yann Dirson Date: Tue, 23 Jul 2024 11:11:28 +0200 Subject: [PATCH 19/24] BaseVM: add missing param_* methods And make the whole easier to copypaste. Signed-off-by: Yann Dirson --- lib/basevm.py | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/lib/basevm.py b/lib/basevm.py index 951c6c598..09ebb4660 100644 --- a/lib/basevm.py +++ b/lib/basevm.py @@ -2,7 +2,7 @@ import lib.commands as commands -from lib.common import _param_get, _param_remove, _param_set +from lib.common import _param_add, _param_clear, _param_get, _param_remove, _param_set from lib.sr import SR class BaseVM: @@ -16,13 +16,24 @@ def __init__(self, uuid, host): self.host = host def param_get(self, param_name, key=None, accept_unknown_key=False): - return _param_get(self.host, BaseVM.xe_prefix, self.uuid, param_name, key, accept_unknown_key) + return _param_get(self.host, self.xe_prefix, self.uuid, + param_name, key, accept_unknown_key) def param_set(self, param_name, value, key=None): - _param_set(self.host, BaseVM.xe_prefix, self.uuid, param_name, value, key) + _param_set(self.host, self.xe_prefix, self.uuid, + param_name, value, key) def param_remove(self, param_name, key, accept_unknown_key=False): - _param_remove(self.host, BaseVM.xe_prefix, self.uuid, param_name, key, accept_unknown_key) + _param_remove(self.host, self.xe_prefix, self.uuid, + param_name, key, accept_unknown_key) + + def param_add(self, param_name, value, key=None): + _param_add(self.host, self.xe_prefix, self.uuid, + param_name, value, key) + + def param_clear(self, param_name): + _param_clear(self.host, self.xe_prefix, self.uuid, + param_name) def name(self): return self.param_get('name-label') From 04008cd5d7639fd9231d81bc7265b30969690e50 Mon Sep 17 00:00:00 2001 From: Yann Dirson Date: Thu, 4 Jul 2024 18:19:59 +0200 Subject: [PATCH 20/24] vm: remove duplicate file_exists() Reported by mypy. Signed-off-by: Yann Dirson --- lib/vm.py | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/lib/vm.py b/lib/vm.py index 51752b65a..def89bf85 100644 --- a/lib/vm.py +++ b/lib/vm.py @@ -322,7 +322,7 @@ def tools_version(self): return "{major}.{minor}.{micro}-{build}".format(**version_dict) def file_exists(self, filepath): - """ Test that the file at filepath exists. """ + """Returns True if the file exists, otherwise returns False.""" return self.ssh_with_result(['test', '-f', filepath]).returncode == 0 def detect_package_manager(self): @@ -434,10 +434,6 @@ def clear_uefi_variables(self): """ self.param_remove('NVRAM', 'EFI-variables') - def file_exists(self, filepath): - """Returns True if the file exists, otherwise returns False.""" - return self.ssh_with_result(['test', '-f', filepath]).returncode == 0 - def sign_bins(self): for f in self.get_all_efi_bins(): self.sign(f) From 2fe3e5475289678ad0e99bbbb5474b94aedb3e94 Mon Sep 17 00:00:00 2001 From: Yann Dirson Date: Mon, 8 Jul 2024 16:06:08 +0200 Subject: [PATCH 21/24] Host.join_pool(): update the host's pool after new pool is joined Signed-off-by: Yann Dirson --- lib/host.py | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/host.py b/lib/host.py index 034b64398..474ac0cb4 100644 --- a/lib/host.py +++ b/lib/host.py @@ -520,6 +520,7 @@ def join_pool(self, pool): lambda: master.xe('host-param-get', {'uuid': self.uuid, 'param-name': 'enabled'}), f"Wait for pool {master} to see joined host {self} as enabled." ) + self.pool = pool def activate_smapi_driver(self, driver): sm_plugins = self.ssh(['grep', '[[:space:]]*sm-plugins[[:space:]]*=[[:space:]]*', XAPI_CONF_FILE]).splitlines() From b700f55290929ae690c707b34602a79f2fd63376 Mon Sep 17 00:00:00 2001 From: Yann Dirson Date: Thu, 25 Jul 2024 14:28:57 +0200 Subject: [PATCH 22/24] pytest: do not restrict timestamping to CLI We want to have it in log-file. Signed-off-by: Yann Dirson --- pytest.ini | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pytest.ini b/pytest.ini index ab6dc1e72..0895f1fbc 100644 --- a/pytest.ini +++ b/pytest.ini @@ -30,8 +30,8 @@ markers = quicktest: runs `quicktest`. log_cli = 1 log_cli_level = info -log_cli_format = %(asctime)s %(levelname)s %(message)s -log_cli_date_format = %b %d %H:%M:%S +log_format = %(asctime)s %(levelname)s %(message)s +log_date_format = %b %d %H:%M:%S filterwarnings = error ignore::DeprecationWarning From 4de1e3d5d1d7e09aa7b795908531fca56129af91 Mon Sep 17 00:00:00 2001 From: Yann Dirson Date: Thu, 25 Jul 2024 16:05:36 +0200 Subject: [PATCH 23/24] pytest: add milliseconds to log timestamp Signed-off-by: Yann Dirson --- pytest.ini | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pytest.ini b/pytest.ini index 0895f1fbc..8697a2a14 100644 --- a/pytest.ini +++ b/pytest.ini @@ -30,7 +30,7 @@ markers = quicktest: runs `quicktest`. log_cli = 1 log_cli_level = info -log_format = %(asctime)s %(levelname)s %(message)s +log_format = %(asctime)s.%(msecs)03d %(levelname)s %(message)s log_date_format = %b %d %H:%M:%S filterwarnings = error From 462ff237929a468c68e5dfac0d7f12d4327d645f Mon Sep 17 00:00:00 2001 From: Yann Dirson Date: Wed, 31 Jul 2024 16:52:16 +0200 Subject: [PATCH 24/24] LocalCommandFailed: fix ctor upcall Signed-off-by: Yann Dirson --- lib/commands.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/commands.py b/lib/commands.py index c5a54bc55..534be4087 100644 --- a/lib/commands.py +++ b/lib/commands.py @@ -26,7 +26,7 @@ def __init__(self, returncode, stdout, cmd): class LocalCommandFailed(BaseCommandFailed): def __init__(self, returncode, stdout, cmd): msg_end = f": {stdout}" if stdout else "." - super(SSHCommandFailed, self).__init__( + super(LocalCommandFailed, self).__init__( returncode, stdout, cmd, f'Local command ({cmd}) failed with return code {returncode}{msg_end}' )