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/conftest.py b/conftest.py index 1e6c0bfcf..0a65879b7 100644 --- a/conftest.py +++ b/conftest.py @@ -19,20 +19,14 @@ # 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 +# Do we cache VMs? +try: + from data import CACHE_IMPORTED_VM +except ImportError: + CACHE_IMPORTED_VM = False +assert CACHE_IMPORTED_VM in [True, False] -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 *** +# pytest hooks def pytest_addoption(parser): parser.addoption( @@ -85,6 +79,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 @@ -302,19 +332,12 @@ 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() 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 @@ -332,18 +355,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") - wait_for(vm.is_ssh_up, "> Wait for VM SSH up") + wait_for(vm.try_get_and_store_ip, "> Wait for VM IP", timeout_secs=5 * 60) 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 @@ -415,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') 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') diff --git a/lib/commands.py b/lib/commands.py index 841f52e38..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}' ) @@ -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: @@ -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, diff --git a/lib/host.py b/lib/host.py index 712c20532..474ac0cb4 100644 --- a/lib/host.py +++ b/lib/host.py @@ -200,21 +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 + vm = self.cached_vm(uri, sr_uuid) + if vm: + return vm params = {} msg = "Import VM %s" % uri @@ -227,7 +237,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) @@ -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 @@ -460,7 +470,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 @@ -480,6 +490,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): @@ -509,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() diff --git a/lib/vm.py b/lib/vm.py index f21c0d5c3..def89bf85 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") @@ -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): @@ -334,10 +334,15 @@ 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): + 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 unmount_guest_tools_iso(self): + 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 @@ -429,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) @@ -476,7 +477,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): diff --git a/pytest.ini b/pytest.ini index 1344531f3..8697a2a14 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 *** @@ -31,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.%(msecs)03d %(levelname)s %(message)s +log_date_format = %b %d %H:%M:%S filterwarnings = error ignore::DeprecationWarning 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 diff --git a/tests/guest-tools/unix/test_guest_tools_unix.py b/tests/guest-tools/unix/test_guest_tools_unix.py index 51664777f..4bb82bc05 100644 --- a/tests/guest-tools/unix/test_guest_tools_unix.py +++ b/tests/guest-tools/unix/test_guest_tools_unix.py @@ -9,13 +9,13 @@ # - 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): 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: @@ -33,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 @@ -62,7 +63,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 +81,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, diff --git a/tests/misc/test_basic_without_ssh.py b/tests/misc/test_basic_without_ssh.py index 52587df77..de2915619 100644 --- a/tests/misc/test_basic_without_ssh.py +++ b/tests/misc/test_basic_without_ssh.py @@ -9,15 +9,14 @@ # 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, # 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 +24,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 +107,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)