e2e: Fix deletion of test security group

We were trying to delete the security group in a DeferCleanup() called
from the test itself. This will execute before the cluster is deleted,
so it can't work as the cluster machines are still using the security
group. We need to delete the security group after the cluster has been
deleted.

In making this change, we also add a context argument to
postClusterCleanup and propagate that to the other users of this
mechanism.

Author: mdbooth

test/e2e/shared/openstack.go

@@ -477,35 +477,35 @@ func DumpOpenStackPorts(e2eCtx *E2EContext, filter ports.ListOpts) ([]ports.Port
 }
 
 // CreateOpenStackSecurityGroup creates a security group to be consumed by a worker node.
-func CreateOpenStackSecurityGroup(e2eCtx *E2EContext, securityGroupName, description string) error {
+func CreateOpenStackSecurityGroup(ctx context.Context, e2eCtx *E2EContext, securityGroupName, description string) (func(context.Context), error) {
 	providerClient, clientOpts, _, err := GetTenantProviderClient(e2eCtx)
 	if err != nil {
-		return fmt.Errorf("error creating provider client: %s", err)
+		return nil, fmt.Errorf("error creating provider client: %s", err)
 	}
 
 	networkClient, err := openstack.NewNetworkV2(providerClient, gophercloud.EndpointOpts{
 		Region: clientOpts.RegionName,
 	})
 	if err != nil {
-		return fmt.Errorf("error creating network client: %s", err)
+		return nil, fmt.Errorf("error creating network client: %s", err)
 	}
 
 	createOpts := groups.CreateOpts{
 		Name:        securityGroupName,
 		Description: description,
 	}
 
-	group, err := groups.Create(context.TODO(), networkClient, createOpts).Extract()
+	group, err := groups.Create(ctx, networkClient, createOpts).Extract()
 	if err != nil {
-		return err
+		return nil, err
 	}
-	// Ensure the group is deleted after the test
-	DeferCleanup(func() error {
+
+	cleanup := func(ctx context.Context) {
 		By(fmt.Sprintf("Deleting test security group %s(%s)", group.Name, group.ID))
-		return groups.Delete(context.TODO(), networkClient, group.ID).ExtractErr()
-	})
+		Expect(groups.Delete(ctx, networkClient, group.ID).ExtractErr()).To(Succeed(), "Delete test security group")
+	}
 
-	return nil
+	return cleanup, nil
 }
 
 // DumpOpenStackTrunks trunks for a given port.
@@ -817,7 +817,7 @@ func CreateOpenStackNetwork(e2eCtx *E2EContext, name, cidr string) (*networks.Ne
 	return net, nil
 }
 
-func DeleteOpenStackNetwork(e2eCtx *E2EContext, id string) error {
+func DeleteOpenStackNetwork(ctx context.Context, e2eCtx *E2EContext, id string) error {
 	providerClient, clientOpts, _, err := GetTenantProviderClient(e2eCtx)
 	if err != nil {
 		_, _ = fmt.Fprintf(GinkgoWriter, "error creating provider client: %s\n", err)
@@ -831,7 +831,7 @@ func DeleteOpenStackNetwork(e2eCtx *E2EContext, id string) error {
 		return fmt.Errorf("error creating network client: %s", err)
 	}
 
-	return networks.Delete(context.TODO(), networkClient, id).ExtractErr()
+	return networks.Delete(ctx, networkClient, id).ExtractErr()
 }
 
 func GetOpenStackVolume(e2eCtx *E2EContext, name string) (*volumes.Volume, error) {

test/e2e/suites/e2e/e2e_test.go

@@ -66,7 +66,7 @@ var _ = Describe("e2e tests [PR-Blocking]", func() {
 		ctx              context.Context
 
 		// Cleanup functions which cannot run until after the cluster has been deleted
-		postClusterCleanup []func()
+		postClusterCleanup []func(context.Context)
 	)
 
 	BeforeEach(func() {
@@ -406,11 +406,10 @@ var _ = Describe("e2e tests [PR-Blocking]", func() {
 			md3Name := clusterName + "-md-3"
 			testSecurityGroupName := "testSecGroup"
 			// create required test security group
-			Eventually(func() int {
-				err := shared.CreateOpenStackSecurityGroup(e2eCtx, testSecurityGroupName, "Test security group")
-				Expect(err).To(BeNil())
-				return 1
-			}, e2eCtx.E2EConfig.GetIntervals(specName, "wait-worker-nodes")...).Should(Equal(1))
+			var securityGroupCleanup func(ctx context.Context)
+			securityGroupCleanup, err = shared.CreateOpenStackSecurityGroup(ctx, e2eCtx, testSecurityGroupName, "Test security group")
+			Expect(err).To(BeNil())
+			postClusterCleanup = append(postClusterCleanup, securityGroupCleanup)
 
 			customPortOptions := &[]infrav1.PortOpts{
 				{
@@ -496,17 +495,17 @@ var _ = Describe("e2e tests [PR-Blocking]", func() {
 
 			extraNet1, err = shared.CreateOpenStackNetwork(e2eCtx, fmt.Sprintf("%s-extraNet1", namespace.Name), "10.14.0.0/24")
 			Expect(err).NotTo(HaveOccurred())
-			postClusterCleanup = append(postClusterCleanup, func() {
+			postClusterCleanup = append(postClusterCleanup, func(ctx context.Context) {
 				shared.Logf("Deleting additional network %s", extraNet1.Name)
-				err := shared.DeleteOpenStackNetwork(e2eCtx, extraNet1.ID)
+				err := shared.DeleteOpenStackNetwork(ctx, e2eCtx, extraNet1.ID)
 				Expect(err).NotTo(HaveOccurred())
 			})
 
 			extraNet2, err = shared.CreateOpenStackNetwork(e2eCtx, fmt.Sprintf("%s-extraNet2", namespace.Name), "10.14.1.0/24")
 			Expect(err).NotTo(HaveOccurred())
-			postClusterCleanup = append(postClusterCleanup, func() {
+			postClusterCleanup = append(postClusterCleanup, func(ctx context.Context) {
 				shared.Logf("Deleting additional network %s", extraNet2.Name)
-				err := shared.DeleteOpenStackNetwork(e2eCtx, extraNet2.ID)
+				err := shared.DeleteOpenStackNetwork(ctx, e2eCtx, extraNet2.ID)
 				Expect(err).NotTo(HaveOccurred())
 			})
 
@@ -806,15 +805,15 @@ var _ = Describe("e2e tests [PR-Blocking]", func() {
 		})
 	})
 
-	AfterEach(func() {
+	AfterEach(func(ctx context.Context) {
 		shared.Logf("Attempting to collect logs for cluster %q in namespace %q", clusterResources.Cluster.Name, namespace.Name)
 		e2eCtx.Environment.BootstrapClusterProxy.CollectWorkloadClusterLogs(ctx, namespace.Name, clusterResources.Cluster.Name, filepath.Join(e2eCtx.Settings.ArtifactFolder, "clusters", e2eCtx.Environment.BootstrapClusterProxy.GetName(), namespace.Name))
 		// Dumps all the resources in the spec namespace, then cleanups the cluster object and the spec namespace itself.
 		shared.DumpSpecResourcesAndCleanup(ctx, specName, namespace, e2eCtx)
 
 		// Cleanup resources which can't be cleaned up until the cluster has been deleted
 		for _, cleanup := range postClusterCleanup {
-			cleanup()
+			cleanup(ctx)
 		}
 	})
 })