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

examples/exec: Describe how to emulate 'exec' with 'start' #391

Closed
wants to merge 1 commit into from

Conversation

wking
Copy link
Contributor

@wking wking commented Apr 20, 2016

Spun off from discussion in #388. There seemed to be
consensus that we need something like this but that it should be its
own pull request.

I've left off the PID-namespaces in state JSON bit until that
thread gets sorted out, but the “look things up in /proc” approach
isn't very robust without it.

I'd also be really happy to make all of root optional.
Requring a value so the runtime can ignore it seems like a
potential source of user confusion.

It also feels a bit strange to have to give the exec'ing start a
container ID, even though this join-only config isn't actually setting
up a sub-sandbox. But short of externalizing the state registry,
I don't see an easy way around that.

@wking
Copy link
Contributor Author

wking commented Apr 20, 2016

Also, this will need links from somewhere more central (runtime.md?), maybe an entry in the Makefile, etc., etc. I've left those off until we figure out whether this approach is a direction we want to take.

@wking wking force-pushed the exec-example branch 2 times, most recently from dbaf8f9 to 3617d28 Compare April 20, 2016 06:00
Spun off from discussion here [1].  There seemed to be consensus that
we need something like this but that it should be its own pull request
[2,3,4].

[1]: opencontainers#388 (comment)
[2]: opencontainers#388 (comment)
[3]: opencontainers#388 (comment)
[4]: opencontainers#388 (comment)

Signed-off-by: W. Trevor King <[email protected]>
@crosbymichael
Copy link
Member

-1

I don't think we need to describe this, people can implement exec however we want and we don't want to talk about exec at all in the spec.

@duglin
Copy link
Contributor

duglin commented May 2, 2016

@crosbymichael perhaps the spec, as a formal RFC2119 type of doc, isn't the right spot, but would you be ok with a side-doc, more like a whitepaper? Just something to provide some guidance/hints/...

@crosbymichael
Copy link
Member

@duglin ya, for one the spec is not the right place, somewhere else would be better for sure but the whole reason why exec is out is because people want to implement it in 1000 different ways to fit their need so we don't really need a suggestion at all but if we do have one, it should be outside of the spec.

@wking
Copy link
Contributor Author

wking commented May 2, 2016

On Mon, May 02, 2016 at 11:08:30AM -0700, Michael Crosby wrote:

-1

I don't think we need to describe this, people can implement exec
however we want and we don't want to talk about exec at all in the
spec.

I agree on “however they want”, but I think describing at least one
supported approach is important. For example, the approach I've
outlined here calls for more clarity around root and required
container IDs in the absence of sub-sandboxing 1. It's also kind to
provide a migration path after the operation removal in #388.

So I'm fine having these docs land in a separate, non-spec location,
but I think we want OCI-maintainer-blessed notes for this emulation
somewhere under opencontainers/ (maybe in the wiki?). Since we can't
make wiki PRs, it seems better to collect OCI-maintainer blessings
here and then either land the PR or move it to the wiki.

@crosbymichael
Copy link
Member

I agree on “however they want”, but I think describing at least one supported approach is important. Its not supported, if it was supported it would be in the spec ;)

@wking
Copy link
Contributor Author

wking commented May 2, 2016

On Mon, May 02, 2016 at 11:22:19AM -0700, Michael Crosby wrote:
“Its not supported, if it was supported it would be in the spec ;)”

@vishh in #388 1:

Exec can be implemented as running additional containers.

and then later 2:

We need blessed cookbook for the Spec.

And then @mrunalp 3:

I think we can handle examples/cookbook etc. after this PR.

so I think there is at least some consensus around:

a. exec-like functionality being supported via a ‘start’ invocation, and
b. wanting some sort of OCI-blessed cookbook for achieving that
functionality.

I agree that the cookbook would be informative, and not normative, but
that doesn't mean we don't need it somewhere ;).

@vishh
Copy link
Contributor

vishh commented May 2, 2016

I agree with Michael Crosby! Having one single example for exec in the Spec is not
ideal. If we had a Spec-Cookbook repo, we can post different ways to run
sub-containers/exec in there.

@wking
Copy link
Contributor Author

wking commented May 2, 2016

On Mon, May 02, 2016 at 11:56:42AM -0700, Vish Kannan wrote:

I agree with Michael Crosby! Having one single example for Spec is not
ideal. If we had a Spec-Cookbook repo, we can post different ways to run
sub-containers/exec in there.

A separate repo sounds fine to me, although you probably want
“runtime” in the name somewhere to distinguish from image-spec. I'd
recommend runtime-spec-examples, but don't really care ;).

Although that seems like an orthogonal concern to number of examples.
The subdirectory structure I'm setting up in this PR (currently
examples/exec.md) can easily extend to multiple approaches
(examples/exec/proc.md, examples/exec/…, …). And unless you do that
namespacing at the repo level (runtime-spec-exec-examples?), we'll
have that directory structure wherever the examples land.

wking added a commit to wking/ocitools-v2 that referenced this pull request May 3, 2016
Add tooling to translate higher-level configs into the basic OCI
config.  On IRC, Julz floated a linux.namespaces[].fromContainer as a
higher-level version of linux.namespaces[].path for emulating exec
[1].  That makes sense to me, and Mrunal is open to something like
[2]:

  $ ocitools generate --template=high-level-config.json --translate=fromContainer --runtime=runc

This commit still needs:

* The state JSON lookup and path logic from [3].
* A way to convert the interface{} to an rspec.Spec (the current FIXME
  raises: FATA[0000] translated template has an invalid schema).

[1]: http://ircbot.wl.linuxfoundation.org/eavesdrop/%23opencontainers/%23opencontainers.2016-04-27.log.html#t2016-04-27T20:32:09
[2]: http://ircbot.wl.linuxfoundation.org/eavesdrop/%23opencontainers/%23opencontainers.2016-04-28.log.html#t2016-04-28T16:12:48
[3]: opencontainers/runtime-spec#391

Signed-off-by: W. Trevor King <[email protected]>
wking added a commit to wking/ocitools-v2 that referenced this pull request May 3, 2016
Add tooling to translate higher-level configs into the basic OCI
config.  On IRC, Julz floated a linux.namespaces[].fromContainer as a
higher-level version of linux.namespaces[].path for emulating exec
[1].  That makes sense to me, and Mrunal is open to something like
[2]:

  $ ocitools generate --template=high-level-config.json --translate=fromContainer --runtime=runc

The interface{} -> rspec.Spec conversion happens through a JSON
serialization trick suggested by 梁辰晔 (Liang Chenye) [3] after the
translations, because before the translations the template may contain
JSON that's not represented in the OCI Spec structure
(e.g. fromContainer in namespace entries).

This commit still needs:

* The state JSON lookup and path logic from [4].
* Adjustments to setupNamespaces to avoid clobbering the template.

[1]: http://ircbot.wl.linuxfoundation.org/eavesdrop/%23opencontainers/%23opencontainers.2016-04-27.log.html#t2016-04-27T20:32:09
[2]: http://ircbot.wl.linuxfoundation.org/eavesdrop/%23opencontainers/%23opencontainers.2016-04-28.log.html#t2016-04-28T16:12:48
[3]: opencontainers#54 (comment)
[4]: opencontainers/runtime-spec#391

Signed-off-by: W. Trevor King <[email protected]>
wking added a commit to wking/ocitools-v2 that referenced this pull request May 3, 2016
Add tooling to translate higher-level configs into the basic OCI
config.  On IRC, Julz floated a linux.namespaces[].fromContainer as a
higher-level version of linux.namespaces[].path for emulating exec
[1].  That makes sense to me, and Mrunal is open to something like
[2]:

  $ ocitools generate --template=high-level-config.json --translate=fromContainer --runtime=runc

The interface{} -> rspec.Spec conversion happens through a JSON
serialization trick suggested by 梁辰晔 (Liang Chenye) [3] after the
translations, because before the translations the template may contain
JSON that's not represented in the OCI Spec structure
(e.g. fromContainer in namespace entries).

This commit still needs:

* The state JSON lookup and path logic from [4].
* Adjustments to setupNamespaces to avoid clobbering the template.

[1]: http://ircbot.wl.linuxfoundation.org/eavesdrop/%23opencontainers/%23opencontainers.2016-04-27.log.html#t2016-04-27T20:32:09
[2]: http://ircbot.wl.linuxfoundation.org/eavesdrop/%23opencontainers/%23opencontainers.2016-04-28.log.html#t2016-04-28T16:12:48
[3]: opencontainers#54 (comment)
[4]: opencontainers/runtime-spec#391

Signed-off-by: W. Trevor King <[email protected]>
@crosbymichael
Copy link
Member

Like @vishh and I said, we do not that things like this should belong in the spec, nor the spec repo. We can wait until a post 1.0 before looking at different use cases and making a contrib or "cookbook" type repo if that fits or there are many other places for this type of content outside of OCI.

@wking
Copy link
Contributor Author

wking commented Jun 2, 2016

In today's meeting the earlier reasons for closing this were
overturned and I volunteered to reroll it with additional, testable
examples 1. After playing around with runC's current master and
opencontainers/runc#827, I think I'm going to wait until either:

before supporting runC in my examples. If I get a new ccon release
out the door before those happen, I may post examples that work with
ccon's OCI-emulation layer.

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

Successfully merging this pull request may close these issues.

4 participants