Merge pull request #1765 from ncdc/0.7/fix-vw-missing-index

[release-0.7] Start centralizing APIExport indexers

pkg/indexers/apiexport.go

@@ -0,0 +1,69 @@
+/*
+Copyright 2022 The KCP 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 indexers
+
+import (
+	"fmt"
+
+	"github.com/kcp-dev/logicalcluster/v2"
+
+	"k8s.io/client-go/tools/clusters"
+
+	apisv1alpha1 "github.com/kcp-dev/kcp/pkg/apis/apis/v1alpha1"
+)
+
+const (
+	// APIExportByIdentity is the indexer name for retrieving APIExports by identity hash.
+	APIExportByIdentity = "APIExportByIdentity"
+	// APIExportBySecret is the indexer name for retrieving APIExports by
+	APIExportBySecret = "APIExportSecret"
+)
+
+// IndexAPIExportByIdentity is an index function that indexes an APIExport by its identity hash.
+func IndexAPIExportByIdentity(obj interface{}) ([]string, error) {
+	apiExport, ok := obj.(*apisv1alpha1.APIExport)
+	if !ok {
+		return []string{}, fmt.Errorf("obj %T is not an APIExport", obj)
+	}
+
+	return []string{apiExport.Status.IdentityHash}, nil
+}
+
+// IndexAPIExportBySecret is an index function that indexes an APIExport by its identity secret references. Index values
+// are of the form <secret reference namespace>/<cluster name><separator><secret reference name> (cache keys).
+func IndexAPIExportBySecret(obj interface{}) ([]string, error) {
+	apiExport, ok := obj.(*apisv1alpha1.APIExport)
+	if !ok {
+		return []string{}, fmt.Errorf("obj %T is not an APIExport", obj)
+	}
+
+	if apiExport.Spec.Identity == nil {
+		return []string{}, nil
+	}
+
+	ref := apiExport.Spec.Identity.SecretRef
+	if ref == nil {
+		return []string{}, nil
+	}
+
+	if ref.Namespace == "" || ref.Name == "" {
+		return []string{}, nil
+	}
+
+	// TODO(ncdc): use future shared key func if we ever create one
+	return []string{ref.Namespace + "/" + clusters.ToClusterAwareKey(logicalcluster.From(apiExport), ref.Name)}, nil
+}

pkg/indexers/indexers.go

@@ -35,8 +35,6 @@ const (
 	ByLogicalCluster = "kcp-global-byLogicalCluster"
 	// ByLogicalClusterAndNamespace is the name for the index that indexes by an object's logical cluster and namespace.
 	ByLogicalClusterAndNamespace = "kcp-global-byLogicalClusterAndNamespace"
-	// IndexAPIExportByIdentity is the indexer name for by identity index for the API Export indexers.
-	IndexAPIExportByIdentity = "byIdentity"
 	// BySyncerFinalizerKey is the name for the index that indexes by syncer finalizer label keys.
 	BySyncerFinalizerKey = "bySyncerFinalizerKey"
 )

pkg/indexers/util.go

@@ -43,3 +43,18 @@ func AppendOrDie(indexers ...cache.Indexers) cache.Indexers {
 	}
 	return ret
 }
+
+// AddIfNotPresentOrDie tries to add everything from toAdd to indexer's indexers that does not already exist. It panics
+// if it encounters an error.
+func AddIfNotPresentOrDie(indexer cache.Indexer, toAdd cache.Indexers) {
+	existing := indexer.GetIndexers()
+	for indexName := range toAdd {
+		if _, exists := existing[indexName]; exists {
+			delete(toAdd, indexName)
+		}
+	}
+
+	if err := indexer.AddIndexers(toAdd); err != nil {
+		panic(fmt.Errorf("error adding indexers: %w", err))
+	}
+}

pkg/reconciler/apis/apiexport/apiexport_controller.go

@@ -56,8 +56,6 @@ const (
 	controllerName = "kcp-apiexport"
 
 	DefaultIdentitySecretNamespace = "kcp-system"
-
-	indexAPIExportBySecret = "bySecret"
 )
 
 // NewController returns a new controller for APIExports.
@@ -97,35 +95,13 @@ func NewController(
 
 	c.getSecret = c.readThroughGetSecret
 
-	if err := apiExportInformer.Informer().AddIndexers(
+	indexers.AddIfNotPresentOrDie(
+		apiExportInformer.Informer().GetIndexer(),
 		cache.Indexers{
-			indexers.IndexAPIExportByIdentity: func(obj interface{}) ([]string, error) {
-				apiExport := obj.(*apisv1alpha1.APIExport)
-				return []string{apiExport.Status.IdentityHash}, nil
-			},
-			indexAPIExportBySecret: func(obj interface{}) ([]string, error) {
-				apiExport := obj.(*apisv1alpha1.APIExport)
-
-				if apiExport.Spec.Identity == nil {
-					return []string{}, nil
-				}
-
-				ref := apiExport.Spec.Identity.SecretRef
-				if ref == nil {
-					return []string{}, nil
-				}
-
-				if ref.Namespace == "" || ref.Name == "" {
-					return []string{}, nil
-				}
-
-				// TODO(ncdc): use future shared key func if we ever create one
-				return []string{ref.Namespace + "/" + clusters.ToClusterAwareKey(logicalcluster.From(apiExport), ref.Name)}, nil
-			},
+			indexers.APIExportByIdentity: indexers.IndexAPIExportByIdentity,
+			indexers.APIExportBySecret:   indexers.IndexAPIExportBySecret,
 		},
-	); err != nil {
-		return nil, err
-	}
+	)
 
 	apiExportInformer.Informer().AddEventHandler(cache.ResourceEventHandlerFuncs{
 		AddFunc: func(obj interface{}) {
@@ -238,7 +214,7 @@ func (c *controller) enqueueSecret(obj interface{}) {
 		return
 	}
 
-	apiExportKeys, err := c.apiExportIndexer.IndexKeys(indexAPIExportBySecret, secretKey)
+	apiExportKeys, err := c.apiExportIndexer.IndexKeys(indexers.APIExportBySecret, secretKey)
 	if err != nil {
 		runtime.HandleError(err)
 		return

pkg/virtual/apiexport/controllers/apireconciler/apiexport_apireconciler_controller.go

@@ -35,6 +35,7 @@ import (
 	kcpclient "github.com/kcp-dev/kcp/pkg/client/clientset/versioned"
 	apisinformer "github.com/kcp-dev/kcp/pkg/client/informers/externalversions/apis/v1alpha1"
 	apislisters "github.com/kcp-dev/kcp/pkg/client/listers/apis/v1alpha1"
+	"github.com/kcp-dev/kcp/pkg/indexers"
 	"github.com/kcp-dev/kcp/pkg/virtual/framework/dynamic/apidefinition"
 	dynamiccontext "github.com/kcp-dev/kcp/pkg/virtual/framework/dynamic/context"
 )
@@ -72,11 +73,13 @@ func NewAPIReconciler(
 		apiSets: map[dynamiccontext.APIDomainKey]apidefinition.APIDefinitionSet{},
 	}
 
-	if err := apiExportInformer.Informer().AddIndexers(cache.Indexers{
-		byWorkspace: indexByWorkspace,
-	}); err != nil {
-		return nil, err
-	}
+	indexers.AddIfNotPresentOrDie(
+		apiExportInformer.Informer().GetIndexer(),
+		cache.Indexers{
+			byWorkspace:                  indexByWorkspace,
+			indexers.APIExportByIdentity: indexers.IndexAPIExportByIdentity,
+		},
+	)
 
 	apiExportInformer.Informer().AddEventHandler(cache.ResourceEventHandlerFuncs{
 		AddFunc: func(obj interface{}) {

pkg/virtual/apiexport/controllers/apireconciler/apiexport_apireconciler_reconcile.go

@@ -116,7 +116,7 @@ func (c *APIReconciler) reconcile(ctx context.Context, apiExport *apisv1alpha1.A
 			continue
 		}
 
-		exports, err := c.apiExportIndexer.ByIndex(indexers.IndexAPIExportByIdentity, pc.IdentityHash)
+		exports, err := c.apiExportIndexer.ByIndex(indexers.APIExportByIdentity, pc.IdentityHash)
 		if err != nil {
 			return err
 		}