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

Issue #760 - updatecli changes to enable JDK updates when new JDK versions are published #761

Merged
merged 9 commits into from
Mar 16, 2024
10 changes: 6 additions & 4 deletions updatecli/scripts/check-jdk.sh
Original file line number Diff line number Diff line change
Expand Up @@ -31,12 +31,14 @@ function get_jdk_download_url() {
return 0;;
21*-ea-beta)
# JDK preview version (21+35-ea-beta, 21.0.1+12-ea-beta)
jdk_version="${jdk_version//-ea-beta}"
# This has been updated to support the new inferred URL pattern that started as of 21.0.3+2-ea-beta. It will not work for earlier preview versions.
# One could update the cases to support all preview versions, if desired.
jdk_version="${jdk_version//-beta}"
## https://github.com/adoptium/temurin21-binaries/releases/download/jdk-21%2B35-ea-beta/OpenJDK21U-jdk_aarch64_linux_hotspot_ea_21-0-35.tar.gz
## https://github.com/adoptium/temurin21-binaries/releases/download/jdk-21.0.1%2B12-ea-beta/OpenJDK21U-jdk_x64_linux_hotspot_ea_21-0-1-12.tar.gz
dashJDKVersion="${jdk_version//+/-}"
completeDashJDKVersion="${dashJDKVersion//./-}"
echo "https://github.com/adoptium/temurin21-binaries/releases/download/jdk-${jdk_version//+/%2B}-ea-beta/OpenJDK21U-jdk_${platform}_hotspot_ea_${completeDashJDKVersion}"
dashJDKVersion="${jdk_version//+/_}"
jdk_version="${jdk_version//-ea}"
echo "https://github.com/adoptium/temurin21-binaries/releases/download/jdk-${jdk_version//+/%2B}-ea-beta/OpenJDK21U-jdk_${platform}_hotspot_${dashJDKVersion}"
return 0;;
21*)
## JDK21 URLs have an underscore ('_') instead of a plus ('+') in their archive names
Expand Down
64 changes: 41 additions & 23 deletions updatecli/updatecli.d/jdk11.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ sources:
kind: regex
# jdk-11.0.12+7(https://github.com/adoptium/temurin11-binaries/releases/tag/jdk-11.0.12%2B7) is OK
# jdk-11.0.16.1+1 (https://github.com/adoptium/temurin11-binaries/releases/tag/jdk-11.0.16.1%2B1) is OK
pattern: "^jdk-11.(\\d*).(\\d*).(\\d*)(.(\\d*))+(\\d*)$"
pattern: "^jdk-11.(\\d*).(\\d*).(\\d*)(.(\\d*))\\+(\\d*)(.\\d*)?$"
transformers:
- trimprefix: "jdk-"
- replacer:
Expand All @@ -37,55 +37,73 @@ conditions:
checkTemurinAlpineDockerImage:
kind: dockerimage
name: Check if the container image "eclipse-temurin:<lastVersion>-jdk-alpine" is available
disablesourceinput: true
transformers:
- findsubmatch:
pattern: "(.*\\_\\d*)(\\.\\d*)?$"
captureindex: 1
- addsuffix: "-jdk-alpine"
Copy link
Contributor

Choose a reason for hiding this comment

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

It looks like this transformation works as expected:

checkTemurinAlpineDockerImage
-----------------------------
✔ docker image eclipse-temurin:11.0.22_7-jdk-alpine found

as per the last builds.

However the targets do not benefit from this:

⚠ - change detected:
	* key "$.services.jdk11.build.args.JAVA_VERSION" should be updated from "\"11.0.20.1_1\"" to "11.0.22_7.1", in file "build-windows.yaml"

same build.

=> We might want to factorize all the transformers directly into the source (at least the pattern selection) so no need to maintain all of these across conditions and targets, WDYT?

=> Which raise the question: with only the fix on the architectures (which I would use with the long form everywhere so it is homogeneous and easier to get), shouldn't the version11.0.22_7 be picked by the source without the combination of (changed regex + removing last digit with transformer to handle the specific aarch64 case)?

Copy link
Contributor

Choose a reason for hiding this comment

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

It looks like this transformation works as expected:

checkTemurinAlpineDockerImage
-----------------------------
✔ docker image eclipse-temurin:11.0.22_7-jdk-alpine found

as per the last builds.

However the targets do not benefit from this:

⚠ - change detected:
	* key "$.services.jdk11.build.args.JAVA_VERSION" should be updated from "\"11.0.20.1_1\"" to "11.0.22_7.1", in file "build-windows.yaml"

same build.

=> We might want to factorize all the transformers directly into the source (at least the pattern selection) so no need to maintain all of these across conditions and targets, WDYT?

=> Which raise the question: with only the fix on the architectures (which I would use with the long form everywhere so it is homogeneous and easier to get), shouldn't the version11.0.22_7 be picked by the source without the combination of (changed regex + removing last digit with transformer to handle the specific aarch64 case)?

I see in #762 that the current source also picks jdk-11.0.22+7.1.

✔ GitHub release version "jdk-11.0.22+7.1" found matching pattern "^jdk-11.(\\d*).(\\d*).(\\d*)(.(\\d*))+(\\d*)$" of kind "regex"

This exotic schemes looks like a problem!

@tmousawNF @gounthar @MarkEWaite @timja

WDYT about keeping the current regexp for source, but add the transformer proposed in this PR to remove any additional number after the + ?

Copy link
Contributor Author

@tmousawNF tmousawNF Mar 11, 2024

Choose a reason for hiding this comment

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

It looks like this transformation works as expected:

checkTemurinAlpineDockerImage
-----------------------------
✔ docker image eclipse-temurin:11.0.22_7-jdk-alpine found

as per the last builds.

However the targets do not benefit from this:

⚠ - change detected:
	* key "$.services.jdk11.build.args.JAVA_VERSION" should be updated from "\"11.0.20.1_1\"" to "11.0.22_7.1", in file "build-windows.yaml"

same build.

This is exactly what I intended. When a hot fix is published, the docker image will be tagged without the hot fix portion (i.e. 11.0.22_7 instead of 11.0.22_7.1 in this particular case) such that the previously published docker image is replaced with the newer docker image. However, the actual "JAVA_VERSION" we're building the docker images for will be based on the hot fix, so I think it is correct to update the "JAVA VERSION" to include the hot fix portion. Otherwise, if we had built the image for version 11.0.22_7, how would we trigger the docker image to build again to pick up the hot fix? It's possible I'm misunderstanding how this works, but my assumption is that the update of the "JAVA_VERSION" triggers the docker image to re-build. Please let me know if I have a misunderstanding here.

=> We might want to factorize all the transformers directly into the source (at least the pattern selection) so no need to maintain all of these across conditions and targets, WDYT?

Yes. This is a very practical suggestion. I'll work on an update to accomplish this.

=> Which raise the question: with only the fix on the architectures (which I would use with the long form everywhere so it is homogeneous and easier to get), shouldn't the version 11.0.22_7 be picked by the source without the combination of (changed regex + removing last digit with transformer to handle the specific aarch64 case)?

If I understand your question, it is true that version 11.0.22_7 will be picked up without the change when the 11.0.22_7 version is published. However, as I stated in my previous response, when 11.0.22_7.1 is published, without the changed regex how is the hot fix built into a docker image? Again, I could be misunderstanding what triggers the build of the docker image and, if so, please let me know.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually, now that I try to build with these suggested changes, I see that this is an issue because the JAVA_VERSION is pulling the docker image. How do we ensure that docker images get built when hot fixes are published?

Copy link
Member

Choose a reason for hiding this comment

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

Hey,
Small suggestion would it make sense to also pin using the docker image digest something like eclipse-temurin:11.0.22_7-jdk-alpine@sha256xxxxx

Copy link
Contributor

Choose a reason for hiding this comment

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

There is no docker image for the 11.0.22+7.1 (as it is macOS-only), so there would not be a new docker image from this repo for this version. 🤷

What @gounthar said: the trick here is that not all releases from Temurin correspond to a full set of platform even for Docker images.

Today:

  • If the "hotfix" (or whatever they call it) does not map to Docker image for all of our platform, then we "only" wait for the next fully fledged version which should be taken in account
    • We want 11.0.22+7 (11.0.22_7) today to be picked because it has published images for all of our platforms
  • Sometimes, if the hotfix is really one (usually a security fix) then we can have it set manually on the subset of covered platform temporarily of course. We never saw the case even with the numerous "hotfix" for Windows they did

However we can still reconsider this choice in the future of course but a full policy with examples must be written (which makes it hard to be exhaustive given each baseline of Temurin JDK seems to have their own schemes : not easy).
If we go back to Temurin installers inside our images, instead of using multi stage builds from the Temurin Docker images (as discussed during former Jenkins Platform SIG meetings) we would have to revisit this of course.

I believe we could benefit from directly retrieving the package through their API (which would ensure the proper version for each platform) as I wrote in this updatecli RFE: updatecli/updatecli#1733

Does it make sense?

Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry maybe I misunderstood the initial problem here. I thought that challenge was to detect "hostfix" update in version like 11.0.22_7.1

Feel free to ignore my suggestion then

No problem @olblak I believe the discussion is mandatory here to ensure the best contribution from @tmousawNF and it also benefits the community! Thanks for weighing in! I believe it is another use case for updatecli/updatecli#1733 (which might interest you)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

With respect to @gounthar's comment:

There is no docker image for the 11.0.22+7.1 (as it is macOS-only), so there would not be a new docker image from this repo for this version. 🤷

I found an example of where there was a "hotfix" that included new docker images (see https://github.com/adoptium/temurin17-binaries/releases/tag/jdk-17.0.9%2B9.1).

I'm going to suggest that I will remove the detection of the "hotfix" versions and only detect the major versions. In the meantime, we are no worse off than before since the "hotfix" causes issues in the existing config file (just try to run the jdk11.yaml config today).

I have also yet to factorize all the transformers. I will work on that now.

Copy link
Contributor

Choose a reason for hiding this comment

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

I remember this one, true!

There was no associated Docker image: https://hub.docker.com/_/eclipse-temurin/tags?page=1&name=17.0.9_9.1 (compared to https://hub.docker.com/_/eclipse-temurin/tags?page=1&name=17.0.9_9).

I wonder about their packaging policy in these cases: do they override the existing images with the hotfix content?

  • If yes then we should not have to worry about catching the hotfix: next release of the Jenkins images will automatically embed the "hotifx" (the tip from @olblak could apply in these cases: if we retrieve the SHA checksum we should be able to have updatecli to help by opening a PR with the same Docker image tag BUT a different checksum)
  • If no, then we should not care about.

Anyway, thanks for your work: it alrerady fixes a LOT of cases and improves the situation so let's roll once you're finished with the refacto!

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 remember this one, true!

There was no associated Docker image: https://hub.docker.com/_/eclipse-temurin/tags?page=1&name=17.0.9_9.1 (compared to https://hub.docker.com/_/eclipse-temurin/tags?page=1&name=17.0.9_9).

I believe what happens is that the "hotfix" is published with the same tag and overwrites the existing docker image (i.e. becomes a newer version of tag 17.0.9_9). You would then need to pull the base image again and rebuild.

Anyway, thanks for your work: it alrerady fixes a LOT of cases and improves the situation so let's roll once you're finished with the refacto!

Thanks for the active discussion! It helps greatly (especially since I am new to dcker-agent and updatecli).

Since I removed the portion looking for "hotfix" versions, I was able to remove the findsubmatch part of the transformations in the conditions of the jdk*.yaml config files. I created a new file values.temurin.yaml and added an extra --values ./updatecli/values.temurin.yaml to .github/workflows/updatecli.yaml. I considered possibly adding the new value to values.github-action.yaml, but that would have meant non-GitHub actions related values in that file. One option is to rename that file and merge the two values files. I'm happy to do that if the consensus is that is better.

spec:
architecture: amd64
architecture: linux/amd64
image: eclipse-temurin
tag: '{{source "lastVersion" }}-jdk-alpine'
checkTemurinDebianDockerImages:
kind: dockerimage
name: Check if the container image "eclipse-temurin:<lastVersion>-jdk-focal" is available
disablesourceinput: true
transformers:
- findsubmatch:
pattern: "(.*\\_\\d*)(\\.\\d*)?$"
captureindex: 1
- addsuffix: "-jdk-focal"
spec:
architectures:
- amd64
- arm64
- s390x
- arm/v7
- linux/amd64
- linux/arm64
- linux/s390x
- linux/arm/v7
image: eclipse-temurin
tag: '{{source "lastVersion" }}-jdk-focal'
checkTemurinNanoserver2019DockerImage:
kind: dockerimage
name: Check if the container image "eclipse-temurin:<lastVersion>-jdk-nanoserver-1809" is available
disablesourceinput: true
transformers:
- findsubmatch:
pattern: "(.*\\_\\d*)(\\.\\d*)?$"
captureindex: 1
- addsuffix: "-jdk-nanoserver-1809"
spec:
architecture: amd64
architecture: windows/amd64
image: eclipse-temurin
tag: '{{source "lastVersion" }}-jdk-nanoserver-1809'
checkTemurinWindowsCore2019DockerImage:
kind: dockerimage
name: Check if the container image "eclipse-temurin:<lastVersion>-jdk-windowsservercore-1809" is available
disablesourceinput: true
transformers:
- findsubmatch:
pattern: "(.*\\_\\d*)(\\.\\d*)?$"
captureindex: 1
- addsuffix: "-jdk-windowsservercore-1809"
spec:
architecture: amd64
architecture: windows/amd64
image: eclipse-temurin
tag: '{{source "lastVersion" }}-jdk-windowsservercore-1809'
checkTemurinNanoserver2022DockerImage:
kind: dockerimage
name: Check if the container image "eclipse-temurin:<lastVersion>-jdk-nanoserver-ltsc2022" is available
disablesourceinput: true
transformers:
- findsubmatch:
pattern: "(.*\\_\\d*)(\\.\\d*)?$"
captureindex: 1
- addsuffix: "-jdk-nanoserver-ltsc2022"
spec:
architecture: amd64
architecture: windows/amd64
image: eclipse-temurin
tag: '{{source "lastVersion" }}-jdk-nanoserver-ltsc2022'
checkTemurinWindowsCore2022DockerImage:
kind: dockerimage
name: Check if the container image "eclipse-temurin:<lastVersion>-jdk-windowsservercore-18ltsc202209" is available
disablesourceinput: true
name: Check if the container image "eclipse-temurin:<lastVersion>-jdk-windowsservercore-ltsc2022" is available
transformers:
- findsubmatch:
pattern: "(.*\\_\\d*)(\\.\\d*)?$"
captureindex: 1
- addsuffix: "-jdk-windowsservercore-ltsc2022"
spec:
architecture: amd64
architecture: windows/amd64
image: eclipse-temurin
tag: '{{source "lastVersion" }}-jdk-windowsservercore-ltsc2022'

targets:
setJDK11VersionDockerBake:
Expand Down
62 changes: 40 additions & 22 deletions updatecli/updatecli.d/jdk17.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ sources:
kind: regex
# jdk-17.0.2+8(https://github.com/adoptium/temurin17-binaries/releases/tag/jdk-17.0.2%2B8) is OK
# jdk-17.0.4.1+1(https://github.com/adoptium/temurin17-binaries/releases/tag/jdk-17.0.4.1%2B1) is OK
pattern: "^jdk-17.(\\d*).(\\d*).(\\d*)(.(\\d*))+(\\d*)$"
pattern: "^jdk-17.(\\d*).(\\d*).(\\d*)(.(\\d*))\\+(\\d*)(.\\d*)?$"
transformers:
- trimprefix: "jdk-"
- replacer:
Expand All @@ -37,55 +37,73 @@ conditions:
checkTemurinAlpineDockerImage:
kind: dockerimage
name: Check if the container image "eclipse-temurin:<lastVersion>-jdk-alpine" is available
disablesourceinput: true
transformers:
- findsubmatch:
pattern: "(.*\\_\\d*)(\\.\\d*)?$"
captureindex: 1
- addsuffix: "-jdk-alpine"
spec:
architecture: amd64
architecture: linux/amd64
image: eclipse-temurin
tag: '{{source "lastVersion" }}-jdk-alpine'
checkTemurinDebianDockerImages:
kind: dockerimage
name: Check if the container image "eclipse-temurin:<lastVersion>-jdk-focal" is available
disablesourceinput: true
transformers:
- findsubmatch:
pattern: "(.*\\_\\d*)(\\.\\d*)?$"
captureindex: 1
- addsuffix: "-jdk-focal"
spec:
architectures:
- amd64
- arm64
- s390x
- arm/v7
- linux/amd64
- linux/arm64
- linux/s390x
- linux/arm/v7
image: eclipse-temurin
tag: '{{source "lastVersion" }}-jdk-focal'
checkTemurinNanoserver2019DockerImage:
kind: dockerimage
name: Check if the container image "eclipse-temurin:<lastVersion>-jdk-nanoserver-1809" is available
disablesourceinput: true
transformers:
- findsubmatch:
pattern: "(.*\\_\\d*)(\\.\\d*)?$"
captureindex: 1
- addsuffix: "-jdk-nanoserver-1809"
spec:
architecture: amd64
architecture: windows/amd64
image: eclipse-temurin
tag: '{{source "lastVersion" }}-jdk-nanoserver-1809'
checkTemurinWindowsCore2019DockerImage:
kind: dockerimage
name: Check if the container image "eclipse-temurin:<lastVersion>-jdk-windowsservercore-1809" is available
disablesourceinput: true
transformers:
- findsubmatch:
pattern: "(.*\\_\\d*)(\\.\\d*)?$"
captureindex: 1
- addsuffix: "-jdk-windowsservercore-1809"
spec:
architecture: amd64
architecture: windows/amd64
image: eclipse-temurin
tag: '{{source "lastVersion" }}-jdk-windowsservercore-1809'
checkTemurinNanoserver2022DockerImage:
kind: dockerimage
name: Check if the container image "eclipse-temurin:<lastVersion>-jdk-nanoserver-ltsc2022" is available
disablesourceinput: true
transformers:
- findsubmatch:
pattern: "(.*\\_\\d*)(\\.\\d*)?$"
captureindex: 1
- addsuffix: "-jdk-nanoserver-ltsc2022"
spec:
architecture: amd64
architecture: windows/amd64
image: eclipse-temurin
tag: '{{source "lastVersion" }}-jdk-nanoserver-ltsc2022'
checkTemurinWindowsCore2022DockerImage:
kind: dockerimage
name: Check if the container image "eclipse-temurin:<lastVersion>-jdk-windowsservercore-ltsc2022" is available
disablesourceinput: true
transformers:
- findsubmatch:
pattern: "(.*\\_\\d*)(\\.\\d*)?$"
captureindex: 1
- addsuffix: "-jdk-windowsservercore-ltsc2022"
spec:
architecture: amd64
architecture: windows/amd64
image: eclipse-temurin
tag: '{{source "lastVersion" }}-jdk-windowsservercore-ltsc2022'

targets:
setJDK17VersionDockerBake:
Expand Down
60 changes: 39 additions & 21 deletions updatecli/updatecli.d/jdk21.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -42,55 +42,73 @@ conditions:
checkTemurinAlpineDockerImage:
kind: dockerimage
name: Check if the container image "eclipse-temurin:<lastVersion>-jdk-alpine" is available
disablesourceinput: true
transformers:
- findsubmatch:
pattern: "(.*\\_\\d*)(\\.\\d*)?$"
captureindex: 1
- addsuffix: "-jdk-alpine"
spec:
architectures:
- amd64
- arm64
- linux/amd64
- linux/arm64
image: eclipse-temurin
tag: '{{ source "getLatestJDK21Version" }}-jdk-alpine'
checkTemurinDebianDockerImages:
kind: dockerimage
name: Check if the container image "eclipse-temurin:<lastVersion>-jdk-focal" is available
disablesourceinput: true
name: Check if the container image "eclipse-temurin:<lastVersion>-jdk-jammy" is available
transformers:
- findsubmatch:
pattern: "(.*\\_\\d*)(\\.\\d*)?$"
captureindex: 1
- addsuffix: "-jdk-jammy"
spec:
architectures:
- amd64
- arm64
- linux/amd64
- linux/arm64
image: eclipse-temurin
tag: '{{ source "getLatestJDK21Version" }}-jdk-focal'
checkTemurinNanoserver2019DockerImage:
kind: dockerimage
name: Check if the container image "eclipse-temurin:<lastVersion>-jdk-nanoserver-1809" is available
disablesourceinput: true
transformers:
- findsubmatch:
pattern: "(.*\\_\\d*)(\\.\\d*)?$"
captureindex: 1
- addsuffix: "-jdk-nanoserver-1809"
spec:
architecture: amd64
architecture: windows/amd64
image: eclipse-temurin
tag: '{{ source "getLatestJDK21Version" }}-jdk-nanoserver-1809'
checkTemurinWindowsCore2019DockerImage:
kind: dockerimage
name: Check if the container image "eclipse-temurin:<lastVersion>-jdk-windowsservercore-1809" is available
disablesourceinput: true
transformers:
- findsubmatch:
pattern: "(.*\\_\\d*)(\\.\\d*)?$"
captureindex: 1
- addsuffix: "-jdk-windowsservercore-1809"
spec:
architecture: amd64
architecture: windows/amd64
image: eclipse-temurin
tag: '{{ source "getLatestJDK21Version" }}-jdk-windowsservercore-1809'
checkTemurinNanoserver2022DockerImage:
kind: dockerimage
name: Check if the container image "eclipse-temurin:<lastVersion>-jdk-nanoserver-ltsc2022" is available
disablesourceinput: true
transformers:
- findsubmatch:
pattern: "(.*\\_\\d*)(\\.\\d*)?$"
captureindex: 1
- addsuffix: "-jdk-nanoserver-ltsc2022"
spec:
architecture: amd64
architecture: windows/amd64
image: eclipse-temurin
tag: '{{ source "getLatestJDK21Version" }}-jdk-nanoserver-ltsc2022'
checkTemurinWindowsCore2022DockerImage:
kind: dockerimage
name: Check if the container image "eclipse-temurin:<lastVersion>-jdk-windowsservercore-ltsc2022" is available
disablesourceinput: true
transformers:
- findsubmatch:
pattern: "(.*\\_\\d*)(\\.\\d*)?$"
captureindex: 1
- addsuffix: "-jdk-windowsservercore-ltsc2022"
spec:
architecture: amd64
architecture: windows/amd64
image: eclipse-temurin
tag: '{{ source "getLatestJDK21Version" }}-jdk-windowsservercore-ltsc2022'

targets:
setJDK21Version:
Expand Down