Add CRI v1 proto #96387
Add CRI v1 proto #96387
Conversation
k8s-ci-robot
added
release-note-none
|
@mrunalp: This issue is currently awaiting triage. If a SIG or subproject determines this is a relevant issue, they will accept it by applying the The 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. |
k8s-ci-robot
added
needs-priority
| function generate_code() { | ||
| RUNTIME_API_VERSION="$1" | ||
| KUBE_REMOTE_RUNTIME_PATH="${KUBE_REMOTE_RUNTIME_ROOT}/${RUNTIME_API_VERSION}" | ||
| PATH="${gogopath}:${PATH}" \ |
There was a problem hiding this comment.
not sure how it works in bash. PATH is a global variable. first call will set the first version. Second - will have the first version already set and add a new runtime to the end. Would it pick up the second runtime at all?
There was a problem hiding this comment.
Moving out of the function.
| @@ -0,0 +1,55 @@ | |||
| /* | |||
| Copyright 2016 The Kubernetes Authors. | |||
There was a problem hiding this comment.
| Copyright 2016 The Kubernetes Authors. | |
| Copyright 2020 The Kubernetes Authors. |
| @@ -0,0 +1,1338 @@ | |||
| /* | |||
| Copyright 2018 The Kubernetes Authors. | |||
There was a problem hiding this comment.
| Copyright 2018 The Kubernetes Authors. | |
| Copyright 2020 The Kubernetes Authors. |
| // | ||
| // WARNING: Log management and how kubelet should interface with the | ||
| // container logs are under active discussion in | ||
| // https://issues.k8s.io/24677. There *may* be future change of direction |
There was a problem hiding this comment.
this issue is closed. Should we keep the warning around?
There was a problem hiding this comment.
Dropped the warning.
| // If the runtime handler is unknown, this request should be rejected. An | ||
| // empty string should select the default handler, equivalent to the | ||
| // behavior before this feature was added. | ||
| // See https://git.k8s.io/enhancements/keps/sig-node/runtime-class.md |
There was a problem hiding this comment.
| // See https://git.k8s.io/enhancements/keps/sig-node/runtime-class.md | |
| // See https://git.k8s.io/enhancements/keps/sig-node/585-runtime-class/README.md |
|
|
||
| // LinuxContainerResources specifies Linux specific configuration for | ||
| // resources. | ||
| // TODO: Consider using Resources from opencontainers/runtime-spec/specs-go |
There was a problem hiding this comment.
Are we keeping TODO in beta?
|
|
||
| // Variables for interactive containers, these have very specialized | ||
| // use-cases (e.g. debugging). | ||
| // TODO: Determine if we need to continue supporting these fields that are |
There was a problem hiding this comment.
should we keep this TODO in beta?
| // StopContainer stops a running container with a grace period (i.e., timeout). | ||
| // This call is idempotent, and must not return an error if the container has | ||
| // already been stopped. | ||
| // TODO: what must the runtime do after the grace period is reached? |
There was a problem hiding this comment.
should we keep TODO in beta?
There was a problem hiding this comment.
Replaced TODO by a line describing that the runtime should kill on grace period expiration.
mrunalp
force-pushed
the
cri_add_beta
branch
from
aa51439
to
47a544d
Compare
|
/lgtm |
k8s-ci-robot
added
the
lgtm
|
thx for updating/removing the todo's I had a todo to do that and got swamped :-) |
|
/milestone v1.20 |
|
/hold we are discussing some options in sig-node to potentially rename the package to v1 to avoid multiple hops. |
k8s-ci-robot
added
the
do-not-merge/hold
|
This seems a little rushed to try and pull this off for v1.20. What's the benefit in doing it now vs waiting for v1.21? |
mrunalp
force-pushed
the
cri_add_beta
branch
from
47a544d
to
d8bc4ea
Compare
k8s-ci-robot
removed
the
lgtm
mrunalp
force-pushed
the
cri_add_beta
branch
from
d8bc4ea
to
d25f1ea
Compare
Add CRI v1 proto and generated code. We need both v1 and v1alpha2 side by side so that containerd and CRI-O could be updated. Once the runtimes are updated and in the CI, we can switch the kubelet to use v1 in 1.21 . We are jumping to v1, so we have to avoid multiple hops to get to GA. The package could stay v1 and declare CRI support to be at beta and eventually GA. Signed-off-by: Mrunal Patel <mpatel@redhat.com>
mrunalp
force-pushed
the
cri_add_beta
branch
from
d25f1ea
to
9fcede9
Compare
|
/retest |
We need a cycle to allow runtimes to support v1 before they can be brought back into CI and change the kubelet code. See - #96325 (comment) |
|
/hold cancel |
k8s-ci-robot
removed
the
do-not-merge/hold
|
Thanks @mrunalp, that makes sense then. |
k8s-ci-robot
added
the
lgtm
|
/approve @mrunalp please remove hold when you are ready |
k8s-ci-robot
added
the
do-not-merge/hold
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: dims, mrunalp, saschagrunert The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
k8s-ci-robot
added
the
approved
|
/hold cancel |
k8s-ci-robot
removed
the
do-not-merge/hold
@derekwaynecarr @SergeyKanzhelev @saschagrunert @mikebrow
Signed-off-by: Mrunal Patel mpatel@redhat.com
What type of PR is this?
/kind feature
What this PR does / why we need it:
Add CRI v1beta proto and generated code.
We need both v1beta1 and v1alpha2 side by side so that
containerd and CRI-O could be updated. Once the runtimes
are updated and in the CI, we can switch the kubelet
to use v1beta1.
Does this PR introduce a user-facing change?:
Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.: