apiextensions: fix missing storage on CRD delete without previous CR access #46501
Conversation
|
@sttts: GitHub didn't allow me to request PR reviews from the following users: nikhita. Note that only kubernetes members can review this PR, and authors cannot review their own PRs. 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. I understand the commands that are listed here. |
k8s-ci-robot
added
the
cncf-cla: yes
k8s-github-robot
added
approved
sttts
added
release-note-none
| } | ||
|
|
||
| func (r *crdHandler) getServingInfoFor(crd *apiextensions.CustomResourceDefinition) *crdInfo { | ||
| storageMap := r.customStorage.Load().(crdStorageMap) | ||
| ret, ok := storageMap[crd.UID] | ||
| r.customStorageLock.RLock() |
There was a problem hiding this comment.
@liggitt specifically requested an atomic.Value is several spots with the claim that it's read performance was better than grabbing a non-contentious read lock. I don't know one way or the other, but it was an intentional choice.
| handler := handlers.ListResource(storage, storage, requestScope, forceWatch, minRequestTimeout) | ||
| handler(w, req) | ||
| return | ||
| handlers.ListResource(storage, storage, requestScope, false, minRequestTimeout)(w, req) |
There was a problem hiding this comment.
inline comment these at least? , false /*forceWatch*/,
There was a problem hiding this comment.
I am lucky that my code editor shows the forceWatch automatically.
| } | ||
| return ret.storage | ||
| func (r *crdHandler) GetCustomResourceListerCollectionDeleter(crd *apiextensions.CustomResourceDefinition) finalizer.ListerCollectionDeleter { | ||
| info := r.getServingInfoFor(crd) |
sttts
force-pushed
the
sttts-crd-finalizer-sync
branch
from
c03929b
to
0984a1a
Compare
k8s-github-robot
removed
the
approved
sttts
force-pushed
the
sttts-crd-finalizer-sync
branch
from
0984a1a
to
60c4862
Compare
k8s-github-robot
added
approved
|
Will leave the lgtm to @nikhita. If her test works fine with this, please tag it. |
|
@k8s-bot pull-kubernetes-unit test this |
|
/lgtm |
k8s-ci-robot
added
the
lgtm
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: deads2k, sttts
Needs approval from an approver in each of these OWNERS Files:
You can indicate your approval by writing |
|
@k8s-bot pull-kubernetes-e2e-kops-aws test this |
|
Automatic merge from submit-queue (batch tested with PRs 46501, 45944, 46473) |
Create CR storage on demand when needed from CRD finalizer controller.
/cc @nikhita