OSSM-2006 Fix HasSynced() [maistra-2.1]

Makefile

@@ -3,7 +3,7 @@ build:
 
 test:
 	go vet ./...
-	go test -v ./...
+	go test -race -v ./...
 
 gen: build
 	./hack/update-codegen.sh

cmd/xns-informer-gen/generators/factory.go

@@ -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}}(v1.NamespaceAll),
+        namespaces:       {{.xnsNewNamespaceSet|raw}}(),
 		defaultResync:    defaultResync,
 		informers:        make(map[{{.reflectType|raw}}]{{.cacheSharedIndexInformer|raw}}),
 		startedInformers: make(map[{{.reflectType|raw}}]bool),

hack/update-codegen.sh

@@ -96,8 +96,7 @@ service_apis_group_versions=(
   --listers-package "sigs.k8s.io/service-apis/pkg/client/listers" \
   --go-header-file "${PROJ_ROOT}/hack/boilerplate.go.txt"
 
-rsync -a --remove-source-files --delete \
-	  "${PROJ_ROOT}/out/github.com/maistra/xns-informer/pkg/generated/" \
-	  "${PROJ_ROOT}/pkg/generated"
+rm -r "${PROJ_ROOT}/pkg/generated"
+mv "${PROJ_ROOT}/out/github.com/maistra/xns-informer/pkg/generated/" "${PROJ_ROOT}/pkg/generated"
 
 rm -rd "${PROJ_ROOT}/out/github.com"

pkg/informers/dynamic.go

@@ -29,7 +29,7 @@ type DynamicSharedInformerFactory interface {
 
 // NewDynamicSharedInformerFactory constructs a new instance of dynamicSharedInformerFactory for all namespaces.
 func NewDynamicSharedInformerFactory(client dynamic.Interface, defaultResync time.Duration) DynamicSharedInformerFactory {
-	namespaces := NewNamespaceSet(metav1.NamespaceAll)
+	namespaces := NewNamespaceSet()
 	return NewFilteredDynamicSharedInformerFactory(client, defaultResync, namespaces, nil)
 }
 

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: xnsinformers.NewNamespaceSet("ns-bar"),
+			informNS: 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: xnsinformers.NewNamespaceSet("ns-foo"),
+			informNS: newNamespaceSet("ns-foo"),
 			ns:       "ns-foo",
 			gvr:      schema.GroupVersionResource{Group: "apps", Version: "v1", Resource: "deployments"},
 			trigger:  triggerFactory(t),
@@ -119,6 +119,7 @@ func TestFilteredDynamicSharedInformerFactory(t *testing.T) {
 }
 
 func TestDynamicSharedInformerFactory(t *testing.T) {
+	ns := "ns-foo"
 	scenarios := []struct {
 		name        string
 		existingObj *unstructured.Unstructured
@@ -130,10 +131,10 @@ func TestDynamicSharedInformerFactory(t *testing.T) {
 		// scenario 1
 		{
 			name: "scenario 1: test if adding an object triggers AddFunc",
-			ns:   "ns-foo",
+			ns:   ns,
 			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-foo", "name-foo")
+				testObject := newUnstructured("extensions/v1beta1", "Deployment", ns, "name-foo")
 				createdObj, err := fakeClient.Resource(gvr).Namespace(ns).Create(context.TODO(), testObject, metav1.CreateOptions{})
 				if err != nil {
 					t.Error(err)
@@ -152,9 +153,9 @@ func TestDynamicSharedInformerFactory(t *testing.T) {
 		// scenario 2
 		{
 			name:        "scenario 2: tests if updating an object triggers UpdateFunc",
-			ns:          "ns-foo",
+			ns:          ns,
 			gvr:         schema.GroupVersionResource{Group: "extensions", Version: "v1beta1", Resource: "deployments"},
-			existingObj: newUnstructured("extensions/v1beta1", "Deployment", "ns-foo", "name-foo"),
+			existingObj: newUnstructured("extensions/v1beta1", "Deployment", ns, "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{})
@@ -175,9 +176,9 @@ func TestDynamicSharedInformerFactory(t *testing.T) {
 		// scenario 3
 		{
 			name:        "scenario 3: test if deleting an object triggers DeleteFunc",
-			ns:          "ns-foo",
+			ns:          ns,
 			gvr:         schema.GroupVersionResource{Group: "extensions", Version: "v1beta1", Resource: "deployments"},
-			existingObj: newUnstructured("extensions/v1beta1", "Deployment", "ns-foo", "name-foo"),
+			existingObj: newUnstructured("extensions/v1beta1", "Deployment", ns, "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 {
@@ -202,17 +203,17 @@ func TestDynamicSharedInformerFactory(t *testing.T) {
 			ctx, cancel := context.WithTimeout(context.Background(), timeout)
 			defer cancel()
 			scheme := runtime.NewScheme()
-			err := extensionsv1beta1.AddToScheme(scheme)
-			if err != nil {
-				t.Fatalf("couldn't add appsv1 to scheme: %v", err)
-			}
 			informerReceiveObjectCh := make(chan *unstructured.Unstructured, 1)
 			objs := []runtime.Object{}
 			if ts.existingObj != nil {
 				objs = append(objs, ts.existingObj)
 			}
-			fakeClient := fake.NewSimpleDynamicClient(scheme, objs...)
+			gvrToListKind := map[schema.GroupVersionResource]string{
+				extensionsv1beta1.SchemeGroupVersion.WithResource("deployments"): "DeploymentList",
+			}
+			fakeClient := fake.NewSimpleDynamicClientWithCustomListKinds(scheme, gvrToListKind, objs...)
 			target := xnsinformers.NewDynamicSharedInformerFactory(fakeClient, 0)
+			target.SetNamespaces(ns)
 
 			// act
 			informerListerForGvr := target.ForResource(ts.gvr)

pkg/informers/informer.go

@@ -265,6 +265,10 @@ func (i *multiNamespaceInformer) HasSynced() bool {
 	i.lock.Lock()
 	defer i.lock.Unlock()
 
+	if !i.namespaces.Initialized() {
+		return false
+	}
+
 	for _, informer := range i.informers {
 		if synced := informer.HasSynced(); !synced {
 			return false

pkg/informers/informer_test.go

@@ -170,7 +170,9 @@ func TestSharedInformerWatchDisruption(t *testing.T) {
 	}
 
 	for _, listener := range listeners {
+		listener.lock.Lock()
 		listener.receivedItemNames = []string{}
+		listener.lock.Unlock()
 	}
 
 	listenerNoResync.expectedItemNames = sets.NewString("pod2", "pod3")
@@ -245,13 +247,14 @@ func TestMultiNamespaceInformerEventHandlers(t *testing.T) {
 	cm1 := internaltesting.NewConfigMap(namespaces[0], "cm1", nil)
 	cm2 := internaltesting.NewConfigMap(namespaces[1], "cm2", nil)
 
+	lock := sync.RWMutex{}
 	addFuncCalled := false
 	updateFuncCalled := false
 	deleteFuncCalled := false
 
 	// These tests use the fake client instead of a FakeControllerSource.
 	client := kubefake.NewSimpleClientset()
-	namespaceSet := xnsinformers.NewNamespaceSet(namespaces...)
+	namespaceSet := newNamespaceSet(namespaces...)
 
 	informer := xnsinformers.NewMultiNamespaceInformer(namespaceSet, 0, func(namespace string) cache.SharedIndexInformer {
 		return cache.NewSharedIndexInformer(
@@ -271,13 +274,19 @@ func TestMultiNamespaceInformerEventHandlers(t *testing.T) {
 
 	informer.AddEventHandler(cache.ResourceEventHandlerFuncs{
 		AddFunc: func(_ interface{}) {
+			lock.Lock()
 			addFuncCalled = true
+			lock.Unlock()
 		},
 		UpdateFunc: func(_, _ interface{}) {
+			lock.Lock()
 			updateFuncCalled = true
+			lock.Unlock()
 		},
 		DeleteFunc: func(_ interface{}) {
+			lock.Lock()
 			deleteFuncCalled = true
+			lock.Unlock()
 		},
 	})
 
@@ -310,6 +319,8 @@ func TestMultiNamespaceInformerEventHandlers(t *testing.T) {
 
 	// Wait for all handler functions to be called.
 	err = wait.PollImmediate(100*time.Millisecond, 1*time.Minute, func() (bool, error) {
+		lock.RLock()
+		defer lock.RUnlock()
 		return addFuncCalled && updateFuncCalled && deleteFuncCalled, nil
 	})
 
@@ -330,3 +341,74 @@ func TestMultiNamespaceInformerEventHandlers(t *testing.T) {
 		t.Fatalf("Delete handler not called after namespace removal: %v", err)
 	}
 }
+
+func TestMultiNamespaceInformerHasSynced(t *testing.T) {
+	namespaceSet := xnsinformers.NewNamespaceSet()
+	hasSynced := false
+
+	informer := xnsinformers.NewMultiNamespaceInformer(namespaceSet, 0, func(namespace string) cache.SharedIndexInformer {
+		return mockInformer{
+			hasSynced: &hasSynced,
+		}
+	})
+
+	if informer.HasSynced() {
+		t.Fatalf("informer is synced, but shouldn't be because namespaces haven't been set yet")
+	}
+
+	namespaceSet.SetNamespaces("ns1", "ns2")
+
+	if informer.HasSynced() {
+		t.Fatalf("informer is synced, but shouldn't be because the underlying informers aren't synced")
+	}
+
+	hasSynced = true
+
+	if !informer.HasSynced() {
+		t.Fatalf("expected informer to be synced")
+	}
+}
+
+type mockInformer struct {
+	hasSynced *bool
+}
+
+func (m mockInformer) AddEventHandler(handler cache.ResourceEventHandler) {
+	panic("not implemented")
+}
+
+func (m mockInformer) AddEventHandlerWithResyncPeriod(handler cache.ResourceEventHandler, resyncPeriod time.Duration) {
+	panic("not implemented")
+}
+
+func (m mockInformer) GetStore() cache.Store {
+	panic("not implemented")
+}
+
+func (m mockInformer) GetController() cache.Controller {
+	panic("not implemented")
+}
+
+func (m mockInformer) Run(stopCh <-chan struct{}) {
+	panic("not implemented")
+}
+
+func (m mockInformer) HasSynced() bool {
+	return *m.hasSynced
+}
+
+func (m mockInformer) LastSyncResourceVersion() string {
+	panic("not implemented")
+}
+
+func (m mockInformer) SetWatchErrorHandler(handler cache.WatchErrorHandler) error {
+	panic("not implemented")
+}
+
+func (m mockInformer) AddIndexers(indexers cache.Indexers) error {
+	panic("not implemented")
+}
+
+func (m mockInformer) GetIndexer() cache.Indexer {
+	panic("not implemented")
+}

pkg/informers/metadata.go

@@ -25,7 +25,7 @@ type MetadataSharedInformerFactory interface {
 
 // NewMetadataSharedInformerFactory constructs a new instance of metadataSharedInformerFactory for all namespaces.
 func NewMetadataSharedInformerFactory(client metadata.Interface, defaultResync time.Duration) MetadataSharedInformerFactory {
-	namespaces := NewNamespaceSet(metav1.NamespaceAll)
+	namespaces := NewNamespaceSet()
 	return NewFilteredMetadataSharedInformerFactory(client, defaultResync, namespaces, nil)
 }
 

pkg/informers/metadata_test.go

@@ -27,6 +27,7 @@ import (
 // }
 
 func TestMetadataSharedInformerFactory(t *testing.T) {
+	ns := "ns-foo"
 	scenarios := []struct {
 		name        string
 		existingObj *metav1.PartialObjectMetadata
@@ -38,10 +39,10 @@ func TestMetadataSharedInformerFactory(t *testing.T) {
 		// scenario 1
 		{
 			name: "scenario 1: test if adding an object triggers AddFunc",
-			ns:   "ns-foo",
+			ns:   ns,
 			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-foo", "name-foo")
+				testObject := newPartialObjectMetadata("extensions/v1beta1", "Deployment", ns, "name-foo")
 				createdObj, err := fakeClient.Resource(gvr).Namespace(ns).(fake.MetadataClient).CreateFake(testObject, metav1.CreateOptions{})
 				if err != nil {
 					t.Error(err)
@@ -60,9 +61,9 @@ func TestMetadataSharedInformerFactory(t *testing.T) {
 		// scenario 2
 		{
 			name:        "scenario 2: tests if updating an object triggers UpdateFunc",
-			ns:          "ns-foo",
+			ns:          ns,
 			gvr:         schema.GroupVersionResource{Group: "extensions", Version: "v1beta1", Resource: "deployments"},
-			existingObj: newPartialObjectMetadata("extensions/v1beta1", "Deployment", "ns-foo", "name-foo"),
+			existingObj: newPartialObjectMetadata("extensions/v1beta1", "Deployment", ns, "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)
@@ -86,9 +87,9 @@ func TestMetadataSharedInformerFactory(t *testing.T) {
 		// scenario 3
 		{
 			name:        "scenario 3: test if deleting an object triggers DeleteFunc",
-			ns:          "ns-foo",
+			ns:          ns,
 			gvr:         schema.GroupVersionResource{Group: "extensions", Version: "v1beta1", Resource: "deployments"},
-			existingObj: newPartialObjectMetadata("extensions/v1beta1", "Deployment", "ns-foo", "name-foo"),
+			existingObj: newPartialObjectMetadata("extensions/v1beta1", "Deployment", ns, "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 {
@@ -121,6 +122,7 @@ func TestMetadataSharedInformerFactory(t *testing.T) {
 			}
 			fakeClient := fake.NewSimpleMetadataClient(scheme, objs...)
 			target := xnsinformers.NewMetadataSharedInformerFactory(fakeClient, 0)
+			target.SetNamespaces(ns)
 
 			// act
 			informerListerForGvr := target.ForResource(ts.gvr)