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

Use alpine's JRE instead of AdoptOpenJDK #193

Merged
merged 8 commits into from
Sep 9, 2021

Conversation

username-is-too-long
Copy link
Contributor

@username-is-too-long username-is-too-long commented Sep 3, 2021

AdoptOpenJDK now has a DEPRECATION NOTICE warning, and Alpine has its own packaged openjdk/jre that uses musl libc, so lines 23-35 and 55-86 could be replaced.

  • 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

[AdoptOpenJDK](https://github.com/AdoptOpenJDK/openjdk-docker) now has a DEPRECATION NOTICE warning, and Alpine has its own packaged openjdk/jre that uses musl libc, so lines 23-35 and 55-86 could be replaced.
@username-is-too-long username-is-too-long requested a review from a team as a code owner September 3, 2021 18:10
@MarkEWaite
Copy link
Contributor

MarkEWaite commented Sep 3, 2021

I recommend against this change based on the comments in jenkinsci/docker#1185 and in jenkinsci/docker#1186. I'm open to reconsider based on guidance from @timja and @dduportal , but the transition to Eclipse Temurin (AdoptOpenJDK) has been a positive change. One Java provider for all our Docker images with synchronized updates across all our platforms (AMD64, Arm64, PowerPC, and s390x).

@timja
Copy link
Member

timja commented Sep 3, 2021

I’ll look at this closer later but I think it might be better to take this as a temporary switch until temurin provides a musl based build,

see adoptium/containers#1 for background

@username-is-too-long
Copy link
Contributor Author

username-is-too-long commented Sep 3, 2021

see adoptium/containers#1 for background

Oh nice, I didn't see they were moving to musl on Alpine.

@timja
Copy link
Member

timja commented Sep 4, 2021

see adoptium/containers#1 for background

Oh nice, I didn't see they were moving to musl on Alpine.

It looks like awhile away, they first need to TCK 16+ and then someone needs to backport to 11 and 8. They also need to setup some infrastructure for it.

I wouldn’t expect it to be quick but I think we can swap to this in the meantime

@timja
Copy link
Member

timja commented Sep 4, 2021

@username-is-too-long the locale appears to not be set to a utf-8 one can you check it?

there was some code in the glibc section doing that, it might just be a matter of switching the locale line to:

ENV LANG C.UTF-8

but I don't think that worked last time I tried, see:
https://grrr.tech/posts/2020/add-locales-to-alpine-linux-docker-image/

@username-is-too-long
Copy link
Contributor Author

@username-is-too-long the locale appears to not be set to a utf-8 one can you check it?

there was some code in the glibc section doing that, it might just be a matter of switching the locale line to:

/usr/glibc-compat/bin/locale doesn't exist without without glibc, I added musl-locales to provide locale, and removed that alpine case in tests.bats.

&& rm -rf /tmp/*.apk /tmp/gcc /tmp/gcc-libs.tar* /tmp/libz /tmp/libz.tar.xz /var/cache/apk/*

RUN apk add --update --no-cache curl bash git git-lfs openssh-client openssl procps \
RUN apk add --update --no-cache curl bash git git-lfs musl-locales openjdk11-jre-headless openssh-client openssl procps \
Copy link
Member

Choose a reason for hiding this comment

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

we should still be running jlink to get a trimmed down runtime, it should be smaller than the jre

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You're right, saved about 80mb

@timja
Copy link
Member

timja commented Sep 6, 2021

(I sorted the build separately in a different PR, green now 😄 )

11/alpine/Dockerfile Outdated Show resolved Hide resolved
11/alpine/Dockerfile Outdated Show resolved Hide resolved
11/alpine/Dockerfile Outdated Show resolved Hide resolved
8/alpine/Dockerfile Outdated Show resolved Hide resolved
11/alpine/Dockerfile Outdated Show resolved Hide resolved
11/alpine/Dockerfile Outdated Show resolved Hide resolved
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.

3 participants