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

Adding Pinot base build/runtime image support for Amazon Corretto and MS OpenJDK #10422

Merged
merged 1 commit into from
Mar 15, 2023

Conversation

xiangfu0
Copy link
Contributor

Make Pinot base build and runtime images support Amazon Corretto and MS OpenJDK

@abhijeetkushe
Copy link
Contributor

Thanks @xiangfu0 Are we going to end creating images for oracle openjdk ?

@xiangfu0
Copy link
Contributor Author

Thanks @xiangfu0 Are we going to end creating images for oracle openjdk ?

I plan to switch the default to amazon jdk then also publish ms-openjdk image.

@abhijeetkushe
Copy link
Contributor

Thanks @xiangfu0 Are we going to end creating images for oracle openjdk ?

I plan to switch the default to amazon jdk then also publish ms-openjdk image.

I see Thanks for working on this

@abhijeetkushe
Copy link
Contributor

Looks good to me

@codecov-commenter
Copy link

Codecov Report

Merging #10422 (30ac267) into master (dadd42c) will decrease coverage by 6.05%.
The diff coverage is n/a.

@@             Coverage Diff              @@
##             master   #10422      +/-   ##
============================================
- Coverage     70.26%   64.21%   -6.05%     
- Complexity     5269     6082     +813     
============================================
  Files          2049     1999      -50     
  Lines        111068   108935    -2133     
  Branches      16898    16646     -252     
============================================
- Hits          78045    69956    -8089     
- Misses        27554    33888    +6334     
+ Partials       5469     5091     -378     
Flag Coverage Δ
integration1 ?
integration2 ?
unittests1 67.99% <ø> (+0.13%) ⬆️
unittests2 13.85% <ø> (-0.05%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

see 525 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@xiangfu0 xiangfu0 merged commit 8565032 into apache:master Mar 15, 2023
@xiangfu0 xiangfu0 deleted the upgrade-github-action-versino branch March 15, 2023 05:06

```SHELL
docker build -t apachepinot/pinot-base-runtime:openjdk11-arm64v8 --platform linux/arm64 --no-cache --network=host --build-arg JAVA_VERSION=11 --build-arg OPENJDK_IMAGE=arm64v8/openjdk -f pinot-base-runtime/Dockerfile .
docker buildx build --no-cache --platform=linux/arm64,linux/amd64 --file pinot-base-build/amazoncorretto.dockerfile --tag apachepinot/pinot-base-build:11-ms-openjdk --push .
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this a typo? I would expect that for MS OpenJDK we should use ms-openjdk.dockerfile as base image.

# fontconfig. The folks who manage the docker hub's
# official image library have found that font management
# is a common usecase, and painpoint, and have
# recommended that Java images include font support.
Copy link
Contributor

Choose a reason for hiding this comment

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

Why don't we install the same in ms or openjdk versions?

#
ARG JAVA_VERSION=11
ARG JDK_IMAGE=mcr.microsoft.com/openjdk/jdk
FROM ${JDK_IMAGE}:${JAVA_VERSION}-ubuntu AS pinot_build_env
Copy link
Contributor

Choose a reason for hiding this comment

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

ms-openjdk.dockerfile and openjdk.dockerfile are just the same except for these two lines:

ARG JDK_IMAGE=openjdk
FROM ${JDK_IMAGE}:${JAVA_VERSION} AS pinot_build_env
ARG JDK_IMAGE=mcr.microsoft.com/openjdk/jdk
FROM ${JDK_IMAGE}:${JAVA_VERSION}-ubuntu AS pinot_build_env

The differences with amazoncoretto.dockerfile are not that large if we also include fontconfig. Shouldn't be enough to have a single dockerfile base image (same with runtime) where FROM is build from docker args?

ARG JAVA_VERSION=11
ARG JDK_IMAGE=amazoncorretto

FROM ${JDK_IMAGE}:${JAVA_VERSION}-al2-jdk
Copy link
Contributor

Choose a reason for hiding this comment

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

This is the only image that doesn't derive from ubuntu. I don't mind to use alpine images (although I've never like the hype on alpine docker images, the base image should be cached in the node anyway), but having to choose between alpine or ubuntu depending on the vendor doesn't sound good.

I mean, I would prefer to have a dockerfile for ubuntu, a dockerfile for alpine and choose the meta-base (either corretto, ms or whatever) based on docker ARGs. Otherwise we will see a combinatorial explosion of different dockerfiles depending on the openjdk distribution and the linux distribution

@abhijeetkushe
Copy link
Contributor

Thanks for the quick work @xiangfu0 .Just curious does the latest-11-amazoncorretto point to 0.12.1

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.

5 participants