From 7726885336063ce3e74df26e4e6f114643f0e5e9 Mon Sep 17 00:00:00 2001 From: killianmuldoon Date: Tue, 7 Dec 2021 10:45:51 +0000 Subject: [PATCH] use ClusterClass name index in ClusterClass webhook Signed-off-by: killianmuldoon --- .../clusterclass_controller_test.go | 3 - .../controllers/clusterclass/suite_test.go | 5 + .../topology/cluster/suite_test.go | 5 + internal/webhooks/clusterclass.go | 13 +- internal/webhooks/clusterclass_test.go | 230 ++---------------- internal/webhooks/test/clusterclass_test.go | 203 +++++++++++----- internal/webhooks/test/suite_test.go | 5 + 7 files changed, 176 insertions(+), 288 deletions(-) diff --git a/internal/controllers/clusterclass/clusterclass_controller_test.go b/internal/controllers/clusterclass/clusterclass_controller_test.go index 857ee4177403..4bcb0ef42238 100644 --- a/internal/controllers/clusterclass/clusterclass_controller_test.go +++ b/internal/controllers/clusterclass/clusterclass_controller_test.go @@ -25,17 +25,14 @@ import ( . "github.com/onsi/gomega" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - utilfeature "k8s.io/component-base/featuregate/testing" "sigs.k8s.io/controller-runtime/pkg/client" clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" - "sigs.k8s.io/cluster-api/feature" tlog "sigs.k8s.io/cluster-api/internal/log" "sigs.k8s.io/cluster-api/internal/test/builder" ) func TestClusterClassReconciler_reconcile(t *testing.T) { - defer utilfeature.SetFeatureGateDuringTest(t, feature.Gates, feature.ClusterTopology, true)() g := NewWithT(t) timeout := 30 * time.Second diff --git a/internal/controllers/clusterclass/suite_test.go b/internal/controllers/clusterclass/suite_test.go index e0407a9d03d1..484c0d3bb81c 100644 --- a/internal/controllers/clusterclass/suite_test.go +++ b/internal/controllers/clusterclass/suite_test.go @@ -30,12 +30,14 @@ import ( "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" clientgoscheme "k8s.io/client-go/kubernetes/scheme" + "k8s.io/component-base/featuregate" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller" clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" "sigs.k8s.io/cluster-api/api/v1beta1/index" + "sigs.k8s.io/cluster-api/feature" "sigs.k8s.io/cluster-api/internal/test/envtest" ) @@ -51,6 +53,9 @@ func init() { _ = apiextensionsv1.AddToScheme(fakeScheme) } func TestMain(m *testing.M) { + if err := feature.Gates.(featuregate.MutableFeatureGate).Set(fmt.Sprintf("%s=%v", feature.ClusterTopology, true)); err != nil { + panic(fmt.Sprintf("unable to set ClusterTopology feature gate: %v", err)) + } setupIndexes := func(ctx context.Context, mgr ctrl.Manager) { if err := index.AddDefaultIndexes(ctx, mgr); err != nil { panic(fmt.Sprintf("unable to setup index: %v", err)) diff --git a/internal/controllers/topology/cluster/suite_test.go b/internal/controllers/topology/cluster/suite_test.go index e2b463228c9c..53cfbdfa35c1 100644 --- a/internal/controllers/topology/cluster/suite_test.go +++ b/internal/controllers/topology/cluster/suite_test.go @@ -53,6 +53,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 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)) + } } setupReconcilers := func(ctx context.Context, mgr ctrl.Manager) { unstructuredCachingClient, err := client.NewDelegatingClient( diff --git a/internal/webhooks/clusterclass.go b/internal/webhooks/clusterclass.go index 4bc75f93fb29..8723d6e1ef88 100644 --- a/internal/webhooks/clusterclass.go +++ b/internal/webhooks/clusterclass.go @@ -32,6 +32,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/webhook" clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" + "sigs.k8s.io/cluster-api/api/v1beta1/index" "sigs.k8s.io/cluster-api/feature" "sigs.k8s.io/cluster-api/internal/topology/check" "sigs.k8s.io/cluster-api/internal/topology/variables" @@ -440,22 +441,14 @@ func (webhook *ClusterClass) classNamesFromWorkerClass(w clusterv1.WorkersClass) func (webhook *ClusterClass) getClustersUsingClusterClass(ctx context.Context, clusterClass *clusterv1.ClusterClass) ([]clusterv1.Cluster, error) { clusters := &clusterv1.ClusterList{} - clustersUsingClusterClass := []clusterv1.Cluster{} err := webhook.Client.List(ctx, clusters, - client.MatchingLabels{ - clusterv1.ClusterTopologyOwnedLabel: "", - }, + client.MatchingFields{index.ClusterClassNameField: clusterClass.Name}, client.InNamespace(clusterClass.Namespace), ) if err != nil { return nil, err } - for _, c := range clusters.Items { - if c.Spec.Topology.Class == clusterClass.Name { - clustersUsingClusterClass = append(clustersUsingClusterClass, c) - } - } - return clustersUsingClusterClass, nil + return clusters.Items, nil } func getClusterClassVariablesMapWithReverseIndex(clusterClassVariables []clusterv1.ClusterClassVariable) (map[string]*clusterv1.ClusterClassVariable, map[string]int) { diff --git a/internal/webhooks/clusterclass_test.go b/internal/webhooks/clusterclass_test.go index b9bb641af108..a78b7b81d96a 100644 --- a/internal/webhooks/clusterclass_test.go +++ b/internal/webhooks/clusterclass_test.go @@ -1148,7 +1148,7 @@ func TestClusterClassValidationWithClusterAwareChecks(t *testing.T) { expectErr bool }{ { - name: "error if a MachineDeploymentClass in use gets removed", + name: "pass if a MachineDeploymentClass not in use gets removed", clusters: []client.Object{ builder.Cluster(metav1.NamespaceDefault, "cluster1"). WithLabels(map[string]string{clusterv1.ClusterTopologyOwnedLabel: ""}). @@ -1190,53 +1190,19 @@ func TestClusterClassValidationWithClusterAwareChecks(t *testing.T) { builder.ControlPlaneTemplate(metav1.NamespaceDefault, "cp1"). Build()). WithWorkerMachineDeploymentClasses( - *builder.MachineDeploymentClass("aa"). + *builder.MachineDeploymentClass("bb"). WithInfrastructureTemplate( builder.InfrastructureMachineTemplate(metav1.NamespaceDefault, "infra1").Build()). WithBootstrapTemplate( builder.BootstrapTemplate(metav1.NamespaceDefault, "bootstrap1").Build()). Build()). Build(), - expectErr: true, + expectErr: false, }, { - name: "error if many MachineDeploymentClasses, used in multiple Clusters using the modified ClusterClass, are removed", + name: "error if a MachineDeploymentClass in use gets removed", clusters: []client.Object{ builder.Cluster(metav1.NamespaceDefault, "cluster1"). - WithLabels(map[string]string{clusterv1.ClusterTopologyOwnedLabel: ""}). - WithTopology( - builder.ClusterTopology(). - WithClass("class1"). - WithMachineDeployment( - builder.MachineDeploymentTopology("workers1"). - WithClass("bb"). - Build(), - ). - WithMachineDeployment( - builder.MachineDeploymentTopology("workers2"). - WithClass("aa"). - Build(), - ). - Build()). - Build(), - builder.Cluster(metav1.NamespaceDefault, "cluster2"). - WithLabels(map[string]string{clusterv1.ClusterTopologyOwnedLabel: ""}). - WithTopology( - builder.ClusterTopology(). - WithClass("class1"). - WithMachineDeployment( - builder.MachineDeploymentTopology("workers1"). - WithClass("aa"). - Build(), - ). - WithMachineDeployment( - builder.MachineDeploymentTopology("workers2"). - WithClass("aa"). - Build(), - ). - Build()). - Build(), - builder.Cluster(metav1.NamespaceDefault, "cluster3"). WithLabels(map[string]string{clusterv1.ClusterTopologyOwnedLabel: ""}). WithTopology( builder.ClusterTopology(). @@ -1276,7 +1242,7 @@ func TestClusterClassValidationWithClusterAwareChecks(t *testing.T) { builder.ControlPlaneTemplate(metav1.NamespaceDefault, "cp1"). Build()). WithWorkerMachineDeploymentClasses( - *builder.MachineDeploymentClass("bb"). + *builder.MachineDeploymentClass("aa"). WithInfrastructureTemplate( builder.InfrastructureMachineTemplate(metav1.NamespaceDefault, "infra1").Build()). WithBootstrapTemplate( @@ -1286,7 +1252,7 @@ func TestClusterClassValidationWithClusterAwareChecks(t *testing.T) { expectErr: true, }, { - name: "pass if a similar MachineDeploymentClass is deleted when it is only used in Clusters not belonging to the ClusterClass", + name: "error if many MachineDeploymentClasses, used in multiple Clusters using the modified ClusterClass, are removed", clusters: []client.Object{ builder.Cluster(metav1.NamespaceDefault, "cluster1"). WithLabels(map[string]string{clusterv1.ClusterTopologyOwnedLabel: ""}). @@ -1298,64 +1264,31 @@ func TestClusterClassValidationWithClusterAwareChecks(t *testing.T) { WithClass("bb"). Build(), ). + WithMachineDeployment( + builder.MachineDeploymentTopology("workers2"). + WithClass("aa"). + Build(), + ). Build()). Build(), builder.Cluster(metav1.NamespaceDefault, "cluster2"). WithLabels(map[string]string{clusterv1.ClusterTopologyOwnedLabel: ""}). WithTopology( builder.ClusterTopology(). - WithClass("class2"). + WithClass("class1"). WithMachineDeployment( builder.MachineDeploymentTopology("workers1"). - - // A MachineDeploymentClass with the same name is in ClusterClass "class1" but - // this cluster is based on ClusterClass "class2" and does not impact deletion. + WithClass("aa"). + Build(), + ). + WithMachineDeployment( + builder.MachineDeploymentTopology("workers2"). WithClass("aa"). Build(), ). Build()). Build(), - }, - oldClusterClass: builder.ClusterClass(metav1.NamespaceDefault, "class1"). - WithInfrastructureClusterTemplate( - builder.InfrastructureClusterTemplate(metav1.NamespaceDefault, "inf").Build()). - WithControlPlaneTemplate( - builder.ControlPlaneTemplate(metav1.NamespaceDefault, "cp1"). - Build()). - WithWorkerMachineDeploymentClasses( - *builder.MachineDeploymentClass("aa"). - WithInfrastructureTemplate( - builder.InfrastructureMachineTemplate(metav1.NamespaceDefault, "infra1").Build()). - WithBootstrapTemplate( - builder.BootstrapTemplate(metav1.NamespaceDefault, "bootstrap1").Build()). - Build(), - *builder.MachineDeploymentClass("bb"). - WithInfrastructureTemplate( - builder.InfrastructureMachineTemplate(metav1.NamespaceDefault, "infra1").Build()). - WithBootstrapTemplate( - builder.BootstrapTemplate(metav1.NamespaceDefault, "bootstrap1").Build()). - Build()). - Build(), - newClusterClass: builder.ClusterClass(metav1.NamespaceDefault, "class1"). - WithInfrastructureClusterTemplate( - builder.InfrastructureClusterTemplate(metav1.NamespaceDefault, "inf").Build()). - WithControlPlaneTemplate( - builder.ControlPlaneTemplate(metav1.NamespaceDefault, "cp1"). - Build()). - WithWorkerMachineDeploymentClasses( - *builder.MachineDeploymentClass("bb"). - WithInfrastructureTemplate( - builder.InfrastructureMachineTemplate(metav1.NamespaceDefault, "infra1").Build()). - WithBootstrapTemplate( - builder.BootstrapTemplate(metav1.NamespaceDefault, "bootstrap1").Build()). - Build()). - Build(), - expectErr: false, - }, - { - name: "pass if a MachineDeploymentClass not in use gets removed", - clusters: []client.Object{ - builder.Cluster(metav1.NamespaceDefault, "cluster1"). + builder.Cluster(metav1.NamespaceDefault, "cluster3"). WithLabels(map[string]string{clusterv1.ClusterTopologyOwnedLabel: ""}). WithTopology( builder.ClusterTopology(). @@ -1402,7 +1335,7 @@ func TestClusterClassValidationWithClusterAwareChecks(t *testing.T) { builder.BootstrapTemplate(metav1.NamespaceDefault, "bootstrap1").Build()). Build()). Build(), - expectErr: false, + expectErr: true, }, } @@ -2450,128 +2383,3 @@ func TestClusterClassValidationWithVariableChecks(t *testing.T) { }) } } - -func TestClusterClass_ValidateDelete(t *testing.T) { - class := builder.ClusterClass(metav1.NamespaceDefault, "class1").Build() - - tests := []struct { - name string - clusters []client.Object - expectErr bool - }{ - { - name: "allow deletion if a cluster exists but does not reference the ClusterClass for deletion", - clusters: []client.Object{ - builder.Cluster(metav1.NamespaceDefault, "cluster1"). - WithLabels(map[string]string{clusterv1.ClusterTopologyOwnedLabel: ""}). - WithTopology( - builder.ClusterTopology(). - WithClass("class2"). - Build()). - Build(), - }, - expectErr: false, - }, - { - name: "error if cluster exists with a reference to the ClusterClass for deletion", - clusters: []client.Object{ - builder.Cluster(metav1.NamespaceDefault, "cluster1"). - WithLabels(map[string]string{clusterv1.ClusterTopologyOwnedLabel: ""}). - WithTopology( - builder.ClusterTopology(). - WithClass("class1"). - Build()). - Build(), - }, - expectErr: true, - }, - { - name: "error if multiple clusters exist and at least one references to the ClusterClass for deletion", - clusters: []client.Object{ - builder.Cluster(metav1.NamespaceDefault, "cluster1"). - WithLabels(map[string]string{clusterv1.ClusterTopologyOwnedLabel: ""}). - WithTopology( - builder.ClusterTopology(). - WithClass("class1"). - Build()). - Build(), - builder.Cluster(metav1.NamespaceDefault, "cluster2"). - WithLabels(map[string]string{clusterv1.ClusterTopologyOwnedLabel: ""}). - WithTopology( - builder.ClusterTopology(). - WithClass("class2"). - Build()). - Build(), - builder.Cluster(metav1.NamespaceDefault, "cluster3"). - WithLabels(map[string]string{clusterv1.ClusterTopologyOwnedLabel: ""}). - WithTopology( - builder.ClusterTopology(). - WithClass("class3"). - Build()). - Build(), - builder.Cluster(metav1.NamespaceDefault, "cluster4"). - WithLabels(map[string]string{clusterv1.ClusterTopologyOwnedLabel: ""}). - WithTopology( - builder.ClusterTopology(). - WithClass("class4"). - Build()). - Build(), - }, - expectErr: true, - }, - { - name: "allow deletion if multiple clusters exist and none of them references to the ClusterClass for deletion", - clusters: []client.Object{ - builder.Cluster(metav1.NamespaceDefault, "cluster1"). - WithLabels(map[string]string{clusterv1.ClusterTopologyOwnedLabel: ""}). - WithTopology( - builder.ClusterTopology(). - WithClass("class5"). - Build()). - Build(), - builder.Cluster(metav1.NamespaceDefault, "cluster2"). - WithLabels(map[string]string{clusterv1.ClusterTopologyOwnedLabel: ""}). - WithTopology( - builder.ClusterTopology(). - WithClass("class2"). - Build()). - Build(), - builder.Cluster(metav1.NamespaceDefault, "cluster3"). - WithLabels(map[string]string{clusterv1.ClusterTopologyOwnedLabel: ""}). - WithTopology( - builder.ClusterTopology(). - WithClass("class3"). - Build()). - Build(), - builder.Cluster(metav1.NamespaceDefault, "cluster4"). - WithLabels(map[string]string{clusterv1.ClusterTopologyOwnedLabel: ""}). - WithTopology( - builder.ClusterTopology(). - WithClass("class4"). - Build()). - Build(), - }, - expectErr: false, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - g := NewWithT(t) - // Sets up the fakeClient for the test case. - fakeClient := fake.NewClientBuilder(). - WithObjects(tt.clusters...). - WithScheme(fakeScheme). - Build() - - // Create the webhook and add the fakeClient as its client. - webhook := &ClusterClass{Client: fakeClient} - err := webhook.ValidateDelete(ctx, class) - if tt.expectErr { - g.Expect(err).To(HaveOccurred()) - return - } - g.Expect(err).ToNot(HaveOccurred()) - }) - } -} diff --git a/internal/webhooks/test/clusterclass_test.go b/internal/webhooks/test/clusterclass_test.go index 83fb9357dd92..702c55ca2975 100644 --- a/internal/webhooks/test/clusterclass_test.go +++ b/internal/webhooks/test/clusterclass_test.go @@ -20,6 +20,7 @@ import ( "testing" . "github.com/onsi/gomega" + apierrors "k8s.io/apimachinery/pkg/api/errors" utilfeature "k8s.io/component-base/featuregate/testing" "sigs.k8s.io/controller-runtime/pkg/client" @@ -258,9 +259,9 @@ func TestClusterWebhook_Fail_Update(t *testing.T) { g.Expect(env.Update(ctx, actualCluster)).NotTo(Succeed()) } -// TestClusterClassWebhook_Succeed_Delete tests the correct deletion behaviour for a ClusterClass with no references in existing Clusters. -// In this case deletion of the ClusterClass should succeed. -func TestClusterClassWebhook_Succeed_Delete(t *testing.T) { +// TestClusterClassWebhook_Fail_Delete tests the correct deletion behaviour for a ClusterClass with references in existing Clusters. +// In this case deletion of the ClusterClass should be blocked by the webhook. +func TestClusterClassWebhook_Delete(t *testing.T) { defer utilfeature.SetFeatureGateDuringTest(t, feature.Gates, feature.ClusterTopology, true)() g := NewWithT(t) @@ -268,7 +269,7 @@ func TestClusterClassWebhook_Succeed_Delete(t *testing.T) { g.Expect(err).ToNot(HaveOccurred()) // Create the objects needed for the integration test: - // - two ClusterClasses with all the related templates + // - a two ClusterClasses with all the related templates // - a Cluster using one of the ClusterClasses clusterClass1 := builder.ClusterClass(ns.Name, "class1"). WithInfrastructureClusterTemplate( @@ -326,85 +327,159 @@ func TestClusterClassWebhook_Succeed_Delete(t *testing.T) { Build()). Build() - // Create the two ClusterClasses in the API server. + // Create the ClusterClasses in the API server. g.Expect(env.CreateAndWait(ctx, clusterClass1)).To(Succeed()) g.Expect(env.CreateAndWait(ctx, clusterClass2)).To(Succeed()) - // Create a cluster. + // Create a clusters. g.Expect(env.CreateAndWait(ctx, cluster)).To(Succeed()) // Clean up the resources once the test is finished. t.Cleanup(func() { - g.Expect(env.CleanupAndWait(ctx, cluster)) - g.Expect(env.Cleanup(ctx, clusterClass1)) + g.Expect(env.Cleanup(ctx, cluster)) g.Expect(env.Cleanup(ctx, ns)) }) + // Attempt to delete ClusterClass "class1" which is in use. + // Expect no error here as the webhook should allow the deletion of an existing ClusterClass. + g.Expect(env.Delete(ctx, clusterClass1)).To(Not(Succeed())) + // Attempt to delete ClusterClass "class2" which is not in use. // Expect no error here as the webhook should allow the deletion of an existing ClusterClass. g.Expect(env.Delete(ctx, clusterClass2)).To(Succeed()) } -// TestClusterClassWebhook_Fail_Delete tests the correct deletion behaviour for a ClusterClass with references in existing Clusters. -// In this case deletion of the ClusterClass should be blocked by the webhook. -func TestClusterClassWebhook_Fail_Delete(t *testing.T) { - defer utilfeature.SetFeatureGateDuringTest(t, feature.Gates, feature.ClusterTopology, true)() +// TestClusterClassWebhook_Succeed_Delete tests the correct deletion behaviour for a ClusterClass with no references in existing Clusters. +// In this case deletion of the ClusterClass should succeed. +func TestClusterClassWebhook_Delete_MultipleExistingClusters(t *testing.T) { g := NewWithT(t) - - ns, err := env.CreateNamespace(ctx, "test-topology-clusterclass-webhook") + ns, err := env.CreateNamespace(ctx, "test-clusterclass-webhook") g.Expect(err).ToNot(HaveOccurred()) // Create the objects needed for the integration test: - // - a ClusterClass with all the related templates - // - a Cluster using one of the ClusterClasses - clusterClass1 := builder.ClusterClass(ns.Name, "class1"). - WithInfrastructureClusterTemplate( - builder.InfrastructureClusterTemplate(ns.Name, "inf").Build()). - WithControlPlaneTemplate( - builder.ControlPlaneTemplate(ns.Name, "cp1"). - Build()). - WithWorkerMachineDeploymentClasses( - *builder.MachineDeploymentClass("md1"). - WithInfrastructureTemplate( - builder.InfrastructureMachineTemplate(ns.Name, "OLD_INFRA").Build()). - WithBootstrapTemplate( - builder.BootstrapTemplate(ns.Name, "bootstrap1").Build()). - Build(), - *builder.MachineDeploymentClass("md2"). - WithInfrastructureTemplate( - builder.InfrastructureMachineTemplate(ns.Name, "infra1").Build()). - WithBootstrapTemplate( - builder.BootstrapTemplate(ns.Name, "bootstrap1").Build()). - Build()). - Build() - - cluster := builder.Cluster(ns.Name, "cluster2"). - WithLabels(map[string]string{clusterv1.ClusterTopologyOwnedLabel: ""}). - WithTopology( - builder.ClusterTopology(). - WithVersion("v1.20.1"). - WithClass("class1"). - WithMachineDeployment( - builder.MachineDeploymentTopology("workers1"). - WithClass("md1"). - Build(), - ). - Build()). - Build() - - // Create the ClusterClass in the API server. - g.Expect(env.CreateAndWait(ctx, clusterClass1)).To(Succeed()) - - // Create a clusters. - g.Expect(env.CreateAndWait(ctx, cluster)).To(Succeed()) + // - Five ClusterClasses with all the related templates + // - Five Clusters using all ClusterClass except "class1" + clusterClasses := []client.Object{ + builder.ClusterClass(ns.Name, "class1"). + WithInfrastructureClusterTemplate( + builder.InfrastructureClusterTemplate(ns.Name, "inf").Build()). + WithControlPlaneTemplate( + builder.ControlPlaneTemplate(ns.Name, "cp1"). + Build()). + Build(), + builder.ClusterClass(ns.Name, "class2"). + WithInfrastructureClusterTemplate( + builder.InfrastructureClusterTemplate(ns.Name, "inf").Build()). + WithControlPlaneTemplate( + builder.ControlPlaneTemplate(ns.Name, "cp1"). + Build()). + Build(), + builder.ClusterClass(ns.Name, "class3"). + WithInfrastructureClusterTemplate( + builder.InfrastructureClusterTemplate(ns.Name, "inf").Build()). + WithControlPlaneTemplate( + builder.ControlPlaneTemplate(ns.Name, "cp1"). + Build()). + Build(), + builder.ClusterClass(ns.Name, "class4"). + WithInfrastructureClusterTemplate( + builder.InfrastructureClusterTemplate(ns.Name, "inf").Build()). + WithControlPlaneTemplate( + builder.ControlPlaneTemplate(ns.Name, "cp1"). + Build()). + Build(), + builder.ClusterClass(ns.Name, "class5"). + WithInfrastructureClusterTemplate( + builder.InfrastructureClusterTemplate(ns.Name, "inf").Build()). + WithControlPlaneTemplate( + builder.ControlPlaneTemplate(ns.Name, "cp1"). + Build()). + Build(), + } + clusters := []client.Object{ + // class1 is not referenced in any of the below Clusters + builder.Cluster(ns.Name, "cluster1"). + WithLabels(map[string]string{clusterv1.ClusterTopologyOwnedLabel: ""}). + WithTopology( + builder.ClusterTopology(). + WithVersion("v1.20.1"). + WithClass("class5"). + Build()). + Build(), + builder.Cluster(ns.Name, "cluster2"). + WithLabels(map[string]string{clusterv1.ClusterTopologyOwnedLabel: ""}). + WithTopology( + builder.ClusterTopology(). + WithVersion("v1.20.1"). + WithClass("class2"). + Build()). + Build(), + builder.Cluster(ns.Name, "cluster3"). + WithLabels(map[string]string{clusterv1.ClusterTopologyOwnedLabel: ""}). + WithTopology( + builder.ClusterTopology(). + WithVersion("v1.20.1"). + WithClass("class3"). + Build()). + Build(), + builder.Cluster(ns.Name, "cluster4"). + WithLabels(map[string]string{clusterv1.ClusterTopologyOwnedLabel: ""}). + WithTopology( + builder.ClusterTopology(). + WithVersion("v1.20.1"). + WithClass("class4"). + Build()). + Build(), + } + var classForDeletion string + // Create the ClusterClasses in the API server. + for _, class := range clusterClasses { + g.Expect(env.CreateAndWait(ctx, class)).To(Succeed()) + } - // Clean up the resources once the test is finished. - t.Cleanup(func() { - g.Expect(env.Cleanup(ctx, cluster)) - g.Expect(env.Cleanup(ctx, ns)) - }) + // Create each of the clusters. + for _, c := range clusters { + g.Expect(env.CreateAndWait(ctx, c)).To(Succeed()) + } - // Attempt to delete ClusterClass "class1" which is in use. - // Expect no error here as the webhook should allow the deletion of an existing ClusterClass. - g.Expect(env.Delete(ctx, clusterClass1)).NotTo(Succeed()) + // defer a function to clean up created resources. + defer func() { + // Delete each of the clusters. + for _, c := range clusters { + g.Expect(env.Delete(ctx, c)).To(Succeed()) + } + + // Delete the ClusterClasses in the API server. + for _, class := range clusterClasses { + // The classForDeletion should not exist at this point. + if class.GetName() != classForDeletion { + if err := env.Delete(ctx, class); err != nil { + if apierrors.IsNotFound(err) { + continue + } + g.Expect(err).To(BeNil()) + } + } + } + }() + + // This is the unused clusterClass to be deleted. + classForDeletion = "class1" + + class := &clusterv1.ClusterClass{} + g.Expect(env.Get(ctx, client.ObjectKey{Namespace: ns.Name, Name: classForDeletion}, class)).To(Succeed()) + + // Attempt to delete ClusterClass "class1" which is not in use. + // Expect no error here as the webhook should allow the deletion of an unused ClusterClass. + g.Expect(env.Delete(ctx, class)).To(Succeed()) + + // This is a clusterClass in use to be deleted. + classForDeletion = "class3" + + class = &clusterv1.ClusterClass{} + g.Expect(env.Get(ctx, client.ObjectKey{Namespace: ns.Name, Name: classForDeletion}, class)).To(Succeed()) + + // Attempt to delete ClusterClass "class3" which is in use. + // Expect an error here as the webhook should not allow the deletion of an existing ClusterClass. + g.Expect(env.Delete(ctx, class)).To(Not(Succeed())) } diff --git a/internal/webhooks/test/suite_test.go b/internal/webhooks/test/suite_test.go index 3489e01eff1f..09303d48805b 100644 --- a/internal/webhooks/test/suite_test.go +++ b/internal/webhooks/test/suite_test.go @@ -22,9 +22,11 @@ import ( "os" "testing" + "k8s.io/component-base/featuregate" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/cluster-api/api/v1beta1/index" + "sigs.k8s.io/cluster-api/feature" "sigs.k8s.io/cluster-api/internal/test/envtest" ) @@ -34,6 +36,9 @@ var ( ) func TestMain(m *testing.M) { + if err := feature.Gates.(featuregate.MutableFeatureGate).Set(fmt.Sprintf("%s=%v", feature.ClusterTopology, true)); err != nil { + panic(fmt.Sprintf("unable to set ClusterTopology feature gate: %v", err)) + } setupIndexes := func(ctx context.Context, mgr ctrl.Manager) { if err := index.AddDefaultIndexes(ctx, mgr); err != nil { panic(fmt.Sprintf("unable to setup index: %v", err))