diff --git a/pkg/compose/compose.go b/pkg/compose/compose.go index e6fce9be3..33ed81af9 100644 --- a/pkg/compose/compose.go +++ b/pkg/compose/compose.go @@ -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 } diff --git a/pkg/compose/convergence_test.go b/pkg/compose/convergence_test.go index 7012576eb..27d312c0c 100644 --- a/pkg/compose/convergence_test.go +++ b/pkg/compose/convergence_test.go @@ -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") } diff --git a/pkg/compose/create.go b/pkg/compose/create.go index ce7eced99..78db3d0fd 100644 --- a/pkg/compose/create.go +++ b/pkg/compose/create.go @@ -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 } diff --git a/pkg/compose/images.go b/pkg/compose/images.go index 24d37c77f..d97f18122 100644 --- a/pkg/compose/images.go +++ b/pkg/compose/images.go @@ -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 }