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/21 18:13:26 UTC
[GitHub] [druid] xvrl opened a new pull request #10786: move integration tests from ZooKeeper 3.4.x to 3.5.x
xvrl opened a new pull request #10786:
URL: https://github.com/apache/druid/pull/10786
as we deprecate ZooKeeper 3.4.x, the first step is to move our integration tests from 3.4.x to 3.5.x
see #10780 for context
@clintropolis @gianm what's the process to build and update the base image?
----------------------------------------------------------------
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
[GitHub] [druid] jihoonson commented on pull request #10786: move integration tests from ZooKeeper 3.4.x to 3.5.x
Posted by GitBox <gi...@apache.org>.
jihoonson commented on pull request #10786:
URL: https://github.com/apache/druid/pull/10786#issuecomment-769907071
@xvrl great, thanks! I will take a look this weekend.
----------------------------------------------------------------
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
[GitHub] [druid] gianm commented on pull request #10786: move integration tests from ZooKeeper 3.4.x to 3.5.x
Posted by GitBox <gi...@apache.org>.
gianm commented on pull request #10786:
URL: https://github.com/apache/druid/pull/10786#issuecomment-764848396
I think it'd be good for now to test _both_ ZK 3.4 and 3.5 since we're meant to be in a migratory period where both servers are supported. Do you think you could add a smoke test that uses ZK 3.4? (I don't think we need to run every single test, but it'd be good to run some subset of the ITs.)
> @clintropolis @gianm what's the process to build and update the base image?
I think someone on the Imply team would need to push a new base image. That's probably the easiest way.
We could also move the base image to the "apache" Docker account? We started using a base image from Imply in #5060 for performance reasons, and IIRC that pre-dates when Druid joined ASF, so using the ASF account wouldn't have been an option at the time. Maybe it is now.
----------------------------------------------------------------
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
[GitHub] [druid] xvrl merged pull request #10786: move integration tests from ZooKeeper 3.4.x to 3.5.x
Posted by GitBox <gi...@apache.org>.
xvrl merged pull request #10786:
URL: https://github.com/apache/druid/pull/10786
----------------------------------------------------------------
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
[GitHub] [druid] clintropolis edited a comment on pull request #10786: move integration tests from ZooKeeper 3.4.x to 3.5.x
Posted by GitBox <gi...@apache.org>.
clintropolis edited a comment on pull request #10786:
URL: https://github.com/apache/druid/pull/10786#issuecomment-765174464
Hmm, since the integration tests are using docker-compose now, I wonder if we should just pull stock zookeeper (and kafka, metadata store) images and use those in the compose files instead...
----------------------------------------------------------------
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
[GitHub] [druid] jihoonson commented on a change in pull request #10786: move integration tests from ZooKeeper 3.4.x to 3.5.x
Posted by GitBox <gi...@apache.org>.
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
[GitHub] [druid] gianm commented on pull request #10786: move integration tests from ZooKeeper 3.4.x to 3.5.x
Posted by GitBox <gi...@apache.org>.
gianm commented on pull request #10786:
URL: https://github.com/apache/druid/pull/10786#issuecomment-764848396
I think it'd be good for now to test _both_ ZK 3.4 and 3.5 since we're meant to be in a migratory period where both servers are supported. Do you think you could add a smoke test that uses ZK 3.4? (I don't think we need to run every single test, but it'd be good to run some subset of the ITs.)
> @clintropolis @gianm what's the process to build and update the base image?
I think someone on the Imply team would need to push a new base image. That's probably the easiest way.
We could also move the base image to the "apache" Docker account? We started using a base image from Imply in #5060 for performance reasons, and IIRC that pre-dates when Druid joined ASF, so using the ASF account wouldn't have been an option at the time. Maybe it is now.
----------------------------------------------------------------
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
[GitHub] [druid] xvrl commented on a change in pull request #10786: move integration tests from ZooKeeper 3.4.x to 3.5.x
Posted by GitBox <gi...@apache.org>.
xvrl commented on a change in pull request #10786:
URL: https://github.com/apache/druid/pull/10786#discussion_r567348025
##########
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:
I updated the docker build to pull in the mysql connector version from the pom property
----------------------------------------------------------------
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
[GitHub] [druid] xvrl commented on a change in pull request #10786: move integration tests from ZooKeeper 3.4.x to 3.5.x
Posted by GitBox <gi...@apache.org>.
xvrl commented on a change in pull request #10786:
URL: https://github.com/apache/druid/pull/10786#discussion_r567348512
##########
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:
true, on the flipside we now ensure the integration tests always match the base image definition. Hopefully adding the ASF mirror will not be make things too flaky. My only concern would be the ZK 3.4.x binaries, which are no longer available from the main mirror and have to be downloaded from the ASF archive.
We can do a follow-up PR to save the intermediate docker cache to a file in the travis cache, so we only have to rebuild the base image if it changes.
----------------------------------------------------------------
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
[GitHub] [druid] xvrl commented on pull request #10786: move integration tests from ZooKeeper 3.4.x to 3.5.x
Posted by GitBox <gi...@apache.org>.
xvrl commented on pull request #10786:
URL: https://github.com/apache/druid/pull/10786#issuecomment-767159509
@gianm agree we should ideally test both. I'll see if it's easy to add separate 3.4.x integration test. All our "unit" tests that rely on curator's `TestingCluster` and `TestingServer` classes would continue to test against embedded 3.4.x nodes until we move to 3.5.x
----------------------------------------------------------------
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
[GitHub] [druid] clintropolis commented on pull request #10786: move integration tests from ZooKeeper 3.4.x to 3.5.x
Posted by GitBox <gi...@apache.org>.
clintropolis commented on pull request #10786:
URL: https://github.com/apache/druid/pull/10786#issuecomment-765174464
Hmm, since the integration tests are using docker-compose now, I wonder if we should just pull stock zookeeper (and kafka) images and use those in the compose files instead...
----------------------------------------------------------------
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
[GitHub] [druid] xvrl commented on pull request #10786: move integration tests from ZooKeeper 3.4.x to 3.5.x
Posted by GitBox <gi...@apache.org>.
xvrl commented on pull request #10786:
URL: https://github.com/apache/druid/pull/10786#issuecomment-769903832
@jihoonson I've updated the integration tests to remove the need for an external image build and added some ZK 3.4 integration tests.
----------------------------------------------------------------
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
[GitHub] [druid] jihoonson commented on pull request #10786: move integration tests from ZooKeeper 3.4.x to 3.5.x
Posted by GitBox <gi...@apache.org>.
jihoonson commented on pull request #10786:
URL: https://github.com/apache/druid/pull/10786#issuecomment-769417687
Hi @xvrl, I'm planning to finish the 0.21.0 release early next week. Do you think we can get this PR merged before that? Let me know if there is anything I can help.
----------------------------------------------------------------
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
[GitHub] [druid] xvrl commented on pull request #10786: move integration tests from ZooKeeper 3.4.x to 3.5.x
Posted by GitBox <gi...@apache.org>.
xvrl commented on pull request #10786:
URL: https://github.com/apache/druid/pull/10786#issuecomment-767159509
@gianm agree we should ideally test both. I'll see if it's easy to add separate 3.4.x integration test. All our "unit" tests that rely on curator's `TestingCluster` and `TestingServer` classes would continue to test against embedded 3.4.x nodes until we move to 3.5.x
----------------------------------------------------------------
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