mirror of
https://github.com/docker/compose.git
synced 2026-10-12 02:13:08 +00:00
plan: Optional operations fail soft, so plans can carry required:false waits
The executor aborted the whole plan on any node failure, which rules out modeling optional semantics — a dependency declared with required: false must be reported as skipped, not fail the operation, and its dependents must still run. This is a prerequisite for planning the start phase (dependency-condition waits) inside the reconciler. Operation gains Optional: at the walker level, a failing Optional node emits a Skipped event plus a warning and completes successfully, so dependent nodes proceed. It is documented against the pre-existing BestEffort flag, which is narrower (one specific expected error, tolerated inside the operation itself) — the two docs now cross-reference each other so they cannot be confused. Covered by an executor unit test asserting a failing Optional node does not fail the plan and its dependent still executes. Groundwork for the start-in-plan convergence. Signed-off-by: Nicolas De Loof <[email protected]>
This commit is contained in:
@@ -22,6 +22,7 @@ import (
|
||||
"sync"
|
||||
|
||||
"github.com/compose-spec/compose-go/v2/types"
|
||||
"github.com/sirupsen/logrus"
|
||||
"golang.org/x/sync/errgroup"
|
||||
)
|
||||
|
||||
@@ -119,6 +120,14 @@ func (exec *planExecutor) run(ctx context.Context, plan *Plan) error {
|
||||
|
||||
err := exec.executeNode(ctx, node)
|
||||
|
||||
if err != nil && node.Operation.Optional && ctx.Err() == nil {
|
||||
// A best-effort operation reports its failure as a skip and
|
||||
// does not fail the plan: dependent nodes still run.
|
||||
logrus.Warnf("%s (%s): %v", node.Operation.Cause, node.Operation.ResourceID, err)
|
||||
events.On(skippedEvent(nodeEventName(node), err.Error()))
|
||||
err = nil
|
||||
}
|
||||
|
||||
if err == nil {
|
||||
// Emit group done event if this is the last node of a group
|
||||
groups.onNodeDone(node, events)
|
||||
|
||||
@@ -154,17 +154,19 @@ func emitDoneEvent(node *PlanNode, events api.EventProcessor) {
|
||||
|
||||
// emitErrorEvent emits an error event for an ungrouped node.
|
||||
func emitErrorEvent(node *PlanNode, events api.EventProcessor, err error) {
|
||||
op := node.Operation
|
||||
var id string
|
||||
switch {
|
||||
case op.Container != nil:
|
||||
id = getContainerProgressName(*op.Container)
|
||||
default:
|
||||
id = op.ResourceID
|
||||
}
|
||||
events.On(api.Resource{
|
||||
ID: id,
|
||||
ID: nodeEventName(node),
|
||||
Status: api.Error,
|
||||
Text: err.Error(),
|
||||
})
|
||||
}
|
||||
|
||||
// nodeEventName resolves the progress-event identifier of a node: the
|
||||
// container's display name when the operation targets one, the resource ID
|
||||
// otherwise.
|
||||
func nodeEventName(node *PlanNode) string {
|
||||
if node.Operation.Container != nil {
|
||||
return getContainerProgressName(*node.Operation.Container)
|
||||
}
|
||||
return node.Operation.ResourceID
|
||||
}
|
||||
|
||||
@@ -57,6 +57,43 @@ func TestExecutePlanEmpty(t *testing.T) {
|
||||
assert.NilError(t, err)
|
||||
}
|
||||
|
||||
func TestExecutePlanOptionalFailure(t *testing.T) {
|
||||
svc, apiClient := newTestService(t)
|
||||
|
||||
bad := types.NetworkConfig{Name: "test_bad"}
|
||||
good := types.NetworkConfig{Name: "test_good"}
|
||||
project := &types.Project{
|
||||
Name: "test",
|
||||
Networks: types.Networks{"bad": bad, "good": good},
|
||||
}
|
||||
|
||||
apiClient.EXPECT().NetworkCreate(gomock.Any(), "test_bad", gomock.Any()).
|
||||
Return(client.NetworkCreateResult{}, errors.New("boom"))
|
||||
// the dependent node must still run after the optional node failed
|
||||
apiClient.EXPECT().NetworkCreate(gomock.Any(), "test_good", gomock.Any()).
|
||||
Return(client.NetworkCreateResult{ID: "net-good"}, nil)
|
||||
|
||||
plan := &Plan{}
|
||||
optional := plan.addNode(Operation{
|
||||
Type: OpCreateNetwork,
|
||||
ResourceID: "network:bad",
|
||||
Cause: "not found",
|
||||
Name: bad.Name,
|
||||
Network: &bad,
|
||||
Optional: true,
|
||||
}, "")
|
||||
plan.addNode(Operation{
|
||||
Type: OpCreateNetwork,
|
||||
ResourceID: "network:good",
|
||||
Cause: "not found",
|
||||
Name: good.Name,
|
||||
Network: &good,
|
||||
}, "", optional)
|
||||
|
||||
err := svc.executePlan(t.Context(), project, emptyObservedState("test"), plan)
|
||||
assert.NilError(t, err, "an Optional node failure must not fail the plan")
|
||||
}
|
||||
|
||||
func TestExecutePlanCreateNetwork(t *testing.T) {
|
||||
svc, apiClient := newTestService(t)
|
||||
|
||||
|
||||
+13
-5
@@ -103,12 +103,20 @@ type Operation struct {
|
||||
Volume *types.VolumeConfig // for volume operations
|
||||
Timeout *time.Duration // for stop operations
|
||||
CreateNodeID int // for OpRenameContainer: ID of the CreateContainer node whose result to rename
|
||||
// BestEffort marks an operation whose failure must not abort the plan. It is
|
||||
// used for the optional removal of the old network on a rename: if the
|
||||
// network is still in use (by non-Compose containers) the removal is skipped
|
||||
// with a warning instead of failing — the new network already carries a
|
||||
// different name, so the migration does not depend on the old one going away.
|
||||
// BestEffort marks an operation that tolerates one specific, expected
|
||||
// error inside its own execution (today: removing the old network on a
|
||||
// rename skips a conflict when the network is still in use — the new
|
||||
// network already carries a different name, so the migration does not
|
||||
// depend on the old one going away). For an operation whose whole outcome
|
||||
// is optional, see Optional.
|
||||
BestEffort bool
|
||||
|
||||
// Optional marks an operation whose outcome is best-effort at the plan
|
||||
// level: any failure is reported as a Skipped event and a warning instead
|
||||
// of aborting the plan, and dependent nodes still run. Used for waits on
|
||||
// dependencies declared with required: false. For tolerating one specific
|
||||
// error inside an operation, see BestEffort.
|
||||
Optional bool
|
||||
}
|
||||
|
||||
// PlanNode is a single node in the reconciliation DAG. It represents one
|
||||
|
||||
Reference in New Issue
Block a user