Skip to content

fix: handle container clean failing on read-only named volume mounts - #2228

Merged
jglogan merged 1 commit into
apple:mainfrom
zdk:fix/2224-clean-readonly-mounts
Sep 2, 2026
Merged

jglogan merged 1 commit into
apple:mainfrom
zdk:fix/2224-clean-readonly-mounts

Conversation

@zdk

@zdk zdk commented Sep 2, 2026 •

Copy link
Copy Markdown
Contributor

Type of Change

  • Bug fix

Motivation and Context

container clean fails with filesystemOperation trim failed when the container has a named volume mounted read-only, because the runtime helper tries to trim every block mount and throws on the first error.

The fix:

  • skips the rootfs when the container was created with --read-only, and skips block mounts whose options include ro.
  • keeps trimming the remaining targets after a failure, then throws one error listing each failed path with its underlying error.

Fixes #2224

Testing

  • Tested locally
  • Added/updated tests

- Skip trim on read-only rootfs and read-only block mounts
- Keep cleaning after a failure, then throw one error listing failed paths
- Add integration tests for read-only rootfs, read-only volume, and mixed volumes

Fixes apple#2224
@zdk zdk changed the title Fix container clean failing on read-only named volume mounts fix: container clean failing on read-only named volume mounts Sep 2, 2026
@zdk zdk changed the title fix: container clean failing on read-only named volume mounts fix: handle container clean failing on read-only named volume mounts Sep 2, 2026
@github-actions

github-actions Bot commented Sep 2, 2026

Copy link
Copy Markdown

Code Coverage

Tier Line Coverage
Unit 24.87%
Integration 66.61%
Combined 76.11%

@jglogan jglogan left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@zdk This looks good, thank you!

@jglogan
jglogan merged commit c82fe3c into apple:main Sep 2, 2026
4 checks passed
daphnediane added a commit to daphnediane/container-distro that referenced this pull request Oct 6, 2026
Cross-references the open roadmap items with their new tracking issues,
moves the container-internals re-verification into the release
checklist (issue #10 closed as a process check), and migrates all
markdown to the reference-link convention.

- TODO.md: link each open item to its issue (#1-#9, #11-#14); drop
  "Upstream-internal assumptions" (now a release-process step)
- Gap docs + remote-wsl-interop.md: "**Tracking:**" header lines
- security.md: pre-1.0 checklist items 5 and 6 link #12/#13
- install.md: note no brew/nix/pkg packages yet (#14)
- release.md: re-verify appRoot internals + extend
  VERIFIED_CONTAINER_MINOR when container's minor version changes
- container-internals.md: re-verify checklist points at release.md
- ops.rs migrate doc comment links #1; VERIFIED_CONTAINER_MINOR doc
  notes the release-checklist step
- All inline `[text](target)` links converted to `[text]` shortcut
  references with targets collected in a bibliography at each file's
  end; bare text kept when the path is the text
- External issue refs use `[owner/repo#N]` (e.g. [apple/container#1805]);
  bare `[#N]` is reserved for this repo's issues
- Em-dashes normalized to `--` (MD060 miscounts Unicode width in
  tables); table pipes re-aligned
- Pipes inside code spans in tables escaped (`\|`) so rows parse as
  two cells (MD056)
- Prose reflowed to ~72 columns now that reference labels are short;
  `**Status:**`/`**Fix level:**`/`**Tracking:**` header lines in gap
  docs became bullet lists

Prompt: [verbatim]
Link those issues to TODO and other documentation / code if it make
sense to link somewhere else as well

Followup: [verbatim]
I don't think #10 should be an issue, that is process check, and
possibly best to just validate if the version changed as part of the
release process. I'm closing that issue

Followup: [summarized]
Prefer reference-style links with targets in a bibliography at the end
of the file; fix all similar links in the markdown files.

Followup: [verbatim]
External project issues should be linked like [apple/container#2228]
not [#2228] with a glossary

Followup: [summarized]
Em-dashes replaced with `--` throughout; MD060's column-width counting
mishandles Unicode in tables.

Followup: [summarized]
Reflow block text: early line breaks left over from long inline links
were re-wrapped to ~72 columns; `**Status:**`-style gap-doc headers
became bullet lists.

Co-Authored-By: Devin (SWE-2 High) <158243242+devin-ai-integration[bot]@users.noreply.github.com>
daphnediane added a commit to daphnediane/container-distro that referenced this pull request Oct 6, 2026
Cross-references the open roadmap items with their new tracking issues,
moves the container-internals re-verification into the release
checklist (issue #10 closed as a process check), and migrates all
markdown to the reference-link convention.

- TODO.md: link each open item to its issue (#1-#9, #11-#14); drop
  "Upstream-internal assumptions" (now a release-process step)
- Gap docs + remote-wsl-interop.md: "**Tracking:**" header lines
- security.md: pre-1.0 checklist items 5 and 6 link #12/#13
- install.md: note no brew/nix/pkg packages yet (#14)
- release.md: re-verify appRoot internals + extend
  VERIFIED_CONTAINER_MINOR when container's minor version changes
- container-internals.md: re-verify checklist points at release.md
- ops.rs migrate doc comment links #1; VERIFIED_CONTAINER_MINOR doc
  notes the release-checklist step
- All inline `[text](target)` links converted to `[text]` shortcut
  references with targets collected in a bibliography at each file's
  end; bare text kept when the path is the text
- External issue refs use `[owner/repo#N]` (e.g. [apple/container#1805]);
  bare `[#N]` is reserved for this repo's issues
- Em-dashes normalized to `--` (MD060 miscounts Unicode width in
  tables); table pipes re-aligned
- Pipes inside code spans in tables escaped (`\|`) so rows parse as
  two cells (MD056)
- Prose reflowed to ~72 columns now that reference labels are short;
  `**Status:**`/`**Fix level:**`/`**Tracking:**` header lines in gap
  docs became bullet lists

Prompt: [verbatim]
Link those issues to TODO and other documentation / code if it make
sense to link somewhere else as well

Followup: [verbatim]
I don't think #10 should be an issue, that is process check, and
possibly best to just validate if the version changed as part of the
release process. I'm closing that issue

Followup: [summarized]
Prefer reference-style links with targets in a bibliography at the end
of the file; fix all similar links in the markdown files.

Followup: [verbatim]
External project issues should be linked like [apple/container#2228]
not [#2228] with a glossary

Followup: [summarized]
Em-dashes replaced with `--` throughout; MD060's column-width counting
mishandles Unicode in tables.

Followup: [summarized]
Reflow block text: early line breaks left over from long inline links
were re-wrapped to ~72 columns; `**Status:**`-style gap-doc headers
became bullet lists.

Co-Authored-By: Devin (SWE-2 High) <158243242+devin-ai-integration[bot]@users.noreply.github.com>
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.

[Bug]: container clean fails on read-only named volume mounts.

2 participants