e2e: Only dump resources on node1

We were calling DumpOpenStack* in the 'all nodes' argument to
SynchronizedBeforeSuite, but only running the checks in the 'node 1'
argument to SynchronizedAfterSuite. This is inefficient if we run with more
than 1 node, as only data dumped by node 1 will be checked.

This change moves the checks to a DeferCleanup() node called from the
creation node, so the cleanup will only run where it was created.

We also refactor CheckResourceCleanup to return error so we can return
its output directly to DeferCleanup.

Author: mdbooth

test/e2e/suites/e2e/e2e_suite_test.go

@@ -20,6 +20,7 @@ limitations under the License.
 package e2e
 
 import (
+	"errors"
 	"os"
 	"testing"
 
@@ -31,20 +32,14 @@ import (
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
 	"k8s.io/klog/v2"
-	"k8s.io/utils/ptr"
 	ctrl "sigs.k8s.io/controller-runtime"
 
 	"sigs.k8s.io/cluster-api-provider-openstack/test/e2e/shared"
 )
 
 var (
-	e2eCtx                *shared.E2EContext
-	initialServers        []servers.Server
-	initialNetworks       []networks.Network
-	initialSecurityGroups []groups.SecGroup
-	initialLoadBalancers  []loadbalancers.LoadBalancer
-	initialVolumes        []volumes.Volume
-	err                   error
+	e2eCtx *shared.E2EContext
+	err    error
 )
 
 func init() {
@@ -66,38 +61,50 @@ func TestE2E(t *testing.T) {
 
 var _ = SynchronizedBeforeSuite(func() []byte {
 	data := shared.Node1BeforeSuite(e2eCtx)
-	return data
-}, func(data []byte) {
-	shared.AllNodesBeforeSuite(e2eCtx, data)
-	initialServers, err = shared.DumpOpenStackServers(e2eCtx, servers.ListOpts{})
+
+	initialServers, err := shared.DumpOpenStackServers(e2eCtx, servers.ListOpts{})
 	Expect(err).NotTo(HaveOccurred())
-	initialNetworks, err = shared.DumpOpenStackNetworks(e2eCtx, networks.ListOpts{})
+	initialNetworks, err := shared.DumpOpenStackNetworks(e2eCtx, networks.ListOpts{})
 	Expect(err).NotTo(HaveOccurred())
-	initialSecurityGroups, err = shared.DumpOpenStackSecurityGroups(e2eCtx, groups.ListOpts{})
+	initialSecurityGroups, err := shared.DumpOpenStackSecurityGroups(e2eCtx, groups.ListOpts{})
 	Expect(err).NotTo(HaveOccurred())
-	initialLoadBalancers, err = shared.DumpOpenStackLoadBalancers(e2eCtx, loadbalancers.ListOpts{})
+	initialLoadBalancers, err := shared.DumpOpenStackLoadBalancers(e2eCtx, loadbalancers.ListOpts{})
 	Expect(err).NotTo(HaveOccurred())
-	initialVolumes, err = shared.DumpOpenStackVolumes(e2eCtx, volumes.ListOpts{})
+	initialVolumes, err := shared.DumpOpenStackVolumes(e2eCtx, volumes.ListOpts{})
 	Expect(err).NotTo(HaveOccurred())
+
+	DeferCleanup(func() error {
+		return errors.Join(
+			CheckResourceCleanup(shared.DumpOpenStackServers, servers.ListOpts{}, initialServers),
+			CheckResourceCleanup(shared.DumpOpenStackNetworks, networks.ListOpts{}, initialNetworks),
+			CheckResourceCleanup(shared.DumpOpenStackSecurityGroups, groups.ListOpts{}, initialSecurityGroups),
+			CheckResourceCleanup(shared.DumpOpenStackLoadBalancers, loadbalancers.ListOpts{}, initialLoadBalancers),
+			CheckResourceCleanup(shared.DumpOpenStackVolumes, volumes.ListOpts{}, initialVolumes),
+		)
+	})
+
+	return data
+}, func(data []byte) {
+	shared.AllNodesBeforeSuite(e2eCtx, data)
 })
 
 // CheckResourceCleanup checks if all resources created during the test are cleaned up by comparing the resources
 // before and after the test.
 // The function f is used to list the resources of type T, whose list opts is of type L.
 // The list of resources is then compared to the initialResources, using the ConsistOfIDs custom matcher.
-func CheckResourceCleanup[T any, L any](f func(*shared.E2EContext, L) ([]T, error), l L, initialResources []T) *string {
+func CheckResourceCleanup[T any, L any](f func(*shared.E2EContext, L) ([]T, error), l L, initialResources []T) error {
 	endResources, err := f(e2eCtx, l)
 	if err != nil {
-		return ptr.To(err.Error())
+		return err
 	}
 
 	matcher := ConsistOfIDs(initialResources)
 	success, err := matcher.Match(endResources)
 	if err != nil {
-		return ptr.To(err.Error())
+		return err
 	}
 	if !success {
-		return ptr.To(matcher.FailureMessage(endResources))
+		return errors.New(matcher.FailureMessage(endResources))
 	}
 
 	return nil
@@ -106,23 +113,5 @@ func CheckResourceCleanup[T any, L any](f func(*shared.E2EContext, L) ([]T, erro
 var _ = SynchronizedAfterSuite(func() {
 	shared.AllNodesAfterSuite(e2eCtx)
 }, func() {
-	failed := false
-	for _, error := range []*string{
-		CheckResourceCleanup(shared.DumpOpenStackServers, servers.ListOpts{}, initialServers),
-		CheckResourceCleanup(shared.DumpOpenStackNetworks, networks.ListOpts{}, initialNetworks),
-		CheckResourceCleanup(shared.DumpOpenStackSecurityGroups, groups.ListOpts{}, initialSecurityGroups),
-		CheckResourceCleanup(shared.DumpOpenStackLoadBalancers, loadbalancers.ListOpts{}, initialLoadBalancers),
-		CheckResourceCleanup(shared.DumpOpenStackVolumes, volumes.ListOpts{}, initialVolumes),
-	} {
-		if error != nil {
-			GinkgoWriter.Println(*error)
-			failed = true
-		}
-	}
-
 	shared.Node1AfterSuite(e2eCtx)
-
-	if failed {
-		Fail("Not all resources were cleaned up")
-	}
 })