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): support cap on retryStrategy backoff. Fixes #13772 #13782

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

chengjoey
Copy link
Contributor

@chengjoey chengjoey commented Oct 18, 2024

Fixes #13772

Motivation

support cap on retryStrategy backoff

Modifications

  1. add cap filed on retryStrategy.Backoff
  2. use cap as retry duration if duration with factor > cap

Verification

unit test

@agilgur5 agilgur5 added the area/retryStrategy Template-level retryStrategy label Oct 18, 2024
@agilgur5 agilgur5 changed the title feat(controller): support cap on retryStrategy backoff feat(controller): support cap on retryStrategy backoff. Fixes #13772 Oct 19, 2024
@eiriklid
Copy link

Do you know why "CI / Codegen (pull_request)" fails @chengjoey ?

@Joibel
Copy link
Member

Joibel commented Oct 24, 2024

Do you know why "CI / Codegen (pull_request)" fails @chengjoey ?

The failure in CI is because make codegen -B hasn't been run locally, or at least the result of doing that hasn't been added to the commit. (It has partially been done, as some of the generated changes are in this).

@chengjoey
Copy link
Contributor Author

The failure in CI is because make codegen -B hasn't been run locally, or at least the result of doing that hasn't been added to the commit. (It has partially been done, as some of the generated changes are in this).

hi @Joibel , I made sure I ran make coegen locally and added the relevant changes to the commit

perl -i -pe 's|argoproj/argo-workflows/|argoproj/argo-workflows/v3/|g' `echo "$(1)" | sed 's/proto/pb.go/g'`

There are some strange things when I execute this step locally. I need to manually add v3 before executing this step, otherwise an error will be reported

import "github.com/argoproj/argo-workflows/pkg/apis/workflow/v1alpha1/generated.proto";

Am I missing the correct prefix step

@Joibel
Copy link
Member

Joibel commented Oct 24, 2024

The failure in CI is because make codegen -B hasn't been run locally, or at least the result of doing that hasn't been added to the commit. (It has partially been done, as some of the generated changes are in this).

hi @Joibel , I made sure I ran make coegen locally and added the relevant changes to the commit

perl -i -pe 's|argoproj/argo-workflows/|argoproj/argo-workflows/v3/|g' `echo "$(1)" | sed 's/proto/pb.go/g'`

There are some strange things when I execute this step locally. I need to manually add v3 before executing this step, otherwise an error will be reported

import "github.com/argoproj/argo-workflows/pkg/apis/workflow/v1alpha1/generated.proto";

Am I missing the correct prefix step

make codegen is the hardest thing to get working in the build process, so I'm not surprised that you're having trouble with it. You're not the first person to struggle with it. My favourite way of getting it to work locally is to use devcontainers so if you're not doing that and haven't tried, I would give that a go. If you are doing that already, perhaps recreating the devcontainer from scratch will help.

You can verify what the CI is complaining about here, which I got to by finding the failing check in the list of checks and hitting the Details button. It shows how make codegen has been run by CI and then there are differences in the resulting files.

I don't quite follow where you need to add v3 from your description, but if you need to edit anything to make this work, something isn't right. What you generate locally must match what CI generates or you get some errors. None of the errors look like something related to a v3 path.

Please note: the -B in make codegen -B may be important here (it ensures everything is rebuilt), so if you just did a plain make codegen try again with -B, and make yourself a nice beverage whilst your computer whirrs away for a while.

@chengjoey
Copy link
Contributor Author

Please note: the -B in make codegen -B may be important here

thanks @Joibel , it worked!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/retryStrategy Template-level retryStrategy
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Set cap on retryStrategy backoff
4 participants