REQUEST: Create kubernetes/cri-client staging repository

[saschagrunert]

New repo, staging repo, or migrate existing

new repo

Is it a staging repo?

yes

Requested name for new repository

cri-client

Which Organization should it reside

kubernetes

Who should have admin access?

SIG Node admins

Who should have write access?

SIG Node admins and the publishing bot

Who should be listed as approvers in OWNERS?

SIG Node Approvers

Who should be listed in SECURITY_CONTACTS?

committee-security-response

What should the repo description be?

Container Runtime Interface client implementation

What SIG and subproject does this fall under?

SIG Node

Please provide references to appropriate approval for this new repo

• Discussion: https://groups.google.com/g/kubernetes-sig-architecture/c/RZH8bY5ONy4/m/MHny9WuYAQAJ

cc @kubernetes/sig-node-leads

Additional context for request

No response

[dims]

+1 from me

[SergeyKanzhelev]

I like the idea of exposing the opinionated client library for CRI. Definitely would have helped when we migrated from v1alpha2 to v1 and had the fallback logic implemented in both places - crictl and kubelet. Expanding a little bit on this for everybody's context.

So the client will have the following features:

• choosing the right API version and establishing connection
• check the service connectivity (validateServiceConnection)
• gRPC options including various timeouts and the fact that some calls do have timeouts and others - don't
• Tracing
• Error handling

It may be a good idea to move the instrumented_service as well to also unify on metric names from those calls.

The client will not change any promises regarding which runtime versions skews we support. Also no promises on back compatibility of golang APIs as it will be versioned as 0.x. Making it public puts a little bit of pressure on maintainers to minimize the breaking changes in golang APIs. But I don't remember many changes there recently, so it is not a big issue.

A few comments on some specifics:

1. Is cri-client will be a better name?
2. Do we need to remove all the logs from the library? It may not be expected by clients to see those logs.
3. As mentioned above, it may be a good idea to move the instrumented_service as well.
4. We have a comment on error handling that we may not need specific TImeoutError type. I think it is coming from the time when we had both - dockershim and CRI. If we decide to clean it up - it will be a golang API change. We need to be open to those kind of changes.

[saschagrunert]

A few comments on some specifics:

1. Is cri-client will be a better name?

Yes probably, what do others think?

2. Do we need to remove all the logs from the library? It may not be expected by clients to see those logs.

Yeah we could make the logging an optional part before moving to code over to staging.

3. As mentioned above, it may be a good idea to move the instrumented_service as well.

Yes, what about utilities like log parsing?

https://github.com/kubernetes/kubernetes/blob/7ea3d0245a63fbbba698f1cb939831fe8143db3e/pkg/kubelet/kuberuntime/logs/logs.go#L283

4. We have a comment on error handling that we may not need specific TImeoutError type. I think it is coming from the time when we had both - dockershim and CRI. If we decide to clean it up - it will be a golang API change. We need to be open to those kind of changes.

Means we should remove that type before going to staging?

[saschagrunert]

We can also move the cri streaming server implementation there: https://github.com/kubernetes/kubernetes/tree/master/staging/src/k8s.io/kubelet/pkg/cri/streaming

So I'd say we can just stick to cri.

[mrbobbytables]

just wanted to check in on the status of this. Is this still a thing?

[saschagrunert]

just wanted to check in on the status of this. Is this still a thing?

Yes, we just wait on consensus from SIG Node. @mrunalp @haircommander do we have an update from that side?

[SergeyKanzhelev]

I am generally supportive. My suggestion is to define what will be in scope of the library and when we do clean ups like removing logs - before or after moving it to staging.

As for including the streaming server to the same library. @saschagrunert do you envision crictl reusing some of the methods? It is still a client, right? I am just not sure the cri as a name of a package is very descriptive and if many people may get confused

[saschagrunert]

@SergeyKanzhelev the pre-cleanup already has started with:

• Decouple kubelet/cri/remote package from pkg/features kubernetes#123796
• Decouple kubelet/cri/remote package from kubelet/metrics kubernetes#123795

In scope for the new repo sould be:

• pkg/kubelet/cri/remote
• staging/src/k8s.io/kubelet/pkg/cri/streaming

do you envision crictl reusing some of the methods

Yes, that's one of the goals. Other folks reached out to me which wanted to build their own clients without relying on crictl. We can also use the library to make kubeadm independent from crictl.

I am just not sure the cri as a name of a package is very descriptive and if many people may get confused

We could also call the repository cri-remote, cri-impl or cri-client, I'm not opposed to that.

[saschagrunert]

Changed the name to cri-client for now.

[SergeyKanzhelev]

I am happy with everything here! I think it will be a great library to expose and will improve all the cri clients

[mrunalp]

👍 from me.

[cblecker]

/assign

I've created the repo and the blank commit, as well as approved #4806

Next steps:

• The staging repo content needs to be merged into https://github.com/kubernetes/kubernetes/tree/master/staging -- note this should include standard templated content (https://github.com/kubernetes/kubernetes-template-project) and things like the pull request template to deter direct PRs
• Add branch protection for cri-client test-infra#32537 needs to merge
• Once all the above is done, it needs to be added to https://github.com/kubernetes/publishing-bot/blob/master/hack/repos.sh
• Added to sigs.yaml

@saschagrunert: will you be taking care of these?

[saschagrunert]

@saschagrunert: will you be taking care of these?

Yes, following-up in kubernetes/kubernetes#123797, kubernetes/publishing-bot#426 and kubernetes/community#7857

[saschagrunert]

Looks like we’re don, thank y’all for the support!