Uploading docker logs on test failures #3236
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,72 @@ | ||
| package dockerutil | ||
|
|
||
| import ( | ||
| "archive/tar" | ||
| "context" | ||
| "fmt" | ||
| "io" | ||
| "path" | ||
| "testing" | ||
|
|
||
| dockertypes "github.com/docker/docker/api/types" | ||
| "github.com/docker/docker/api/types/filters" | ||
| 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() | ||
|
|
||
| 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", testLabel+"="+t.Name()), | ||
| ), | ||
| }) | ||
|
|
||
| if err != nil { | ||
| return nil, fmt.Errorf("failed listing containers: %s", err) | ||
| } | ||
|
|
||
| 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, | ||
| ShowStderr: true, | ||
| }) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("failed reading logs in test cleanup: %s", err) | ||
| } | ||
| return io.ReadAll(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("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 | ||
| } | ||
|
Comment on lines
+62
to
+65
There was a problem hiding this comment. this function is assuming as single file exists with the name provided, if this is no longer a valid assumption to make we can make this function more flexible. |
||
|
|
||
| if hdr.Name != fileName { | ||
| return nil, fmt.Errorf("expected to find %s but found %s", fileName, hdr.Name) | ||
| } | ||
|
|
||
| return io.ReadAll(tr) | ||
| } | ||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,157 @@ | ||
| 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" | ||
| ) | ||
|
|
||
| const ( | ||
| e2eDir = "e2e" | ||
| ) | ||
|
|
||
| // Collect can be used in `t.Cleanup` and will copy all the of the container logs and relevant files | ||
| // into e2e/<test-suite>/<test-name>.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) | ||
| return | ||
| } | ||
|
|
||
| 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 %s: %s", containerDir, err) | ||
| continue | ||
| } | ||
|
|
||
| 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)...) | ||
|
Comment on lines
+76
to
+77
There was a problem hiding this comment. note: this will mean that we are basically trying to get the chain-a files from chain-b and chain-b files from chain-a. They will just be skipped. I chose to make it simpler and just try them all for every container instead of finding only the correct files on a per container basis which would make things a little more complex. I think this doesn't really matter, as this is simply a tear-down function anyway. |
||
|
|
||
| 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 | ||
| } | ||
|
|
||
| const maxAttempts = 100 | ||
| count := 0 | ||
| for ; !strings.HasSuffix(wd, e2eDir) || count > maxAttempts; wd = ospath.Dir(wd) { | ||
| count++ | ||
| } | ||
|
Comment on lines
+146
to
+148
There was a problem hiding this comment. I'm a bit confused by this. Is it possible to run these e2e's when your wd isn't the e2e directory? There was a problem hiding this comment. this is not strictly necessary, I just wanted to make sure that the test errors out if for some reason this case is ever true instead of hanging forever. It's probably safe to remove this entirely but I just went the extra careful route |
||
|
|
||
| // 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 | ||
| } | ||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this means uploading logs failing will not cause a test failure.