When creating exit command, pass storage options on

[mheon]

We made changes earlier that empty storage options when setting storage driver explicitly. Unfortunately, this breaks rootless cleanup commands, as they lose the fuse-overlayfs mount program path.

Fix this by passing along the storage options to the cleanup process.

Also, fix --syslog, which was broken a while ago (probably when we broke up main to add main_remote).

Fixes #3326

[openshift-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: mheon

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

The pull request process is described here

Details

Needs approval from an approver in each of these files:

• OWNERS [mheon]

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

[mheon]

@edsantiago PTAL. Can't promise this does anything about #3325 but I confirmed it fixes 3326.

[edsantiago]

Good news / bad news. You've fixed the problems when running as nonroot; but those problems (both 3325 and 3326) are now present when run as root. (via sudo, at least)

[edsantiago]

More news: the (root) hang on podman logs -f happens only when mountopt = "nodev,metacopy=on". Without metacopy, both the container and the logs process exit fine. Have a nice day :-(

[mheon]

So... metacopy is breaking cleanup processes, somehow. That's rather confusing.

[mheon]

bin/podman[29233]: time="2019-06-13T17:38:35-04:00" level=error msg="could not get runtime: overlay: Unknown option metacopy"

[mheon]

This is rather confusing because I'm pretty sure metacopy is overlay-specific?

[edsantiago]

I can't reproduce that error message. I've tried using all manner of combinations of driver and mountopt in storage.conf.

Tomorrow is another day, and maybe the bits will behave better then.

[mheon]

@edsantiago podman run -d -t --log-level=debug --syslog fedora sleep 60; podman stop --timeout=0 --latest and check syslog - you'll see that error out of the podman cleanup process

[giuseppe]

LGTM

[giuseppe]

I confirm this fixes #3325

[edsantiago]

@giuseppe please try it as root, with driver = "overlay" and mountopt = "nodev,metacopy=on"

[giuseppe]

nodev,metacopy=on should not be converted to --storage-opt nodev --storage-opt metacopy=on but to --storage-opt $DRIVER.mountopt=nodev,metacopy=on. The issue now is that cobra splits the option in $DRIVER.mountopt=nodev and metacopy=on which is also not correct

[giuseppe]

with the following patch it seems to work for me:

diff --git a/cmd/podman/main_local.go b/cmd/podman/main_local.go
index b4f21bd0..132f35ab 100644
--- a/cmd/podman/main_local.go
+++ b/cmd/podman/main_local.go
@@ -48,7 +48,7 @@ func init() {
        rootCmd.PersistentFlags().StringVar(&MainGlobalOpts.Runtime, "runtime", "", "Path to the OCI-compatible binary used to run containers, default is /usr/bin/runc")
        // -s is depracated due to conflict with -s on subcommands
        rootCmd.PersistentFlags().StringVar(&MainGlobalOpts.StorageDriver, "storage-driver", "", "Select which storage driver is used to manage storage of images and containers (default is overlay)")
-       rootCmd.PersistentFlags().StringSliceVar(&MainGlobalOpts.StorageOpts, "storage-opt", []string{}, "Used to pass an option to the storage driver")
+       rootCmd.PersistentFlags().StringArrayVar(&MainGlobalOpts.StorageOpts, "storage-opt", []string{}, "Used to pass an option to the storage driver")
        rootCmd.PersistentFlags().BoolVar(&MainGlobalOpts.Syslog, "syslog", false, "Output logging information to syslog as well as the console")
 
        rootCmd.PersistentFlags().StringVar(&MainGlobalOpts.TmpDir, "tmpdir", "", "Path to the tmp directory")

[mheon]

Repushed with your patch @giuseppe - thanks

[mheon]

podman-in-podman seems like it might be failing consistently on the runlabel global options test...

Going to try again. If we get another failure I'll fix it.

[mheon]

Alright, I got a theory. Pushed a commit. Let's see...

[mheon]

Alright, that wasn't it...

[mheon]

Alright, I have no idea what's going on with the test failure here. No clues in the logs and I can't reproduce...

Probably going to just skip (for podman in podman only, if possible)

[mheon]

@haircommander @rhatdan @jwhonce Can I get a /lgtm and /hold on this one, so we can merge and start the release?

[jwhonce]

/hold
/lgtm

[mheon]

It's ready. Finally.
/hold cancel