server/apiextensions: use indexes and fixes #1294
Conversation
sttts
commented
Jun 20, 2022
sttts
commented
- fix meta data lookup to only be for wildcard requests
- fix cache mutation
- use indexes.
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: sttts 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 |
openshift-ci
bot
added
the
approved
sttts
force-pushed
the
sttts-apiextensions-index
branch
from
fe70dcb to
2fb0837
Compare
|
/assign @ncdc |
sttts
force-pushed
the
sttts-apiextensions-index
branch
from
2fb0837 to
d8da562
Compare
| @@ -235,16 +227,12 @@ func (c *apiBindingAwareCRDLister) List(ctx context.Context, selector labels.Sel | |||
| seen.Insert(crdName(kcpSystemCRDs[i])) | |||
| } | |||
|
|
|||
| apiBindings, err := c.apiBindingLister.List(labels.Everything()) | |||
| objs, err := c.apiBindingIndexer.ByIndex(byWorkspace, clusterName.String()) | |||
There was a problem hiding this comment.
okay, so it replaces logicalcluster.From(apiBinding) != clusterName
| @@ -257,7 +245,7 @@ func (c *apiBindingAwareCRDLister) List(ctx context.Context, selector labels.Sel | |||
| continue | |||
| } | |||
|
|
|||
| if !matchesSelector(crd) { | |||
| if !selector.Matches(labels.Set(crd.Labels)) { | |||
There was a problem hiding this comment.
we don't have to check if the selector is empty anymore?
| for _, obj := range objs { | ||
| crd := obj.(*apiextensionsv1.CustomResourceDefinition) | ||
|
|
||
| if !selector.Matches(labels.Set(crd.Labels)) { |
There was a problem hiding this comment.
okay, ic, the same labels needs to be applied here.
|
|
||
| switch { | ||
| case identity != "": | ||
| if identity != "" { |
There was a problem hiding this comment.
this if, else statement looks okay to me
| for _, v := range crd.Spec.Versions { | ||
| v.Schema = &apiextensionsv1.CustomResourceValidation{ | ||
| old := crd.Spec.Versions | ||
| crd.Spec.Versions = make([]apiextensionsv1.CustomResourceDefinitionVersion, len(old)) |
There was a problem hiding this comment.
since this is a slice, I think it should be make([]apiextensionsv1.CustomResourceDefinitionVersion, 0, len(old))
There was a problem hiding this comment.
ah, nvm, you index down below
| @@ -332,7 +321,7 @@ func (c *apiBindingAwareCRDLister) Refresh(crd *apiextensionsv1.CustomResourceDe | |||
| } | |||
|
|
|||
| // Start with a shallow copy | |||
| refreshed := shallowCopyCRD(updatedCRD) | |||
| refreshed := shallowCopyCRDAndDeepCopyAnnotations(updatedCRD) | |||
There was a problem hiding this comment.
w00t? why are we modifying a live object? can we ensure no one else is actually using the obj?
|
/lgtm |
