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

Handle NOT_BUILT and ABORTED as other results #400

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

hashar
Copy link
Contributor

@hashar hashar commented Oct 18, 2024

Use case

My use case is a post build script triggering a job which does not need to fully complete. Since the build step can be cancelled while waiting or building, it causes the triggering build to be marked as a failure despite asking to never block.

To fix that, I need the plugin to pass the result of the build step through BlockingBehaviour. That lets one define how to behave when the triggered build is NOT_BUILT (it got cancelled frmo the queue). In my case I need it to never block.

See: https://phabricator.wikimedia.org/T352319

Solution

There are multiple reasons for a job to not fully complete:

  • it is interrupted, an InterruptedException is thrown, this is still rethrown and Jenkins will mark the build as ABORTED.

  • it can be cancelled from the build queue raising a CancellationException. This previously raised an AbortException which Jenkins handles by marking the build as a failure. I have changed it to a NOT_BUILT result which can be process as other results (addressing my use case to have it to never block).

The Jenkins Result class ranks the results as:

  • SUCCESS
  • UNSTABLE
  • FAILURE
  • NOT_BUILT
  • ABORTED.

The NOT_BUILT and ABORTED results are thus worse than a FAILURE and would be matched as such in BlockingBehavior mapBuildStepResult() and mapBuildResult() which both use isWorseOrEqualTo() for comparison.

Add a test testCancelledFromBuildQueue() to cover the CancellationException() is caught and it results in a SUCCESS (since the test blocking behavior is to never block).

The ResultConditionTest test covers that BlockingBehavior is able to map NOT_BUILD and ABORTED since it has two tests explicitly cancelling and interrupting jobs.

Examples

When a build is ongoing and when aborting it:

  Waiting for the completion of downstream-project
  downstream-project #7 started.
  downstream-project #7 completed. Result was ABORTED
  Build step 'Trigger/call builds on other projects' marked build as failure
  Finished: FAILURE

When it is waiting in the build queue and get cancelled:

  Waiting for the completion of downstream-project
  Not built: downstream-project has been cancelled while waiting in the queue.
  Build step 'Trigger/call builds on other projects' marked build as failure
  Finished: FAILURE

Testing done

I have created two jobs in a Jenkins spinned up with mvn hpi:run. I have covered the behavior for cancelled jobs with a new test TriggerBuilderTest.testCancelledFromBuildQueue().

The result mapping for ABORTED and NOT_BUILD is already covered by ResultConditionTest.

Submitter checklist

  • Make sure you are opening from a topic/feature/bugfix branch (right side) and not your main branch! I have used a branch named handle-not_built-canceled
  • Ensure that the pull request title represents the desired changelog entry
  • Please describe what you did
  • Link to relevant issues in GitHub or Jira
  • Ensure you have provided tests - that demonstrates feature works or fixes the issue

@hashar hashar requested a review from a team as a code owner October 18, 2024 12:44
@hashar hashar force-pushed the handle-not_built-canceled branch from ec4350c to 9f7e52b Compare October 23, 2024 13:58
@hashar hashar force-pushed the handle-not_built-canceled branch from 9f7e52b to a95cb34 Compare November 4, 2024 15:33
@hashar hashar marked this pull request as draft November 13, 2024 10:00
@hashar
Copy link
Contributor Author

hashar commented Nov 13, 2024

In https://github.com/jenkinsci/parameterized-trigger-plugin/compare/9f7e52bbba881e1444aa22d8cbde18f7a5ed0a03..a95cb34ef00de439568ce662fbdc083624b15247 I did:

  } catch (InterruptedException x) {
  ...
- completedResult = Result.ABORTED
+ throw x; // rethrow so that the triggering project get flagged as cancelled

The commit message and the pull request text should be adjusted to take that in account. I have thus marked this as being in draft until I fix the texts.

@hashar hashar force-pushed the handle-not_built-canceled branch from a95cb34 to a455d7e Compare November 14, 2024 13:03
Instead of:

  if (condition != null) {
    // indented large block code
  } else {
    log.info("Skipped");
  }

Inverse the condition check to move the logging code at the top and add
an explicit continue. This saves a level of indentation on the code
block:

  if (condition == null) {
    log.info("Skipped");
    continue;
  }
  // large block code

Also adjust the logging message while at it:

                              vv
- The project was not trigger by some reason.
+ The project was not trigger for some reason.
                              ^^^
Instead of invoking `completedRun.getResult()` several times, invoke it
once and reuse the resust. That makes the code slightly easier to read.
@hashar hashar force-pushed the handle-not_built-canceled branch from a455d7e to 9eaaf80 Compare November 14, 2024 13:04
@hashar hashar marked this pull request as ready for review November 14, 2024 13:05
@hashar hashar force-pushed the handle-not_built-canceled branch from 9eaaf80 to d1e4c3a Compare November 18, 2024 09:45
Use case
--------

My use case is a post build script triggering a job which does not need
to fully complete. Since the build step can be cancelled while waiting
or building, it causes the triggering build to be marked as a failure
despite asking to never block.

To fix that, I need the plugin to pass the result of the build step
through BlockingBehaviour. That lets one define how to behave when the
triggered build is NOT_BUILT (it got cancelled from the queue). In my
case I need it to never block.

See: https://phabricator.wikimedia.org/T352319

Solution
--------

There are multiple reasons for a job to not fully complete:

- it is interrupted, an InterruptedException is thrown, this is still
  rethrown and Jenkins will mark the build as ABORTED.

- it can be cancelled from the build queue raising a
  CancellationException. This previously raised an AbortException which
  Jenkins handles by marking the build as a failure. I have changed it to
  a NOT_BUILT result which can be process as other results (addressing
  my use case to have it to never block).

The Jenkins Result class ranks the results as:
- SUCCESS
- UNSTABLE
- FAILURE
- NOT_BUILT
- ABORTED.

The NOT_BUILT and ABORTED results are thus worse than a FAILURE and
would be matched as such in BlockingBehavior mapBuildStepResult() and
mapBuildResult() which both use isWorseOrEqualTo() for comparison.

Add a test testCancelledFromBuildQueue() to cover the
CancellationException() is caught and it results in a SUCCESS (since the
test blocking behavior is to never block).

The ResultConditionTest test covers that BlockingBehavior is able to map
NOT_BUILD and ABORTED since it has two tests explicitly cancelling and
interrupting jobs.

Examples
--------

When a build is ongoing and when aborting it:

  Waiting for the completion of downstream-project
  downstream-project jenkinsci#7 started.
  downstream-project jenkinsci#7 completed. Result was ABORTED
  Build step 'Trigger/call builds on other projects' marked build as failure
  Finished: FAILURE

When it is waiting in the build queue and get cancelled:

  Waiting for the completion of downstream-project
  Not built: downstream-project has been cancelled while waiting in the queue.
  Build step 'Trigger/call builds on other projects' marked build as failure
  Finished: FAILURE
@hashar hashar force-pushed the handle-not_built-canceled branch from d1e4c3a to ee51c9c Compare November 18, 2024 09:52
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.

1 participant