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

chore(updatecli) Fix JDK manifests to use the proper Ubuntu version #762

Merged
merged 12 commits into from Mar 18, 2024

Conversation

gounthar
Copy link
Contributor

@gounthar gounthar commented Mar 6, 2024

Moved from focal to jammy for JDK17/21 and added the architecture for Windows and linux/arm/v7 docker images.
JDK11 will fail because of :

jdk-11.0.22+7.1 contains a respin for aarch64_mac described in the 'aarch64 macOS Respin' section of this blog post https://adoptium.net/blog/2024/01/eclipse-temurin-8u402-11022-1710-and-2102-available/
So since it was specific to that platform, it is not relevant for any containers we currently publish to Docker Hub.

Testing done

updatecli diff --config ./updatecli/updatecli.d/jdk21.yaml --values ./updatecli/values.github-action.yaml
updatecli diff --config ./updatecli/updatecli.d/jdk17.yaml --values ./updatecli/values.github-action.yaml
updatecli diff --config ./updatecli/updatecli.d/jdk11.yaml --values ./updatecli/values.github-action.yaml

Submitter checklist

  • Make sure you are opening from a topic/feature/bugfix branch (right side) and not your main branch!
  • Ensure that the pull request title represents the desired changelog entry
  • Please describe what you did
  • Link to relevant issues in GitHub or Jira
  • Link to relevant pull requests, esp. upstream and downstream changes
  • Ensure you have provided tests - that demonstrates feature works or fixes the issue

@gounthar gounthar requested a review from a team as a code owner March 6, 2024 17:30
@gounthar gounthar changed the title fix(dependencies): Updatecli now needs to get more precise about the … fix(dependencies): Updatecli now needs to get a more precise architecture for docker images. Mar 6, 2024
@@ -51,44 +51,44 @@ conditions:
tag: '{{ source "getLatestJDK21Version" }}-jdk-alpine'
checkTemurinDebianDockerImages:
kind: dockerimage
name: Check if the container image "eclipse-temurin:<lastVersion>-jdk-focal" is available
name: Check if the container image "eclipse-temurin:<lastVersion>-jdk-jammy" is available
disablesourceinput: true
spec:
architectures:
- amd64
Copy link
Contributor

Choose a reason for hiding this comment

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

Don't you need OS here as well? That is:

  - linux/amd64
  - linux/arm64

This would apply to the checkTemurinAlpineDockerImage above as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, as in the documentation, I can read:

The default operating system is linux.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, right. Doesn't hurt, but isn't necessary.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, doesn't hurt, it's some kind of "the documentation is the code". 👍

@gounthar
Copy link
Contributor Author

gounthar commented Mar 6, 2024

Got a 503, as the #761.

@gounthar
Copy link
Contributor Author

gounthar commented Mar 8, 2024

Now it builds! 🚀

@MarkEWaite
Copy link
Contributor

@gounthar can you resolve the conflicts in this that came from my merge of

@gounthar
Copy link
Contributor Author

It's done.
Our two PRs were quite similar, except Tim went way farther with rewriting the script. 😉
Thanks, Mark. 👍

@gounthar gounthar changed the title fix(dependencies): Updatecli now needs to get a more precise architecture for docker images. chore(updatecli) Fix JDK manifests to use the proper Ubuntu version Mar 18, 2024
Copy link
Contributor

@dduportal dduportal left a comment

Choose a reason for hiding this comment

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

Run Summary
===========
Pipeline(s) run:
  * Changed:	0
  * Failed:	0
  * Skipped:	0
  * Succeeded:	10
  * Total:	10

LGTM!

Food for though (for another PR): we should be able to add a 2nd source to updatecli manifest which would retrieve the current Docker image suffix (or distribution) to make it more universal. But it is nitpicking.

@dduportal dduportal merged commit d50e9fc into jenkinsci:master Mar 18, 2024
10 checks passed
@gounthar gounthar deleted the updatecli-jdk branch March 18, 2024 13:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants