ci: split storage ci jobs for root and non-root testing

.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

Makefile.am

@@ -149,9 +149,13 @@ 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"
+
+.PHONY: tests-storage-root
+tests-storage-root: tox
+	tox -e "storage-root"
 
 .PHONY: storage
 storage:

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

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.

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)

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,34 +187,39 @@ 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)
 
 
 @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)
 
 
 @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)
 
 
 @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
 
 
 @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.

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))
 

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

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)

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"))

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)