"podman volume rm" behaviour

[rhatdan]

Discussed in #23922

^{Originally posted by tokudan September 10, 2024}
I've been very surprised by the unpredictable behaviour of the podman volume rm command.
It boils down to this:

$ podman volume create x
x
$ podman volume create xa
xa
$ podman volume rm x
x
$ podman volume rm x
xa

Podman deletes the volume "xa". Obviously this is a constructed example, but in practice, just running "podman volume rm x" again if you're were interrupted and not sure if you already did it, probably happens.

The function of the "rm" command is so obvious that I don't actually look up the manpage if I don't want some special behaviour. The manpage actually documents that behaviour:

Volumes can be removed individually by providing their full name or a unique partial name.

I think guessing what a user wants to delete is pretty dangerous and makes it unpredictable.
If I copy a volume to backup its contents, I cannot give it a similar name as "forgejo-backup" could be removed, because I run "podman rm forgejo". Am I sure no automatic process has removed the production volume while I was busy? Are there any automatic cleanup jobs running, that might cause me to remove the backup instead of the volume I specify?

/bin/rm removes exactly what you tell it to remove. Which is obviously were "podman volume rm" got its name.
I believe "podman volume rm" should behave the same, at least by default.

The guesswork that "podman volume rm" is doing probably saved someone some keystrokes. I'd argue that's the point of tab completion, but I think the guesswork should be opt-in.

[rhatdan]

I agree this is an issue and we should not be removing volumes this way.

@mheon PTAL

[mheon]

I would have to double-check, but I believe this is a duplicate of Docker behavior, and as such I don't think we can change it. And even if we could, I'm not sure we should.

If you were two have two volumes, ab and abc, and podman volume rm a - you'd get an error about an ambiguous name. But this scenario, a is a fully qualified name, we will remove it happily - and I think changing that is also very unexpected behavior. Any sort of Bash script that operates on Podman, for example, will expect that podman volume rm $name will always work, so long as $name is the full name of the volume, even if there's a second volume with $name as a prefix to its own name. Breaking the assumption that removal by full name always works seems dangerous.

[rhatdan]

The issue is if you only have one volume named abc, and do podman volume rm a it is removing abc, as I understand it, which seems very dangerous.

[mheon]

Only once a is gone. Once a is gone, we will remove abc as long as there are no other volumes whose names start with a; otherwise we error. Unambiguous partial names are allowed.

[rhatdan]

Yup, but I find that surprising and likely to cause volumes to be removed by accident.

[rhatdan]

And I see little upside in this other then perhaps Docker does it this way.

As the reporter states it easy enough to do podman volume rm a<tab>

[eriksjolund]

Docker has different behaviour

$ docker volume ls
DRIVER    VOLUME NAME
$ docker volume create x
Error response from daemon: create x: volume name is too short, names should be at least two alphanumeric characters
$ docker volume create xx
xx
$ docker volume create xxa
xxa
$ docker volume rm xx
xx
$ docker volume rm xx
Error response from daemon: get xx: no such volume
$ docker volume rm xxa
xxa
$

Tested with docker 27.1.2

More info about the system:

Click me

$ docker version
Client:
 Version:           27.1.2
 API version:       1.46
 Go version:        go1.23.0
 Git commit:        1.fc42
 Built:             Wed Aug 21 00:00:00 2024
 OS/Arch:           linux/arm64
 Context:           default

Server:
 Engine:
  Version:          27.1.2
  API version:      1.46 (minimum version 1.24)
  Go version:       go1.23.0
  Git commit:       1.fc42
  Built:            Wed Aug 21 00:00:00 2024
  OS/Arch:          linux/arm64
  Experimental:     false
 containerd:
  Version:          1.7.21
  GitCommit:        1.fc42
 runc:
  Version:          1.1.12
  GitCommit:        
 tini-static:
  Version:          0.19.0
  GitCommit:        
$ rpm -qf /usr/bin/docker
docker-cli-27.1.2-1.fc42.aarch64
$ rpm -q moby-engine
moby-engine-27.1.2-1.fc42.aarch64
$ rpm-ostree status
State: idle
warning: Failed to query journal: couldn't find current boot in journal
Deployments:
● ostree-remote-image:fedora:docker://quay.io/fedora/fedora-coreos:rawhide
                   Digest: sha256:b49001cc46b33e163c44eef55f26a722acf21d1bc0dceac3dac99bcc9a97f34e
                  Version: 42.20240830.91.0 (2024-08-30T14:00:28Z)

[baude]

this seems valid then ... lets tidy it up ?

[mheon]

Could still be considered a breaking change since we're altering behvaior on volume rm rather significantly. @Luap99 Thoughts?

[Luap99]

Well other commands behave sort of similar with partial ids. The difference in volume is that there is only one Name field and not Name and ID as separate things so the logic here got conflated into the same things which is not nice.

Of course if there is an exact match we must remove that is clear but if docker doesn't remove partial name matches then I think we should not either. Although I would compare this behavior against an actual ID (anonymous volume).

As far as breaking people, I am not sure that happens much in practice. Scripts should use the full name anyway and would need to get out of their way to rely on the partial lookup and for interactive use shell completion should be good enough.

[zackattackz]

Apologies if this isn't the place to ask, but would this be a good beginner issue? I'm interested to look into it if it isn't already being worked on by someone else.

[Luap99]

@zackattackz You can try for sure, I don't think it is to easy as you have to find the right places where to logic applies and the compare this against the docker behavior but it should not be super difficult either I guess.

[zackattackz]

@Luap99
Yeah it probably would be better if someone who's already familiar with the code worked on it. But I figured it could be a good opportunity to get some familiarity with the code base. If it's not urgent I'd like to try :)

[zackattackz]

PR made at #24027