Add a new container runtime interface #25899

yujuhong · 2016-05-19T19:33:23Z

This PR includes a proposal and a Go file to re-define the container runtime interface.
This is based on the original doc: https://docs.google.com/document/d/1ietD5eavK0aTuMQTw6-21r67UU73_vqYSUIPFdA0J5Q/

The umbrella issues is #22964

/cc @kubernetes/sig-node

vishh · 2016-05-19T20:08:29Z

cc @mrunalp

dchen1107 · 2016-05-19T20:26:02Z

cc/ @philips @jonboulle @yifan-gu @euank from coreos / rkt
cc/ @smarterclayton @derekwaynecarr @timothysc @mrunalp from redhat and runc integration
cc/ @feiskyer @resouer from hyper.sh and hyperd integration
cc/ @krobertson from Apcera

yifan-gu · 2016-05-19T20:38:36Z

cc @tmrts @s-urbaniak

tmrts · 2016-05-19T20:38:50Z

docs/proposals/container-runtime-interface-v1.md

+	Pull(image ImageSpec, auth AuthConfig) error
+	Remove(image ImageSpec) error
+	Status(image ImageSpec) (Image, error)
+    Metrics(image ImageSpec) (ImageMetrics, error)


nit: spacing

will fix it. thanks

vishh · 2016-05-19T21:24:02Z

@yujuhong Would it be useful to mention explicitly that this PR is not intended to be the final API / interface and that the individual fields/features can be fixed/updated/improved in subsequent PRs?

mrunalp · 2016-05-19T21:45:35Z

pkg/kubelet/container/interface.go

+}
+
+// Namespaces contains paths to the namespaces.
+type Namespaces struct {


Should probably include paths to all shared namespaces for completeness.

We can approach this in two different ways. One is, as you suggested, returning everything for completeness. The other is to add fields when we need it. I went with the second approach, and we can iterate and add more fields if required.

yujuhong · 2016-07-01T19:11:54Z

I recognize that there are still some concerns, but we're not going to get anything done here. We should take additional discussion either into new issues or into other communication forms. I'm sending out an email to sig-node today to try and kick off that discussion.

+1. There is no way we can have effective discussions on this PR anymore. Let's open up separate issues/PRs for a more focused discussion.

dchen1107 · 2016-07-01T19:13:12Z

LGTM

We carried a lot of good discussion through the PR and identified several issues. But at the high level, I didn't see much progress through this pr any more.

k8s-bot · 2016-07-01T19:37:51Z

GCE e2e build/test passed for commit 96c0103455f27a974085e9a2fc4148f669ac4dec.

tmrts · 2016-07-01T20:00:13Z

It seems like I wasn't able to communicate my intentions well. I meant addressing the comments of other people so that theirs don’t get lost and carry the discussion about my concerns to another PR. I agree with 100% with the recent comments 👍

This commit includes a proposal and a Go file to re-define the container runtime interface. Note that this is an experimental interface and is expected to go through multiple revisions once developers start implementing against it. As stated in the proposal, there are also individual issues to carry discussions of specific features.

yujuhong · 2016-07-01T22:43:06Z

Updated the boillerplate to the new style. Reapplying the lgtm.

k8s-bot · 2016-07-01T23:25:18Z

GCE e2e build/test passed for commit 08dc661.

k8s-github-robot · 2016-07-01T23:55:44Z

Automatic merge from submit-queue

@brendandburns

Automatic merge from submit-queue Proposal: client/server container runtime Ref #25899 #13768 Proposal for client/server container runtime CC @brendandburns @dchen1107 @kubernetes/goog-node @kubernetes/sig-node

runcom · 2016-07-15T15:53:40Z

pkg/kubelet/container/interface.go

+
+	// Linux contains configurations specific to Linux hosts.
+	Linux *LinuxPodSandboxConfig
+}


is there any particular issue in having an User property in PodSandboxConfig above? OCI would want to specify an user in the config.json as part of running a container with a specified user. Having the CRI proto to support that would let OCI containers to run with a particular user. @mrunalp @yujuhong @vishh

Since the securityContext of a container can have a runAsUser, we need something in this interface to be able to handle that. However, I think it should be solely part of a container's config, not pod sandbox.

Want to file a followup issue or make a PR to add this and discuss there instead?

I agree that adding this at the container level makes sense. PodSandbox could be left upto the runtime.

👍 I'll make a PR to discuss that then

BTW, @euank sorry for the confusion, I did mean ContainerConfig not PodSandboxConfig

+1 for adding user to container config.

MikeSpreitzer · 2016-07-19T18:35:25Z

pkg/kubelet/container/interface.go

+	// Mounts specifies mounts for the container
+	Mounts []Mount
+	// Labels are key value pairs that may be used to scope and select individual resources.
+	Labels Labels


From where do these labels come?
What if the container runtime needs to know the origin of this container in k8s API terms, such as k8s namespace, k8s pod name, and user container name in the pod --- how could the runtime know those?

This interface abstracts those details away. Those labels realistically do contain such information, but it's an internal contract for the kubelet to use for itself only, not something the runtime is supposed to interpret in any way.

Right now those come mostly from here iirc and are used for things like calculating restart count and checkpointing data.

The interface explicitly does not express more to the runtime than it needs to know.

That being said, a sufficiently smart runtime could lookup a container it manages in the kubelet's pods endpoint based on its container ID I expect.
Realistically though, k8s namespaces and pod names and so on are all higher level details that the runtime should not have to care about.

@mrunalp

Automatic merge from submit-queue CRI: add LinuxUser to LinuxContainerConfig Following discussion in #25899 (comment) The Container Runtime Interface should provide runtimes with User information to run the container process as (OCI being one of them). This patch introduces a new field `user` into `LinuxContainerConfig` structure. The `user` field introduces also a new type structure `LinuxUser` which consists of `uid`, `gid` and `additional_gids`. The `LinuxUser` struct has been embedded into `LinuxContainerConfig` to leave space for future implementations which are not Linux-related (e.g. Windows may have a different representation of _Users_). If you feel naming can be better we can probably move `LinuxUser` to `UnixUser` also. /cc @mrunalp @vishh @euank @yujuhong Signed-off-by: Antonio Murdaca <runcom@redhat.com>

@brendandburns

…er-proposal Automatic merge from submit-queue Proposal: client/server container runtime Ref kubernetes#25899 kubernetes#13768 Proposal for client/server container runtime CC @brendandburns @dchen1107 @kubernetes/goog-node @kubernetes/sig-node

yujuhong added the sig/node Categorizes an issue or PR as relevant to SIG Node. label May 19, 2016

googlebot added the cla: yes label May 19, 2016

yujuhong assigned dchen1107 May 19, 2016

k8s-github-robot added kind/design Categorizes issue or PR as related to design. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. release-note-label-needed labels May 19, 2016

yujuhong mentioned this pull request May 19, 2016

Revisit Container Runtime interface #22964

Closed

yujuhong force-pushed the ncri branch from 8248fa7 to 7674a35 Compare May 19, 2016 20:09

tmrts reviewed May 19, 2016
View reviewed changes

mrunalp reviewed May 19, 2016
View reviewed changes

yifan-gu added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jul 1, 2016

yujuhong force-pushed the ncri branch from 96c0103 to 08dc661 Compare July 1, 2016 22:42

yujuhong added lgtm "Looks good to me", indicates that a PR is ready to be merged. and removed lgtm "Looks good to me", indicates that a PR is ready to be merged. labels Jul 1, 2016

yujuhong added the retest-not-required label Jul 1, 2016

k8s-github-robot merged commit fb19362 into kubernetes:master Jul 1, 2016

tmrts mentioned this pull request Jul 4, 2016

stage1: add pod mutability as a service rkt/rkt#2867

Closed

erictune mentioned this pull request Jul 11, 2016

Add sysctl proposal #26057

Closed

12 tasks

yujuhong mentioned this pull request Jul 11, 2016

Container Runtime Interface/API Planning #28789

Closed

tmrts mentioned this pull request Jul 14, 2016

*: expose application-level operations rkt/rkt#2932

Merged

runcom reviewed Jul 15, 2016
View reviewed changes

This was referenced Jul 15, 2016

CRI: add LinuxUser to LinuxContainerConfig #29026

Merged

[RFC] remove .docker/config.json parsing containers/image#41

Closed

POC runc as container runtime #24036

Closed

mrunalp mentioned this pull request Jul 19, 2016

Import ocitools/generate library mrunalp/ocid#7

Merged

MikeSpreitzer reviewed Jul 19, 2016
View reviewed changes

aronchick mentioned this pull request Jul 22, 2016

Redefine the Container Runtime Interface kubernetes/enhancements#54

Closed

22 tasks

tmrts mentioned this pull request Jul 27, 2016

Container Runtime Interface (CRI) Integration rkt/rkt#2992

Open

15 tasks

yujuhong deleted the ncri branch September 21, 2016 00:15

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add a new container runtime interface #25899

Add a new container runtime interface #25899

yujuhong commented May 19, 2016

vishh commented May 19, 2016

dchen1107 commented May 19, 2016

yifan-gu commented May 19, 2016

tmrts May 19, 2016

yujuhong May 24, 2016

vishh commented May 19, 2016

mrunalp May 19, 2016

yujuhong May 27, 2016

yujuhong commented Jul 1, 2016 •

edited

Loading

dchen1107 commented Jul 1, 2016

k8s-bot commented Jul 1, 2016

tmrts commented Jul 1, 2016

yujuhong commented Jul 1, 2016

k8s-bot commented Jul 1, 2016

k8s-github-robot commented Jul 1, 2016

runcom Jul 15, 2016 •

edited

Loading

euank Jul 15, 2016

mrunalp Jul 15, 2016

runcom Jul 15, 2016

runcom Jul 15, 2016

vishh Jul 15, 2016

MikeSpreitzer Jul 19, 2016

euank Jul 19, 2016 •

edited

Loading

Add a new container runtime interface #25899

Add a new container runtime interface #25899

Conversation

yujuhong commented May 19, 2016

vishh commented May 19, 2016

dchen1107 commented May 19, 2016

yifan-gu commented May 19, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vishh commented May 19, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yujuhong commented Jul 1, 2016 • edited Loading

dchen1107 commented Jul 1, 2016

k8s-bot commented Jul 1, 2016

tmrts commented Jul 1, 2016

yujuhong commented Jul 1, 2016

k8s-bot commented Jul 1, 2016

k8s-github-robot commented Jul 1, 2016

runcom Jul 15, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

euank Jul 19, 2016 • edited Loading

Choose a reason for hiding this comment

yujuhong commented Jul 1, 2016 •

edited

Loading

runcom Jul 15, 2016 •

edited

Loading

euank Jul 19, 2016 •

edited

Loading