Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix(controller): Report reconciliation errors better #4877

Merged
merged 17 commits into from
Jan 21, 2021
Merged

fix(controller): Report reconciliation errors better #4877

Show file tree
Hide file tree
Changes from 12 commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
75e7186
fix(controller): Report reconcilliation errors better
alexec Jan 14, 2021
aa85f19
:err: M workflow/controller/operator.go
alexec Jan 14, 2021
74cba75
:err: M workflow/controller/operator.go
alexec Jan 14, 2021
f35ae24
:err: M workflow/controller/dag.go
alexec Jan 14, 2021
acae007
:err: M workflow/controller/operator.go
alexec Jan 14, 2021
98c67ed
Merge branch 'master' into err
alexec Jan 15, 2021
5fd768b
:err: M .github/workflows/ci-build.yaml
alexec Jan 15, 2021
28f2be6
Update operator.go
alexec Jan 16, 2021
28ec329
Merge branch 'master' into err
alexec Jan 17, 2021
4bd02ba
Merge branch 'master' into err
alexec Jan 18, 2021
71e34cb
Merge branch 'master' into err
alexec Jan 18, 2021
4e2bf56
:err: M workflow/controller/operator.go
alexec Jan 18, 2021
9004467
Merge branch 'master' into err
alexec Jan 19, 2021
f94da6d
:err: A util/diff/diff.go
alexec Jan 19, 2021
98c19b5
:err: M workflow/controller/operator.go
alexec Jan 19, 2021
fc9faf5
pending error
alexec Jan 19, 2021
f18de64
Update util/diff/diff.go
alexec Jan 21, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 2 additions & 3 deletions workflow/controller/dag.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -461,7 +461,7 @@ func (woc *wfOperationCtx) executeDAGTask(ctx context.Context, dagCtx *dagContex
}

// Finally execute the template
node, err = woc.executeTemplate(ctx, taskNodeName, &t, dagCtx.tmplCtx, t.Arguments, &executeTemplateOpts{boundaryID: dagCtx.boundaryID, onExitTemplate: dagCtx.onExitTemplate})
_, err = woc.executeTemplate(ctx, taskNodeName, &t, dagCtx.tmplCtx, t.Arguments, &executeTemplateOpts{boundaryID: dagCtx.boundaryID, onExitTemplate: dagCtx.onExitTemplate})
if err != nil {
switch err {
case ErrDeadlineExceeded:
Expand All @@ -471,8 +471,7 @@ func (woc *wfOperationCtx) executeDAGTask(ctx context.Context, dagCtx *dagContex
_ = woc.markNodePhase(taskNodeName, wfv1.NodeFailed, err.Error())
return
default:
woc.log.Infof("DAG %s deemed errored due to task %s error: %s", node.ID, taskNodeName, err.Error())
_ = woc.markNodePhase(taskNodeName, wfv1.NodeError, fmt.Sprintf("task '%s' errored", taskNodeName))
_ = woc.markNodeError(taskNodeName, fmt.Errorf("task '%s' errored: %v", taskNodeName, err))
return
}
}
Expand Down
23 changes: 14 additions & 9 deletions workflow/controller/operator.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -344,13 +344,20 @@ func (woc *wfOperationCtx) operate(ctx context.Context) {

node, err := woc.executeTemplate(ctx, woc.wf.ObjectMeta.Name, &wfv1.WorkflowStep{Template: woc.execWf.Spec.Entrypoint}, tmplCtx, woc.execWf.Spec.Arguments, &executeTemplateOpts{})
if err != nil {
// the error are handled in the callee so just log it.
msg := "error in entry template execution"
woc.log.WithError(err).Error(msg)
msg = fmt.Sprintf("%s %s: %+v", woc.wf.Name, msg, err)
woc.log.WithError(err).Error("error in entry template execution")
alexec marked this conversation as resolved.
Show resolved Hide resolved
// we wrap this error up to report a clear message
x := fmt.Errorf("error in entry template execution: %w", err)
switch err {
case ErrDeadlineExceeded:
woc.eventRecorder.Event(woc.wf, apiv1.EventTypeWarning, "WorkflowTimedOut", msg)
woc.eventRecorder.Event(woc.wf, apiv1.EventTypeWarning, "WorkflowTimedOut", x.Error())
case ErrParallelismReached:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Currently we won't log ErrParallelismReached. Is this intended or did you mean to add a fallthrough?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not quite, we will log it (just before the switch statement) - but we don't want to error the workflow

case ErrTimeout:
woc.markWorkflowFailed(ctx, x.Error())
alexec marked this conversation as resolved.
Show resolved Hide resolved
return
default:
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

guard with transient check

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add feature flag (envvar)

if !woc.wf.Status.Phase.Completed() {
woc.markWorkflowError(ctx, x)
}
}
return
}
Expand Down Expand Up @@ -881,10 +888,8 @@ func (woc *wfOperationCtx) podReconciliation(ctx context.Context) error {
}
node := woc.wf.Status.Nodes[pod.ObjectMeta.Name]
if node.Fulfilled() && !node.IsDaemoned() {
if tmpVal, tmpOk := pod.Labels[common.LabelKeyCompleted]; tmpOk {
if tmpVal == "true" {
return
}
if pod.GetLabels()[common.LabelKeyCompleted] == "true" {
alexec marked this conversation as resolved.
Show resolved Hide resolved
return
}
woc.completedPods[pod.ObjectMeta.Name] = true
if woc.shouldPrintPodSpec(node) {
Expand Down
2 changes: 1 addition & 1 deletion workflow/controller/operator_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4164,7 +4164,7 @@ func TestValidReferenceMode(t *testing.T) {
woc.wf.Status.StoredWorkflowSpec.Entrypoint = "different"
woc = newWorkflowOperationCtx(woc.wf, controller)
woc.operate(ctx)
assert.Equal(t, wfv1.NodeRunning, woc.wf.Status.Phase)
assert.Equal(t, wfv1.NodeError, woc.wf.Status.Phase)
}

var workflowStatusMetric = `
Expand Down
4 changes: 1 addition & 3 deletions workflow/controller/steps.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -247,10 +247,8 @@ func (woc *wfOperationCtx) executeStepGroup(ctx context.Context, stepGroup []wfv
case ErrTimeout:
return woc.markNodePhase(node.Name, wfv1.NodeFailed, fmt.Sprintf("child '%s' timedout", childNodeName))
default:
errMsg := fmt.Sprintf("child '%s' errored", childNodeName)
woc.log.Infof("Step group node %s deemed errored due to child %s error: %s", node.ID, childNodeName, err.Error())
woc.addChildNode(sgNodeName, childNodeName)
return woc.markNodePhase(node.Name, wfv1.NodeError, errMsg)
return woc.markNodeError(node.Name, fmt.Errorf("step group deemed errored due to child %s error: %w", childNodeName, err))
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

again, obfuscating the error to the user

}
}
if childNode != nil {
Expand Down