workflow-basic-steps-plugin [JENKINS-67697] Create a step to set the result of the current stage

Add stageResult step to setup only the result of the current stage.

[x] Make sure you are opening from a feature branch (right side) and not your main branch!
[x] Ensure that the pull request title represents the desired changelog entry
[x] Please describe what you did
[x] Link to relevant issues in GitHub or Jira
[x] Link to relevant pull requests, esp. upstream and downstream changes
[x] Ensure you have provided tests - that demonstrates feature works or fixes the issue

Additional example of usage of this "pipeline step" than reported in JIRA is: we have stage for JIRA integration. On release we create a new JIRA version and link all issues found in commits since last tag to version we are releasing. If integration fails we have to continue the build and deploy artifacts on our ALM. Failed stage indicates to the project manager (no exceptions are thrown by integration steps) that JIRA issues must be linked manually.

Jan 28 '22 14:01 nfalco79

yes, no, maybe?

Feb 16 '22 11:02 nfalco79

@car-roll what do you think about this build step?

Mar 04 '22 16:03 nfalco79

What is the use case that is not already covered by the unstable step?

Mar 14 '22 16:03 jglick

unstable set the build result as unstable instead I want the build SUCCESS and mark a single stage as unstable or failed because for example some integration (like JIRA integration) failed. This means for example in a release process the build is ok and who trigger the build is notified that the JIRA integration did not work. Other scenario for example is that in a stage there is a failure because of report/threashold (dependency check or sonarqube) and you want mark which stages are failed (there could be more than one). Usually publisher does not support natively pipeline (for whay I know only xunit and junit support it). This step allow us to mark what are stage failed (there could be more than one) and user is notify by that.

Why we do not want mark build as unstable if a stage is failed? Because otherwise bitbucket build is red and branch restriction does not allow merge PRs.

Mar 14 '22 16:03 nfalco79

it would be sufficient if unstable step allowed to take a result other than UNSTABLE. But I suppose the name of the step would lose its meaning

Mar 14 '22 16:03 nfalco79

it would be sufficient if unstable step allowed to take a result other than UNSTABLE

I think it would suffice to add an option to suppress https://github.com/jenkinsci/workflow-basic-steps-plugin/blob/9d389ecd7313b52b44395f954e43d4daf366112e/src/main/java/org/jenkinsci/plugins/workflow/steps/UnstableStep.java#L76

(Defaulting to the current behavior for compatibility with existing builds, but also safety—you do not want someone inadvertently letting builds be considered passing, and thus PRs ready to merge etc., when something is known to be wrong.)

the name of the step would lose its meaning

Maybe. Does not seem too bad to me. https://github.com/jenkinsci/workflow-basic-steps-plugin/blob/9d389ecd7313b52b44395f954e43d4daf366112e/src/main/java/org/jenkinsci/plugins/workflow/steps/UnstableStep.java#L92 would still be correct.

Would also make sense to offer the same option to warnError https://github.com/jenkinsci/workflow-basic-steps-plugin/blob/2cc223c3b5f52448bb5ea3c3dc17b38165ca674c/src/main/java/org/jenkinsci/plugins/workflow/steps/WarnErrorStep.java#L106 https://github.com/jenkinsci/workflow-basic-steps-plugin/blob/2cc223c3b5f52448bb5ea3c3dc17b38165ca674c/src/main/java/org/jenkinsci/plugins/workflow/steps/WarnErrorStep.java#L69-L73

See #83.

Mar 14 '22 18:03 jglick

Ok I will perform the changes

Mar 15 '22 08:03 nfalco79

I look into the changes and as you suggest the limit is set stage only to unstable. If someone want set stage to other different value like failed or maybe not build to indicate the step has been skipped to avoid the "Stage View" loose stages of previous build (in the example stages of build n.1 miss because there was additional step in the middle).

Mar 16 '22 09:03 nfalco79

I am afraid neither Stage View nor Blue Ocean are actively maintained, and the ability to set a per-stage status is relatively new, so there are liable to be feature gaps in visualizations. @dwnusbaum would know more.

Mar 16 '22 14:03 jglick

I do not necessarily think there is anything wrong with a step like this, although I would make it set the build status as well by default. I agree with though Jesse that just adding an option to the unstable step to suppress setting the build result seems like a simpler option that would be fine for most use cases (I do not think it is critical to be able to use the full range of results, and UNSTABLE is the only result that historically had "something went wrong but we aren't stopping the build" semantics in Jenkins).

The description in Jira mentions using catchError to do this with an error step as a workaround, but the expected use case is that you have catchError around a step that itself throws an exception in the case you care about.

I do not quite follow https://github.com/jenkinsci/workflow-basic-steps-plugin/pull/171#issuecomment-1068920286, but "stage result" is not really a first-class concept as far as Pipeline internals go. It only matters insofar as visualizations support it. Blue Ocean supports it as far as I know, but I am not sure about Stage view (at least I never did anything to add support for it myself). If the visualization is not showing the stages you expect, then I do not think a step like this will change anything.

Mar 16 '22 14:03 dwnusbaum

So it sounds like it would work out to add a simple flag to both unstable and warnError to suppress¹ the current behavior of setting Run.result, but continue to only support UNSTABLE status?

¹Defaulting to current behavior for safety and compatibility. This is most easily done by adding a boolean flag defaulting to false but then the negative phrasing (e.g., suppressBuildStatus: true) is confusing to read. Better style is to use positive phrasing (e.g., setBuildStatus: false) with a flag defaulting to true, which you can accomplish with a boolean-valued JavaBeans property (getter/setter pair) but a Boolean field defaulting to null ~ true and a <f:checkbox default="true"/>. https://www.jenkins.io/doc/developer/plugin-development/pipeline-integration/#handling-default-values

Mar 16 '22 14:03 jglick

I called it markBuildResult.

but continue to only support UNSTABLE status?

Yes the question is that

Mar 16 '22 14:03 nfalco79

@jglick any specific reason why use Boolean instead or boolean ? In my local test it works also with boolean defaulted to true. Since it is a pipeline step there is no serialisation issue when attribute are missing in the XML

Mar 16 '22 15:03 nfalco79

It's strange to hear that Stage View and Blue Ocean not mantained. Anyway

Mar 16 '22 15:03 nfalco79

So it sounds like it would work out to add a simple flag to both unstable and warnError to suppress¹ the current behavior of setting Run.result, but continue to only support UNSTABLE status?

Seems fine, although I probably would not bother with warnError.

but continue to only support UNSTABLE status?

I think this is fine and simplest, but I do not feel too strongly either way. I am not completely opposed to a full setStageResult step.

Since it is a pipeline step there is no serialisation issue when attribute are missing in the XML

You still have to consider the case where a Pipeline that uses unstable is suspended across the Jenkins restart that upgrades this plugin to a version that include this PR.

Mar 16 '22 15:03 dwnusbaum

a Pipeline that uses unstable is suspended across the Jenkins restart

Probably impossible since this is not a durable step. But there is serialization in ArgumentsActionImpl to consider; and you at least need to interactively test the behavior of the snippet generator: by default the form should show it checked and generated Groovy should not mention the attribute.

Mar 16 '22 15:03 jglick

The snippet generator it's ok. It show checked at first time and does not generate the optional attribute. Anyway not a problem I'm changing to Boolean with designed name. Anway...I would push for a stageResult step that is most generic purpose and avoid in the future to create additional warnAbort etc etc to cover any other Result value.

Mar 16 '22 15:03 nfalco79

of course would need new test coverage

sure it was just like a preview also because miss changes on warnError step. My intention was understand if cutoff stageResultStep or not

Mar 16 '22 15:03 nfalco79

I guess I have a mild preference for a flag on the existing steps over introducing a new step or steps, but do not feel strongly about it. @dwnusbaum?

Mar 16 '22 16:03 jglick

Build passed, develop completed. I did three commits. Each commit contains the changes of a specific step that allow for easy revert

Mar 16 '22 16:03 nfalco79

I have a mild preference for a flag on the existing steps over introducing a new step or steps, but do not feel strongly about it

Same for me. I guess if we do introduce a new step, My preference would be to call it setBuildResult, have it both set the build result and add a WarningAction, and then give it a boolean option like setStageResultOnly that defaults to false. That way it would be a strictly better version of currentBuild.result = foo. Defaulting to setting the build result and adding WarningAction seems like the least surprising option for users given that WarningAction only affects visualizations and nothing else.

Mar 16 '22 20:03 dwnusbaum

I guess if we do introduce a new step, My preference would be to call it setBuildResult, have it both set the build result and add a WarningAction ... That way it would be a strictly better version of currentBuild.result = foo ...

I would prefer to this one.

RECAP This is now

stage("first") {
    unstable(message: "msg") // build result is set too
}
stage("second") {
    warnError(message: "msg") { // build result is set too
        sh ("exit -1")
    }
}
stage("third") {
    catchError(message: "msg", stageResult: 'UNSTABLE') { // works but horrible
        sh ("exit -1")
    }
}

After changes

stage("first") {
    // limited to UNSTABLE
    unstable(message: "msg", setBuildStatus: false)
}
stage("second") {
    // limited to UNSTABLE and horrible
    warnError(message: "msg", setBuildResult: false) {
        sh ("exit -1")
    }
}
stage("third") {
    // set state to any valid value
    stageResult(message: "msg", result: 'UNSTABLE')
}

@dwnusbaum option

stage("as described") {
    // sets current stage to UNSTABLE and currentBuild.result = "FAILURE"
    setBuildResult(buildResult: "FAILURE", stageResult: "UNSTABLE", stageResultOnly: false) 
    // sets current stage to UNSTABLE, not very clear about buildResult
    setBuildResult(buildResult: "FAILURE", stageResult: "UNSTABLE", stageResultOnly: true) // or stageOnly: true
    // sets current stage to UNSTABLE
    setBuildResult(stageResult: "UNSTABLE", stageResultOnly: true)
}

@dwnusbaum option but with attribute optionals like in catchError step

stage("attributes optional") {
    setBuildResult(buildResult: "FAILURE") // currentBuild.result = "FAILURE", stage untouched
    setBuildResult(stageResult: "UNSTABLE") // stage to UNSTABLE. currentBuild.result as is
}

Another option that wire stage (or any other future object similar to stage)

stage("mark in which stage fails") {
    setBuildResult(buildResult: "FAILURE") // currentBuild.result = "FAILURE", stage FAILURE
    setBuildResult(buildResult: "FAILURE", stageResult: "SUCCESS") // currentBuild.result = "FAILURE", stage will be green
    setBuildResult(stageResult: "UNSTABLE") // stage to UNSTABLE. currentBuild.result = UNSTABLE
}

Generic purpose

stage("third") {
    setStatusFor(result: "SUCCESS", target: "BUILD")
    setStatusFor(result: "UNSTABLE", target: "STAGE")
}

same syntax and open for the future for additional target objects

Mar 16 '22 21:03 nfalco79

To clarify, for the new step I described, there would be no way to set distinct build and stage results (too niche in my opinion), and there would be a default message. I also see no reason to allow the build result to be set without adding a WarningAction. The syntax would be like this:

 // Sets both stage and build result. WarningAction message would be "Build marked as Failure". Recommended replacement for `currentBuild.result = 'FAILURE'` or anything besides "UNSTABLE".
setBuildResult('FAILURE')
 // Sets both stage and build result. WarningAction message would be "foo".
setBuildResult(result: 'FAILURE', message: 'foo')
 // Only sets stage result. WarningAction message would be "Stage marked as Failure".
setBuildResult(result: 'UNSTABLE', stageResultOnly: true)
 // Only sets stage result. WarningAction message would be "foo".
setBuildResult(result: 'UNSTABLE', message: 'foo', stageResultOnly: true)

Mar 16 '22 22:03 dwnusbaum

So how I should proceed?

Mar 17 '22 15:03 nfalco79

I revert commit for stageResult because both of you do not like the approace to have a dedicated step to set the result for a stage.

How we should proceed than?

Mar 22 '22 08:03 nfalco79

any news?

May 25 '22 10:05 nfalco79

superseded by https://github.com/nfalco79/workflow-steps-plugin

Jan 26 '23 17:01 nfalco79