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.

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

feat(controller): Log expression context for conditional retries. #7181

Closed
wants to merge 1 commit into from

Conversation

jmcarp
Copy link
Contributor

@jmcarp jmcarp commented Nov 8, 2021

I was debugging an issue with conditional retries, and it took a while to figure out that a certain failure mode was associated with an exit code of -1. Logging the expression context would help with this kind of task in the future. Let me know if there's a better place to put this than the operator logs!

cc @jli

Signed-off-by: Josh Carp [email protected]

Don't bother creating a PR until you've done this:

  • Run make pre-commit -B to fix codegen, lint, and commit message problems.

@jmcarp
Copy link
Contributor Author

jmcarp commented Nov 9, 2021

By the way, I'm still confused about why preempted pods have no outputs or exit code. Here's an example from my testing:

   retry-test-preempt-01-3853357172:
      displayName: retry-test-preempt-01(1)
      finishedAt: null
      hostNodeName: ...
      id: retry-test-preempt-01-3853357172
      message: Node is shutting, evicting pods
      name: retry-test-preempt-01(1)
      phase: Failed
      progress: 1/1
      resourcesDuration:
        cpu: 0
        memory: 0
      startedAt: "2021-11-07T07:13:40Z"
      templateName: retry-crash
      templateScope: local/retry-test-preempt-01
      type: Pod

@jmcarp jmcarp marked this pull request as ready for review November 10, 2021 23:43
@alexec
Copy link
Contributor

alexec commented Nov 23, 2021

Can you please add the output of this change?

Copy link
Contributor

@alexec alexec left a comment

Choose a reason for hiding this comment

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

info requested

@sarabala1979 sarabala1979 enabled auto-merge (squash) December 10, 2021 07:04
@kennytrytek
Copy link
Contributor

I attempted to generate log output for this issue, but it appears that using expression in a retry strategy is currently broken. I filed a bug for that issue here: #7539

Until that is resolved, I do not know of a good way to show this output as requested.

@@ -791,6 +791,7 @@ func (woc *wfOperationCtx) processNodeRetries(node *wfv1.NodeStatus, retryStrate
if retryStrategy.Expression != "" && len(node.Children) > 0 {
localScope := buildRetryStrategyLocalScope(node, woc.wf.Status.Nodes)
scope := env.GetFuncMap(localScope)
woc.log.WithField("node", node.Name).Infof("evaluating retry conditional with scope %s", localScope)
Copy link
Contributor

Choose a reason for hiding this comment

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

Should be Debug level?

@stale
Copy link

stale bot commented Feb 7, 2022

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the problem/stale This has not had a response in some time label Feb 7, 2022
@stale stale bot closed this Feb 12, 2022
auto-merge was automatically disabled February 12, 2022 18:22

Pull request was closed

@tooptoop4
Copy link
Contributor

where @jmcarp

@agilgur5 agilgur5 added area/templating Templating with `{{...}}` area/retryStrategy Template-level retryStrategy labels Oct 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/retryStrategy Template-level retryStrategy area/templating Templating with `{{...}}` problem/stale This has not had a response in some time
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants