[TT-630] Moves Cleanup into Builder (#10910)

* Moves Cleanup into Builder

* Fix imports

* Simplify cleanup code

* Log > return

* Remove debug

integration-tests/docker/test_env/test_env.go

@@ -231,83 +231,103 @@ func (te *CLClusterTestEnv) Terminate() error {
 	return nil
 }
 
-// Cleanup cleans the environment up after it's done being used, mainly for returning funds when on live networks.
-// Intended to be used as part of t.Cleanup() in tests.
-func (te *CLClusterTestEnv) Cleanup(t *testing.T) error {
-	if te.EVMClient == nil {
-		return errors.New("blockchain client is nil, unable to return funds from chainlink nodes")
+// Cleanup cleans the environment up after it's done being used, mainly for returning funds when on live networks and logs.
+func (te *CLClusterTestEnv) Cleanup() error {
+	te.l.Info().Msg("Cleaning up test environment")
+	if te.t == nil {
+		return errors.New("cannot cleanup test environment without a testing.T")
 	}
 	if te.CLNodes == nil {
-		return errors.New("chainlink nodes are nil, unable to return funds from chainlink nodes")
+		return errors.New("chainlink nodes are nil, unable cleanup chainlink nodes")
 	}
 
 	// TODO: This is an imperfect and temporary solution, see TT-590 for a more sustainable solution
 	// Collect logs if the test fails, or if we just want them
-	if t.Failed() || os.Getenv("TEST_LOG_COLLECT") == "true" {
-		folder := fmt.Sprintf("./logs/%s-%s", t.Name(), time.Now().Format("2006-01-02T15-04-05"))
-		if err := os.MkdirAll(folder, os.ModePerm); err != nil {
+	if te.t.Failed() || os.Getenv("TEST_LOG_COLLECT") == "true" {
+		if err := te.collectTestLogs(); err != nil {
 			return err
 		}
+	}
 
-		te.l.Info().Msg("Collecting test logs")
-		eg := &errgroup.Group{}
-		for _, n := range te.CLNodes {
-			node := n
-			eg.Go(func() error {
-				logFileName := filepath.Join(folder, fmt.Sprintf("node-%s.log", node.ContainerName))
-				logFile, err := os.OpenFile(logFileName, os.O_CREATE|os.O_WRONLY, 0644)
-				if err != nil {
-					return err
-				}
-				defer logFile.Close()
-				logReader, err := node.Container.Logs(context.Background())
-				if err != nil {
-					return err
-				}
-				_, err = io.Copy(logFile, logReader)
-				if err != nil {
-					return err
-				}
-				te.l.Info().Str("Node", node.ContainerName).Str("File", logFileName).Msg("Wrote Logs")
-				return nil
-			})
-		}
-
-		if err := eg.Wait(); err != nil {
+	if te.EVMClient == nil {
+		return errors.New("evm client is nil, unable to return funds from chainlink nodes during cleanup")
+	} else if te.EVMClient.NetworkSimulated() {
+		te.l.Info().
+			Str("Network Name", te.EVMClient.GetNetworkName()).
+			Msg("Network is a simulated network. Skipping fund return.")
+	} else {
+		if err := te.returnFunds(); err != nil {
 			return err
 		}
+	}
+
+	return nil
+}
 
-		te.l.Info().Str("Logs Location", folder).Msg("Wrote test logs")
+// collectTestLogs collects the logs from all the Chainlink nodes in the test environment and writes them to local files
+func (te *CLClusterTestEnv) collectTestLogs() error {
+	te.l.Info().Msg("Collecting test logs")
+	folder := fmt.Sprintf("./logs/%s-%s", te.t.Name(), time.Now().Format("2006-01-02T15-04-05"))
+	if err := os.MkdirAll(folder, os.ModePerm); err != nil {
+		return err
 	}
 
-	// Check if we need to return funds
-	if te.EVMClient.NetworkSimulated() {
-		te.l.Info().Str("Network Name", te.EVMClient.GetNetworkName()).
-			Msg("Network is a simulated network. Skipping fund return.")
-	} else {
-		te.l.Info().Msg("Attempting to return Chainlink node funds to default network wallets")
-		for _, chainlinkNode := range te.CLNodes {
-			fundedKeys, err := chainlinkNode.API.ExportEVMKeysForChain(te.EVMClient.GetChainID().String())
+	eg := &errgroup.Group{}
+	for _, n := range te.CLNodes {
+		node := n
+		eg.Go(func() error {
+			logFileName := filepath.Join(folder, fmt.Sprintf("node-%s.log", node.ContainerName))
+			logFile, err := os.OpenFile(logFileName, os.O_CREATE|os.O_WRONLY, 0644)
+			if err != nil {
+				return err
+			}
+			defer logFile.Close()
+			logReader, err := node.Container.Logs(context.Background())
+			if err != nil {
+				return err
+			}
+			_, err = io.Copy(logFile, logReader)
+			if err != nil {
+				return err
+			}
+			te.l.Info().Str("Node", node.ContainerName).Str("File", logFileName).Msg("Wrote Logs")
+			return nil
+		})
+	}
+
+	if err := eg.Wait(); err != nil {
+		return err
+	}
+
+	te.l.Info().Str("Logs Location", folder).Msg("Wrote test logs")
+	return nil
+}
+
+func (te *CLClusterTestEnv) returnFunds() error {
+	te.l.Info().Msg("Attempting to return Chainlink node funds to default network wallets")
+	for _, chainlinkNode := range te.CLNodes {
+		fundedKeys, err := chainlinkNode.API.ExportEVMKeysForChain(te.EVMClient.GetChainID().String())
+		if err != nil {
+			return err
+		}
+		for _, key := range fundedKeys {
+			keyToDecrypt, err := json.Marshal(key)
+			if err != nil {
+				return err
+			}
+			// This can take up a good bit of RAM and time. When running on the remote-test-runner, this can lead to OOM
+			// issues. So we avoid running in parallel; slower, but safer.
+			decryptedKey, err := keystore.DecryptKey(keyToDecrypt, client.ChainlinkKeyPassword)
 			if err != nil {
 				return err
 			}
-			for _, key := range fundedKeys {
-				keyToDecrypt, err := json.Marshal(key)
-				if err != nil {
-					return err
-				}
-				// This can take up a good bit of RAM and time. When running on the remote-test-runner, this can lead to OOM
-				// issues. So we avoid running in parallel; slower, but safer.
-				decryptedKey, err := keystore.DecryptKey(keyToDecrypt, client.ChainlinkKeyPassword)
-				if err != nil {
-					return err
-				}
-				if err = te.EVMClient.ReturnFunds(decryptedKey.PrivateKey); err != nil {
-					return err
-				}
+			if err = te.EVMClient.ReturnFunds(decryptedKey.PrivateKey); err != nil {
+				// If we fail to return funds from one, go on to try the others anyway
+				te.l.Error().Err(err).Str("Node", chainlinkNode.ContainerName).Msg("Error returning funds from node")
 			}
 		}
 	}
 
+	te.l.Info().Msg("Returned funds from Chainlink nodes")
 	return nil
 }

integration-tests/docker/test_env/test_env_builder.go

@@ -171,6 +171,12 @@ func (b *CLTestEnvBuilder) Build() (*CLClusterTestEnv, error) {
 		}
 	}
 
+	b.t.Cleanup(func() {
+		if err := b.te.Cleanup(); err != nil {
+			b.l.Error().Err(err).Msg("Error cleaning up test environment")
+		}
+	})
+
 	if b.nonDevGethNetworks != nil {
 		b.te.WithPrivateChain(b.nonDevGethNetworks)
 		err := b.te.StartPrivateChain()

integration-tests/load/vrfv2plus/vrfv2plus_test.go

@@ -3,19 +3,21 @@ package loadvrfv2plus
 import (
 	"context"
 	"fmt"
+	"math/big"
+	"sync"
+	"testing"
+	"time"
+
 	"github.com/kelseyhightower/envconfig"
 	"github.com/smartcontractkit/chainlink-testing-framework/logging"
+	"github.com/smartcontractkit/wasp"
+	"github.com/stretchr/testify/require"
+
 	"github.com/smartcontractkit/chainlink/integration-tests/actions"
 	"github.com/smartcontractkit/chainlink/integration-tests/actions/vrfv2plus"
 	"github.com/smartcontractkit/chainlink/integration-tests/actions/vrfv2plus/vrfv2plus_config"
 	"github.com/smartcontractkit/chainlink/integration-tests/contracts"
 	"github.com/smartcontractkit/chainlink/integration-tests/docker/test_env"
-	"github.com/smartcontractkit/wasp"
-	"github.com/stretchr/testify/require"
-	"math/big"
-	"sync"
-	"testing"
-	"time"
 )
 
 func TestVRFV2PlusLoad(t *testing.T) {
@@ -34,11 +36,6 @@ func TestVRFV2PlusLoad(t *testing.T) {
 		WithLogWatcher().
 		Build()
 	require.NoError(t, err, "error creating test env")
-	t.Cleanup(func() {
-		if err := env.Cleanup(t); err != nil {
-			l.Error().Err(err).Msg("Error cleaning up test environment")
-		}
-	})
 
 	env.ParallelTransactions(true)
 

integration-tests/smoke/automation_test.go

@@ -17,16 +17,17 @@ import (
 	"github.com/smartcontractkit/chainlink-testing-framework/logging"
 	"github.com/smartcontractkit/chainlink-testing-framework/networks"
 
+	cltypes "github.com/smartcontractkit/chainlink/v2/core/chains/evm/types"
+	"github.com/smartcontractkit/chainlink/v2/core/gethwrappers/generated/automation_utils_2_1"
+	"github.com/smartcontractkit/chainlink/v2/core/store/models"
+
 	"github.com/smartcontractkit/chainlink/integration-tests/actions"
 	"github.com/smartcontractkit/chainlink/integration-tests/client"
 	"github.com/smartcontractkit/chainlink/integration-tests/contracts"
 	"github.com/smartcontractkit/chainlink/integration-tests/contracts/ethereum"
 	"github.com/smartcontractkit/chainlink/integration-tests/docker/test_env"
 	"github.com/smartcontractkit/chainlink/integration-tests/types/config/node"
 	it_utils "github.com/smartcontractkit/chainlink/integration-tests/utils"
-	cltypes "github.com/smartcontractkit/chainlink/v2/core/chains/evm/types"
-	"github.com/smartcontractkit/chainlink/v2/core/gethwrappers/generated/automation_utils_2_1"
-	"github.com/smartcontractkit/chainlink/v2/core/store/models"
 )
 
 var utilsABI = cltypes.MustGetABI(automation_utils_2_1.AutomationUtilsABI)

integration-tests/smoke/cron_test.go

@@ -26,11 +26,6 @@ func TestCronBasic(t *testing.T) {
 		WithCLNodes(1).
 		Build()
 	require.NoError(t, err)
-	t.Cleanup(func() {
-		if err := env.Cleanup(t); err != nil {
-			l.Error().Err(err).Msg("Error cleaning up test environment")
-		}
-	})
 
 	err = env.MockAdapter.SetAdapterBasedIntValuePath("/variable", []string{http.MethodGet, http.MethodPost}, 5)
 	require.NoError(t, err, "Setting value path in mock adapter shouldn't fail")

integration-tests/smoke/flux_test.go

@@ -32,11 +32,6 @@ func TestFluxBasic(t *testing.T) {
 		WithCLNodes(3).
 		Build()
 	require.NoError(t, err)
-	t.Cleanup(func() {
-		if err := env.Cleanup(t); err != nil {
-			l.Error().Err(err).Msg("Error cleaning up test environment")
-		}
-	})
 
 	nodeAddresses, err := env.ChainlinkNodeAddresses()
 	require.NoError(t, err, "Retrieving on-chain wallet addresses for chainlink nodes shouldn't fail")

integration-tests/smoke/forwarder_ocr_test.go

@@ -27,11 +27,6 @@ func TestForwarderOCRBasic(t *testing.T) {
 		WithFunding(big.NewFloat(.1)).
 		Build()
 	require.NoError(t, err)
-	t.Cleanup(func() {
-		if err := env.Cleanup(t); err != nil {
-			l.Error().Err(err).Msg("Error cleaning up test environment")
-		}
-	})
 
 	env.ParallelTransactions(true)
 

integration-tests/smoke/forwarders_ocr2_test.go

@@ -36,11 +36,6 @@ func TestForwarderOCR2Basic(t *testing.T) {
 		WithFunding(big.NewFloat(.1)).
 		Build()
 	require.NoError(t, err)
-	t.Cleanup(func() {
-		if err := env.Cleanup(t); err != nil {
-			l.Error().Err(err).Msg("Error cleaning up test environment")
-		}
-	})
 
 	env.ParallelTransactions(true)
 

integration-tests/smoke/keeper_test.go

@@ -1105,7 +1105,6 @@ func setupKeeperTest(t *testing.T) (
 	clNodeConfig.Keeper.TurnLookBack = &turnLookBack
 	clNodeConfig.Keeper.Registry.SyncInterval = &syncInterval
 	clNodeConfig.Keeper.Registry.PerformGasOverhead = &performGasOverhead
-	l := logging.GetTestLogger(t)
 
 	env, err := test_env.NewCLTestEnvBuilder().
 		WithTestLogger(t).
@@ -1115,11 +1114,6 @@ func setupKeeperTest(t *testing.T) (
 		WithFunding(big.NewFloat(.5)).
 		Build()
 	require.NoError(t, err, "Error deploying test environment")
-	t.Cleanup(func() {
-		if err := env.Cleanup(t); err != nil {
-			l.Error().Err(err).Msg("Error cleaning up test environment")
-		}
-	})
 
 	env.ParallelTransactions(true)
 

integration-tests/smoke/ocr2_test.go

@@ -44,11 +44,6 @@ func TestOCRv2Basic(t *testing.T) {
 		WithFunding(big.NewFloat(.1)).
 		Build()
 	require.NoError(t, err)
-	t.Cleanup(func() {
-		if err := env.Cleanup(t); err != nil {
-			l.Error().Err(err).Msg("Error cleaning up test environment")
-		}
-	})
 
 	env.ParallelTransactions(true)
 

integration-tests/smoke/ocr_test.go

@@ -25,11 +25,6 @@ func TestOCRBasic(t *testing.T) {
 		WithFunding(big.NewFloat(.1)).
 		Build()
 	require.NoError(t, err)
-	t.Cleanup(func() {
-		if err := env.Cleanup(t); err != nil {
-			l.Error().Err(err).Msg("Error cleaning up test environment")
-		}
-	})
 
 	env.ParallelTransactions(true)
 

integration-tests/smoke/runlog_test.go

@@ -30,11 +30,6 @@ func TestRunLogBasic(t *testing.T) {
 		WithFunding(big.NewFloat(.1)).
 		Build()
 	require.NoError(t, err)
-	t.Cleanup(func() {
-		if err := env.Cleanup(t); err != nil {
-			l.Error().Err(err).Msg("Error cleaning up test environment")
-		}
-	})
 
 	lt, err := env.ContractDeployer.DeployLinkTokenContract()
 	require.NoError(t, err, "Deploying Link Token Contract shouldn't fail")

integration-tests/smoke/vrf_test.go

@@ -30,11 +30,6 @@ func TestVRFBasic(t *testing.T) {
 		WithFunding(big.NewFloat(.1)).
 		Build()
 	require.NoError(t, err)
-	t.Cleanup(func() {
-		if err := env.Cleanup(t); err != nil {
-			l.Error().Err(err).Msg("Error cleaning up test environment")
-		}
-	})
 	env.ParallelTransactions(true)
 
 	lt, err := actions.DeployLINKToken(env.ContractDeployer)

integration-tests/smoke/vrfv2_test.go

@@ -29,11 +29,6 @@ func TestVRFv2Basic(t *testing.T) {
 		WithFunding(vrfConst.ChainlinkNodeFundingAmountEth).
 		Build()
 	require.NoError(t, err)
-	t.Cleanup(func() {
-		if err := env.Cleanup(t); err != nil {
-			l.Error().Err(err).Msg("Error cleaning up test environment")
-		}
-	})
 	env.ParallelTransactions(true)
 
 	mockFeed, err := actions.DeployMockETHLinkFeed(env.ContractDeployer, vrfConst.LinkEthFeedResponse)