[DX-4146] migrate lib & chip router to moby#2590
Conversation
|
👋 Tofel, thanks for creating this pull request! To help reviewers, please consider creating future PRs as drafts first. This allows you to self-review and make any final changes before notifying the team. Once you're ready, you can mark it as "Ready for review" to request feedback. Thanks! |
There was a problem hiding this comment.
Pull request overview
This PR migrates the lib module and the chip-router component from github.com/docker/docker to github.com/moby/moby, updates related Docker/testcontainers port handling utilities, and bumps the Go toolchain/dependencies to align with the newer Docker stack.
Changes:
- Replace Docker client/types imports with Moby equivalents across
lib/dockerandframework/components/chiprouter. - Standardize exposed port formatting (
NNN/tcp) and remove direct usage ofdocker/go-connections/natinlib/docker/test_env. - Bump Go/tooling versions and refresh
go.mod/go.sum, plus add changesets documenting the migration.
Reviewed changes
Copilot reviewed 41 out of 43 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| lib/utils/cmd/main.go | Switch to stdlib context import |
| lib/logging/log.go | Use embedded testing.T logging method |
| lib/go.sum | Dependency checksum updates for migration/version bumps |
| lib/go.mod | Bump Go version, move to Moby deps, update libs |
| lib/gauntlet/gauntlet.go | Use fmt.Fprintf for env writes |
| lib/docker/test_env/zookeeper.go | Use strings.ReplaceAll |
| lib/docker/test_env/wait_strategy.go | Port type changes; MappedPort/JoinHostPort update |
| lib/docker/test_env/validator_keys_generator.go | Moby imports; image mirror assignment fix |
| lib/docker/test_env/utils.go | Remove nat; expose port helpers return strings |
| lib/docker/test_env/reth_eth2.go | Moby imports; container naming & mirror updates |
| lib/docker/test_env/prysm.go | Moby imports; mirror updates |
| lib/docker/test_env/postgres.go | Remove nat; use NatPort helper |
| lib/docker/test_env/nethermind_eth2.go | Moby imports; naming & mirror updates |
| lib/docker/test_env/nethermind_eth1.go | Moby imports; naming & mirror updates |
| lib/docker/test_env/mockserver.go | Use strings.ReplaceAll |
| lib/docker/test_env/killgrave.go | Moby imports |
| lib/docker/test_env/geth_eth2.go | Moby imports; naming & mirror updates |
| lib/docker/test_env/geth_eth1.go | Moby imports; naming & mirror updates |
| lib/docker/test_env/genesis_generator.go | Moby imports; mirror updates |
| lib/docker/test_env/ethereum_env.go | Assign promoted embedded config fields directly |
| lib/docker/test_env/eth2_init_helpers.go | Moby imports |
| lib/docker/test_env/erigon_eth2.go | Moby imports; naming & mirror updates |
| lib/docker/test_env/erigon_eth1.go | Moby imports; naming & mirror updates |
| lib/docker/test_env/env_component.go | Use fmt.Fprintf with strings.Builder |
| lib/docker/test_env/besu_eth2.go | Moby imports; naming & mirror updates |
| lib/docker/test_env/besu_eth1.go | Moby imports; naming & mirror updates |
| lib/docker/logging.go | Switch to Moby client option types; container list iteration updates |
| lib/docker/docker.go | Use Moby client remove options; updated remove call |
| lib/client/rpc.go | Use canonical "port/tcp" for wait + mapping |
| lib/client/prometheus.go | Call promoted embedded API methods directly |
| lib/blockchain/config.go | Marshal duration via promoted String() |
| lib/.tool-versions | Add golangci-lint; bump Go version entry |
| lib/.changeset/v1.54.8.md | Changeset for Docker→Moby migration |
| framework/examples/chip_router/.tool-versions | Add golangci-lint entry and header comment |
| framework/examples/chip_ingress/.tool-versions | Add golangci-lint entry and header comment |
| framework/components/dockercompose/.tool-versions | Add golangci-lint entry and header comment |
| framework/components/chiprouter/cmd/chip-router/main.go | Add timeouts; use errors.Is for ErrServerClosed |
| framework/components/chiprouter/client/client.go | Simplify HTTP readiness check |
| framework/components/chiprouter/chiprouter.go | Simplify mapped-port error handling blocks |
| framework/components/chiprouter/.changeset/v1.0.4.md | Changeset for Docker→Moby migration |
| .tool-versions | Add golangci-lint; bump Go version entry |
Comments suppressed due to low confidence (3)
lib/docker/logging.go:51
containerInfo.Names[0]typically includes a leading "/"; using it infilepath.Join(directory, fmt.Sprintf("%s.log", containerName))will ignoredirectory(absolute path) and may also create unintended paths. Sanitize the name (trim leading "/" and replace path separators) before using it for filenames.
containerName := containerInfo.Names[0]
if shouldIgnore(logger, containerName) {
return nil
}
logger.Debug().Str("Container", containerName).Msg("Collecting logs")
logOpts := client.ContainerLogsOptions{ShowStdout: true, ShowStderr: true}
logs, err := provider.Client().ContainerLogs(context.Background(), containerInfo.ID, logOpts)
if err != nil {
logger.Error().Err(err).Str("Container", containerName).Msg("failed to fetch logs for container")
return err
}
logFilePath := filepath.Join(directory, fmt.Sprintf("%s.log", containerName))
logFile, err := os.Create(logFilePath)
lib/docker/logging.go:56
logs(fromContainerLogs) andlogFileare never closed. This can leak file descriptors and keep HTTP connections open; adddefer logs.Close()anddefer logFile.Close()after successful creation (and ensure closes still happen on early breaks).
logs, err := provider.Client().ContainerLogs(context.Background(), containerInfo.ID, logOpts)
if err != nil {
logger.Error().Err(err).Str("Container", containerName).Msg("failed to fetch logs for container")
return err
}
logFilePath := filepath.Join(directory, fmt.Sprintf("%s.log", containerName))
logFile, err := os.Create(logFilePath)
if err != nil {
logger.Error().Err(err).Str("Container", containerName).Msg("failed to create container log file")
return err
}
// Parse and write logs
lib/docker/test_env/wait_strategy.go:137
NewWebSocketStrategyaccepts a logger but never stores it in the returned struct;WaitUntilReadythen logs viaw.l, which will be the zero-value logger. Set thelfield in the struct literal (or remove the parameter/field if you don't want logging here).
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
No description provided.