search

gocd / gocd

GoCD - Continuous Delivery server main repository
https://www.gocd.org
Apache License 2.0
7.05k stars 971 forks source link

Replace bundled deb/rpm x64 JRE with arch-independent package deps #12712

Closed matthiaskraaz closed 3 weeks ago

matthiaskraaz commented 4 weeks ago

Removed x86 JRE from architecture-independent package. Instead, added dependency on openjdk-17-jre-headless to DEB and dependency on jre-17-openjdk-headless to RPM. Fixes #11354.

Tested installation of go-agent and go-server.

Tested under:

Each OS both in the x86_64 variant and arm64 variant.

Result: all ok except for:

chadlwilson commented 4 weeks ago

Thanks for this, I'll have a play around.

While I am not an expert in the area I'm thinking that to avoid conflicts and maximise compatibility

  1. it might be good to add alternative optional dependencies that also satisfy it, such as a JDK or a non-headless variant - or maybe even the abstract capabilities for each platform?. It might be moot if the JDK and non-headless variants actually augment (and dont conflict with) the headless runtime variants.
  2. we might consider using --deb-recommends package dependencies on Deb/Ubuntu. Don't think there is an equivalent on rpm though (and weak deps is not what we want), so maybe just causes more confusion than necessary?
  3. On Ubuntu/Debian openjdk-17-jre-headless conflicts openjdk-11-jdk-headless << 11~18-2. I don't really understand why or the implications, as they install in different locations but might be misunderstanding the meaning (just keeping a note here so I can double check it). What I am thinking about is sanity checking that adding this dependency isn't going to cause issues for other things/versions people might need on their systems for other reasons (especially on agent systems where people typically install other runtimes and such for use in jobs).
matthiaskraaz commented 4 weeks ago

DEB: I'd love to have a dependency on a virtual package instead of a concrete package, but I can't find a virtual package specifically for Java 17. The virtual package java17-runtime-headless is also provided by openjdk-18-jre-headless, openjdk-19-jre-headless, ... So "Depends: java17-runtime-headless" or "Depends: openjdk-17-jre-headless | java17-runtime-headless" would allow to try to use the go-* package with Java 18 or later, which would break our dependency on /usr/lib/jvm/java-17-openjdk-$arch/bin/java. And AFAICS it is not possible to specify a (major) version for a virtual package dependency.

RPM: Same, with the exception that RPM automatically selects the latest package that fulfills a dependency.

chadlwilson commented 2 weeks ago

This is merged and automated tests are fine. https://build.gocd.org/go/pipelines/installer-tests/2456/install-tests/1

I made some changes before this to improve #s/types of OSes this is tested against: https://github.com/gocd/installer-testing/blob/47b859f67f2b508b74ba527060ebc5995364a4d2/Rakefile#L169-L175

It ensures server and agent both start up OK and agent can connect to server, with both a fresh install and a package upgrade from 20.5.0 to latest. I added validation against Ubuntu 24.04 and Almalinux 8/9 as a RHEL equivalent. Alma validates using systemd as init system and service runner since /etc/init.d isn't available on RHEL 9-compatibles.

I'll do a bit more sanity testing for alternate Java dependencies - ideally I'd like the package to not complain if the user has a JDK installed that may otherwise clash/conflicts with the main JRE dependency we add. Adding a dependency on the concrete package as primary seems the canonical way with an alternative virtual or concrete package in an OR; as long as we can locate its installation folder reliably (and noting that we don't want to "allow" Java 18+ so some virtual packages are not suitable).

matthiaskraaz commented 2 weeks ago

I pondered this a little more:

chadlwilson commented 2 weeks ago

Yes, I intend to get to Java 21 some time soon. Just want to limit versions by changing baseline to Java 17 first (which is a requirement of 24.1.0) so I don't have to keep thinking about Java 11 as well as 21.

chadlwilson commented 6 days ago

I did some more real-world sanity testing of this with both debs and rpms to see what happens if there is already a "full" JDK installed before installing the gocd packages.

In both cases, with the way packages are modelled in the respective ecosystems the JDKs either layer on top of the headless JREs and regular JREs OR "replace" the JRE package - so having those pre-installed will mean the package requirement is already met when installing gocd packages.

So there's nothing more needed to be done here on adding an "OR" dependency. Apologies if you already knew that, I hadn't followed!

Accordingly, released 24.1.0: https://www.gocd.org/releases/#24-1-0

Thanks for the help, this is a much better situation!

matthiaskraaz commented 6 days ago

There is never anything wrong with more tests! I have written to openjdk-17@packages.debian.org to ask if there is some better approach than the one taken, but have not received any answer. So what are the most pressing issues now?

chadlwilson commented 6 days ago

"Pressing" is probably a matter of perspective, but other than the likely-nasty long-standing EOL dependency problems (e.g Hibernate) - it'd be nice to to some sanity testing with Java 21 and start flushing out the various issues.

The "easy" stuff is at https://github.com/gocd/gocd/pull/12762 but failures in the pipelines there will probably be hard to debug as they will be full functional regression test failures. If you're interested in exploring, it's probably better to first try and run all the normal tests with Java 21 by having a Java 21 JRE available and then changing this to 21:

https://github.com/gocd/gocd/blob/756866c6974df118928d1951265104a537238c51/build.gradle#L139

matthiaskraaz commented 6 days ago

Sounds like fun. Are all tests in the main repository?

chadlwilson commented 6 days ago

Sounds like fun. Are all tests in the main repository?

The main JUnit driven unit, API and component tests that give the fastest feedback are, yes. They currently only run on a single Java release. They take a long time to run all of them locally so usually better to target just some of them (./gradlew server:fastUnitTest is a good one to aim for) . They still rely on a few dependencies documented at https://developer.gocd.org/current/ but without specifics for installing on Linux if that's your local Dev env. 😬

I imagine it'll take quite a few iterations to make various Java 21 forward compatible fixes for various pieces, and we might get stuck on some legacy dependencies which don't function properly on Java 21 in unfixable ways. But don't know any of these right now, so have to flush 'em out 😞

There are also fuller functional regression tests (that run against packaged server/agent) at https://github.com/gocd/ruby-functional-tests but that is a big suite and generally better to let the pipeline run those.

And then there are the rpm/deb tests at https://github.com/gocd/installer-testing (the ones I augmented for this issue) but those are at the very end of the pipeline running against signed packages, and really just basic smoke test of the Linux packages.

chadlwilson commented 6 days ago

I really need to get around to refactoring https://build.gocd.org pull request pipelines to make them visible to guests which would make it a lot easier to collaborate on experimental stuff like this 😓

chadlwilson commented 6 days ago

As a starting point, all the smoke tests and functional regression tests pass on Java 21 right now, so if there are issues they are likely only found at a lower layer with more specific unit tests (or relate to deprecated behaviour) But it's a good starting point to be at.

chadlwilson commented 5 days ago

Sorry to give you the runaround, but it seems that Java 21 is actually pretty straight-forward in most areas - at least enough to merge to master and trial it out on https://build.gocd.org soon. I ran all of the Gradle tests and they were all fine.

There is a problem that seems specific to running local dev servers and the compile/build infrastructure (JRuby/Rails nonsense) which I'll track at https://github.com/gocd/gocd/pull/12762#issuecomment-2109259718 but might not be much we can about around that for now. If you're still planning to have a look yourself or look further forward to address deprecations, we can move the conversation to that PR :-)

chadlwilson commented 4 days ago

Sigh, no Java 21 variant packages available on Debian 11 or 12 yet. 🤦‍♂️

matthiaskraaz commented 18 hours ago

Surprises me too that Debian hasn't a Java 21 package yet.

BuildWorkTest.shouldCleanAgentWorkingDirectoryIfExistsWhenCleanWorkingDirIsTrue()
BuildWorkTest.shouldCleanAgentWorkingDirectoryWithSymlinksIfExistsWhenCleanWorkingDirIsTrue()
BuildWorkTest.shouldCreateAgentWorkingDirectoryIfNotExist()
BuildWorkTest.shouldCreateAgentWorkingDirectoryIfNotExistWhenFetchMaterialsIsFalse()
BuildWorkTest.shouldNotBombWhenCreatingWorkingDirectoryIfCleanWorkingDirectoryFlagIsTrue()
BuildWorkTest.shouldReportBuildIsFailedWhenAntBuildPassed()
BuildWorkTest.shouldReportConsoleOut()
BuildWorkTest.shouldUpdateBothStatusAndResultWhenBuildHasPassed()
AntTaskBuilderTest.shouldFailWhenTargetDoesNotExist()

fail. I'm digging into it...

chadlwilson commented 18 hours ago

Yeah, for now I've worked around it by changing the package dependency to be an | so it uses the preferred packaged version if available (21 - Ubuntu), otherwise falls back to the previous target version (17 - Debian) with some scripty hijinx:

https://github.com/gocd/gocd/blob/cd642431fb79e52dd8eb9a1d66a21a6a4e9ee77b/installers/linux.gradle#L24-L38

chadlwilson commented 7 hours ago 
BuildWorkTest.shouldCleanAgentWorkingDirectoryIfExistsWhenCleanWorkingDirIsTrue()
BuildWorkTest.shouldCleanAgentWorkingDirectoryWithSymlinksIfExistsWhenCleanWorkingDirIsTrue()
BuildWorkTest.shouldCreateAgentWorkingDirectoryIfNotExist()
BuildWorkTest.shouldCreateAgentWorkingDirectoryIfNotExistWhenFetchMaterialsIsFalse()
BuildWorkTest.shouldNotBombWhenCreatingWorkingDirectoryIfCleanWorkingDirectoryFlagIsTrue()
BuildWorkTest.shouldReportBuildIsFailedWhenAntBuildPassed()
BuildWorkTest.shouldReportConsoleOut()
BuildWorkTest.shouldUpdateBothStatusAndResultWhenBuildHasPassed()
AntTaskBuilderTest.shouldFailWhenTargetDoesNotExist()

fail. I'm digging into it...

Assume these are the tests that are failing for you? Might be able to make a guess if I can see a couple of the stack traces and error messages. Might be something weird with your git working dir and/or some cruft left behind (at a random guess).

Could try a ./gradlew clean first to start freshish if it's that.

matthiaskraaz commented 6 hours ago

So they are not supposed to fail?! May be my setup: Ubuntu 22.04.4 LTS via WSL on a Windows machine. Let me try on a real Linux machine instead. PS: I did a git clean -fdx beforehand, so it shouldn't be any cruft.

chadlwilson commented 3 hours ago

So they are not supposed to fail?! May be my setup: Ubuntu 22.04.4 LTS via WSL on a Windows machine. Let me try on a real Linux machine instead. PS: I did a git clean -fdx beforehand, so it shouldn't be any cruft.

All tests should and do pass in CI and locally - but will never have been tested via Linux on WSL2. Windows on its own, yes. Linux yes. The vagaries of filesystems and locks via WSL2 I've no idea, because they are a pain on Windows normally, and on Linux the underlying FS wouldn't be expected to affect things.

Depends on what the errors are though..

chadlwilson commented 3 hours ago

Some tests are targetting towards Windows or non-Windows where testing filesystem specific behaviour, for example.

I did try with WSL2 locally once I believe, but I only have windows via virtualisation and at the moment aarch64-only (so Win 11 insider preview) so I'm not sure I want to go down that rabbit hole 😅