Merge cluster class namespace and name index into one

Signed-off-by: Danil-Grigorev <danil.grigorev@suse.com>
kubernetes-sigs · Jan 13, 2025 · c49f071 · c49f071
1 parent 54e0a4d
commit c49f071
Show file tree

Hide file tree

Showing 7 changed files with 33 additions and 43 deletions.
diff --git a/api/v1beta1/index/cluster.go b/api/v1beta1/index/cluster.go
@@ -28,10 +28,10 @@ import (
 )
 
 const (
-	// ClusterClassNameField is used by the Cluster controller to index Clusters by ClusterClass name.
+	// ClusterClassNameField is used by the Cluster controller to index Clusters by ClusterClass name and namespace.
 	ClusterClassNameField = "spec.topology.class"
-	// ClusterClassNamespaceField is used by the Cluster controller to index Clusters by ClusterClass namespace.
-	ClusterClassNamespaceField = "spec.topology.classNamespace"
+	// ClusterClassFmt is used to correctly format Class index
+	ClusterClassFmt = "%s/%s"
 )
 
 // ByClusterClassName adds the cluster class name  index to the
@@ -46,38 +46,20 @@ func ByClusterClassName(ctx context.Context, mgr ctrl.Manager) error {
 	return nil
 }
 
-// ByClusterClassNamespace adds the cluster class namespace index to the
-// managers cache.
-func ByClusterClassNamespace(ctx context.Context, mgr ctrl.Manager) error {
-	if err := mgr.GetCache().IndexField(ctx, &clusterv1.Cluster{},
-		ClusterClassNamespaceField,
-		ClusterByClusterClassNamespace,
-	); err != nil {
-		return errors.Wrap(err, "error setting index field")
-	}
-	return nil
-}
-
 // ClusterByClusterClassClassName contains the logic to index Clusters by ClusterClass name.
 func ClusterByClusterClassClassName(o client.Object) []string {
 	cluster, ok := o.(*clusterv1.Cluster)
 	if !ok {
 		panic(fmt.Sprintf("Expected Cluster but got a %T", o))
 	}
 	if cluster.Spec.Topology != nil {
-		return []string{cluster.GetClassKey().Name}
+		key := cluster.GetClassKey()
+		return []string{fmt.Sprintf(ClusterClassFmt, key.Name, key.Namespace)}
 	}
 	return nil
 }
 
-// ClusterByClusterClassNamespace contains the logic to index Clusters by ClusterClass namespace.
-func ClusterByClusterClassNamespace(o client.Object) []string {
-	cluster, ok := o.(*clusterv1.Cluster)
-	if !ok {
-		panic(fmt.Sprintf("Expected Cluster but got a %T", o))
-	}
-	if cluster.Spec.Topology != nil {
-		return []string{cluster.GetClassKey().Namespace}
-	}
-	return nil
+// ClusterClassKey returns ClusterClass index key to be used for search
+func ClusterClassKey(o client.Object) string {
+	return fmt.Sprintf(ClusterClassFmt, o.GetName(), o.GetNamespace())
 }
diff --git a/api/v1beta1/index/cluster_test.go b/api/v1beta1/index/cluster_test.go
@@ -20,6 +20,7 @@ import (
 	"testing"
 
 	. "github.com/onsi/gomega"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 
 	clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
@@ -39,13 +40,33 @@ func TestClusterByClassName(t *testing.T) {
 		{
 			name: "when cluster has a valid Topology",
 			object: &clusterv1.Cluster{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "cluster",
+					Namespace: "default",
+				},
 				Spec: clusterv1.ClusterSpec{
 					Topology: &clusterv1.Topology{
 						Class: "class1",
 					},
 				},
 			},
-			expected: []string{"class1"},
+			expected: []string{"class1/default"},
+		},
+		{
+			name: "when cluster has a valid Topology with namespace specified",
+			object: &clusterv1.Cluster{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "cluster",
+					Namespace: "default",
+				},
+				Spec: clusterv1.ClusterSpec{
+					Topology: &clusterv1.Topology{
+						Class:          "class1",
+						ClassNamespace: "other",
+					},
+				},
+			},
+			expected: []string{"class1/other"},
 		},
 	}
 

diff --git a/api/v1beta1/index/index.go b/api/v1beta1/index/index.go
@@ -39,10 +39,6 @@ func AddDefaultIndexes(ctx context.Context, mgr ctrl.Manager) error {
 		if err := ByClusterClassName(ctx, mgr); err != nil {
 			return err
 		}
-
-		if err := ByClusterClassNamespace(ctx, mgr); err != nil {
-			return err
-		}
 	}
 
 	if feature.Gates.Enabled(feature.MachinePool) {

diff --git a/internal/controllers/topology/cluster/cluster_controller.go b/internal/controllers/topology/cluster/cluster_controller.go
@@ -493,8 +493,7 @@ func (r *Reconciler) clusterClassToCluster(ctx context.Context, o client.Object)
 		ctx,
 		clusterList,
 		client.MatchingFields{
-			index.ClusterClassNameField:      clusterClass.Name,
-			index.ClusterClassNamespaceField: clusterClass.Namespace,
+			index.ClusterClassNameField: index.ClusterClassKey(clusterClass),
 		},
 	); err != nil {
 		return nil

diff --git a/internal/controllers/topology/cluster/suite_test.go b/internal/controllers/topology/cluster/suite_test.go
@@ -60,14 +60,11 @@ func TestMain(m *testing.M) {
 		if err := index.AddDefaultIndexes(ctx, mgr); err != nil {
 			panic(fmt.Sprintf("unable to setup index: %v", err))
 		}
-		// Set up the ClusterClassName and ClusterClassNamespace index explicitly here. This index is ordinarily created in
+		// Set up the ClusterClassName index explicitly here. This index is ordinarily created in
 		// index.AddDefaultIndexes. That doesn't happen here because the ClusterClass feature flag is not set.
 		if err := index.ByClusterClassName(ctx, mgr); err != nil {
 			panic(fmt.Sprintf("unable to setup index: %v", err))
 		}
-		if err := index.ByClusterClassNamespace(ctx, mgr); err != nil {
-			panic(fmt.Sprintf("unable to setup index: %v", err))
-		}
 	}
 	setupReconcilers := func(ctx context.Context, mgr ctrl.Manager) {
 		clusterCache, err := clustercache.SetupWithManager(ctx, mgr, clustercache.Options{

diff --git a/internal/webhooks/clusterclass.go b/internal/webhooks/clusterclass.go
@@ -380,8 +380,7 @@ func (webhook *ClusterClass) getClustersUsingClusterClass(ctx context.Context, c
 	clusters := &clusterv1.ClusterList{}
 	err := webhook.Client.List(ctx, clusters,
 		client.MatchingFields{
-			index.ClusterClassNameField:      clusterClass.Name,
-			index.ClusterClassNamespaceField: clusterClass.Namespace,
+			index.ClusterClassNameField: index.ClusterClassKey(clusterClass),
 		},
 	)
 	if err != nil {

diff --git a/internal/webhooks/clusterclass_test.go b/internal/webhooks/clusterclass_test.go
@@ -92,7 +92,6 @@ func TestClusterClassDefaultNamespaces(t *testing.T) {
 	fakeClient := fake.NewClientBuilder().
 		WithScheme(fakeScheme).
 		WithIndex(&clusterv1.Cluster{}, index.ClusterClassNameField, index.ClusterByClusterClassClassName).
-		WithIndex(&clusterv1.Cluster{}, index.ClusterClassNamespaceField, index.ClusterByClusterClassNamespace).
 		Build()
 
 	// Create the webhook and add the fakeClient as its client.
@@ -1867,7 +1866,6 @@ func TestClusterClassValidation(t *testing.T) {
 			fakeClient := fake.NewClientBuilder().
 				WithScheme(fakeScheme).
 				WithIndex(&clusterv1.Cluster{}, index.ClusterClassNameField, index.ClusterByClusterClassClassName).
-				WithIndex(&clusterv1.Cluster{}, index.ClusterClassNamespaceField, index.ClusterByClusterClassNamespace).
 				Build()
 
 			// Pin the compatibility version used in variable CEL validation to 1.29, so we don't have to continuously refactor
@@ -2515,7 +2513,6 @@ func TestClusterClassValidationWithClusterAwareChecks(t *testing.T) {
 				WithScheme(fakeScheme).
 				WithObjects(tt.clusters...).
 				WithIndex(&clusterv1.Cluster{}, index.ClusterClassNameField, index.ClusterByClusterClassClassName).
-				WithIndex(&clusterv1.Cluster{}, index.ClusterClassNamespaceField, index.ClusterByClusterClassNamespace).
 				Build()
 
 			// Create the webhook and add the fakeClient as its client.
@@ -2569,7 +2566,6 @@ func TestGetClustersUsingClusterClass(t *testing.T) {
 				WithScheme(fakeScheme).
 				WithObjects(tt.clusters...).
 				WithIndex(&clusterv1.Cluster{}, index.ClusterClassNameField, index.ClusterByClusterClassClassName).
-				WithIndex(&clusterv1.Cluster{}, index.ClusterClassNamespaceField, index.ClusterByClusterClassNamespace).
 				Build()
 
 			// Create the webhook and add the fakeClient as its client.