Skip to content

Commit 30e3da0

Browse files
argo-cd-cherry-pick-bot[bot]Joibelclaude
authored
fix: complete orphaned TaskGroup nodes stuck Running. Fixes #16450 (cherry-pick #16454 for 4.0) (#16465)
Signed-off-by: Alan Clucas <alan@clucas.org> Co-authored-by: Alan Clucas <alan@clucas.org> Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
1 parent 445b84e commit 30e3da0

2 files changed

Lines changed: 109 additions & 5 deletions

File tree

workflow/controller/dag.go

Lines changed: 51 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,11 @@ type dagContext struct {
5757
// are only computed once per operation
5858
dependsLogic map[string]string
5959

60+
// taskGroupsToComplete collects the names of TaskGroup nodes that assessDAGPhase
61+
// found stuck Running with all of their children fulfilled, mapped to the phase
62+
// they should complete with. executeDAG marks them once assessment is done.
63+
taskGroupsToComplete map[string]wfv1.NodePhase
64+
6065
// used for logging in the dag
6166
log logging.Logger
6267
}
@@ -169,11 +174,23 @@ func (d *dagContext) assessDAGPhase(ctx context.Context, targetTasks []string, n
169174
branchPhase := curr.phase
170175

171176
if !node.Fulfilled() {
172-
return wfv1.NodeRunning, nil
173-
}
174-
175-
// Only overwrite the branchPhase if this node completed. (If it didn't we can just inherit our parent's branchPhase).
176-
if node.Completed() {
177+
// A fan-out TaskGroup can be left Running with every expanded child
178+
// already fulfilled, for example when a retry resets the group but never
179+
// re-runs it because its dependents have already completed. executeDAGTask
180+
// only visits unfulfilled tasks, so it never revisits such a group, which
181+
// would then hold the DAG Running forever. Complete it from its children
182+
// instead of blocking here.
183+
groupPhase, ok := completableTaskGroupPhase(node, nodes)
184+
if !ok {
185+
return wfv1.NodeRunning, nil
186+
}
187+
if d.taskGroupsToComplete == nil {
188+
d.taskGroupsToComplete = make(map[string]wfv1.NodePhase)
189+
}
190+
d.taskGroupsToComplete[node.Name] = groupPhase
191+
branchPhase = groupPhase
192+
} else if node.Completed() {
193+
// Only overwrite the branchPhase if this node completed. (If it didn't we can just inherit our parent's branchPhase).
177194
branchPhase = node.Phase
178195
}
179196

@@ -227,6 +244,28 @@ func (d *dagContext) assessDAGPhase(ctx context.Context, targetTasks []string, n
227244
return result, nil
228245
}
229246

247+
// completableTaskGroupPhase reports whether node is a TaskGroup that is not yet
248+
// fulfilled even though all of its expanded children are, and if so the phase it
249+
// should complete with (Succeeded unless a child failed or errored, matching the
250+
// aggregation executeDAGTask uses). Such a group is never revisited by
251+
// executeDAGTask, so it must be completed during DAG assessment.
252+
func completableTaskGroupPhase(node *wfv1.NodeStatus, nodes wfv1.Nodes) (wfv1.NodePhase, bool) {
253+
if node.Type != wfv1.NodeTypeTaskGroup || len(node.Children) == 0 {
254+
return "", false
255+
}
256+
phase := wfv1.NodeSucceeded
257+
for _, childID := range node.Children {
258+
child, err := nodes.Get(childID)
259+
if err != nil || !child.Fulfilled() {
260+
return "", false
261+
}
262+
if child.FailedOrError() {
263+
phase = child.Phase
264+
}
265+
}
266+
return phase, true
267+
}
268+
230269
func (woc *wfOperationCtx) executeDAG(ctx context.Context, nodeName string, tmplCtx *templateresolution.TemplateContext, templateScope string, tmpl *wfv1.Template, orgTmpl wfv1.TemplateReferenceHolder, opts *executeTemplateOpts) (*wfv1.NodeStatus, error) {
231270

232271
node, err := woc.wf.GetNodeByName(nodeName)
@@ -320,6 +359,13 @@ func (woc *wfOperationCtx) executeDAG(ctx context.Context, nodeName string, tmpl
320359
return nil, err
321360
}
322361

362+
// Complete any orphaned TaskGroups that assessment found stuck Running with all
363+
// children fulfilled. Done regardless of the overall DAG phase so a group is
364+
// healed even while other tasks are still legitimately running.
365+
for name, phase := range dagCtx.taskGroupsToComplete {
366+
woc.markNodePhase(ctx, name, phase)
367+
}
368+
323369
switch dagPhase {
324370
case wfv1.NodeRunning:
325371
return node, nil

workflow/controller/dag_test.go

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4589,3 +4589,61 @@ func TestDAGSkippedInlineExpressionFallback(t *testing.T) {
45894589
require.NotNil(t, in.Value)
45904590
assert.Equal(t, "inline-fallback", in.Value.String())
45914591
}
4592+
4593+
// TestDAGOrphanedTaskGroupCompletes verifies that a fan-out TaskGroup left Running
4594+
// with every expanded child already fulfilled — the state a retry can produce when
4595+
// it resets the group but never re-runs it, because its dependents already
4596+
// completed — does not hold the DAG Running forever. executeDAGTask never revisits
4597+
// such a group, so the controller must complete it from its children during DAG
4598+
// assessment.
4599+
func TestDAGOrphanedTaskGroupCompletes(t *testing.T) {
4600+
wf := wfv1.MustUnmarshalWorkflow(`
4601+
apiVersion: argoproj.io/v1alpha1
4602+
kind: Workflow
4603+
metadata:
4604+
name: dag-orphaned-taskgroup
4605+
namespace: argo
4606+
spec:
4607+
entrypoint: main
4608+
templates:
4609+
- name: main
4610+
dag:
4611+
tasks:
4612+
- name: fanout
4613+
template: echo
4614+
withItems: [a, b]
4615+
- name: leaf
4616+
template: echo
4617+
depends: fanout
4618+
- name: echo
4619+
container:
4620+
image: alpine:3.23
4621+
command: [sh, -c, "exit 0"]
4622+
`)
4623+
wf.Status.Phase = wfv1.WorkflowRunning
4624+
wf.Status.StartedAt = metav1.Now()
4625+
4626+
root := wf.Name
4627+
fanout := root + ".fanout"
4628+
child0 := root + ".fanout(0:a)"
4629+
child1 := root + ".fanout(1:b)"
4630+
leaf := root + ".leaf"
4631+
id := wf.NodeID
4632+
4633+
// The fan-out TaskGroup is Running while its children and the downstream leaf
4634+
// have all Succeeded — an orphaned group that nothing will otherwise complete.
4635+
wf.Status.Nodes = wfv1.Nodes{
4636+
id(root): {ID: id(root), Name: root, Type: wfv1.NodeTypeDAG, Phase: wfv1.NodeRunning, TemplateName: "main", Children: []string{id(fanout)}},
4637+
id(fanout): {ID: id(fanout), Name: fanout, Type: wfv1.NodeTypeTaskGroup, Phase: wfv1.NodeRunning, BoundaryID: id(root), TemplateName: "echo", Children: []string{id(child0), id(child1)}},
4638+
id(child0): {ID: id(child0), Name: child0, Type: wfv1.NodeTypePod, Phase: wfv1.NodeSucceeded, BoundaryID: id(root), TemplateName: "echo", Children: []string{id(leaf)}},
4639+
id(child1): {ID: id(child1), Name: child1, Type: wfv1.NodeTypePod, Phase: wfv1.NodeSucceeded, BoundaryID: id(root), TemplateName: "echo", Children: []string{id(leaf)}},
4640+
id(leaf): {ID: id(leaf), Name: leaf, Type: wfv1.NodeTypePod, Phase: wfv1.NodeSucceeded, BoundaryID: id(root), TemplateName: "echo"},
4641+
}
4642+
4643+
ctx := logging.TestContext(t.Context())
4644+
woc := newWoc(ctx, *wf)
4645+
woc.operate(ctx)
4646+
4647+
assert.Equal(t, wfv1.NodeSucceeded, woc.wf.Status.Nodes[id(fanout)].Phase, "orphaned TaskGroup should be completed from its children")
4648+
assert.Equal(t, wfv1.WorkflowSucceeded, woc.wf.Status.Phase, "workflow should complete")
4649+
}

0 commit comments

Comments
 (0)