refactor(services): introduce Bind class and test moving deeper

selfprivacy_api/services/moving.py

@@ -67,6 +67,6 @@ def ensure_folder_ownership(folders: List[Bind]) -> None:
         folder.ensure_ownership()
 
 
-def bind_folders(folders: List[Bind], volume: BlockDevice):
+def bind_folders(folders: List[Bind]):
     for folder in folders:
         folder.bind()

selfprivacy_api/services/owned_path.py

@@ -2,9 +2,13 @@ from __future__ import annotations
 import subprocess
 import pathlib
 from pydantic import BaseModel
+from os.path import exists
 
 from selfprivacy_api.utils.block_devices import BlockDevice, BlockDevices
 
+# tests override it to a tmpdir
+VOLUMES_PATH = "/volumes"
+
 
 class BindError(Exception):
     pass
@@ -19,9 +23,8 @@ class OwnedPath(BaseModel):
 
 class Bind:
     """
-    A directory that resides on some volume but we mount it into fs
-    where we need it.
-    Used for service data.
+    A directory that resides on some volume but we mount it into fs where we need it.
+    Used for storing service data.
     """
 
     def __init__(self, binding_path: str, owner: str, group: str, drive: BlockDevice):
@@ -30,7 +33,7 @@ class Bind:
         self.group = group
         self.drive = drive
 
-    # TODO: make Service return a list of binds instead of owned paths
+    # TODO: delete owned path interface from Service
     @staticmethod
     def from_owned_path(path: OwnedPath, drive_name: str) -> Bind:
         drive = BlockDevices().get_block_device(drive_name)
@@ -45,9 +48,9 @@ class Bind:
         return self.binding_path.split("/")[-1]
 
     def location_at_volume(self) -> str:
-        return f"/volumes/{self.drive.name}/{self.bind_foldername()}"
+        return f"{VOLUMES_PATH}/{self.drive.name}/{self.bind_foldername()}"
 
-    def validate(self) -> str:
+    def validate(self) -> None:
         path = pathlib.Path(self.location_at_volume())
 
         if not path.exists():
@@ -58,23 +61,29 @@ class Bind:
             raise BindError(f"{path} is not owned by {self.owner}.")
 
     def bind(self) -> None:
+        if not exists(self.binding_path):
+            raise BindError(f"cannot bind to a non-existing path: {self.binding_path}")
+
+        source = self.location_at_volume()
+        target = self.binding_path
+
         try:
             subprocess.run(
-                [
-                    "mount",
-                    "--bind",
-                    self.location_at_volume(),
-                    self.binding_path,
-                ],
+                ["mount", "--bind", source, target],
+                stderr=subprocess.PIPE,
                 check=True,
             )
         except subprocess.CalledProcessError as error:
-            print(error.output)
-            raise BindError(f"Unable to mount new volume:{error.output}")
+            print(error.stderr)
+            raise BindError(f"Unable to bind {source} to {target} :{error.stderr}")
 
     def unbind(self) -> None:
+        if not exists(self.binding_path):
+            raise BindError(f"cannot unbind a non-existing path: {self.binding_path}")
+
         try:
             subprocess.run(
+                # umount -l ?
                 ["umount", self.binding_path],
                 check=True,
             )
@@ -94,10 +103,11 @@ class Bind:
                     true_location,
                 ],
                 check=True,
+                stderr=subprocess.PIPE,
             )
         except subprocess.CalledProcessError as error:
-            print(error.output)
+            print(error.stderr)
             error_message = (
-                f"Unable to set ownership of {true_location} :{error.output}"
+                f"Unable to set ownership of {true_location} :{error.stderr}"
             )
             raise BindError(error_message)

selfprivacy_api/utils/block_devices.py

@@ -171,6 +171,9 @@ class BlockDevice:
         return False
 
 
+# TODO: SingletonMetaclass messes with tests and is able to persist state
+# between them. If you have very weird test crosstalk that's probably why
+# I am not sure it NEEDS to be SingletonMetaclass
 class BlockDevices(metaclass=SingletonMetaclass):
     """Singleton holding all Block devices"""
 

tests/conftest.py

@@ -4,6 +4,7 @@
 import os
 import pytest
 import datetime
+import subprocess
 
 from os import path
 from os import makedirs
@@ -135,6 +136,18 @@ def wrong_auth_client(huey_database, redis_repo_with_tokens):
     return client
 
 
+@pytest.fixture()
+def volume_folders(tmpdir, mocker):
+    volumes_dir = path.join(tmpdir, "volumes")
+
+    makedirs(volumes_dir)
+    volumenames = ["sda1", "sda2"]
+    for d in volumenames:
+        service_dir = path.join(volumes_dir, d)
+        makedirs(service_dir)
+    mock = mocker.patch("selfprivacy_api.services.owned_path.VOLUMES_PATH", volumes_dir)
+
+
 @pytest.fixture()
 def raw_dummy_service(tmpdir):
     dirnames = ["test_service", "also_test_service"]
@@ -161,11 +174,38 @@ def raw_dummy_service(tmpdir):
     return service
 
 
+def ensure_user_exists(user: str):
+    try:
+        output = subprocess.check_output(
+            ["useradd", "-U", user], stderr=subprocess.PIPE, shell=False
+        )
+    except subprocess.CalledProcessError as error:
+        if b"already exists" not in error.stderr:
+            raise error
+
+    try:
+        output = subprocess.check_output(
+            ["useradd", user], stderr=subprocess.PIPE, shell=False
+        )
+    except subprocess.CalledProcessError as error:
+        assert b"already exists" in error.stderr
+        return
+
+    raise ValueError("could not create user", user)
+
+
 @pytest.fixture()
 def dummy_service(
     tmpdir, raw_dummy_service, generic_userdata
 ) -> Generator[Service, None, None]:
     service = raw_dummy_service
+    user = service.get_user()
+
+    # TODO: use create_user from users actions. But it will need NIXOS to be there
+    # and react to our changes to files.
+    # from selfprivacy_api.actions.users import create_user
+    # create_user(user, "yay, it is me")
+    ensure_user_exists(user)
 
     # register our service
     services.services.append(service)

tests/test_binds.py

@@ -0,0 +1,92 @@
+import pytest
+from os import mkdir, rmdir
+from os.path import join, exists
+
+
+from tests.conftest import ensure_user_exists
+from tests.test_graphql.test_services import mock_lsblk_devices
+
+from selfprivacy_api.services.owned_path import Bind, BindError
+from selfprivacy_api.utils.block_devices import BlockDevices
+from selfprivacy_api.utils.waitloop import wait_until_true
+
+
+BINDTESTS_USER = "binduser"
+TESTFILE_CONTENTS = "testissimo"
+TESTFILE_NAME = "testfile"
+
+
+@pytest.fixture()
+def bind_user():
+    ensure_user_exists(BINDTESTS_USER)
+    return BINDTESTS_USER
+
+
+def prepare_test_bind(tmpdir, bind_user) -> Bind:
+    test_binding_name = "bindy_dir"
+    binding_path = join(tmpdir, test_binding_name)
+    drive = BlockDevices().get_block_device("sda2")
+    assert drive is not None
+
+    bind = Bind(
+        binding_path=binding_path, owner=bind_user, group=bind_user, drive=drive
+    )
+
+    source_dir = bind.location_at_volume()
+    mkdir(source_dir)
+    mkdir(binding_path)
+
+    testfile_path = join(source_dir, TESTFILE_NAME)
+    with open(testfile_path, "w") as file:
+        file.write(TESTFILE_CONTENTS)
+
+    return bind
+
+
+def test_bind_unbind(volume_folders, tmpdir, bind_user, mock_lsblk_devices):
+    bind = prepare_test_bind(tmpdir, bind_user)
+    bind.ensure_ownership()
+    bind.validate()
+
+    testfile_path = join(bind.location_at_volume(), TESTFILE_NAME)
+    assert exists(testfile_path)
+    with open(testfile_path, "r") as file:
+        assert file.read() == TESTFILE_CONTENTS
+
+    bind.bind()
+
+    testfile_binding_path = join(bind.binding_path, TESTFILE_NAME)
+    assert exists(testfile_path)
+    with open(testfile_path, "r") as file:
+        assert file.read() == TESTFILE_CONTENTS
+
+    bind.unbind()
+    # wait_until_true(lambda : not exists(testfile_binding_path), timeout_sec=2)
+    assert not exists(testfile_binding_path)
+    assert exists(bind.binding_path)
+
+
+def test_bind_nonexistent_target(volume_folders, tmpdir, bind_user, mock_lsblk_devices):
+    bind = prepare_test_bind(tmpdir, bind_user)
+
+    bind.ensure_ownership()
+    bind.validate()
+    rmdir(bind.binding_path)
+
+    with pytest.raises(BindError):
+        bind.bind()
+
+
+def test_unbind_nonexistent_target(
+    volume_folders, tmpdir, bind_user, mock_lsblk_devices
+):
+    bind = prepare_test_bind(tmpdir, bind_user)
+
+    bind.ensure_ownership()
+    bind.validate()
+    bind.bind()
+
+    bind.binding_path = "/bogus"
+
+    with pytest.raises(BindError):
+        bind.unbind()

tests/test_block_device_utils.py

@@ -410,6 +410,7 @@ def lsblk_full_mock(mocker):
     mock = mocker.patch(
         "subprocess.check_output", autospec=True, return_value=FULL_LSBLK_OUTPUT
     )
+    BlockDevices().update()
     return mock
 
 

tests/test_graphql/test_services.py

@@ -1,5 +1,8 @@
 import pytest
+import shutil
+
 from typing import Generator
+from os import mkdir
 
 from selfprivacy_api.utils.block_devices import BlockDevices
 
@@ -8,13 +11,77 @@ from selfprivacy_api.services import get_service_by_id
 from selfprivacy_api.services.service import Service, ServiceStatus
 from selfprivacy_api.services.test_service import DummyService
 
-# from selfprivacy_api.services.moving import check_folders
-
 from tests.common import generate_service_query
 from tests.test_graphql.common import assert_empty, assert_ok, get_data
 from tests.test_block_device_utils import lsblk_singular_mock
 
-from subprocess import CompletedProcess
+
+LSBLK_BLOCKDEVICES_DICTS = [
+    {
+        "name": "sda1",
+        "path": "/dev/sda1",
+        "fsavail": "4614107136",
+        "fssize": "19814920192",
+        "fstype": "ext4",
+        "fsused": "14345314304",
+        "mountpoints": ["/nix/store", "/"],
+        "label": None,
+        "uuid": "ec80c004-baec-4a2c-851d-0e1807135511",
+        "size": 20210236928,
+        "model": None,
+        "serial": None,
+        "type": "part",
+    },
+    {
+        "name": "sda2",
+        "path": "/dev/sda2",
+        "fsavail": "4614107136",
+        "fssize": "19814920192",
+        "fstype": "ext4",
+        "fsused": "14345314304",
+        "mountpoints": ["/home"],
+        "label": None,
+        "uuid": "deadbeef-baec-4a2c-851d-0e1807135511",
+        "size": 20210236928,
+        "model": None,
+        "serial": None,
+        "type": "part",
+    },
+]
+
+
+@pytest.fixture()
+def mock_lsblk_devices(mocker):
+    mock = mocker.patch(
+        "selfprivacy_api.utils.block_devices.BlockDevices.lsblk_device_dicts",
+        autospec=True,
+        return_value=LSBLK_BLOCKDEVICES_DICTS,
+    )
+    BlockDevices().update()
+    assert BlockDevices().lsblk_device_dicts() == LSBLK_BLOCKDEVICES_DICTS
+    devices = BlockDevices().get_block_devices()
+
+    assert len(devices) == 2
+
+    names = [device.name for device in devices]
+    assert "sda1" in names
+    assert "sda2" in names
+    return mock
+
+
+@pytest.fixture()
+def dummy_service_with_binds(dummy_service, mock_lsblk_devices, volume_folders):
+    binds = dummy_service.binds()
+    for bind in binds:
+        path = bind.binding_path
+        shutil.move(bind.binding_path, bind.location_at_volume())
+        mkdir(bind.binding_path)
+
+        bind.ensure_ownership()
+        bind.validate()
+
+        bind.bind()
+    return dummy_service
 
 
 @pytest.fixture()
@@ -28,9 +95,17 @@ def only_dummy_service(dummy_service) -> Generator[DummyService, None, None]:
     service_module.services.extend(back_copy)
 
 
-MOVER_MOCK_PROCESS = CompletedProcess(["ls"], returncode=0)
+@pytest.fixture()
+def mock_check_volume(mocker):
+    mock = mocker.patch(
+        "selfprivacy_api.services.service.check_volume",
+        autospec=True,
+        return_value=None,
+    )
+    return mock
 
 
+# TODO: remove
 @pytest.fixture()
 def mock_check_service_mover_binds(mocker):
     mock = mocker.patch(
@@ -41,20 +116,6 @@ def mock_check_service_mover_binds(mocker):
     return mock
 
 
-@pytest.fixture()
-def mock_subprocess_run(mocker):
-    mock = mocker.patch(
-        "subprocess.run", autospec=True, return_value=MOVER_MOCK_PROCESS
-    )
-    return mock
-
-
-@pytest.fixture()
-def mock_shutil_move(mocker):
-    mock = mocker.patch("shutil.move", autospec=True, return_value=None)
-    return mock
-
-
 API_START_MUTATION = """
 mutation TestStartService($service_id: String!) {
     services {
@@ -551,7 +612,7 @@ def test_move_same_volume(authorized_client, dummy_service):
 def test_graphql_move_service_without_folders_on_old_volume(
     authorized_client,
     generic_userdata,
-    lsblk_singular_mock,
+    mock_lsblk_devices,
     dummy_service: DummyService,
 ):
     target = "sda1"
@@ -564,26 +625,27 @@ def test_graphql_move_service_without_folders_on_old_volume(
 
     data = get_data(mutation_response)["services"]["moveService"]
     assert_errorcode(data, 400)
+    assert "sda2/test_service is not found" in data["message"]
 
 
 def test_graphql_move_service(
     authorized_client,
     generic_userdata,
+    # TODO: substitute with a weaker mock or delete altogether
     mock_check_service_mover_binds,
-    lsblk_singular_mock,
-    dummy_service: DummyService,
-    mock_subprocess_run,
-    mock_shutil_move,
+    mock_check_volume,
+    dummy_service_with_binds,
 ):
-    # Does not check real moving,
-    # but tests the finished job propagation through API
+    dummy_service = dummy_service_with_binds
 
-    target = "sda1"
-    BlockDevices().update()
+    origin = "sda1"
+    target = "sda2"
     assert BlockDevices().get_block_device(target) is not None
+    assert BlockDevices().get_block_device(origin) is not None
 
+    dummy_service.set_drive(origin)
     dummy_service.set_simulated_moves(False)
-    dummy_service.set_drive("sda2")
+
     mutation_response = api_move(authorized_client, dummy_service, target)
 
     data = get_data(mutation_response)["services"]["moveService"]