From 60ce72d550590be3e8dff480364fd9b749f66c7d Mon Sep 17 00:00:00 2001 From: Matthew Booth Date: Wed, 17 Jul 2024 16:02:19 +0100 Subject: [PATCH] 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. --- test/e2e/suites/e2e/e2e_suite_test.go | 65 +++++++++++---------------- 1 file changed, 27 insertions(+), 38 deletions(-) diff --git a/test/e2e/suites/e2e/e2e_suite_test.go b/test/e2e/suites/e2e/e2e_suite_test.go index cf98522d3a..1969ce9539 100644 --- a/test/e2e/suites/e2e/e2e_suite_test.go +++ b/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") - } })