-
Notifications
You must be signed in to change notification settings - Fork 3.1k
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.
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
fix(controller): Report reconciliation errors better #4877
Conversation
Signed-off-by: Alex Collins <[email protected]>
Signed-off-by: Alex Collins <[email protected]>
Signed-off-by: Alex Collins <[email protected]>
Signed-off-by: Alex Collins <[email protected]>
Signed-off-by: Alex Collins <[email protected]>
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)) |
There was a problem hiding this comment.
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
Signed-off-by: Alex Collins <[email protected]>
Signed-off-by: Alex Collins <[email protected]>
|
Signed-off-by: Alex Collins <[email protected]>
Signed-off-by: Alex Collins <[email protected]>
Signed-off-by: Alex Collins <[email protected]>
Signed-off-by: Alex Collins <[email protected]>
case ErrTimeout: | ||
woc.markWorkflowFailed(ctx, x.Error()) | ||
return | ||
default: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
guard with transient check
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
add feature flag (envvar)
Signed-off-by: Alex Collins <[email protected]>
Signed-off-by: Alex Collins <[email protected]>
|
Signed-off-by: Alex Collins <[email protected]>
Signed-off-by: Alex Collins <[email protected]>
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: |
There was a problem hiding this comment.
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
?
There was a problem hiding this comment.
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
Co-authored-by: Simon Behar <[email protected]>
Signed-off-by: Alex Collins [email protected]
Checklist: