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

feat(alpine-jdk8) switch base image from deprecated adoptopenjdk to eclipse-temurin (adoptium) #119

Merged
merged 3 commits into from
May 11, 2022

Conversation

dduportal
Copy link
Contributor

@dduportal dduportal commented May 11, 2022

  • 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

@dduportal dduportal mentioned this pull request May 11, 2022
5 tasks
8/alpine/Dockerfile Outdated Show resolved Hide resolved
@dduportal
Copy link
Contributor Author

Good! the test are failing as described by @MarkEWaite in #118 . Gotta diagnose now.

@dduportal
Copy link
Contributor Author

Ok, the change of base image introduce the following test failure (only 1):

#11 [docker.io/jenkins/ssh-agent:alpine-jdk8] has utf-8 locale – jenkinsci.docker.alpine_jdk8.Default

which is this test:

https://github.com/jenkinsci/docker-ssh-agent/blob/master/tests/tests.bats#L187-L194

@dduportal
Copy link
Contributor Author

The adoptopenJDK image defined the UTF-8 locale in 2 different places:

@dduportal
Copy link
Contributor Author

On the temurin image, only the environment variables are used: https://github.com/adoptium/containers/blob/087b411c6bb456e4c2a417aa358e6ff81d5775c4/8/jdk/alpine/Dockerfile.releases.full#L22.

=> That explains the root cause of the test failing.
=> Now we have to take a decision by comparing to other Jenkins agent images + Jenkins controller images (as they all use temurin): do we need to change the test to only check the environment variable, or do we need to run the localedef command on our own

@dduportal
Copy link
Contributor Author

Please note that temurin relies on the native musl localedef: https://github.com/adoptium/containers/blob/087b411c6bb456e4c2a417aa358e6ff81d5775c4/8/jdk/alpine/Dockerfile.releases.full#L24 . Sounds like good enough

@dduportal
Copy link
Contributor Author

@MarkEWaite
Copy link
Contributor

I've merged #118. Apologies for the conflicts but most sincere thanks for applying your skills to the failing tests!

@dduportal dduportal changed the title feat(alpine-jdk8) switch base image from deprecated openjdk to temurin feat(alpine-jdk8) switch base image from deprecated adoptopenjdk to eclipse-temurin (adoptium) May 11, 2022
@dduportal dduportal marked this pull request as ready for review May 11, 2022 17:47
@dduportal dduportal requested a review from a team as a code owner May 11, 2022 17:47
@dduportal
Copy link
Contributor Author

PR ready for review: tests are now passing \o/

@dduportal dduportal requested a review from MarkEWaite May 11, 2022 17:47
Copy link
Contributor

@MarkEWaite MarkEWaite left a comment

Choose a reason for hiding this comment

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

That's great! Removed a special case in the test and switched to use a supported and maintained distribution in the Docker image.

@MarkEWaite MarkEWaite merged commit a94f307 into jenkinsci:master May 11, 2022
@dduportal dduportal deleted the patch-1 branch May 11, 2022 19:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants