Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

podman --storage-opt can make permanent destructive changes #15698

Closed
edsantiago opened this issue Sep 8, 2022 · 9 comments · Fixed by containers/storage#1329
Closed

podman --storage-opt can make permanent destructive changes #15698

edsantiago opened this issue Sep 8, 2022 · 9 comments · Fixed by containers/storage#1329
Labels
locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments.

Comments

@edsantiago
Copy link
Member

Semi-related to #15488, but not exactly, because that one is still an unexplained flake; this one is 100% reproducible.

Run this as root on a fresh VM (NOT ON YOUR LAPTOP, I'M WARNING YOU!)

# podman run quay.io/libpod/alpine true
...pull, etc... (because this is a fresh VM)
# podman --storage-opt=mount_program=/bin/false info >/dev/null
(errors out, expected)
# podman run quay.io/libpod/alpine true
WARN[0000] Ignoring global metacopy option, not supported with booted kernel

Your system is now destroyed. This does not fix it:

# podman --storage-opt= info >/dev/null
# podman run quay.io/libpod/alpine true
WARN[0000] Ignoring global metacopy option, not supported with booted kernel

It looks like podman system reset -f fixes things, but I'm not sure yet. Anyhow, this seems like kind of a bug? To have a temporary --storage-opt make permanent changes somewhere, that cannot be overridden?

@rhatdan
Copy link
Member

rhatdan commented Sep 8, 2022

@giuseppe @nalind PTAL

I got this to happen on my machine as well. It looks like this does something to get the kernel to report it does not work with metacopy=on

DEBU[0000] overlay: storage already configured with a mount-program
DEBU[0000] backingFs=xfs, projectQuotaSupported=false, useNativeDiff=false, usingMetacopy=false

@rhatdan
Copy link
Member

rhatdan commented Sep 8, 2022

The problem is podman is dropping a file .has-mount-program in place which causes it to fail.

# rm /var/lib/containers/storage/overlay/.has-mount-program
# podman run alpine echo hi
hi

@nalind
Copy link
Member

nalind commented Sep 8, 2022

fuse-overlayfs doesn't support metacopy.

Your system is now destroyed. This does not fix it:

# podman --storage-opt= info >/dev/null
# podman run quay.io/libpod/alpine true
WARN[0000] Ignoring global metacopy option, not supported with booted kernel

It looks like podman system reset -f fixes things, but I'm not sure yet. Anyhow, this seems like kind of a bug? To have a temporary --storage-opt make permanent changes somewhere, that cannot be overridden?

Try --storage-opt=mount_program=? Though given #13458 and #13459, if the tests are generally assuming that the same storage area can be used both with and without fuse-overlayfs, then they're mistaken.

@edsantiago
Copy link
Member Author

Sorry @nalind, I don't really understand that. (Because I don't really know what --storage-opt is used for). My concern, as a very ignorant user, is that a command-line option should act only in the scope of that one command invocation (unless it's a --set option, I suppose). An option like --storage-opt, much like --runroot or --cgroup-manager, should not really render my system unusable.

@nalind
Copy link
Member

nalind commented Sep 8, 2022

The reproducer didn't break my system. It started triggering the warning, but the container still ran.

@edsantiago
Copy link
Member Author

Fair enough: I may have a stricter definition of "broken". How about: a command-line option should not cause all subsequent podman run commands to issue a warning that doesn't go away, and affects all tests, and alarms a user because they might think there is something wrong with their installation.

@rhatdan
Copy link
Member

rhatdan commented Sep 8, 2022

The thing is the overlay is still using native overlay but without metacopy support. As far as I see.

@giuseppe
Copy link
Member

giuseppe commented Sep 9, 2022

once the mount program is used, even once, it must be used for all the subsequent commands (unless explicitly overridden) because the images are written in a format understood by fuse-overlayfs.

In other words, setting the mount program is a non-reversible operation, and the only way to clean it up is to start with a fresh storage.

We could change the verbosity of the missing metacopy flag when a mount program is used, but I think the behavior should not be changed.

giuseppe added a commit to giuseppe/storage that referenced this issue Sep 9, 2022
if a mount program is specified, change the content and the verbosity
for the message printed.

Closes: containers/podman#15698

Signed-off-by: Giuseppe Scrivano <[email protected]>
@rhatdan
Copy link
Member

rhatdan commented Sep 9, 2022

But in this case we never used the fuse-overlay.

There is currently a check to the storage looking for fuse-overlay specific files. Isn't this enough?

Should we only record that you used a mount program only if it was successfully used?

edsantiago added a commit to edsantiago/libpod that referenced this issue Sep 28, 2022
I was testing --log-level by --storage-opt=mount_program=/bin/false

Stop doing that. It's just constantly breaking everything (containers#15698
and containers#15977).

I am violently of the opinion that a command-line option must
not destroy a user's system (except for --set-something, --config,
something that makes it very very clear that it is a lasting
change). I seem to be in the minority on this opinion. So, I
give up.

Signed-off-by: Ed Santiago <[email protected]>
@github-actions github-actions bot added the locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments. label Sep 16, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 16, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments.
Projects
None yet
4 participants