Clarify safe restart won't wait for Pipeline jobs

[omehegan]

A customer raised the fact that this text was a little confusing, as the service will restart even when Pipeline jobs are running - it only waits for Freestyle etc. jobs.

Proposed changelog entries

Clarify safe restart won't wait for Pipeline jobs.

Proposed upgrade guidelines

N/A

Submitter checklist

• The Jira issue, if it exists, is well-described.
• The changelog entries and upgrade guidelines are appropriate for the audience affected by the change (users or developers, depending on the change) and are in the imperative mood (see examples).
  • Fill in the Proposed upgrade guidelines section only if there are breaking changes or changes that may require extra steps from users during upgrade.
• There is automated testing or an explanation as to why this change has no tests.
• New public classes, fields, and methods are annotated with @Restricted or have @since TODO Javadocs, as appropriate.
• New deprecations are annotated with @Deprecated(since = "TODO") or @Deprecated(forRemoval = true, since = "TODO"), if applicable.
• New or substantially changed JavaScript is not defined inline and does not call eval to ease future introduction of Content Security Policy (CSP) directives (see documentation).
• For dependency updates, there are links to external changelogs and, if possible, full differentials.
• For new APIs and extension points, there is a link to at least one consumer.

Desired reviewers

@mention

Maintainer checklist

Before the changes are marked as ready-for-merge:

• There are at least two (2) approvals for the pull request and no outstanding requests for change.
• Conversations in the pull request are over, or it is explicit that a reviewer is not blocking the change.
• Changelog entries in the pull request title and/or Proposed changelog entries are accurate, human-readable, and in the imperative mood.
• Proper changelog labels are set so that the changelog can be generated automatically.
• If the change needs additional upgrade steps from users, the upgrade-guide-needed label is set and there is a Proposed upgrade guidelines section in the pull request title (see example).
• If it would make sense to backport the change to LTS, a Jira issue must exist, be a Bug or Improvement, and be labeled as lts-candidate to be considered (see query).

[daniel-beck]

This isn't any truer than the current content AFAICT. Parts of Pipeline executions block restart as well.

[omehegan]

This isn't any truer than the current content AFAICT. Parts of Pipeline executions block restart as well.

Fair enough, I guess I had forgotten that. Not sure how else to phrase it without being either too descriptive or uselessly vague, but I felt it was a fair point for this person to raise. "A running Pipeline job may allow a restart depending on its running state" ? Seems too vague. But if I say, "if it is in a safe state" it seems to immediately beg the question of what states are safe vs. unsafe, etc.

[basil]

it seems to immediately beg the question

Raise the question. "Begging the question" is a form of logical fallacy. See http://begthequestion.info/

Pedantry aside, I think we are in agreement that the existing documentation is incomplete and does not cover the Pipeline case. This is bad for end users, so this PR (which makes the documentation more complete) is very welcome! The problem is that the Pipeline case is more complicated to describe correctly than what is currently proposed in this pull request.

The Pipeline behavior is described in this code comment:

https://github.com/jenkinsci/workflow-step-api-plugin/blob/6ecacd8c04aa1326eab48553730541b0abc9c1f9/src/main/java/org/jenkinsci/plugins/workflow/steps/StepExecution.java#L132-L143

So Pipeline steps generally do not block restart, except for:

• Short-lived scheduling activity in the CPS VM thread
• Synchronous steps using a background thread, such as SynchronousNonBlockingStepExecution or GeneralNonBlockingStepExecution

Unfortunately, accurately describing both of these concepts would require referencing internal implementation details that would be out of place in user-facing documentation.

I think it would suffice to say that Pipeline jobs usually do not block a restart except for brief periods of internal housekeeping or synchronous activity (intentionally vaguely defined).

Perhaps @dwnusbaum might be interested in helping us to come up with a description in a way that is high-level enough for end users to understand but yet still accurate to the underlying implementation.

[dwnusbaum]

I tried to add some suggestions.

It might make sense to track this against JENKINS-65524 and then close that issue as "Not a defect" but note that we are improving the documentation to make the behavior clearer.

For what it's worth, see also JENKINS-60434 where users want to disable this behavior and run Pipeline builds to completion when Jenkins is in quiet mode.

Like Basil mentioned, a full description of what is happening is kind of complicated. Here are the main points if it helps you rephrase things more succinctly:

• If a Pipeline is running an asynchronous step that does not block restart, like sh, then the Pipeline continues running (generally because nothing is happening on the controller's side), and Jenkins is allowed to restart while the build is still running
  • If this step completes, then the Pipeline will be paused (and Jenkins will still be able to restart)
• If a Pipeline is actively running CPS-transformed code or a step whose execution blocks restart, then the Pipeline will continue executing until the code yields or the step completes, at which point the Pipeline will be paused and will no longer block Jenkins from restarting
• When Jenkins restarts, it will resume the Pipeline (unpausing it if necessary), and resume the execution of Pipeline steps that were running across the restart

[daniel-beck]

WDYT? Is this useful or overkill? (Message is way too long, this evolved from a simple rephrasing to lists of stuff…)

WIP at https://github.com/daniel-beck/jenkins/tree/restart-message

[dwnusbaum]

The idea seems good to me. I would consider hiding the new details behind a <details> element or something like that and loading the info asynchronously via JS when requested to avoid potential issues on busy instances. Maybe we could use help buttons to add more detail about the behavior of Pipeline builds to the "temporarily blocking" and "non blocking" sections so that we can keep the main text relatively simple.

[omehegan]

Great discussion, thanks @daniel-beck and @dwnusbaum!

@daniel-beck I think your idea is really interesting and not necessarily too long/detailed. One change I would make is to move "Are you sure you want to restart Jenkins?" down to the bottom so it is adjacent to the Yes button.

Given that we are proposing to show a list of blocking tasks, maybe that allows us to scrap or condense the explanation. Now we can just say, "Jenkins will restart when these blocking tasks complete, or immediately if there are none shown. [Running Pipeline jobs will pause at a safe point and resume after restart.]" Second sentence optional for added clarity - answers the possibly implied question of "why aren't my running Pipeline jobs blocking the restart?"

[StefanSpieker]

@daniel-beck I really like the idea of listing blocking jobs. With many jobs on many agents I sometimes wonder which job still blocks the restart.

[basil]

Proposing for close, as this PR is not suitable in its current form and the suggestions previously given have not been applied to this PR.

[daniel-beck]

FTR I am unsure when I will find the time to finish my proposal. Anyone else is free to take that over and finish it (ideally let me know so we don't end up both working on it at the same time).

[omehegan]

I can apply @dwnusbaum's suggestions, I was just holding off in case @daniel-beck landed his feature improvement.

[basil]

This PR is now ready for merge. We will merge it after approximately 24 hours if there is no negative feedback. Please see the merge process documentation for more information about the merge process. Thanks!

[daniel-beck]

Caused https://issues.jenkins.io/browse/JENKINS-69904; indicating that nobody ran this after applying suggested changes.

[dwnusbaum]

Oops, yeah parentheses cannot be used inside of ${% } directly, so the value needs to be moved to a properties file.

[dwnusbaum]

Also, this broke all of the existing localizations.

[dwnusbaum]

Actually #3871 broke the existing localizations first. This PR only broke that ones that had been fixed in the meantime.

[dwnusbaum]

That said, I'm not sure if our preference is to intentionally break existing localizations when the English text changes so that localizations are never missing information compared to the latest version of the English text, or to preserve existing localizations in cases where the general meaning of the English text has been preserved.

[dwnusbaum]

See #7286.