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

Update Java/base image and added DCE images #461

Merged
merged 8 commits into from
Feb 11, 2021

Conversation

tobias-trabelsi-sonarsource
Copy link
Contributor

No description provided.

@tobias-trabelsi-sonarsource tobias-trabelsi-sonarsource deleted the feature/tt/dce-images branch February 11, 2021 07:13
@@ -7,39 +8,39 @@ ENV JAVA_VERSION="jdk-11.0.8+10" \
#
# glibc setup
#
RUN set -eux; \
Copy link
Contributor

Choose a reason for hiding this comment

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

Our experience is that set -eux is much more reliable and consistent than using a long && chain -- any particular reason this was switched the other direction in a seemingly unrelated PR?

(Coming here from the downstream review at docker-library/official-images#9631)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi @tianon ,
to ease the update of java. we want to stay on alpine but use adoptopenjdk and they are using the && notation

Copy link

Choose a reason for hiding this comment

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

Hi @tianon, I think Tobias' point is that the && chain is directly taken from the Dockerfile of Adoptopenjdk official image which is already accepted by Docker.

We are currently trying to release Sonarqube 8.7 and we are only waiting for the official docker image to be published. I would like to understand whether using long "&& chain" instead of "set -eux" is considered a regression for you, and it's the only issue blocking Docker from public image for Sonarqube ?

If it is, we can discuss with @tobias-trabelsi-sonarsource if we can change back to use "set -eux", or directly use adoptopenjdk/openjdk11 as base image.

Copy link
Contributor

Choose a reason for hiding this comment

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

The Alpine variants of the adoptopenjdk image were not accepted after a discussion with the maintainer (see Alpine-related discussion points in docker-library/official-images#5710 -- otherwise I would be suggesting you adjust to be FROM adoptopenjdk:11-alpine or similar); this change in particular stands out because it makes the diff significantly larger for no apparent purpose (and in a way that's strictly worse than the previous implementation).

Using FROM adoptopenjdk/openjdk11 will not be acceptable (https://github.com/docker-library/official-images#repeatability).

Copy link

Choose a reason for hiding this comment

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

HI @tianon, thanks a lot for your clarification. @tobias-trabelsi-sonarsource knows much more on the docker topic, but i get your point now. Due to Europe time zone, we will get back to fix it tomorrow morning.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@tianon we are reverting back to set -eux (see PR #468) . can you give any explanation why you think this is a worse implementation? i would value them as equal if we set -o pipefail to the long && chain and it would greatly reduce the maintenance effort on our side

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.

4 participants