Skip to content

Upgrade GoCD to run and build with Java 17#9818

Merged
chadlwilson merged 11 commits intogocd:masterfrom
chadlwilson:java-16
Feb 5, 2022
Merged

Upgrade GoCD to run and build with Java 17#9818
chadlwilson merged 11 commits intogocd:masterfrom
chadlwilson:java-16

Conversation

@chadlwilson
Copy link
Member

@chadlwilson chadlwilson commented Oct 30, 2021 •
edited
Loading

Fixes #9635, #9469

  • Upgrades to Java 17
    • Since Gradle 6 was not compatible with Java 16+ when initially working on this, in order to decouple, introduces use of Gradle Java Toolchains to compile and test using a different JVM to that Gradle runs with (this is now moot, since the Gradle 7 upgrade was completed, but preserving ability to run tests with target JVM and Gradle with a newer JVM is possibly desirable)
    • Adds bare minimum --add-opens to open up internal JVM modules for reflection, that seems to be required in a number of places, including use of the deep object cloner (but not limited to this) and Hibernate 3.6/javassist
    • Avoids use of non-idealsource/target compatibility, preferring the release flag which ensures code doesn't depend on APIs from earlier JVMs it shouldn't.
  • Switches Java builds to Eclipse Adoptium Temurin JRE builds
    • Adds ability to use an Alpine-specific Adoptium JRE, but not currently used due to issues with the Tanuki Service Wrapper on musl libc

Task list

  • Get pre-requisite JVM on build agents
  • Get tests passing
  • A lot of cleanups/TODOs/FIXMEs
  • Validate the various distributions
    • Windows
    • MacOS
    • Linux (.deb via Ubuntu)
  • Factor out the --add-opens args so the tests for agent run with different set of args than server since they seem to require different packages to be open
  • Much more deep assessment of correctness of server and agent with the existing --add-opens. The current list was derived through trial and error.
    • Review all the dodgy setAccessible reflection stuff throughout the code
      • There's a lot, but most of it seems to be via walking configuration, and params resolver stuff. I hope that this is OK with the existing opens.
    • Check the SvnCommand and such exception handling which sets exception internals. Is this used on agents?
      • The smudgeException code is only used when checking modifications on the server, it appears. So should be fine.
    • brute force through some more features "of interest" on server locally, and some relevant plugins?
  • Consider changing compatibility back to Java 11 (previous LTS) rather than Java 13 which is not an LTS release.
  • Sanity check that 22.1.0 server can talk to 21.4.0 agent bootstrapper after server upgrade
  • Update developer docs to reflect need to --add-opens when running certain tests inside IDE
  • Rebase/squash

Before merge

  • Drop the final commit (that hacks the Gradle JDK version).
    • Take a backup of build.gocd.org :-)
    • Update the pipeline definition on build.gocd.org to BUILD_ON_JDK=17

Plugin testing / sanity checks

  • YAML Configuration Repo (local + build.gocd.org)
  • Groovy DSL Configuration Repo (build.gocd.org)
  • ECS Elastic Agent (build.gocd.org)
  • Kubernetes Elastic Agent (local)
  • Docker registry artifact (local)
  • Password file Authentication (local + build.gocd.org)
  • Guest Login Authentication(local + build.gocd.org)
  • Github OAuth Authentication/Authorization (local + build.gocd.org)
  • LDAP Authentication (local)
  • LDAP Authorization (local)
  • File Based Secrets (build.gocd.org)
  • Kubernetes-based Secrets (local)
  • Git Path Material SCM (local)
  • Script executor task (local)

More testing

  • Downgrade server to 21.2.0 with 22.1.0 agents+bootstrapper (agent auto-downgrades)
    • This won't work properly as with Java 17 bootstrapper and old code you will get Unable to make field private java.lang.String java.util.regex.Pattern.pattern accessible: module java.base does not "opens java.util.regex" to unnamed module and Unable to make field private transient java.util.NavigableMap java.util.TreeSet.m accessible: module java.base does not "opens java.util" to unnamed module @51565ec2. Should be OK with 21.4.0 as the serialization was fixed.
    • Workaround on agent wrapper-config/wrapper.conf - set set.default.AGENT_STARTUP_ARGS=-Xms128m -Xmx256m --add-opens=java.base/java.util=ALL-UNNAMED --add-opens=java.base/java.util.regex=ALL-UNNAMED
    • Documented on Working around Java 16/17 internals compatibility issues #10265
  • Downgrade agent to 21.2.0
  • Upgrade server to 22.1.0 with 21.2.0 agents+bootstrapper (agent auto-upgrades OK)
  • Upgrade agent to 22.1.0

@chadlwilson chadlwilson added installers build-script container-images dependencies java Pull requests that update Java code no stalebot Don't mark this stale. labels Oct 30, 2021
@chadlwilson chadlwilson added this to the Release 21.4.0 milestone Oct 30, 2021
@chadlwilson chadlwilson linked an issue Oct 30, 2021 that may be closed by this pull request
Unable to start Go CD server with Java 16 #9469
Closed
@chadlwilson chadlwilson force-pushed the java-16 branch 4 times, most recently from 37bd1f0 to 1b8b802 Compare October 30, 2021 18:18
@chadlwilson chadlwilson mentioned this pull request Oct 31, 2021
Merged
4 tasks
This was referenced Oct 31, 2021
Merged
Merged
@chadlwilson chadlwilson force-pushed the java-16 branch 3 times, most recently from ad41c4f to 3a91c7d Compare November 1, 2021 06:47
chadlwilson added a commit to gocd-contrib/gocd-oss-cookbooks that referenced this pull request Nov 2, 2021
@chadlwilson
Add JDK 17 (JDK 15 remains default) and remove older non-LTS out-of-s…
1da85a1 
…upport JDKs.

Supports work on JDK 17 compatibility gocd/gocd#9818
@chadlwilson chadlwilson force-pushed the java-16 branch 9 times, most recently from c199f17 to cb1c606 Compare November 7, 2021 02:20
@chadlwilson chadlwilson mentioned this pull request Nov 7, 2021
Merged
@chadlwilson chadlwilson force-pushed the java-16 branch 2 times, most recently from c21d4b0 to 2be5ef1 Compare January 6, 2022 15:35
@chadlwilson chadlwilson self-assigned this Jan 10, 2022
@chadlwilson chadlwilson force-pushed the java-16 branch 2 times, most recently from c19d019 to 102f86f Compare January 23, 2022 03:37
@chadlwilson chadlwilson force-pushed the java-16 branch 3 times, most recently from 61a5a9f to 5f97d78 Compare January 30, 2022 15:47
chadlwilson added a commit to gocd/developer.go.cd that referenced this pull request Jan 30, 2022
@chadlwilson
Document some extra requirements for working with JDK 16+
a662978 
- Anticipates the merge of gocd/gocd#9818 which has more details
- fixes some typos
- updates the requirements per current server
@chadlwilson
Migrate to Adoptium Java 17.0.1+12 LTS builds
2ea21ee
@chadlwilson
Allow targetting Alpine Linux independently of regular Linux when dow…
6c5ae0a 
…nloading Adoptium/Eclipse Temurin builds

Adoptium now publish validated Alpine Linux builds of Eclipse Temurin JVM, compiled directly against musl libc (rather than glibc). Using these would avoid the need to use the Alpine GLIBC package which is not recommended by the Alpine team, and which is no longer supported by Adopt in their own containers. See the long discussion at adoptium/containers#1

At time of writing we cannot move away from glibc because the Tanuki wrapper does not seem to work against musl, OR with either gcompat or libc6-compat compatibility layers (both seem to be missing some symbols). However this allows us to do so in future by changing the targetted `OperatingSystem` in the alpine and docker-dind `Distro`s
@chadlwilson
Switch to use of Gradle Toolchains to allow Gradle JVM and test/compi…
16a9c16 
…le JVM to vary, using Java 17 for compile+test JVM

- Gradle 6.x does not support Java 17; but we want to target it which toolchains allow us to do https://docs.gradle.org/current/userguide/toolchains.html
- In future this also allows us to use later Java version for Gradle, and earlier version for release if necessary
- source and target compatibility flags are not considered the best way to target JVM releases now using javac, the `release` flag is, which ensures the boot classpath is correct to avoid unintended usages of newer APIs: https://docs.gradle.org/current/userguide/building_java_projects.html#sec:java_cross_compilation
- Prevent auto-download of JVMs specified by toolchains. Ensure that user has control; and avoid mysteriously slow builds on build.gocd.org by failing fast. Requires `gocddev` build images `3.1.4`+ for JDK 17 onwards.
@chadlwilson
Only ensure fields are accessible when we actually need them to be …
af3ac9b 
…(they are relevant config fields annotated etc as such)

Since many of our config classes extend `java.util` collections, they have many fields (static and otherwise) for which `setAccessible` will fail be default on Java 16/17 where the Java internals are closed by default.

While we will likely still need to `--add-opens` for these in order to do deep cloning of config using the `Cloner`s, it adds a lot more clarity to have these errors occur in the right places; when cloning config, rather than during initial bootstrap of config. This will also allow us to add more optimized cloners for these classes in some locations which might avoid needing to deep clone inside Java internals (`ArrayList`s, `HashMap`s etc) entirely.
@chadlwilson
Open required internal JDK packages for illegal reflective access
e79f0e7 
- Illegal reflective access is denied by default on JDK 16, and JDK 17 requires specific add-opens (you cannot permit illegal access globally).
- Java 17 enforces https://openjdk.java.net/jeps/403 which strongly encapsulates internals per gocd#9469
- We appear to depend on a lot of such reflection due to use of the Cloning/Cloner library and some internal to Hibernate 3.6
- These were just discovered through running automated tests, plus starting server and clicking around various functions and looking at what breaks where.
- There are possibly some others, however this seemed sufficient to pass all the tests and some adhoc usage

Defined the required opens alongside the installers; which is where they mainly need to live. We also, unfortunately, need these when running server-based tests
- an attempt is made here to include or exclude them based on whether the tests are for agent or server code, although this isn't easily to discover, so by default we consider modules to run on the server, and only classify agents which seem to require less opened modules
- they also are needed for Rails RSpec tests which start a server
- Also workaround issue with use of selenium-webdriver 3.142.17 and childprocess on JRuby with Java 17. It's dying on Windows with `illegal access getting field 'private final java.io.FileDescriptor sun.nio.ch.FileChannelImpl.fd' : class org.jruby.java.invokers.InstanceFieldGetter cannot access class sun.nio.ch.FileChannelImpl (in module java.base) because module java.base does not export sun.nio.ch to unnamed module 2f465398`

Let's try following the warning and see how far we go. The Jasmine Gem is going to wrap Node Jasmine soon-enough, so this will probably become moot.
@chadlwilson
Try and stabilise server fastUnitTest
8632f2e 
Reduce the amount of memory it uses at minimum and decrease the number of test classes run per fork.
@chadlwilson
Workaround issue with use of Grolifant and our DownloaderTask on Java 17
29fc690 
There appears to be issues with use of Groovy-generated dynamic proxies for interfaces (with default methods) on Groovy 3.0.9. This workaround opens access to the module system, so that we can then open access to the dynamic proxies. Intend to follow up with Grolifant folks as well as Groovy folks after a bit more clarity on possible backport, at https://issues.apache.org/jira/browse/GROOVY-10145
@chadlwilson chadlwilson marked this pull request as ready for review February 2, 2022 13:56
@chadlwilson chadlwilson requested a review from arvindsv February 2, 2022 14:01
@chadlwilson
Ensure Gradle itself can run on Java 17 by working around issue with …
49760a3 
…use of Grolifant and DownloaderTask.groovy on Java 17

There appears to be issues with use of Groovy-generated dynamic proxies for interfaces (with default methods) on Groovy 3.0.9. This workaround opens access to the module system, so that we can then open access to the dynamic proxies. Intend to follow up with Grolifant folks as well as Groovy folks after a bit more clarity on possible backport, at https://issues.apache.org/jira/browse/GROOVY-10145

This currently needs to be addressed since we have custom scripting to download Selenium Drivers for Chrome and Firefox for use in (some) UI tests. May want to evaluate pre-existing automation to handle this later.
@chadlwilson
Align setting of maxHeapSize, jvmArgs and args for JRuby tasks
bfeea5f
@chadlwilson
Bump JDK version from 17.0.1 to 17.0.2
ce246f1
@chadlwilson chadlwilson removed the request for review from arvindsv February 5, 2022 13:18
@chadlwilson
Copy link
Member Author

chadlwilson commented Feb 5, 2022

All is clear now; failing build is due to remove the BUILD_ON_JDK=17 being removed for merge (forgot to pause the PR build)

@chadlwilson chadlwilson mentioned this pull request Feb 6, 2022
Merged
@chadlwilson
Copy link
Member Author

chadlwilson commented Mar 14, 2022

Workaround guide at #10265

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

Reviewers

No reviews

Assignees

@chadlwilson chadlwilson

Labels

build-script container-images dependencies installers java Pull requests that update Java code no stalebot Don't mark this stale.

Projects

None yet

Milestone

Release 22.1.0

Development

Successfully merging this pull request may close these issues.

GOCD new version with build-in Java 15.0.3+ Unable to start Go CD server with Java 16

1 participant

@chadlwilson