Skip to content

fix: correct manual retry logic. Fixes #14124 #14328

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

Merged
merged 1 commit into from
Apr 23, 2025
Merged

Conversation

jswxstw
Copy link
Member

@jswxstw jswxstw commented Mar 25, 2025

Fixes #14124

Motivation

In certain scenarios, manual retries do not work properly.

Modifications

  • Retry all failed execution nodes
  • Reset all group nodes and non-boundary parent nodes if needed
  • Do not retry nodes which their descendant nodes are Succeeded

Verification

Case 1:

apiVersion: argoproj.io/v1alpha1
kind: Workflow
metadata:
  generateName: http-template-
spec:
  entrypoint: main
  arguments:
    parameters:
      # good: https://raw.githubusercontent.com/argoproj/argo-workflows/4e450e250168e6b4d51a126b784e90b11a0162bc/pkg/apis/workflow/v1alpha1/generated.swagger.json
      # bad: https://raw.githubusercontent.com/argoproj/argo-workflows/thisisnotahash/pkg/apis/workflow/v1alpha1/generated.swagger.json
      - name: url
        value: "https://raw.githubusercontent.com/argoproj/argo-workflows/thisisnotahash/pkg/apis/workflow/v1alpha1/generated.swagger.json"
  templates:
    - name: main
      steps:
        - - name: fail1
            template: http
          - name: fail2
            template: http
    - name: http
      http:
        url: "{{workflow.parameters.url}}"

Case 2:

apiVersion: argoproj.io/v1alpha1
kind: Workflow
metadata:
  name: workflow-exit-handler-fail
spec:
  entrypoint: echo
  onExit: exit-handler
  templates:
  - name: echo
    http:
      url: "https://raw.githubusercontent.com/argoproj/argo-workflows/4e450e250168e6b4d51a126b784e90b11a0162bc/pkg/apis/workflow/v1alpha1/generated.swagger.json"
  - name: fail
    container:
      image: alpine:3.18
      command: [sh, -c]
      args: ["exit 1"]
  - name: exit-handler
    steps:
      - - name: exit-handler-task
          template: fail

Case 3:

apiVersion: argoproj.io/v1alpha1
kind: Workflow
metadata:
  name: workflow-steps-with-retry-fail
spec:
  entrypoint: main
  templates:
  - name: main
    steps:
    - - name: retry-step-group-case
        template: fail-step-group
  - name: fail-with-rate
    container:
      image: python:alpine3.6
      command: ["python", -c]
      args: ["import random; import sys; exit_code = random.choice([0, 1]); sys.exit(exit_code);"]
  - name: fail-step-group
    steps:
    - - name: step1
        template: fail-with-rate
    - - name: step2
        template: fail-with-rate
    - - name: step3
        template: fail-with-rate
    retryStrategy:
      limit: "1"

Case 4

apiVersion: argoproj.io/v1alpha1
kind: Workflow
metadata:
  name: dag-contiue-on-fail
spec:
  retryStrategy:
    limit: 1
  entrypoint: workflow
  templates:
  - name: workflow
    dag:
      tasks:
      - name: A
        template: hello-world
      - name: B
        depends: "A"
        template: intentional-fail
      - name: C
        depends: "A"
        template: hello-world
      - name: D
        depends: "B.Failed && C"
        template: hello-world
      - name: E
        depends: "A"
        template: intentional-fail
      - name: F
        depends: "A"
        template: hello-world
      - name: G
        depends: "E && F"
        template: hello-world

  - name: hello-world
    container:
      image: busybox
      command: [echo]
      args: ["hello world"]

  - name: intentional-fail
    container:
      image: alpine:latest
      command: [sh, -c]
      args: ["echo intentional failure; exit 1"]

Documentation

@jswxstw jswxstw marked this pull request as draft March 25, 2025 04:01
@jswxstw jswxstw changed the title fix: manual retry fix: correct manual retry logic. Fixes #14124 Apr 2, 2025
@jswxstw jswxstw force-pushed the fix-14124 branch 2 times, most recently from ae51c3b to 3209d68 Compare April 3, 2025 06:32
@jswxstw
Copy link
Member Author

jswxstw commented Apr 3, 2025

/retest

@jswxstw jswxstw marked this pull request as ready for review April 3, 2025 07:22
@jswxstw jswxstw requested a review from isubasinghe April 3, 2025 07:23
Copy link
Member

@isubasinghe isubasinghe left a comment

Choose a reason for hiding this comment

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

Nice, thanks for simplifying the logic and fixing the bugs in the handling of http and plugin nodes.

@isubasinghe isubasinghe enabled auto-merge (squash) April 22, 2025 06:52
auto-merge was automatically disabled April 22, 2025 11:15

Head branch was pushed to by a user without write access

@jswxstw
Copy link
Member Author

jswxstw commented Apr 22, 2025

/retest

@Joibel Joibel merged commit bd13b1c into argoproj:main Apr 23, 2025
56 of 57 checks passed
Joibel pushed a commit that referenced this pull request Apr 28, 2025
(cherry picked from commit bd13b1c)
Signed-off-by: oninowang <[email protected]>
Signed-off-by: Alan Clucas <[email protected]>
Joibel pushed a commit that referenced this pull request Apr 28, 2025
(cherry picked from commit bd13b1c)
Signed-off-by: oninowang <[email protected]>
Signed-off-by: Alan Clucas <[email protected]>
Joibel added a commit that referenced this pull request Apr 28, 2025
…14430)

Signed-off-by: oninowang <[email protected]>
Signed-off-by: Alan Clucas <[email protected]>
Co-authored-by: jswxstw <[email protected]>
ericsengineer pushed a commit to ericsengineer/argo-workflows that referenced this pull request May 6, 2025
kim-codefresh added a commit to codefresh-io/argo-workflows that referenced this pull request May 20, 2025
…abilities fixes (Cr 28355) (#358)

* fix: bump deps for k8schain to fix ecr-login (argoproj#14008) (release-3.6 cherry-pick) (argoproj#14174)

* fix(ci): python sdk release process (release-3.6) (argoproj#14183)

Signed-off-by: Alan Clucas <[email protected]>

* docs: clarify qps/burst on controller (cherry-pick argoproj#14190) (argoproj#14192)

Signed-off-by: Tim Collins <[email protected]>
Co-authored-by: Tim Collins <[email protected]>

* fix(api/jsonschema): use unchanging JSON Schema version (cherry-pick argoproj#14092) (argoproj#14256)

Signed-off-by: Roger Peppe <[email protected]>
Co-authored-by: Roger Peppe <[email protected]>

* fix(api/jsonschema): use working `$id` (cherry-pick argoproj#14257) (argoproj#14258)

Signed-off-by: Roger Peppe <[email protected]>
Co-authored-by: Roger Peppe <[email protected]>

* docs: autogenerate tested k8s versions and centralize config (argoproj#14176) (release-3.6) (argoproj#14262)

Signed-off-by: Mason Malone <[email protected]>
Signed-off-by: Alan Clucas <[email protected]>
Co-authored-by: Mason Malone <[email protected]>

* chore(deps): bump minio-go to newer version (argoproj#14185) (release-3.6) (argoproj#14261)

Co-authored-by: Vaibhav Kaushik <[email protected]>

* fix: split pod controller from workflow controller (argoproj#14129) (release-3.6) (argoproj#14263)

* chore(deps): fix snyk (argoproj#14264) (release-3.6) (argoproj#14268)

* chore: revert to correct k8s versions

Accidental bump from argoproj#14176 cherry-pick

Signed-off-by: Alan Clucas <[email protected]>

* chore(deps): bump github.com/go-jose/go-jose/v3 from 3.0.3 to 3.0.4 in the go_modules group (cherry-pick argoproj#14231) (argoproj#14269)

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* fix: wait for workflow informer to sync before pod informer (cherry-pick argoproj#14248) (argoproj#14266)

Signed-off-by: Rohan K <[email protected]>
Co-authored-by: Rohan K <[email protected]>

* fix(cli): remove red from log colour selection. Fixes argoproj#6740 (cherry-pick argoproj#14215) (argoproj#14278)

Signed-off-by: Prabakaran Kumaresshan <[email protected]>
Co-authored-by: Prabakaran Kumaresshan <[email protected]>

* fix: correct semaphore configmap keys for multiple semaphores (argoproj#14184) (release-3.6) (argoproj#14281)

* fix: don't print help for non-validation errors. Fixes argoproj#14234 (cherry-pick argoproj#14249) (argoproj#14283)

Signed-off-by: Koichi Shimada <[email protected]>
Signed-off-by: Mason Malone <[email protected]>
Co-authored-by: koichi <[email protected]>
Co-authored-by: Mason Malone <[email protected]>

* docs: fix kubernetes versions (release-3.6) (argoproj#14273)

Signed-off-by: Alan Clucas <[email protected]>

* fix(workflow/sync): use RWMutex to prevent concurrent map access (cherry-pick argoproj#14321) (argoproj#14322)

Signed-off-by: Ryan Currah <[email protected]>
Co-authored-by: Ryan Currah <[email protected]>

* chore(lint): update golangci-lint to 2.1.1 (argoproj#14390) (cherry-pick release-3.6) (argoproj#14417)

* chore: bump golang 1.23->1.24 (argoproj#14385) (cherry-pick release-3.6) (argoproj#14418)

* fix: gracefully handle invalid CronWorkflows and simplify logic.  (cherry-pick argoproj#14197) (argoproj#14419)

Signed-off-by: Mason Malone <[email protected]>

* fix: prevent dfs sorter infinite recursion on cycle. Fixes argoproj#13395 (cherry-pick argoproj#14391) (argoproj#14420)

Signed-off-by: Adrien Delannoy <[email protected]>
Co-authored-by: Adrien Delannoy <[email protected]>

* chore(deps): bump github.com/expr-lang/expr from 1.16.9 to 1.17.0 (argoproj#14307) (release-3.6) (argoproj#14421)

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* chore(deps)!: update k8s and argo-events (release-3.6) (argoproj#14424)

Signed-off-by: dependabot[bot] <[email protected]>
Signed-off-by: william.vanhevelingen <[email protected]>
Signed-off-by: Mason Malone <[email protected]>
Signed-off-by: William Van Hevelingen <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: William Van Hevelingen <[email protected]>
Co-authored-by: Mason Malone <[email protected]>

* fix: correct retry logic (argoproj#13734) (release-3.6) (argoproj#14428)

Signed-off-by: isubasinghe <[email protected]>
Signed-off-by: Alan Clucas <[email protected]>
Co-authored-by: Isitha Subasinghe <[email protected]>

* fix: manual retries exit handler cleanup. Fixes argoproj#14180 (argoproj#14181) (release-3.6) (argoproj#14429)

Signed-off-by: isubasinghe <[email protected]>
Signed-off-by: Alan Clucas <[email protected]>
Co-authored-by: Isitha Subasinghe <[email protected]>

* fix: correct manual retry logic. Fixes argoproj#14124 (argoproj#14328) (release-3.6) (argoproj#14430)

Signed-off-by: oninowang <[email protected]>
Signed-off-by: Alan Clucas <[email protected]>
Co-authored-by: jswxstw <[email protected]>

* fix: disable ALPN in argo-server as a workaround (argoproj#14433)

Signed-off-by: Alan Clucas <[email protected]>

* result of codegen

Signed-off-by: Kim <[email protected]>

* fix:lint

Signed-off-by: Kim <[email protected]>

---------

Signed-off-by: Alan Clucas <[email protected]>
Signed-off-by: Tim Collins <[email protected]>
Signed-off-by: Roger Peppe <[email protected]>
Signed-off-by: Mason Malone <[email protected]>
Signed-off-by: dependabot[bot] <[email protected]>
Signed-off-by: Rohan K <[email protected]>
Signed-off-by: Prabakaran Kumaresshan <[email protected]>
Signed-off-by: Koichi Shimada <[email protected]>
Signed-off-by: Ryan Currah <[email protected]>
Signed-off-by: Adrien Delannoy <[email protected]>
Signed-off-by: william.vanhevelingen <[email protected]>
Signed-off-by: William Van Hevelingen <[email protected]>
Signed-off-by: isubasinghe <[email protected]>
Signed-off-by: oninowang <[email protected]>
Signed-off-by: Kim <[email protected]>
Co-authored-by: Alan Clucas <[email protected]>
Co-authored-by: gcp-cherry-pick-bot[bot] <98988430+gcp-cherry-pick-bot[bot]@users.noreply.github.com>
Co-authored-by: Tim Collins <[email protected]>
Co-authored-by: Roger Peppe <[email protected]>
Co-authored-by: Mason Malone <[email protected]>
Co-authored-by: Vaibhav Kaushik <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Rohan K <[email protected]>
Co-authored-by: Prabakaran Kumaresshan <[email protected]>
Co-authored-by: koichi <[email protected]>
Co-authored-by: Ryan Currah <[email protected]>
Co-authored-by: Adrien Delannoy <[email protected]>
Co-authored-by: William Van Hevelingen <[email protected]>
Co-authored-by: Isitha Subasinghe <[email protected]>
Co-authored-by: jswxstw <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

HTTP and Plugin nodes cannot be manually retried
3 participants