-
-
Notifications
You must be signed in to change notification settings - Fork 45
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
updatecli jdk tools #4998
updatecli jdk tools #4998
Conversation
Signed-off-by: Sarath Chandra Oruganti <39090092+SarathChandra24@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Many many thanks for this change proposal!
- The problem targeted is legit and have to be fixed to allow us to update the JDK tools on this controller!
- You caught a first good problem (commented out updatecli lines which forbids to create automated PR to update JDK versions)
However your PR underlines a flaw in the existing manifest (not your fault!): the generated URLs are invalid as only partially updated (either parts of the PATH are updated OR the filename).
As such we need to fix the manifest to merge your PR (otherwise it would create invalid PR: again not your fault).
# jdk-11.0.22+7.1 (https://github.com/adoptium/temurin11-binaries/releases/tag/jdk-11.0.22%2B7.1) is Ok | ||
pattern: ^jdk-11.*.(\d*)+(\d*)$ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You should look at https://github.com/jenkinsci/docker-agent/pull/761/files/f95d636942e2b4308f3f1726064c4cde3ef0dc49#r1520406646 and reuse the same behavior to avoid repeating the same problems we faced on the official Jenkins Docker agent images:
- With this pattern change, the "latest" version found is
11.0.22+7.1
which only provides binaries for macOS aarch64 (see https://github.com/adoptium/temurin11-binaries/releases/tag/jdk-11.0.22%2B7.1): we want to usejdk-11.0.22+7
instead (e.g. dropping any "hotfix")
scmid: default | ||
|
||
# actions: | ||
# default: | ||
# kind: github/pullrequest | ||
# scmid: default | ||
# title: Bump JDK17 version (Jenkins tools) on all controllers to {{ source "lastVersion" }} | ||
# spec: | ||
# labels: | ||
# - dependencies | ||
# - jdk17 | ||
actions: | ||
default: | ||
kind: github/pullrequest | ||
scmid: default | ||
title: Bump JDK17 version (Jenkins tools) on all controllers to {{ source "lastVersion" }} | ||
spec: | ||
labels: | ||
- dependencies | ||
- jdk17 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch to uncomment these lines!
However the manifest still generates an invalid URL: it might come from error on the current manifest but it needs to be fixed to avoid failing PRs:
setJDK17UrlFilenames
--------------------
**Dry Run enabled**
"config/jenkins_infra.ci.jenkins.io.yaml" updated with [dry run] content "_17.0.10_7.$2\""
```
--- config/jenkins_infra.ci.jenkins.io.yaml
+++ config/jenkins_infra.ci.jenkins.io.yaml
@@ -755,15 +755,15 @@
- zip:
label: "linux && amd64"
subdir: "jdk-17.0.8.1+1"
- url: "https://github.com/adoptium/temurin17-binaries/releases/download/jdk-17.0.8.1+1/OpenJDK17U-jdk_x64_linux_hotspot_17.0.8.1_1.tar.gz"
+ url: "https://github.com/adoptium/temurin17-binaries/releases/download/jdk-17.0.8.1+1/OpenJDK17U-jdk_x64_linux_hotspot_17.0.10_7.tar.gz"
- zip:
label: "windows"
subdir: "jdk-17.0.8.1+1"
- url: "https://github.com/adoptium/temurin17-binaries/releases/download/jdk-17.0.8.1+1/OpenJDK17U-jdk_x64_windows_hotspot_17.0.8.1_1.zip"
+ url: "https://github.com/adoptium/temurin17-binaries/releases/download/jdk-17.0.8.1+1/OpenJDK17U-jdk_x64_windows_hotspot_17.0.10_7.zip"
- zip:
label: "linux && arm64"
subdir: "jdk-17.0.8.1+1"
- url: "https://github.com/adoptium/temurin17-binaries/releases/download/jdk-17.0.8.1+1/OpenJDK17U-jdk_aarch64_linux_hotspot_17.0.8.1_1.tar.gz"
+ url: "https://github.com/adoptium/temurin17-binaries/releases/download/jdk-17.0.8.1+1/OpenJDK17U-jdk_aarch64_linux_hotspot_17.0.10_7.tar.gz"
- name: "jdk21"
properties:
- installSource:
```
scmid: default | ||
|
||
# actions: | ||
# default: | ||
# kind: github/pullrequest | ||
# scmid: default | ||
# title: Bump JDK21 version (Jenkins tools) on all controllers to {{ source "lastVersion" }} | ||
# spec: | ||
# labels: | ||
# - dependencies | ||
# - jdk21 | ||
actions: | ||
default: | ||
kind: github/pullrequest | ||
scmid: default | ||
title: Bump JDK21 version (Jenkins tools) on all controllers to {{ source "lastVersion" }} | ||
spec: | ||
labels: | ||
- dependencies | ||
- jdk21 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same as the JDK17 above:
- Your change is a good catch!
- But we cannot merge it unless the manifest is fixed to generate the proper URLs
Hi @sarathchandra24 , any news on this PR or shall we close it? |
Closing as superseded by #5171 |
#4630
updatecli track JDK tools installation
Updated the regex to include the release of the following:
jdk-21.0.2+13
jdk-21+35
jdk-21.0.1+12
or something similar in java 11 and java 17.
Will ignore the rest: eg: jdk21u-2024-02-07-08-08-beta