Skip to content

Commit f1743e1

Browse files
committed
more tests
Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
1 parent 137c312 commit f1743e1

5 files changed

Lines changed: 2196 additions & 22 deletions

File tree

pkg/compose/create.go

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -134,7 +134,7 @@ func (s *composeService) create(ctx context.Context, project *types.Project, opt
134134
s.emitUntouchedContainerEvents(project, observed, plan)
135135

136136
// Phase 3: Execute the plan
137-
return s.ExecutePlan(ctx, project, plan)
137+
return s.ExecutePlan(ctx, project, plan, observed)
138138
}
139139

140140
// emitUntouchedContainerEvents emits progress events for containers that are
@@ -143,13 +143,7 @@ func (s *composeService) emitUntouchedContainerEvents(project *types.Project, ob
143143
for _, service := range project.Services {
144144
for _, ctr := range observed.Containers[service.Name] {
145145
ctrName := getCanonicalContainerName(ctr)
146-
if _, touched := plan.Operations["stop-container:"+ctrName]; touched {
147-
continue
148-
}
149-
if _, touched := plan.Operations["start-container:"+ctrName]; touched {
150-
continue
151-
}
152-
if _, touched := plan.Operations["create-container:"+ctrName]; touched {
146+
if plan.ContainerTouched(ctrName) {
153147
continue
154148
}
155149
if ctr.State == container.StateRunning {

pkg/compose/observed_state_test.go

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -155,3 +155,62 @@ func TestReconciliationPlanString(t *testing.T) {
155155
`
156156
assert.Equal(t, plan.String(), expected)
157157
}
158+
159+
func TestContainerTouchedMatchesContainerOps(t *testing.T) {
160+
plan := &ReconciliationPlan{
161+
Operations: map[string]*Operation{
162+
"stop-container:web-1": {
163+
ID: "stop-container:web-1",
164+
Type: OpStopContainer,
165+
ContainerOp: &ContainerOperation{
166+
ContainerName: "web-1",
167+
},
168+
},
169+
"create-container:tmpname_web-2": {
170+
ID: "create-container:tmpname_web-2",
171+
Type: OpCreateContainer,
172+
ContainerOp: &ContainerOperation{
173+
ContainerName: "tmpname_web-2",
174+
},
175+
},
176+
"rename-container:web-2": {
177+
ID: "rename-container:web-2",
178+
Type: OpRenameContainer,
179+
RenameOp: &RenameOperation{
180+
CurrentName: "tmpname_web-2",
181+
NewName: "web-2",
182+
},
183+
// No ContainerOp — rename ops use RenameOp
184+
},
185+
"remove-container:db-1": {
186+
ID: "remove-container:db-1",
187+
Type: OpRemoveContainer,
188+
ContainerOp: &ContainerOperation{
189+
ContainerName: "db-1",
190+
},
191+
},
192+
"create-network:mynet": {
193+
ID: "create-network:mynet",
194+
Type: OpCreateNetwork,
195+
NetworkOp: &NetworkOperation{
196+
NetworkKey: "default",
197+
},
198+
},
199+
},
200+
Dependents: map[string][]string{},
201+
}
202+
203+
// Containers with ContainerOp are touched
204+
assert.Assert(t, plan.ContainerTouched("web-1"), "stop op should mark web-1 as touched")
205+
assert.Assert(t, plan.ContainerTouched("tmpname_web-2"), "create op should mark tmpname_web-2 as touched")
206+
assert.Assert(t, plan.ContainerTouched("db-1"), "remove op should mark db-1 as touched")
207+
208+
// Rename ops have RenameOp, not ContainerOp — rename target is NOT touched via ContainerOp
209+
assert.Assert(t, !plan.ContainerTouched("web-2"), "rename op (RenameOp only) should not mark web-2 as touched")
210+
211+
// Network ops are not container ops
212+
assert.Assert(t, !plan.ContainerTouched("mynet"), "network op should not match container name")
213+
214+
// Non-existent container
215+
assert.Assert(t, !plan.ContainerTouched("nonexistent"), "unknown container should not be touched")
216+
}

pkg/compose/plan_executor.go

Lines changed: 30 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -29,8 +29,12 @@ import (
2929
"github.com/compose-spec/compose-go/v2/types"
3030
containerType "github.com/moby/moby/api/types/container"
3131
"github.com/moby/moby/client"
32+
"go.opentelemetry.io/otel"
33+
"go.opentelemetry.io/otel/attribute"
34+
"go.opentelemetry.io/otel/codes"
3235
"golang.org/x/sync/errgroup"
3336

37+
"github.com/docker/compose/v5/internal/tracing"
3438
"github.com/docker/compose/v5/pkg/api"
3539
)
3640

@@ -148,19 +152,15 @@ func (es *executionState) resolveSharedNamespaces(service *types.ServiceConfig)
148152
// ExecutePlan executes a reconciliation plan using DAG traversal similar to
149153
// graphTraversal.visit() in dependencies.go. Operations are executed
150154
// concurrently, respecting dependency ordering.
151-
func (s *composeService) ExecutePlan(ctx context.Context, project *types.Project, plan *ReconciliationPlan) error {
155+
func (s *composeService) ExecutePlan(ctx context.Context, project *types.Project, plan *ReconciliationPlan, observed *ObservedState) error {
152156
if plan.IsEmpty() {
153157
return nil
154158
}
155159

156160
// Pre-populate execution state with existing containers so that
157161
// resolveServiceReferences can find containers for services not
158162
// included in the plan (e.g. --no-deps scenarios).
159-
allContainers, err := s.getContainers(ctx, project.Name, oneOffExclude, true)
160-
if err != nil {
161-
return err
162-
}
163-
state := newExecutionStateFrom(allContainers)
163+
state := newExecutionStateFrom(observed.allContainers())
164164

165165
// Build dependency count map: number of unsatisfied deps per operation.
166166
// The consumer goroutine is single-threaded, so no mutex is needed for depCount.
@@ -219,6 +219,30 @@ func (s *composeService) ExecutePlan(ctx context.Context, project *types.Project
219219
}
220220

221221
func (s *composeService) executeOperation(ctx context.Context, project *types.Project, op *Operation, state *executionState) error {
222+
spanName := op.Type.String()
223+
opts := tracing.SpanOptions{}
224+
if op.ContainerOp != nil {
225+
opts = tracing.ServiceOptions(op.ContainerOp.Service)
226+
}
227+
ctx, span := otel.Tracer("").Start(ctx, spanName, opts.SpanStartOptions()...)
228+
defer span.End()
229+
span.SetAttributes(
230+
attribute.String("operation.id", op.ID),
231+
attribute.String("operation.resource", op.Resource),
232+
attribute.String("operation.reason", op.Reason),
233+
)
234+
235+
err := s.dispatchOperation(ctx, project, op, state)
236+
if err != nil {
237+
span.SetStatus(codes.Error, err.Error())
238+
span.RecordError(err)
239+
} else {
240+
span.SetStatus(codes.Ok, "")
241+
}
242+
return err
243+
}
244+
245+
func (s *composeService) dispatchOperation(ctx context.Context, project *types.Project, op *Operation, state *executionState) error {
222246
switch op.Type {
223247
case OpCreateNetwork:
224248
return s.executePlanCreateNetwork(ctx, project, op, state)

pkg/compose/reconcile.go

Lines changed: 21 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -165,6 +165,17 @@ func (p *ReconciliationPlan) IsEmpty() bool {
165165
return len(p.Operations) == 0
166166
}
167167

168+
// ContainerTouched reports whether the plan contains any operation that
169+
// directly affects the named container (stop, start, create, or remove).
170+
func (p *ReconciliationPlan) ContainerTouched(containerName string) bool {
171+
for _, op := range p.Operations {
172+
if op.ContainerOp != nil && op.ContainerOp.ContainerName == containerName {
173+
return true
174+
}
175+
}
176+
return false
177+
}
178+
168179
// String returns a deterministic, test-friendly dump of the plan.
169180
//
170181
// Operations are listed in topological order, each prefixed by a sequential
@@ -502,9 +513,15 @@ func reconcileNetworks(project *types.Project, observed *ObservedState, plan *Re
502513
Reason: fmt.Sprintf("network %q has been recreated", n.Name),
503514
}
504515

505-
// Start the container (depends on connect)
516+
// Start the container (depends on all connect ops for this container)
506517
startID := fmt.Sprintf("start-container:%s", ctrName)
507-
if _, exists := plan.Operations[startID]; !exists {
518+
if existing, exists := plan.Operations[startID]; exists {
519+
// Container is connected to multiple recreated networks;
520+
// add the new connect op as a dependency.
521+
if !slices.Contains(existing.DependsOn, connectID) {
522+
existing.DependsOn = append(existing.DependsOn, connectID)
523+
}
524+
} else {
508525
plan.Operations[startID] = &Operation{
509526
ID: startID,
510527
Type: OpStartContainer,
@@ -1098,7 +1115,7 @@ func needsRecreate(expected types.ServiceConfig, actual container.Summary, netwo
10981115
return true, "image digest changed", nil
10991116
}
11001117

1101-
if networks != nil && actual.State == container.StateRunning {
1118+
if networks != nil && actual.State == container.StateRunning && actual.NetworkSettings != nil {
11021119
if checkExpectedNetworks(expected, actual, networks) {
11031120
return true, "network configuration changed", nil
11041121
}
@@ -1164,13 +1181,9 @@ func addCascadingRestarts(project *types.Project, observed *ObservedState, plan
11641181
continue
11651182
}
11661183
// This service's dependency is being recreated and has restart: true.
1167-
// Add stop+start ops for its running containers (if not already being recreated/stopped).
1184+
// Add stop+start ops for its running containers (if not already being stopped).
11681185
for _, ctr := range observed.Containers[service.Name] {
11691186
ctrName := getCanonicalContainerName(ctr)
1170-
// Skip if we already have operations for this container
1171-
if _, exists := plan.Operations["recreate-container:"+ctrName]; exists {
1172-
continue
1173-
}
11741187
if _, exists := plan.Operations["stop-container:"+ctrName]; exists {
11751188
continue
11761189
}

0 commit comments

Comments
 (0)