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
CRI: expose optional runtime features #32803
Comments
@yujuhong Why checking runtime versions is not viable? |
Do we need this as an explicit API? Why not just pass sysctl to the On Thu, Sep 15, 2016 at 8:34 PM, Pengfei Ni notifications@github.com
|
@thockin One benefit is failing early before calling runtime API if features are not supported. |
Not saying including features in versions. My thought is the version itself is sufficient to provide information on what features are supported or not (e.g. docker 1.1x support feature a, b, c, etc) Once the runtime tells us the version, we can infer what features are supported or not, right? |
@yifan-gu Versions of different runtime mean different things. if we just let runtime return its version, kubelet must have some runtime specific logic to figure out what docker 1.1x supports, what rkt 1.0x supports etc. |
Fair enough. |
I am not sure that is a compelling enough reason on its own. On Sep 15, 2016 8:45 PM, "Pengfei Ni" notifications@github.com wrote:
|
|
On Mon, Sep 19, 2016 at 2:38 PM, Yu-Ju Hong notifications@github.com wrote:
This is what I was thinking
Kubelet should NOT be checking versions of plugins. It must be |
Ignoring features may be ok in some cases, and I think this is what we do in k8s today (older kubelets ignore new features). On the other hand, if the feature is related to security (e.g., apparmor?), letting the runtime silently ignore it seems dangerous. |
Can we somehow close the loop? Maybe pass back the runtime's pod, then do On Mon, Sep 19, 2016 at 4:23 PM, Yu-Ju Hong notifications@github.com
|
Let's break down the problem a bit more.
apiserver <-> kubelet doesn't have problem (1). I'd like to enforce that runtimes should support all fields in the config and it should reject the request if it can't. This does have the implication that every time we want to add a new feature, we'd have to force all runtimes to support this (or they cannot upgrade). cc @kubernetes/sig-rktnetes As for (2), we also don't handle this in apiserver <-> kubelet in general, with some exceptions.
This either requires kubelet to check runtime+version, or the runtime to ACK back. If (1) is true, kubelet could potentially just check the API version that the runtime uses and determine whether the feature is supported (but it wouldn't be able to reject a pod outright without trying the request first). |
On Mon, Sep 26, 2016 at 11:54 AM, Yu-Ju Hong notifications@github.com wrote:
Will this cause huge friction?Our API is not perfectly abstract, and
We already have the security problem. If kubelet is 1.3.x and reads a
|
This is also partly a distribution problem I think, or at least can be solved by making sufficiently opinionated distribution coupling decisions. If we can mandate "the shim is tightly tied to kubelet in distribution and version", then things become much easier. It could be mandated that each shim must tightly tie to a kubelet version. For rktlet/dockerlet for now, this will be enforced by being in the same binary. I'm suggesting that other runtimes could do something similar to that; e.g. you could have the It would then be up to the shim to either have a tighter or looser coupling; right now This would at least let kubelet assume an arbitrary runtime shim knows about every field it might set (whether or not it ignores it, which would then be a runtime shim implementation choice). Thoughts? |
It's a little draconian, but not a terrible idea. But is this not covered On Fri, Sep 30, 2016 at 5:37 PM, Euan Kemp notifications@github.com wrote:
|
Seems we need to achieve: So it sounds like a BIG WARNING is the way to go... |
Automatic merge from submit-queue Add sysctls for dockershim This PR adds sysctls support for dockershim. All sysctls e2e tests are passed in my local settings. Note that sysctls runtimeAdmit is not included in this PR, it is addressed in #32803. cc/ @yujuhong @Random-Liu
Bump to 1.6 @dims |
thanks @euank |
Yes, that's the sort of thing we need. Then there may be a need to provide other hints as well.. for example are certain capabilities within each also supported. For example, some defaults may not be specified on the platform / environment. (docker/default, unconfined, ...) As another example, there may be a minimum version number for the capability that is necessary for the container. |
Or.. we make the demand that to be a CRI implementation, of a certain type, you must implement a specified set of features. That might be easier? By type I mean, to qualify the difference between linux/windows, vms / containers, ... |
this lgtm, or else it will be too complex. @mikebrow any ideas on the initial capabilities list? |
I've been trying to figure that out as I go through implementing some of them :-) The pattern I'm using is crawl through the docker shim implementation to derive what the expectations are for each security product, and code it up on the cri-containerd side. For example, for seccomp, {minumum version, docker/default, unconfined, localhost/, runtime/default} |
IMO the best way to ensure the expected features are there is to have tests for them in e2e node or cri-tools validate. For example, we have the SYSCTL and APPARMOR stuff working on cri-containerd. But verifying via e2e node is not a guarantee that we've done all of the needed features for each. For example, as far as I saw, none of the e2e node apparmor tests confirm if runtime/default is supported. |
Side note: I'd rather see an external skip list than internal skips. For example, when testing for apparmor support for a runtime I shouldn't have to first PR a filter somewhere in a validator in kublet maybe another filter in the e2e tests to convince it that while docker or rkt might not support the feature maybe remote crio does. An external skip file would give us the ability to manage the filtering without editing the validators and test cases themselves. That said.. where would the skip list go and who would manage it. One alternative would be the above discussion with an api that queries the capabilities (and possibly sub features of capabilities). Another would be more like how we do white lists and black lists for focus/skips for testing. |
Another example just popped up: #51319 |
Issues go stale after 90d of inactivity. Prevent issues from auto-closing with an If this issue is safe to close now please do so with Send feedback to sig-testing, kubernetes/test-infra and/or |
Stale issues rot after 30d of inactivity. If this issue is safe to close now please do so with Send feedback to sig-testing, kubernetes/test-infra and/or fejta. |
Rotten issues close after 30d of inactivity. Send feedback to sig-testing, kubernetes/test-infra and/or fejta. |
@jessfraz Any comments on this? |
Yeah I think /info or something like it should return so the admission controllers can deny or not for features like they do with docker versions |
#64005 is adding a similar |
@yujuhong @Random-Liu @mikebrow @mrunalp For the list of features, any more ideas of this list: enum RuntimeCapability {
CAPABILITY_SYSCTL = 0;
CAPABILITY_SECCOMP = 1;
CAPABILITY_SELINUX = 2;
CAPABILITY_APPARMOR = 3;
} |
Issues go stale after 90d of inactivity. If this issue is safe to close now please do so with Send feedback to sig-testing, kubernetes/test-infra and/or fejta. |
Stale issues rot after 30d of inactivity. If this issue is safe to close now please do so with Send feedback to sig-testing, kubernetes/test-infra and/or fejta. |
Rotten issues close after 30d of inactivity. Send feedback to sig-testing, kubernetes/test-infra and/or fejta. |
@fejta-bot: Closing this issue. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
Meta-issue for CRI: #28789
For some features, kubelet checks if the underlying runtime supports such features and reject the pod if necessary. One example is the
sysctl
support. Right now, kubelet simply checks if (1) the runtime is docker, and (2) if docker version is higher than the minimum requirement. With CRI, checking runtime versions directly is no longer viable. One way to solve this is to ask runtime to expose a set of features it supports through CRI. E.g., anInfo
call can be added to CRI to retrieve a list of supported features.Thoughts? Objections?
/cc @kubernetes/sig-node @thockin
Note: sysctl is still an experimental feature and relies on pod annotations in the k8s API. However, the pod admission logic is already live in kubelet.
The text was updated successfully, but these errors were encountered: