Skip to content

Replace docker/docker with moby/moby client+api modules - #1534

Open
Slach wants to merge 1 commit into
masterfrom
moby-client-migration
Open

Replace docker/docker with moby/moby client+api modules#1534
Slach wants to merge 1 commit into
masterfrom
moby-client-migration

Conversation

@Slach

@Slach Slach commented Sep 5, 2026

Copy link
Copy Markdown
Collaborator

What changed

test/integration/containers.go (build tag integration, the only importer of the Docker SDK in this repo) now uses github.com/moby/moby/client v0.6.0 + github.com/moby/moby/api v1.56.0 instead of github.com/docker/docker v28.5.2+incompatible. github.com/docker/go-connections drops to an indirect dependency (the nat port types are no longer used).

grep docker/docker go.mod is now empty.

Why

The github.com/docker/docker module path is frozen at v28.5.2+incompatible and will never get a patched release — security fixes only land in the split-out github.com/moby/moby/{client,api} modules. That leaves four Dependabot alerts permanently open against this repo:

All four are daemon-side bugs (we only ship a client, and only in tests — the dependency is not part of the clickhouse-backup binary, so THIRD_PARTY_NOTICES.md is unaffected), but they cannot be dismissed by bumping the old module path. Moving to the maintained modules removes them.

API differences adapted

Old (docker/docker) New (moby/moby)
container.ListOptions, container.StopOptions, container.RemoveOptions, container.StartOptions, container.LogsOptions, network.ListOptions, network.CreateOptions, volume.ListOptions, volume.CreateOptions, image.PullOptions all moved into the client package as client.Container*Options / client.Network*Options / client.Volume*Options / client.ImagePullOptions
api/types/filters (filters.NewArgs(filters.Arg(...))) client.Filters (make(client.Filters).Add("name", "tc_"))
ContainerList[]container.Summary, NetworkList[]network.Summary, VolumeList.Volumes result wrappers: .Items
ContainerInspect(ctx, id)container.InspectResponse ContainerInspect(ctx, id, client.ContainerInspectOptions{})ContainerInspectResult, payload under .Container
ContainerStop/Remove/Start/Restart, NetworkRemove, VolumeRemove returned error now return (Result, error)
ContainerCreate(ctx, cfg, hostCfg, netCfg, platform, name) ContainerCreate(ctx, client.ContainerCreateOptions{Config, HostConfig, NetworkingConfig, Name})
NetworkRemove(ctx, id) / VolumeRemove(ctx, id, force bool) now take explicit option structs (NetworkRemoveOptions{}, VolumeRemoveOptions{Force: true})
go-connections/nat: nat.Port("8123/tcp"), nat.PortSet, nat.PortMap, nat.PortBinding{HostIP: "0.0.0.0"} api/types/network: network.ParsePort / network.MustParsePort (Port is now an opaque struct, not a string), network.PortSet, network.PortMap, network.PortBinding{HostIP: netip.IPv4Unspecified()} (HostIP is a netip.Addr)
inspect.State.Status, inspect.State.Health.Status were string named types container.ContainerState / container.HealthStatus, need explicit string(...) conversion for concatenation

GetMappedPort now resolves the host IP via netip.Addr.IsValid()/IsUnspecified() instead of comparing against ""/"0.0.0.0".

Verification

  • go run ./cmd/clickhouse-backup --help — OK
  • go vet -tags=integration ./test/integration/... — clean
  • GOFLAGS= make test (unit tests) — all pass
  • Integration, locally on macOS/OrbStack, ClickHouse 26.8:
RUN_TESTS='TestSkipTablesAndSkipTableEngines|TestRestoreMapping|TestTablesCommand|TestTablePatterns' RUN_PARALLEL=4 ./test/integration/run.sh
--- PASS: TestTablesCommand (23.60s)
--- PASS: TestSkipTablesAndSkipTableEngines (32.41s)
--- PASS: TestTablesCommandListParts (10.95s)
--- PASS: TestRestoreMapping (48.03s)
--- PASS: TestTablePatterns (81.21s)
PASS
ok  	test/integration	127.333s

These exercise network create/remove, named volume create/remove, image inspect/pull, container create/start/inspect/healthcheck-poll/logs/stop/remove, and the host port mapping lookup.

Second run, covering the ContainerRestart path (TestChangeReplicationPathIfReplicaExists restarts clickhouse, TestLongListRemote restarts minio), which the first batch did not touch:

RUN_TESTS='TestChangeReplicationPathIfReplicaExists|TestLongListRemote' RUN_PARALLEL=2 ./test/integration/run.sh
--- PASS: TestChangeReplicationPathIfReplicaExists (66.80s)
--- PASS: TestLongListRemote (76.58s)
PASS
ok  	test/integration	126.147s

Both restarts came back healthy on the first attempt (no "restarting again" / "not healthy after" warnings), so ContainerRestart + the pollHealthy inspect loop are confirmed against the new client.

Full local integration run

RUN_PARALLEL=10 ./test/integration/run.sh (macOS/OrbStack, ClickHouse 26.8): 117 PASS, 4 FAIL in 39 min. None of the failures is in container lifecycle code; all happen inside clickhouse-backup running in the container:

  • TestBwLimitEmbeddedGCS, TestEmbeddedGCSOverS3: read /etc/clickhouse-backup/credentials.json: is a directory — no local GCS credentials file, docker bind-mounts a directory instead (same on master locally).
  • TestResumeOperationsAfterRestartCOS: timeout waiting for resumed operation under 10-way load (known flaky, real COS streaming upload).
  • TestWatchScheduleServerMetrics: expect at least 3 scheduled full backups on remote, got 0 under 10-way load.

Isolated rerun RUN_TESTS='TestResumeOperationsAfterRestartCOS|TestWatchScheduleServerMetrics' RUN_PARALLEL=2 ./test/integration/run.sh:

--- PASS: TestWatchScheduleServerMetrics (137.63s)
--- PASS: TestResumeOperationsAfterRestartCOS (262.28s)

🤖 Generated with Claude Code

The github.com/docker/docker module path is frozen at v28.5.2+incompatible
and will never receive patched releases; security fixes now land only in the
split-out github.com/moby/moby/{client,api} modules. Four permanently-open
Dependabot alerts (GHSA-rg2x-37c3-w2rh, GHSA-vp62-88p7-qqf5,
GHSA-x86f-5xw2-fm2r, GHSA-pxq6-2prw-chj9) are daemon-side bugs that cannot be
resolved on the old path, so switch the only importer -
test/integration/containers.go, a test-only dependency - to the new modules.

API differences adapted:
- option structs moved from api/types/* into the client package
  (container.ListOptions -> client.ContainerListOptions, etc.)
- api/types/filters replaced by client.Filters ("make(Filters).Add(...)")
- list/inspect/create calls now return result wrappers
  (ContainerListResult.Items, ContainerInspectResult.Container, ...)
- mutating calls return (Result, error) instead of just error
- ContainerCreate takes a single ContainerCreateOptions struct
- ContainerInspect takes an explicit options argument
- go-connections/nat port types replaced by api/types/network Port/PortSet/
  PortMap; Port is now opaque (ParsePort/MustParsePort) and PortBinding.HostIP
  is a netip.Addr
- container State.Status / Health.Status are now named string types
@coveralls

Copy link
Copy Markdown

Coverage Report for CI Build 33949555037

Coverage increased (+0.02%) to 66.803%

Details

  • Coverage increased (+0.02%) from the base build.
  • Patch coverage: No coverable lines changed in this PR.
  • 11 coverage regressions across 3 files.

Uncovered Changes

No uncovered changes found.

Coverage Regressions

11 previously-covered lines in 3 files lost coverage.

File Lines Losing Coverage Coverage
pkg/clickhouse/clickhouse.go 6 80.22%
pkg/storage/gcs.go 3 54.46%
pkg/storage/object_disk/object_disk.go 2 68.63%

Coverage Stats

Coverage Status
Relevant Lines: 25605
Covered Lines: 17105
Line Coverage: 66.8%
Coverage Strength: 35680.94 hits per line

💛 - Coveralls

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants