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

JENKINS-69581: Install java-11-openjdk #777

Merged
merged 1 commit into from
Jan 6, 2023

Conversation

grogersxyz
Copy link
Contributor

@grogersxyz grogersxyz commented Sep 19, 2022

If Jenkins recieves a non-zero error code when runing java -fullversion on the remote agent then it will install java 11 using yum.

Previously this installed java 8 which is no longer supported on Jenkins agents where the controller is running version 2.361.x or higher.

This issue is discussed further in JENKINS-69581

  • Make sure you are opening from a topic/feature/bugfix branch (right side) and not your main branch!
  • Ensure that the pull request title represents the desired changelog entry
  • Please describe what you did
  • Link to relevant issues in GitHub or Jira
  • Link to relevant pull requests, esp. upstream and downstream changes
  • Ensure you have provided tests - that demonstrates feature works or fixes the issue

Copy link
Member

@basil basil left a comment

Choose a reason for hiding this comment

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

Thanks for the PR! How was this PR tested? This page states that the command to install Java 11 on Amazon Linux is sudo amazon-linux-extras install java-openjdk11, which is the same command we use in the Jenkins packaging test suite. There, the (tested) algorithm is as follows:

  • On Amazon Linux, check for /usr/lib/jvm/jre-11-openjdk; if it does not exist, run amazon-linux-extras install java-openjdk11 -y.
  • On all RPM-based distributions (including Amazon Linux), run yum install -y fontconfig java-11-openjdk (on Amazon Linux, the java11-openjdk portion will be a no-op because of the previous command, while on other RPM-based distributions that will be the place where Java 11 is installed).

I am not the maintainer of this plugin, but I would expect this needs to work out of the box for the default EC2 Linux image at the very least, if not other common RPM-based distributions.

@grogersxyz
Copy link
Contributor Author

Of course yeap! I can work on that now and will run it through on a staging server I have. If we need unit tests for this I might need some help with that, I wouldn't say I'm especially familiar with Java :)

@basil
Copy link
Member

basil commented Sep 20, 2022

Although I am not the maintainer of this plugin, I think the primary concern is avoiding regressions for the 11,802 users of this plugin rather than achieving a perfect work of software engineering. That is why my comments were mostly centered around testing. We are happy to have your contribution regardless of your Java experience. Regarding testing, I think there are two main use cases we need to think about:

  • A user who is already using an image with Java 11: either as the default Java (returned by java -version) or in a non-default path specified by javaPath as in [JENKINS-69304] Allow specifying a java path #766): in this use case, we should not install anything.
  • A user who is using e.g. a default EC2 Amazon Linux image (or possibly some other RPM-based distribution) without Java 11 (or any Java at all) installed by default: in this use case, we want to install a working Java runtime as described in my previous post.

@grogersxyz
Copy link
Contributor Author

I gave it a go trying to do it in Java but didn't really know enough about input and output streams and buffers etc so I'll just leave what I did here sorry. I imagine someone will want to add this support in the future if they use the feature and upgrade to a version of jenkins that doesn't support Java8.

If I were able to just do it in bash I'd do something like [ $(java -fullversion 2>&1 | cut -d'"' -f2 | sed 's/^1\.//' | cut -d'.' -f1) == "11" ] 2>/dev/null

@basil
Copy link
Member

basil commented Oct 10, 2022

This PR was a promising start but lacks sufficient testing and cannot be merged as-is. If nobody else picks up this work I would suggest closing this PR.

@nfplatzke
Copy link

When is this going to be fixed? This plugin is currently broken.

@basil
Copy link
Member

basil commented Nov 16, 2022

When someone submits an acceptable pull request that fixes it, I suppose.

@grogersxyz
Copy link
Contributor Author

I might have some time this weekend to take another look at this. If I’m not able to get it working and test it I’ll close this PR as suggested but keep JENKINS-69581 open for further discussion.

@grogersxyz
Copy link
Contributor Author

Here's the config that I used to test this build on Jenkins version 2.361.4:

Here's the console outputs of my testing:

Given that this is my first contribution to this repository I might suggest someone else verifying that this is working for them. Happy to make any modifications as needed!

As I mentioned in my commit it would be cool to check the version of Java installed, but at this point I wanted to focus on fixing this regression for current users more than implementing a new feature.

@grogersxyz
Copy link
Contributor Author

In the meantime if anyone finds this you can resolve this issue temporarily by adding the following commands to your init scripts:

# For Amazon Linux 2
sudo amazon-linux-extras install java-openjdk11 -y
# For RedHat Enterprise Linux
sudo yum install -y fontconfig java-11-openjdk
# For Ubuntu
sudo apt-get install openjdk-11-jdk

cc @nfplatzke

As of Jenkins Weekly 2.357 and LTS 2.361 the minimum required java
version is java 11.

If the command java -fullversion fails we will try to install openjdk-11.

We first try and install it using amazon-linux-extras. If this passes
then the launch continues. If it fails (for example we're launching a
linux node on a distribution other than amazon-linux we try and install
openjdk-11 using yum.

If both of these commands fail we won't launch the node.

At this point we don't explicitly check the java version, but this could
perhaps be a future addition.
@grogersxyz
Copy link
Contributor Author

Just force pushed a change to the commit. I had previously been joining the commands together with || but that would lead to the second command not running in the instance the first command was a success (for example in amazon-linux2). I've now replaced this with a ; so they're treated as two seperate commands and will both run regardless of the others return code.

@grogersxyz
Copy link
Contributor Author

@thoulen tagging you in this PR as a maintainer, sorry I don't have permission to add reviewers as suggested in the contributing docs. This PR also fixes https://issues.jenkins.io/browse/JENKINS-68946 and https://issues.jenkins.io/browse/JENKINS-69383

@basil
Copy link
Member

basil commented Jan 5, 2023

Perhaps @res0nance would be interested in merging this as he was the last active maintainer. If not, my apologies for the obtrusive notification.

@res0nance res0nance added the enhancement Feature additions or enhancements label Jan 6, 2023
@res0nance res0nance merged commit c50beb8 into jenkinsci:master Jan 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Feature additions or enhancements
Projects
None yet
4 participants