Skip to content

fix: clean up container-owned storage#256

Open
nineinchnick wants to merge 1 commit into
floci-io:mainfrom
iceguard:user/jwas/fix-persistent-storage-cleanup
Open

fix: clean up container-owned storage#256
nineinchnick wants to merge 1 commit into
floci-io:mainfrom
iceguard:user/jwas/fix-persistent-storage-cleanup

Conversation

@nineinchnick

Copy link
Copy Markdown

Why

  • Host-backed Floci storage can contain root-owned entries created by sibling service containers such as RDS.
  • FlociContainer currently stops before traversing that storage, so an inaccessible directory can raise UncheckedIOException and fail an otherwise successful test run.
  • Silent File.delete failures can also leave temporary storage behind.

Approach

  • Restore deletable permissions from the running Floci container before shutdown.
  • Delete paths with the NIO API and log any remaining best-effort cleanup failure.
  • Cover cleanup with a container-owned, mode-000 storage regression case.

@greptile-apps

greptile-apps Bot commented Jul 14, 2026

Copy link
Copy Markdown

Greptile Summary

This PR fixes two related cleanup bugs in FlociContainer.stop(): root-owned paths created by sibling containers could make the old File.delete traversal throw UncheckedIOException, and silent File.delete failures silently left temp storage behind.

  • Before stopping, the container now runs chmod -R a+rwX /app/data on itself to restore permissions on any root-owned paths, with separate handling for IOException and InterruptedException.
  • deletePersistentStorage is rewritten with Files.walkFileTree / SimpleFileVisitor so that every individual delete or traversal failure is logged and the walk continues rather than aborting.
  • A new test creates a mode-000 subdirectory via execInContainer, then asserts that persistentStorage no longer exists after stop(), directly covering the regression scenario.

Confidence Score: 5/5

Safe to merge — the change is a targeted cleanup fix with no impact on normal container lifecycle paths and new test coverage for the exact regression scenario.

The stop() refactor is well-scoped: pre-shutdown chmod is gated behind isRunning() and all exceptions are caught and logged rather than propagated; the walkFileTree visitor continues on every per-path failure matching the stated best-effort intent; and the new test exercises the mode-000 regression path end-to-end. No correctness issues found.

No files require special attention.

Important Files Changed

Filename Overview
testcontainers-floci/src/main/java/io/floci/testcontainers/FlociContainer.java Adds pre-shutdown chmod inside the running container to restore permissions on root-owned paths, then replaces the silent File.delete stream with a Files.walkFileTree visitor that logs each deletion failure and continues — addressing both the UncheckedIOException and the silent-failure regressions.
testcontainers-floci/src/test/java/io/floci/testcontainers/FlociContainerTest.java Adds regression test that creates a mode-000 subdirectory inside the Floci data volume then asserts the entire persistent storage path is gone after stop(), covering the scenario described in the PR.

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
    participant Caller
    participant FlociContainer
    participant DockerDaemon
    participant HostFS

    Caller->>FlociContainer: stop()
    FlociContainer->>FlociContainer: preparePersistentStorageForCleanup()
    alt hostPersistentPath present AND isRunning()
        FlociContainer->>DockerDaemon: execInContainer("chmod -R a+rwX /app/data")
        DockerDaemon-->>FlociContainer: ExecResult (exitCode / stderr)
        alt "exitCode != 0"
            FlociContainer->>FlociContainer: logger.warn(...)
        end
    end
    FlociContainer->>DockerDaemon: super.stop()
    DockerDaemon-->>FlociContainer: container stopped
    FlociContainer->>FlociContainer: deletePersistentStorage()
    alt hostPersistentPath present
        FlociContainer->>HostFS: Files.walkFileTree(path, visitor)
        loop each file
            HostFS-->>FlociContainer: visitFile(file, attrs)
            FlociContainer->>HostFS: Files.deleteIfExists(file)
        end
        loop each directory (post-order)
            HostFS-->>FlociContainer: postVisitDirectory(dir, failure?)
            FlociContainer->>HostFS: Files.deleteIfExists(dir)
        end
        alt visitFileFailed / IOException
            FlociContainer->>FlociContainer: logger.warn(...) + CONTINUE
        end
    end
    FlociContainer-->>Caller: (done)
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
    participant Caller
    participant FlociContainer
    participant DockerDaemon
    participant HostFS

    Caller->>FlociContainer: stop()
    FlociContainer->>FlociContainer: preparePersistentStorageForCleanup()
    alt hostPersistentPath present AND isRunning()
        FlociContainer->>DockerDaemon: execInContainer("chmod -R a+rwX /app/data")
        DockerDaemon-->>FlociContainer: ExecResult (exitCode / stderr)
        alt "exitCode != 0"
            FlociContainer->>FlociContainer: logger.warn(...)
        end
    end
    FlociContainer->>DockerDaemon: super.stop()
    DockerDaemon-->>FlociContainer: container stopped
    FlociContainer->>FlociContainer: deletePersistentStorage()
    alt hostPersistentPath present
        FlociContainer->>HostFS: Files.walkFileTree(path, visitor)
        loop each file
            HostFS-->>FlociContainer: visitFile(file, attrs)
            FlociContainer->>HostFS: Files.deleteIfExists(file)
        end
        loop each directory (post-order)
            HostFS-->>FlociContainer: postVisitDirectory(dir, failure?)
            FlociContainer->>HostFS: Files.deleteIfExists(dir)
        end
        alt visitFileFailed / IOException
            FlociContainer->>FlociContainer: logger.warn(...) + CONTINUE
        end
    end
    FlociContainer-->>Caller: (done)
Loading

Reviews (2): Last reviewed commit: "fix: clean up container-owned storage" | Re-trigger Greptile

Floci child containers can create root-owned entries in the host-backed
data directory. Traversing those entries after the Floci container has
stopped can raise an unchecked access error and fail an otherwise
successful test run.

Restore deletable permissions while Floci still has access, then report
rather than propagate any remaining best-effort cleanup failure.
@nineinchnick
nineinchnick force-pushed the user/jwas/fix-persistent-storage-cleanup branch from 2387bf4 to 5d89985 Compare July 14, 2026 10:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant