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

Backport of ClassCastException fix for JDK 11u16 #1381

Merged
merged 1 commit into from
Aug 5, 2022

Conversation

jglick
Copy link
Member

@jglick jglick commented Aug 5, 2022

In https://github.com/jenkinsci/workflow-cps-plugin/releases/tag/2660.2664.v4c114e93f4c1 I backported jenkinsci/workflow-cps-plugin#543 which (among other things) solves an error reproducible when using JDK 11u16 but not 11u15:

JAVA_HOME=…/jdk-11.0.16+8 LINE=2.319.x PLUGINS=pipeline-model-definition TEST=DurabilityTest bash local-test.sh

I guess CI agents recently updated Java versions @dduportal? Blocking an important #1379 (comment) and also seen in #1380 (https://github.com/jenkinsci/bom/pull/1380/checks?check_run_id=7686555358). Originally released in https://github.com/jenkinsci/workflow-cps-plugin/releases/tag/2705.v0449852ee36f but jenkinsci/workflow-cps-plugin#512 restricted that to a newer line so #940 did not pick this up on 2.319.x.

@jglick jglick added the dependencies Pull requests that update a dependency file label Aug 5, 2022
@jglick jglick enabled auto-merge (squash) August 5, 2022 14:52
@basil
Copy link
Member

basil commented Aug 5, 2022

Nice! Great example of BOM/PCT testing catching real issues for the benefit of Jenkins users.

@basil
Copy link
Member

basil commented Aug 5, 2022

FYI the same logic is present in jenkinsci/script-security-plugin#420 which only seems to have made it out as far back as 2.332.1, so we may want to consider a backport of that as well for completeness.

@jglick
Copy link
Member Author

jglick commented Aug 5, 2022

we may want to consider a backport of that as well

Could do that too. In that case the fix is to SecureGroovyScript which is used by a relatively minor assortment of plugins. I will see if I have time to do it.

@dduportal
Copy link
Contributor

In https://github.com/jenkinsci/workflow-cps-plugin/releases/tag/2660.2664.v4c114e93f4c1 I backported jenkinsci/workflow-cps-plugin#543 which (among other things) solves an error reproducible when using JDK 11u16 but not 11u15:

JAVA_HOME=…/jdk-11.0.16+8 LINE=2.319.x PLUGINS=pipeline-model-definition TEST=DurabilityTest bash local-test.sh

I guess CI agents recently updated Java versions @dduportal? Blocking an important #1379 (comment) and also seen in #1380 (https://github.com/jenkinsci/bom/pull/1380/checks?check_run_id=7686555358). Originally released in https://github.com/jenkinsci/workflow-cps-plugin/releases/tag/2705.v0449852ee36f but jenkinsci/workflow-cps-plugin#512 restricted that to a newer line so #940 did not pick this up on 2.319.x.

I'm not sure if I understood everything, but I can confirm that the infra was upgraded earlier this week to use the latest 11u16 JDK:

Did this JDK upgrade broke things (additionnaly to the trilead thing)?

@jglick
Copy link
Member Author

jglick commented Aug 5, 2022

Did this JDK upgrade broke things

Well, yes, it broke builds of this repository, which this PR corrects by picking up a change to production code that fixes a bug affecting u16 which I just backported to an older LTS line. So a good thing except to the extent that the failures were unexpected (interrupted progress on other things) and took me a while to track down since they were not associated with some versioned change.

@dduportal
Copy link
Contributor

Did this JDK upgrade broke things

Well, yes, it broke builds of this repository, which this PR corrects by picking up a change to production code that fixes a bug affecting u16 which I just backported to an older LTS line. So a good thing except to the extent that the failures were unexpected (interrupted progress on other things) and took me a while to track down since they were not associated with some versioned change.

First of all, really sorry for this unexpected change and the wasted time. With the holidays rotation, we (infra-team) failed our hand overs and our communications.

Thanks for the clarification. It's the 2nd time this year that the we (the infra team) are breaking builds like this by updating really early. @basil already mentionned this with the Maven 3.8.5 update.

I propose the following 2 improvements on the future, the goal is to limit the waste of time WDYT of them?

  • Communicate on the developer mailing list every time there is a JDK / Maven tools update (both Jenkins tools and agent VM templates)
  • Ensure that developer can specify a custom JDK / Maven tool version which is not the default: that mean infra-team would always keep the version N and N-1 of each JDK and Maven. So we can keep updating soon, BUT we provide a way to not having to waste time for developers by pinning the JDK version to the previous one.

@timja
Copy link
Member

timja commented Aug 5, 2022

Ensure that developer can specify a custom JDK / Maven tool version which is not the default: that mean infra-team would always keep the version N and N-1 of each JDK and Maven. So we can keep updating soon, BUT we provide a way to not having to waste time for developers by pinning the JDK version to the previous one.

seems overkill to me

Communicate on the developer mailing list every time there is a JDK / Maven tools update (both Jenkins tools and agent VM templates)

+1, but don't go too overboard on every update, people can always subscribe to releases of https://github.com/jenkins-infra/packer-images/releases/tag/0.37.0

@jglick
Copy link
Member Author

jglick commented Aug 5, 2022

Communicate on the developer mailing list every time there is a JDK / Maven tools update

That would be helpful and sounds simple enough.

Ensure that developer can specify a custom JDK / Maven tool version

Yes but doing it right requires some forethought. There would need to be a way to specify a particular tool set version in project sources, with some assurances that this image or whatever will be available indefinitely (so you can go back and play with branches). And there needs to be a system, like Dependabot or updatecli, for automatically proposing updates in a timely PR flow. In this case such a system would have offered an update from u15 to u16, which would have gotten a failing check which we would immediately suspect was caused by the Java update, unrelated PRs could go through CD, and the fix of the test failures (in this case a plugin backport) would go into that automated PR or something filed by hand to subsume it. The cost is of course a higher volume of PRs, most of which are uninteresting.

I have proposed something like this for pipeline-library actually, since occasionally an innocent-sounding change turns out to be incompatible in a few places. That case may be tougher since some parts of the library are really tied to server configuration or something that actually has to be updates for everyone simultaneously.

@jglick jglick merged commit d76bba2 into jenkinsci:master Aug 5, 2022
@jglick jglick deleted the DurabilityTest branch August 7, 2022 20:30
@jglick
Copy link
Member Author

jglick commented Oct 6, 2022

consider a backport of that as well

Never found the time, but FTR if anyone (perhaps my future self) wishes to clean up: jenkinsci/script-security-plugin#161 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants