Merge pull request #306 from shiftstack/fix_tests

OCPBUGS-9925: Fix tests for OpenStack platform

Author: openshift-merge-bot[bot]

pkg/autoscaler/autoscaler.go

@@ -148,27 +148,28 @@ var _ = Describe("Autoscaler should", framework.LabelAutoscaler, Serial, func()
 
 		// Anything we create we must cleanup
 		cleanupObjects = make(map[string]runtimeclient.Object)
-	})
 
-	AfterEach(func() {
-		var machineSets []*machinev1.MachineSet
+		// Make sure to clean up the resources we created
+		DeferCleanup(func() {
+			var machineSets []*machinev1.MachineSet
 
-		for name, obj := range cleanupObjects {
-			if machineSet, ok := obj.(*machinev1.MachineSet); ok {
-				// Once we delete a MachineSet we should make sure that the
-				// all of its machines are deleted as well.
-				// Collect MachineSets to wait for.
-				machineSets = append(machineSets, machineSet)
-			}
+			for name, obj := range cleanupObjects {
+				if machineSet, ok := obj.(*machinev1.MachineSet); ok {
+					// Once we delete a MachineSet we should make sure that the
+					// all of its machines are deleted as well.
+					// Collect MachineSets to wait for.
+					machineSets = append(machineSets, machineSet)
+				}
 
-			Expect(deleteObject(name, obj)).To(Succeed(), "Failed to delete object %v", name)
-		}
+				Expect(deleteObject(name, obj)).To(Succeed(), "Failed to delete object %v", name)
+			}
 
-		if len(machineSets) > 0 {
-			// Wait for all MachineSets and their Machines to be deleted.
-			By("Waiting for MachineSets to be deleted...")
-			framework.WaitForMachineSetsDeleted(ctx, client, machineSets...)
-		}
+			if len(machineSets) > 0 {
+				// Wait for all MachineSets and their Machines to be deleted.
+				By("Waiting for MachineSets to be deleted...")
+				framework.WaitForMachineSetsDeleted(ctx, client, machineSets...)
+			}
+		})
 	})
 
 	Context("use a ClusterAutoscaler that has 100 maximum total nodes count", framework.LabelPeriodic, func() {

pkg/infra/infra.go

@@ -8,6 +8,7 @@ import (
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
 
+	configv1 "github.com/openshift/api/config/v1"
 	machinev1 "github.com/openshift/api/machine/v1beta1"
 	corev1 "k8s.io/api/core/v1"
 	policyv1 "k8s.io/api/policy/v1"
@@ -165,20 +166,24 @@ var _ = Describe("Managed cluster should", framework.LabelMachines, func() {
 		client, err = framework.LoadClient()
 		Expect(err).ToNot(HaveOccurred(), "Controller-runtime client should be able to be created")
 
+		// Reset the machineSet between each test
+		machineSet = nil
+
+		// Make sure to clean up the resources we created
+		DeferCleanup(func() {
+			if machineSet != nil {
+				By("Deleting the new MachineSet")
+				Expect(client.Delete(ctx, machineSet)).To(Succeed(), "MachineSet should be able to be deleted")
+				framework.WaitForMachineSetsDeleted(ctx, client, machineSet)
+			}
+		})
 	})
 
 	AfterEach(func() {
 		specReport := CurrentSpecReport()
 		if specReport.Failed() {
 			Expect(gatherer.WithSpecReport(specReport).GatherAll()).To(Succeed(), "StateGatherer should be able to gather resources")
 		}
-
-		if machineSet != nil {
-			By("Deleting the new MachineSet")
-			Expect(client.Delete(ctx, machineSet)).To(Succeed(), "MachineSet should be able to be deleted")
-			framework.WaitForMachineSetsDeleted(ctx, client, machineSet)
-		}
-
 	})
 
 	When("machineset has one replica", func() {
@@ -382,6 +387,19 @@ var _ = Describe("Managed cluster should", framework.LabelMachines, func() {
 	// Machines required for test: 0
 	// Reason: The machineSet creation is rejected by the webhook.
 	It("reject invalid machinesets", func() {
+		client, err := framework.LoadClient()
+		Expect(err).ToNot(HaveOccurred(), "Controller-runtime client should be able to be created")
+		// Only run on platforms that have webhooks
+		clusterInfra, err := framework.GetInfrastructure(ctx, client)
+		Expect(err).NotTo(HaveOccurred(), "Should be able to get Infrastructure")
+		platform := clusterInfra.Status.PlatformStatus.Type
+		switch platform {
+		case configv1.AWSPlatformType, configv1.AzurePlatformType, configv1.GCPPlatformType, configv1.VSpherePlatformType, configv1.PowerVSPlatformType, configv1.NutanixPlatformType:
+			// Do Nothing
+		default:
+			Skip(fmt.Sprintf("Platform %s does not have webhooks, skipping.", platform))
+		}
+
 		By("Creating invalid machineset")
 		invalidMachineSet := invalidMachinesetWithEmptyProviderConfig()
 		expectedAdmissionWebhookErr := "admission webhook \"default.machineset.machine.openshift.io\" denied the request: providerSpec.value: Required value: a value must be provided"

pkg/infra/lifecyclehooks.go

@@ -58,6 +58,19 @@ var _ = Describe("Lifecycle Hooks should", framework.LabelMachines, func() {
 		// Create machine set
 		machineSet, err = framework.CreateMachineSet(client, machineSetParams)
 		Expect(err).ToNot(HaveOccurred(), "MachineSet should be able to be created")
+
+		// Make sure to clean up the machineSet, if we create one.
+		DeferCleanup(func() {
+			By("Deleting the machineset")
+			cascadeDelete := metav1.DeletePropagationForeground
+			Expect(client.Delete(context.Background(), machineSet, &runtimeclient.DeleteOptions{
+				PropagationPolicy: &cascadeDelete,
+			})).To(Succeed(), "MachineSet should be able to be deleted")
+
+			By("Waiting for the MachineSet to be deleted...")
+			framework.WaitForMachineSetsDeleted(ctx, client, machineSet)
+		})
+
 		// Wait for machine to be running
 		framework.WaitForMachineSet(ctx, client, machineSet.GetName())
 
@@ -71,6 +84,15 @@ var _ = Describe("Lifecycle Hooks should", framework.LabelMachines, func() {
 			})
 		Expect(client.Create(context.Background(), workload)).To(Succeed(), "Could not create workload job")
 
+		// Make sure to clean up the workload job, if we create one.
+		DeferCleanup(func() {
+			cascadeDelete := metav1.DeletePropagationForeground
+			By("Deleting workload job")
+			Expect(client.Delete(context.Background(), workload, &runtimeclient.DeleteOptions{
+				PropagationPolicy: &cascadeDelete,
+			})).To(Succeed(), "Workload job should be able to be deleted")
+		})
+
 		By("Waiting for job pod to start running on machine.")
 		Eventually(func() (bool, error) {
 			jobPodList, err := framework.GetPods(client, map[string]string{lifecycleHooksPodLabel: ""})
@@ -91,20 +113,6 @@ var _ = Describe("Lifecycle Hooks should", framework.LabelMachines, func() {
 		if specReport.Failed() {
 			Expect(gatherer.WithSpecReport(specReport).GatherAll()).To(Succeed(), "StateGatherer should be able to gather resources")
 		}
-
-		By("Deleting the machineset")
-		cascadeDelete := metav1.DeletePropagationForeground
-		Expect(client.Delete(context.Background(), machineSet, &runtimeclient.DeleteOptions{
-			PropagationPolicy: &cascadeDelete,
-		})).To(Succeed(), "MachineSet should be able to be deleted")
-
-		By("Waiting for the MachineSet to be deleted...")
-		framework.WaitForMachineSetsDeleted(ctx, client, machineSet)
-
-		By("Deleting workload job")
-		Expect(client.Delete(context.Background(), workload, &runtimeclient.DeleteOptions{
-			PropagationPolicy: &cascadeDelete,
-		})).To(Succeed(), "Workload job should be able to be deleted")
 	})
 
 	// Machines required for test: 1

pkg/infra/spot.go

@@ -52,6 +52,28 @@ var _ = Describe("Running on Spot", framework.LabelMachines, framework.LabelSpot
 	BeforeEach(func() {
 		delObjects = make(map[string]runtimeclient.Object)
 
+		// Make sure to clean up the resources we created
+		DeferCleanup(func() {
+			var machineSets []*machinev1.MachineSet
+
+			for _, obj := range delObjects {
+				if machineSet, ok := obj.(*machinev1.MachineSet); ok {
+					// Once we delete a MachineSet we should make sure that the
+					// all of its machines are deleted as well.
+					// Collect MachineSets to wait for.
+					machineSets = append(machineSets, machineSet)
+				}
+
+				Expect(deleteObject(client, obj)).To(Succeed(), "Should be able to cleanup test objects")
+			}
+
+			if len(machineSets) > 0 {
+				// Wait for all MachineSets and their Machines to be deleted.
+				By("Waiting for MachineSets to be deleted...")
+				framework.WaitForMachineSetsDeleted(ctx, client, machineSets...)
+			}
+		})
+
 		var err error
 
 		gatherer, err = framework.NewGatherer()
@@ -117,25 +139,6 @@ var _ = Describe("Running on Spot", framework.LabelMachines, framework.LabelSpot
 		if specReport.Failed() {
 			Expect(gatherer.WithSpecReport(specReport).GatherAll()).To(Succeed(), "StateGatherer should be able to gather resources")
 		}
-
-		var machineSets []*machinev1.MachineSet
-
-		for _, obj := range delObjects {
-			if machineSet, ok := obj.(*machinev1.MachineSet); ok {
-				// Once we delete a MachineSet we should make sure that the
-				// all of its machines are deleted as well.
-				// Collect MachineSets to wait for.
-				machineSets = append(machineSets, machineSet)
-			}
-
-			Expect(deleteObject(client, obj)).To(Succeed(), "Should be able to cleanup test objects")
-		}
-
-		if len(machineSets) > 0 {
-			// Wait for all MachineSets and their Machines to be deleted.
-			By("Waiting for MachineSets to be deleted...")
-			framework.WaitForMachineSetsDeleted(ctx, client, machineSets...)
-		}
 	})
 
 	// Machines required for test: 1

pkg/infra/webhooks.go

@@ -70,23 +70,26 @@ var _ = Describe("Webhooks", framework.LabelMachines, func() {
 				return framework.IsValidatingWebhookConfigurationSynced(ctx, client)
 			}, framework.WaitShort).Should(BeTrue(), "ValidingWebhookConfiguration must be synced before running these tests")
 		})
+
+		// Make sure to clean up the resources we created
+		DeferCleanup(func() {
+			machineSets, err := framework.GetMachineSets(client, testSelector)
+			Expect(err).ToNot(HaveOccurred(), "Should be able to list test MachineSets")
+			Expect(framework.DeleteMachineSets(client, machineSets...)).To(Succeed(), "Should be able to delete test MachineSets")
+			framework.WaitForMachineSetsDeleted(ctx, client, machineSets...)
+
+			machines, err := framework.GetMachines(ctx, client, testSelector)
+			Expect(err).ToNot(HaveOccurred(), "Should be able to get test Machines")
+			Expect(framework.DeleteMachines(ctx, client, machines...)).To(Succeed(), "Should be able to delete test Machines")
+			framework.WaitForMachinesDeleted(client, machines...)
+		})
 	})
 
 	AfterEach(func() {
 		specReport := CurrentSpecReport()
 		if specReport.Failed() {
 			Expect(gatherer.WithSpecReport(specReport).GatherAll()).To(Succeed(), "StateGatherer should be able to gather resources")
 		}
-
-		machineSets, err := framework.GetMachineSets(client, testSelector)
-		Expect(err).ToNot(HaveOccurred(), "Should be able to list test MachineSets")
-		Expect(framework.DeleteMachineSets(client, machineSets...)).To(Succeed(), "Should be able to delete test MachineSets")
-		framework.WaitForMachineSetsDeleted(ctx, client, machineSets...)
-
-		machines, err := framework.GetMachines(ctx, client, testSelector)
-		Expect(err).ToNot(HaveOccurred(), "Should be able to get test Machines")
-		Expect(framework.DeleteMachines(ctx, client, machines...)).To(Succeed(), "Should be able to delete test Machines")
-		framework.WaitForMachinesDeleted(client, machines...)
 	})
 
 	// Machines required for test: 1

pkg/machinehealthcheck/machinehealthcheck.go

@@ -55,6 +55,17 @@ var _ = Describe("MachineHealthCheck", framework.LabelMachineHealthChecks, func(
 		machineSet, err = framework.CreateMachineSet(client, machineSetParams)
 		Expect(err).ToNot(HaveOccurred(), "failed to create a new machineSet resource")
 
+		// Make sure to clean up the resources we created
+		DeferCleanup(func() {
+			By("Deleting the MachineHealthCheck resource")
+			Expect(client.Delete(context.Background(), machinehealthcheck)).To(Succeed(), "failed to delete MHC")
+
+			By("Deleting the new MachineSet")
+			Expect(client.Delete(context.Background(), machineSet)).To(Succeed(), "failed to delete machineSet")
+
+			framework.WaitForMachineSetsDeleted(ctx, client, machineSet)
+		})
+
 		framework.WaitForMachineSet(ctx, client, machineSet.GetName())
 	})
 
@@ -63,14 +74,6 @@ var _ = Describe("MachineHealthCheck", framework.LabelMachineHealthChecks, func(
 		if specReport.Failed() {
 			Expect(gatherer.WithSpecReport(specReport).GatherAll()).To(Succeed(), "failed to gather spec report")
 		}
-
-		By("Deleting the MachineHealthCheck resource")
-		Expect(client.Delete(context.Background(), machinehealthcheck)).To(Succeed(), "failed to delete MHC")
-
-		By("Deleting the new MachineSet")
-		Expect(client.Delete(context.Background(), machineSet)).To(Succeed(), "failed to delete machineSet")
-
-		framework.WaitForMachineSetsDeleted(ctx, client, machineSet)
 	})
 
 	// Machines required for test: 3

pkg/providers/aws.go

@@ -50,18 +50,21 @@ var _ = Describe("MetadataServiceOptions", framework.LabelCloudProviderSpecific,
 		if platform != configv1.AWSPlatformType {
 			Skip(fmt.Sprintf("skipping AWS specific tests on %s", platform))
 		}
+
+		// Make sure to clean up the resources we created
+		DeferCleanup(func() {
+			Expect(framework.DeleteMachineSets(client, toDelete...)).To(Succeed())
+			toDelete = make([]*machinev1.MachineSet, 0, 3)
+
+			framework.WaitForMachineSetsDeleted(ctx, client, toDelete...)
+		})
 	})
 
 	AfterEach(func() {
 		specReport := CurrentSpecReport()
 		if specReport.Failed() {
 			Expect(gatherer.WithSpecReport(specReport).GatherAll()).To(Succeed())
 		}
-
-		Expect(framework.DeleteMachineSets(client, toDelete...)).To(Succeed())
-		toDelete = make([]*machinev1.MachineSet, 0, 3)
-
-		framework.WaitForMachineSetsDeleted(ctx, client, toDelete...)
 	})
 
 	createMachineSet := func(metadataAuth string) (*machinev1.MachineSet, error) {