From b7699ba408b07d5b920c20021c30d08b1f0ed592 Mon Sep 17 00:00:00 2001 From: Cian Hatton Date: Fri, 3 Mar 2023 11:24:07 +0000 Subject: [PATCH 01/15] uploading docker logs on test failures --- .../e2e-compatibility-workflow-call.yaml | 7 ++ .gitignore | 3 +- e2e/dockerutil/dockerutil.go | 49 ++++++++++ e2e/tests/transfer/base_test.go | 2 + e2e/testsuite/testsuite.go | 95 ++++++++++++++++++- 5 files changed, 152 insertions(+), 4 deletions(-) create mode 100644 e2e/dockerutil/dockerutil.go diff --git a/.github/workflows/e2e-compatibility-workflow-call.yaml b/.github/workflows/e2e-compatibility-workflow-call.yaml index 58c5642a4f4..d4ef60a9e6f 100644 --- a/.github/workflows/e2e-compatibility-workflow-call.yaml +++ b/.github/workflows/e2e-compatibility-workflow-call.yaml @@ -54,3 +54,10 @@ jobs: CHAIN_B_TAG: "${{ matrix.chain-b }}" CHAIN_BINARY: "${{ matrix.chain-binary }}" RELAYER_TYPE: "${{ matrix.relayer-type }}" + - name: Upload logs on failure + uses: actions/upload-artifact@v3 +# if: true + with: + name: "${{ matrix.entrypoint }}-${{ matrix.test }}" + path: e2e/logs + retention-days: 5 diff --git a/.gitignore b/.gitignore index 1844a2347f6..9e56ec2dd97 100644 --- a/.gitignore +++ b/.gitignore @@ -32,6 +32,7 @@ mytestnet coverage.txt profile.out sim_log_file +e2e/logs # Vagrant .vagrant/ @@ -57,4 +58,4 @@ dependency-graph.png # Go go.work -go.work.sum \ No newline at end of file +go.work.sum diff --git a/e2e/dockerutil/dockerutil.go b/e2e/dockerutil/dockerutil.go new file mode 100644 index 00000000000..75a6aa4e621 --- /dev/null +++ b/e2e/dockerutil/dockerutil.go @@ -0,0 +1,49 @@ +package dockerutil + +import ( + "bytes" + "context" + "fmt" + "testing" + + dockertypes "github.com/docker/docker/api/types" + "github.com/docker/docker/api/types/filters" + dockerclient "github.com/docker/docker/client" +) + +func GetTestContainers(t *testing.T, ctx context.Context, dc *dockerclient.Client) ([]dockertypes.Container, error) { + t.Helper() + + testContainers, err := dc.ContainerList(ctx, dockertypes.ContainerListOptions{ + All: true, + Filters: filters.NewArgs( + // see: https://github.com/strangelove-ventures/interchaintest/blob/0bdc194c2aa11aa32479f32b19e1c50304301981/internal/dockerutil/setup.go#L31-L36 + // for the label needed to identify test containers. + filters.Arg("label", "ibc-test="+t.Name()), + ), + }) + + if err != nil { + return nil, fmt.Errorf("failed listing containers: %s", err) + } + + return testContainers, nil +} + +func GetContainerLogs(ctx context.Context, dc *dockerclient.Client, containerName string) ([]byte, error) { + readCloser, err := dc.ContainerLogs(ctx, containerName, dockertypes.ContainerLogsOptions{ + ShowStdout: true, + ShowStderr: true, + }) + if err != nil { + return nil, fmt.Errorf("failed reading logs in test cleanup: %s", err) + } + + b := new(bytes.Buffer) + _, err = b.ReadFrom(readCloser) + if err != nil { + return nil, fmt.Errorf("failed reading logs in test cleanup: %s", err) + } + + return b.Bytes(), nil +} diff --git a/e2e/tests/transfer/base_test.go b/e2e/tests/transfer/base_test.go index d858bc04058..ba9eb6b51d0 100644 --- a/e2e/tests/transfer/base_test.go +++ b/e2e/tests/transfer/base_test.go @@ -94,6 +94,8 @@ func (s *TransferTestSuite) TestMsgTransfer_Succeeds_Nonincentivized() { s.StartRelayer(relayer) }) + t.FailNow() + chainBIBCToken := testsuite.GetIBCToken(chainADenom, channelA.Counterparty.PortID, channelA.Counterparty.ChannelID) t.Run("packets are relayed", func(t *testing.T) { diff --git a/e2e/testsuite/testsuite.go b/e2e/testsuite/testsuite.go index edd4642dcb5..6b297178e25 100644 --- a/e2e/testsuite/testsuite.go +++ b/e2e/testsuite/testsuite.go @@ -4,8 +4,11 @@ import ( "context" "errors" "fmt" + "os" + ospath "path" "strconv" "strings" + "testing" "time" "github.com/cosmos/cosmos-sdk/client" @@ -32,6 +35,7 @@ import ( "google.golang.org/grpc" "google.golang.org/grpc/credentials/insecure" + "github.com/cosmos/ibc-go/e2e/dockerutil" "github.com/cosmos/ibc-go/e2e/relayer" "github.com/cosmos/ibc-go/e2e/semverutil" "github.com/cosmos/ibc-go/e2e/testconfig" @@ -67,6 +71,83 @@ type E2ETestSuite struct { pathNameIndex uint64 } +// getE2EDir finds the e2e directory above the test. +func getE2EDir(t *testing.T) (string, error) { + t.Helper() + + wd, err := os.Getwd() + if err != nil { + return "", err + } + + count := 0 + for ; !strings.HasSuffix(wd, "e2e") || count > 100; wd = ospath.Dir(wd) { + count++ + } + + if count > 100 { + return "", fmt.Errorf("unable to find e2e directory after 100 tries") + } + + return wd, nil +} + +// DumpDockerLogs can be used in `t.Cleanup` and will copy all the of the container logs. +// into e2e//.log. These log files will be uploaded to GH upon test failure. +func (s *E2ETestSuite) DumpDockerLogs(t *testing.T) { + t.Helper() + + if !t.Failed() { + t.Logf("test passed, not uploading logs") + return + } + + t.Logf("writing logs for test: %s", t.Name()) + + ctx := context.TODO() + e2eDir, err := getE2EDir(t) + if err != nil { + t.Logf("failed finding log directory: %s", err) + } + + logsDir := fmt.Sprintf("%s/logs", e2eDir) + + if err := os.MkdirAll(fmt.Sprintf("%s/%s", logsDir, t.Name()), 0750); err != nil { + t.Logf("failed creating logs directory in test cleanup: %s", err) + return + } + + testContainers, err := dockerutil.GetTestContainers(t, ctx, s.DockerClient) + if err != nil { + t.Logf("failed listing containers test cleanup: %s", err) + return + } + + for _, container := range testContainers { + logsBz, err := dockerutil.GetContainerLogs(ctx, s.DockerClient, container.ID) + if err != nil { + t.Logf("failed reading logs in test cleanup: %s", err) + continue + } + + // container will always have an id, by may not have a name. + containerName := container.ID + if len(container.Names) > 0 { + containerName = container.Names[0] + // remove the test name from the container as the folder structure will provide this + // information already. + containerName = strings.TrimRight(containerName, "-"+t.Name()) + } + + logFile := fmt.Sprintf("%s/%s/%s.log", logsDir, t.Name(), containerName) + if err := os.WriteFile(logFile, logsBz, 0750); err != nil { + t.Logf("failed writing log file for container %s in test cleanup: %s", containerName, err) + continue + } + t.Logf("successfully wrote log file %s", logFile) + } +} + // GRPCClients holds a reference to any GRPC clients that are needed by the tests. // These should typically be used for query clients only. If we need to make changes, we should // use E2ETestSuite.BroadcastMessages to broadcast transactions instead. @@ -442,17 +523,25 @@ func (s *E2ETestSuite) AssertPacketRelayed(ctx context.Context, chain *cosmos.Co // test and can be retrieved with GetChains. func (s *E2ETestSuite) createCosmosChains(chainOptions testconfig.ChainOptions) (*cosmos.CosmosChain, *cosmos.CosmosChain) { client, network := interchaintest.DockerSetup(s.T()) + t := s.T() + + // this is intentionally called after the interchaintest.DockerSetup function. The above function registers a + // cleanup task which deletes all containers. By registering a cleanup function afterwards, it is executed first + // this allows us to process the logs before the containers are removed. + t.Cleanup(func() { + s.DumpDockerLogs(t) + }) s.logger = zap.NewExample() s.DockerClient = client s.network = network - logger := zaptest.NewLogger(s.T()) + logger := zaptest.NewLogger(t) numValidators, numFullNodes := getValidatorsAndFullNodes() - chainA := cosmos.NewCosmosChain(s.T().Name(), *chainOptions.ChainAConfig, numValidators, numFullNodes, logger) - chainB := cosmos.NewCosmosChain(s.T().Name(), *chainOptions.ChainBConfig, numValidators, numFullNodes, logger) + chainA := cosmos.NewCosmosChain(t.Name(), *chainOptions.ChainAConfig, numValidators, numFullNodes, logger) + chainB := cosmos.NewCosmosChain(t.Name(), *chainOptions.ChainBConfig, numValidators, numFullNodes, logger) return chainA, chainB } From cc5841542fff3186db67005c8bccc00ce2d3d25c Mon Sep 17 00:00:00 2001 From: Cian Hatton Date: Fri, 3 Mar 2023 11:33:24 +0000 Subject: [PATCH 02/15] continue on failure so that the logs are uploaded --- .github/workflows/e2e-compatibility-workflow-call.yaml | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/.github/workflows/e2e-compatibility-workflow-call.yaml b/.github/workflows/e2e-compatibility-workflow-call.yaml index d4ef60a9e6f..e4398b3b8e1 100644 --- a/.github/workflows/e2e-compatibility-workflow-call.yaml +++ b/.github/workflows/e2e-compatibility-workflow-call.yaml @@ -44,6 +44,8 @@ jobs: with: go-version: 1.19 - name: Run e2e Test + continue-on-error: true + id: e2e_test # provide an id so that it can be referenced in the next step. run: | cd e2e make e2e-test entrypoint=${{ matrix.entrypoint }} test=${{ matrix.test }} @@ -56,8 +58,9 @@ jobs: RELAYER_TYPE: "${{ matrix.relayer-type }}" - name: Upload logs on failure uses: actions/upload-artifact@v3 -# if: true + # we only want to upload logs on test failures. + if: steps.e2e_test.outcome == 'failure' with: name: "${{ matrix.entrypoint }}-${{ matrix.test }}" - path: e2e/logs + path: ibc-go/e2e/logs retention-days: 5 From 02018f1c4e6932eb4aab88c0625efabaf0150328 Mon Sep 17 00:00:00 2001 From: Cian Hatton Date: Fri, 3 Mar 2023 11:51:32 +0000 Subject: [PATCH 03/15] add always() to condition --- .github/workflows/e2e-compatibility-workflow-call.yaml | 6 ++++-- e2e/dockerutil/dockerutil.go | 5 ++++- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/.github/workflows/e2e-compatibility-workflow-call.yaml b/.github/workflows/e2e-compatibility-workflow-call.yaml index e4398b3b8e1..65513bf9309 100644 --- a/.github/workflows/e2e-compatibility-workflow-call.yaml +++ b/.github/workflows/e2e-compatibility-workflow-call.yaml @@ -44,8 +44,10 @@ jobs: with: go-version: 1.19 - name: Run e2e Test + # set continue-on-error to true so that the next steps run even on failure. continue-on-error: true - id: e2e_test # provide an id so that it can be referenced in the next step. + # provide an id so that it can be referenced in the next step. + id: e2e_test run: | cd e2e make e2e-test entrypoint=${{ matrix.entrypoint }} test=${{ matrix.test }} @@ -59,7 +61,7 @@ jobs: - name: Upload logs on failure uses: actions/upload-artifact@v3 # we only want to upload logs on test failures. - if: steps.e2e_test.outcome == 'failure' + if: always() && steps.e2e_test.outcome == 'failure' with: name: "${{ matrix.entrypoint }}-${{ matrix.test }}" path: ibc-go/e2e/logs diff --git a/e2e/dockerutil/dockerutil.go b/e2e/dockerutil/dockerutil.go index 75a6aa4e621..1cfe3a57cd9 100644 --- a/e2e/dockerutil/dockerutil.go +++ b/e2e/dockerutil/dockerutil.go @@ -11,6 +11,9 @@ import ( dockerclient "github.com/docker/docker/client" ) +const testLabel = "ibc-test" + +// GetTestContainers returns all docker containers that have been created by interchain test. func GetTestContainers(t *testing.T, ctx context.Context, dc *dockerclient.Client) ([]dockertypes.Container, error) { t.Helper() @@ -19,7 +22,7 @@ func GetTestContainers(t *testing.T, ctx context.Context, dc *dockerclient.Clien Filters: filters.NewArgs( // see: https://github.com/strangelove-ventures/interchaintest/blob/0bdc194c2aa11aa32479f32b19e1c50304301981/internal/dockerutil/setup.go#L31-L36 // for the label needed to identify test containers. - filters.Arg("label", "ibc-test="+t.Name()), + filters.Arg("label", testLabel+"="+t.Name()), ), }) From 5ad54eaa257cbb3ca4a2ee23696f4859e93210a3 Mon Sep 17 00:00:00 2001 From: Cian Hatton Date: Fri, 3 Mar 2023 12:08:32 +0000 Subject: [PATCH 04/15] adding upload to correct file --- .github/workflows/e2e-compatibility-workflow-call.yaml | 4 +--- .github/workflows/e2e-test-workflow-call.yml | 9 +++++++++ 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/.github/workflows/e2e-compatibility-workflow-call.yaml b/.github/workflows/e2e-compatibility-workflow-call.yaml index 65513bf9309..d9cd4eb7ab5 100644 --- a/.github/workflows/e2e-compatibility-workflow-call.yaml +++ b/.github/workflows/e2e-compatibility-workflow-call.yaml @@ -46,8 +46,6 @@ jobs: - name: Run e2e Test # set continue-on-error to true so that the next steps run even on failure. continue-on-error: true - # provide an id so that it can be referenced in the next step. - id: e2e_test run: | cd e2e make e2e-test entrypoint=${{ matrix.entrypoint }} test=${{ matrix.test }} @@ -61,7 +59,7 @@ jobs: - name: Upload logs on failure uses: actions/upload-artifact@v3 # we only want to upload logs on test failures. - if: always() && steps.e2e_test.outcome == 'failure' + if: ${{ failure() }} with: name: "${{ matrix.entrypoint }}-${{ matrix.test }}" path: ibc-go/e2e/logs diff --git a/.github/workflows/e2e-test-workflow-call.yml b/.github/workflows/e2e-test-workflow-call.yml index 3ed7f572c11..7de7d29241d 100644 --- a/.github/workflows/e2e-test-workflow-call.yml +++ b/.github/workflows/e2e-test-workflow-call.yml @@ -160,6 +160,15 @@ jobs: with: go-version: 1.19 - name: Run e2e Test + # set continue-on-error to true so that the next steps run even on failure. + continue-on-error: true run: | cd e2e make e2e-test entrypoint=${{ matrix.entrypoint }} test=${{ matrix.test }} + - name: Upload logs on failure + uses: actions/upload-artifact@v3 + if: ${{ failure() }} + with: + name: "${{ matrix.entrypoint }}-${{ matrix.test }}" + path: ibc-go/e2e/logs + retention-days: 5 From 35cf22b410b3c33dbd53104b94e1e244b133874b Mon Sep 17 00:00:00 2001 From: Cian Hatton Date: Fri, 3 Mar 2023 12:25:18 +0000 Subject: [PATCH 05/15] trying always with outcome check --- .github/workflows/e2e-test-workflow-call.yml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.github/workflows/e2e-test-workflow-call.yml b/.github/workflows/e2e-test-workflow-call.yml index 7de7d29241d..97ff99bec03 100644 --- a/.github/workflows/e2e-test-workflow-call.yml +++ b/.github/workflows/e2e-test-workflow-call.yml @@ -162,12 +162,13 @@ jobs: - name: Run e2e Test # set continue-on-error to true so that the next steps run even on failure. continue-on-error: true + id: e2e_test run: | cd e2e make e2e-test entrypoint=${{ matrix.entrypoint }} test=${{ matrix.test }} - name: Upload logs on failure uses: actions/upload-artifact@v3 - if: ${{ failure() }} + if: ${{ always() && steps.e2e_test.outcome == 'failure' }} with: name: "${{ matrix.entrypoint }}-${{ matrix.test }}" path: ibc-go/e2e/logs From afdf4d4dbdadd0b5c54bf442846d81bd6ab7eeda Mon Sep 17 00:00:00 2001 From: Cian Hatton Date: Fri, 3 Mar 2023 12:59:08 +0000 Subject: [PATCH 06/15] adding continue on error to tests --- .github/workflows/e2e-test-workflow-call.yml | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/.github/workflows/e2e-test-workflow-call.yml b/.github/workflows/e2e-test-workflow-call.yml index 97ff99bec03..6f2fdb53e7a 100644 --- a/.github/workflows/e2e-test-workflow-call.yml +++ b/.github/workflows/e2e-test-workflow-call.yml @@ -169,7 +169,11 @@ jobs: - name: Upload logs on failure uses: actions/upload-artifact@v3 if: ${{ always() && steps.e2e_test.outcome == 'failure' }} + continue-on-error: true with: name: "${{ matrix.entrypoint }}-${{ matrix.test }}" - path: ibc-go/e2e/logs + path: e2e/logs retention-days: 5 +# - name: Fail test +# if: ${{ always() && steps.e2e_test.outcome == 'failure' }} +# run: exit 1 From 4908af04b9c7ae7c912060b9588b70c96284e756 Mon Sep 17 00:00:00 2001 From: Cian Hatton Date: Fri, 3 Mar 2023 14:10:39 +0000 Subject: [PATCH 07/15] cause test to show up as failed in UI --- .github/workflows/e2e-test-workflow-call.yml | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/.github/workflows/e2e-test-workflow-call.yml b/.github/workflows/e2e-test-workflow-call.yml index 6f2fdb53e7a..8db63daf620 100644 --- a/.github/workflows/e2e-test-workflow-call.yml +++ b/.github/workflows/e2e-test-workflow-call.yml @@ -174,6 +174,8 @@ jobs: name: "${{ matrix.entrypoint }}-${{ matrix.test }}" path: e2e/logs retention-days: 5 -# - name: Fail test -# if: ${{ always() && steps.e2e_test.outcome == 'failure' }} -# run: exit 1 + # this step is required to mark the action as having failed. Using continue-on-error: true above + # allows the log collection to happen, but the UI is still green showing that the test succeeded. + - name: Fail test + if: ${{ always() && steps.e2e_test.outcome == 'failure' }} + run: exit 1 From 55237306b19028f6289536823f8fcf61f8eebfea Mon Sep 17 00:00:00 2001 From: Cian Hatton Date: Fri, 3 Mar 2023 14:15:45 +0000 Subject: [PATCH 08/15] adding docstrings --- e2e/dockerutil/dockerutil.go | 1 + 1 file changed, 1 insertion(+) diff --git a/e2e/dockerutil/dockerutil.go b/e2e/dockerutil/dockerutil.go index 1cfe3a57cd9..71089036676 100644 --- a/e2e/dockerutil/dockerutil.go +++ b/e2e/dockerutil/dockerutil.go @@ -33,6 +33,7 @@ func GetTestContainers(t *testing.T, ctx context.Context, dc *dockerclient.Clien return testContainers, nil } +// GetContainerLogs returns the logs of a container as a byte array. func GetContainerLogs(ctx context.Context, dc *dockerclient.Client, containerName string) ([]byte, error) { readCloser, err := dc.ContainerLogs(ctx, containerName, dockertypes.ContainerLogsOptions{ ShowStdout: true, From a234c57c04b7556224a040505f073ecee829d369 Mon Sep 17 00:00:00 2001 From: Cian Hatton Date: Fri, 3 Mar 2023 14:26:19 +0000 Subject: [PATCH 09/15] try using just failure() instead --- .github/workflows/e2e-test-workflow-call.yml | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/.github/workflows/e2e-test-workflow-call.yml b/.github/workflows/e2e-test-workflow-call.yml index 8db63daf620..671068c5c5c 100644 --- a/.github/workflows/e2e-test-workflow-call.yml +++ b/.github/workflows/e2e-test-workflow-call.yml @@ -161,14 +161,20 @@ jobs: go-version: 1.19 - name: Run e2e Test # set continue-on-error to true so that the next steps run even on failure. - continue-on-error: true +# continue-on-error: true id: e2e_test run: | cd e2e make e2e-test entrypoint=${{ matrix.entrypoint }} test=${{ matrix.test }} - name: Upload logs on failure uses: actions/upload-artifact@v3 - if: ${{ always() && steps.e2e_test.outcome == 'failure' }} + + # if a status function is not used (success(), always() or failure()) the workflow will automatically assume success(). + # this means that if we want to run something when a task failed, we need to use always(). We cannot actually use failure() here + # as the `continue-on-error: true` option means that the task will not technically fail. always() is the only status condition that + # we can use for this purpose. The stored `outcome` will however be 'failure' which we can use to verify the test itself failed. +# if: ${{ always() && steps.e2e_test.outcome == 'failure' }} + if: ${{ failure() }} continue-on-error: true with: name: "${{ matrix.entrypoint }}-${{ matrix.test }}" @@ -176,6 +182,7 @@ jobs: retention-days: 5 # this step is required to mark the action as having failed. Using continue-on-error: true above # allows the log collection to happen, but the UI is still green showing that the test succeeded. - - name: Fail test - if: ${{ always() && steps.e2e_test.outcome == 'failure' }} - run: exit 1 +# - name: Fail test +# if: ${{ always() && steps.e2e_test.outcome == 'failure' }} +# if: ${{ failure() }} +# run: exit 1 From 713bfd7645151dccd0bf12d8b80b62fcb885d867 Mon Sep 17 00:00:00 2001 From: Cian Hatton Date: Fri, 3 Mar 2023 14:47:44 +0000 Subject: [PATCH 10/15] adding field to enable uploading of logs --- .../e2e-compatibility-workflow-call.yaml | 7 +++---- .github/workflows/e2e-test-workflow-call.yml | 21 ++++++------------- .github/workflows/e2e.yaml | 1 + 3 files changed, 10 insertions(+), 19 deletions(-) diff --git a/.github/workflows/e2e-compatibility-workflow-call.yaml b/.github/workflows/e2e-compatibility-workflow-call.yaml index d9cd4eb7ab5..d0fa182c0a2 100644 --- a/.github/workflows/e2e-compatibility-workflow-call.yaml +++ b/.github/workflows/e2e-compatibility-workflow-call.yaml @@ -44,8 +44,6 @@ jobs: with: go-version: 1.19 - name: Run e2e Test - # set continue-on-error to true so that the next steps run even on failure. - continue-on-error: true run: | cd e2e make e2e-test entrypoint=${{ matrix.entrypoint }} test=${{ matrix.test }} @@ -56,11 +54,12 @@ jobs: CHAIN_B_TAG: "${{ matrix.chain-b }}" CHAIN_BINARY: "${{ matrix.chain-binary }}" RELAYER_TYPE: "${{ matrix.relayer-type }}" - - name: Upload logs on failure + - name: Upload logs uses: actions/upload-artifact@v3 # we only want to upload logs on test failures. if: ${{ failure() }} + continue-on-error: true with: name: "${{ matrix.entrypoint }}-${{ matrix.test }}" - path: ibc-go/e2e/logs + path: e2e/logs retention-days: 5 diff --git a/.github/workflows/e2e-test-workflow-call.yml b/.github/workflows/e2e-test-workflow-call.yml index 671068c5c5c..c22c717fe31 100644 --- a/.github/workflows/e2e-test-workflow-call.yml +++ b/.github/workflows/e2e-test-workflow-call.yml @@ -61,6 +61,11 @@ on: required: false type: boolean default: false + upload-logs: + description: "Specify flag to indicate that logs should be uploaded on failure" + required: false + type: boolean + default: false env: REGISTRY: ghcr.io @@ -160,29 +165,15 @@ jobs: with: go-version: 1.19 - name: Run e2e Test - # set continue-on-error to true so that the next steps run even on failure. -# continue-on-error: true id: e2e_test run: | cd e2e make e2e-test entrypoint=${{ matrix.entrypoint }} test=${{ matrix.test }} - name: Upload logs on failure uses: actions/upload-artifact@v3 - - # if a status function is not used (success(), always() or failure()) the workflow will automatically assume success(). - # this means that if we want to run something when a task failed, we need to use always(). We cannot actually use failure() here - # as the `continue-on-error: true` option means that the task will not technically fail. always() is the only status condition that - # we can use for this purpose. The stored `outcome` will however be 'failure' which we can use to verify the test itself failed. -# if: ${{ always() && steps.e2e_test.outcome == 'failure' }} - if: ${{ failure() }} + if: ${{ failure() && inputs.upload-logs }} continue-on-error: true with: name: "${{ matrix.entrypoint }}-${{ matrix.test }}" path: e2e/logs retention-days: 5 - # this step is required to mark the action as having failed. Using continue-on-error: true above - # allows the log collection to happen, but the UI is still green showing that the test succeeded. -# - name: Fail test -# if: ${{ always() && steps.e2e_test.outcome == 'failure' }} -# if: ${{ failure() }} -# run: exit 1 diff --git a/.github/workflows/e2e.yaml b/.github/workflows/e2e.yaml index 9fe4b02cefd..585d4eca11d 100644 --- a/.github/workflows/e2e.yaml +++ b/.github/workflows/e2e.yaml @@ -62,6 +62,7 @@ jobs: secrets: inherit with: build-and-push-docker-image: true + upload-logs: true chain-image: ghcr.io/cosmos/ibc-go-simd chain-a-tag: "${{ needs.determine-image-tag.outputs.simd-tag }}" chain-b-tag: "${{ needs.determine-image-tag.outputs.simd-tag }}" From 4242333c025aaf3982f2d95430ba99eb6351c8e7 Mon Sep 17 00:00:00 2001 From: Cian Hatton Date: Fri, 3 Mar 2023 16:41:21 +0000 Subject: [PATCH 11/15] created separate diagnostics package and collect additional files --- .../e2e-compatibility-workflow-call.yaml | 4 +- .github/workflows/e2e-test-workflow-call.yml | 4 +- .gitignore | 2 +- e2e/dockerutil/dockerutil.go | 29 +++- e2e/testsuite/diagnostics/diagnostics.go | 149 ++++++++++++++++++ e2e/testsuite/testsuite.go | 97 ++---------- 6 files changed, 187 insertions(+), 98 deletions(-) create mode 100644 e2e/testsuite/diagnostics/diagnostics.go diff --git a/.github/workflows/e2e-compatibility-workflow-call.yaml b/.github/workflows/e2e-compatibility-workflow-call.yaml index d0fa182c0a2..fddf2f6ffdc 100644 --- a/.github/workflows/e2e-compatibility-workflow-call.yaml +++ b/.github/workflows/e2e-compatibility-workflow-call.yaml @@ -54,12 +54,12 @@ jobs: CHAIN_B_TAG: "${{ matrix.chain-b }}" CHAIN_BINARY: "${{ matrix.chain-binary }}" RELAYER_TYPE: "${{ matrix.relayer-type }}" - - name: Upload logs + - name: Upload Diagnostics uses: actions/upload-artifact@v3 # we only want to upload logs on test failures. if: ${{ failure() }} continue-on-error: true with: name: "${{ matrix.entrypoint }}-${{ matrix.test }}" - path: e2e/logs + path: e2e/diagnostics retention-days: 5 diff --git a/.github/workflows/e2e-test-workflow-call.yml b/.github/workflows/e2e-test-workflow-call.yml index c22c717fe31..8f45c8c1aa2 100644 --- a/.github/workflows/e2e-test-workflow-call.yml +++ b/.github/workflows/e2e-test-workflow-call.yml @@ -169,11 +169,11 @@ jobs: run: | cd e2e make e2e-test entrypoint=${{ matrix.entrypoint }} test=${{ matrix.test }} - - name: Upload logs on failure + - name: Upload Diagnostics uses: actions/upload-artifact@v3 if: ${{ failure() && inputs.upload-logs }} continue-on-error: true with: name: "${{ matrix.entrypoint }}-${{ matrix.test }}" - path: e2e/logs + path: e2e/diagnostics retention-days: 5 diff --git a/.gitignore b/.gitignore index 9e56ec2dd97..b9c46cafa65 100644 --- a/.gitignore +++ b/.gitignore @@ -32,7 +32,7 @@ mytestnet coverage.txt profile.out sim_log_file -e2e/logs +e2e/diagnostics # Vagrant .vagrant/ diff --git a/e2e/dockerutil/dockerutil.go b/e2e/dockerutil/dockerutil.go index 71089036676..aa7497645bf 100644 --- a/e2e/dockerutil/dockerutil.go +++ b/e2e/dockerutil/dockerutil.go @@ -1,9 +1,11 @@ package dockerutil import ( - "bytes" + "archive/tar" "context" "fmt" + "io" + "path" "testing" dockertypes "github.com/docker/docker/api/types" @@ -42,12 +44,29 @@ func GetContainerLogs(ctx context.Context, dc *dockerclient.Client, containerNam if err != nil { return nil, fmt.Errorf("failed reading logs in test cleanup: %s", err) } + return io.ReadAll(readCloser) +} - b := new(bytes.Buffer) - _, err = b.ReadFrom(readCloser) +// GetFileContentsFromContainer reads the contents of a specific file from a container. +func GetFileContentsFromContainer(ctx context.Context, dc *dockerclient.Client, containerID, absolutePath string) ([]byte, error) { + readCloser, _, err := dc.CopyFromContainer(ctx, containerID, absolutePath) if err != nil { - return nil, fmt.Errorf("failed reading logs in test cleanup: %s", err) + return nil, fmt.Errorf("copying from container: %w", err) + } + + defer readCloser.Close() + + fileName := path.Base(absolutePath) + tr := tar.NewReader(readCloser) + + hdr, err := tr.Next() + if err != nil { + return nil, err + } + + if hdr.Name != fileName { + return nil, fmt.Errorf("expected to find %s but found %s", fileName, hdr.Name) } - return b.Bytes(), nil + return io.ReadAll(tr) } diff --git a/e2e/testsuite/diagnostics/diagnostics.go b/e2e/testsuite/diagnostics/diagnostics.go new file mode 100644 index 00000000000..317316e1f13 --- /dev/null +++ b/e2e/testsuite/diagnostics/diagnostics.go @@ -0,0 +1,149 @@ +package diagnostics + +import ( + "context" + "fmt" + "os" + ospath "path" + "strings" + "testing" + + dockertypes "github.com/docker/docker/api/types" + dockerclient "github.com/docker/docker/client" + "github.com/strangelove-ventures/interchaintest/v7/ibc" + + "github.com/cosmos/ibc-go/e2e/dockerutil" + "github.com/cosmos/ibc-go/e2e/testconfig" +) + +// Collect can be used in `t.Cleanup` and will copy all the of the container logs and relevant files +// into e2e//.log. These log files will be uploaded to GH upon test failure. +func Collect(t *testing.T, dc *dockerclient.Client, cfg testconfig.ChainOptions) { + t.Helper() + + if !t.Failed() { + t.Logf("test passed, not uploading logs") + return + } + + t.Logf("writing logs for test: %s", t.Name()) + + ctx := context.TODO() + e2eDir, err := getE2EDir(t) + if err != nil { + t.Logf("failed finding log directory: %s", err) + } + + logsDir := fmt.Sprintf("%s/diagnostics", e2eDir) + + if err := os.MkdirAll(fmt.Sprintf("%s/%s", logsDir, t.Name()), 0750); err != nil { + t.Logf("failed creating logs directory in test cleanup: %s", err) + return + } + + testContainers, err := dockerutil.GetTestContainers(t, ctx, dc) + if err != nil { + t.Logf("failed listing containers test cleanup: %s", err) + return + } + + for _, container := range testContainers { + containerName := getContainerName(t, container) + containerDir := fmt.Sprintf("%s/%s/%s", logsDir, t.Name(), containerName) + if err := os.MkdirAll(containerDir, 0750); err != nil { + t.Logf("failed creating logs directory for container %sL %s", containerDir, err) + return + } + + logsBz, err := dockerutil.GetContainerLogs(ctx, dc, container.ID) + if err != nil { + t.Logf("failed reading logs in test cleanup: %s", err) + continue + } + + logFile := fmt.Sprintf("%s/%s.log", containerDir, containerName) + if err := os.WriteFile(logFile, logsBz, 0750); err != nil { + t.Logf("failed writing log file for container %s in test cleanup: %s", containerName, err) + continue + } + + t.Logf("successfully wrote log file %s", logFile) + diagnosticFiles := chainDiagnosticAbsoluteFilePaths(*cfg.ChainAConfig) + diagnosticFiles = append(diagnosticFiles, chainDiagnosticAbsoluteFilePaths(*cfg.ChainBConfig)...) + + for _, absoluteFilePathInContainer := range diagnosticFiles { + localFilePath := ospath.Join(containerDir, ospath.Base(absoluteFilePathInContainer)) + if err := fetchAndWriteDiagnosticsFile(ctx, dc, container.ID, localFilePath, absoluteFilePathInContainer); err != nil { + t.Logf("failed to fetch and write file %s for container %s in test cleanup: %s", absoluteFilePathInContainer, containerName, err) + continue + } + t.Logf("successfully wrote diagnostics file %s", absoluteFilePathInContainer) + } + } +} + +// getContainerName returns a either the ID of the container or stripped down human-readable +// version of the name if the name is non-empty. +// +// Note: You should still always use the ID when interacting with the docker client. +func getContainerName(t *testing.T, container dockertypes.Container) string { + t.Helper() + // container will always have an id, by may not have a name. + containerName := container.ID + if len(container.Names) > 0 { + containerName = container.Names[0] + // remove the test name from the container as the folder structure will provide this + // information already. + containerName = strings.TrimRight(containerName, "-"+t.Name()) + containerName = strings.TrimLeft(containerName, "/") + } + return containerName +} + +// fetchAndWriteDiagnosticsFile fetches the contents of a single file from the given container id and writes +// the contents of the file to a local path provided. +func fetchAndWriteDiagnosticsFile(ctx context.Context, dc *dockerclient.Client, containerID, localPath, absoluteFilePathInContainer string) error { + fileBz, err := dockerutil.GetFileContentsFromContainer(ctx, dc, containerID, absoluteFilePathInContainer) + if err != nil { + return err + } + + filePath := ospath.Join(localPath) + if err := os.WriteFile(filePath, fileBz, 0750); err != nil { + return err + } + + return nil +} + +// chainDiagnosticAbsoluteFilePaths returns a slice of absolute file paths (in the containers) which are the files that should be +// copied locally when fetching diagnostics. +func chainDiagnosticAbsoluteFilePaths(cfg ibc.ChainConfig) []string { + return []string{ + fmt.Sprintf("/var/cosmos-chain/%s/config/genesis.json", cfg.Name), + fmt.Sprintf("/var/cosmos-chain/%s/config/app.toml", cfg.Name), + fmt.Sprintf("/var/cosmos-chain/%s/config/config.toml", cfg.Name), + fmt.Sprintf("/var/cosmos-chain/%s/config/client.toml", cfg.Name), + } +} + +// getE2EDir finds the e2e directory above the test. +func getE2EDir(t *testing.T) (string, error) { + t.Helper() + + wd, err := os.Getwd() + if err != nil { + return "", err + } + + count := 0 + for ; !strings.HasSuffix(wd, "e2e") || count > 100; wd = ospath.Dir(wd) { + count++ + } + + if count > 100 { + return "", fmt.Errorf("unable to find e2e directory after 100 tries") + } + + return wd, nil +} diff --git a/e2e/testsuite/testsuite.go b/e2e/testsuite/testsuite.go index 6b297178e25..965e4177c86 100644 --- a/e2e/testsuite/testsuite.go +++ b/e2e/testsuite/testsuite.go @@ -4,11 +4,8 @@ import ( "context" "errors" "fmt" - "os" - ospath "path" "strconv" "strings" - "testing" "time" "github.com/cosmos/cosmos-sdk/client" @@ -35,10 +32,10 @@ import ( "google.golang.org/grpc" "google.golang.org/grpc/credentials/insecure" - "github.com/cosmos/ibc-go/e2e/dockerutil" "github.com/cosmos/ibc-go/e2e/relayer" "github.com/cosmos/ibc-go/e2e/semverutil" "github.com/cosmos/ibc-go/e2e/testconfig" + "github.com/cosmos/ibc-go/e2e/testsuite/diagnostics" "github.com/cosmos/ibc-go/e2e/testvalues" controllertypes "github.com/cosmos/ibc-go/v7/modules/apps/27-interchain-accounts/controller/types" feetypes "github.com/cosmos/ibc-go/v7/modules/apps/29-fee/types" @@ -71,83 +68,6 @@ type E2ETestSuite struct { pathNameIndex uint64 } -// getE2EDir finds the e2e directory above the test. -func getE2EDir(t *testing.T) (string, error) { - t.Helper() - - wd, err := os.Getwd() - if err != nil { - return "", err - } - - count := 0 - for ; !strings.HasSuffix(wd, "e2e") || count > 100; wd = ospath.Dir(wd) { - count++ - } - - if count > 100 { - return "", fmt.Errorf("unable to find e2e directory after 100 tries") - } - - return wd, nil -} - -// DumpDockerLogs can be used in `t.Cleanup` and will copy all the of the container logs. -// into e2e//.log. These log files will be uploaded to GH upon test failure. -func (s *E2ETestSuite) DumpDockerLogs(t *testing.T) { - t.Helper() - - if !t.Failed() { - t.Logf("test passed, not uploading logs") - return - } - - t.Logf("writing logs for test: %s", t.Name()) - - ctx := context.TODO() - e2eDir, err := getE2EDir(t) - if err != nil { - t.Logf("failed finding log directory: %s", err) - } - - logsDir := fmt.Sprintf("%s/logs", e2eDir) - - if err := os.MkdirAll(fmt.Sprintf("%s/%s", logsDir, t.Name()), 0750); err != nil { - t.Logf("failed creating logs directory in test cleanup: %s", err) - return - } - - testContainers, err := dockerutil.GetTestContainers(t, ctx, s.DockerClient) - if err != nil { - t.Logf("failed listing containers test cleanup: %s", err) - return - } - - for _, container := range testContainers { - logsBz, err := dockerutil.GetContainerLogs(ctx, s.DockerClient, container.ID) - if err != nil { - t.Logf("failed reading logs in test cleanup: %s", err) - continue - } - - // container will always have an id, by may not have a name. - containerName := container.ID - if len(container.Names) > 0 { - containerName = container.Names[0] - // remove the test name from the container as the folder structure will provide this - // information already. - containerName = strings.TrimRight(containerName, "-"+t.Name()) - } - - logFile := fmt.Sprintf("%s/%s/%s.log", logsDir, t.Name(), containerName) - if err := os.WriteFile(logFile, logsBz, 0750); err != nil { - t.Logf("failed writing log file for container %s in test cleanup: %s", containerName, err) - continue - } - t.Logf("successfully wrote log file %s", logFile) - } -} - // GRPCClients holds a reference to any GRPC clients that are needed by the tests. // These should typically be used for query clients only. If we need to make changes, we should // use E2ETestSuite.BroadcastMessages to broadcast transactions instead. @@ -525,13 +445,6 @@ func (s *E2ETestSuite) createCosmosChains(chainOptions testconfig.ChainOptions) client, network := interchaintest.DockerSetup(s.T()) t := s.T() - // this is intentionally called after the interchaintest.DockerSetup function. The above function registers a - // cleanup task which deletes all containers. By registering a cleanup function afterwards, it is executed first - // this allows us to process the logs before the containers are removed. - t.Cleanup(func() { - s.DumpDockerLogs(t) - }) - s.logger = zap.NewExample() s.DockerClient = client s.network = network @@ -542,6 +455,14 @@ func (s *E2ETestSuite) createCosmosChains(chainOptions testconfig.ChainOptions) chainA := cosmos.NewCosmosChain(t.Name(), *chainOptions.ChainAConfig, numValidators, numFullNodes, logger) chainB := cosmos.NewCosmosChain(t.Name(), *chainOptions.ChainBConfig, numValidators, numFullNodes, logger) + + // this is intentionally called after the interchaintest.DockerSetup function. The above function registers a + // cleanup task which deletes all containers. By registering a cleanup function afterwards, it is executed first + // this allows us to process the logs before the containers are removed. + t.Cleanup(func() { + diagnostics.Collect(t, s.DockerClient, chainOptions) + }) + return chainA, chainB } From 4981c15688d2ca77a3c5f1245aad1270f96f38fe Mon Sep 17 00:00:00 2001 From: Cian Hatton Date: Mon, 6 Mar 2023 09:23:22 +0000 Subject: [PATCH 12/15] continuing on logs error --- e2e/testsuite/diagnostics/diagnostics.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/e2e/testsuite/diagnostics/diagnostics.go b/e2e/testsuite/diagnostics/diagnostics.go index 317316e1f13..94a0d11cc92 100644 --- a/e2e/testsuite/diagnostics/diagnostics.go +++ b/e2e/testsuite/diagnostics/diagnostics.go @@ -51,8 +51,8 @@ func Collect(t *testing.T, dc *dockerclient.Client, cfg testconfig.ChainOptions) containerName := getContainerName(t, container) containerDir := fmt.Sprintf("%s/%s/%s", logsDir, t.Name(), containerName) if err := os.MkdirAll(containerDir, 0750); err != nil { - t.Logf("failed creating logs directory for container %sL %s", containerDir, err) - return + t.Logf("failed creating logs directory for container %s: %s", containerDir, err) + continue } logsBz, err := dockerutil.GetContainerLogs(ctx, dc, container.ID) From 252313ae61160384127b3137e0c2e1aa920940c6 Mon Sep 17 00:00:00 2001 From: Cian Hatton Date: Mon, 6 Mar 2023 09:27:14 +0000 Subject: [PATCH 13/15] removing FailNow invocation --- e2e/tests/transfer/base_test.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/e2e/tests/transfer/base_test.go b/e2e/tests/transfer/base_test.go index ba9eb6b51d0..d858bc04058 100644 --- a/e2e/tests/transfer/base_test.go +++ b/e2e/tests/transfer/base_test.go @@ -94,8 +94,6 @@ func (s *TransferTestSuite) TestMsgTransfer_Succeeds_Nonincentivized() { s.StartRelayer(relayer) }) - t.FailNow() - chainBIBCToken := testsuite.GetIBCToken(chainADenom, channelA.Counterparty.PortID, channelA.Counterparty.ChannelID) t.Run("packets are relayed", func(t *testing.T) { From db808de6136b347810f846b4b79eceadcbdd5276 Mon Sep 17 00:00:00 2001 From: Cian Hatton Date: Mon, 6 Mar 2023 09:31:33 +0000 Subject: [PATCH 14/15] adding constant for max attempts --- e2e/testsuite/diagnostics/diagnostics.go | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/e2e/testsuite/diagnostics/diagnostics.go b/e2e/testsuite/diagnostics/diagnostics.go index 94a0d11cc92..6226dd4a339 100644 --- a/e2e/testsuite/diagnostics/diagnostics.go +++ b/e2e/testsuite/diagnostics/diagnostics.go @@ -16,6 +16,10 @@ import ( "github.com/cosmos/ibc-go/e2e/testconfig" ) +const ( + e2eDir = "e2e" +) + // Collect can be used in `t.Cleanup` and will copy all the of the container logs and relevant files // into e2e//.log. These log files will be uploaded to GH upon test failure. func Collect(t *testing.T, dc *dockerclient.Client, cfg testconfig.ChainOptions) { @@ -32,6 +36,7 @@ func Collect(t *testing.T, dc *dockerclient.Client, cfg testconfig.ChainOptions) e2eDir, err := getE2EDir(t) if err != nil { t.Logf("failed finding log directory: %s", err) + return } logsDir := fmt.Sprintf("%s/diagnostics", e2eDir) @@ -136,13 +141,16 @@ func getE2EDir(t *testing.T) (string, error) { return "", err } + const maxAttempts = 100 count := 0 - for ; !strings.HasSuffix(wd, "e2e") || count > 100; wd = ospath.Dir(wd) { + for ; !strings.HasSuffix(wd, e2eDir) || count > maxAttempts; wd = ospath.Dir(wd) { count++ } - if count > 100 { - return "", fmt.Errorf("unable to find e2e directory after 100 tries") + // arbitrary value to avoid getting stuck in an infinite loop if this is called + // in a context where the e2e directory does not exist. + if count > maxAttempts { + return "", fmt.Errorf("unable to find e2e directory after %d tries", maxAttempts) } return wd, nil From da0a192730b7532d4b7129063fa71789a4a8c401 Mon Sep 17 00:00:00 2001 From: Cian Hatton Date: Mon, 6 Mar 2023 15:19:39 +0000 Subject: [PATCH 15/15] adding entry to readme to outline log collection --- e2e/README.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/e2e/README.md b/e2e/README.md index 56463c219be..d6a03e4485c 100644 --- a/e2e/README.md +++ b/e2e/README.md @@ -354,6 +354,12 @@ json matrix files under .github/compatibility-test-matrices and is equivalent to This issue doesn't seem to occur on other operating systems. +### Accessing Logs + +- When a test fails in GitHub. The logs of the test will be uploaded (viewable in the summary page of the workflow). Note: There + may be some discrepancy in the logs collected and the output of interchain test. The containers may run for a some + time after the logs are collected, resulting in the displayed logs to differ slightly. + ## Importable Workflow This repository contains an [importable workflow](https://github.com/cosmos/ibc-go/blob/bc963bcfd115a0e06b8196b114496db5ea011247/.github/workflows/e2e-compatibility-workflow-call.yaml) that can be used from any other repository to test chain upgrades. The workflow