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.

Sign up for GitHub

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

Jump to bottom

Default to the bundled JDK for integration tests in CI #40531

Closed
alpar-t opened this issue Mar 27, 2019 · 8 comments
Closed

Default to the bundled JDK for integration tests in CI #40531

alpar-t opened this issue Mar 27, 2019 · 8 comments
Assignees
@alpar-t
Labels
:Delivery/Build Build or test infrastructure :Delivery/Packaging RPM and deb packaging, tar and zip archives, shell and batch scripts >non-issue Team:Delivery Meta label for Delivery team

Comments

@alpar-t
Copy link
Contributor

alpar-t commented Mar 27, 2019

Right now we only test with the bundled JDK version in the packaging tests but for other integration tests when RUNTIME_JAVA_HOME is not set.
Since we always set the lather in CI we don't get any coverage with respect to integration tests for the bundled JDK there.

We should move to default to the bundled JDK in CI for everything other than the specific java matrix testing. Since the integ test distribution does not bundle the jdk ( and we don't want it to ) we need a way in the build to provision and use the same jdk as the one we bundle for those tests.
@alpar-t alpar-t added >non-issue :Delivery/Build Build or test infrastructure :Delivery/Packaging RPM and deb packaging, tar and zip archives, shell and batch scripts labels Mar 27, 2019
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-infra

This was referenced Mar 27, 2019
Closed
Merged
@alpar-t alpar-t mentioned this issue Jun 11, 2019
Merged
This was referenced Sep 4, 2019
Merged
Merged
@alpar-t alpar-t self-assigned this Oct 23, 2019
alpar-t added a commit to alpar-t/elasticsearch that referenced this issue Oct 28, 2019
@alpar-t
Addretries to the curl command
df516ae 
Look like the `--retry` argument is not always affective.

Closes elastic#40531
@alpar-t alpar-t mentioned this issue Oct 28, 2019
Merged
@alpar-t
Copy link
Contributor Author

alpar-t commented Oct 28, 2019

I have been thinking about how to get to a consistent setup here.
We used to have runtimeJavaHome extra property on the project that would get configured up-front. This worked well until we added the jdk to the distribution and we ended up with situations where we don't have to configure anything, unless we are specifically testing for other java versions in CI.
The best way to model this is to allow the property to be null and pass it on conditionally on a case by case basis. Mostly for test clusters we'll pass JAVA_HOME to the cluster while for unit tests we'll provision the same JDK as we bundle to run them.
This implies that the runtime jdk for these will be changing from the one defined in .ci/java-version.proeprties to the one we bundle.

The rule will be simple: we test everything with the same JDK we bundle ( or the actual bundled JDK when testing a distribution ), except for when a specific system property points to a different one, in which case that one will be used.
CI will be configured so that the Java matrix job will be the only one to pass that.

We also have system properties for java versions which we use in the reproduction line printer to possibly raise awareness that some java version is relevant for the reproduction.
This doesn't fit very well and requires additional setup to work which we would like to avoid as part of #47796 , so the proposal is to simply drop them.
We don't have reproduction lines for OS specific tests, encryption at rest, and other setups we have in CI.
We could replace it with the name of the job just to have the information there on the command line and spark the possibility that it might have something to do with it.

There's quite a few places that use runtime java directly, so we'll have to look at all of them, and we'll need the ability to have a branch specific CI setup since we can't back-port this all the way.

alpar-t added a commit to alpar-t/elasticsearch that referenced this issue Oct 28, 2019
@alpar-t
Provision the correct JDK for test tasks
2039f83 
This PR adds build configuration to use the `jdk-download` plugin with
unit tests when no runtime java is configured externally.

It's a first part in a longer chain of changes described in elastic#40531.
@alpar-t alpar-t mentioned this issue Oct 28, 2019
Merged
alpar-t added a commit that referenced this issue Nov 3, 2019
@alpar-t
Improve jdk download retries (#48560)
a91f0f8 
* Addretries to the curl command

Look like the `--retry` argument is not always affective.

Closes #40531
@alpar-t
Copy link
Contributor Author

alpar-t commented Nov 15, 2019

We need to update CI to no longer pass RUNTIME_JAVA_HOME except for the JDK test matrix.
When this variable is not set, we'll just use the bundled JDK, either from the distribution or the same version that was provisioned as part of #48561.
The runtime.java system property will be supported as a number only, and it will work with provisioned version only, as a semi correct reproduction. It will work for provisioning the same version from a specific vendor, but the exact same vendor will not be guaranteed.

alpar-t added a commit that referenced this issue Nov 18, 2019
@alpar-t
Provision the correct JDK for test tasks (#48561)
5fdd817 
This PR adds build configuration to use the `jdk-download` plugin with
unit tests when no runtime java is configured externally.

It's a first part in a longer chain of changes described in #40531.
alpar-t added a commit that referenced this issue Nov 18, 2019
@alpar-t
Provision the correct JDK for test tasks (#48561)
6e775cf 
This PR adds build configuration to use the `jdk-download` plugin with
unit tests when no runtime java is configured externally.

It's a first part in a longer chain of changes described in #40531.
@mark-vieira mark-vieira mentioned this issue Jan 24, 2020
Closed
@mark-vieira
Copy link
Contributor

With #51505 I think we can revisit this and remove usages of RUNTIME_JAVA_HOME for most everything in CI.

@rjernst rjernst added the Team:Core/Infra Meta label for core/infra team label May 4, 2020
@mark-vieira mark-vieira added Team:Delivery Meta label for Delivery team and removed Team:Core/Infra Meta label for core/infra team labels Nov 11, 2020
@pugnascotia
Copy link
Contributor

@mark-vieira is this issue still value? If so, what needs to be done?

@pugnascotia pugnascotia reopened this Jul 25, 2022
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-delivery (Team:Delivery)

@mark-vieira
Copy link
Contributor

I think we might still be setting RUNTIME_JAVA_HOME unnecessarily in some places. I'll take a look.

@mark-vieira mark-vieira mentioned this issue Mar 2, 2023
Closed
@mark-vieira
Copy link
Contributor

We've cleaned up all remaining instances of us not using the bundled JDK.

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

@alpar-t alpar-t

Labels
:Delivery/Build Build or test infrastructure :Delivery/Packaging RPM and deb packaging, tar and zip archives, shell and batch scripts >non-issue Team:Delivery Meta label for Delivery team
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Default to bundled JDK for all runtime test execution mark-vieira/elasticsearch
6 participants
@rjernst @alpar-t @mark-vieira @pugnascotia @elasticmachine @elasticsearchmachine