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

Switch jenkins.version recommendation to be 2.xxx.3 rather than 2.xxx.1 (except for latest line) #4876

Merged
merged 6 commits into from
May 16, 2022

Conversation

jglick
Copy link
Contributor

@jglick jglick commented Jan 31, 2022

Not complete as this would require updating the automated version selection algorithm. Not going to bother with that unless and until there is consensus that we should make this policy change.

Amends #3655; see #3643 (comment), #3630 (comment), jenkinsci/pipeline-maven-plugin#368 (comment), jenkinsci/archetypes#376 (comment), jenkinsci/git-plugin#1212 (comment), etc.

We have not been able to consistently get plugins at the bottom of the food chain to use 2.xxx.1 dependencies. In a number of cases it would not even make sense, because a subsequent security advisory (typically 2.xxx.2 or 2.xxx.3) involved material changes in behavior that affects functional tests.

@probot-autolabeler probot-autolabeler bot added the documentation Jenkins documentation, including user and developer docs, solution pages, etc. label Jan 31, 2022
@jglick
Copy link
Contributor Author

jglick commented Jan 31, 2022

I am not permitted to assign reviewers, so CCing at least those people who seem to have had opinions on this topic in the past: @MarkEWaite @timja @daniel-beck @uhafner

@MarkEWaite
Copy link
Contributor

I am 👍 for this change. I think it makes sense to use the latest release of the base version (in my case 2.289.3).

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.

+1, not approving as other changes required.

agree with reasoning above

@uhafner
Copy link
Contributor

uhafner commented Jan 31, 2022

I am 👎 because that produces more work for an unrelated benefit (make users aware that there is a more modern LTS).

Example for the additional work required: I want to update my plugins very soon to the upcoming LTS version to consume the new UI changes (new UI classes). Since there is no .2 or .3 yet, I need to use the .1 version (or need to wait another 12 weeks). That means if the .2 version will be released a couple of weeks later I need to create a new release of my plugins with nothing new in it, just the LTS version upgrade. And when .3 release is ready then I need to make a release again. Hopefully .4 are not required anymore...

@jglick
Copy link
Contributor Author

jglick commented Jan 31, 2022

make users aware that there is a more modern LTS

That is not at all the goal of this policy change.

update my plugins very soon to the upcoming LTS version

The current LTS line should perhaps be excluded; see #4876 (comment).

if the .2 version will be released a couple of weeks later I need to create a new release of my plugins with nothing new in it, just the LTS version upgrade

I certainly did not intend to recommend that. There is no harm in depending on a 2.xxx.1 version; but our current policy specifically discourages you from ever depending on anything else.

The question here is what would be a good choice of baseline version for two cases:

@uhafner
Copy link
Contributor

uhafner commented Jan 31, 2022

make users aware that there is a more modern LTS

That is not at all the goal of this policy change.

Ok sorry, then I misinterpreted the discussion. From my observation I thought that 99% of plugin integration tests will be not affected by the actual patch level of an LTS release. Maybe I am somewhat biased because I needed to change my baseline selection from 1.277.3 to 1.277.4 in all my plugins because of jenkinsci/pipeline-maven-plugin#368 (comment). So I wanted to minimize the probability that this happens again.

There is no harm in depending on a 2.xxx.1 version; but our current policy specifically discourages you from ever depending on anything else.

Ok, then I am fine with this PR and hope that it will not create too many unwanted release upgrades anymore.

@jglick
Copy link
Contributor Author

jglick commented Jan 31, 2022

I thought that 99% of plugin integration tests will be not affected by the actual patch level of an LTS release.

Probably true?

I needed to change my baseline selection from 1.277.3 to 1.277.4 in all my plugins because of …

Maybe you meant because of jenkinsci/workflow-multibranch-plugin#103? So, yes, you would need to pick 1.277.4 for your plugins if one of their dependencies used .4.

I am not sure I am properly following your concerns.

Co-authored-by: Tim Jacomb <[email protected]>
@daniel-beck
Copy link
Contributor

daniel-beck commented Jan 31, 2022

It's unclear in what configurations this actually improves the situation. Could you provide examples of unavoidable (or perhaps at least justified) core dependency increases to later releases in the same LTS baseline, ideally in plugins that are in BOM scope?

Major intentional behavioral changes during an LTS line are rare (probably only security fixes), and those would take 6+ months to trickle down to the baseline recommendations given here anyway, at which point a plugin caring about this could just go with the subsequent .1 (or figure out another way to make the PCT happy).

This suggestion also (as given in the existing explanation) causes a problem of compatibility with weekly releases that do not yet have whatever changes were backported into the ongoing LTS line, both behavior changes and simple bad (unannotated) backports of new API.

On the "pro" side we now have more flexible update site tiers (since June 2020, which I think some discussion on the topic pre-dates). Previously, we ended up suggesting plugins requiring 2.xxx.3 to users on 2.xxx.1 and similar, that is no longer the case.

@uhafner
Copy link
Contributor

uhafner commented Feb 1, 2022

I am not sure I am properly following your concerns.

Well I just want to avoid unnecessary work. I think that with the new policy more people need to update their plugins just because a dependency switches to a new Jenkins baseline (same minor release, larger patch release). And that the number of plugins that have a benefit from the new policy is smaller. That is all.

@jglick
Copy link
Contributor Author

jglick commented Feb 1, 2022

Could you provide examples of unavoidable (or perhaps at least justified) core dependency increases to later releases in the same LTS baseline, ideally in plugins that are in BOM scope?

Yes there have some of these. jenkinsci/workflow-durable-task-step-plugin#177 is the first one that comes to mind. Usually in response to some nontrivial security advisory.

those would take 6+ months to trickle down to the baseline recommendations given here anyway

I think you are missing the point, which is that downstream plugins which pick 2.xxx.1 wind up being forced to move to 2.xxx.3 (or similar) sooner or later anyway because of some dependency. So why not start on 2.xxx.3?

a problem of compatibility with weekly releases that do not yet have whatever changes were backported into the ongoing LTS line

Yes but all such changes are supposed to be in the newest weekly, and we do not “support” older weeklies. Anyway this is already true of things in 2.xxx.1 not in 2.(xxx+1) right?

I just want to avoid unnecessary work

So do I.

@MarkEWaite
Copy link
Contributor

I think that with the new policy more people need to update their plugins just because a dependency switches to a new Jenkins baseline (same minor release, larger patch release). And that the number of plugins that have a benefit from the new policy is smaller. That is all.

I think the result of the new policy will result in less work rather than more work, though my logic may be flawed.

The recommendation guides the plugin maintainer to choose the final release of LTS-1 (2.303.3) or LTS-2 (2.289.3) as the Jenkins minimum version. Those minimum versions are not receiving new releases because the new releases happen on LTS (2.319.x), not on LTS-1 or LTS-2.

By choosing a Jenkins minimum version 2.303.3 instead of choosing 2.303.1, I've assured that the plugin won't update to a newer version on that LTS line because there won't be a newer version on that LTS line.

I agree that the recommendation does not handle the case that you noted earlier where a plugin needs to set its minimum version to LTS rather than LTS-1 or LTS-2. I don't have a concept for that case, since a plugin can't declare a dependency on an LTS version that has not been released.

@daniel-beck
Copy link
Contributor

daniel-beck commented Feb 2, 2022

downstream plugins which pick 2.xxx.1 wind up being forced to move to 2.xxx.3 (or similar) sooner or later anyway because of some dependency

That's a problem with unnecessary updates upstream. Even your example is just test code and could probably have been done with an assume for the core version.

In particular, at the time 2.303.x wasn't a historical line, so this advice would not have applied – that's what I meant that plugins could just go with the next .1 when they depend on historical lines.

So rather than fix the root cause -- unnecessary core dependency updates in upstream plugins (or perhaps unnecessary inclusion of those plugin releases in the BOM), this simply adjusts the documentation to make the current problematic state a recommendation.

Yes but all such changes are supposed to be in the newest weekly, and we do not “support” older weeklies. Anyway this is already true of things in 2.xxx.1 not in 2.(xxx+1) right?

True, but both the number of changes and the number of affected weekly releases is lower.

IIRC, a long time ago (when the wiki had the docs), the recommendation was to depend on the weekly release that became an LTS baseline, e.g. 2.319. 😄

Copy link
Contributor

@daniel-beck daniel-beck left a comment

Choose a reason for hiding this comment

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

I'm not convinced this is a good choice, but I'm not confident it'll be a problem either. Limiting this to historical lines is an important aspect here. Infrastructure also supports this.

@daniel-beck
Copy link
Contributor

Suggested scripting enhancement: Ideally this would list some latest releases, to prevent people shortcutting blindly to .3 when it may well be .4.

@uhafner
Copy link
Contributor

uhafner commented Feb 2, 2022

I think the result of the new policy will result in less work rather than more work, though my logic may be flawed.

Given that everybody is actually using the recommendation: If the number of plugins x that actually need an LTS 2.x.3 to get their tests passed is larger than the number of plugins y that need an LTS that has no 2.x.3 version yet, then yes. But my personal feeling is that y > x. But anyway, let´s use the new recommendation, the numbers x and y are so small that we do not need to make the discussion longer than the actual release times...

Suggested scripting enhancement: Ideally this would list some latest releases, to prevent people shortcutting blindly to .3 when it may well be .4.

Even better: can't we get an injected test that checks this (or another static analysis rule)?

@daniel-beck
Copy link
Contributor

daniel-beck commented Feb 2, 2022

Dev list discussion triggering this proposal was https://groups.google.com/g/jenkinsci-dev/c/VVToMZjVV14/m/VkZr-Q2iAgAJ

Caused by jenkinsci/configuration-as-code-plugin#1840 which went with .3 for apparently no reason, it builds even with .1.

@jglick
Copy link
Contributor Author

jglick commented Feb 2, 2022

Even your example is just test code and could probably have been done with an assume for the core version.

Possibly yes in this case, just leaves behind technical debt and less test coverage. The question remains why we should bother compiling and testing against a version we know to be obsolete and which no one should be running.

To some extent the same is true of any prior LTS lines, though admins are I suppose more likely to upgrade “micro” than “minor” releases in response to a security advisory, so plugin authors are reluctant to depend on the current line without a specific reason. (The SemVer-like format is of course bogus since we have no actual intention of changing the “major” version even when making incompatible changes.)

at the time 2.303.x wasn't a historical line, so this advice would not have applied

The way I was trying to word things was that for the current line you should go with 2.xxx.1 by default but should not hesitate to use a later micro version if there is a specific reason for the update.

this simply adjusts the documentation to make the current problematic state a recommendation

Yes, depending on whether you consider the current state to actually be problematic. To my mind the only problem is downstream plugins fighting to stay on 2.xxx.1.

Sounds like there is no actual consensus here. OTOH there is no permanent damage done by switching recommendations—if we decide in a year that we do after all want to force upstream plugins to stay on 2.xxx.1 whenever at all possible, we just revert the docs and in a few months downstream plugins depending on 2.xxx.3 from an older line will have the choice of either doing nothing or switching to a recent LTS line for which all deps are compatible with 2.xxx.1.

@jglick
Copy link
Contributor Author

jglick commented Feb 2, 2022

unnecessary inclusion of those plugin releases in the BOM

FYI this is a matter of policy: https://github.com/jenkinsci/bom/blob/043a03ff60a771a0c8af816a64cce14cfad2ec33/sample-plugin/pom.xml#L453

@daniel-beck
Copy link
Contributor

FYI this is a matter of policy: https://github.com/jenkinsci/bom/blob/043a03ff60a771a0c8af816a64cce14cfad2ec33/sample-plugin/pom.xml#L453

So, basically, adjust BOM definition to match this existing recommendation and we're done here? 😄

@jglick
Copy link
Contributor Author

jglick commented Feb 2, 2022

adjust BOM definition to match this existing recommendation

Also jenkinsci/archetypes#376 (comment), and file PRs for all plugins of interest currently using 2.xxx.3. Yes that is an option, if not one I personally like much. Would mean that a plugin update which does require, say, 2.303.3 would show up in the 2.319.x and weekly BOMs but not in the 2.303.x BOM.

@daniel-beck
Copy link
Contributor

daniel-beck commented Feb 2, 2022

Would mean that a plugin update which does require, say, 2.303.3 would show up in the 2.319.x and weekly BOMs but not in the 2.303.x BOM.

As a minimum dependency; it's not like this prevents admins from updating. And plugins could still override the version from the BOM if they need a newer dependency for features.

This also would serve as motivation to keep to .1 if you want downstream plugins to depend on your newest releases. I expect it's much more common for new point releases on historical lines to be chosen arbitrarily rather than from necessity.

@jglick
Copy link
Contributor Author

jglick commented Feb 2, 2022

Any other opinions (@timja / @MarkEWaite / @uhafner) given the above discussion?

@jglick jglick marked this pull request as ready for review May 12, 2022 21:27
@jglick jglick requested a review from a team as a code owner May 12, 2022 21:27
@jglick jglick requested review from daniel-beck and timja May 12, 2022 21:27
@jglick jglick changed the title Switch jenkins.version recommendation to be 2.xxx.3 rather than 2.xxx.1 Switch jenkins.version recommendation to be 2.xxx.3 rather than 2.xxx.1 (except for latest line) May 12, 2022
Copy link
Contributor

@MarkEWaite MarkEWaite left a comment

Choose a reason for hiding this comment

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

This is great. Thanks so much for bringing this to consensus. I think we're ready to merge.

Copy link
Contributor

@daniel-beck daniel-beck left a comment

Choose a reason for hiding this comment

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

Thanks!

@daniel-beck
Copy link
Contributor

Screenshot 2022-05-13 at 23 27 00

uhafner added a commit to jenkinsci/analysis-pom-plugin that referenced this pull request May 14, 2022
@MarkEWaite
Copy link
Contributor

Thanks to everyone for the improvements here. Merging with 4 approvals

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Jenkins documentation, including user and developer docs, solution pages, etc.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants