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

[RFC] runc cli: --rootless flag idiosyncrasies #2645

Open
kolyshkin opened this issue Oct 12, 2020 · 2 comments
Open

[RFC] runc cli: --rootless flag idiosyncrasies #2645

kolyshkin opened this issue Oct 12, 2020 · 2 comments

Comments

@kolyshkin
Copy link
Contributor

While looking into #2639 I found out we have two --rootless flags with different meaning:

  1. Global flag, documented in runc help as
   --rootless value    ignore cgroup permission errors ('true', 'false', or 'auto') (default: "auto")
  1. A flag specific to spec, documented in runc spec --help as
   --rootless                generate a configuration for a rootless container

The problem here is non-uniform syntax:

  1. The first (global) option applies (I guess) to create, run, exec and update), the second applies to spec.

  2. The second option is true boolean, meaning that specifying --rootless is equivalent to --rootless=true. This is what I would expect. The first option is "bool-or-auto" meaning that specifying --rootless is equivalent to no option, and to actually enable rootless mode one has to use --rootless=true.

Both items are confusing, second is more.

Proposal

I think we can do the following to rectify this:

  1. Deprecate the local --rootless flag for runc spec, adding a warning that the global one should be used.

  2. Make the global --rootless equivalent to --rootless=yes, and warn that using --rootless=auto is deprecated (as it is "auto" without specifying any option).

  3. Eventually remove the local --rootless and the global --rootless=auto.

@AkihiroSuda @cyphar @mrunalp what do you think?

@AkihiroSuda
Copy link
Member

I understand the motivation, but changing runc run interface is too risky.
So 1 SGTM but 2 and 3 NOTSGTM, sorry

@cyphar
Copy link
Member

cyphar commented Oct 13, 2020

The two --rootless flags were added at different times, hence the inconsistency:

  • runc spec --rootless was originally added along with rootless support in Rootless Containers #774, so that you could easily configure a rootless container (because quite a few of the default configuration bits were not compatible with rootless mode). However, since then we have umoci unpack --rootless which is arguably better for a variety of reasons and generally speaking I would argue that folks shouldn't really use runc spec outside of just playing around with runc.

  • runc --rootless was added some time later by @AkihiroSuda to allow you to forcefully enable rootless mode for nested containers as well as a few other usecases. I prefer --rootless=auto to be possible to set explicitly, and I agree with @AkihiroSuda -- changes to runc run make me quite nervous.

I don't really mind if we remove runc spec --rootless, though it will mean runc spec's behaviour will change quite drastically (most people don't run it as root so it will always generate a rootless configuration if it obeys the default --rootless=auto) but I don't really see a strong benefit to doing so. Yes there are two rootless flags but they're used in quite different contexts (and runc spec I would argue shouldn't really be used by people outside of those playing around with runc).

(As an aside, looking back I think runc --rootless should actually have had the arguments never, auto, always to match most other tools -- but I didn't think of that at the time, oh well.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants