Switch to moby for container operations#154
Conversation
Based on my experience doing this for xgo: https://github.com/techknowlogick/xgo/blob/main/runtime_docker.go docker_cli.go has been updated to match: https://github.com/docker/cli/blob/9a471180cb7d39c236d090399a9d362c3f5a8ebd/cli/command/container/opts.go as the newer versions import moby
|
Thanks, I wonder why you have submitted this on github.com
|
|
@ChristopherHX figured I'd commit this upstream first so that it would eventually make its way to the runner. I updated tags to ensure that none of the docker stuff spills over no-docker, and I bumped CI stuff for later go versions. The longer has some reports for files that weren't touched in this PR, but I thought that would be best solved in a different PR to not extend the scope of this PR (I'm happy to fix it in this PR too if that's what you'd prefer) |
Silverwind and I already did some work for this, just need to move this from gitea.com/gitea/runner/act up to here. |
|
Ah lol nice. I missed that one. I'll close this PR:) |
|
I only meant linting, not this main topic. |
|
@ChristopherHX ah, my apologies. I'll resubmit the PR. |
|
@ChristopherHX turns out I just had to re-push the branch and it let me re-open this PR |
|
ah ok, thanks
|
No worries, lmk if I can help with anything:) |
Fixes: https://gitea.com/gitea/runner/issues/859 Migration approach mirrors [actions-oss/act-cli#154](actions-oss/act-cli#154). ### Dependency changes - `github.com/docker/docker` v25.0.15 → **removed** (v29 doesn't exist as docker/docker; the project moved to moby/moby) - `github.com/docker/cli` v25.0.7 → v29.4.3 - `github.com/docker/go-connections` v0.6.0 → v0.7.0 - `github.com/docker/docker-credential-helpers` v0.9.5 → v0.9.6 - `github.com/moby/go-archive` added at v0.2.0 - `github.com/moby/moby/api` added at v1.54.2 - `github.com/moby/moby/client` added at v0.4.1 - `github.com/moby/buildkit` removed (only used `dockerignore.ReadAll`, swapped for `moby/patternmatcher/ignorefile.ReadAll` directly) - `github.com/containerd/errdefs` v0.3.0 → v1.0.0 ### Migration - v28: type aliases moved to their subpackages (`types.{Container,Image,Network,Exec}*` → `container/image/network/...`); deprecated APIs replaced (`ImageInspectWithRaw`, `client.IsErrNotFound`, `archive.CanonicalTarNameForPath`, `opts.ValidateMACAddress`, `ListOpts.GetAll`) - v29: structural client redesign — every `cli.X(ctx, ...)` call switched to options-everywhere/Result-typed signatures, `ContainerExec*` → `Exec*`, `ContainerWait` returns a struct with `Result`/`Error` channels, `Tty`→`TTY`, `Copy*Container` takes options struct, `client.NewClientWithOpts` → `client.New`. `pkg/stdcopy` moved to `moby/moby/api/pkg/stdcopy`. The vendored copy of `cli/command/container/opts.go` was refreshed from cli v29 (now uses `netip.Addr` for IPs, port-set conversion helpers). A small local `parsePlatform` helper centralises the `os/arch[/variant]` parsing previously inlined into multiple call sites. ### Behaviour preservation The migration introduced several behavioural shifts vs the v25 client; all were caught in review and reverted/fixed in follow-up commits: - `GetDockerClient`: cli v29's `Ping(NegotiateAPIVersion: true)` returns errors that the old `NegotiateAPIVersion` silently swallowed. Restored best-effort behaviour (warn-log + continue) so daemons with blocked `_ping` or API < 1.40 keep working. The SSH-helper `client.New` call no longer inherits `client.FromEnv`, matching the old `NewClientWithOpts(WithHost, WithDialContext)` so `DOCKER_API_VERSION`/`DOCKER_TLS_VERIFY` don't leak into the SSH-tunneled client - `parsePlatform`: malformed input now returns an explicit error instead of silently dropping to "no platform constraint" and pulling the host-default architecture. Single-segment (`"linux"`), 4+-segment (`"linux/arm/v7/extra"`), and trailing-slash (`"linux/arm/"`) inputs are all rejected - `LoadDockerAuthConfig`/`LoadDockerAuthConfigs`: `config.LoadDefaultConfigFile(nil)` panics on a malformed config file (it does `fmt.Fprintln` on the nil `io.Writer`). Switched to `config.Load(config.Dir())` so load errors reach the logger and the panic path is gone. Restored the old behaviour of returning `config.Load` and `GetAuthConfig` errors to the caller (the v29 refactor had silently downgraded them to warn-only). A `reference.ParseNormalizedNamed` failure on the image string falls through to the `docker.io` default rather than aborting, since the old string-based hostname extraction was infallible Test assertions also updated for two upstream error-message string shifts (`go-connections` port-range parser; `cli/opts` envfile BOM check). Added unit-test coverage for the new `parsePlatform` helper, locking in the intentional limits (single-segment, 4+-segment, and trailing-slash platforms rejected). --- This PR was written with the help of Claude Opus 4.7 Reviewed-on: https://gitea.com/gitea/runner/pulls/943 Reviewed-by: Lunny Xiao <xiaolunwen@gmail.com> Co-authored-by: silverwind <me@silverwind.io> Co-committed-by: silverwind <me@silverwind.io>
Based on my experience doing this for xgo: https://github.com/techknowlogick/xgo/blob/main/runtime_docker.go
docker_cli.go has been updated to match: https://github.com/docker/cli/blob/9a471180cb7d39c236d090399a9d362c3f5a8ebd/cli/command/container/opts.go as the newer versions import moby