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

Download of JDK from the Elastic catalog instead of Adoptium #15514

Merged
merged 3 commits into from
Oct 30, 2023
Merged
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 27 additions & 1 deletion build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -707,8 +707,34 @@ class JDKDetails {
}

String createDownloadUrl() {
return createElasticCatalogDownloadUrl()
}

private String createElasticCatalogDownloadUrl() {
// Ask details to catalog https://jvm-catalog.elastic.co/jdk and return the url to download the JDK

// arch x86_64 never used, only aarch64 if macos
def url = "https://jvm-catalog.elastic.co/jdk/latest_adoptiumjdk_${major}_${osName}"
jsvd marked this conversation as resolved.
Show resolved Hide resolved

// Append the cpu's arch only if Mac on aarch64, all the other OSes doesn't have CPU extension
Copy link
Contributor

@dliappis dliappis Oct 30, 2023

Choose a reason for hiding this comment

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

Hm, I don't think that's correct, for example for Linux aarch64 artifacts we have https://jvm-catalog.elastic.co/jdk/latest_adoptiumjdk_17_linux_aarch64, for Windows e.g. /jdk/latest_zulu_19_windows_aarch64 or even 32bit variants like /jdk/latest_adoptopenjdk_16_windows_x86_32.

Or maybe I musunderstood the comment as in we don't need it, but for DRA my understanding is that we need all combinations e.g. in https://buildkite.com/elastic/logstash-dra-snapshot-pipeline/builds/98#018b6ef8-3807-4234-8cd3-10b2e9c0fb8e/70-298 ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can't use a generalized URL template like https://jvm-catalog.elastic.co/jdk/latest_adoptiumjdk_${major}_${osName}_${arch} because the url https://jvm-catalog.elastic.co/jdk/latest_adoptiumjdk_17_linux_x86_64 return a

Could not find download for latest_adoptiumjdk_17_linux_x86_64

But the https://jvm-catalog.elastic.co/jdk/latest_adoptiumjdk_17_linux return the x86_64 artifact.

So we could use the arch format iff it's an ARM (not only for MacOS but also for Linux).

WDYT?

Copy link
Contributor

@dliappis dliappis Oct 30, 2023

Choose a reason for hiding this comment

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

I agree. We can use the default URL for x86_64 and use a suffix (aarch64 in our case) for the specific aarch64 artifacts for macOS, Linux or Windows.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed by b968888

if (arch == "aarch64") {
url += "_${arch}"
}
def catalogMetadataUrl = new URL(url)
def catalogConnection = catalogMetadataUrl.openConnection()
catalogConnection.requestMethod = 'GET'
assert catalogConnection.responseCode == 200

def metadataRetrieved = catalogConnection.content.text
println "Retrieved!"

def catalogMetadata = new JsonSlurper().parseText(metadataRetrieved)
return catalogMetadata.url
}

private String createAdoptDownloadUrl() {
String releaseName = major > 8 ?
"jdk-${revision}+${build}":
"jdk-${revision}+${build}" :
"jdk${revision}u${build}"
String vendorOsName = vendorOsName(osName)
switch (vendor) {
Expand Down