Skip to content

ORC-694: Update docker files for adding Java11 support#655

Closed
yuokada wants to merge 9 commits into
apache:masterfrom
yuokada:ORC-694
Closed

ORC-694: Update docker files for adding Java11 support#655
yuokada wants to merge 9 commits into
apache:masterfrom
yuokada:ORC-694

Conversation

@yuokada

@yuokada yuokada commented Mar 15, 2021

Copy link
Copy Markdown
Contributor

What changes were proposed in this pull request?

  • Add jdk argument in Dockerfile to support JDK11

Why are the changes needed?

  • Since github actions for the repository use java 11, we should provide the images using JDK 11

How was this patch tested?

  • Confirmed that Dockerfiles are built successfully with reinit.sh
  • bash -xe ./run-one.sh yuokada ORC-694 ubuntu20

@dongjoon-hyun dongjoon-hyun left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thank you for working on this, @yuokada .

BTW, do we need to change MAINTAINER in this PR?

@yuokada

yuokada commented Mar 15, 2021

Copy link
Copy Markdown
Contributor Author

@dongjoon-hyun
MAINTAINER command is deprecated since docker 1.13 or later. So I replaced it with the LABEL command.
https://docs.docker.com/engine/reference/builder/#maintainer-deprecated

But, we don't need to change it in this PR.

Should we create another ticket to replace MAINTAINER command?

@dongjoon-hyun

Copy link
Copy Markdown
Member

Shall we do that in a separate PR because it's irrelevant to Java?

@yuokada

yuokada commented Mar 16, 2021

Copy link
Copy Markdown
Contributor Author

Created: #656

Comment thread docker/README.md Outdated

cd docker/$os
docker build -t orc-$os .
docker build -t "orc-$os-jdk8" --build-arg jdk=8 . OR docker build -t "orc-$os-jdk11" --build-arg jdk=11 .

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Is this change easy to understand?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Lets make it more explicit.
For jdk8: docker build -t "orc-$os-jdk8" --build-arg jdk=8 .
For jdk11: docker build -t "orc-$os-jdk11" --build-arg jdk=11 .

@yuokada yuokada marked this pull request as ready for review March 16, 2021 07:21

@dongjoon-hyun dongjoon-hyun left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Could you minimize the size of patch at CentOS Dockerfile? Technically, if statement should be related to java-xx-openjdk. For the other part git/maven/make/..., we don't need to duplicate.

Comment thread docker/ubuntu18/Dockerfile
Comment thread docker/ubuntu20-clang/Dockerfile
Comment thread docker/ubuntu20/Dockerfile
Comment thread docker/run-all.sh Outdated
( cd $os && docker build -t "orc-$os" . ) > logs/$os-build.log 2>&1 || exit 1
for jdk in 8 11; do
for os in `cat os-list.txt`; do
echo "Building $os"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Could you show $jdk together?

@dongjoon-hyun dongjoon-hyun left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thank you for updating, @yuokada . The PR looks good mostly. I left one minor comment about log message.

Comment thread docker/README.md Outdated

cd docker/$os
docker build -t orc-$os .
docker build -t "orc-$os-jdk8" --build-arg jdk=8 . OR docker build -t "orc-$os-jdk11" --build-arg jdk=11 .

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Lets make it more explicit.
For jdk8: docker build -t "orc-$os-jdk8" --build-arg jdk=8 .
For jdk11: docker build -t "orc-$os-jdk11" --build-arg jdk=11 .

Comment thread docker/README.md
@@ -21,7 +21,7 @@ The scripts are:
A base image for each OS is built using:

@pgaref pgaref Mar 17, 2021

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Lets add above that:
run-all.sh, run-one.sh and reinit.sh tests both on jdk8 and 11 across OSes

@pgaref

pgaref commented Mar 17, 2021

Copy link
Copy Markdown
Contributor

Thanks @yuokada and @dongjoon-hyun for the work on this! Left some minor comments.
Seems to work well locally!

@dongjoon-hyun dongjoon-hyun left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I built and ran this PR.

$ for i in $(docker images | grep orc | awk '{print $1}'); do echo $i; docker run -it --rm $i java -version; done
orc-ubuntu20-clang-jdk11
openjdk version "11.0.10" 2021-01-19
OpenJDK Runtime Environment (build 11.0.10+9-Ubuntu-0ubuntu1.20.04)
OpenJDK 64-Bit Server VM (build 11.0.10+9-Ubuntu-0ubuntu1.20.04, mixed mode, sharing)
orc-ubuntu20-jdk11
openjdk version "11.0.10" 2021-01-19
OpenJDK Runtime Environment (build 11.0.10+9-Ubuntu-0ubuntu1.20.04)
OpenJDK 64-Bit Server VM (build 11.0.10+9-Ubuntu-0ubuntu1.20.04, mixed mode, sharing)
orc-ubuntu18-jdk11
openjdk version "11.0.10" 2021-01-19
OpenJDK Runtime Environment (build 11.0.10+9-Ubuntu-0ubuntu1.18.04)
OpenJDK 64-Bit Server VM (build 11.0.10+9-Ubuntu-0ubuntu1.18.04, mixed mode, sharing)
orc-centos8-jdk11
openjdk version "11.0.9.1" 2020-11-04 LTS
OpenJDK Runtime Environment 18.9 (build 11.0.9.1+1-LTS)
OpenJDK 64-Bit Server VM 18.9 (build 11.0.9.1+1-LTS, mixed mode, sharing)
orc-centos7-jdk11
openjdk version "11.0.10" 2021-01-19 LTS
OpenJDK Runtime Environment 18.9 (build 11.0.10+9-LTS)
OpenJDK 64-Bit Server VM 18.9 (build 11.0.10+9-LTS, mixed mode, sharing)
orc-ubuntu20-clang-jdk8
openjdk version "1.8.0_282"
OpenJDK Runtime Environment (build 1.8.0_282-8u282-b08-0ubuntu1~20.04-b08)
OpenJDK 64-Bit Server VM (build 25.282-b08, mixed mode)
orc-ubuntu20-jdk8
openjdk version "1.8.0_282"
OpenJDK Runtime Environment (build 1.8.0_282-8u282-b08-0ubuntu1~20.04-b08)
OpenJDK 64-Bit Server VM (build 25.282-b08, mixed mode)
orc-ubuntu18-jdk8
openjdk version "1.8.0_282"
OpenJDK Runtime Environment (build 1.8.0_282-8u282-b08-0ubuntu1~18.04-b08)
OpenJDK 64-Bit Server VM (build 25.282-b08, mixed mode)
orc-ubuntu16-jdk11
openjdk version "1.8.0_282"
OpenJDK Runtime Environment (build 1.8.0_282-8u282-b08-0ubuntu1~16.04-b08)
OpenJDK 64-Bit Server VM (build 25.282-b08, mixed mode)
orc-ubuntu16-jdk8
openjdk version "1.8.0_282"
OpenJDK Runtime Environment (build 1.8.0_282-8u282-b08-0ubuntu1~16.04-b08)
OpenJDK 64-Bit Server VM (build 25.282-b08, mixed mode)
orc-debian10-jdk11
openjdk version "11.0.9.1" 2020-11-04
OpenJDK Runtime Environment (build 11.0.9.1+1-post-Debian-1deb10u2)
OpenJDK 64-Bit Server VM (build 11.0.9.1+1-post-Debian-1deb10u2, mixed mode, sharing)
orc-debian10-jdk8
openjdk version "11.0.9.1" 2020-11-04
OpenJDK Runtime Environment (build 11.0.9.1+1-post-Debian-1deb10u2)
OpenJDK 64-Bit Server VM (build 11.0.9.1+1-post-Debian-1deb10u2, mixed mode, sharing)
orc-debian9-jdk11
openjdk version "1.8.0_275"
OpenJDK Runtime Environment (build 1.8.0_275-8u275-b01-1~deb9u1-b01)
OpenJDK 64-Bit Server VM (build 25.275-b01, mixed mode)
orc-debian9-jdk8
openjdk version "1.8.0_275"
OpenJDK Runtime Environment (build 1.8.0_275-8u275-b01-1~deb9u1-b01)
OpenJDK 64-Bit Server VM (build 25.275-b01, mixed mode)
orc-centos8-jdk8
openjdk version "1.8.0_275"
OpenJDK Runtime Environment (build 1.8.0_275-b01)
OpenJDK 64-Bit Server VM (build 25.275-b01, mixed mode)
orc-centos7-jdk8
openjdk version "1.8.0_282"
OpenJDK Runtime Environment (build 1.8.0_282-b08)
OpenJDK 64-Bit Server VM (build 25.282-b08, mixed mode)

In short, the following three look suspicious to me. Could you double-check it, @yuokada ?

orc-ubuntu16-jdk11
openjdk version "1.8.0_282"
OpenJDK Runtime Environment (build 1.8.0_282-8u282-b08-0ubuntu1~16.04-b08)
OpenJDK 64-Bit Server VM (build 25.282-b08, mixed mode)
...
orc-debian10-jdk8
openjdk version "11.0.9.1" 2020-11-04
OpenJDK Runtime Environment (build 11.0.9.1+1-post-Debian-1deb10u2)
OpenJDK 64-Bit Server VM (build 11.0.9.1+1-post-Debian-1deb10u2, mixed mode, sharing)
...
orc-debian9-jdk11
openjdk version "1.8.0_275"
OpenJDK Runtime Environment (build 1.8.0_275-8u275-b01-1~deb9u1-b01)
OpenJDK 64-Bit Server VM (build 25.275-b01, mixed mode)

@yuokada

yuokada commented Mar 17, 2021

Copy link
Copy Markdown
Contributor Author

@dongjoon-hyun
Just questions.
For debian 10, the OS image already use jdk10 jdk11. Should we support jdk8 on it?

For debian 9 and ubuntu16, the openjdk-11 deb package isn't provided for both OSes.
Do we need to support JDK 11 in both?

Or should we skip building an image on those OSes?

@dongjoon-hyun

Copy link
Copy Markdown
Member

If there is no JDK11 package, we had better document it and skip JDK11 for old OSes inside the above script like run-all.sh.

@omalley omalley closed this in 75726b1 Mar 18, 2021
@dongjoon-hyun

Copy link
Copy Markdown
Member

Thank you, @yuokada and @omalley .

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.

3 participants