diff --git a/pkg/compose/create.go b/pkg/compose/create.go index 04b8dd9e5..19503f368 100644 --- a/pkg/compose/create.go +++ b/pkg/compose/create.go @@ -1453,6 +1453,14 @@ func (s *composeService) createNetwork(ctx context.Context, n *types.NetworkConf s.events.On(creatingEvent(networkEventName)) if _, err := s.apiClient().NetworkCreate(ctx, n.Name, createOpts); err != nil { + // A concurrent `docker compose up|run` may have created the same network + // between the observed-state snapshot and now. Treat the resulting + // conflict as success rather than failing hard, mirroring the retry the + // previous ensureNetwork performed. + if errdefs.IsConflict(err) { + s.events.On(createdEvent(networkEventName)) + return nil + } s.events.On(errorEvent(networkEventName, err.Error())) return fmt.Errorf("failed to create network %s: %w", n.Name, err) } diff --git a/pkg/compose/executor_test.go b/pkg/compose/executor_test.go index 019deb471..ea0cafbde 100644 --- a/pkg/compose/executor_test.go +++ b/pkg/compose/executor_test.go @@ -385,8 +385,38 @@ func TestExecutePlanRecreateNetwork(t *testing.T) { assert.NilError(t, err) } +// TestExecutePlanCreateNetworkConflictIsSuccess verifies that a NetworkCreate +// conflict (a concurrent up/run created the same network) is treated as success +// rather than surfacing as a hard failure. +func TestExecutePlanCreateNetworkConflictIsSuccess(t *testing.T) { + svc, apiClient := newTestService(t) + + nw := types.NetworkConfig{Name: "test_default"} + project := &types.Project{Name: "test", Networks: types.Networks{"default": nw}} + + apiClient.EXPECT().NetworkCreate(gomock.Any(), "test_default", gomock.Any()). + Return(client.NetworkCreateResult{}, conflictError{}) + + plan := &Plan{} + plan.addNode(Operation{ + Type: OpCreateNetwork, + ResourceID: "network:default", + Cause: "not found", + Name: nw.Name, + Network: &nw, + }, "") + + assert.NilError(t, svc.executePlan(t.Context(), project, emptyObservedState("test"), plan)) +} + // notFoundError implements the errdefs.ErrNotFound interface for test mocks. type notFoundError struct{} func (notFoundError) Error() string { return "not found" } func (notFoundError) NotFound() {} + +// conflictError implements the errdefs.ErrConflict interface for test mocks. +type conflictError struct{} + +func (conflictError) Error() string { return "conflict" } +func (conflictError) Conflict() {} diff --git a/pkg/compose/observed_state.go b/pkg/compose/observed_state.go index 80f4b6b94..10ec46cab 100644 --- a/pkg/compose/observed_state.go +++ b/pkg/compose/observed_state.go @@ -198,13 +198,27 @@ func (s *composeService) discoverUnmanagedNetworks(ctx context.Context, project ID: inspected.Network.ID, Name: inspected.Network.Name, ProjectName: inspected.Network.Labels[api.ProjectLabel], - // ConfigHash intentionally left empty: the network is not owned by - // this project, so we must not treat it as diverged and recreate it. + // Preserve the config-hash only when the network belongs to this + // project (e.g. an older Compose wrote the project label but not the + // network-key label): the reconciler then still detects genuine + // divergence. For a network we don't own the hash is left empty so we + // reuse it untouched rather than recreate it. + ConfigHash: ownedConfigHash(inspected.Network.Labels, project.Name), } } return nil } +// ownedConfigHash returns the config-hash label only when the resource belongs +// to the given project; otherwise it returns "" so the reconciler treats the +// resource as an unmanaged match to be reused untouched. +func ownedConfigHash(labels map[string]string, projectName string) string { + if labels[api.ProjectLabel] != projectName { + return "" + } + return labels[api.ConfigHashLabel] +} + // discoverUnmanagedVolumes augments the observed state with volumes that match a // declared volume by name but carry no compose label — pre-label Compose or // manually created volumes, missed by the label-filtered VolumeList. Each is @@ -231,8 +245,12 @@ func (s *composeService) discoverUnmanagedVolumes(ctx context.Context, project * Name: inspected.Volume.Name, ProjectName: inspected.Volume.Labels[api.ProjectLabel], Driver: inspected.Volume.Driver, - // ConfigHash intentionally left empty: the volume is not owned by - // this project, so we must not treat it as diverged and recreate it. + // Preserve the config-hash only when the volume belongs to this + // project (older Compose wrote the project label but not the + // volume-key label): the reconciler then still detects divergence. + // For a volume we don't own the hash is left empty so we reuse it + // untouched rather than recreate it. + ConfigHash: ownedConfigHash(inspected.Volume.Labels, project.Name), } } return nil diff --git a/pkg/compose/observed_state_test.go b/pkg/compose/observed_state_test.go index 94ca256cb..919408c69 100644 --- a/pkg/compose/observed_state_test.go +++ b/pkg/compose/observed_state_test.go @@ -236,6 +236,40 @@ func TestCollectObservedState_LegacyNetworkMatchedByName(t *testing.T) { assert.Equal(t, obs.ConfigHash, "", "unmanaged match must have an empty config hash") } +// TestCollectObservedState_OwnedNetworkMissingKeyLabelKeepsHash verifies that a +// network owned by this project but missing the network-key label (e.g. written +// by an older Compose) keeps its config-hash, so genuine divergence is still +// detected rather than silently skipped. +func TestCollectObservedState_OwnedNetworkMissingKeyLabelKeepsHash(t *testing.T) { + project := &types.Project{Name: "myproject", Networks: types.Networks{"frontend": {Name: "myproject_frontend"}}} + state, err := collectByNameDiscovery(t, project, func(apiClient *mocks.MockAPIClient) { + apiClient.EXPECT().NetworkInspect(gomock.Any(), "myproject_frontend", gomock.Any()).Return(client.NetworkInspectResult{ + Network: network.Inspect{Network: network.Network{ID: "net1", Name: "myproject_frontend", Labels: map[string]string{ + api.ProjectLabel: "myproject", // owned, but no NetworkLabel + api.ConfigHashLabel: "realhash", + }}}, + }, nil) + }) + assert.NilError(t, err) + assert.Equal(t, state.Networks["frontend"].ConfigHash, "realhash") +} + +// TestCollectObservedState_OwnedVolumeMissingKeyLabelKeepsHash is the volume +// counterpart of the network case above. +func TestCollectObservedState_OwnedVolumeMissingKeyLabelKeepsHash(t *testing.T) { + project := &types.Project{Name: "myproject", Volumes: types.Volumes{"data": {Name: "myproject_data"}}} + state, err := collectByNameDiscovery(t, project, func(apiClient *mocks.MockAPIClient) { + apiClient.EXPECT().VolumeInspect(gomock.Any(), "myproject_data", gomock.Any()).Return(client.VolumeInspectResult{ + Volume: volume.Volume{Name: "myproject_data", Labels: map[string]string{ + api.ProjectLabel: "myproject", // owned, but no VolumeLabel + api.ConfigHashLabel: "realhash", + }}, + }, nil) + }) + assert.NilError(t, err) + assert.Equal(t, state.Volumes["data"].ConfigHash, "realhash") +} + // TestCollectObservedState_ForeignProjectNetworkMatchedByName verifies that a // network owned by another project but matching the declared name is recorded // with an empty ConfigHash (reused untouched) and keeps the foreign project name.