Merge pull request kubernetes-sigs#2145 from shiftstack/e2e_securitygroup

🐛 e2e: Fix deletion of test security group

Author: k8s-ci-robot

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)
 		}
 	})
 })