HACK: Search for the right CRD cluster when watching with wildcards #15
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This hack fixes issue kcp-dev/kcp#183: One cannot watch with wildcards (across logical clusters) if the CRD of the related API Resource hasn't been added in the admin logical cluster first. The fix in this HACK is limited since the request will fail if 2 logical clusters contain CRDs for the same GVK with non-equal specs (especially non-equal schemas). Signed-off-by: David Festal <dfestal@redhat.com>
sttts
reviewed
Nov 4, 2021
| crds, err = r.crdLister.List(labels.Everything()) | ||
| if err == nil { | ||
| if len(crds) == 0 { | ||
| err = errors.NewNotFound(schema.GroupResource{Group: apiextensionsv1.SchemeGroupVersion.Group, Resource: "customresourcedefinitions"}, "") |
There was a problem hiding this comment.
would expect an empty set rather than a NotFound.
There was a problem hiding this comment.
Well, it seems to me that returning an empty set would mainly mean: this API exists but there is not resource of this API.
The situation here is that the API itself is unknown (no CRD is available), which afaict is reflected by a NotFound error.
But I realize that I should also trigger a NotFound error when no CRD was found after iterating the CRD list.
There was a problem hiding this comment.
sounds reasonable. The proxy will ignore NotFound if there is another shard that has the CRD.
sttts
reviewed
Nov 4, 2021
| crd = aCRD | ||
| crdClusterName = aCRD.GetClusterName() | ||
| } else { | ||
| if !equality.Semantic.DeepEqual(crd.Spec, aCRD.Spec) { |
There was a problem hiding this comment.
I see why you say it is a temporary hack :)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What type of PR is this?
/kind bug
What this PR does / why we need it:
Before this PR one could not watch with wildcards (across logical clusters) if the CRD of the related API Resource hadn't been added in the
adminlogical cluster first.Now when watching or listing with wildcards on a resource driven by a CRD, we search for the right logical cluster hosting the given CRD.
Which issue(s) this PR fixes:
Fixes kcp-dev/kcp#183
Special notes for your reviewer:
The fix in this Kube HACK is limited since the request will fail if 2 logical clusters
contain CRDs for the same GVK with non-equal specs (especially non-equal schemas).
The complete implementation would come when implementation directions for API Definition management in KCP are clearly defined.
Does this PR introduce a user-facing change?
NONE
Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.: