Skip to content
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

updating CRD causes connection loss with active watches of custom resources #113966

Open
l1b0k opened this issue Nov 17, 2022 · 11 comments
Open

updating CRD causes connection loss with active watches of custom resources #113966

l1b0k opened this issue Nov 17, 2022 · 11 comments
Assignees
Labels
area/apiserver area/custom-resources kind/bug Categorizes issue or PR as related to a bug. sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. triage/accepted Indicates an issue or PR is ready to be actively worked on.

Comments

@l1b0k
Copy link

l1b0k commented Nov 17, 2022

What happened?

  1. update CRD 's description fields
  2. the client watching for the cr resource will lost watch Terminate custom resource watches when storage is destroyed #78029

What did you expect to happen?

As i only chang the CRD's descriptions , this is not necessory to notify all client about the change.
This increase signifient pressure for kube-apiserver in large cluster.

How can we reproduce it (as minimally and precisely as possible)?

update CRD fields, and you can see watch connection is lost

Anything else we need to know?

No response

Kubernetes version

$ kubectl version
Server Version: version.Info{Major:"1", Minor:"22+", GitVersion:"v1.22.15-aliyun.1", GitCommit:"707e514954f0f3ba8ce36face7cf7058403057bc", GitTreeState:"clean", BuildDate:"2022-09-22T03:45:47Z", GoVersion:"go1.16.15", Compiler:"gc", Platform:"linux/amd64"}

/sig api-machinery

@l1b0k l1b0k added the kind/bug Categorizes issue or PR as related to a bug. label Nov 17, 2022
@k8s-ci-robot k8s-ci-robot added needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Nov 17, 2022
@l1b0k
Copy link
Author

l1b0k commented Nov 17, 2022

/sig api-machinery

@k8s-ci-robot k8s-ci-robot added sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. and removed needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. labels Nov 17, 2022
@Ritikaa96
Copy link
Contributor

Maybe we can retitle it as:
updating CRD causes connection loss with active watches of custom resources.
for a better understanding.

@Ritikaa96
Copy link
Contributor

/area api-server
/area custom-resources

@k8s-ci-robot
Copy link
Contributor

@Ritikaa96: The label(s) area/api-server cannot be applied, because the repository doesn't have them.

In response to this:

/area api-server
/area custom-resources

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.

@Ritikaa96
Copy link
Contributor

Ritikaa96 commented Nov 17, 2022

Whoops , my bad!
/area apiserver

@l1b0k l1b0k changed the title Update CRD cause apiserver updating CRD causes connection loss with active watches of custom resources Nov 17, 2022
@Sakuralbj
Copy link
Contributor

Sakuralbj commented Nov 17, 2022

Now crdHandler compare the spec and acceptedNames of crd to decide if crd update should be ignored.
So if some description info such as spec.versions.additionalPrinterColumns.description was changed,
kube-apiserver will destory storage and close watches. Can we make a more precise comparison here to reduce unnecessary connection close. In large-cluster the reconnection will bring huge load to apiserver. https://github.com/kubernetes/kubernetes/blob/master/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/customresource_handler.go#L499

@Sakuralbj
Copy link
Contributor

Maybe spec and acceptedNames here is not correct enough to compare against if a change is made on a CRD. Whether we can build a new struct to save necessary info which can be used to judge if storage need change.

@leilajal
Copy link
Contributor

/triage accepted

@k8s-ci-robot k8s-ci-robot added triage/accepted Indicates an issue or PR is ready to be actively worked on. and removed needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Nov 17, 2022
@k8s-triage-robot
Copy link

This issue has not been updated in over 1 year, and should be re-triaged.

You can:

  • Confirm that this issue is still relevant with /triage accepted (org members only)
  • Close this issue with /close

For more details on the triage process, see https://www.kubernetes.dev/docs/guide/issue-triage/

/remove-triage accepted

@k8s-ci-robot k8s-ci-robot added needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. and removed triage/accepted Indicates an issue or PR is ready to be actively worked on. labels Jan 19, 2024
@Ritikaa96
Copy link
Contributor

hi @l1b0k does the error still exist on your side?

@jiahuif
Copy link
Member

jiahuif commented Jan 25, 2024

/triage accepted

@k8s-ci-robot k8s-ci-robot added triage/accepted Indicates an issue or PR is ready to be actively worked on. and removed needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Jan 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/apiserver area/custom-resources kind/bug Categorizes issue or PR as related to a bug. sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. triage/accepted Indicates an issue or PR is ready to be actively worked on.
Projects
None yet
Development

No branches or pull requests

8 participants