You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@druid.apache.org by GitBox <gi...@apache.org> on 2021/01/31 00:56:00 UTC
[GitHub] [druid] jihoonson commented on a change in pull request #10786: move integration tests from ZooKeeper 3.4.x to 3.5.x
jihoonson commented on a change in pull request #10786:
URL: https://github.com/apache/druid/pull/10786#discussion_r567342811
##########
File path: integration-tests/docker/Dockerfile
##########
@@ -13,16 +13,19 @@
# See the License for the specific language governing permissions and
# limitations under the License.
+ARG JDK_VERSION=8-slim
+FROM openjdk:$JDK_VERSION as druidbase
-# This is default value for base image in case DOCKER_IMAGE is not given when building
-ARG DOCKER_IMAGE=imply/druiditbase:openjdk-1.8.0_191-1
-# Base image is built from integration-tests/docker-base in the Druid repo
-FROM $DOCKER_IMAGE
+# Bundle everything into one script so cleanup can reduce image size.
+# Otherwise docker's layered images mean that things are not actually deleted.
+
+COPY base-setup.sh /root/base-setup.sh
+RUN /root/base-setup.sh && rm -f /root/base-setup.sh
+
+FROM druidbase
+ARG MYSQL_CONNECTOR_VERSION=5.1.49
Review comment:
nit: better to bump the mysql connector version in the pom to match to this. If you are going to change the version, can you please add a comment in the pom file that says these versions should be in sync? It would be nice if we can sync all these library versions automatically, but it could be done in a different PR in the future.
##########
File path: integration-tests/docker/Dockerfile
##########
@@ -13,16 +13,19 @@
# See the License for the specific language governing permissions and
# limitations under the License.
+ARG JDK_VERSION=8-slim
+FROM openjdk:$JDK_VERSION as druidbase
-# This is default value for base image in case DOCKER_IMAGE is not given when building
-ARG DOCKER_IMAGE=imply/druiditbase:openjdk-1.8.0_191-1
-# Base image is built from integration-tests/docker-base in the Druid repo
-FROM $DOCKER_IMAGE
+# Bundle everything into one script so cleanup can reduce image size.
+# Otherwise docker's layered images mean that things are not actually deleted.
+
+COPY base-setup.sh /root/base-setup.sh
+RUN /root/base-setup.sh && rm -f /root/base-setup.sh
Review comment:
This change makes every Travis job to download Kafka and ZooKeeper binaries every time. This can be a problem because integration tests will become flaky if the HTTP connection is unstable. I tested the flakiness by restarting all integration tests of this PR and it seems OK. So, I'm OK with this change in this PR, but we need to address this potential flakiness as soon as possible.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org