WIP: volume protection by pinning #26846

tobwen · 2025-08-16T16:14:06Z

Does this PR introduce a user-facing change?

Yes.

* Added `--pinned` flag to `volume create` to create pinned volumes
* Added new `volume pin` commands to pin/unpin existing volumes
* Added `--include-pinned` flag to `volume rm` to allow removing pinned volumes
* Added pinned volume filtering support
* Added support for pinned volumes in `system prune` command
* Added HTTP API and ABI support for volume pinning
* Added runtime methods for handling pinned volumes and pruning

References

Reference #26807
Reference #23217

Actions required

This is just the first step to demonstrate the function. Things like tunnel/remote implementation are still missing (it's just a stub).
It needs to be discussed if we want to overload the CLI with volume unpin or rather use volume pin --unpin as supplied.

Note

Those are my first steps with the Podman code - please bear with me.

openshift-ci · 2025-08-16T16:14:13Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: tobwen
Once this PR has been reviewed and has the lgtm label, please assign ygalblum for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

mheon · 2025-08-18T18:01:32Z

Should pinned volumes require --force to remove? It feels like they ought to

mheon · 2025-08-18T18:02:34Z

libpod/volume.go

 	MountLabel string `json:"mountlabel,omitempty"`
+	// Pinned indicates that this volume should be excluded from
+	// system prune operations by default
+	Pinned bool `json:"pinned,omitempty"`


Probably better to stick this in state, since it can be toggled on/off

mheon · 2025-08-18T18:03:20Z

libpod/volume.go

+		return err
+	}
+
+	v.config.Pinned = pinned


Yeah, this won't work with v.config - that's static data, never changes. You want v.state - which is refreshed by update() and saved by save()

tobwen · 2025-08-18T20:27:11Z

Should pinned volumes require --force to remove? It feels like they ought to

I've thought about that before, but I figured we should treat it like the immutable flag. If this flag is set, even root can't remove a file with rm -f either. My concern here is protecting important data, and maybe root has some stupid script that sets --force to keep the script running - but then the data is gone (excluding backups for now).

packit-as-a-service · 2025-08-18T23:03:13Z

[NON-BLOCKING] Packit jobs failed. @containers/packit-build please check. Everyone else, feel free to ignore.

packit-as-a-service · 2025-08-18T23:03:13Z

[NON-BLOCKING] Packit jobs failed. @containers/packit-build please check. Everyone else, feel free to ignore.

packit-as-a-service · 2025-08-18T23:03:13Z

[NON-BLOCKING] Packit jobs failed. @containers/packit-build please check. Everyone else, feel free to ignore.

packit-as-a-service · 2025-08-18T23:03:13Z

[NON-BLOCKING] Packit jobs failed. @containers/packit-build please check. Everyone else, feel free to ignore.

github-actions · 2025-10-02T00:09:09Z

A friendly reminder that this PR had no activity for 30 days.

tobwen · 2025-10-02T09:43:07Z

So sad... all the work for nothing.

mheon · 2025-10-02T21:13:54Z

We generally don't review things until CI starts passing. In this case, looks like it's yelling about missing a manpage for the new commands? I can do a more thorough review early next week.

mheon · 2025-10-02T21:15:05Z

cmd/podman/volumes/pin.go

+var (
+	pinDescription = `Mark or unmark a volume as pinned.
+
+Pinned volumes are excluded from system prune operations by default.`


System reset still catches them, yes?

That's a thing we should discuss... My main intention for pinning is to help inexperienced DevOps who are close to reaching capacity limits on their live systems, then execute prune or reset for some reason, and then realize that the volume containing their live database is gone and the backup is two days old.

After checking the implementation I found that Reset() was NOT respecting the pinned flag. Thanks, I fixed this.

I've added an optional solution for reset: #26846 (comment)

mheon · 2025-10-02T21:16:53Z

pkg/domain/entities/types/volumes.go

 	GID *int `schema:"gid"`
+	// Pinned indicates that this volume should be excluded from
+	// system prune operations by default
+	Pinned bool `schema:"pinned"`


Is there a good reason for this? It feels more natural to podman volume create + podman volume pin but I suppose that could be racey

While podman volume create + podman volume pin would work, there's a window where the volume could be pruned between those two commands. I added a comment explaining this is to avoid race conditions when atomically creating and pinning a volume.

cmd/podman/system/prune.go

libpod/options.go

libpod/runtime_volume.go

libpod/volume.go

Signed-off-by: tobwen <[email protected]>

see discussion in containers#23217 Signed-off-by: tobwen <[email protected]>

Signed-off-by: tobwen <[email protected]>

Adds a new flag `--include-pinned` to podman system reset that allows users to optionally include pinned volumes in the reset operation. Previously, pinned volumes were always excluded from being reset. This change: - Updates warning messages to correctly reflect volume behavior - Modifies the reset functionality to respect the new inclusion flag - Propagates the new option through the system architecture - Clarifies in the prune warning message that pinned volumes are optionally included Signed-off-by: tobwen <[email protected]>

Signed-off-by: tobwen <[email protected]>

* Renames the global variable 'includePinned' to 'resetIncludePinned' to improve code clarity and prevent potential variable shadowing. * Adds SystemResetOptions type alias to the entities package. Signed-off-by: tobwen <[email protected]>

Updates Podman package import references from version 5 to version 6. Signed-off-by: tobwen <[email protected]>

Signed-off-by: tobwen <[email protected]>

openshift-ci bot added release-note do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. labels Aug 16, 2025

github-actions bot added the kind/api-change Change to remote API; merits scrutiny label Aug 16, 2025

tobwen mentioned this pull request Aug 16, 2025

Feature Request - Pin Podman Volumes to be Ignored when Pruning #23217

Open

tobwen force-pushed the podman-26807 branch from 85227e6 to 02876a9 Compare August 16, 2025 17:08

mheon reviewed Aug 18, 2025

View reviewed changes

tobwen force-pushed the podman-26807 branch from 6ba6324 to 3380a8b Compare August 18, 2025 22:57

tobwen force-pushed the podman-26807 branch from 5ca022a to 8c6bb95 Compare August 18, 2025 23:13

tobwen force-pushed the podman-26807 branch from 8c6bb95 to 48e390b Compare September 1, 2025 02:19

github-actions bot added the stale-pr label Oct 2, 2025