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

Require Java 11 infrastructure thread #2945

Closed
basil opened this issue May 18, 2022 · 31 comments
Closed

Require Java 11 infrastructure thread #2945

basil opened this issue May 18, 2022 · 31 comments

Comments

@basil
Copy link
Collaborator

basil commented May 18, 2022

Service(s)

release.ci.jenkins.io

Summary

With the announcement that weekly releases will require Java 11 as of June 21, 2022, I have begun preparing for this major change from an infrastructure perspective. My goal in this thread is to identify all the work that needs to be done on the infrastructure side when weeklies begin requiring Java 11. Note that weeklies will require Java 11 before LTS releases do (in September).

The jenkinsciinfra/packaging:latest Docker image contains Java 8 in /opt/jdk-8 and Java 11 in /opt/jdk-11. However, the default Java command is Java 8, and this is also the version of Java used for Maven. Although jenkins-infra/release/PodTemplates.d/release-linux.yaml defines JENKINS_JAVA_BIN to use Java 11 for the agent connection to the controller (and I confirmed this is set by examining the console output in the release job), the Maven commands seem to be using Java 8 to actually compile Jenkins and perform the release. We will want to continue using Java 8 for LTS releases, but not for weekly releases. I think the solution here is to modify the job to set the PATH and JAVA_HOME environment variables to the Java 11 versions when building/releasing a weekly.

The job for jenkinsci/packaging, which is executed in the same Docker image, invokes mvn org.kohsuke:apt-ftparchive-merge:1.6:merge in the various publish.sh scripts, currently with Java 8. I think this should work when Maven is invoked using Java 11 with no code changes, but I do not believe it has been tested, nor do I see any practical way of testing it outside of production.

jenkinsci/docker currently produces Java 8 and Java 11 Docker images for weekly releases. This needs to be changed to produce only Java 11 Docker images for weekly releases but to produce Java 8 and Java 11 Docker images for LTS releases. I foresee this being a challenge, because as I understand it this repository has no stable branch, i.e. the main branch is used for both weekly and LTS releases. So I think we will have to add conditional logic in the repository to skip Java 8 when building/releasing weeklies.

I could not find any Java 8 references in jenkinsci/helm-charts. This repository looks ready to go for Java 11.

If something comes to mind that I have missed, I would greatly appreciate it if you could bring it up now so that we can do proper planning. My goal is to come to a consensus on the set of tasks prior to the delivery date.

Reproduction steps

Try to release Jenkins with Java 11. 😄

@github-actions github-actions bot added release.ci.jenkins.io triage Incoming issues that need review labels May 18, 2022
@timja
Copy link
Member

timja commented May 18, 2022

For the packaging bits It should work just fine on Java 11 I don’t think we need to do both, but for the highest safety sure that’s the way to go.

for stable docker we can probably just push a temporary branch and update the release docs for the existing LTS line?

@dduportal
Copy link
Contributor

for stable docker we can probably just push a temporary branch and update the release docs for the existing LTS line?

WDYT about using the docker bake HCL logic to implement the condition (specifying the JAVA_VERSION as build arg)?

@timja
Copy link
Member

timja commented May 18, 2022

sure that would be nicer if someone has time to work on it 👍

@timja
Copy link
Member

timja commented May 18, 2022

I guess the script would set a minimum_java_version depending on whether it's a weekly or lts ?

@dduportal
Copy link
Contributor

sure that would be nicer if someone has time to work on it 👍

I volunteer as I'm interested. still need to decide if doing it as part of my SRE job or as contributor (depends on team priorities).

@dduportal
Copy link
Contributor

I guess the script would set a minimum_java_version depending on whether it's a weekly or lts ?

That is the idea (at least superficial idea).

@basil
Copy link
Collaborator Author

basil commented May 18, 2022

For the packaging bits It should work just fine on Java 11 I don’t think we need to do both, but for the highest safety sure that’s the way to go.

Not clear to me what "it" or "both" are referring to in this sentence.

@timja
Copy link
Member

timja commented May 18, 2022

The jenkinsciinfra/packaging:latest Docker image contains Java 8 in /opt/jdk-8 and Java 11 in /opt/jdk-11. However, the default Java command is Java 8, and this is also the version of Java used for Maven. Although jenkins-infra/release/PodTemplates.d/release-linux.yaml defines JENKINS_JAVA_BIN to use Java 11 for the agent connection to the controller (and I confirmed this is set by examining the console output in the release job), the Maven commands seem to be using Java 8 to actually compile Jenkins and perform the release. We will want to continue using Java 8 for LTS releases, but not for weekly releases. I think the solution here is to modify the job to set the PATH and JAVA_HOME environment variables to the Java 11 versions when building/releasing a weekly.

refers to the above.
I don't anticipate any issues switching it to Java 11, but we could keep it frozen on 8.
Alternatively we just switch master to 11 in a new container version and use the branches for release (which we already do currently) and don't update the container version

@basil
Copy link
Collaborator Author

basil commented May 18, 2022

Still not clear to me. The above is a long paragraph with dozens of nouns. "It" must be referring to a single noun, but I cannot figure out which one. "Both" must referring to two nouns, but I cannot figure out which two.

@timja
Copy link
Member

timja commented May 18, 2022

We will want to continue using Java 8 for LTS releases, but not for weekly releases

@basil
Copy link
Collaborator Author

basil commented May 18, 2022

I don't anticipate any issues switching it to Java 11, but we could keep it frozen on 8.

So which noun does the pronoun "it" refer to in the above sentence? If it refers to "the job that compiles Jenkins core and performs the core release", then no, we cannot keep it frozen on Java 8. If it refers to "the job that builds packaging and invokes Kohsuke's Maven FTP archive plugin", then yes, we could keep it frozen on Java 8. So to avoid ambiguity and help me understand your points, please be precise and avoid pronouns without a clear antecedent.

@basil
Copy link
Collaborator Author

basil commented May 18, 2022

Here's my summary of the discussion about the packaging image, the release job, and the packaging job. I have not yet read and thought about the discussion about jenkinsci/docker. I will focus on the latter after we reach consensus regarding the former.

Proposal A

  • Keep the jenkinsciinfra/packaging image as-is (Java 8 and Java 11 present, Java 8 used by default). This image is used in both the core/release job as well as the core/packaging job.
  • The core/release job runs in the jenkinsciinfra/packaging image. If and only if RELEASE_PROFILE is equal to weekly, set JAVA_HOME and PATH to use Java 11 rather than Java 8 when invoking the "Prepare Release" stage of this job, which runs Maven to build Jenkins. Functional testing of this change is not needed because https://ci.jenkins.io/job/Core/job/jenkins/ builds are already passing on Java 11.
  • The core/packaging job runs in the jenkinsciinfra/packaging image. If and only if RELEASE_PROFILE is equal to weekly, set JAVA_HOME and PATH to use Java 11 rather than Java 8 when invoking the "Publish" stages of this job, which run mvn org.kohsuke:apt-ftparchive-merge:1.6:merge.
  • Once LTS is converted to use this system, remove Java 8 from the jenkinsciinfra/packaging image.

Pros of Proposal A:

  • Complete elimination of Java 8 from weekly releases.
  • Once LTS releases are converted to the same system, makes it easy to drop Java 8 from the jenkinsciinfra/packaging Docker image and simplify the code in the future.

Cons of Proposal A:

  • Risk of regression when running mvn org.kohsuke:apt-ftparchive-merge:1.6:merge under Java 11 (currently untested, and not easily testable).

Proposal B

  • Keep the jenkinsciinfra/packaging image as-is (Java 8 and Java 11, Java 8 by default). This image is used in both the core/release job as well as the core/packaging job.
  • The core/release job runs in the jenkinsciinfra/packaging image. If and only if RELEASE_PROFILE is equal to weekly, set JAVA_HOME and PATH to use Java 11 rather than Java 8 when invoking the "Prepare Release" stage of this job, which runs Maven to build Jenkins. Functional testing of this change is not needed because https://ci.jenkins.io/job/Core/job/jenkins/ builds are already passing on Java 11.
  • The core/packaging job runs in the jenkinsciinfra/packaging image. Continue to use Java 8 for this image.

Pros of Proposal B:

  • Little to no risk, as we are not changing the invocation of mvn org.kohsuke:apt-ftparchive-merge:1.6:merge in the core/packaging job, and we are confident that the invocation of Maven in the "Prepare Release" stage of the core/release job will work on Java 11.

Cons of Proposal B:

  • Java 8 not completely eliminated from weekly releases.
  • Once LTS releases are converted to the same system, we will not be able to drop Java 8 from the jenkinsciinfra/packaging Docker image and simplify the code, because it will remain in use by mvn org.kohsuke:apt-ftparchive-merge:1.6:merge in the core/packaging job.

Can we take a decision regarding Proposal A vs Proposal B? (Or feel free to ask me to modify Proposal A or Proposal B, or to create a new Proposal C.) My vote would be: be bold and attempt Proposal A. If, on the day of release, the mvn org.kohsuke:apt-ftparchive-merge:1.6:merge invocation in the core/packaging job fails when run for the first time under Java 11, hotfix the job to revert to Proposal B and file a bug to address the long term problem that prevents us from using Proposal A. If Proposal A succeeds, then great! Effectively, my vote is "Test Proposal A in production." 😎

If there is consensus regarding this, I will file the PR for Proposal A (in draft state until June 21, 2022). Then I will read through the discussion about jenkinsci/docker in this thread.

@timja
Copy link
Member

timja commented May 18, 2022

A is fine with me 👍

@basil
Copy link
Collaborator Author

basil commented May 18, 2022

Great! If Damien agrees I will take the action item to file the PR.

@dduportal
Copy link
Contributor

A is fine with me (thanks for taking time to exchange and propose these choices folks!)

@basil
Copy link
Collaborator Author

basil commented May 18, 2022

I created jenkins-infra/release#233.

@basil
Copy link
Collaborator Author

basil commented May 18, 2022

Docker changes

I don't have access to trusted.ci.jenkins.io (nor do I really want it) and therefore have no visibility into the "Containers/Core Release Containers" job(s) there. Based on my reading of the code in jenkinsci/docker, I am assuming that there are two jobs, one to build the latest LTS (which sets LATEST_LTS to true and invokes the Jenkinsfile in jenkinsci/docker) and one to build the latest weekly (which sets LATEST_WEEKLY to true and invokes the Jenkinsfile in jenkinsci/docker). I am assuming LATEST_WEEKLY and LATEST_LTS are never both set to true in the same job. If these assumptions are incorrect, please correct me and then disregard the rest of this post.

Proposal A

for stable docker we can probably just push a temporary branch

Calling this "Proposal A", I think I understand the proposal: To create a temporary branch, remove all references to alpine_jdk8, centos7_jdk8, debian_jdk8, and debian_slim_jdk8 from jenkinsci/docker on that branch, and then run the job that sets LATEST_WEEKLY to true (but not the job that sets LATEST_LTS to true) using that branch of jenkinsci/docker.

(Conversely, we could remove all references to alpine_jdk8, centos7_jdk8, debian_jdk8, and debian_slim_jdk8 from the main branch and create a stable branch that retains them for use with LTS releases. This variation might be called Proposal A2. But its pros and cons seem identical.)

Pros of Proposal A:

Easy to implement in the short term, as is common with branching-based solutions.

Cons of Proposal A:

Not the most elegant solution in the long term. The branch needs to be kept up-to-date. On the other hand, it is throwaway code that will not be needed after fall when the LTS release switches to Java 11. So it might not be so bad.

Proposal B

Instead of creating a temporary branch, implement a good long-term solution: add a new MINIMUM_JAVA_VERSION argument to the Docker Bake HCL logic. Conditionally exclude alpine_jdk8, centos7_jdk8, debian_jdk8, and debian_slim_jdk8 within the existing single branch based on this parameter. Update the Jenkins jobs on trusted.ci.jenkins.io to pass in this new parameter, setting it to 8 for the LTS job and 11 for the weekly job.

Pros of Proposal B:

More elegant solution as it eliminates the need to maintain a separate branch.

Cons of Proposal B:

Nontrivial to implement, at least for me. Could probably be done efficiently over a night/weekend by someone familiar with this logic, or a bit longer than that by someone like me who would need to learn it from scratch.

Decision

Can we take a decision on this as well? My vote is as follows: I would normally insist on the long-term solution over a short-term workaround, but in this particular case the short-term workaround doesn't seem so bad because it truly is short-term: until the fall LTS starts requiring Java 11. Compounding the issue is my own lack of familiarity with the Docker Bake HCL logic, which precludes me from quickly and efficiently implementing Proposal B. I highly appreciate Damien's offer to work on Proposal B, but I understand it might not be possible to implement such a solution in a short amount of time. So my vote would be to use an implementation of Proposal B if someone other than me offers it by the June 21, 2022 weekly switchover date, or otherwise to fall back to Proposal A. If Proposal A is selected I am happy to create the PR to remove the JDK 8 variants from whichever branch we want to remove them from (either the main branch as in Proposal A, or a stable branch as in Proposal A2). Regarding A vs A2 (i.e., which branch should the Java 8 variants be deleted from, I have no strong preference.) If you do have a preference, please let me know.

Open Questions

and update the release docs for the existing LTS line?

I looked at the documentation here and here but did not find any references to Java 8, so I do not understand what this is referring to. I have already filed JENKINS-68564, JENKINS-68565, JENKINS-68566, and JENKINS-68567 to cover all the documentation updates I know of that need to occur. If there is a documentation update that needs to occur but is not covered by one of the above tickets, please let me know and I will file a separate ticket for it.

@timja
Copy link
Member

timja commented May 18, 2022

I am assuming that there are two jobs, one to build the latest LTS (which sets LATEST_LTS to true and invokes the Jenkinsfile in jenkinsci/docker) and one to build the latest weekly (which sets LATEST_WEEKLY to true and invokes the Jenkinsfile in jenkinsci/docker)

It's a multi-branch pipeline that currently just builds the master branch (but it is configured to pickup any origin branches) which will publish both LTS and weekly versions

This line of the publish script fetches the last 30 versions and checks if they are published looping through each version:
https://github.com/jenkinsci/docker/blob/master/.ci/publish.sh#L101

Based on the version number format it determines if it's a weekly or LTS release and then runs docker buildx bake on each non published version

LATEST_WEEKLY and LATEST_LTS are never both set to true in the same job. If these assumptions are incorrect, please correct me and then disregard the rest of this post.

Correct they are mutually exclusive.

and update the release docs for the existing LTS line?

Documentation I was referring to is https://github.com/jenkins-infra/release/blob/master/.github/ISSUE_TEMPLATE/1-lts-release-checklist.md

specifically "Run trusted.ci.jenkins.io Docker image creation job."

Which may be updated to refer to a non master branch if proposal A is implemented


Both proposal A and B are fine with me

@basil
Copy link
Collaborator Author

basil commented May 19, 2022

I created jenkinsci/docker#1356 to cover the quick and dirty implementation of Proposal A in case we do not get an implementation of Proposal B.

@dduportal
Copy link
Contributor

I am assuming LATEST_WEEKLY and LATEST_LTS are never both set to true in the same job. If these assumptions are incorrect, please correct me and then disregard the rest of this post.

That is correct.
(I assume that you already got a good handle of the following information, but I'm adding it to the discussion for the later reader).

Please note that we assume that LATEST_LTS and LATEST_WEEKLY are never set to true at the same time based on this logic, but there is no safety mechanism to ensure a fail fast if by any means this situation happen (would be there https://github.com/jenkinsci/docker/blob/master/.ci/publish.sh#L168). Might be wrong but it seems that this case is not prone to happen though.

@dduportal
Copy link
Contributor

Many thanks @basil for writing this proposal! Many thanks @timja for the help and confirmation!

I'm interested in working on the Proposal B for docker, unless someone already started or is eager to do it (I do not mind!).

The fallback proposal A is still an excellent idea!

@basil
Copy link
Collaborator Author

basil commented Jun 24, 2022

Circling back to this thread. I believe the following action items remain:

  • Create a stable-2.346 branch.
  • Configure LTS Docker image builds to use the stable-2.346 branch.
  • Ensure weekly Docker image builds continue to use the master branch.
  • Ensure the 2.356 weekly Docker images were published successfully for Java 8, 11, and 17.
  • Disable/freeze execution of the job so that weekly Docker images are not built again until after 2.357 is released.
  • Merge Removing Java 8 jenkinsci/docker#1356 to the master branch.
  • Release 2.357.
  • Verify 2.357 Docker images were published successfully for Java 11 and 17.
  • Verify no 2.357 Docker image was published for Java 8.

Is anyone interested in taking these tasks?

@timja
Copy link
Member

timja commented Jun 25, 2022

Disable/freeze execution of the job so that weekly Docker images are not built again until after 2.357 is released.

I don't think this is needed, the job checks if images are published and if so does nothing.

@dduportal
Copy link
Contributor

Circling back to this thread. I believe the following action items remain:

* [x]  Create a `stable-2.346` branch.

* [ ]  Configure LTS Docker image builds to use the `stable-2.346` branch.

* [ ]  Ensure weekly Docker image builds continue to use the `master` branch.

* [ ]  Ensure the 2.356 weekly Docker images were published successfully for Java 8, 11, and 17.

* [ ]  Disable/freeze execution of the job so that weekly Docker images are not built again until after 2.357 is released.

* [x]  Merge [Removing Java 8 jenkinsci/docker#1356](https://github.com/jenkinsci/docker/pull/1356) to the `master` branch.

* [ ]  Release 2.357.

* [ ]  Verify 2.357 Docker images were published successfully for Java 11 and 17.

* [ ]  Verify no 2.357 Docker image was published for Java 8.

Is anyone interested in taking these tasks?

Taking it

@dduportal
Copy link
Contributor

dduportal commented Jun 25, 2022

  • Configured the job in trusted.ci.jenkins.io to check the stable-2.346 branch:

Capture d’écran 2022-06-25 à 14 01 24

Started
[Sat Jun 25 12:01:30 UTC 2022] Starting branch indexing...
 > JGit ls-remote # timeout=10
Setting origin to https://github.com/jenkinsci/docker
Fetching & pruning origin...
Listing remote references...
 > JGit ls-remote # timeout=10
 > JGit fetch # timeout=10
Checking branches...
  Checking branch stable-2.346
      ‘Jenkinsfile’ found
    Met criteria
Scheduled build for branch: stable-2.346
  Checking branch master
      ‘Jenkinsfile’ found
    Met criteria
Changes detected: master (423973bd21d330557102e6038600bc00f42a7975 → 2951760f5a10a95588044fe8cb8b9949ac0d1fa6)
Scheduled build for branch: master
Processed 7 branches
[Sat Jun 25 12:01:32 UTC 2022] Finished branch indexing. Indexing took 1.8 sec
Finished: SUCCESS

First job is running, Linux part happenned successfully and the publish part confirms the expected behavior (e.g. no image published):

[2022-06-25T12:01:47.078Z] * JENKINS_REPO: jenkins/jenkins

[2022-06-25T12:01:51.654Z] Tag is already published: 2.329

[2022-06-25T12:01:55.277Z] Tag is already published: 2.330

[2022-06-25T12:01:58.906Z] Tag is already published: 2.331

[2022-06-25T12:02:02.535Z] Tag is already published: 2.332

[2022-06-25T12:02:06.159Z] Tag is already published: 2.332.1

[2022-06-25T12:02:10.733Z] Tag is already published: 2.332.2

[2022-06-25T12:02:13.554Z] Tag is already published: 2.332.3

[2022-06-25T12:02:17.181Z] Tag is already published: 2.332.4

[2022-06-25T12:02:21.763Z] Tag is already published: 2.333

[2022-06-25T12:02:25.392Z] Tag is already published: 2.334

[2022-06-25T12:02:29.023Z] Tag is already published: 2.335

[2022-06-25T12:02:32.651Z] Tag is already published: 2.336

[2022-06-25T12:02:36.279Z] Tag is already published: 2.337

[2022-06-25T12:02:39.908Z] Tag is already published: 2.338

[2022-06-25T12:02:44.487Z] Tag is already published: 2.339

[2022-06-25T12:02:48.121Z] Tag is already published: 2.340

[2022-06-25T12:02:52.701Z] Tag is already published: 2.341

[2022-06-25T12:02:56.337Z] Tag is already published: 2.342

[2022-06-25T12:03:00.915Z] Tag is already published: 2.343

[2022-06-25T12:03:04.543Z] Tag is already published: 2.344

[2022-06-25T12:03:08.168Z] Tag is already published: 2.345

[2022-06-25T12:03:12.750Z] Tag is already published: 2.346

[2022-06-25T12:03:21.401Z] Tag is already published: 2.346.1

[2022-06-25T12:03:25.980Z] Tag is already published: 2.347

[2022-06-25T12:03:29.613Z] Tag is already published: 2.348

[2022-06-25T12:03:34.190Z] Tag is already published: 2.349

[2022-06-25T12:03:37.820Z] Tag is already published: 2.350

[2022-06-25T12:03:42.400Z] Tag is already published: 2.354

[2022-06-25T12:03:46.027Z] Tag is already published: 2.355

[2022-06-25T12:03:53.080Z] Tag is already published: 2.356

@dduportal
Copy link
Contributor

I assume there isn't anything else to do ?

@timja
Copy link
Member

timja commented Jun 25, 2022

The stable branch may try to produce java 8 builds for weekly in its current state? I think that should be checked

the publish check may protect it but I think there’s an all variants checks too

@basil
Copy link
Collaborator Author

basil commented Jun 27, 2022

The stable branch may try to produce java 8 builds for weekly in its current state? I think that should be checked

Are we confident the stable-2.346 branch will not try to produce Java 8 builds for weekly releases in its current state?

@timja
Copy link
Member

timja commented Jun 27, 2022

No but it won’t be triggered in the short term, we have a couple of weeks to solve it I believe

@dduportal dduportal removed the triage Incoming issues that need review label Jun 28, 2022
@smerle33 smerle33 removed this from the infra-team-sync-2022-07-05 milestone Jul 5, 2022
@basil
Copy link
Collaborator Author

basil commented Sep 16, 2022

With the release of Jenkins 2.357 and 2.361.1 LTS, I think our work here is done. If we all agree, please close this ticket. And a HUGE thank you to everyone involved in this effort!

@dduportal
Copy link
Contributor

With the release of Jenkins 2.357 and 2.361.1 LTS, I think our work here is done. If we all agree, please close this ticket. And a HUGE thank you to everyone involved in this effort!

+1, thanks for taking care of this issue and driving the topic. Many thanks to everyone involved!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants