-
Notifications
You must be signed in to change notification settings - Fork 29
fix: stage status in case of retries #150
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
Conversation
|
Hi @jglick , Please review this PR |
|
@olamy maybe. AFAIK this API is only used by Stage View, which is mostly unmaintained. |
|
@stuartrowe, @timja, do you think Pipeline Graph View could use this fix too? |
|
Maybe I'm reading the fix wrong, but it seems like it's ignoring the WarningAction if the run was a success overall. We do not want that change. |
|
I'm very confused either. @dharanikesav I cannot reproduce your problem: chrome_9T1jANDxfd.mp4pipeline {
agent any
stages {
stage('Pass 1') {
steps {
sh 'echo pass 1'
}
}
stage('Retry pass 3rd attempt') {
steps {
retry(3) {
sh '''
ATTEMPT_FILE=attempt.txt
if [ -f $ATTEMPT_FILE ]; then
ATTEMPT=$(cat $ATTEMPT_FILE)
else
ATTEMPT=0
fi
ATTEMPT=$((ATTEMPT + 1))
echo $ATTEMPT > $ATTEMPT_FILE
if [ $ATTEMPT -lt 3 ]; then
echo "Failing attempt $ATTEMPT"
exit 1
else
echo "Passing attempt $ATTEMPT"
fi
'''
}
}
}
stage('Retry pass 1st attempt') {
steps {
retry(1) {
sh 'echo retry pass'
}
}
}
stage('Pass final') {
steps {
sh 'echo pass final'
}
}
}
}As you can see in the video, the stage is being correctly marked as passing. Unless I'm missing your issue too. Please clarify if so, providing a Jenkinsfile example would be nice. |
timja
left a comment
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.
needs at least a pipeline reproducing / demonstrating the issue and ideally a test
|
#164 (comment) FTR |
|
Hi @felipecrs @timja , Here is the pipeline script:
Downstream build has one parameter and executable shell build step with following shell script:
Here is the image of parameters of downstream job Below is the screenshot of main pipeline. Build 93 is successful but both the steps in the pipeline are marked as failed : |
|
@dharanikesav I think I see what your issue is. The build step will mark your stage as failed, and it's not possible to improve the stage result once it was set as failed once. To fix this issue, you need to set def buildObj = build propagate: false
if (buildObj.getResult() == "SUCCESS") {
echo "build passed"
}Please confirm whether that's indeed the case, and if yes, you can close this PR. |
|
Hi @felipecrs , When we do not propagate failures and have a custom script to throw an exception in case of failures, this works. But, Theoretically, the plugin should display correct status of each stage. Instead of workaround, is it possible to incorporate a fix at plugin level, so that the status is shown correctly. Thanks, |
That's exactly what it appears to be doing. I expect you still want to have warning actions shown when a run has passed? Is this just a problem in stage view that doesn't differentiate between warning and failed? Alternatively is it an issue where its looking for the worst result and not the last result?
Not sure I don't think there's any known issues around retries though, likely this is stage view only. |
|
I don't think there's any issue here. I demonstrated that retries work as expected in my example above. The quirk is that the build step changes the stage result directly when propagate: true, and once a stage is marked as failed, it can no longer be marked as passing. This is by design and IIRC is documented. |
|
Using propagate: false is not a workaround, it's the proper way to handle this. |
|
Hi @felipecrs , I understand that propagate: false can be used, but it still requires handling downstream build failures and warnings through custom scripting. It does feel a bit like a workaround rather than a clean solution. If propagate: false is used without handling downstream build failures, the main pipeline is marked as SUCCESS even if there is real failure in downstream build. What I'm really aiming for is this: if the same stage/job is retried and succeeds on the second attempt, ideally, the stage should be marked as SUCCESS. Currently, it remains marked as FAILURE. Would it make sense to revisit this behavior and see if there's room for improvement? Thanks, |
|
@dharanikesav , I am just a Jenkins user like you and not anyhow a Jenkins representative, thus I cannot speak on behalf of Jenkins. Have you checked whether the same issue happens when using the |
|
I think you'd have a better luck discussing this issue in some other place. In the Graph Analysis itself I don't think there's anything to be done. The stage is being marked as failed by the build step, and thus it should be shown as failed in the UI. |
|
I'm not sure if a stage result works setting it multiple times though |
|
@timja, see: I believe the very same thing applies to the stage result as well. That's why you can't let |
|
The most clean "fix" I can think of is adding a build retry: 3 |
|
Makes sense, I thought that was the case for builds, wasn't sure on stage. I don't think its great, but fixing it here isn't the right change. |
To be fair it isn't documented under |
|
Maybe this is the cleanest solution for now: {
int attempt = 0
int maxAttempts = 3
retry(maxAttempts) {
attempt++
build propagate: attempt == maxAttempts
}
} |
This is a result of the behaviour of One option is to disable the warning action look up by setting Alternatively you can add a |




When a retry block is used inside a stage and the step failed in first attempt but passed in one of the subsequent attempts, expected stage result is "Success", expected build result is "Success" but the actual stage result is "Failure", actual build result is "Success". Screenshot of this bug is attached below (check second stage in build 36900 and third stage in build 36899 ).
Fix implemented:
While debugging, i noticed that this failure status is returned from WarningAction condition. So, have added another condition to check run result, so that this condition does not return failure when run result is success.
Testing done
Have tested this change locally with following test cases:
below is the screenshot of stage view of local test execution
