From 067fcc5486edee83d59d96cd0c19f92830ae0311 Mon Sep 17 00:00:00 2001 From: Danil-Grigorev Date: Mon, 13 Jan 2025 18:52:14 +0100 Subject: [PATCH] Merge cluster class namespace and name index into one Signed-off-by: Danil-Grigorev --- api/v1beta1/index/cluster.go | 34 +++++-------------- api/v1beta1/index/cluster_test.go | 23 ++++++++++++- api/v1beta1/index/index.go | 4 --- .../topology/cluster/cluster_controller.go | 3 +- .../topology/cluster/suite_test.go | 5 +-- internal/webhooks/clusterclass.go | 3 +- internal/webhooks/clusterclass_test.go | 4 --- 7 files changed, 33 insertions(+), 43 deletions(-) diff --git a/api/v1beta1/index/cluster.go b/api/v1beta1/index/cluster.go index 09fa67cb2ec5..305d4b380a1b 100644 --- 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,18 +46,6 @@ 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) @@ -65,19 +53,13 @@ func ClusterByClusterClassClassName(o client.Object) []string { 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.Namespace, key.Name)} } 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.GetNamespace(), o.GetName()) } diff --git a/api/v1beta1/index/cluster_test.go b/api/v1beta1/index/cluster_test.go index 4205103c7cf5..7200e1195e02 100644 --- 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{"default/class1"}, + }, + { + 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{"other/class1"}, }, } diff --git a/api/v1beta1/index/index.go b/api/v1beta1/index/index.go index b5f0e7d9df55..c6a17bf175bc 100644 --- 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 index a3eebd539d92..81bf13ddd8b1 100644 --- 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 index 32a41031689e..715f4a52bd47 100644 --- 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 index ab0e842e0400..a8ca7e97c38a 100644 --- 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 index 6275a271da7b..857bbc928477 100644 --- 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.