Add the --no-pivot flag to the run command

[afbjorklund]

--no-pivot: "do not use pivot root to jail process inside rootfs.
This should be used whenever the rootfs is on top of a ramdisk"

This is used in minikube, currently with $DOCKER_RAMDISK
(but I'm not sure if this clients wants to honor that env variable...)

[rh-atomic-bot]

Can one of the admins verify this patch?
I understand the following commands:

• bot, add author to whitelist
• bot, test pull request
• bot, test pull request once

[TomSweeneyRedHat]

bot, add author to whitelist

[TomSweeneyRedHat]

If this goes through, we'll need some updates to the man page and a test or two would probably be good. However it looked like @nalind and @giuseppe thought this should be removed and it was in #921. Gents WDYT?

[afbjorklund]

@TomSweeneyRedHat:
It is the same runc run flag (without the "root"), but it is used for a different purpose.
WIthout avoiding pivot_root(2), it is not possible to run containers from a tmpfs root...

Either with docker run:
docker: Error response from daemon: OCI runtime create failed: container_linux.go:348: starting container process caused "process_linux.go:402: container init caused \"rootfs_linux.go:107: jailing process inside rootfs caused \\\"pivot_root invalid argument\\\"\"": unknown.

Or with buildah run:
container_linux.go:296: starting container process caused "process_linux.go:398: container init caused \"rootfs_linux.go:107: jailing process inside rootfs caused \\\"pivot_root invalid argument\\\"\"" error running container: error creating container for [/bin/sh]: : exit status 1

This implementation uses a parameter, which means that it is all pushed on the user. :-(
The docker daemon has an environment line in the systemd service, so it "just works":

https://github.com/kubernetes/minikube/blob/master/pkg/provision/buildroot.go#L79:L80

We have the same thing for crio, but yet no global configuration for podman and buildah.
But apparently there is a (correctly spelled) "no_pivot_root" option in libpod.conf already ?

https://github.com/kubernetes/minikube/blob/master/deploy/iso/minikube-iso/package/crio-bin/crio.conf#L70:L71

---

Currently there is a little bit of struggle left, getting the new images "all the way" to cri-o/k8s.

But that's mostly config issues like "overlay" vs "overlay2", or mounting /var/lib/containers...

The ways of working is not yet set, for now minikube docker-env is replaced with minikube ssh :

https://kubernetes.io/docs/setup/minikube/#reusing-the-docker-daemon (--> running buildah on VM)

[afbjorklund]

What is a DCO ? (Travis failure)

[TomSweeneyRedHat]

@afbjorklund the DCO is due to the commit not being signed, sorry, didn't spot that last night. You'll need to do:
git commit --amend -s and then repush. That should clear that up.

[afbjorklund]

Weird that such a thing should break the build, but whatever. Signed off the commit...

[TomSweeneyRedHat]

Test failure due to a network flake talking to the registry, going to retest.

[TomSweeneyRedHat]

bot, retest this please.

[TomSweeneyRedHat]

@afbjorklund TYVM for the background info btb.

[rhatdan]

@afbjorklund We need modifications to the man pages and command completions. I agree we need @giuseppe and @nalind To approve.

[afbjorklund]

I can do the man pages and bash completions, just wanted some feedback if we should add an environment variable so that it can be configured once and transparently ? And the name of it, if so...

Basically it would be nice if the user could just use buildah and not worry about it (i.e. --no-pivot). A global configuration option would also work, like we already are doing for e.g. /etc/crictl.yaml

[rhatdan]

@afbjorklund We are doing config via environment variables, since we are trying to avoid config files, for now. BUILDAH_NOPIVOT would be my pick.

[afbjorklund]

Thanks. I guess we could set this in /etc/environment or something, to make it globally available.

[afbjorklund]

Make that /etc/profile.d/buildah.sh, I don't think that it reads from /etc/environment

[rhatdan]

@afbjorklund Looks like @nalind Removed --no-pivot-root in f941593

[afbjorklund]

Yeah, I noticed that - but I think it was for a different reason ? (rootless rather than tmpfs)

[giuseppe]

the --no-pivot flag was automatically added for rootless containers, and that wasn't always required. The current PR instead just exposes this feature to the user.

LGTM

[rhatdan]

@rh-atomic-bot r+

[rh-atomic-bot]

📌 Commit 4386d22 has been approved by rhatdan

[rh-atomic-bot]

⌛ Testing commit 4386d22 with merge a84b205...

[rh-atomic-bot]

☀️ Test successful - status-papr
Approved by: rhatdan
Pushing a84b205 to master...

[afbjorklund]

Thank you, seems to be working with it fine in minikube - see linked issue for a complete example.