Drop use of kcp-dev fork of kubernetes #13
Merged
Conversation
This file contains hidden or 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
fabianvf
force-pushed
the
no-go-mod-changes
branch
from
d93caf4 to
29269aa
Compare
fabianvf
commented
Jun 10, 2022
fabianvf
commented
Jun 10, 2022
fabianvf
commented
Jun 10, 2022
ncdc
reviewed
Jun 16, 2022
fabianvf
force-pushed
the
no-go-mod-changes
branch
from
12a8fca to
2b949dd
Compare
- Remove customizable keyfunction - Add ability to customize function used to build new informers - Embed cluster name in reconcile.Request instead of client.ObjectKey
fabianvf
force-pushed
the
no-go-mod-changes
branch
from
2b949dd to
0106f54
Compare
There was a problem hiding this comment.
@johnmcollier would this PR solve your dependency problems?
Comment on lines
+200
to
+209
| cluster, ok := kcpclient.ClusterFromContext(ctx) | ||
| if ok { | ||
| return kcpcache.ToClusterAwareKey(cluster.String(), k.Namespace, k.Name) | ||
| } |
There was a problem hiding this comment.
This looks like it should support also the case when the context doesn't have the cluster set - in other words in a standard K8s/OpenShift environment. However, I'm afraid that it won't work if the cache reader still depends on the different keys. See the changes in my PR: #14
There was a problem hiding this comment.
Yup, assuming this works, this should unblock me.
|
@fabianvf can you squash down the shared_informer.go commits? |
Signed-off-by: Andy Goldstein <[email protected]>
Signed-off-by: Andy Goldstein <[email protected]>
|
@fabianvf's changes LGTM |
fabianvf
commented
Jun 24, 2022
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.
adds plumbing of a NewSharedInformer func that allows us to provide multi-cluster informers without requiring us to depend on the kcp-dev fork of kubernetes