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

Use virtual Java packages on Red Hat and set java_bin #627

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

ekohl
Copy link

@ekohl ekohl commented Mar 22, 2024

This uses the virtual packages jre-VERSION-headless instead of explicitly openjdk. This allows other JREs to provide the same.

It also uses an explicit path to the JRE specific java bin. At least on EL9 this allows the following upgrade path to work:

dnf install https://yum.puppet.com/puppet7-release-el-9.noarch.rpm
dnf install puppetserver
dnf install https://yum.puppet.com/puppet8-release-el-9.noarch.rpm
dnf upgrade puppetserver

Prior to this patch it would break, because it used /usr/bin/java which will point to Java 8. By using /usr/lib/jvm/jre-17/bin/java we know for sure it is Java 17.

@ekohl ekohl requested review from a team as code owners March 22, 2024 10:53
@ekohl ekohl changed the title Please add all notable changes to the "Unreleased" section of the CHANGELOG. Use virtual Java packages on Red Hat and set java_bin Mar 22, 2024
options.java = '(jre-17-headless or jre-11-headless)'
# TODO: which bin to use? /usr/bin/java may be anything
elsif options.os_version == 9
options.java = 'jre-17-headless'
Copy link
Author

Choose a reason for hiding this comment

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

You can question why EL 9 always uses Java 17 while EL 8 uses 17 or 11. Both EL 8 and EL 9 provide both versions.

Copy link
Contributor

@austb austb Apr 4, 2024

Choose a reason for hiding this comment

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

You can blame me for that one. At the time I did this because I wanted to require 17 where possible because it lined up with what we are using in PE and most heavily testing, but from what I could tell the initial releases of EL 8 were before Java 17 was released and would have only had 11.

@@ -181,25 +182,28 @@
options.systemd_el = 1
elsif options.operating_system == :el && options.os_version >= 7 # systemd el
if ! options.is_pe
# https://bugzilla.redhat.com/show_bug.cgi?id=2224427
Copy link
Author

Choose a reason for hiding this comment

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

This bug is resolved, so it could be dropped. But this at least provides a reference of why it's there.

jonathannewman and others added 2 commits July 19, 2024 16:32
This updates the README with some information about behvior that
wasn't necessarily obvious, and helpful for composite projects.
Making it a config option allows for differentation. The comment for
version 8 was outdated, since puppetserver 8 refuses to start up with
Java 8.
@ekohl
Copy link
Author

ekohl commented Jul 19, 2024

Now updated to also define JAVA_BIN in the systemd service file. See the additional commit for the reasoning.

The EnvironmentFile paths are config locations so if a user ever
modified them, they're not replaced. By defining this in the service
file, the packaging will update the location. The result is that even
if a java location changes that a simple yum/apt upgrade will respect
the new value in most situations.
This is a noop refactor, but makes the later diffs easier to read.
This uses the virtual packages jre-VERSION-headless instead of
explicitly openjdk. This allows other JREs to provide the same.

It also uses an explicit path to the JRE specific java bin. At least on
EL9 this allows the following upgrade path to work:

    dnf install https://yum.puppet.com/puppet7-release-el-9.noarch.rpm
    dnf install puppetserver
    dnf install https://yum.puppet.com/puppet8-release-el-9.noarch.rpm
    dnf upgrade puppetserver

Prior to this patch it would break, because it used /usr/bin/java which
will point to Java 8. By using /usr/lib/jvm/jre-17/bin/java we know for
sure it is Java 17.
# Location of your Java binary (version 8)
JAVA_BIN="/usr/bin/java"
# Location of your Java binary
#JAVA_BIN=""
Copy link
Author

Choose a reason for hiding this comment

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

I found out that the puppetserver bin (which I can't find the source of now) also reads the defaults file. If that needs JAVA_BIN then this change is breaking.

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

Successfully merging this pull request may close these issues.

3 participants