refactor: merge RuntimeVersion and CurrentAPIVersion into RuntimeAPIVersion

After API negotiation, Compose should only rely on the negotiated version
and never use the daemon's raw max version for request shaping. Merge both
functions into a single RuntimeAPIVersion that negotiates via Ping and
returns ClientVersion, erroring if the client reports an empty version
instead of silently falling back to ServerVersion.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Signed-off-by: Guillaume Lours <glours@users.noreply.github.com>
This commit is contained in:
Guillaume Lours 2026-03-31 11:24:24 +02:00 committed by Nicolas De loof
parent ef836856fe
commit 9cab43945a
4 changed files with 30 additions and 81 deletions

View file

@ -216,8 +216,7 @@ type composeService struct {
maxConcurrency int
dryRun bool
runtimeVersion runtimeVersionCache
currentAPIVersion runtimeVersionCache
runtimeAPIVersion runtimeVersionCache
}
// Close releases any connections/resources held by the underlying clients.
@ -502,34 +501,18 @@ type runtimeVersionCache struct {
val string
}
// RuntimeVersion returns the raw API version reported by the daemon.
// Callers that need the negotiated/effective client API version should use
// CurrentAPIVersion instead.
func (s *composeService) RuntimeVersion(ctx context.Context) (string, error) {
s.runtimeVersion.mu.Lock()
defer s.runtimeVersion.mu.Unlock()
if s.runtimeVersion.val != "" {
return s.runtimeVersion.val, nil
}
version, err := s.apiClient().ServerVersion(ctx, client.ServerVersionOptions{})
if err != nil {
return "", err
}
s.runtimeVersion.val = version.APIVersion
return s.runtimeVersion.val, nil
}
// CurrentAPIVersion returns the API version currently used by the Docker client.
// Trigger negotiation first so version-gated request shaping matches the version
// that subsequent API calls will actually use.
// RuntimeAPIVersion returns the negotiated API version that will be used for
// requests to the Docker daemon. It triggers version negotiation via Ping so
// that version-gated request shaping matches the version subsequent API calls
// will actually use.
//
// Lock ordering: currentAPIVersion.mu must be acquired before runtimeVersion.mu
// (via the RuntimeVersion fallback). No code path should reverse this order.
func (s *composeService) CurrentAPIVersion(ctx context.Context) (string, error) {
s.currentAPIVersion.mu.Lock()
defer s.currentAPIVersion.mu.Unlock()
if s.currentAPIVersion.val != "" {
return s.currentAPIVersion.val, nil
// After negotiation, Compose should never rely on features or request attributes
// not defined by this API version, even if the daemon's raw version is higher.
func (s *composeService) RuntimeAPIVersion(ctx context.Context) (string, error) {
s.runtimeAPIVersion.mu.Lock()
defer s.runtimeAPIVersion.mu.Unlock()
if s.runtimeAPIVersion.val != "" {
return s.runtimeAPIVersion.val, nil
}
cli := s.apiClient()
@ -539,17 +522,10 @@ func (s *composeService) CurrentAPIVersion(ctx context.Context) (string, error)
}
version := cli.ClientVersion()
if version != "" {
s.currentAPIVersion.val = version
return s.currentAPIVersion.val, nil
if version == "" {
return "", fmt.Errorf("docker client returned empty version after successful API negotiation")
}
// Defensive fallback for unexpected client implementations or mocks that
// do not populate ClientVersion after a successful negotiated ping.
val, err := s.RuntimeVersion(ctx)
if err != nil {
return "", err
}
s.currentAPIVersion.val = val
return s.currentAPIVersion.val, nil
s.runtimeAPIVersion.val = version
return s.runtimeAPIVersion.val, nil
}

View file

@ -498,7 +498,7 @@ func TestCreateMobyContainer(t *testing.T) {
assert.NilError(t, err)
}
func TestCurrentAPIVersionCachesNegotiation(t *testing.T) {
func TestRuntimeAPIVersionCachesNegotiation(t *testing.T) {
mockCtrl := gomock.NewController(t)
defer mockCtrl.Finish()
@ -508,52 +508,25 @@ func TestCurrentAPIVersionCachesNegotiation(t *testing.T) {
cli.EXPECT().Client().Return(apiClient).AnyTimes()
// Ping reports the server's max API version (1.44), but after negotiation
// the client may settle on a lower version (1.43) — e.g. when the client
// SDK caps at an older version. RuntimeAPIVersion must return the negotiated
// ClientVersion, not the server's raw APIVersion.
apiClient.EXPECT().Ping(gomock.Any(), client.PingOptions{NegotiateAPIVersion: true}).Return(client.PingResult{
APIVersion: "1.44",
}, nil).Times(1)
apiClient.EXPECT().ClientVersion().Return("1.43").Times(1)
version, err := tested.CurrentAPIVersion(t.Context())
version, err := tested.RuntimeAPIVersion(t.Context())
assert.NilError(t, err)
assert.Equal(t, version, "1.43")
version, err = tested.CurrentAPIVersion(t.Context())
version, err = tested.RuntimeAPIVersion(t.Context())
assert.NilError(t, err)
assert.Equal(t, version, "1.43")
}
func TestRuntimeVersionRetriesOnTransientError(t *testing.T) {
mockCtrl := gomock.NewController(t)
defer mockCtrl.Finish()
apiClient := mocks.NewMockAPIClient(mockCtrl)
cli := mocks.NewMockCli(mockCtrl)
tested := &composeService{dockerCli: cli}
cli.EXPECT().Client().Return(apiClient).AnyTimes()
// First call: ServerVersion fails with a transient error
firstCall := apiClient.EXPECT().ServerVersion(gomock.Any(), gomock.Any()).
Return(client.ServerVersionResult{}, context.DeadlineExceeded).Times(1)
// Second call: succeeds
apiClient.EXPECT().ServerVersion(gomock.Any(), gomock.Any()).
Return(client.ServerVersionResult{APIVersion: "1.48"}, nil).Times(1).After(firstCall)
_, err := tested.RuntimeVersion(t.Context())
assert.ErrorIs(t, err, context.DeadlineExceeded)
version, err := tested.RuntimeVersion(t.Context())
assert.NilError(t, err)
assert.Equal(t, version, "1.48")
// Third call returns cached value
version, err = tested.RuntimeVersion(t.Context())
assert.NilError(t, err)
assert.Equal(t, version, "1.48")
}
func TestCurrentAPIVersionRetriesOnTransientError(t *testing.T) {
func TestRuntimeAPIVersionRetriesOnTransientError(t *testing.T) {
mockCtrl := gomock.NewController(t)
defer mockCtrl.Finish()
@ -573,16 +546,16 @@ func TestCurrentAPIVersionRetriesOnTransientError(t *testing.T) {
apiClient.EXPECT().ClientVersion().Return("1.44").Times(1)
// First call should return the transient error
_, err := tested.CurrentAPIVersion(t.Context())
_, err := tested.RuntimeAPIVersion(t.Context())
assert.ErrorIs(t, err, context.DeadlineExceeded)
// Second call should succeed — error was not cached
version, err := tested.CurrentAPIVersion(t.Context())
version, err := tested.RuntimeAPIVersion(t.Context())
assert.NilError(t, err)
assert.Equal(t, version, "1.44")
// Third call should return the cached value without calling Ping again
version, err = tested.CurrentAPIVersion(t.Context())
version, err = tested.RuntimeAPIVersion(t.Context())
assert.NilError(t, err)
assert.Equal(t, version, "1.44")
}

View file

@ -252,7 +252,7 @@ func (s *composeService) getCreateConfigs(ctx context.Context,
if err != nil {
return createConfigs{}, err
}
apiVersion, err := s.CurrentAPIVersion(ctx)
apiVersion, err := s.RuntimeAPIVersion(ctx)
if err != nil {
return createConfigs{}, err
}
@ -899,7 +899,7 @@ func (s *composeService) buildContainerVolumes(
case mount.TypeImage:
// The daemon validates image mounts against the negotiated API version
// from the request path, not the server's own max version.
version, err := s.CurrentAPIVersion(ctx)
version, err := s.RuntimeAPIVersion(ctx)
if err != nil {
return nil, nil, err
}

View file

@ -58,7 +58,7 @@ func (s *composeService) Images(ctx context.Context, projectName string, options
// The daemon validates the platform field in ImageInspect against the
// negotiated API version from the request path, not the server's own max version.
version, err := s.CurrentAPIVersion(ctx)
version, err := s.RuntimeAPIVersion(ctx)
if err != nil {
return nil, err
}