From 400cd271db175d6801ab262ed4c07dd1275a3edb Mon Sep 17 00:00:00 2001 From: Adam Hamrick Date: Wed, 11 Oct 2023 14:14:46 -0400 Subject: [PATCH] [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 | 134 ++++++++++-------- .../docker/test_env/test_env_builder.go | 6 + .../load/vrfv2plus/vrfv2plus_test.go | 19 ++- integration-tests/smoke/automation_test.go | 7 +- integration-tests/smoke/cron_test.go | 5 - integration-tests/smoke/flux_test.go | 5 - integration-tests/smoke/forwarder_ocr_test.go | 5 - .../smoke/forwarders_ocr2_test.go | 5 - integration-tests/smoke/keeper_test.go | 6 - integration-tests/smoke/ocr2_test.go | 5 - integration-tests/smoke/ocr_test.go | 5 - integration-tests/smoke/runlog_test.go | 5 - integration-tests/smoke/vrf_test.go | 5 - integration-tests/smoke/vrfv2_test.go | 5 - integration-tests/smoke/vrfv2plus_test.go | 16 +-- 15 files changed, 98 insertions(+), 135 deletions(-) diff --git a/integration-tests/docker/test_env/test_env.go b/integration-tests/docker/test_env/test_env.go index 02db1ae788c..34e3dd17e34 100644 --- a/integration-tests/docker/test_env/test_env.go +++ b/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 } diff --git a/integration-tests/docker/test_env/test_env_builder.go b/integration-tests/docker/test_env/test_env_builder.go index ced617bc060..c9c68f0ea9c 100644 --- a/integration-tests/docker/test_env/test_env_builder.go +++ b/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() diff --git a/integration-tests/load/vrfv2plus/vrfv2plus_test.go b/integration-tests/load/vrfv2plus/vrfv2plus_test.go index ee8be6bd8a5..805b9711eda 100644 --- a/integration-tests/load/vrfv2plus/vrfv2plus_test.go +++ b/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) diff --git a/integration-tests/smoke/automation_test.go b/integration-tests/smoke/automation_test.go index b7aba624e4c..ca5db25cd97 100644 --- a/integration-tests/smoke/automation_test.go +++ b/integration-tests/smoke/automation_test.go @@ -17,6 +17,10 @@ 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" @@ -24,9 +28,6 @@ import ( "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) diff --git a/integration-tests/smoke/cron_test.go b/integration-tests/smoke/cron_test.go index 26e039289cf..dee37af3540 100644 --- a/integration-tests/smoke/cron_test.go +++ b/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") diff --git a/integration-tests/smoke/flux_test.go b/integration-tests/smoke/flux_test.go index c2fbf2d435b..b517cb5b57a 100644 --- a/integration-tests/smoke/flux_test.go +++ b/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") diff --git a/integration-tests/smoke/forwarder_ocr_test.go b/integration-tests/smoke/forwarder_ocr_test.go index a6c0cacb96c..21f108c732f 100644 --- a/integration-tests/smoke/forwarder_ocr_test.go +++ b/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) diff --git a/integration-tests/smoke/forwarders_ocr2_test.go b/integration-tests/smoke/forwarders_ocr2_test.go index 2840658122e..994a1038f12 100644 --- a/integration-tests/smoke/forwarders_ocr2_test.go +++ b/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) diff --git a/integration-tests/smoke/keeper_test.go b/integration-tests/smoke/keeper_test.go index 5b002b7b9ba..3280a7a23a6 100644 --- a/integration-tests/smoke/keeper_test.go +++ b/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) diff --git a/integration-tests/smoke/ocr2_test.go b/integration-tests/smoke/ocr2_test.go index ccc75574f21..ac5ab3fdbb2 100644 --- a/integration-tests/smoke/ocr2_test.go +++ b/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) diff --git a/integration-tests/smoke/ocr_test.go b/integration-tests/smoke/ocr_test.go index dad3b0272fe..75c31e4aa2e 100644 --- a/integration-tests/smoke/ocr_test.go +++ b/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) diff --git a/integration-tests/smoke/runlog_test.go b/integration-tests/smoke/runlog_test.go index 380d0602c06..14b467764ba 100644 --- a/integration-tests/smoke/runlog_test.go +++ b/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") diff --git a/integration-tests/smoke/vrf_test.go b/integration-tests/smoke/vrf_test.go index b4a129ad8dc..d375506a776 100644 --- a/integration-tests/smoke/vrf_test.go +++ b/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) diff --git a/integration-tests/smoke/vrfv2_test.go b/integration-tests/smoke/vrfv2_test.go index 18e9efeae44..1b431397574 100644 --- a/integration-tests/smoke/vrfv2_test.go +++ b/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) diff --git a/integration-tests/smoke/vrfv2plus_test.go b/integration-tests/smoke/vrfv2plus_test.go index 661a1035fa0..14e5a3b1e16 100644 --- a/integration-tests/smoke/vrfv2plus_test.go +++ b/integration-tests/smoke/vrfv2plus_test.go @@ -2,12 +2,13 @@ package smoke import ( "context" - "github.com/kelseyhightower/envconfig" - "github.com/smartcontractkit/chainlink/v2/core/gethwrappers/generated/vrf_v2plus_upgraded_version" "math/big" "testing" "time" + "github.com/kelseyhightower/envconfig" + "github.com/smartcontractkit/chainlink/v2/core/gethwrappers/generated/vrf_v2plus_upgraded_version" + "github.com/ethereum/go-ethereum/common" "github.com/pkg/errors" "github.com/stretchr/testify/require" @@ -35,11 +36,6 @@ func TestVRFv2Plus(t *testing.T) { WithFunding(big.NewFloat(vrfv2PlusConfig.ChainlinkNodeFunding)). 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) @@ -288,12 +284,6 @@ func TestVRFv2PlusMigration(t *testing.T) { WithFunding(big.NewFloat(vrfv2PlusConfig.ChainlinkNodeFunding)). 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) mockETHLinkFeedAddress, err := actions.DeployMockETHLinkFeed(env.ContractDeployer, big.NewInt(vrfv2PlusConfig.LinkNativeFeedResponse))