From 3ae7b6592ef4e63b22b290563d096180752d6e2b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20Luk=C5=A1a?= Date: Fri, 7 Oct 2022 14:17:15 +0200 Subject: [PATCH] OSSM-2006 Alternative multiNamespaceInformer.HasSynced() fix (#24) (#27) In the previous fix for informer.HasSynced(), the informer factories were constructed with no namespaces, enabling multiNamespaceInformer.HasSynced() to only return true after the member roll controller initialized the factory and informers by calling SetNamespaces(). Unfortunately, this broke all istio tests where the member roll controller isn't involved. In those cases, the factory and informers must be constructed so that they track all namespaces. Here, we add the ability for the member roll controller to call SetNamespaces(nil) when the factory is added to it as a listener. This ensures that the informer will return false on HasSynced() calls until the controller invokes SetNamespaces() with the actual list of namespaces read from the ServiceMeshMemberRoll. This ensures that the informer factory works correctly in both scenarios - with and without the member roll controller. --- cmd/xns-informer-gen/generators/factory.go | 10 ++-- pkg/generated/istio/factory.go | 10 ++-- pkg/generated/kube/factory.go | 10 ++-- pkg/generated/serviceapis/factory.go | 10 ++-- pkg/informers/dynamic.go | 8 +-- pkg/informers/dynamic_test.go | 18 +++---- pkg/informers/informer_test.go | 6 +-- pkg/informers/metadata.go | 8 +-- pkg/informers/metadata_test.go | 14 +++-- pkg/informers/namespace_set.go | 37 +++++++++----- pkg/informers/namespace_set_test.go | 59 +++++++++++++++------- 11 files changed, 110 insertions(+), 80 deletions(-) diff --git a/cmd/xns-informer-gen/generators/factory.go b/cmd/xns-informer-gen/generators/factory.go index 6d28a3a3..02db8b33 100644 --- a/cmd/xns-informer-gen/generators/factory.go +++ b/cmd/xns-informer-gen/generators/factory.go @@ -140,7 +140,7 @@ func WithTweakListOptions(tweakListOptions internalinterfaces.TweakListOptionsFu // WithNamespaces limits the SharedInformerFactory to the specified namespaces. func WithNamespaces(namespaces ...string) SharedInformerOption { return func(factory *sharedInformerFactory) *sharedInformerFactory { - factory.SetNamespaces(namespaces...) + factory.SetNamespaces(namespaces) return factory } } @@ -154,7 +154,7 @@ func NewSharedInformerFactory(client {{.clientSetInterface|raw}}, defaultResync func NewSharedInformerFactoryWithOptions(client {{.clientSetInterface|raw}}, defaultResync {{.timeDuration|raw}}, options ...SharedInformerOption) SharedInformerFactory { factory := &sharedInformerFactory{ client: client, - namespaces: {{.xnsNewNamespaceSet|raw}}(), + namespaces: {{.xnsNewNamespaceSet|raw}}(v1.NamespaceAll), defaultResync: defaultResync, informers: make(map[{{.reflectType|raw}}]{{.cacheSharedIndexInformer|raw}}), startedInformers: make(map[{{.reflectType|raw}}]bool), @@ -170,11 +170,11 @@ func NewSharedInformerFactoryWithOptions(client {{.clientSetInterface|raw}}, def } // SetNamespaces updates the set of namespaces for all current and future informers. -func (f *sharedInformerFactory) SetNamespaces(namespaces ...string) { +func (f *sharedInformerFactory) SetNamespaces(namespaces []string) { f.lock.Lock() defer f.lock.Unlock() - f.namespaces.SetNamespaces(namespaces...) + f.namespaces.SetNamespaces(namespaces) } // Start initializes all requested informers. @@ -242,7 +242,7 @@ var sharedInformerFactoryInterface = ` // API group versions. type SharedInformerFactory interface { {{.informerFactoryInterface|raw}} - SetNamespaces(namespaces ...string) + SetNamespaces(namespaces []string) ForResource(resource {{.schemaGroupVersionResource|raw}}) (GenericInformer, error) WaitForCacheSync(stopCh <-chan struct{}) map[reflect.Type]bool diff --git a/pkg/generated/istio/factory.go b/pkg/generated/istio/factory.go index 4e424244..49169ec4 100644 --- a/pkg/generated/istio/factory.go +++ b/pkg/generated/istio/factory.go @@ -72,7 +72,7 @@ func WithTweakListOptions(tweakListOptions internalinterfaces.TweakListOptionsFu // WithNamespaces limits the SharedInformerFactory to the specified namespaces. func WithNamespaces(namespaces ...string) SharedInformerOption { return func(factory *sharedInformerFactory) *sharedInformerFactory { - factory.SetNamespaces(namespaces...) + factory.SetNamespaces(namespaces) return factory } } @@ -86,7 +86,7 @@ func NewSharedInformerFactory(client versioned.Interface, defaultResync time.Dur func NewSharedInformerFactoryWithOptions(client versioned.Interface, defaultResync time.Duration, options ...SharedInformerOption) SharedInformerFactory { factory := &sharedInformerFactory{ client: client, - namespaces: informers.NewNamespaceSet(), + namespaces: informers.NewNamespaceSet(v1.NamespaceAll), defaultResync: defaultResync, informers: make(map[reflect.Type]cache.SharedIndexInformer), startedInformers: make(map[reflect.Type]bool), @@ -102,11 +102,11 @@ func NewSharedInformerFactoryWithOptions(client versioned.Interface, defaultResy } // SetNamespaces updates the set of namespaces for all current and future informers. -func (f *sharedInformerFactory) SetNamespaces(namespaces ...string) { +func (f *sharedInformerFactory) SetNamespaces(namespaces []string) { f.lock.Lock() defer f.lock.Unlock() - f.namespaces.SetNamespaces(namespaces...) + f.namespaces.SetNamespaces(namespaces) } // Start initializes all requested informers. @@ -171,7 +171,7 @@ func (f *sharedInformerFactory) InformerFor(obj runtime.Object, newFunc internal // API group versions. type SharedInformerFactory interface { internalinterfaces.SharedInformerFactory - SetNamespaces(namespaces ...string) + SetNamespaces(namespaces []string) ForResource(resource schema.GroupVersionResource) (GenericInformer, error) WaitForCacheSync(stopCh <-chan struct{}) map[reflect.Type]bool diff --git a/pkg/generated/kube/factory.go b/pkg/generated/kube/factory.go index 7f8821ff..5d6af7fd 100644 --- a/pkg/generated/kube/factory.go +++ b/pkg/generated/kube/factory.go @@ -87,7 +87,7 @@ func WithTweakListOptions(tweakListOptions internalinterfaces.TweakListOptionsFu // WithNamespaces limits the SharedInformerFactory to the specified namespaces. func WithNamespaces(namespaces ...string) SharedInformerOption { return func(factory *sharedInformerFactory) *sharedInformerFactory { - factory.SetNamespaces(namespaces...) + factory.SetNamespaces(namespaces) return factory } } @@ -101,7 +101,7 @@ func NewSharedInformerFactory(client kubernetes.Interface, defaultResync time.Du func NewSharedInformerFactoryWithOptions(client kubernetes.Interface, defaultResync time.Duration, options ...SharedInformerOption) SharedInformerFactory { factory := &sharedInformerFactory{ client: client, - namespaces: informers.NewNamespaceSet(), + namespaces: informers.NewNamespaceSet(v1.NamespaceAll), defaultResync: defaultResync, informers: make(map[reflect.Type]cache.SharedIndexInformer), startedInformers: make(map[reflect.Type]bool), @@ -117,11 +117,11 @@ func NewSharedInformerFactoryWithOptions(client kubernetes.Interface, defaultRes } // SetNamespaces updates the set of namespaces for all current and future informers. -func (f *sharedInformerFactory) SetNamespaces(namespaces ...string) { +func (f *sharedInformerFactory) SetNamespaces(namespaces []string) { f.lock.Lock() defer f.lock.Unlock() - f.namespaces.SetNamespaces(namespaces...) + f.namespaces.SetNamespaces(namespaces) } // Start initializes all requested informers. @@ -186,7 +186,7 @@ func (f *sharedInformerFactory) InformerFor(obj runtime.Object, newFunc internal // API group versions. type SharedInformerFactory interface { internalinterfaces.SharedInformerFactory - SetNamespaces(namespaces ...string) + SetNamespaces(namespaces []string) ForResource(resource schema.GroupVersionResource) (GenericInformer, error) WaitForCacheSync(stopCh <-chan struct{}) map[reflect.Type]bool diff --git a/pkg/generated/serviceapis/factory.go b/pkg/generated/serviceapis/factory.go index a33465d4..7ae92735 100644 --- a/pkg/generated/serviceapis/factory.go +++ b/pkg/generated/serviceapis/factory.go @@ -71,7 +71,7 @@ func WithTweakListOptions(tweakListOptions internalinterfaces.TweakListOptionsFu // WithNamespaces limits the SharedInformerFactory to the specified namespaces. func WithNamespaces(namespaces ...string) SharedInformerOption { return func(factory *sharedInformerFactory) *sharedInformerFactory { - factory.SetNamespaces(namespaces...) + factory.SetNamespaces(namespaces) return factory } } @@ -85,7 +85,7 @@ func NewSharedInformerFactory(client versioned.Interface, defaultResync time.Dur func NewSharedInformerFactoryWithOptions(client versioned.Interface, defaultResync time.Duration, options ...SharedInformerOption) SharedInformerFactory { factory := &sharedInformerFactory{ client: client, - namespaces: informers.NewNamespaceSet(), + namespaces: informers.NewNamespaceSet(v1.NamespaceAll), defaultResync: defaultResync, informers: make(map[reflect.Type]cache.SharedIndexInformer), startedInformers: make(map[reflect.Type]bool), @@ -101,11 +101,11 @@ func NewSharedInformerFactoryWithOptions(client versioned.Interface, defaultResy } // SetNamespaces updates the set of namespaces for all current and future informers. -func (f *sharedInformerFactory) SetNamespaces(namespaces ...string) { +func (f *sharedInformerFactory) SetNamespaces(namespaces []string) { f.lock.Lock() defer f.lock.Unlock() - f.namespaces.SetNamespaces(namespaces...) + f.namespaces.SetNamespaces(namespaces) } // Start initializes all requested informers. @@ -170,7 +170,7 @@ func (f *sharedInformerFactory) InformerFor(obj runtime.Object, newFunc internal // API group versions. type SharedInformerFactory interface { internalinterfaces.SharedInformerFactory - SetNamespaces(namespaces ...string) + SetNamespaces(namespaces []string) ForResource(resource schema.GroupVersionResource) (GenericInformer, error) WaitForCacheSync(stopCh <-chan struct{}) map[reflect.Type]bool diff --git a/pkg/informers/dynamic.go b/pkg/informers/dynamic.go index 008ffe6e..90a434e3 100644 --- a/pkg/informers/dynamic.go +++ b/pkg/informers/dynamic.go @@ -22,14 +22,14 @@ import ( // namespaces, it will not work for cluster-scoped resources. type DynamicSharedInformerFactory interface { Start(stopCh <-chan struct{}) - SetNamespaces(namespaces ...string) + SetNamespaces(namespaces []string) ForResource(gvr schema.GroupVersionResource) informers.GenericInformer WaitForCacheSync(stopCh <-chan struct{}) map[schema.GroupVersionResource]bool } // NewDynamicSharedInformerFactory constructs a new instance of dynamicSharedInformerFactory for all namespaces. func NewDynamicSharedInformerFactory(client dynamic.Interface, defaultResync time.Duration) DynamicSharedInformerFactory { - namespaces := NewNamespaceSet() + namespaces := NewNamespaceSet(metav1.NamespaceAll) return NewFilteredDynamicSharedInformerFactory(client, defaultResync, namespaces, nil) } @@ -86,11 +86,11 @@ func (f *dynamicSharedInformerFactory) ForResource(gvr schema.GroupVersionResour } // SetNamespaces updates the set of namespaces for all current and future informers. -func (f *dynamicSharedInformerFactory) SetNamespaces(namespaces ...string) { +func (f *dynamicSharedInformerFactory) SetNamespaces(namespaces []string) { f.lock.Lock() defer f.lock.Unlock() - f.namespaces.SetNamespaces(namespaces...) + f.namespaces.SetNamespaces(namespaces) } // Start initializes all requested informers. diff --git a/pkg/informers/dynamic_test.go b/pkg/informers/dynamic_test.go index 8fbc3e2c..25f90b29 100644 --- a/pkg/informers/dynamic_test.go +++ b/pkg/informers/dynamic_test.go @@ -55,7 +55,7 @@ func TestFilteredDynamicSharedInformerFactory(t *testing.T) { // scenario 1 { name: "scenario 1: test adding an object in different namespace should not trigger AddFunc", - informNS: newNamespaceSet("ns-bar"), + informNS: xnsinformers.NewNamespaceSet("ns-bar"), ns: "ns-foo", gvr: schema.GroupVersionResource{Group: "apps", Version: "v1", Resource: "deployments"}, trigger: triggerFactory(t), @@ -64,7 +64,7 @@ func TestFilteredDynamicSharedInformerFactory(t *testing.T) { // scenario 2 { name: "scenario 2: test adding an object should trigger AddFunc", - informNS: newNamespaceSet("ns-foo"), + informNS: xnsinformers.NewNamespaceSet("ns-foo"), ns: "ns-foo", gvr: schema.GroupVersionResource{Group: "apps", Version: "v1", Resource: "deployments"}, trigger: triggerFactory(t), @@ -119,7 +119,6 @@ func TestFilteredDynamicSharedInformerFactory(t *testing.T) { } func TestDynamicSharedInformerFactory(t *testing.T) { - ns := "ns-foo" scenarios := []struct { name string existingObj *unstructured.Unstructured @@ -131,10 +130,10 @@ func TestDynamicSharedInformerFactory(t *testing.T) { // scenario 1 { name: "scenario 1: test if adding an object triggers AddFunc", - ns: ns, + ns: "ns-foo", gvr: schema.GroupVersionResource{Group: "extensions", Version: "v1beta1", Resource: "deployments"}, trigger: func(gvr schema.GroupVersionResource, ns string, fakeClient *fake.FakeDynamicClient, _ *unstructured.Unstructured) *unstructured.Unstructured { - testObject := newUnstructured("extensions/v1beta1", "Deployment", ns, "name-foo") + testObject := newUnstructured("extensions/v1beta1", "Deployment", "ns-foo", "name-foo") createdObj, err := fakeClient.Resource(gvr).Namespace(ns).Create(context.TODO(), testObject, metav1.CreateOptions{}) if err != nil { t.Error(err) @@ -153,9 +152,9 @@ func TestDynamicSharedInformerFactory(t *testing.T) { // scenario 2 { name: "scenario 2: tests if updating an object triggers UpdateFunc", - ns: ns, + ns: "ns-foo", gvr: schema.GroupVersionResource{Group: "extensions", Version: "v1beta1", Resource: "deployments"}, - existingObj: newUnstructured("extensions/v1beta1", "Deployment", ns, "name-foo"), + existingObj: newUnstructured("extensions/v1beta1", "Deployment", "ns-foo", "name-foo"), trigger: func(gvr schema.GroupVersionResource, ns string, fakeClient *fake.FakeDynamicClient, testObject *unstructured.Unstructured) *unstructured.Unstructured { testObject.Object["spec"] = "updatedName" updatedObj, err := fakeClient.Resource(gvr).Namespace(ns).Update(context.TODO(), testObject, metav1.UpdateOptions{}) @@ -176,9 +175,9 @@ func TestDynamicSharedInformerFactory(t *testing.T) { // scenario 3 { name: "scenario 3: test if deleting an object triggers DeleteFunc", - ns: ns, + ns: "ns-foo", gvr: schema.GroupVersionResource{Group: "extensions", Version: "v1beta1", Resource: "deployments"}, - existingObj: newUnstructured("extensions/v1beta1", "Deployment", ns, "name-foo"), + existingObj: newUnstructured("extensions/v1beta1", "Deployment", "ns-foo", "name-foo"), trigger: func(gvr schema.GroupVersionResource, ns string, fakeClient *fake.FakeDynamicClient, testObject *unstructured.Unstructured) *unstructured.Unstructured { err := fakeClient.Resource(gvr).Namespace(ns).Delete(context.TODO(), testObject.GetName(), metav1.DeleteOptions{}) if err != nil { @@ -213,7 +212,6 @@ func TestDynamicSharedInformerFactory(t *testing.T) { } fakeClient := fake.NewSimpleDynamicClientWithCustomListKinds(scheme, gvrToListKind, objs...) target := xnsinformers.NewDynamicSharedInformerFactory(fakeClient, 0) - target.SetNamespaces(ns) // act informerListerForGvr := target.ForResource(ts.gvr) diff --git a/pkg/informers/informer_test.go b/pkg/informers/informer_test.go index 9c341dd7..152eb5d0 100644 --- a/pkg/informers/informer_test.go +++ b/pkg/informers/informer_test.go @@ -103,7 +103,7 @@ func newInformer(obj runtime.Object, lws map[string]cache.ListerWatcher) xnsinfo namespaces = append(namespaces, ns) } - namespaceSet.SetNamespaces(namespaces...) + namespaceSet.SetNamespaces(namespaces) return xnsinformers.NewMultiNamespaceInformer(namespaceSet, resync, func(ns string) cache.SharedIndexInformer { return cache.NewSharedIndexInformer(lws[ns], obj, resync, indexers) @@ -254,7 +254,7 @@ func TestMultiNamespaceInformerEventHandlers(t *testing.T) { // These tests use the fake client instead of a FakeControllerSource. client := kubefake.NewSimpleClientset() - namespaceSet := newNamespaceSet(namespaces...) + namespaceSet := xnsinformers.NewNamespaceSet(namespaces...) informer := xnsinformers.NewMultiNamespaceInformer(namespaceSet, 0, func(namespace string) cache.SharedIndexInformer { return cache.NewSharedIndexInformer( @@ -356,7 +356,7 @@ func TestMultiNamespaceInformerHasSynced(t *testing.T) { t.Fatalf("informer is synced, but shouldn't be because namespaces haven't been set yet") } - namespaceSet.SetNamespaces("ns1", "ns2") + namespaceSet.SetNamespaces([]string{"ns1", "ns2"}) if informer.HasSynced() { t.Fatalf("informer is synced, but shouldn't be because the underlying informers aren't synced") diff --git a/pkg/informers/metadata.go b/pkg/informers/metadata.go index c87f5d61..8925f5dc 100644 --- a/pkg/informers/metadata.go +++ b/pkg/informers/metadata.go @@ -18,14 +18,14 @@ import ( // MetadataSharedInformerFactory provides access to shared informers and listers for metadata client. type MetadataSharedInformerFactory interface { Start(stopCh <-chan struct{}) - SetNamespaces(namespaces ...string) + SetNamespaces(namespaces []string) ForResource(gvr schema.GroupVersionResource) informers.GenericInformer WaitForCacheSync(stopCh <-chan struct{}) map[schema.GroupVersionResource]bool } // NewMetadataSharedInformerFactory constructs a new instance of metadataSharedInformerFactory for all namespaces. func NewMetadataSharedInformerFactory(client metadata.Interface, defaultResync time.Duration) MetadataSharedInformerFactory { - namespaces := NewNamespaceSet() + namespaces := NewNamespaceSet(metav1.NamespaceAll) return NewFilteredMetadataSharedInformerFactory(client, defaultResync, namespaces, nil) } @@ -74,11 +74,11 @@ func (f *metadataSharedInformerFactory) ForResource(gvr schema.GroupVersionResou } // SetNamespaces updates the set of namespaces for all current and future informers. -func (f *metadataSharedInformerFactory) SetNamespaces(namespaces ...string) { +func (f *metadataSharedInformerFactory) SetNamespaces(namespaces []string) { f.lock.Lock() defer f.lock.Unlock() - f.namespaces.SetNamespaces(namespaces...) + f.namespaces.SetNamespaces(namespaces) } // Start initializes all requested informers. diff --git a/pkg/informers/metadata_test.go b/pkg/informers/metadata_test.go index f4229237..03d580c0 100644 --- a/pkg/informers/metadata_test.go +++ b/pkg/informers/metadata_test.go @@ -27,7 +27,6 @@ import ( // } func TestMetadataSharedInformerFactory(t *testing.T) { - ns := "ns-foo" scenarios := []struct { name string existingObj *metav1.PartialObjectMetadata @@ -39,10 +38,10 @@ func TestMetadataSharedInformerFactory(t *testing.T) { // scenario 1 { name: "scenario 1: test if adding an object triggers AddFunc", - ns: ns, + ns: "ns-foo", gvr: schema.GroupVersionResource{Group: "extensions", Version: "v1beta1", Resource: "deployments"}, trigger: func(gvr schema.GroupVersionResource, ns string, fakeClient *fake.FakeMetadataClient, _ *metav1.PartialObjectMetadata) *metav1.PartialObjectMetadata { - testObject := newPartialObjectMetadata("extensions/v1beta1", "Deployment", ns, "name-foo") + testObject := newPartialObjectMetadata("extensions/v1beta1", "Deployment", "ns-foo", "name-foo") createdObj, err := fakeClient.Resource(gvr).Namespace(ns).(fake.MetadataClient).CreateFake(testObject, metav1.CreateOptions{}) if err != nil { t.Error(err) @@ -61,9 +60,9 @@ func TestMetadataSharedInformerFactory(t *testing.T) { // scenario 2 { name: "scenario 2: tests if updating an object triggers UpdateFunc", - ns: ns, + ns: "ns-foo", gvr: schema.GroupVersionResource{Group: "extensions", Version: "v1beta1", Resource: "deployments"}, - existingObj: newPartialObjectMetadata("extensions/v1beta1", "Deployment", ns, "name-foo"), + existingObj: newPartialObjectMetadata("extensions/v1beta1", "Deployment", "ns-foo", "name-foo"), trigger: func(gvr schema.GroupVersionResource, ns string, fakeClient *fake.FakeMetadataClient, testObject *metav1.PartialObjectMetadata) *metav1.PartialObjectMetadata { if testObject.Annotations == nil { testObject.Annotations = make(map[string]string) @@ -87,9 +86,9 @@ func TestMetadataSharedInformerFactory(t *testing.T) { // scenario 3 { name: "scenario 3: test if deleting an object triggers DeleteFunc", - ns: ns, + ns: "ns-foo", gvr: schema.GroupVersionResource{Group: "extensions", Version: "v1beta1", Resource: "deployments"}, - existingObj: newPartialObjectMetadata("extensions/v1beta1", "Deployment", ns, "name-foo"), + existingObj: newPartialObjectMetadata("extensions/v1beta1", "Deployment", "ns-foo", "name-foo"), trigger: func(gvr schema.GroupVersionResource, ns string, fakeClient *fake.FakeMetadataClient, testObject *metav1.PartialObjectMetadata) *metav1.PartialObjectMetadata { err := fakeClient.Resource(gvr).Namespace(ns).Delete(context.TODO(), testObject.GetName(), metav1.DeleteOptions{}) if err != nil { @@ -122,7 +121,6 @@ func TestMetadataSharedInformerFactory(t *testing.T) { } fakeClient := fake.NewSimpleMetadataClient(scheme, objs...) target := xnsinformers.NewMetadataSharedInformerFactory(fakeClient, 0) - target.SetNamespaces(ns) // act informerListerForGvr := target.ForResource(ts.gvr) diff --git a/pkg/informers/namespace_set.go b/pkg/informers/namespace_set.go index 01002afa..8174aea6 100644 --- a/pkg/informers/namespace_set.go +++ b/pkg/informers/namespace_set.go @@ -42,22 +42,30 @@ func (h NamespaceSetHandlerFuncs) OnRemove(namespace string) { type NamespaceSet interface { // Initialized returns true if SetNamespaces() has been called at least once Initialized() bool - SetNamespaces(namespaces ...string) + SetNamespaces(namespaces []string) AddHandler(handler NamespaceSetHandler) Contains(namespace string) bool List() []string } type namespaceSet struct { - initialized bool - lock sync.Mutex - namespaces sets.Set - handlers []NamespaceSetHandler + lock sync.Mutex + namespaces sets.Set + handlers []NamespaceSetHandler } -// NewNamespaceSet returns a new NamespaceSet. -func NewNamespaceSet() NamespaceSet { - return &namespaceSet{} +// NewNamespaceSet returns a new NamespaceSet tracking the given namespaces. +func NewNamespaceSet(namespaces ...string) NamespaceSet { + n := &namespaceSet{} + n.SetNamespaces(namespaces) + + return n +} + +// NewUninitializedNamespaceSet returns a new uninitialized NamespaceSet +func NewUninitializedNamespaceSet() NamespaceSet { + n := &namespaceSet{} + return n } // Contains indicates whether the given namespace is in the set. @@ -83,20 +91,21 @@ func (n *namespaceSet) Initialized() bool { n.lock.Lock() defer n.lock.Unlock() - return n.initialized + return n.namespaces != nil } // SetNamespaces replaces the set of namespaces. -func (n *namespaceSet) SetNamespaces(namespaces ...string) { +func (n *namespaceSet) SetNamespaces(namespaces []string) { n.lock.Lock() defer n.lock.Unlock() - if !n.initialized { - n.initialized = true + var newNamespaceSet sets.Set + if namespaces == nil { + newNamespaceSet = nil + } else { + newNamespaceSet = sets.NewSet(namespaces...) } - newNamespaceSet := sets.NewSet(namespaces...) - // If the set of namespaces, includes metav1.NamespaceAll, then it // only makes sense to track that. if newNamespaceSet.Contains(metav1.NamespaceAll) { diff --git a/pkg/informers/namespace_set_test.go b/pkg/informers/namespace_set_test.go index f1545c4d..28366aed 100644 --- a/pkg/informers/namespace_set_test.go +++ b/pkg/informers/namespace_set_test.go @@ -22,17 +22,17 @@ func TestNamespaceSet(t *testing.T) { name: "initially empty", namespaceSet: xnsinformers.NewNamespaceSet(), testFunc: func(ns xnsinformers.NamespaceSet) { - ns.SetNamespaces("ns-one", "ns-two") + ns.SetNamespaces([]string{"ns-one", "ns-two"}) }, expectedAdds: []string{"ns-one", "ns-two"}, expectedRemoves: nil, }, { name: "initially populated", - namespaceSet: newNamespaceSet("ns-one"), + namespaceSet: xnsinformers.NewNamespaceSet("ns-one"), testFunc: func(ns xnsinformers.NamespaceSet) { - ns.SetNamespaces("ns-one", "ns-two", "ns-three") - ns.SetNamespaces("new-ns") + ns.SetNamespaces([]string{"ns-one", "ns-two", "ns-three"}) + ns.SetNamespaces([]string{"new-ns"}) }, expectedAdds: []string{"ns-one", "ns-two", "ns-three", "new-ns"}, expectedRemoves: []string{"ns-one", "ns-two", "ns-three"}, @@ -42,11 +42,20 @@ func TestNamespaceSet(t *testing.T) { namespaceSet: xnsinformers.NewNamespaceSet(), testFunc: func(ns xnsinformers.NamespaceSet) { // Adding metav1.NamespaceAll means all others are ignored. - ns.SetNamespaces(metav1.NamespaceAll, "ns-ignored") + ns.SetNamespaces([]string{metav1.NamespaceAll, "ns-ignored"}) }, expectedAdds: []string{metav1.NamespaceAll}, expectedRemoves: nil, }, + { + name: "uninitialized later", + namespaceSet: xnsinformers.NewNamespaceSet("ns-one"), + testFunc: func(ns xnsinformers.NamespaceSet) { + ns.SetNamespaces(nil) + }, + expectedAdds: []string{"ns-one"}, + expectedRemoves: []string{"ns-one"}, + }, } for _, tc := range testCases { @@ -86,14 +95,19 @@ func TestNamespaceSetInitialized(t *testing.T) { t.Errorf("didn't expect new NamespaceSet to be initialized") } - set.SetNamespaces("foo") + set.SetNamespaces([]string{"foo"}) if !set.Initialized() { t.Errorf("expected NamespaceSet to be initialized after invoking SetNamespaces()") } - set.SetNamespaces( /* no namespaces */ ) + set.SetNamespaces([]string{}) if !set.Initialized() { - t.Errorf("expected NamespaceSet to still be initialized after invoking SetNamespaces() with no namespaces") + t.Errorf("expected NamespaceSet to still be initialized after invoking SetNamespaces() with empty namespace slice") + } + + set.SetNamespaces(nil) + if set.Initialized() { + t.Errorf("didn't expect NamespaceSet to still be initialized after invoking SetNamespaces() with nil namespace slice") } } @@ -108,9 +122,14 @@ func TestNamespaceSetList(t *testing.T) { namespaceSet: xnsinformers.NewNamespaceSet(), expectedList: []string{}, }, + { + name: "uninitialized", + namespaceSet: xnsinformers.NewUninitializedNamespaceSet(), + expectedList: []string{}, + }, { name: "populated", - namespaceSet: newNamespaceSet("c", "a", "b"), + namespaceSet: xnsinformers.NewNamespaceSet("c", "a", "b"), expectedList: []string{"a", "b", "c"}, // Should be sorted. }, } @@ -133,15 +152,27 @@ func TestNamespaceSetContains(t *testing.T) { search string expected bool }{ + { + name: "empty", + namespaceSet: xnsinformers.NewNamespaceSet(), + search: "a", + expected: false, + }, + { + name: "uninitialized", + namespaceSet: xnsinformers.NewUninitializedNamespaceSet(), + search: "a", + expected: false, + }, { name: "found", - namespaceSet: newNamespaceSet("c", "a", "b"), + namespaceSet: xnsinformers.NewNamespaceSet("c", "a", "b"), search: "b", expected: true, }, { name: "not found", - namespaceSet: newNamespaceSet("e", "f", "g"), + namespaceSet: xnsinformers.NewNamespaceSet("e", "f", "g"), search: "z", expected: false, }, @@ -157,9 +188,3 @@ func TestNamespaceSetContains(t *testing.T) { }) } } - -func newNamespaceSet(namespaces ...string) xnsinformers.NamespaceSet { - set := xnsinformers.NewNamespaceSet() - set.SetNamespaces(namespaces...) - return set -}