KEP: Custom errors in the CRI #1654
Conversation
k8s-ci-robot
added
cncf-cla: yes
k8s-ci-robot
added
kind/kep
| look idiomatic. | ||
|
|
||
| #### Option 3: Use grpc rpc status and add helper package | ||
| ``` |
There was a problem hiding this comment.
seems option 3 is not complete? Would those error codes part of CRI API?
There was a problem hiding this comment.
We will use the error codes from grpc with a wrapper helper package in the CRI API.
derekwaynecarr
left a comment
derekwaynecarr
left a comment
There was a problem hiding this comment.
link to existing GRPC error codes, otherwise, i think this is good to go per sig-node meeting discussion. we can evaluate how to handle richer errors if the need arises after we have worked through an experience with the simplest NOT_FOUND error.
| } | ||
|
|
||
| ``` | ||
|
|
There was a problem hiding this comment.
maybe link to the current status code definitions:
https://github.com/grpc/grpc/blob/master/doc/statuscodes.md
the implication is that we would add support in kubelet to make NOT_FOUND idiomatic for all GET operations that do not mutate system state. we could possibly also do the same with ALREADY_EXISTS for mutating operations, but I dont view that as an immediate requirement.
| - Works across programming langauges. | ||
| - The helper package allows us to change implementation in the future | ||
| if needed. | ||
|
|
There was a problem hiding this comment.
enables us to optimize kubelet further without needing to do major code sweep given that kubelet always has to handle the error!
Signed-off-by: Mrunal Patel <mpatel@redhat.com>
k8s-ci-robot
added
the
sig/architecture
|
@mrunalp: The following test failed, say
Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR. 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. I understand the commands that are listed here. |
|
Updated. I will open a PR for the cri api later today. |
|
Opened PR - kubernetes/kubernetes#91273 |
|
thanks for update mrunal. /approve |
k8s-ci-robot
added
the
lgtm
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: derekwaynecarr, mrunalp 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
|
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. |
k8s-ci-robot
added
the
lifecycle/stale
|
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. |
k8s-ci-robot
added
lifecycle/rotten
|
Rotten issues close after 30d of inactivity. Send feedback to sig-testing, kubernetes/test-infra and/or fejta. |
|
@fejta-bot: Closed this PR. 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. |
update vSphere driver configuration enhancement
5 participants
This PR adds a KEP to introduce custom errors in the CRI as discussed in sig node.
Signed-off-by: Mrunal Patel mpatel@redhat.com