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 to OpenJDK11 in Docker image #2433

Merged
merged 4 commits into from
Oct 16, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 5 additions & 4 deletions docker/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -20,20 +20,20 @@
FROM centos:7
MAINTAINER Apache BookKeeper <[email protected]>

ARG BK_VERSION=4.9.0
ARG BK_VERSION=4.11.0
Copy link
Contributor

Choose a reason for hiding this comment

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

This also upgrades the image to use 4.11.0, apart from changing the Java version. Would it make sense to separate the two and merge it to older release lines, so that users using older versions of Bookkeeper like v4.9.2 can also benefit from the Java upgrade?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am not sure that the bash files on 4.9.2 work with java11 due to removed JVM command line flags.

4.10 should work.

Btw we are close to releasing 4.12 so 4.9 is becoming far distant from current versions

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Also It would be better to use the version in pom files and in current master it should be 4.12.0-SNAPSHOT but if I change it the image cannot be built so currently I can only use this version.
We are in the middle of the process of releasing 4 11.1 so probably we will set 4.11.1 here on current master.

Another solution would be to pick the binaries from local filesystem if the version is a SNAPSHOT

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is my fix for using JRE
that said, it won't be available to old BK versions
#2436

with BK 4.12.0 we will be able to use JRE 11

ARG DISTRO_NAME=bookkeeper-server-${BK_VERSION}-bin
ARG DISTRO_URL=https://archive.apache.org/dist/bookkeeper/bookkeeper-${BK_VERSION}/${DISTRO_NAME}.tar.gz

ENV BOOKIE_PORT=3181
EXPOSE $BOOKIE_PORT
ENV BK_USER=bookkeeper
ENV BK_HOME=/opt/bookkeeper
ENV JAVA_HOME=/usr/lib/jvm/jre-1.8.0
ENV JAVA_HOME=/usr/lib/jvm/jdk-11

# Download Apache Bookkeeper, untar and clean up
RUN set -x \
&& adduser "${BK_USER}" \
&& yum install -y java-1.8.0-openjdk-headless wget bash python sudo\
&& yum install -y java-11-openjdk-devel wget bash python sudo\
&& mkdir -pv /opt \
&& cd /opt \
&& wget -q "${DISTRO_URL}" \
Expand All @@ -52,7 +52,8 @@ RUN set -x \
&& pip install zk-shell \
&& rm -rf get-pip.py \
&& yum remove -y wget \
&& yum clean all
&& yum clean all \
&& ls /usr/lib/jvm

WORKDIR /opt/bookkeeper

Expand Down
2 changes: 1 addition & 1 deletion docker/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ Bookkeeper needs [Zookeeper](https://zookeeper.apache.org/) in order to preserve
Just like running a BookKeeper cluster in one machine(http://bookkeeper.apache.org/docs/latest/getting-started/run-locally/), you can run a standalone BookKeeper in one docker container, the command is:
```
docker run -it \
--env JAVA_HOME=/usr/lib/jvm/jre-1.8.0 \
--env JAVA_HOME=/usr/lib/jvm/jdk-11 \
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we install the JRE instead? I suppose installing JDK instead of JRE would increase the image size. I'm not sure whether that increase in size is significant in relation to the overall image size or not. What are your thoughts about this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I would like, but in current 4.11 we are using the presence of shell to detect jdk11 and it is not present in the.

We should fix that problem, then we can use the jre.

Btw I don't know a good way to detect jdk11+

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ravisharda here it is a fix to support JRE and only only JDK
#2436

--entrypoint "/bin/bash" \
apache/bookkeeper \
-c "/opt/bookkeeper/bin/bookkeeper localbookie 3"
Expand Down
6 changes: 3 additions & 3 deletions docker/docker-compose.yml
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ services:
links:
- zookeeper
environment:
- JAVA_HOME=/usr/lib/jvm/jre-1.8.0
- JAVA_HOME=/usr/lib/jvm/jdk-11
- BK_zkServers=zookeeper:2181
- BK_zkLedgersRootPath=/ledgers

Expand All @@ -38,7 +38,7 @@ services:
links:
- zookeeper
environment:
- JAVA_HOME=/usr/lib/jvm/jre-1.8.0
- JAVA_HOME=/usr/lib/jvm/jdk-11
- BK_zkServers=zookeeper:2181
- BK_zkLedgersRootPath=/ledgers

Expand All @@ -48,7 +48,7 @@ services:
links:
- zookeeper
environment:
- JAVA_HOME=/usr/lib/jvm/jre-1.8.0
- JAVA_HOME=/usr/lib/jvm/jdk-11
- BK_zkServers=zookeeper:2181
- BK_zkLedgersRootPath=/ledgers

Expand Down
2 changes: 1 addition & 1 deletion docker/scripts/entrypoint.sh
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
# */

export PATH=$PATH:/opt/bookkeeper/bin
export JAVA_HOME=/usr/lib/jvm/jre-1.8.0
export JAVA_HOME=/usr/lib/jvm/jdk-11

BK_HOME=/opt/bookkeeper
BINDIR=${BK_HOME}/bin
Expand Down
2 changes: 1 addition & 1 deletion docker/scripts/healthcheck.sh
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@

set -x -e -u

export JAVA_HOME=/usr/lib/jvm/jre-1.8.0
export JAVA_HOME=/usr/lib/jvm/jdk-11

# Sanity check that creates a ledger, writes a few entries, reads them and deletes the ledger.
DEFAULT_HEALTH_CHECK_CMD="/opt/bookkeeper/bin/bookkeeper shell bookiesanity"
Expand Down
4 changes: 2 additions & 2 deletions tests/docker-images/current-version-image/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -30,13 +30,13 @@ ENV BOOKIE_GRPC_PORT=4181
EXPOSE ${BOOKIE_PORT} ${BOOKIE_HTTP_PORT} ${BOOKIE_GRPC_PORT}
ENV BK_USER=bookkeeper
ENV BK_HOME=/opt/bookkeeper
ENV JAVA_HOME=/usr/lib/jvm/jre-1.8.0
ENV JAVA_HOME=/usr/lib/jvm/jdk-11

# prepare utils
RUN set -x \
&& adduser "${BK_USER}" \
&& yum install -y epel-release \
&& yum install -y java-1.8.0-openjdk-headless wget bash python-pip python-devel sudo netcat gcc gcc-c++ \
&& yum install -y java-11-openjdk-devel wget bash python-pip python-devel sudo netcat gcc gcc-c++ \
&& mkdir -pv /opt \
&& cd /opt \
# install zookeeper shell
Expand Down