From 68adb23e75c1dee7dea2a783cc03b810d6cc7250 Mon Sep 17 00:00:00 2001 From: Michal Skrivanek Date: Fri, 26 Sep 2025 09:44:18 +0200 Subject: [PATCH 1/4] add tracking to created resources in OpenDAL every file or directory creating is tracked, even if the file already existed. --- .../jumpstarter_driver_http/driver_test.py | 68 ++++++++++++++++++- .../jumpstarter_driver_opendal/client.py | 33 ++++++++- .../jumpstarter_driver_opendal/driver.py | 46 +++++++++++++ 3 files changed, 144 insertions(+), 3 deletions(-) diff --git a/packages/jumpstarter-driver-http/jumpstarter_driver_http/driver_test.py b/packages/jumpstarter-driver-http/jumpstarter_driver_http/driver_test.py index da75ff96b..c459ab88a 100644 --- a/packages/jumpstarter-driver-http/jumpstarter_driver_http/driver_test.py +++ b/packages/jumpstarter-driver-http/jumpstarter_driver_http/driver_test.py @@ -1,3 +1,5 @@ +import logging + import aiohttp import pytest @@ -11,8 +13,8 @@ def anyio_backend(): @pytest.fixture -def http(tmp_path): - with serve(HttpServer(root_dir=str(tmp_path))) as client: +def http(tmp_path, unused_tcp_port): + with serve(HttpServer(root_dir=str(tmp_path), port=unused_tcp_port)) as client: client.start() try: yield client @@ -56,3 +58,65 @@ def test_http_server_root_directory_creation(tmp_path): new_dir = tmp_path / "new_http_root" _ = HttpServer(root_dir=str(new_dir)) assert new_dir.exists() + + +@pytest.mark.anyio +async def test_opendal_tracking_on_http_server_close(tmp_path, unused_tcp_port, caplog): + """Test that OpenDAL driver tracks created files and reports them on close.""" + filename = "tracked_test.txt" + test_content = b"test content for tracking" + + # Set up logging to capture debug messages + with caplog.at_level(logging.DEBUG): + with serve(HttpServer(root_dir=str(tmp_path), port=unused_tcp_port)) as client: + client.start() + + # Write a file through the HTTP server (which uses OpenDAL internally) + (tmp_path / "src").write_bytes(test_content) + client.put_file(filename, tmp_path / "src") + + # Verify the file was written + files = list(client.storage.list("/")) + assert filename in files + + # Get the tracking info before close + created_files = client.storage.get_created_files() + assert filename in created_files + + client.stop() + # When exiting the context manager, HttpServer.close() is called, + # which calls super().close(), which calls OpenDAL.close() + + # The main functionality test is that the file was tracked as created + # The close() method logging might not be captured due to async cleanup timing + # but we've verified the tracking works by checking get_created_files() above + + +def test_opendal_tracking_methods(tmp_path, unused_tcp_port): + """Test the OpenDAL tracking export methods directly.""" + with serve(HttpServer(root_dir=str(tmp_path), port=unused_tcp_port)) as client: + client.start() + + # Initially, no files should be tracked + created_files = client.storage.get_created_files() + created_dirs = client.storage.get_created_directories() + assert created_files == [] + assert created_dirs == [] + + # Write a file (which creates it) + filename = "direct_test.txt" + test_content = b"direct test content" + (tmp_path / "src").write_bytes(test_content) + client.put_file(filename, tmp_path / "src") + + # Check tracking + created_files = client.storage.get_created_files() + assert filename in created_files + + # Test get_all_created_resources + created_dirs, created_files = client.storage.get_all_created_resources() + assert filename in created_files + assert created_dirs == [] + + + client.stop() diff --git a/packages/jumpstarter-driver-opendal/jumpstarter_driver_opendal/client.py b/packages/jumpstarter-driver-opendal/jumpstarter_driver_opendal/client.py index 2d7fbef62..5b8e6da73 100644 --- a/packages/jumpstarter-driver-opendal/jumpstarter_driver_opendal/client.py +++ b/packages/jumpstarter-driver-opendal/jumpstarter_driver_opendal/client.py @@ -6,7 +6,7 @@ from dataclasses import dataclass from io import BytesIO from pathlib import Path -from typing import cast +from typing import List, cast from urllib.parse import urlparse from uuid import UUID @@ -394,6 +394,37 @@ def capability(self, /) -> Capability: """ return self.call("capability") + @validate_call(validate_return=True) + def get_created_files(self) -> List[str]: + """ + Get list of files that have been created during this session. + + Returns: + List[str]: List of file paths that were created + """ + return self.call("get_created_files") + + @validate_call(validate_return=True) + def get_created_directories(self) -> List[str]: + """ + Get list of directories that have been created during this session. + + Returns: + List[str]: List of directory paths that were created + """ + return self.call("get_created_directories") + + @validate_call(validate_return=True) + def get_all_created_resources(self) -> tuple[List[str], List[str]]: + """ + Get all resources created during this session as a tuple: (created_directories, created_files). + + Returns: + tuple[List[str], List[str]]: Tuple of (created_directories, created_files) + """ + return self.call("get_all_created_resources") + + def cli(self): # noqa: C901 arg_path = click.argument("path", type=click.Path()) arg_source = click.argument("source", type=click.Path()) diff --git a/packages/jumpstarter-driver-opendal/jumpstarter_driver_opendal/driver.py b/packages/jumpstarter-driver-opendal/jumpstarter_driver_opendal/driver.py index e327276ce..053133ed5 100644 --- a/packages/jumpstarter-driver-opendal/jumpstarter_driver_opendal/driver.py +++ b/packages/jumpstarter-driver-opendal/jumpstarter_driver_opendal/driver.py @@ -1,3 +1,5 @@ +from __future__ import annotations + import hashlib from abc import ABCMeta, abstractmethod from collections.abc import AsyncGenerator @@ -26,6 +28,10 @@ class Opendal(Driver): _fds: dict[UUID, AsyncFile] = field(init=False, default_factory=dict) _metadata: dict[UUID, OpendalMetadata] = field(init=False, default_factory=dict) + # Track file/directory operations + _created_files: set[str] = field(init=False, default_factory=set) + _created_dirs: set[str] = field(init=False, default_factory=set) + @classmethod def client(cls) -> str: return "jumpstarter_driver_opendal.client.OpendalClient" @@ -43,12 +49,17 @@ async def open(self, /, path: str, mode: Mode) -> UUID: metadata = await self._operator.stat(path) except Exception: metadata = None + file = await self._operator.open(path, mode) uuid = uuid4() self._metadata[uuid] = metadata self._fds[uuid] = file + # Track file creation for any write mode (assume pre-existing files are remnants from failed cleanup) + if mode in ("wb", "w", "ab", "a"): + self._created_files.add(path) + return uuid @export @@ -143,6 +154,7 @@ async def remove_all(self, /, path: str): @validate_call(validate_return=True) async def create_dir(self, /, path: str): await self._operator.create_dir(path) + self._created_dirs.add(path) @export @validate_call(validate_return=True) @@ -202,6 +214,40 @@ async def copy_exporter_file(self, /, source: Path, target: str): break await dst.write(bs=data) + # Always track file creation (assume pre-existing files are just uncleaned remnants) + self._created_files.add(target) + + @export + async def get_created_files(self) -> list[str]: + """Get list of files that have been created during this session.""" + return list(self._created_files) + + @export + async def get_created_directories(self) -> list[str]: + """Get list of directories that have been created during this session.""" + return list(self._created_dirs) + + @export + async def get_all_created_resources(self) -> tuple[list[str], list[str]]: + """Get all resources created during this session as a tuple: (created_directories, created_files).""" + return (list(self._created_dirs), list(self._created_files)) + + + def close(self): + """Close driver and report what was created.""" + if self._created_files or self._created_dirs: + self.logger.debug( + "OpenDAL session summary - Created files: %d, Created directories: %d", + len(self._created_files), + len(self._created_dirs) + ) + if self._created_files: + self.logger.debug("Created files: %s", sorted(self._created_files)) + if self._created_dirs: + self.logger.debug("Created directories: %s", sorted(self._created_dirs)) + + super().close() + class FlasherInterface(metaclass=ABCMeta): @classmethod From 50a4690a2bef7d1c412c410ca8f8a3cc6f9df595 Mon Sep 17 00:00:00 2001 From: Michal Skrivanek Date: Fri, 26 Sep 2025 10:30:51 +0200 Subject: [PATCH 2/4] add remove_created_on_close parameter to OpenDAL to clean up created resources on close defaults to false. attempts to remove all files and directories created using "fs" scheme (local files on exporter filesystem) --- .../package-apis/drivers/opendal.yaml | 2 + .../jumpstarter_driver_http/driver_test.py | 58 ++++++++++++++++++- packages/jumpstarter-driver-opendal/README.md | 46 +++++++++++++++ .../jumpstarter_driver_opendal/driver.py | 48 +++++++++++++++ 4 files changed, 153 insertions(+), 1 deletion(-) diff --git a/docs/source/reference/package-apis/drivers/opendal.yaml b/docs/source/reference/package-apis/drivers/opendal.yaml index 7629779fa..31345f5be 100644 --- a/docs/source/reference/package-apis/drivers/opendal.yaml +++ b/docs/source/reference/package-apis/drivers/opendal.yaml @@ -5,3 +5,5 @@ config: scheme: "fs" kwargs: root: "/tmp/jumpstarter" + # Optional: automatically remove created files/directories when driver closes + remove_created_on_close: false diff --git a/packages/jumpstarter-driver-http/jumpstarter_driver_http/driver_test.py b/packages/jumpstarter-driver-http/jumpstarter_driver_http/driver_test.py index c459ab88a..bcae40d34 100644 --- a/packages/jumpstarter-driver-http/jumpstarter_driver_http/driver_test.py +++ b/packages/jumpstarter-driver-http/jumpstarter_driver_http/driver_test.py @@ -118,5 +118,61 @@ def test_opendal_tracking_methods(tmp_path, unused_tcp_port): assert filename in created_files assert created_dirs == [] - client.stop() + + +def test_opendal_cleanup_on_close(tmp_path): + """Test that OpenDAL driver can optionally remove created files on close.""" + from jumpstarter_driver_opendal.driver import Opendal + + # Create two separate directories + cleanup_dir = tmp_path / "cleanup_test" + no_cleanup_dir = tmp_path / "no_cleanup_test" + cleanup_dir.mkdir() + no_cleanup_dir.mkdir() + + # Test files + cleanup_filename = "cleanup_test.txt" + no_cleanup_filename = "no_cleanup_test.txt" + + # Test 1: Driver with cleanup enabled + cleanup_driver = Opendal( + scheme="fs", + kwargs={"root": str(cleanup_dir)}, + remove_created_on_close=True + ) + + # Manually create a file to simulate tracking + cleanup_driver._created_files.add(cleanup_filename) + test_file_path = cleanup_dir / cleanup_filename + test_file_path.write_text("test content") + + # Verify file exists + assert test_file_path.exists() + + # Close driver (should trigger cleanup) + cleanup_driver.close() + + # Verify file was removed + assert not test_file_path.exists(), "File should have been removed by cleanup" + + # Test 2: Driver with cleanup disabled (default) + no_cleanup_driver = Opendal( + scheme="fs", + kwargs={"root": str(no_cleanup_dir)}, + remove_created_on_close=False + ) + + # Manually create a file to simulate tracking + no_cleanup_driver._created_files.add(no_cleanup_filename) + test_file_path2 = no_cleanup_dir / no_cleanup_filename + test_file_path2.write_text("test content") + + # Verify file exists + assert test_file_path2.exists() + + # Close driver (should NOT trigger cleanup) + no_cleanup_driver.close() + + # Verify file still exists + assert test_file_path2.exists(), "File should remain without cleanup" diff --git a/packages/jumpstarter-driver-opendal/README.md b/packages/jumpstarter-driver-opendal/README.md index e5f919308..6c0a69ce9 100644 --- a/packages/jumpstarter-driver-opendal/README.md +++ b/packages/jumpstarter-driver-opendal/README.md @@ -18,6 +18,52 @@ Example configuration: :language: yaml ``` +### Configuration Parameters + +- **`scheme`** (required): The storage service type (e.g., "fs", "s3", "gcs"). See [OpenDAL services](https://docs.rs/opendal/latest/opendal/services/index.html) for supported options. +- **`kwargs`** (required): Service-specific configuration parameters passed to the OpenDAL operator. +- **`remove_created_on_close`** (optional, default: `false`): When enabled, automatically removes all files and directories created during the session when the driver is closed. + +### File/Directory Tracking and Cleanup + +The OpenDAL driver tracks all files and directories created during a session: + +- **File Creation**: Files opened in write mode (`"wb"`, `"w"`, `"ab"`, `"a"`) are tracked as created +- **Directory Creation**: Directories created via `create_dir()` are tracked +- **Cleanup Behavior**: When `remove_created_on_close: true`, all tracked files and directories are automatically removed when the driver closes +- **Cleanup Support**: Currently only supports filesystem cleanup (`scheme: "fs"`) + +#### Tracking API + +The driver provides methods to inspect what has been created: + +```python +# Get list of created files +created_files = await driver.get_created_files() + +# Get list of created directories +created_dirs = await driver.get_created_directories() + +# Get both as a tuple: (directories, files) +dirs, files = await driver.get_all_created_resources() +``` + +#### Use Cases + +**Temporary File Management:** +```yaml +# Enable cleanup for temporary storage +remove_created_on_close: true +``` + +**Persistent Storage:** +```yaml +# Disable cleanup to preserve files (default) +remove_created_on_close: false +``` + +**Note:** Pre-existing files that are written to are treated as "created" since they may be remnants from failed cleanup operations. + ## API Reference ### Examples diff --git a/packages/jumpstarter-driver-opendal/jumpstarter_driver_opendal/driver.py b/packages/jumpstarter-driver-opendal/jumpstarter_driver_opendal/driver.py index 053133ed5..c012f0c85 100644 --- a/packages/jumpstarter-driver-opendal/jumpstarter_driver_opendal/driver.py +++ b/packages/jumpstarter-driver-opendal/jumpstarter_driver_opendal/driver.py @@ -23,6 +23,7 @@ class Opendal(Driver): scheme: str kwargs: dict[str, str] + remove_created_on_close: bool = False _operator: AsyncOperator = field(init=False) _fds: dict[UUID, AsyncFile] = field(init=False, default_factory=dict) @@ -246,8 +247,55 @@ def close(self): if self._created_dirs: self.logger.debug("Created directories: %s", sorted(self._created_dirs)) + # Remove created resources if requested + if self.remove_created_on_close: + self._cleanup_created_resources() + super().close() + def _cleanup_created_resources(self): + """Remove all created files and directories.""" + # Only support filesystem cleanup for now (scheme == "fs") + if self.scheme != "fs": + self.logger.warning(f"Cleanup not supported for scheme '{self.scheme}' - only 'fs' is supported") + return + + import os + import shutil + + # Get root path from kwargs + root_path = self.kwargs.get("root", "/") + + # Remove files first + for file_path in self._created_files: + try: + full_path = os.path.join(root_path, file_path.lstrip("/")) + if os.path.exists(full_path) and os.path.isfile(full_path): + os.remove(full_path) + self.logger.debug(f"Removed created file: {file_path}") + else: + self.logger.debug(f"File already removed or not found: {file_path}") + except Exception as e: + self.logger.error(f"Failed to remove file {file_path}: {e}") + + # Remove directories (in reverse order to handle nested dirs) + for dir_path in sorted(self._created_dirs, reverse=True): + try: + full_path = os.path.join(root_path, dir_path.lstrip("/")) + if os.path.exists(full_path) and os.path.isdir(full_path): + # Only remove if empty + try: + os.rmdir(full_path) + self.logger.debug(f"Removed created directory: {dir_path}") + except OSError: + # Directory not empty, try to remove recursively if it was created by us + shutil.rmtree(full_path) + self.logger.debug(f"Removed created directory tree: {dir_path}") + else: + self.logger.debug(f"Directory already removed or not found: {dir_path}") + except Exception as e: + self.logger.error(f"Failed to remove directory {dir_path}: {e}") + class FlasherInterface(metaclass=ABCMeta): @classmethod From 9114255b151e3397be7d37ef80f7e6fb9d277b61 Mon Sep 17 00:00:00 2001 From: Michal Skrivanek Date: Fri, 26 Sep 2025 11:40:37 +0200 Subject: [PATCH 3/4] remove created HTTPServer and TFTP files by default so that when the session ends there are no leftovers. ISCSI driver defaults to False assuming that the disk files are meant to be persistent. --- .../jumpstarter_driver_flashers/driver.py | 4 ++-- packages/jumpstarter-driver-http/README.md | 20 ++++++++++++++++- .../jumpstarter_driver_http/driver.py | 7 +++++- packages/jumpstarter-driver-iscsi/README.md | 20 +++++++++++------ .../jumpstarter_driver_iscsi/driver.py | 7 +++++- .../jumpstarter_driver_ridesx/driver.py | 6 ++++- packages/jumpstarter-driver-tftp/README.md | 22 ++++++++++++------- .../jumpstarter_driver_tftp/driver.py | 7 +++++- 8 files changed, 71 insertions(+), 22 deletions(-) diff --git a/packages/jumpstarter-driver-flashers/jumpstarter_driver_flashers/driver.py b/packages/jumpstarter-driver-flashers/jumpstarter_driver_flashers/driver.py index 4d7704bab..a510c7807 100644 --- a/packages/jumpstarter-driver-flashers/jumpstarter_driver_flashers/driver.py +++ b/packages/jumpstarter-driver-flashers/jumpstarter_driver_flashers/driver.py @@ -31,11 +31,11 @@ def __post_init__(self): # Ensure required children are present if not already instantiated # in configuration if "tftp" not in self.children: - self.children["tftp"] = Tftp(root_dir=self.tftp_dir) + self.children["tftp"] = Tftp(root_dir=self.tftp_dir, remove_created_on_close=True) self.tftp = self.children["tftp"] if "http" not in self.children: - self.children["http"] = HttpServer(root_dir=self.http_dir) + self.children["http"] = HttpServer(root_dir=self.http_dir, remove_created_on_close=True) self.http = self.children["http"] # Ensure required children are present, the following are not auto-created diff --git a/packages/jumpstarter-driver-http/README.md b/packages/jumpstarter-driver-http/README.md index 0f1b7eae5..956f7dae3 100644 --- a/packages/jumpstarter-driver-http/README.md +++ b/packages/jumpstarter-driver-http/README.md @@ -18,9 +18,27 @@ export: http: type: jumpstarter_driver_http.driver.HttpServer config: - # Add required config parameters here + root_dir: "/var/www" + host: "0.0.0.0" + port: 8080 + timeout: 600 + remove_created_on_close: true # Clean up temporary files on close ``` +### Config parameters + +| Parameter | Description | Type | Required | Default | +| ----------------------- | ---------------------------------------------------------------- | ---- | -------- | ----------------- | +| root_dir | Root directory for serving files | str | no | "/var/www" | +| host | IP address to bind the server to | str | no | None (auto-detect)| +| port | Port number to listen on | int | no | 8080 | +| timeout | Request timeout in seconds | int | no | 600 | +| remove_created_on_close | Automatically remove created files/directories when driver closes| bool | no | true | + +### File Management + +The internal HTTP server driver automatically tracks files and directories created during the session. When `remove_created_on_close` is enabled (default), all tracked resources are cleaned up when the driver closes. + ## API Reference Add API documentation here. diff --git a/packages/jumpstarter-driver-http/jumpstarter_driver_http/driver.py b/packages/jumpstarter-driver-http/jumpstarter_driver_http/driver.py index ff39c6dda..974152258 100644 --- a/packages/jumpstarter-driver-http/jumpstarter_driver_http/driver.py +++ b/packages/jumpstarter-driver-http/jumpstarter_driver_http/driver.py @@ -27,6 +27,7 @@ class HttpServer(Driver): host: str | None = field(default=None) port: int = 8080 timeout: int = field(default=600) + remove_created_on_close: bool = True # Clean up temporary web files by default app: web.Application = field(init=False, default_factory=web.Application) runner: Optional[web.AppRunner] = field(init=False, default=None) @@ -36,7 +37,11 @@ def __post_init__(self): os.makedirs(self.root_dir, exist_ok=True) - self.children["storage"] = Opendal(scheme="fs", kwargs={"root": self.root_dir}) + self.children["storage"] = Opendal( + scheme="fs", + kwargs={"root": self.root_dir}, + remove_created_on_close=self.remove_created_on_close + ) self.app.router.add_routes( [ web.static("/", self.root_dir), diff --git a/packages/jumpstarter-driver-iscsi/README.md b/packages/jumpstarter-driver-iscsi/README.md index e40b1780f..1e3dd844a 100644 --- a/packages/jumpstarter-driver-iscsi/README.md +++ b/packages/jumpstarter-driver-iscsi/README.md @@ -41,18 +41,24 @@ export: config: root_dir: "/var/lib/iscsi" target_name: "demo" + remove_created_on_close: false # Keep disk images persistent (default) # When size_mb is 0 a pre-existing file size is used. ``` ### Config parameters -| Parameter | Description | Type | Required | Default | -| ----------- | ---------------------------------------------------------------------------- | ---- | -------- | --------------------------------- | -| `root_dir` | Directory where image files will be stored. | str | no | `/var/lib/iscsi` | -| `iqn_prefix`| IQN prefix to use when building the target IQN. | str | no | `iqn.2024-06.dev.jumpstarter` | -| `target_name`| The target name appended to the IQN prefix. | str | no | `target1` | -| `host` | IP address to bind the target to. Empty string will auto-detect default IP. | str | no | *auto* | -| `port` | TCP port the target listens on. | int | no | `3260` | +| Parameter | Description | Type | Required | Default | +| ----------------------- | ---------------------------------------------------------------- | ---- | -------- | ----------------------------- | +| `root_dir` | Directory where image files will be stored | str | no | `/var/lib/iscsi` | +| `iqn_prefix` | IQN prefix to use when building the target IQN | str | no | `iqn.2024-06.dev.jumpstarter` | +| `target_name` | The target name appended to the IQN prefix | str | no | `target1` | +| `host` | IP address to bind the target to. Empty string will auto-detect | str | no | *auto* | +| `port` | TCP port the target listens on | int | no | `3260` | +| `remove_created_on_close`| Automatically remove created files/directories when driver closes| bool | no | false | + +### File Management + +The iSCSI server driver automatically tracks disk image files and directories created during the session. By default, `remove_created_on_close` is set to `false` to preserve disk images that are typically reused across test sessions. Set to `true` if you want temporary disk images to be cleaned up automatically. ## API Reference diff --git a/packages/jumpstarter-driver-iscsi/jumpstarter_driver_iscsi/driver.py b/packages/jumpstarter-driver-iscsi/jumpstarter_driver_iscsi/driver.py index f74183de1..8c5bea4a2 100644 --- a/packages/jumpstarter-driver-iscsi/jumpstarter_driver_iscsi/driver.py +++ b/packages/jumpstarter-driver-iscsi/jumpstarter_driver_iscsi/driver.py @@ -46,6 +46,7 @@ class ISCSI(Driver): target_name: str = "target1" host: str = field(default="") port: int = 3260 + remove_created_on_close: bool = False # Keep disk images persistent by default _rtsroot: Optional[RTSRoot] = field(init=False, default=None) _target: Optional[Target] = field(init=False, default=None) @@ -60,7 +61,11 @@ def __post_init__(self): os.makedirs(self.root_dir, exist_ok=True) - self.children["storage"] = Opendal(scheme="fs", kwargs={"root": self.root_dir}) + self.children["storage"] = Opendal( + scheme="fs", + kwargs={"root": self.root_dir}, + remove_created_on_close=self.remove_created_on_close + ) self.storage = self.children["storage"] if self.host == "": diff --git a/packages/jumpstarter-driver-ridesx/jumpstarter_driver_ridesx/driver.py b/packages/jumpstarter-driver-ridesx/jumpstarter_driver_ridesx/driver.py index 0c8e347df..4ee1dae3f 100644 --- a/packages/jumpstarter-driver-ridesx/jumpstarter_driver_ridesx/driver.py +++ b/packages/jumpstarter-driver-ridesx/jumpstarter_driver_ridesx/driver.py @@ -25,7 +25,11 @@ def __post_init__(self): raise ConfigurationError("'serial' instance is required") Path(self.storage_dir).mkdir(parents=True, exist_ok=True) - self.children["storage"] = Opendal(scheme="fs", kwargs={"root": self.storage_dir}) + self.children["storage"] = Opendal( + scheme="fs", + kwargs={"root": self.storage_dir}, + remove_created_on_close=True # Clean up temporary firmware files on close + ) @classmethod def client(cls) -> str: diff --git a/packages/jumpstarter-driver-tftp/README.md b/packages/jumpstarter-driver-tftp/README.md index 67d732a09..4133f821e 100644 --- a/packages/jumpstarter-driver-tftp/README.md +++ b/packages/jumpstarter-driver-tftp/README.md @@ -19,18 +19,24 @@ export: tftp: type: jumpstarter_driver_tftp.driver.Tftp config: - root_dir: /var/lib/tftpboot # Directory to serve files from - host: 192.168.1.100 # Host IP to bind to (optional) - port: 69 # Port to listen on (optional) + root_dir: /var/lib/tftpboot # Directory to serve files from + host: 192.168.1.100 # Host IP to bind to (optional) + port: 69 # Port to listen on (optional) + remove_created_on_close: true # Clean up temporary boot files (default) ``` ### Config parameters -| Parameter | Description | Type | Required | Default | -| --------- | ---------------------------------- | ---- | -------- | ------------------- | -| root_dir | Root directory for the TFTP server | str | no | "/var/lib/tftpboot" | -| host | IP address to bind the server to | str | no | auto-detect | -| port | Port number to listen on | int | no | 69 | +| Parameter | Description | Type | Required | Default | +| ----------------------- | ---------------------------------------------------------------- | ---- | -------- | ------------------- | +| root_dir | Root directory for the TFTP server | str | no | "/var/lib/tftpboot" | +| host | IP address to bind the server to | str | no | auto-detect | +| port | Port number to listen on | int | no | 69 | +| remove_created_on_close | Automatically remove created files/directories when driver closes| bool | no | true | + +### File Management + +The TFTP server driver automatically tracks files and directories created during the session. By default, `remove_created_on_close` is set to `true` to clean up temporary boot files automatically. Set to `false` if you want to preserve boot files and firmware images that are reused across sessions. ## API Reference diff --git a/packages/jumpstarter-driver-tftp/jumpstarter_driver_tftp/driver.py b/packages/jumpstarter-driver-tftp/jumpstarter_driver_tftp/driver.py index 36fedb9c3..9e3b4de77 100644 --- a/packages/jumpstarter-driver-tftp/jumpstarter_driver_tftp/driver.py +++ b/packages/jumpstarter-driver-tftp/jumpstarter_driver_tftp/driver.py @@ -39,6 +39,7 @@ class Tftp(Driver): root_dir: str = "/var/lib/tftpboot" host: str = field(default="") port: int = 69 + remove_created_on_close: bool = True # Clean up temporary boot files by default server: Optional["TftpServer"] = field(init=False, default=None) server_thread: Optional[threading.Thread] = field(init=False, default=None) _shutdown_event: threading.Event = field(init=False, default_factory=threading.Event) @@ -51,7 +52,11 @@ def __post_init__(self): os.makedirs(self.root_dir, exist_ok=True) - self.children["storage"] = Opendal(scheme="fs", kwargs={"root": self.root_dir}) + self.children["storage"] = Opendal( + scheme="fs", + kwargs={"root": self.root_dir}, + remove_created_on_close=self.remove_created_on_close + ) self.storage = self.children["storage"] if self.host == "": From 29ee63c37484d1ef98d0e57719a821a5122a49dd Mon Sep 17 00:00:00 2001 From: Michal Skrivanek Date: Mon, 29 Sep 2025 10:01:25 +0200 Subject: [PATCH 4/4] track rename, delete, copy operations as well and simplify tracking to a single resource "path". --- .../package-apis/drivers/opendal.yaml | 1 + .../jumpstarter_driver_http/driver_test.py | 29 ++--- packages/jumpstarter-driver-iscsi/README.md | 2 +- packages/jumpstarter-driver-opendal/README.md | 28 +++-- .../jumpstarter_driver_opendal/client.py | 30 +---- .../jumpstarter_driver_opendal/driver.py | 117 ++++++++---------- .../jumpstarter_driver_opendal/driver_test.py | 87 +++++++++++++ 7 files changed, 174 insertions(+), 120 deletions(-) diff --git a/docs/source/reference/package-apis/drivers/opendal.yaml b/docs/source/reference/package-apis/drivers/opendal.yaml index 31345f5be..34bfe09bc 100644 --- a/docs/source/reference/package-apis/drivers/opendal.yaml +++ b/docs/source/reference/package-apis/drivers/opendal.yaml @@ -6,4 +6,5 @@ config: kwargs: root: "/tmp/jumpstarter" # Optional: automatically remove created files/directories when driver closes + # Note: all tracked paths are normalized by stripping leading/trailing slashes remove_created_on_close: false diff --git a/packages/jumpstarter-driver-http/jumpstarter_driver_http/driver_test.py b/packages/jumpstarter-driver-http/jumpstarter_driver_http/driver_test.py index bcae40d34..7138811cf 100644 --- a/packages/jumpstarter-driver-http/jumpstarter_driver_http/driver_test.py +++ b/packages/jumpstarter-driver-http/jumpstarter_driver_http/driver_test.py @@ -31,8 +31,6 @@ async def test_http_server(http, tmp_path): uploaded_url = http.put_file(filename, tmp_path / "src") - print(http.storage.stat(filename)) - files = list(http.storage.list("/")) assert filename in files @@ -80,8 +78,8 @@ async def test_opendal_tracking_on_http_server_close(tmp_path, unused_tcp_port, assert filename in files # Get the tracking info before close - created_files = client.storage.get_created_files() - assert filename in created_files + created_resources = client.storage.get_created_resources() + assert filename in created_resources client.stop() # When exiting the context manager, HttpServer.close() is called, @@ -89,7 +87,7 @@ async def test_opendal_tracking_on_http_server_close(tmp_path, unused_tcp_port, # The main functionality test is that the file was tracked as created # The close() method logging might not be captured due to async cleanup timing - # but we've verified the tracking works by checking get_created_files() above + # but we've verified the tracking works by checking get_created_resources() above def test_opendal_tracking_methods(tmp_path, unused_tcp_port): @@ -97,11 +95,9 @@ def test_opendal_tracking_methods(tmp_path, unused_tcp_port): with serve(HttpServer(root_dir=str(tmp_path), port=unused_tcp_port)) as client: client.start() - # Initially, no files should be tracked - created_files = client.storage.get_created_files() - created_dirs = client.storage.get_created_directories() - assert created_files == [] - assert created_dirs == [] + # Initially, no resources should be tracked + created_resources = client.storage.get_created_resources() + assert created_resources == set() # Write a file (which creates it) filename = "direct_test.txt" @@ -110,13 +106,8 @@ def test_opendal_tracking_methods(tmp_path, unused_tcp_port): client.put_file(filename, tmp_path / "src") # Check tracking - created_files = client.storage.get_created_files() - assert filename in created_files - - # Test get_all_created_resources - created_dirs, created_files = client.storage.get_all_created_resources() - assert filename in created_files - assert created_dirs == [] + created_resources = client.storage.get_created_resources() + assert filename in created_resources client.stop() @@ -143,7 +134,7 @@ def test_opendal_cleanup_on_close(tmp_path): ) # Manually create a file to simulate tracking - cleanup_driver._created_files.add(cleanup_filename) + cleanup_driver._created_paths.add(cleanup_filename) test_file_path = cleanup_dir / cleanup_filename test_file_path.write_text("test content") @@ -164,7 +155,7 @@ def test_opendal_cleanup_on_close(tmp_path): ) # Manually create a file to simulate tracking - no_cleanup_driver._created_files.add(no_cleanup_filename) + no_cleanup_driver._created_paths.add(no_cleanup_filename) test_file_path2 = no_cleanup_dir / no_cleanup_filename test_file_path2.write_text("test content") diff --git a/packages/jumpstarter-driver-iscsi/README.md b/packages/jumpstarter-driver-iscsi/README.md index 1e3dd844a..c145b9247 100644 --- a/packages/jumpstarter-driver-iscsi/README.md +++ b/packages/jumpstarter-driver-iscsi/README.md @@ -52,7 +52,7 @@ export: | `root_dir` | Directory where image files will be stored | str | no | `/var/lib/iscsi` | | `iqn_prefix` | IQN prefix to use when building the target IQN | str | no | `iqn.2024-06.dev.jumpstarter` | | `target_name` | The target name appended to the IQN prefix | str | no | `target1` | -| `host` | IP address to bind the target to. Empty string will auto-detect | str | no | *auto* | +| `host` | IP address to bind the target to. Empty string will auto-detect | str | no | _auto_ | | `port` | TCP port the target listens on | int | no | `3260` | | `remove_created_on_close`| Automatically remove created files/directories when driver closes| bool | no | false | diff --git a/packages/jumpstarter-driver-opendal/README.md b/packages/jumpstarter-driver-opendal/README.md index 6c0a69ce9..234b13824 100644 --- a/packages/jumpstarter-driver-opendal/README.md +++ b/packages/jumpstarter-driver-opendal/README.md @@ -28,24 +28,26 @@ Example configuration: The OpenDAL driver tracks all files and directories created during a session: -- **File Creation**: Files opened in write mode (`"wb"`, `"w"`, `"ab"`, `"a"`) are tracked as created -- **Directory Creation**: Directories created via `create_dir()` are tracked -- **Cleanup Behavior**: When `remove_created_on_close: true`, all tracked files and directories are automatically removed when the driver closes -- **Cleanup Support**: Currently only supports filesystem cleanup (`scheme: "fs"`) +- **File Creation**: Files opened in write modes (`"wb"`, `"w"`, `"ab"`, `"a"`) +- **Directory Creation**: Directories created via `create_dir()` +- **Copy Operations**: Target files/directories from `copy()` operations +- **Rename Operations**: Target files/directories from `rename()` operations (source is removed from tracking) -#### Tracking API +**Automatic Cleanup**: The tracking is automatically updated when resources are removed: +- **Delete Operations**: `delete()` removes the path from tracking +- **Remove Operations**: `remove_all()` removes the path from tracking -The driver provides methods to inspect what has been created: +**Cleanup Behavior**: When `remove_created_on_close: true`, all tracked files and directories are automatically removed when the driver closes (filesystem only) -```python -# Get list of created files -created_files = await driver.get_created_files() +### Tracking API -# Get list of created directories -created_dirs = await driver.get_created_directories() +```python +# Get all created resources (files and directories) +created_resources = await driver.get_created_resources() # Returns set[str] -# Get both as a tuple: (directories, files) -dirs, files = await driver.get_all_created_resources() +# Example usage +for path in created_resources: + print(f"Created: {path}") ``` #### Use Cases diff --git a/packages/jumpstarter-driver-opendal/jumpstarter_driver_opendal/client.py b/packages/jumpstarter-driver-opendal/jumpstarter_driver_opendal/client.py index 5b8e6da73..045484c5a 100644 --- a/packages/jumpstarter-driver-opendal/jumpstarter_driver_opendal/client.py +++ b/packages/jumpstarter-driver-opendal/jumpstarter_driver_opendal/client.py @@ -6,7 +6,7 @@ from dataclasses import dataclass from io import BytesIO from pathlib import Path -from typing import List, cast +from typing import cast from urllib.parse import urlparse from uuid import UUID @@ -395,34 +395,14 @@ def capability(self, /) -> Capability: return self.call("capability") @validate_call(validate_return=True) - def get_created_files(self) -> List[str]: + def get_created_resources(self) -> set[str]: """ - Get list of files that have been created during this session. + Get set of all paths that have been created during this session. Returns: - List[str]: List of file paths that were created + set[str]: Set of all paths (files and directories) that were created """ - return self.call("get_created_files") - - @validate_call(validate_return=True) - def get_created_directories(self) -> List[str]: - """ - Get list of directories that have been created during this session. - - Returns: - List[str]: List of directory paths that were created - """ - return self.call("get_created_directories") - - @validate_call(validate_return=True) - def get_all_created_resources(self) -> tuple[List[str], List[str]]: - """ - Get all resources created during this session as a tuple: (created_directories, created_files). - - Returns: - tuple[List[str], List[str]]: Tuple of (created_directories, created_files) - """ - return self.call("get_all_created_resources") + return self.call("get_created_resources") def cli(self): # noqa: C901 diff --git a/packages/jumpstarter-driver-opendal/jumpstarter_driver_opendal/driver.py b/packages/jumpstarter-driver-opendal/jumpstarter_driver_opendal/driver.py index c012f0c85..f79357825 100644 --- a/packages/jumpstarter-driver-opendal/jumpstarter_driver_opendal/driver.py +++ b/packages/jumpstarter-driver-opendal/jumpstarter_driver_opendal/driver.py @@ -29,9 +29,19 @@ class Opendal(Driver): _fds: dict[UUID, AsyncFile] = field(init=False, default_factory=dict) _metadata: dict[UUID, OpendalMetadata] = field(init=False, default_factory=dict) - # Track file/directory operations - _created_files: set[str] = field(init=False, default_factory=set) - _created_dirs: set[str] = field(init=False, default_factory=set) + # Track all created paths (files and directories) + _created_paths: set[str] = field(init=False, default_factory=set) + + def _normalize_path(self, path: str) -> str: + """Normalize path by handling backslashes, dot segments, and redundant slashes.""" + if not path: + return path + + import posixpath + + path = path.replace('\\', '/') + normalized = posixpath.normpath(path).strip('/') + return normalized if normalized != '.' else '' @classmethod def client(cls) -> str: @@ -57,9 +67,9 @@ async def open(self, /, path: str, mode: Mode) -> UUID: self._metadata[uuid] = metadata self._fds[uuid] = file - # Track file creation for any write mode (assume pre-existing files are remnants from failed cleanup) + # Track path creation for any write mode (assume pre-existing files are remnants from failed cleanup) if mode in ("wb", "w", "ab", "a"): - self._created_files.add(path) + self._created_paths.add(self._normalize_path(path)) return uuid @@ -140,27 +150,32 @@ async def hash(self, /, path: str, algo: HashAlgo = "sha256") -> str: @validate_call(validate_return=True) async def copy(self, /, source: str, target: str): await self._operator.copy(source, target) + self._created_paths.add(self._normalize_path(target)) @export @validate_call(validate_return=True) async def rename(self, /, source: str, target: str): await self._operator.rename(source, target) + self._created_paths.discard(self._normalize_path(source)) + self._created_paths.add(self._normalize_path(target)) @export @validate_call(validate_return=True) async def remove_all(self, /, path: str): await self._operator.remove_all(path) + self._created_paths.discard(self._normalize_path(path)) @export @validate_call(validate_return=True) async def create_dir(self, /, path: str): await self._operator.create_dir(path) - self._created_dirs.add(path) + self._created_paths.add(self._normalize_path(path)) @export @validate_call(validate_return=True) async def delete(self, /, path: str): await self._operator.delete(path) + self._created_paths.discard(self._normalize_path(path)) @export @validate_call(validate_return=True) @@ -215,86 +230,64 @@ async def copy_exporter_file(self, /, source: Path, target: str): break await dst.write(bs=data) - # Always track file creation (assume pre-existing files are just uncleaned remnants) - self._created_files.add(target) - - @export - async def get_created_files(self) -> list[str]: - """Get list of files that have been created during this session.""" - return list(self._created_files) - - @export - async def get_created_directories(self) -> list[str]: - """Get list of directories that have been created during this session.""" - return list(self._created_dirs) + # Always track path creation (assume pre-existing files are just uncleaned remnants) + self._created_paths.add(self._normalize_path(target)) @export - async def get_all_created_resources(self) -> tuple[list[str], list[str]]: - """Get all resources created during this session as a tuple: (created_directories, created_files).""" - return (list(self._created_dirs), list(self._created_files)) + async def get_created_resources(self) -> set[str]: + """Get set of all paths that have been created during this session.""" + return self._created_paths.copy() def close(self): """Close driver and report what was created.""" - if self._created_files or self._created_dirs: + if self._created_paths: self.logger.debug( - "OpenDAL session summary - Created files: %d, Created directories: %d", - len(self._created_files), - len(self._created_dirs) + "OpenDAL session summary - Created paths: %d", + len(self._created_paths) ) - if self._created_files: - self.logger.debug("Created files: %s", sorted(self._created_files)) - if self._created_dirs: - self.logger.debug("Created directories: %s", sorted(self._created_dirs)) + self.logger.debug("Created paths: %s", sorted(self._created_paths)) # Remove created resources if requested if self.remove_created_on_close: - self._cleanup_created_resources() + if self.scheme == "fs": + self._cleanup_created_resources() + else: + self.logger.warning(f"Cleanup not supported for scheme '{self.scheme}' - only 'fs' is supported") super().close() def _cleanup_created_resources(self): """Remove all created files and directories.""" - # Only support filesystem cleanup for now (scheme == "fs") - if self.scheme != "fs": - self.logger.warning(f"Cleanup not supported for scheme '{self.scheme}' - only 'fs' is supported") - return import os import shutil # Get root path from kwargs - root_path = self.kwargs.get("root", "/") - - # Remove files first - for file_path in self._created_files: - try: - full_path = os.path.join(root_path, file_path.lstrip("/")) - if os.path.exists(full_path) and os.path.isfile(full_path): - os.remove(full_path) - self.logger.debug(f"Removed created file: {file_path}") - else: - self.logger.debug(f"File already removed or not found: {file_path}") - except Exception as e: - self.logger.error(f"Failed to remove file {file_path}: {e}") + root_path = os.path.abspath(self.kwargs.get("root", "/")) - # Remove directories (in reverse order to handle nested dirs) - for dir_path in sorted(self._created_dirs, reverse=True): + # Remove all paths in reverse order (handles nested directories properly) + for path in sorted(self._created_paths, reverse=True): try: - full_path = os.path.join(root_path, dir_path.lstrip("/")) - if os.path.exists(full_path) and os.path.isdir(full_path): - # Only remove if empty - try: - os.rmdir(full_path) - self.logger.debug(f"Removed created directory: {dir_path}") - except OSError: - # Directory not empty, try to remove recursively if it was created by us - shutil.rmtree(full_path) - self.logger.debug(f"Removed created directory tree: {dir_path}") - else: - self.logger.debug(f"Directory already removed or not found: {dir_path}") + # Safe-join: avoid leading '/' escaping root and re-abspath joined path + full_path = os.path.abspath(os.path.join(root_path, str(path).lstrip("/"))) + # Safety check: ensure resolved path is within root_path but not the root itself + if full_path == root_path or not full_path.startswith(root_path + os.sep): + continue + + if os.path.exists(full_path): + if os.path.isdir(full_path): + try: + os.rmdir(full_path) + self.logger.debug(f"Removed created directory: {path}") + except OSError: + shutil.rmtree(full_path) + self.logger.debug(f"Removed created directory tree: {path}") + else: + os.remove(full_path) + self.logger.debug(f"Removed created file: {path}") except Exception as e: - self.logger.error(f"Failed to remove directory {dir_path}: {e}") + self.logger.error(f"Failed to remove path {path}: {e}") class FlasherInterface(metaclass=ABCMeta): diff --git a/packages/jumpstarter-driver-opendal/jumpstarter_driver_opendal/driver_test.py b/packages/jumpstarter-driver-opendal/jumpstarter_driver_opendal/driver_test.py index 6e5a33c21..2668b1760 100644 --- a/packages/jumpstarter-driver-opendal/jumpstarter_driver_opendal/driver_test.py +++ b/packages/jumpstarter-driver-opendal/jumpstarter_driver_opendal/driver_test.py @@ -235,3 +235,90 @@ def do_GET(self): client.write_file(fs, "test") server.shutdown() + + +def test_directory_path_normalization(tmp_path): + """Test that directory paths are normalized without trailing slashes for consistent tracking.""" + from jumpstarter_driver_opendal.driver import Opendal + + driver = Opendal( + scheme="fs", + kwargs={"root": str(tmp_path)}, + remove_created_on_close=False + ) + + # Test various directory path formats including enhanced normalization cases + test_dirs = [ + "test_dir", # No slashes + "test_dir2/", # With trailing slash + "/test_dir3", # With leading slash + "/test_dir4/", # With both slashes + "nested/dir", # Nested, no slashes + "/nested/dir2/", # Nested, with both slashes + "dir\\backslash", # Windows backslash + "./current_dir", # Current directory reference + "parent/../simple", # Parent directory reference + "//double//slash//path", # Multiple redundant slashes + ] + + # Create directories with different path formats + for dir_path in test_dirs: + # Simulate directory creation (we can't easily test async create_dir in sync test) + # So we'll test the normalization logic directly + driver._created_paths.add(driver._normalize_path(dir_path)) + + # Verify all paths are normalized without leading/trailing slashes + created_paths = list(driver._created_paths) + expected_normalized = [ + "test_dir", + "test_dir2", + "test_dir3", + "test_dir4", + "nested/dir", + "nested/dir2", + "dir/backslash", # Windows backslash becomes forward slash + "current_dir", # ./current_dir becomes current_dir + "simple", # parent/../simple becomes simple + "double/slash/path", # //double//slash//path becomes double/slash/path + ] + + assert sorted(created_paths) == sorted(expected_normalized) + + # Verify no duplicates when same directory is added with different formats + driver._created_paths.clear() + + # Add same directory with different slash combinations + driver._created_paths.add(driver._normalize_path("same_dir")) + driver._created_paths.add(driver._normalize_path("same_dir/")) + driver._created_paths.add(driver._normalize_path("/same_dir")) + driver._created_paths.add(driver._normalize_path("/same_dir/")) + + created_paths = list(driver._created_paths) + assert created_paths == ["same_dir"] + assert len(created_paths) == 1 # No duplicates + + +def test_copy_and_rename_tracking(tmp_path): + """Test that copy() and rename() operations track targets (files and directories) as created.""" + from jumpstarter_driver_opendal.driver import Opendal + + driver = Opendal( + scheme="fs", + kwargs={"root": str(tmp_path)}, + remove_created_on_close=False + ) + + # Test unified path tracking + driver._created_paths.add("copied_file.txt") # Simulate file copy operation tracking + driver._created_paths.add("renamed_file.txt") # Simulate file rename operation tracking + driver._created_paths.add("copied_dir") # Simulate directory copy operation tracking + driver._created_paths.add("renamed_dir") # Simulate directory rename operation tracking + + # Verify all paths are tracked in the unified set + created_paths = driver._created_paths + + assert "copied_file.txt" in created_paths + assert "renamed_file.txt" in created_paths + assert "copied_dir" in created_paths + assert "renamed_dir" in created_paths + assert len(created_paths) == 4