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

Autodetect boot JDK location based on playbook defaults #2167

Merged
merged 6 commits into from
Feb 17, 2021

Conversation

sxa
Copy link
Member

@sxa sxa commented Oct 21, 2020

Draft PR to auto-set JDK_BOOT_DIR based on default locations symlinked by the ansible playbooks. This should remove the need to have JDKxx_BOOT_DIR definitions on the build machines in jenkins and reduce the number of times a boot JDK is downloaded dynamically during the builds

VPC run

Signed-off-by: Stewart X Addison [email protected]

@sxa sxa added the enhancement Issues that enhance the code or documentation of the repo in any way label Oct 21, 2020
@sxa sxa added this to the October 2020 milestone Oct 21, 2020
@sxa sxa self-assigned this Oct 21, 2020
@adoptopenjdk-github-bot
Copy link
Contributor

🟠 PR TESTER RESULT 🟠

❎ Some pipelines failed or the job was aborted! ❎
See the pipeline-build-check below for more information...

Copy link
Contributor

@karianna karianna left a comment

Choose a reason for hiding this comment

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

LGTM

@adoptopenjdk-github-bot
Copy link
Contributor

🟠 PR TESTER RESULT 🟠

❎ Some pipelines failed or the job was aborted! ❎
See the pipeline-build-check below for more information...

1 similar comment
@adoptopenjdk-github-bot
Copy link
Contributor

🟠 PR TESTER RESULT 🟠

❎ Some pipelines failed or the job was aborted! ❎
See the pipeline-build-check below for more information...

@sxa
Copy link
Member Author

sxa commented Oct 23, 2020

Needs more work as it's causing issues in VPC with JDK8 builds using JDK7 to boot
Working build:

15:05:06 Detecting boot jdk for: jdk8u
15:05:06 Found build version: 8
15:05:06 Required boot JDK version: 7
15:05:06 /usr/local/gcc/bin/ccache
15:05:06 Boot jdk directory: /usr/lib/jvm/java-1.7.0-openjdk.x86_64:
15:05:07 BOOT JDK: java version "1.7.0_261"

New failing build:

14:17:03 Detecting boot jdk for: jdk8u
14:17:03 Found build version: 8
14:17:03 Required boot JDK version: 7
14:17:03 Downloading GA release of boot JDK version 7...
14:17:03 
14:17:03 gzip: stdin: unexpected end of file
14:17:03 tar: Child returned status 1
14:17:03 tar: Error is not recoverable: exiting now

@AdamBrousseau
Copy link
Contributor

Could bootjdk location be added to the config files jdkX_pipeline_config.groovy?
I'm thinking if a machine does not use the default playbook locations and the preference is to not have to define the variable on the Jenkins node, then it may be nice to have the option available in the config file (Related #2132).

@M-Davies M-Davies modified the milestones: October 2020, November 2020 Nov 7, 2020
@karianna karianna modified the milestones: November 2020, December 2020 Jan 3, 2021
@karianna
Copy link
Contributor

karianna commented Feb 3, 2021

@sxa - Did you want to continue with this PR or are you happy to park it given our refactoring efforts?

@sxa
Copy link
Member Author

sxa commented Feb 8, 2021

@sxa - Did you want to continue with this PR or are you happy to park it given our refactoring efforts?

On my list for this week, but I don't believe this PR is part of the stuff that's going to be moved. This will make the build and infra a bit simpler, so I want it in :-)

@sxa sxa force-pushed the bootjdkdetet branch 3 times, most recently from c5f7d49 to 061a758 Compare February 8, 2021 17:25
@sxa
Copy link
Member Author

sxa commented Feb 8, 2021

Could bootjdk location be added to the config files jdkX_pipeline_config.groovy?

I think a default could reasonably be made there. My interest in particular is making sure that if someone is trying to debug locally on a machine (so not running via the playbooks) that these scripts are as self-sufficient as possible, but I certainly wouldn't be averse to having them in another location too

@sxa
Copy link
Member Author

sxa commented Feb 8, 2021

Not quite certain which VPC platforms are stable at the moment unfortunately so I've run two for this to be able to compare:

@sxa
Copy link
Member Author

sxa commented Feb 15, 2021

VPC - JDK15 JDK8

@sxa
Copy link
Member Author

sxa commented Feb 16, 2021

New vpc after further minor adjustments: JDK15 - JDK8

@sxa
Copy link
Member Author

sxa commented Feb 16, 2021

run tests

@sxa sxa marked this pull request as ready for review February 16, 2021 15:07
@sxa sxa requested a review from M-Davies February 16, 2021 15:08
Copy link
Contributor

@M-Davies M-Davies left a comment

Choose a reason for hiding this comment

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

Indentations appear off after the removal of the following if block. Might be just GH's editor being funky with tabs however

# Any version above 8 (11 for now due to openjdk-build#1409
if [ "$JAVA_FEATURE_VERSION" -gt 11 ]; then

@sxa
Copy link
Member Author

sxa commented Feb 16, 2021

Indentations appear off after the removal of the following if block. Might be just GH's editor being funky with tabs however

# Any version above 8 (11 for now due to openjdk-build#1409
if [ "$JAVA_FEATURE_VERSION" -gt 11 ]; then

Can you be more specific with "off"? Looks ok to me
image

@sxa
Copy link
Member Author

sxa commented Feb 16, 2021

Oh hang on - you're talking about the AIX one specifically ...

@sxa
Copy link
Member Author

sxa commented Feb 16, 2021

OK I've put the conditional back in for now - adjusting it should probably be the scope of a separate PR

@M-Davies
Copy link
Contributor

All of them actually. If you look at each file appears off after that if block has been removed. Mac for example:
Screenshot 2021-02-16 at 17 15 48

@sxa
Copy link
Member Author

sxa commented Feb 16, 2021

Got stuck in rebase hell there for a bit. Not sure what was going on, anyway hopefully it's ok now and I'm running new VPCs a new vpc after further minor adjustments: JDK15 - JDK8

Signed-off-by: Stewart X Addison <[email protected]>
Copy link
Member

@gdams gdams left a comment

Choose a reason for hiding this comment

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

Happy for this to go in - I'd like to see someone break this function out into a commonplace though to avoid duplication at some point

@sxa
Copy link
Member Author

sxa commented Feb 17, 2021

Happy for this to go in - I'd like to see someone break this function out into a commonplace though to avoid duplication at some point

There a lot of cleaning up I'd like to do with these scripts but I'm focusing on usability for now. The directory that it selects is different for each platform which makes it more difficult to get advantages from unifying under a single function (unless we always installed into, day, /usr/local/java regardless of platform

@sxa
Copy link
Member Author

sxa commented Feb 17, 2021

Merging and I'll run a JDK17 pipeline (which failed last night due to the freemarker version issue) in order to verify and fix if required.

@sxa sxa merged commit 9e2b144 into adoptium:master Feb 17, 2021
sxa added a commit to sxa/temurin-build that referenced this pull request Feb 19, 2021
* Autodetect boot JDK location based on playbook defaults

Signed-off-by: Stewart X Addison <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Issues that enhance the code or documentation of the repo in any way
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants