Re: [PATCH v2] tests: Simplify 4k tests setup and teardown

[Amit Bawer]

great improvement for us toxers :)

On Mon, May 20, 2019 at 1:12 AM Nir Soffer <nirsof(a)gmail.com&gt; wrote:

>  Previously you had to setup the environment manually to enable the
> 4k
>  tests, and pass environment variable to tests. The tests used single
>  path, so you had to run the tests twice to test both 4k block device and
>  filesystem backed by a 4k block device.
>
>  Add storage helper script to make it easier to setup storage for the
>  tests.
>
>  To setup 4k storage for the tests, run:
>
>     $ python tests/storage.py setup
>
>  This script creates both a 4k block device and filesystem backed by a 4k
>  device. The user_4k_path() fixture enables the tests if the device and
>  mount exist, and runs the test twice, once with the loop device, and
>  once with the filesystem.
>
>  To clean up the environment, run:
>
>     $ python tests/storage.py teardown
>
>  Signed-off-by: Nir Soffer <nsoffer(a)redhat.com&gt;
>  ---
>
>  Changes since v1:
>  - Fix device checks in teardown (Amit)
>  - Fix typos (Pavel)
>  - Merge setup and teardown to single script (Pavel)
>  - Rewrite helper script in python
>
>  v1 was here:
>
>
> https://lists.fedorahosted.org/archives/list/[e-mail]...
>
>   README.dev        |  44 ++++----------
>   tests/conftest.py |  38 ++++---------
>   tests/storage.py  | 142 ++++++++++++++++++++++++++++++++++++++++++++++
>   3 files changed, 165 insertions(+), 59 deletions(-)
>   create mode 100644 tests/storage.py
>
>  diff --git a/README.dev b/README.dev
>  index bfb170c..439b7b4 100644
>  --- a/README.dev
>  +++ b/README.dev
>  @@ -27,44 +27,22 @@ To run only tests matching the substring "foo":
>
>
>   Testing 4K support
>   ==================
>
>  -This requires manual setup for creating a block device using 4k sector
>  size,
>  -optionally creating and mounting a file system, and passing the test path
>  to
>  -tests. The setup must be done as root, but the tests can run later as the
>  -current user.
>  +To enable the 4k tests, you need to setup 4k stroage for the tests:
>
>  -Common setup
>  -------------
>  +    $ python tests/storage.py setup
>
>  -1. Create a backing file:
>  +This creates two loop devices with 4k sector size; one for testing
>  sanlock with
>  +4k block device, and the other for testing with a filesystem backed by a
>  4k
>  +block device.
>
>  -    $ truncate -s 1G /var/tmp/backing
>  +To teardown the storage:
>
>  -2. Create a loop device with 4k sector size:
>  +    $ python tests/storage.py teardown
>
>  -    $ sudo losetup -f /var/tmp/backing --show --sector-size=4096
>  -    /dev/loop2
>  +The script unmounts the filesystem and detaches the loop devices.
>
>  -3. Change the device (or mountpoint) owner to current user
>  -
>  -    $ sudo chown $USER:$USER /dev/loop2
>  -
>  -Testing 4k block device
>  ------------------------
>  -
>  -Run the tests with USER_4K_PATH environment variable:
>  -
>  -    $ USER_4K_PATH=/dev/loop2 tox -e py27
>  -
>  -Testing 4k file system
>  -----------------------
>  -
>  -To test file system on top of 4k block device, create a file system on the
>  -device, mount it, and create a file for testing lockspace or resources:
>  -
>  -    $ sudo mkfs.xfs /dev/loop2
>  -    $ sudo mount /dev/loop2 /tmp/sanlock-4k
>  -    $ sudo chown $USER:$USER /tmp/sanlock-4k
>  -    $ truncate -s 128m /tmp/sanlock-4k/disk
>  -    $ USER_4K_PATH=/tmp/sanlock-4k/disk tox -e py27
>  +The storage helper script uses sudo to perform privileged operations. The
>  best
>  +way to use it is to setup the environment once at the start of the
>  session, and
>  +teardown when you finish.
>  diff --git a/tests/conftest.py b/tests/conftest.py
>  index bb95d45..5212679 100644
>  --- a/tests/conftest.py
>  +++ b/tests/conftest.py
>  @@ -5,10 +5,11 @@ Fixtures for sanlock testing.
>   import os
>   import stat
>
>   import pytest
>
>  +from . import storage
>   from . import util
>
>
>   @pytest.fixture
>   def sanlock_daemon():
>  @@ -24,36 +25,21 @@ def sanlock_daemon():
>           # which takes about 3 seconds, slowing down the tests.
>           p.kill()
>           p.wait()
>
>
>  -(a)pytest.fixture
>  -def user_4k_path():
>  +(a)pytest.fixture(params=[
>  +    pytest.param(storage.BLOCK, id="block"),
>  +    pytest.param(storage.FILE, id="file"),
>  +])
>  +def user_4k_path(request):
>       """
>       A path to block device or file on file system on top of 4k block
>  device,
>       provided by the user.
>
>  -    The user must create the block device or the file system before
>  running the
>  -    tests, and specify the path to the file in the USER_4K_PATH
>  environment
>  -    variable.
>  -
>  -    If USER_4K_PATH was not specified, tests using this fixture will be
>  skipped.
>  -    If USER_4K_PATH was specified but does not exist, or is not a file or
>  block
>  -    device, RuntimeError is raised.
>  -
>  -    Return path to the user specified file.
>  +    If storage is not available, skip the tests.
>       """
>  -    path = os.environ.get("USER_4K_PATH")
>  -    if path is None:
>  -        pytest.skip("USER_4K_PATH pointing to a 4k block device or file
>  was "
>  -                    "not specified")
>  -
>  -    if not os.path.exists(path):
>  -        raise RuntimeError("USER_4K_PATH {!r} does not
>  exist".format(path))
>  -
>  -    mode = os.stat(path).st_mode
>  -    if not (stat.S_ISBLK(mode) or stat.S_ISREG(mode)):
>  -        raise RuntimeError(
>  -            "USER_4K_PATH {!r} is not a block device or regular file"
>  -            .format(path))
>  -
>  -    return path
>  +    if not os.path.exists(request.param):
>  +        pytest.skip(
>  +            "user storage available - run 'python tests/strorage.py
>  setup' "
>  +            "to enable 4k storage tests")
>  +    return request.param
>  diff --git a/tests/storage.py b/tests/storage.py
>  new file mode 100644
>  index 0000000..3258ed4
>  --- /dev/null
>  +++ b/tests/storage.py
>  @@ -0,0 +1,142 @@
>  +"""
>  +storage - provide storage for sanlock tests.
>  +"""
>  +
>  +import argparse
>  +import errno
>  +import logging
>  +import os
>  +import subprocess
>  +
>  +BASE_DIR = "/var/tmp/sanlock-storage"
>  +MOUNTPOINT = os.path.join(BASE_DIR, "mnt")
>  +
>  +# For testing sanlock with 4k block device.
>  +LOOP1 = os.path.join(BASE_DIR, "loop1")
>  +BACKING1 = os.path.join(BASE_DIR, "backing1")
>  +
>  +# For testing sanlock with a filesystem backed by 4k block device.
>  +LOOP2 = os.path.join(BASE_DIR, "loop2")
>  +BACKING2 = os.path.join(BASE_DIR, "backing2")
>  +
>  +# Test paths.
>  +BLOCK = LOOP1
>  +FILE = os.path.join(MOUNTPOINT, "file")
>  +
>  +log = logging.getLogger("storage")
>  +
>  +
>  +def main():
>  +    parser = argparse.ArgumentParser(
>  +        description='Storage helper for sanlock tests')
>  +    parser.add_argument("command", choices=["setup",
> "teardown"])
>  +    args = parser.parse_args()
>  +
>  +    logging.basicConfig(level=logging.INFO, format="storage: %(message)s")
>  +
>  +    if args.command == "setup":
>  +        setup()
>  +    elif args.command == "teardown":
>  +        teardown()
>  +
>  +
>  +def setup():
>  +    create_dir(BASE_DIR)
>  +
>  +    if not os.path.exists(LOOP1):
>  +        create_loop_device(LOOP1, BACKING1)
>  +
>  +    if not os.path.exists(LOOP2):
>  +        create_loop_device(LOOP2, BACKING2)
>  +        create_dir(MOUNTPOINT)
>  +        create_filesystem(LOOP2, MOUNTPOINT)
>  +
>  +        # Sanlock allocates spaces as needed.
>  +        with open(FILE, "wb") as f:
>  +            f.truncate(0)
>  +
>  +
>  +def teardown():
>  +    if is_mounted(MOUNTPOINT):
>  +        remove_filesystem(MOUNTPOINT)
>  +
>  +    if os.path.exists(LOOP2):
>  +        remove_loop_device(LOOP2, BACKING2)
>  +
>  +    if os.path.exists(LOOP1):
>  +        remove_loop_device(LOOP1, BACKING1)
>  +
>  +
>  +def create_loop_device(link_path, backing_file, size=1024**3,
>  +                       sector_size=4096):
>  +    log.info("Creating loop device %s", link_path)
>  +
>  +    with open(backing_file, "wb") as f:
>  +        f.truncate(size)
>  +
>  +    out = subprocess.check_output([
>  +        "sudo",
>  +        "losetup",
>  +        "-f", backing_file,
>  +        "--sector-size", str(sector_size),
>  +        "--show",
>  +    ])
>  +
>  +    device = out.decode("utf-8").strip()
>  +
>  +    os.symlink(device, link_path)
>  +    chown(link_path)
>  +
>  +
>  +def remove_loop_device(link_path, backing_file):
>  +    log.info("Removing loop device %s", link_path)
>  +
>  +    subprocess.check_call(["sudo", "losetup", "-d",
> link_path])
>  +    remove_file(link_path)
>  +    remove_file(backing_file)
>  +
>  +
>  +def create_filesystem(device, mountpoint):
>  +    log.info("Creating filesystem %s", mountpoint)
>  +
>  +    subprocess.check_call(["sudo", "mkfs.xfs", "-q",
> device])
>  +    subprocess.check_call(["sudo", "mount", device, mountpoint])
>  +    chown(mountpoint)
>  +
>  +
>  +def remove_filesystem(mountpoint):
>  +    log.info("Removing filesystem %s", mountpoint)
>  +    subprocess.check_call(["sudo", "umount", mountpoint])
>  +
>  +
>  +def is_mounted(mountpoint):
>  +    with open("/proc/self/mounts") as f:
>  +        for line in f:
>  +            if mountpoint in line:
>  +                return True
>  +    return False
>  +
>  +
>  +def chown(path):
>  +    user_group = "%(USER)s:%(USER)s" % os.environ
>  +    subprocess.check_call(["sudo", "chown", user_group, path])
>  +
>  +
>  +def create_dir(path):
>  +    try:
>  +        os.makedirs(path)
>  +    except EnvironmentError as e:
>  +        if e.errno != errno.EEXIST:
>  +            raise
>  +
>  +
>  +def remove_file(path):
>  +    try:
>  +        os.remove(path)
>  +    except EnvironmentError as e:
>  +        if e.errno != errno.ENOENT:
>  +            raise
>  +
>  +
>  +if __name__ == "__main__":
>  +    main()
>  --
>  2.17.2
>
>