Drop --sr-device-config option

This option would have received the same "stop using a test parameter"
handling, but it turns out that:
- we cannot tell this is a config for e.g. NFS or CephFS, although they
  take different parameters
- all fixtures deriving from sr_device_config just take what has been
  given (which can not work at all when a test sessions includes more
  than one such SR stype)
- all verification is delegated to the final `ssh xe sr-create` call
- the need for overriding `data.py` with this flag seems very small indeed

This drastically simplifies the `*_device_config` fixtures, and
changes the variables in data.py should to drop their "DEFAULT_"
prefix, since they are now the sole source of configuration.

Signed-off-by: Yann Dirson <[email protected]>

Author: ydirson

conftest.py

@@ -47,13 +47,6 @@ def pytest_addoption(parser):
         default=[],
         help="VM key or OVA URL for tests that require only one VM",
     )
-    parser.addoption(
-        "--sr-device-config",
-        action="append",
-        default=[],
-        help="device-config keys and values for a remote SR. "
-             "Example: 'server:10.0.0.1,serverpath:/vms,nfsversion:4.1'.",
-    )
     parser.addoption(
         "--second-network",
         action="store",
@@ -377,21 +370,6 @@ def uefi_vm(imported_vm):
         pytest.skip('This test requires an UEFI VM')
     yield vm
 
-@pytest.fixture(scope='session')
-def sr_device_config(request):
-    raw_config = request.param
-
-    if raw_config is None:
-        # Use defaults
-        return None
-
-    config = {}
-    for key_val in raw_config.split(','):
-        key = key_val.split(':')[0]
-        value = key_val[key_val.index(':') + 1:]
-        config[key] = value
-    return config
-
 @pytest.fixture(scope='session')
 def additional_repos(request, hosts):
     if request.param is None:
@@ -444,13 +422,6 @@ def pytest_generate_tests(metafunc):
         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")
-    if "sr_device_config" in metafunc.fixturenames:
-        configs = metafunc.config.getoption("sr_device_config")
-        if not configs:
-            # No --sr-device-config parameter doesn't mean skip the test.
-            # For us it means use the defaults.
-            configs = [None]
-        metafunc.parametrize("sr_device_config", configs, indirect=True, scope="session")
 
 def pytest_collection_modifyitems(items, config):
     # Automatically mark tests based on fixtures they require.

data.py-dist

@@ -54,46 +54,55 @@ DEFAULT_SR = 'default'
 CACHE_IMPORTED_VM = False
 
 # Default NFS device config:
-DEFAULT_NFS_DEVICE_CONFIG = {
+NFS_DEVICE_CONFIG = {
 #    'server': '10.0.0.2', # URL/Hostname of NFS server
 #    'serverpath': '/path/to/shared/mount' # Path to shared mountpoint
 }
 
 # Default NFS4+ only device config:
-DEFAULT_NFS4_DEVICE_CONFIG = {
+NFS4_DEVICE_CONFIG = {
 #    'server': '10.0.0.2', # URL/Hostname of NFS server
 #    'serverpath': '/path_to_shared_mount' # Path to shared mountpoint
 #    'nfsversion': '4.1'
 }
 
 # Default NFS ISO device config:
-DEFAULT_NFS_ISO_DEVICE_CONFIG = {
+NFS_ISO_DEVICE_CONFIG = {
 #    'location': '10.0.0.2:/path/to/shared/mount' # URL/Hostname of NFS server and path to shared mountpoint
 }
 
 # Default CIFS ISO device config:
-DEFAULT_CIFS_ISO_DEVICE_CONFIG = {
+CIFS_ISO_DEVICE_CONFIG = {
 #    'location': r'\\10.0.0.2\<shared folder name>',
 #    'username': '<user>',
 #    'cifspassword': '<password>',
 #    'type': 'cifs',
 #    'vers': '<1.0> or <3.0>'
 }
 
-DEFAULT_CEPHFS_DEVICE_CONFIG = {
+CEPHFS_DEVICE_CONFIG = {
 #    'server': '10.0.0.2',
 #    'serverpath': '/vms'
 }
 
-DEFAULT_MOOSEFS_DEVICE_CONFIG = {
+MOOSEFS_DEVICE_CONFIG = {
 #    'masterhost': 'mfsmaster',
 #    'masterport': '9421',
 #    'rootpath': '/vms'
 }
 
-DEFAULT_LVMOISCSI_DEVICE_CONFIG = {
+LVMOISCSI_DEVICE_CONFIG = {
 #    'target': '192.168.1.1',
 #    'port': '3260',
 #    'targetIQN': 'target.example',
 #    'SCSIid': 'id'
 }
+
+# compatibility settings for older tests
+DEFAULT_NFS_DEVICE_CONFIG = NFS_DEVICE_CONFIG
+DEFAULT_NFS4_DEVICE_CONFIG = NFS4_DEVICE_CONFIG
+DEFAULT_NFS_ISO_DEVICE_CONFIG = NFS_ISO_DEVICE_CONFIG
+DEFAULT_CIFS_ISO_DEVICE_CONFIG = CIFS_ISO_DEVICE_CONFIG
+DEFAULT_CEPHFS_DEVICE_CONFIG = CEPHFS_DEVICE_CONFIG
+DEFAULT_MOOSEFS_DEVICE_CONFIG = MOOSEFS_DEVICE_CONFIG
+DEFAULT_LVMOISCSI_DEVICE_CONFIG = LVMOISCSI_DEVICE_CONFIG

lib/config.py

@@ -1,2 +1,6 @@
 ignore_ssh_banner = False
 ssh_output_max_lines = 20
+
+def sr_device_config(datakey):
+    import data # import here to avoid depending on this user file for collecting tests
+    return getattr(data, datakey)

tests/storage/cephfs/conftest.py

@@ -2,6 +2,7 @@
 import pytest
 
 from lib.common import exec_nofail, raise_errors
+from lib import config
 
 # explicit import for package-scope fixtures
 from pkgfixtures import pool_with_saved_yum_state
@@ -22,21 +23,8 @@ def pool_with_ceph(pool_without_ceph, pool_with_saved_yum_state):
     yield pool
 
 @pytest.fixture(scope='package')
-def cephfs_device_config(sr_device_config):
-    if sr_device_config is not None:
-        # SR device config from CLI param
-        config = sr_device_config
-    else:
-        # SR device config from data.py defaults
-        try:
-            from data import DEFAULT_CEPHFS_DEVICE_CONFIG
-        except ImportError:
-            DEFAULT_CEPHFS_DEVICE_CONFIG = {}
-        if DEFAULT_CEPHFS_DEVICE_CONFIG:
-            config = DEFAULT_CEPHFS_DEVICE_CONFIG
-        else:
-            raise Exception("No default CephFS device-config found, neither in CLI nor in data.py defaults")
-    return config
+def cephfs_device_config():
+    return config.sr_device_config("CEPHFS_DEVICE_CONFIG")
 
 @pytest.fixture(scope='package')
 def cephfs_sr(host, cephfs_device_config, pool_with_ceph):

tests/storage/iso/conftest.py

@@ -2,6 +2,8 @@
 import time
 import os
 
+from lib import config
+
 # Explicitly import package-scoped fixtures (see explanation in pkgfixtures.py)
 from pkgfixtures import formatted_and_mounted_ext4_disk
 
@@ -23,38 +25,12 @@ def local_iso_sr(host, formatted_and_mounted_ext4_disk):
     sr.destroy()
 
 @pytest.fixture(scope='module')
-def nfs_iso_device_config(sr_device_config):
-    if sr_device_config is not None:
-        # SR device config from CLI param
-        config = sr_device_config
-    else:
-        # SR device config from data.py defaults
-        try:
-            from data import DEFAULT_NFS_ISO_DEVICE_CONFIG
-        except ImportError:
-            DEFAULT_NFS_ISO_DEVICE_CONFIG = {}
-        if DEFAULT_NFS_ISO_DEVICE_CONFIG:
-            config = DEFAULT_NFS_ISO_DEVICE_CONFIG
-        else:
-            raise Exception("No default NFS ISO device-config found, neither in CLI nor in data.py defaults")
-    return config
+def nfs_iso_device_config():
+    return config.sr_device_config("NFS_ISO_DEVICE_CONFIG")
 
 @pytest.fixture(scope='module')
-def cifs_iso_device_config(sr_device_config):
-    if sr_device_config is not None:
-        # SR device config from CLI param
-        config = sr_device_config
-    else:
-        # SR device config from data.py defaults
-        try:
-            from data import DEFAULT_CIFS_ISO_DEVICE_CONFIG
-        except ImportError:
-            DEFAULT_CIFS_ISO_DEVICE_CONFIG = {}
-        if DEFAULT_CIFS_ISO_DEVICE_CONFIG:
-            config = DEFAULT_CIFS_ISO_DEVICE_CONFIG
-        else:
-            raise Exception("No default CIFS ISO device-config found, neither in CLI nor in data.py defaults")
-    return config
+def cifs_iso_device_config():
+    return config.sr_device_config("CIFS_ISO_DEVICE_CONFIG")
 
 @pytest.fixture(scope='module')
 def nfs_iso_sr(host, nfs_iso_device_config):

tests/storage/lvmoiscsi/conftest.py

@@ -1,22 +1,11 @@
 import logging
 import pytest
 
+from lib import config
+
 @pytest.fixture(scope='package')
-def lvmoiscsi_device_config(sr_device_config):
-    if sr_device_config is not None:
-        # SR device config from CLI param
-        config = sr_device_config
-    else:
-        # SR device config from data.py defaults
-        try:
-            from data import DEFAULT_LVMOISCSI_DEVICE_CONFIG
-        except ImportError:
-            DEFAULT_LVMOISCSI_DEVICE_CONFIG = {}
-        if DEFAULT_LVMOISCSI_DEVICE_CONFIG:
-            config = DEFAULT_LVMOISCSI_DEVICE_CONFIG
-        else:
-            raise Exception("No default lvmoiscsi device-config found, neither in CLI nor in data.py defaults")
-    return config
+def lvmoiscsi_device_config():
+    return config.sr_device_config("LVMOISCSI_DEVICE_CONFIG")
 
 @pytest.fixture(scope='package')
 def lvmoiscsi_sr(host, lvmoiscsi_device_config):

tests/storage/moosefs/conftest.py

@@ -1,6 +1,8 @@
 import logging
 import pytest
 
+from lib import config
+
 # explicit import for package-scope fixtures
 from pkgfixtures import pool_with_saved_yum_state
 
@@ -35,21 +37,8 @@ def pool_with_moosefs_enabled(pool_with_moosefs_installed):
     pool.exec_on_hosts_on_error_continue(disable_moosefs)
 
 @pytest.fixture(scope='package')
-def moosefs_device_config(sr_device_config):
-    if sr_device_config is not None:
-        # SR device config from CLI param
-        config = sr_device_config
-    else:
-        # SR device config from data.py defaults
-        try:
-            from data import DEFAULT_MOOSEFS_DEVICE_CONFIG
-        except ImportError:
-            DEFAULT_MOOSEFS_DEVICE_CONFIG = {}
-        if DEFAULT_MOOSEFS_DEVICE_CONFIG:
-            config = DEFAULT_MOOSEFS_DEVICE_CONFIG
-        else:
-            raise Exception("No default MooseFS device-config found, neither in CLI nor in data.py defaults")
-    return config
+def moosefs_device_config():
+    return config.sr_device_config("MOOSEFS_DEVICE_CONFIG")
 
 @pytest.fixture(scope='package')
 def moosefs_sr(moosefs_device_config, pool_with_moosefs_enabled):

tests/storage/nfs/conftest.py

@@ -1,6 +1,8 @@
 import logging
 import pytest
 
+from lib import config
+
 # --- Dispatch fixture for NFS versions ----------------------------------------
 
 @pytest.fixture
@@ -10,21 +12,8 @@ def dispatch_nfs(request):
 # --- NFS3 fixtures ------------------------------------------------------------
 
 @pytest.fixture(scope='package')
-def nfs_device_config(sr_device_config):
-    if sr_device_config is not None:
-        # SR device config from CLI param
-        config = sr_device_config
-    else:
-        # SR device config from data.py defaults
-        try:
-            from data import DEFAULT_NFS_DEVICE_CONFIG
-        except ImportError:
-            DEFAULT_NFS_DEVICE_CONFIG = {}
-        if DEFAULT_NFS_DEVICE_CONFIG:
-            config = DEFAULT_NFS_DEVICE_CONFIG
-        else:
-            raise Exception("No default NFS device-config found, neither in CLI nor in data.py defaults")
-    return config
+def nfs_device_config():
+    return config.sr_device_config("NFS_DEVICE_CONFIG")
 
 @pytest.fixture(scope='package')
 def nfs_sr(host, nfs_device_config):
@@ -51,21 +40,8 @@ def vm_on_nfs_sr(host, nfs_sr, vm_ref):
 # --- NFS4+ only fixtures ------------------------------------------------------
 
 @pytest.fixture(scope='package')
-def nfs4_device_config(sr_device_config):
-    if sr_device_config is not None:
-        # SR device config from CLI param
-        config = sr_device_config
-    else:
-        # SR device config from data.py defaults
-        try:
-            from data import DEFAULT_NFS4_DEVICE_CONFIG
-        except ImportError:
-            DEFAULT_NFS4_DEVICE_CONFIG = {}
-        if DEFAULT_NFS4_DEVICE_CONFIG:
-            config = DEFAULT_NFS4_DEVICE_CONFIG
-        else:
-            raise Exception("No default NFS4+ device-config found, neither in CLI nor in data.py defaults")
-    return config
+def nfs4_device_config():
+    return config.sr_device_config("NFS4_DEVICE_CONFIG")
 
 @pytest.fixture(scope='package')
 def nfs4_sr(host, nfs4_device_config):

tests/storage/nfs/test_nfs_sr.py

@@ -5,8 +5,6 @@
 # Requirements:
 # - one XCP-ng host >= 8.0 with an additional unused disk for the SR
 
-# Make sure this fixture is called before the parametrized one
-@pytest.mark.usefixtures('sr_device_config')
 class TestNFSSRCreateDestroy:
     @pytest.mark.parametrize('dispatch_nfs', ['nfs_device_config', 'nfs4_device_config'], indirect=True)
     def test_create_and_destroy_sr(self, host, dispatch_nfs):
@@ -19,8 +17,6 @@ def test_create_and_destroy_sr(self, host, dispatch_nfs):
         vm.destroy(verify=True)
         sr.destroy(verify=True)
 
-# Make sure these fixtures are called before the parametrized one
-@pytest.mark.usefixtures('sr_device_config')
 class TestNFSSR:
     @pytest.mark.quicktest
     @pytest.mark.parametrize('dispatch_nfs', ['nfs_sr', 'nfs4_sr'], indirect=True)

tests/storage/nfs/test_nfs_sr_crosspool_migration.py

@@ -12,7 +12,7 @@
 @pytest.mark.big_vm # and ideally with a big VM to test it scales
 @pytest.mark.usefixtures("hostB1", "local_sr_on_hostB1")
 # Make sure these fixtures are called before the parametrized one
-@pytest.mark.usefixtures('sr_device_config', 'vm_ref')
+@pytest.mark.usefixtures('vm_ref')
 class Test:
     @pytest.mark.parametrize('dispatch_nfs', ['vm_on_nfs_sr', 'vm_on_nfs4_sr'], indirect=True)
     def test_cold_crosspool_migration(self, host, hostB1, dispatch_nfs, local_sr_on_hostB1):