Skip to content

Commit

Permalink
Make volume classes immutable and create copies of them in launch_con…
Browse files Browse the repository at this point in the history
…tainer()
  • Loading branch information
dcermak committed Oct 29, 2024
1 parent 383f019 commit 23f2873
Show file tree
Hide file tree
Showing 7 changed files with 191 additions and 103 deletions.
6 changes: 3 additions & 3 deletions CHANGELOG.rst
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ Next Release
Breaking changes:

- Change addition of SELinux flags to volumes: SELinux flags are only added if
:py:attr:`~pytest_container.container.ContainerVolumeBase.flags` is ``None``.
:py:attr:`~pytest_container.volume.ContainerVolumeBase.flags` is ``None``.

Improvements and new features:

Expand Down Expand Up @@ -233,8 +233,8 @@ Improvements and new features:
and comparing versions.

- Container volumes and bind mounts can now be automatically created via the
:py:class:`~pytest_container.container.ContainerVolume` and
:py:class:`~pytest_container.container.BindMount` classes and adding them to
:py:class:`~pytest_container.volume.ContainerVolume` and
:py:class:`~pytest_container.volume.BindMount` classes and adding them to
the :py:attr:`~pytest_container.container.ContainerBase.volume_mounts`
attribute.

Expand Down
26 changes: 18 additions & 8 deletions pytest_container/container.py
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,11 @@
from pytest_container.runtime import OciRuntimeBase
from pytest_container.volume import BindMount
from pytest_container.volume import ContainerVolume
from pytest_container.volume import CreatedBindMount
from pytest_container.volume import CreatedContainerVolume
from pytest_container.volume import get_volume_creator


if sys.version_info >= (3, 8):
from importlib import metadata
from typing import Literal
Expand Down Expand Up @@ -273,7 +276,6 @@ def get_launch_cmd(
if self.extra_environment_variables
else []
)
+ [vol.cli_arg for vol in self.volume_mounts]
)

id_or_url = self.container_id or self.url
Expand Down Expand Up @@ -840,20 +842,26 @@ def release_lock() -> None:
else:
self._stack.callback(release_lock)

for cont_vol in self.container.volume_mounts:
self._stack.enter_context(
get_volume_creator(cont_vol, self.container_runtime)
)

forwarded_ports = self.container.forwarded_ports

extra_run_args = self.extra_run_args

if self.container_name:
extra_run_args.extend(("--name", self.container_name))

extra_run_args.append(f"--cidfile={self._cidfile}")

new_container_volumes = []
for cont_vol in self.container.volume_mounts:
vol_creator = get_volume_creator(cont_vol, self.container_runtime)
self._stack.enter_context(vol_creator)

new_volume = vol_creator.created_volume
assert new_volume
new_container_volumes.append(new_volume)

extra_run_args.append(new_volume.cli_arg)

forwarded_ports = self.container.forwarded_ports

# Create a copy of the container which was used to parametrize this test
cls = type(self.container)
constructor = inspect.signature(cls.__init__)
Expand All @@ -864,6 +872,8 @@ def release_lock() -> None:
for k, v in self.container.__dict__.items()
if k in constructor.parameters
}
kwargs["volume_mounts"] = new_container_volumes

# We must perform the launches in separate branches, as containers with
# port forwards must be launched while the lock is being held. Otherwise
# another container could pick the same ports before this one launches.
Expand Down
134 changes: 94 additions & 40 deletions pytest_container/volume.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,20 +2,30 @@
and bind mounts.
"""

from subprocess import check_output
from types import TracebackType
from typing import Iterable, List, Optional, Type, Union, overload
from os.path import exists
from os.path import isabs
from typing_extensions import TypedDict
import enum
import tempfile
import sys
from dataclasses import KW_ONLY, dataclass
import tempfile
from dataclasses import dataclass
from dataclasses import field
from os.path import exists
from os.path import isabs
from subprocess import check_output
from types import TracebackType
from typing import Callable
from typing import List
from typing import Optional
from typing import overload
from typing import Sequence
from typing import Type
from typing import Union

from pytest_container.runtime import OciRuntimeBase
from pytest_container.logging import _logger
from pytest_container.runtime import OciRuntimeBase

try:
from typing import TypedDict
except ImportError:
from typing_extensions import TypedDict


@enum.unique
Expand Down Expand Up @@ -60,16 +70,15 @@ class ContainerVolumeBase:
#: Path inside the container where this volume will be mounted
container_path: str

_: KW_ONLY

#: Flags for mounting this volume.
#:
#: Note that some flags are mutually exclusive and potentially not supported
#: by all container runtimes.
#:
#: The :py:attr:`VolumeFlag.SELINUX_PRIVATE` flag will be added by default
#: The :py:attr:`VolumeFlag.SELINUX_PRIVATE` flag will be used by default
#: if flags is ``None``, unless :py:attr:`ContainerVolumeBase.shared` is
#: ``True``, then :py:attr:`VolumeFlag.SELINUX_SHARED` is added.
#: ``True``, then :py:attr:`VolumeFlag.SELINUX_SHARED` is added to the list
#: of flags to use.
#:
#: If flags is a list (even an empty one), then no flags are added.
flags: Optional[List[VolumeFlag]] = None
Expand All @@ -81,10 +90,6 @@ class ContainerVolumeBase:
#: :py:attr:`~ContainerVolumeBase.flags`.
shared: bool = False

#: internal volume name via which the volume can be mounted, e.g. the
#: volume's ID or the path on the host
# _vol_name: str = ""

def __post_init__(self) -> None:

for mutually_exclusive_flags in (
Expand All @@ -102,20 +107,28 @@ def __post_init__(self) -> None:
)

@property
def _flags(self) -> Iterable[VolumeFlag]:
if self.flags:
def _flags(self) -> Sequence[VolumeFlag]:
"""Internal sequence of flags to be used to mount this volume. If the
user supplied no flags, then this property gives you one of the SELinux
flags.
"""
if self.flags is not None:
return self.flags

if self.shared:
return (VolumeFlag.SELINUX_SHARED,)
else:
return (VolumeFlag.SELINUX_PRIVATE,)

return (VolumeFlag.SELINUX_PRIVATE,)

def container_volume_cli_arg(

def _container_volume_cli_arg(
container_volume: ContainerVolumeBase, volume_name: str
) -> str:
"""Command line argument to mount the supplied ``container_volume`` volume."""
"""Command line argument to mount the supplied ``container_volume`` volume
via the "name" `volume_name` (can be a volume ID or a path on the host).
"""
res = f"-v={volume_name}:{container_volume.container_path}"
res += ":" + ",".join(str(f) for f in container_volume._flags)
return res
Expand All @@ -129,10 +142,29 @@ class ContainerVolume(ContainerVolumeBase):
"""


def _create_required_parameter_factory(
parameter_name: str,
) -> Callable[[], str]:
def _required_parameter() -> str:
raise ValueError(f"Parameter {parameter_name} is required")

Check warning on line 149 in pytest_container/volume.py

View check run for this annotation

Codecov / codecov/patch

pytest_container/volume.py#L149

Added line #L149 was not covered by tests

return _required_parameter


@dataclass(frozen=True)
class CreatedContainerVolume(ContainerVolume):
"""A container volume that exists and has a volume id assigned to it."""

#: The hash/ID of the volume in the container runtime.
#: This parameter is required
volume_id: str = field(
default_factory=_create_required_parameter_factory("volume_id")
)

volume_id: str
@property
def cli_arg(self) -> str:
"""The command line argument to mount this volume."""
return _container_volume_cli_arg(self, self.volume_id)


@dataclass(frozen=True)
Expand All @@ -144,19 +176,34 @@ class BindMount(ContainerVolumeBase):
container via :py:attr:`~ContainerVolumeBase.container_path`. The
``container*`` fixtures will then create a temporary directory on the host
for you that will be used as the mount point. Alternatively, you can also
specify the path on the host yourself via :py:attr:`host_path`.
specify the path on the host yourself via :py:attr:`BindMount.host_path`.
"""

#: Path on the host that will be mounted if absolute. if relative,
#: Path on the host that will be mounted if absolute. If relative,
#: it refers to a volume to be auto-created. When omitted, a temporary
#: directory will be created and the path will be saved in this attribute.
host_path: Optional[str] = None


@dataclass(frozen=True)
class CreatedBindMount(ContainerVolumeBase):
host_path: str
class CreatedBindMount(BindMount):
"""An established bind mount of the directory :py:attr:`BindMount.host_path`
on the host to :py:attr:`~ContainerVolumeBase.container_path` in the
container.
"""

#: Path on the host that is bind mounted into the container.
#: This parameter must be provided.
host_path: str = field(
default_factory=_create_required_parameter_factory("host_path")
)

@property
def cli_arg(self) -> str:
"""The command line argument to mount this volume."""
return _container_volume_cli_arg(self, self.host_path)


class _ContainerVolumeKWARGS(TypedDict, total=False):
Expand All @@ -174,8 +221,13 @@ class VolumeCreator:
"""Context Manager to create and remove a :py:class:`ContainerVolume`.
This context manager creates a volume using the supplied
:py:attr:`container_runtime` When the ``with`` block is entered and removes
:py:attr:`container_runtime` when the ``with`` block is entered and removes
it once it is exited.
The :py:class:`ContainerVolume` in :py:attr:`volume` is used as the
blueprint to create the volume, the actually created container volume is
saved in :py:attr:`created_volume`.
"""

#: The volume to be created
Expand All @@ -184,7 +236,9 @@ class VolumeCreator:
#: The container runtime, via which the volume is created & destroyed
container_runtime: OciRuntimeBase

_created_volume: Optional[CreatedContainerVolume] = None
#: The created container volume once it has been setup & created. It's
#: ``None`` until then.
created_volume: Optional[CreatedContainerVolume] = None

def __enter__(self) -> "VolumeCreator":
"""Creates the container volume"""
Expand All @@ -195,7 +249,7 @@ def __enter__(self) -> "VolumeCreator":
.decode()
.strip()
)
self._created_volume = CreatedContainerVolume(
self.created_volume = CreatedContainerVolume(
container_path=self.volume.container_path,
flags=self.volume.flags,
shared=self.volume.shared,
Expand All @@ -211,11 +265,11 @@ def __exit__(
__traceback: Optional[TracebackType],
) -> None:
"""Cleans up the container volume."""
assert self._created_volume and self._created_volume.volume_id
assert self.created_volume and self.created_volume.volume_id

_logger.debug(
"cleaning up volume %s via %s",
self._created_volume.volume_id,
self.created_volume.volume_id,
self.container_runtime.runner_binary,
)

Expand All @@ -226,7 +280,7 @@ def __exit__(
"volume",
"rm",
"-f",
self._created_volume.volume_id,
self.created_volume.volume_id,
],
)

Expand All @@ -244,7 +298,7 @@ class BindMountCreator:
#: internal temporary directory
_tmpdir: Optional[TEMPDIR_T] = None

_created_volume: Optional[CreatedBindMount] = None
created_volume: Optional[CreatedBindMount] = None

def __post__init__(self) -> None:
# the tempdir must not be set accidentally by the user
Expand Down Expand Up @@ -281,7 +335,7 @@ def __enter__(self) -> "BindMountCreator":
"was requested but the directory does not exist"
)

self._created_volume = CreatedBindMount(**kwargs)
self.created_volume = CreatedBindMount(**kwargs)

return self

Expand All @@ -292,13 +346,13 @@ def __exit__(
__traceback: Optional[TracebackType],
) -> None:
"""Cleans up the temporary host directory or the container volume."""
assert self._created_volume and self._created_volume.host_path
assert self.created_volume

if self._tmpdir:
_logger.debug(
"cleaning up directory %s for the container volume %s",
self.volume.host_path,
self.volume.container_path,
self.created_volume.host_path,
self.created_volume.container_path,
)
self._tmpdir.cleanup()

Expand Down
8 changes: 8 additions & 0 deletions source/api.rst
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,14 @@ The pod module
:undoc-members:


The volume module
-----------------

.. automodule:: pytest_container.volume
:members:
:undoc-members:


The build module
----------------

Expand Down
Loading

0 comments on commit 23f2873

Please sign in to comment.