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: add a new global library 'withRetryAndTimeout' to allow circuit breaking pipeline steps #212

Closed
wants to merge 4 commits into from
Closed

Conversation

dduportal
Copy link
Contributor

This PR provide a circuit breaker of type "retry with timeout per retry" library.

Its initial case it to ensure that some of the jenkins-infra's docker builds involving network operations were not blocking (e.g. 10 min timeout before failing the step), but retrying a few times before really failing the job.

Ref. https://issues.jenkins.io/browse/JENKINS-51454

Please note that there are incoming PRs on the Jenkins Pipeline-related plugins that would allow using pure retry and timeout methods instead of try/catch: the library would have to be reconsidered or updated and better tested at this moment.

Another added value of this library: an environment variable PIPELINE_RETRY_COUNTER is injected to hold the retry counter's values (e.g. "how many retries?"). This value can be used by consumer to identify uniquely some resources such as warning-ng reports.

… breaking pipeline steps

Signed-off-by: Damien Duportal <[email protected]>
Copy link
Member

@timja timja left a comment

Choose a reason for hiding this comment

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

seems sensible? any other thoughts?

vars/withRetryAndTimeout.groovy Outdated Show resolved Hide resolved
Signed-off-by: Damien Duportal <[email protected]>
@dduportal
Copy link
Contributor Author

@timja I'm confused by the compiler error messages (I got the same locally).
I understand that the exceptions types are not known by the Eclipse Groovy compiler: in real life it works as expected (see. jenkins-infra/docker-jenkins-weeklyci#208) but not in the build/test environment.

Should we add the hpi plugins of the workflow steps (which provides timeout and the exception types) as part of the build/test dependencies here? Or did I miss something?

@jglick
Copy link
Contributor

jglick commented Jun 1, 2021

dduportal and others added 2 commits June 1, 2021 18:28
Signed-off-by: Damien Duportal <[email protected]>
@dduportal
Copy link
Contributor Author

dduportal commented Jun 1, 2021

see jenkinsci/workflow-basic-steps-plugin#144

@jglick Should I look at this change as a FYI, or is it related to the dependency question (or something else)?

@timja
Copy link
Member

timja commented Jun 1, 2021

see jenkinsci/workflow-basic-steps-plugin#144

@jglick Should I look at this change as a FYI, or is it related to the dependency question (or something else)?

it means we wouldn't need this PR at all

@dduportal
Copy link
Contributor Author

Fair, given that the linked PR is "almost there" (modulo the GitHub incident/ end of approval window / merge / release), I'll go back to the road of "try catch on pipeline with approved call to causes" on the few repositories (https://github.com/jenkins-infra/docker-jenkins-weekly/ and https://github.com/jenkins-infra/docker-jenkins-lts) which requires the change right now. Thanks a lot for taking time to review and give advises on this!

@dduportal dduportal closed this Jun 2, 2021
@dduportal dduportal deleted the feat/withretryandtimeout branch June 2, 2021 10:01
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.

4 participants