updatecli jdk tools #4998
Closed
updatecli jdk tools #4998
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: Sarath Chandra Oruganti <39090092+SarathChandra24@users.noreply.github.com>
dduportal
requested changes
Mar 18, 2024
There was a problem hiding this comment.
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).
Comment on lines
+27
to
+28
| # 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.
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.1which 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+7instead (e.g. dropping any "hotfix")
Comment on lines
+78
to
+88
| 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.
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:
```
Comment on lines
+78
to
+88
| 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.
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? |
This was referenced Apr 24, 2024
|
Closing as superseded by #5171 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
#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