diff --git a/pkg/cache/cache.go b/pkg/cache/cache.go index e4accc7011..0add6afd53 100644 --- a/pkg/cache/cache.go +++ b/pkg/cache/cache.go @@ -22,7 +22,6 @@ import ( "net/http" "time" - "github.com/kcp-dev/apimachinery/v2/third_party/informers" "golang.org/x/exp/maps" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/meta" @@ -486,7 +485,7 @@ func defaultOpts(config *rest.Config, opts Options) (Options, error) { } if opts.NewInformerFunc == nil { - opts.NewInformerFunc = informers.NewSharedIndexInformer + opts.NewInformerFunc = toolscache.NewSharedIndexInformer } return opts, nil } diff --git a/pkg/cache/cache_test.go b/pkg/cache/cache_test.go index aeebcbe580..5e5353ae9a 100644 --- a/pkg/cache/cache_test.go +++ b/pkg/cache/cache_test.go @@ -26,7 +26,6 @@ import ( "strings" "time" - kcpcache "github.com/kcp-dev/apimachinery/v2/pkg/cache" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" corev1 "k8s.io/api/core/v1" @@ -545,14 +544,9 @@ func NonBlockingGetTest(createCacheFunc func(config *rest.Config, opts cache.Opt Expect(err).NotTo(HaveOccurred()) By("creating the informer cache") - v := reflect.ValueOf(&opts).Elem() - newInformerField := v.FieldByName("NewInformerFunc") - newFakeInformer := func(_ kcache.ListerWatcher, _ runtime.Object, _ time.Duration, _ kcache.Indexers) kcpcache.ScopeableSharedIndexInformer { + opts.NewInformerFunc = func(_ kcache.ListerWatcher, _ runtime.Object, _ time.Duration, _ kcache.Indexers) kcache.SharedIndexInformer { return &controllertest.FakeInformer{Synced: false} } - reflect.NewAt(newInformerField.Type(), newInformerField.Addr().UnsafePointer()). - Elem(). - Set(reflect.ValueOf(newFakeInformer)) informerCache, err = createCacheFunc(cfg, opts) Expect(err).NotTo(HaveOccurred()) By("running the cache and waiting for it to sync") diff --git a/pkg/cache/internal/informers.go b/pkg/cache/internal/informers.go index 18025104d2..305d150bc9 100644 --- a/pkg/cache/internal/informers.go +++ b/pkg/cache/internal/informers.go @@ -24,8 +24,6 @@ import ( "sync" "time" - kcpcache "github.com/kcp-dev/apimachinery/v2/pkg/cache" - "github.com/kcp-dev/apimachinery/v2/third_party/informers" apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -48,7 +46,7 @@ type InformersOpts struct { Mapper meta.RESTMapper ResyncPeriod time.Duration Namespace string - NewInformer func(cache.ListerWatcher, runtime.Object, time.Duration, cache.Indexers) kcpcache.ScopeableSharedIndexInformer + NewInformer func(cache.ListerWatcher, runtime.Object, time.Duration, cache.Indexers) cache.SharedIndexInformer Selector Selector Transform cache.TransformFunc UnsafeDisableDeepCopy bool @@ -57,7 +55,7 @@ type InformersOpts struct { // NewInformers creates a new InformersMap that can create informers under the hood. func NewInformers(config *rest.Config, options *InformersOpts) *Informers { - newInformer := informers.NewSharedIndexInformer + newInformer := cache.NewSharedIndexInformer if options.NewInformer != nil { newInformer = options.NewInformer } @@ -177,7 +175,7 @@ type Informers struct { unsafeDisableDeepCopy bool // NewInformer allows overriding of the shared index informer constructor for testing. - newInformer func(cache.ListerWatcher, runtime.Object, time.Duration, cache.Indexers) kcpcache.ScopeableSharedIndexInformer + newInformer func(cache.ListerWatcher, runtime.Object, time.Duration, cache.Indexers) cache.SharedIndexInformer // WatchErrorHandler allows the shared index informer's // watchErrorHandler to be set by overriding the options diff --git a/pkg/cache/kcp_test.go b/pkg/cache/kcp_test.go new file mode 100644 index 0000000000..cfc1ab3de9 --- /dev/null +++ b/pkg/cache/kcp_test.go @@ -0,0 +1,90 @@ +/* +Copyright 2024 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package cache_test + +import ( + "context" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + corev1 "k8s.io/api/core/v1" + "sigs.k8s.io/controller-runtime/pkg/cache" + "sigs.k8s.io/controller-runtime/pkg/client" +) + +var _ = Describe("KCP cluster-unaware informer cache", func() { + // Test whether we can have a cluster-unaware informer cache against a single workspace. + // I.e. every object has a kcp.io/cluster annotation, but it should not be taken + // into consideration by the cache to compute the key. + It("should be able to get the default namespace despite kcp.io/cluster annotation", func() { + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + c, err := cache.New(cfg, cache.Options{}) + Expect(err).NotTo(HaveOccurred()) + + By("Annotating the default namespace with kcp.io/cluster") + cl, err := client.New(cfg, client.Options{}) + Expect(err).NotTo(HaveOccurred()) + ns := &corev1.Namespace{} + err = cl.Get(ctx, client.ObjectKey{Name: "default"}, ns) + Expect(err).NotTo(HaveOccurred()) + ns.Annotations = map[string]string{"kcp.io/cluster": "cluster1"} + err = cl.Update(ctx, ns) + Expect(err).NotTo(HaveOccurred()) + + go c.Start(ctx) //nolint:errcheck // Start is blocking, and error not relevant here. + c.WaitForCacheSync(ctx) + + By("By getting the default namespace with the informer") + err = c.Get(ctx, client.ObjectKey{Name: "default"}, ns) + Expect(err).NotTo(HaveOccurred()) + }) +}) + +// TODO: get envtest in place with kcp +/* +var _ = Describe("KCP cluster-aware informer cache", func() { + It("should be able to get the default namespace with kcp.io/cluster annotation", func() { + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + c, err := kcp.NewClusterAwareCache(cfg, cache.Options{}) + Expect(err).NotTo(HaveOccurred()) + + By("Annotating the default namespace with kcp.io/cluster") + cl, err := client.New(cfg, client.Options{}) + Expect(err).NotTo(HaveOccurred()) + ns := &corev1.Namespace{} + err = cl.Get(ctx, client.ObjectKey{Name: "default"}, ns) + Expect(err).NotTo(HaveOccurred()) + ns.Annotations = map[string]string{"kcp.io/cluster": "cluster1"} + err = cl.Update(ctx, ns) + Expect(err).NotTo(HaveOccurred()) + + go c.Start(ctx) //nolint:errcheck // Start is blocking, and error not relevant here. + c.WaitForCacheSync(ctx) + + By("By getting the default namespace with the informer, but cluster-less key should fail") + err = c.Get(ctx, client.ObjectKey{Name: "default"}, ns) + Expect(err).To(HaveOccurred()) + + By("By getting the default namespace with the informer, but cluster-aware key should succeed") + err = c.Get(kontext.WithCluster(ctx, "cluster1"), client.ObjectKey{Name: "default", Namespace: "cluster1"}, ns) + }) +}) +*/ diff --git a/pkg/client/interfaces.go b/pkg/client/interfaces.go index 050055858a..6c47fc35b8 100644 --- a/pkg/client/interfaces.go +++ b/pkg/client/interfaces.go @@ -20,7 +20,6 @@ import ( "context" "time" - kcpcache "github.com/kcp-dev/apimachinery/v2/pkg/cache" apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/client-go/tools/cache" @@ -49,7 +48,7 @@ type Patch interface { // NewInformerFunc describes a function that creates SharedIndexInformers. // Its signature matches cache.NewSharedIndexInformer from client-go. -type NewInformerFunc func(cache.ListerWatcher, runtime.Object, time.Duration, cache.Indexers) kcpcache.ScopeableSharedIndexInformer +type NewInformerFunc func(cache.ListerWatcher, runtime.Object, time.Duration, cache.Indexers) cache.SharedIndexInformer // TODO(directxman12): is there a sane way to deal with get/delete options? diff --git a/pkg/controller/controllertest/util.go b/pkg/controller/controllertest/util.go index 5eb3d8490a..60ec61edec 100644 --- a/pkg/controller/controllertest/util.go +++ b/pkg/controller/controllertest/util.go @@ -19,14 +19,11 @@ package controllertest import ( "time" - kcpcache "github.com/kcp-dev/apimachinery/v2/pkg/cache" - "github.com/kcp-dev/logicalcluster/v3" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/tools/cache" ) var _ cache.SharedIndexInformer = &FakeInformer{} -var _ kcpcache.ScopeableSharedIndexInformer = &FakeInformer{} // FakeInformer provides fake Informer functionality for testing. type FakeInformer struct { @@ -81,11 +78,6 @@ func (e eventHandlerWrapper) OnDelete(obj interface{}) { e.handler.(legacyResourceEventHandler).OnDelete(obj) } -// Cluster returns the fake Informer. -func (f *FakeInformer) Cluster(clusterName logicalcluster.Name) cache.SharedIndexInformer { - return f -} - // AddIndexers does nothing. TODO(community): Implement this. func (f *FakeInformer) AddIndexers(indexers cache.Indexers) error { return nil diff --git a/pkg/kcp/wrappers.go b/pkg/kcp/wrappers.go index 2e173ccbc6..7baa0e9d6d 100644 --- a/pkg/kcp/wrappers.go +++ b/pkg/kcp/wrappers.go @@ -70,7 +70,7 @@ func NewClusterAwareCache(config *rest.Config, opts cache.Options) (cache.Cache, c := rest.CopyConfig(config) c.Host += "/clusters/*" - opts.NewInformerFunc = func(lw k8scache.ListerWatcher, obj runtime.Object, syncPeriod time.Duration, indexers k8scache.Indexers) kcpcache.ScopeableSharedIndexInformer { + opts.NewInformerFunc = func(lw k8scache.ListerWatcher, obj runtime.Object, syncPeriod time.Duration, indexers k8scache.Indexers) k8scache.SharedIndexInformer { indexers[kcpcache.ClusterIndexName] = kcpcache.ClusterIndexFunc indexers[kcpcache.ClusterAndNamespaceIndexName] = kcpcache.ClusterAndNamespaceIndexFunc