From 7290c51408c1582c220f17923542b8f0fb89cfa8 Mon Sep 17 00:00:00 2001
From: Albert Esteve <aesteve@redhat.com>
Date: Fri, 1 Jul 2022 13:30:33 +0200
Subject: [PATCH 1/5] storage: mark root all tests that req root

Mark all storage tests that require root with
the root label so that they get correctly picked
or excluded up by the CI jobs.

Signed-off-by: Albert Esteve <aesteve@redhat.com>
---
 tests/storage/blockdev_test.py      |  4 ++++
 tests/storage/blocksd_test.py       |  3 +++
 tests/storage/devicemapper_test.py  | 10 ++++++++++
 tests/storage/dmsetup_test.py       |  1 +
 tests/storage/fileutil_test.py      |  2 ++
 tests/storage/iscsiadm_test.py      |  3 +++
 tests/storage/loopback_test.py      |  3 +++
 tests/storage/lvmfilter_test.py     |  2 ++
 tests/storage/managedvolume_test.py | 16 ++++++++++++++++
 tests/storage/misc_test.py          |  1 +
 tests/storage/mount_test.py         |  2 ++
 tests/storage/multipath_test.py     |  5 +++++
 tests/storage/outofprocess_test.py  |  2 ++
 tests/storage/qemuimg_test.py       |  1 +
 14 files changed, 55 insertions(+)

diff --git a/tests/storage/blockdev_test.py b/tests/storage/blockdev_test.py
index 2b3b9ae648..4afee7f6ae 100644
--- a/tests/storage/blockdev_test.py
+++ b/tests/storage/blockdev_test.py
@@ -59,6 +59,7 @@ def loop_device(tmpdir):
 class TestZero:
 
     @requires_root
+    @pytest.mark.root
     def test_entire_device(self, loop_device):
         # Write some data to the device.
         with directio.open(loop_device.path, "r+") as f:
@@ -78,6 +79,7 @@ def test_entire_device(self, loop_device):
             assert data == ZERO
 
     @requires_root
+    @pytest.mark.root
     def test_size(self, loop_device):
         # Write some data to the device.
         with directio.open(loop_device.path, "r+") as f:
@@ -94,6 +96,7 @@ def test_size(self, loop_device):
             assert data == DATA
 
     @requires_root
+    @pytest.mark.root
     @pytest.mark.parametrize("size", [
         sc.BLOCK_SIZE_4K,
         250 * sc.BLOCK_SIZE_4K
@@ -154,6 +157,7 @@ def test_not_supported(self, tmpdir):
             assert data == DATA
 
     @requires_root
+    @pytest.mark.root
     def test_supported(self, loop_device):
         # If the loop device backing file is on a file system that does not
         # support discard, discard is not supported.
diff --git a/tests/storage/blocksd_test.py b/tests/storage/blocksd_test.py
index e197349b8d..86791ed1a6 100644
--- a/tests/storage/blocksd_test.py
+++ b/tests/storage/blocksd_test.py
@@ -1616,6 +1616,7 @@ def bad_vol_size():
 
 
 @requires_root
+@pytest.mark.root
 @pytest.mark.parametrize("domain_version", [5])
 def test_create_illegal_volume(domain_factory, domain_version, fake_task,
                                fake_sanlock):
@@ -1644,6 +1645,7 @@ def test_create_illegal_volume(domain_factory, domain_version, fake_task,
 
 
 @requires_root
+@pytest.mark.root
 def test_reduce_volume_called(domain_factory, fake_task, fake_sanlock):
     sd_uuid = str(uuid.uuid4())
     dom = domain_factory.create_domain(sd_uuid=sd_uuid, version=5)
@@ -1676,6 +1678,7 @@ def test_reduce_volume_called(domain_factory, fake_task, fake_sanlock):
 
 
 @requires_root
+@pytest.mark.root
 def test_reduce_volume_skipped(domain_factory, fake_task, fake_sanlock):
     sd_uuid = str(uuid.uuid4())
     dom = domain_factory.create_domain(sd_uuid=sd_uuid, version=5)
diff --git a/tests/storage/devicemapper_test.py b/tests/storage/devicemapper_test.py
index 70c3b76976..9c85ebbf41 100644
--- a/tests/storage/devicemapper_test.py
+++ b/tests/storage/devicemapper_test.py
@@ -145,6 +145,7 @@ def test_get_paths_status_no_device(fake_dmsetup_status):
 
 @broken_on_ci
 @requires_root
+@pytest.mark.root
 def test_remove_mapping(zero_dm_device):
     device_path = "{}{}".format(DMPATH_PREFIX, zero_dm_device)
     assert os.path.exists(device_path)
@@ -155,6 +156,7 @@ def test_remove_mapping(zero_dm_device):
 
 @broken_on_ci
 @requires_root
+@pytest.mark.root
 def test_dm_id(zero_dm_device):
     # Resolve the dm link and get dm name of the device.
     device_path = "{}{}".format(DMPATH_PREFIX, zero_dm_device)
@@ -176,6 +178,7 @@ def test_dm_id(zero_dm_device):
 
 @broken_on_ci
 @requires_root
+@pytest.mark.root
 def test_dev_name(zero_dm_device):
     dm_id = devicemapper.getDmId(zero_dm_device)
     device_name = devicemapper.getDevName(dm_id)
@@ -184,6 +187,7 @@ def test_dev_name(zero_dm_device):
 
 @broken_on_ci
 @requires_root
+@pytest.mark.root
 def test_is_virtual_device(zero_dm_device):
     dm_id = devicemapper.getDmId(zero_dm_device)
     assert devicemapper.isVirtualDevice(dm_id)
@@ -191,6 +195,7 @@ def test_is_virtual_device(zero_dm_device):
 
 @broken_on_ci
 @requires_root
+@pytest.mark.root
 def test_is_block_device(zero_dm_device):
     dm_id = devicemapper.getDmId(zero_dm_device)
     assert devicemapper.isBlockDevice(dm_id)
@@ -198,6 +203,7 @@ def test_is_block_device(zero_dm_device):
 
 @broken_on_ci
 @requires_root
+@pytest.mark.root
 def test_is_dm_device(zero_dm_device):
     dm_id = devicemapper.getDmId(zero_dm_device)
     assert devicemapper.isDmDevice(dm_id)
@@ -205,6 +211,7 @@ def test_is_dm_device(zero_dm_device):
 
 @broken_on_ci
 @requires_root
+@pytest.mark.root
 def test_get_all_mapped_devices(zero_dm_device):
     devices = devicemapper.getAllMappedDevices()
     assert zero_dm_device in devices
@@ -212,6 +219,7 @@ def test_get_all_mapped_devices(zero_dm_device):
 
 @broken_on_ci
 @requires_root
+@pytest.mark.root
 def test_get_all_slaves(zero_dm_device):
     slaves = devicemapper.getAllSlaves()
     assert zero_dm_device in slaves
@@ -221,6 +229,7 @@ def test_get_all_slaves(zero_dm_device):
 
 @broken_on_ci
 @requires_root
+@pytest.mark.root
 def test_get_slaves(zero_dm_device):
     slaves = devicemapper.getSlaves(zero_dm_device)
     # Zero device mapping has no slaves.
@@ -229,6 +238,7 @@ def test_get_slaves(zero_dm_device):
 
 @broken_on_ci
 @requires_root
+@pytest.mark.root
 def test_get_holders(zero_dm_device):
     holders = devicemapper.getHolders(zero_dm_device)
     # Zero device mapping has no holders.
diff --git a/tests/storage/dmsetup_test.py b/tests/storage/dmsetup_test.py
index e9ee0e2f3d..3564e8ea00 100644
--- a/tests/storage/dmsetup_test.py
+++ b/tests/storage/dmsetup_test.py
@@ -70,6 +70,7 @@ def fake_dmsetup(monkeypatch, fake_executable):
 
 
 @requires_root
+@pytest.mark.root
 def test_status(fake_dmsetup):
     fake_dmsetup.write(DMSETUP_SCRIPT.format(FAKE_DMSETUP_OUTPUT))
 
diff --git a/tests/storage/fileutil_test.py b/tests/storage/fileutil_test.py
index 442f5c8b2c..ed120a6f48 100644
--- a/tests/storage/fileutil_test.py
+++ b/tests/storage/fileutil_test.py
@@ -156,6 +156,7 @@ def test_createdir_file_exists_no_mode():
 
 
 @requires_root
+@pytest.mark.root
 def test_chown():
     targetId = 666
     with temporaryPath() as srcPath:
@@ -165,6 +166,7 @@ def test_chown():
 
 
 @requires_root
+@pytest.mark.root
 def test_chown_names():
     # I convert to some id because I have no
     # idea what users are defined and what
diff --git a/tests/storage/iscsiadm_test.py b/tests/storage/iscsiadm_test.py
index fc026b3cf5..849361c3b7 100644
--- a/tests/storage/iscsiadm_test.py
+++ b/tests/storage/iscsiadm_test.py
@@ -22,12 +22,15 @@
 
 import six
 
+import pytest
+
 from vdsm.storage import iscsiadm
 
 from . marks import requires_root
 
 
 @requires_root
+@pytest.mark.root
 def test_run_cmd():
     out = iscsiadm.run_cmd(["--version"])
     assert isinstance(out, six.text_type)
diff --git a/tests/storage/loopback_test.py b/tests/storage/loopback_test.py
index e3b78d9984..85d47af4b5 100644
--- a/tests/storage/loopback_test.py
+++ b/tests/storage/loopback_test.py
@@ -41,6 +41,7 @@
 
 
 @requires_root
+@pytest.mark.root
 @pytest.mark.parametrize("sector_size", [
     None,
     pytest.param(sc.BLOCK_SIZE_512, marks=requires_loopback_sector_size),
@@ -67,6 +68,7 @@ def test_with_device(tmpdir, sector_size):
 
 
 @requires_root
+@pytest.mark.root
 def test_attach_detach_manually(tmpdir):
     filename = str(tmpdir.join("file"))
     prepare_backing_file(filename)
@@ -82,6 +84,7 @@ def test_attach_detach_manually(tmpdir):
 
 
 @requires_root
+@pytest.mark.root
 @pytest.mark.stress
 def test_many_devices(tmpdir):
     filename = str(tmpdir.join("file"))
diff --git a/tests/storage/lvmfilter_test.py b/tests/storage/lvmfilter_test.py
index 255dbd26a5..f03de9a49e 100644
--- a/tests/storage/lvmfilter_test.py
+++ b/tests/storage/lvmfilter_test.py
@@ -162,6 +162,7 @@ def test_format_option():
 
 
 @requires_root
+@pytest.mark.root
 def test_real_find_lvm_mounts():
     mounts = lvmfilter.find_lvm_mounts()
     # This will return different results on any host, but we expect to find a
@@ -172,6 +173,7 @@ def test_real_find_lvm_mounts():
 
 
 @requires_root
+@pytest.mark.root
 def test_real_build_filter():
     mounts = lvmfilter.find_lvm_mounts()
     lvm_filter = lvmfilter.build_filter(mounts)
diff --git a/tests/storage/managedvolume_test.py b/tests/storage/managedvolume_test.py
index 5598344e72..efb3fb69d2 100644
--- a/tests/storage/managedvolume_test.py
+++ b/tests/storage/managedvolume_test.py
@@ -122,6 +122,7 @@ def remove_managed_udev_rule(self, sd_id, vol_id):
 
 
 @requires_root
+@pytest.mark.root
 def test_connector_info_not_installed(monkeypatch):
     # Simulate missing os_brick.
     monkeypatch.setattr(managedvolume, "os_brick", None)
@@ -130,12 +131,14 @@ def test_connector_info_not_installed(monkeypatch):
 
 
 @requires_root
+@pytest.mark.root
 def test_connector_info_ok(monkeypatch, fake_os_brick):
     monkeypatch.setenv("FAKE_CONNECTOR_INFO_RESULT", "OK")
     assert managedvolume.connector_info() == {"multipath": True}
 
 
 @requires_root
+@pytest.mark.root
 def test_connector_info_fail(monkeypatch, fake_os_brick):
     monkeypatch.setenv("FAKE_CONNECTOR_INFO_RESULT", "FAIL")
     with pytest.raises(se.ManagedVolumeHelperFailed):
@@ -143,6 +146,7 @@ def test_connector_info_fail(monkeypatch, fake_os_brick):
 
 
 @requires_root
+@pytest.mark.root
 def test_connector_info_fail_json(monkeypatch, fake_os_brick):
     monkeypatch.setenv("FAKE_CONNECTOR_INFO_RESULT", "FAIL_JSON")
     with pytest.raises(se.ManagedVolumeHelperFailed):
@@ -150,6 +154,7 @@ def test_connector_info_fail_json(monkeypatch, fake_os_brick):
 
 
 @requires_root
+@pytest.mark.root
 def test_connector_info_raise(monkeypatch, fake_os_brick):
     monkeypatch.setenv("FAKE_CONNECTOR_INFO_RESULT", "RAISE")
     with pytest.raises(se.ManagedVolumeHelperFailed) as e:
@@ -158,6 +163,7 @@ def test_connector_info_raise(monkeypatch, fake_os_brick):
 
 
 @requires_root
+@pytest.mark.root
 def test_attach_volume_not_installed_attach(monkeypatch):
     # Simulate missing os_brick.
     monkeypatch.setattr(managedvolume, "os_brick", None)
@@ -166,6 +172,7 @@ def test_attach_volume_not_installed_attach(monkeypatch):
 
 
 @requires_root
+@pytest.mark.root
 def test_attach_volume_ok_iscsi(monkeypatch, fake_os_brick, tmp_db, fake_lvm,
                                 fake_supervdsm):
     monkeypatch.setenv("FAKE_ATTACH_RESULT", "OK")
@@ -206,6 +213,7 @@ def test_attach_volume_ok_iscsi(monkeypatch, fake_os_brick, tmp_db, fake_lvm,
 
 
 @requires_root
+@pytest.mark.root
 @pytest.mark.xfail(reason='RBD monkeypatching not implemented yet')
 def test_attach_volume_ok_rbd(monkeypatch, fake_os_brick, tmp_db, fake_lvm,
                               fake_supervdsm):
@@ -244,6 +252,7 @@ def test_attach_volume_ok_rbd(monkeypatch, fake_os_brick, tmp_db, fake_lvm,
 
 
 @requires_root
+@pytest.mark.root
 @pytest.mark.parametrize("vol_type", ["iscsi", "fibre_channel"])
 def test_attach_volume_no_multipath_id(monkeypatch, fake_os_brick, tmp_db,
                                        vol_type, fake_lvm, fake_supervdsm):
@@ -273,6 +282,7 @@ def test_attach_volume_no_multipath_id(monkeypatch, fake_os_brick, tmp_db,
 
 
 @requires_root
+@pytest.mark.root
 def test_reattach_volume_ok_iscsi(monkeypatch, fake_os_brick, tmpdir, tmp_db,
                                   fake_lvm, fake_supervdsm):
     monkeypatch.setenv("FAKE_ATTACH_RESULT", "OK")
@@ -305,6 +315,7 @@ def test_reattach_volume_ok_iscsi(monkeypatch, fake_os_brick, tmpdir, tmp_db,
 
 
 @requires_root
+@pytest.mark.root
 def test_attach_volume_fail_update(monkeypatch, fake_os_brick, tmpdir, tmp_db,
                                    fake_lvm, fake_supervdsm):
     monkeypatch.setenv("FAKE_ATTACH_RESULT", "OK")
@@ -336,6 +347,7 @@ def raise_error(*args, **kargs):
 
 
 @requires_root
+@pytest.mark.root
 def test_reattach_volume_other_connection(monkeypatch, fake_os_brick, tmp_db,
                                           fake_lvm, fake_supervdsm):
     monkeypatch.setenv("FAKE_ATTACH_RESULT", "OK")
@@ -368,6 +380,7 @@ def test_reattach_volume_other_connection(monkeypatch, fake_os_brick, tmp_db,
 
 
 @requires_root
+@pytest.mark.root
 def test_detach_volume_iscsi_not_attached(monkeypatch, fake_os_brick, tmp_db,
                                           fake_lvm, fake_supervdsm):
     monkeypatch.setenv("FAKE_ATTACH_RESULT", "OK")
@@ -394,6 +407,7 @@ def test_detach_volume_iscsi_not_attached(monkeypatch, fake_os_brick, tmp_db,
 
 
 @requires_root
+@pytest.mark.root
 def test_detach_volume_not_installed(monkeypatch, fake_os_brick, tmp_db,
                                      fake_lvm):
     # Simulate missing os_brick.
@@ -410,6 +424,7 @@ def test_detach_volume_not_installed(monkeypatch, fake_os_brick, tmp_db,
 
 
 @requires_root
+@pytest.mark.root
 def test_detach_not_in_db(monkeypatch, fake_os_brick, tmp_db, fake_lvm,
                           fake_supervdsm):
     managedvolume.detach_volume("sd_id", "fake_vol_id")
@@ -426,6 +441,7 @@ def test_detach_not_in_db(monkeypatch, fake_os_brick, tmp_db, fake_lvm,
 
 
 @requires_root
+@pytest.mark.root
 def test_detach_volume_iscsi_attached(monkeypatch, fake_os_brick, tmpdir,
                                       tmp_db, fake_lvm, fake_supervdsm):
     monkeypatch.setenv("FAKE_ATTACH_RESULT", "OK")
diff --git a/tests/storage/misc_test.py b/tests/storage/misc_test.py
index 9c639c6b62..2fd88a8bd5 100644
--- a/tests/storage/misc_test.py
+++ b/tests/storage/misc_test.py
@@ -586,6 +586,7 @@ def testStdErr(self):
         self.assertEqual(stderr[0].decode("ascii"), "it works!")
 
     @requires_root
+    @pytest.mark.root
     def testSudo(self):
         """
         Tests that when running with sudo the user really is root (or other
diff --git a/tests/storage/mount_test.py b/tests/storage/mount_test.py
index 2df1f0d7d6..ab1bd78cf1 100644
--- a/tests/storage/mount_test.py
+++ b/tests/storage/mount_test.py
@@ -137,6 +137,7 @@ def loop_mount(m):
 class TestMount(VdsmTestCase):
 
     @requires_root
+    @pytest.mark.root
     @broken_on_ci("mount check fails after successful mount", name="TRAVIS_CI")
     def testLoopMount(self):
         with namedTemporaryDir() as mpath:
@@ -147,6 +148,7 @@ def testLoopMount(self):
                     self.assertTrue(m.isMounted())
 
     @requires_root
+    @pytest.mark.root
     @broken_on_ci("mount check fails after successful mount", name="TRAVIS_CI")
     def testSymlinkMount(self):
         with namedTemporaryDir() as root_dir:
diff --git a/tests/storage/multipath_test.py b/tests/storage/multipath_test.py
index 35fcb65de8..2867ca07c5 100644
--- a/tests/storage/multipath_test.py
+++ b/tests/storage/multipath_test.py
@@ -116,12 +116,14 @@ def fake_scsi_id(monkeypatch, fake_executable):
 
 
 @requires_root
+@pytest.mark.root
 def test_resize_map(fake_multipathd):
     fake_multipathd.write(MULTIPATHD_SCRIPT.format("ok"))
     multipath.resize_map("fake_device")
 
 
 @requires_root
+@pytest.mark.root
 def test_resize_map_failed(fake_multipathd):
     fake_multipathd.write(MULTIPATHD_SCRIPT.format("fail"))
 
@@ -130,6 +132,7 @@ def test_resize_map_failed(fake_multipathd):
 
 
 @requires_root
+@pytest.mark.root
 def test_scsi_id(fake_scsi_id):
     fake_scsi_id.write(SCSI_ID_SCRIPT.format(FAKE_SCSI_ID_OUTPUT))
 
@@ -138,6 +141,7 @@ def test_scsi_id(fake_scsi_id):
 
 
 @requires_root
+@pytest.mark.root
 def test_scsi_id_no_serial(fake_scsi_id):
     fake_scsi_id.write(SCSI_ID_SCRIPT.format(FAKE_SCSI_ID_NO_SERIAL))
 
@@ -151,6 +155,7 @@ def test_scsi_id_no_serial(fake_scsi_id):
 
 
 @requires_root
+@pytest.mark.root
 def test_scsi_id_fails(fake_scsi_id):
     fake_scsi_id.write("#!/bin/sh\nexit 1\n")
 
diff --git a/tests/storage/outofprocess_test.py b/tests/storage/outofprocess_test.py
index d5e4688d02..70ad7dce01 100644
--- a/tests/storage/outofprocess_test.py
+++ b/tests/storage/outofprocess_test.py
@@ -521,6 +521,7 @@ def test_fileutils_pathexists(oop_cleanup, tmpdir):
 
 
 @requires_root
+@pytest.mark.root
 def test_fileutils_validateqemureadable_other_group(oop_cleanup, tmpdir):
     iop = oop.getProcessPool("test")
 
@@ -544,6 +545,7 @@ def test_fileutils_validateqemureadable_other_group(oop_cleanup, tmpdir):
     (36, 0o750),    # kvm
 ])
 @requires_root
+@pytest.mark.root
 def test_fileutils_validateqemureadable_qemu_or_kvm_group(
         oop_cleanup, tmpdir, gid, mode):
     iop = oop.getProcessPool("test")
diff --git a/tests/storage/qemuimg_test.py b/tests/storage/qemuimg_test.py
index 4beca9ae4d..f28f6d2fa5 100644
--- a/tests/storage/qemuimg_test.py
+++ b/tests/storage/qemuimg_test.py
@@ -1663,6 +1663,7 @@ def test_full(self, tmpdir, compat, format, compressed):
         self.check_measure(filename, compat, format, compressed)
 
     @requires_root
+    @pytest.mark.root
     def test_block(self, block_chain):
         # Creating a block chain is very slow, so we reuse it for many tests.
 

From 2fd4212344360c60a8f50350e89538bb29296724 Mon Sep 17 00:00:00 2001
From: Albert Esteve <aesteve@redhat.com>
Date: Fri, 8 Jul 2022 09:19:33 +0200
Subject: [PATCH 2/5] tox.ini: add user and root storage envs

Add tox environments for storage user and root
tests (i.e., storage-user and storage-root, respectively).
These environments are a subset of the storage
environment, that is split considering the root mark.

This way, we can easily run tests marked as
as a root user, and tests marked as non-root,
separately.

The new environments have their own different
target coverage. The 'storage-root' environment
sets a different coverage file and folder to avoid
overwriting other environment's coverage reports
with root ownership.

Signed-off-by: Albert Esteve <aesteve@redhat.com>
---
 tox.ini | 30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/tox.ini b/tox.ini
index 63fed53746..a2ea7861f1 100644
--- a/tox.ini
+++ b/tox.ini
@@ -97,6 +97,36 @@ commands =
     --cov-fail-under={env:STORAGE_COVERAGE:68} \
     {posargs:storage}
 
+[testenv:storage-user]
+passenv = {[base]passenv}
+setenv =
+    {[base]setenv}
+    COVERAGE_FILE=.coverage-storage-user
+deps = {[base]deps}
+changedir = {[base]changedir}
+commands =
+    python profile {envname} pytest -m {[base]markers}" and not root" \
+    --durations=20 \
+    --cov=vdsm.storage \
+    --cov-report=html:htmlcov-storage-user \
+    --cov-fail-under={env:STORAGE_USER_COVERAGE:61} \
+    {posargs:storage}
+
+[testenv:storage-root]
+passenv = {[base]passenv}
+setenv =
+    {[base]setenv}
+    COVERAGE_FILE=.coverage-storage-root
+deps = {[base]deps}
+changedir = {[base]changedir}
+commands =
+    python profile {envname} pytest -m {[base]markers}" and root" \
+    --durations=20 \
+    --cov=vdsm.storage \
+    --cov-report=html:htmlcov-storage-root \
+    --cov-fail-under={env:STORAGE_ROOT_COVERAGE:47} \
+    {posargs:storage}
+
 [testenv:virt]
 passenv = {[base]passenv}
 setenv =

From 7cfa475473934a505a7515f4f0efc35a2e1b08ea Mon Sep 17 00:00:00 2001
From: Albert Esteve <aesteve@redhat.com>
Date: Fri, 8 Jul 2022 09:37:18 +0200
Subject: [PATCH 3/5] makefile: tests-storage targets for user and root

Add targets for running tox in the storage-user
and storage-root environments.

Signed-off-by: Albert Esteve <aesteve@redhat.com>
---
 Makefile.am | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/Makefile.am b/Makefile.am
index 2056910e5a..bb8defee15 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -153,6 +153,14 @@ tests: tox
 tests-storage: tox
 	tox -e "storage"
 
+.PHONY: tests-storage-user
+tests-storage-user: tox
+	tox -e "storage-user"
+
+.PHONY: tests-storage-root
+tests-storage-root: tox
+	tox -e "storage-root"
+
 .PHONY: storage
 storage:
 	python3 tests/storage/userstorage.py setup

From 6b6b823a02e065eb1899a9a92f9a1def42fc9c33 Mon Sep 17 00:00:00 2001
From: Albert Esteve <aesteve@redhat.com>
Date: Wed, 22 Jun 2022 16:44:03 +0200
Subject: [PATCH 4/5] ci: split root and non-root storage tests

Most of vdsm code run as vdsm:kvm, but vdsm tests run
as root by default in the CI, causing several problems, e.g.:
- Different behaviour, when run locally and the CI
- Some tests are skipped as root

We can separate the tests in two different jobs:
- storage-tests: run test marked as "not root".
  It still requires the container to be privileged
  for setting up the storage.
- storage-tests-root: run tests marked as "root".

Fixes: #128
Signed-off-by: Albert Esteve <aesteve@redhat.com>
---
 .github/workflows/ci.yml    | 17 ++++++++--
 automation/tests-storage.sh | 65 ++++++++++++++++++++++++++++---------
 2 files changed, 65 insertions(+), 17 deletions(-)

diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
index ec99c6c4e5..21400d9cb1 100644
--- a/.github/workflows/ci.yml
+++ b/.github/workflows/ci.yml
@@ -10,6 +10,7 @@ jobs:
     - uses: actions/checkout@v2
     - name: Run linters
       run: ./automation/lint.sh
+  # TODO: rename to 'test-storage-user' once the github settings allow it.
   test-storage:
     env:
       TRAVIS_CI: 1
@@ -20,8 +21,20 @@ jobs:
       options: --privileged
     steps:
     - uses: actions/checkout@v2
-    - name: Run storage tests
-      run: ./automation/tests-storage.sh
+    - name: Run storage tests as vdsm
+      run: ./automation/tests-storage.sh vdsm
+  test-storage-root:
+    env:
+      TRAVIS_CI: 1
+    runs-on: ubuntu-latest
+    container:
+      image: quay.io/ovirt/vdsm-test-centos-8
+      # Required to create loop devices.
+      options: --privileged
+    steps:
+    - uses: actions/checkout@v2
+    - name: Run storage tests as root
+      run: ./automation/tests-storage.sh root
   tests:
     env:
       TRAVIS_CI: 1
diff --git a/automation/tests-storage.sh b/automation/tests-storage.sh
index 5f883e458e..795bf225a4 100755
--- a/automation/tests-storage.sh
+++ b/automation/tests-storage.sh
@@ -1,5 +1,17 @@
 #!/bin/sh -e
 
+print_help() {
+    echo "Usage: $0 USER"
+    echo ""
+    echo "  Helper script to setup and run storage tests as USER."
+    echo ""
+    echo "Examples:"
+    echo "  Run tests as root user"
+    echo "    $ $0 root"
+    echo "  Run tests as vdsm user"
+    echo "    $ $0 vdsm"
+}
+
 create_loop_devices() {
     local last=$(($1-1))
     local min
@@ -12,28 +24,51 @@ create_loop_devices() {
 }
 
 setup_storage() {
-    python3 tests/storage/userstorage.py setup
+    # Configure lvm to ignore udev events, otherwise some lvm tests hang.
+    mkdir -p /etc/lvm
+    cp docker/lvmlocal.conf /etc/lvm/
+
+    # Make sure we have enough loop device nodes. Using 16 devices since with 8
+    # devices we have random mount failures.
+    create_loop_devices 16
+
+    # Build vdsm.
+    ./autogen.sh --system
+    make
+
+    # Setup user storage during the tests.
+    make storage
 }
 
 teardown_storage() {
-    python3 tests/storage/userstorage.py teardown \
+    make clean-storage \
         || echo "WARNING: Ingoring error while tearing down user storage"
 }
 
-# Configure lvm to ignore udev events, otherwise some lvm tests hang.
-mkdir -p /etc/lvm
-cp docker/lvmlocal.conf /etc/lvm/
-
-# Make sure we have enough loop device nodes. Using 16 devices since with 8
-# devices we have random mount failures.
-create_loop_devices 16
+# Process user argument
+user=$1
+if [ -z "$user" ]; then
+    echo "ERROR: user required"
+    print_help
+    exit 1
+fi
 
-# Build vdsm.
-./autogen.sh --system
-make
+echo "Running tests as user $user"
 
-# Setup user stoage during the tests.
-trap teardown_storage EXIT
 setup_storage
+if [ "$user" != "root" ]; then
+    # Change ownership of current and storage folders
+    # to allow non-privileged access.
+    chown -R $user ./ /var/tmp/vdsm*
+fi
+# Teardown storage before exit.
+trap teardown_storage EXIT
 
-make tests-storage
+if [ "$user" = "root" ]; then
+    # Run only tests marked as root
+    make tests-storage-root
+else
+    # Run tests not marked as root as $user
+    # Use 'su' instead of 'sudo' in order to preserve the environment.
+    su $user -c "make tests-storage-user"
+fi

From 96569666f7690f235af05b2c7ba4ff5b237cb2a7 Mon Sep 17 00:00:00 2001
From: Albert Esteve <aesteve@redhat.com>
Date: Fri, 15 Jul 2022 14:37:58 +0200
Subject: [PATCH 5/5] tox: remove storage environment

Remove tox storage environment as it is not
used in the CIs anymore. Consequently, remove
also the associated make target 'tests-storage'.

Signed-off-by: Albert Esteve <aesteve@redhat.com>
---
 Makefile.am |  4 ----
 tox.ini     | 16 ----------------
 2 files changed, 20 deletions(-)

diff --git a/Makefile.am b/Makefile.am
index bb8defee15..aa4f9a3c4d 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -149,10 +149,6 @@ venv:
 tests: tox
 	tox -e "tests,lib,network,virt,gluster,hooks"
 
-.PHONY: tests-storage
-tests-storage: tox
-	tox -e "storage"
-
 .PHONY: tests-storage-user
 tests-storage-user: tox
 	tox -e "storage-user"
diff --git a/tox.ini b/tox.ini
index a2ea7861f1..f11fce37b6 100644
--- a/tox.ini
+++ b/tox.ini
@@ -81,22 +81,6 @@ commands =
         network/integration \
         network/unit
 
-[testenv:storage]
-passenv = {[base]passenv}
-setenv =
-    {[base]setenv}
-    COVERAGE_FILE=.coverage-storage
-deps = {[base]deps}
-changedir = {[base]changedir}
-# TODO: Remove --ignore
-commands =
-    python profile {envname} pytest -m {[base]markers} \
-    --durations=20 \
-    --cov=vdsm.storage \
-    --cov-report=html:htmlcov-storage \
-    --cov-fail-under={env:STORAGE_COVERAGE:68} \
-    {posargs:storage}
-
 [testenv:storage-user]
 passenv = {[base]passenv}
 setenv =