refactor: NewGraph stops mutating the project · docker/compose@deb2d77 · GitHub
Skip to content

Commit deb2d77

Browse files
ndeloofndeloof
committed
refactor: NewGraph stops mutating the project
Building the dependency graph silently deleted unresolvable optional depends_on references from the caller's project — and since DependsOn is a map, even from projects sharing it. What later readers observed depended on how many graphs had been built before them. The pruning becomes an explicit step, applied once where the project consumed by the engines is established — create and start — before the graph, the dependency waits and the com.docker.compose.depends_on label read the model, using Project.WithoutUnresolvedOptionalDependencies introduced by compose-spec/compose-go#922 (go.mod bumped to the post-merge commit). NewGraph now just skips the edge; required dangling references still flow to its existing errors (naming the profiles that would enable the dependency). No behavior change; epic #14081, Lot 0 step 4 — precondition for a canonical project object across plan phases. Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
1 parent 6d3d68c commit deb2d77

6 files changed

Lines changed: 52 additions & 7 deletions

File tree

‎go.mod‎

Lines changed: 1 addition & 1 deletion

‎go.sum‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -110,8 +110,8 @@ github.com/cloudflare/circl v1.6.3 h1:9GPOhQGF9MCYUeXyMYlqTR6a5gTrgR/fBLXvUgtVcg
110110
github.com/cloudflare/circl v1.6.3/go.mod h1:2eXP6Qfat4O/Yhh8BznvKnJ+uzEoTQ6jVKJRn81BiS4=
111111
github.com/codahale/rfc6979 v0.0.0-20141003034818-6a90f24967eb h1:EDmT6Q9Zs+SbUoc7Ik9EfrFqcylYqgPZ9ANSbTAntnE=
112112
github.com/codahale/rfc6979 v0.0.0-20141003034818-6a90f24967eb/go.mod h1:ZjrT6AXHbDs86ZSdt/osfBi5qfexBrKUdONk989Wnk4=
113-
github.com/compose-spec/compose-go/v2 v2.14.0 h1:uaJeo5B3+OVlu+Rx2qLBcAdXPEUUzm5nQrRiGJafRAQ=
114-
github.com/compose-spec/compose-go/v2 v2.14.0/go.mod h1:ZU6zlcweCZKyiB7BVfCizQT9XmkEIMFE+PRZydVcsZg=
113+
github.com/compose-spec/compose-go/v2 v2.14.1-0.20260825154407-6c1c2d727681 h1:jqD2pgesJ/zRJwgaefxuSuT+/TGUlarjB93Zde40UtI=
114+
github.com/compose-spec/compose-go/v2 v2.14.1-0.20260825154407-6c1c2d727681/go.mod h1:ZU6zlcweCZKyiB7BVfCizQT9XmkEIMFE+PRZydVcsZg=
115115
github.com/containerd/cgroups/v3 v3.1.3 h1:eUNflyMddm18+yrDmZPn3jI7C5hJ9ahABE5q6dyLYXQ=
116116
github.com/containerd/cgroups/v3 v3.1.3/go.mod h1:PKZ2AcWmSBsY/tJUVhtS/rluX0b1uq1GmPO1ElCmbOw=
117117
github.com/containerd/console v1.0.5 h1:R0ymNeydRqH2DmakFNdmjR2k0t7UPuiOV/N/27/qqsc=

‎pkg/compose/create.go‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,11 @@ func (s *composeService) create(ctx context.Context, project *types.Project, opt
7070
options.Services = project.ServiceNames()
7171
}
7272

73+
// resolve the model once: optional depends_on references left dangling by
74+
// profiles or service selection are pruned before anything (dependency
75+
// graph, container labels) reads them
76+
project = project.WithoutUnresolvedOptionalDependencies()
77+
7378
err := project.CheckContainerNameUnicity()
7479
if err != nil {
7580
return err

‎pkg/compose/dependencies.go‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -247,7 +247,11 @@ func (v *Vertex) GetChildren() []*Vertex {
247247
return res
248248
}
249249

250-
// NewGraph returns the dependency graph of the services
250+
// NewGraph returns the dependency graph of the services. It never modifies
251+
// the project: an optional (required: false) dependency on a service absent
252+
// from the model simply contributes no edge; pruning such references from
253+
// the model itself is the caller's explicit decision — see
254+
// Project.WithoutUnresolvedOptionalDependencies.
251255
func NewGraph(project *types.Project, initialStatus ServiceStatus) (*Graph, error) {
252256
graph := &Graph{
253257
lock: sync.RWMutex{},
@@ -258,13 +262,11 @@ func NewGraph(project *types.Project, initialStatus ServiceStatus) (*Graph, erro
258262
graph.AddVertex(s.Name, s.Name, initialStatus)
259263
}
260264

261-
for index, s := range project.Services {
265+
for _, s := range project.Services {
262266
for _, name := range s.GetDependencies() {
263267
err := graph.AddEdge(s.Name, name)
264268
if err != nil {
265269
if !s.DependsOn[name].Required {
266-
delete(s.DependsOn, name)
267-
project.Services[index] = s
268270
continue
269271
}
270272
if api.IsNotFoundError(err) {

‎pkg/compose/dependencies_test.go‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -323,6 +323,40 @@ func TestBuildGraphDependsOn(t *testing.T) {
323323
}
324324
}
325325

326+
// NewGraph must never rewrite the project it reads: pruning unresolved
327+
// optional dependencies is an explicit, separate step. Building a graph any
328+
// number of times leaves the model byte-identical, so what later readers
329+
// (dependency waits, container labels) observe no longer depends on how many
330+
// graphs were built before them.
331+
func TestNewGraphDoesNotMutateProject(t *testing.T) {
332+
project := &types.Project{
333+
Services: types.Services{
334+
"app": {
335+
Name: "app",
336+
DependsOn: types.DependsOnConfig{
337+
"db": {Condition: types.ServiceConditionStarted, Required: true},
338+
"debug": {Condition: types.ServiceConditionStarted, Required: false},
339+
},
340+
},
341+
"db": {Name: "db"},
342+
},
343+
DisabledServices: types.Services{
344+
"debug": {Name: "debug", Profiles: []string{"debug"}},
345+
},
346+
}
347+
348+
for range 2 {
349+
graph, err := NewGraph(project, ServiceStopped)
350+
assert.NilError(t, err)
351+
// the unresolved optional dependency contributes no edge...
352+
assert.Equal(t, len(graph.Vertices["app"].Children), 1)
353+
// ...but stays in the model
354+
assert.Equal(t, len(project.Services["app"].DependsOn), 2)
355+
_, ok := project.Services["app"].DependsOn["debug"]
356+
assert.Check(t, ok)
357+
}
358+
}
359+
326360
func isVertexEqual(a, b Vertex) bool {
327361
childrenEquality := true
328362
for c := range a.Children {

‎pkg/compose/start.go‎

Lines changed: 4 additions & 0 deletions

0 commit comments

Comments
 (0)