Skip to content

move integration tests from ZooKeeper 3.4.x to 3.5.x#10786

Merged
xvrl merged 11 commits intoapache:masterfrom
xvrl:zk-35-tests
Jan 31, 2021
Merged

move integration tests from ZooKeeper 3.4.x to 3.5.x#10786
xvrl merged 11 commits intoapache:masterfrom
xvrl:zk-35-tests

Conversation

@xvrl
Copy link
Copy Markdown
Member

@xvrl xvrl commented Jan 21, 2021

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

  • Removes the need to build separate docker-base image:
    • use multi-stage build for the base image
    • use openjdk base image instead of building our own JDK base
    • workaround Debian not including MySQL by using MariaDB and
      downloading mysql connector directly
  • Run integration tests with ZK 3.5 by default
  • Run a subset of our integration tests with ZK 3.4 for backwards compatibility testing.

@gianm
Copy link
Copy Markdown
Contributor

gianm commented Jan 21, 2021

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.

@clintropolis
Copy link
Copy Markdown
Member

clintropolis commented Jan 22, 2021

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...

@xvrl
Copy link
Copy Markdown
Member Author

xvrl commented Jan 25, 2021

@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

@jihoonson
Copy link
Copy Markdown
Contributor

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.

xvrl added 2 commits January 28, 2021 23:16
- use multi-stage build for the base image
- use openjdk base image instead of building our own JDK base
- workaround Debian not including MySQL by using MariaDB
- download mysql connector directly instead of using distro version
@xvrl xvrl force-pushed the zk-35-tests branch 2 times, most recently from 2af7c93 to 3be0716 Compare January 29, 2021 07:58
@xvrl
Copy link
Copy Markdown
Member Author

xvrl commented Jan 29, 2021

@jihoonson I've updated the integration tests to remove the need for an external image build and added some ZK 3.4 integration tests.

@jihoonson
Copy link
Copy Markdown
Contributor

@xvrl great, thanks! I will take a look this weekend.

Copy link
Copy Markdown
Contributor

@jihoonson jihoonson left a comment

Choose a reason for hiding this comment

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

@xvrl, can you add another test against ZooKeeper 3.4 that tests ingestion and segment loading? The "Kafka index integration test with various formats" test looks good enough to test both.

Comment thread integration-tests/docker/Dockerfile Outdated
RUN /root/base-setup.sh && rm -f /root/base-setup.sh

FROM druidbase
ARG MYSQL_CONNECTOR_VERSION=5.1.49
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.

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.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I updated the docker build to pull in the mysql connector version from the pom property

# 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
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.

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.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

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.

Copy link
Copy Markdown
Contributor

@jihoonson jihoonson left a comment

Choose a reason for hiding this comment

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

LGTM. +1 after CI.

@xvrl xvrl merged commit c346ce6 into apache:master Jan 31, 2021
@xvrl xvrl deleted the zk-35-tests branch January 31, 2021 16:35
jihoonson pushed a commit to jihoonson/druid that referenced this pull request Feb 2, 2021
* move integration tests from ZooKeeper 3.4.x to 3.5.x
* run a subset of our integration tests with ZK 3.4 for backwards compatibility testing.
* remove need to build separate docker-base image
- use multi-stage build for the base image
- use openjdk base image instead of building our own JDK base
- workaround Debian not including MySQL by using MariaDB
- download mysql connector directly instead of using distro version
* fix incorrect openssl command failing on Debian
* keep mysql connector version in sync with pom version
jihoonson added a commit that referenced this pull request Feb 3, 2021
* move integration tests from ZooKeeper 3.4.x to 3.5.x
* run a subset of our integration tests with ZK 3.4 for backwards compatibility testing.
* remove need to build separate docker-base image
- use multi-stage build for the base image
- use openjdk base image instead of building our own JDK base
- workaround Debian not including MySQL by using MariaDB
- download mysql connector directly instead of using distro version
* fix incorrect openssl command failing on Debian
* keep mysql connector version in sync with pom version

Co-authored-by: Xavier Léauté <xvrl@apache.org>
jon-wei added a commit to jon-wei/druid that referenced this pull request Feb 11, 2021
* move integration tests from ZooKeeper 3.4.x to 3.5.x (apache#10786)

* move integration tests from ZooKeeper 3.4.x to 3.5.x
* run a subset of our integration tests with ZK 3.4 for backwards compatibility testing.
* remove need to build separate docker-base image
- use multi-stage build for the base image
- use openjdk base image instead of building our own JDK base
- workaround Debian not including MySQL by using MariaDB
- download mysql connector directly instead of using distro version
* fix incorrect openssl command failing on Debian
* keep mysql connector version in sync with pom version

* K8s IT Test enhance  (apache#10785)

* do build and stop action in IT

* change base dir from druidHome to druidHome/integration-tests

* add env DRUID_HOME

* bug fix

* modify stop_sh

* ready to test

* bug fix

* modify dir

* tested on dev

* modify dir

* move DRUID_HOME env

* done

Co-authored-by: yuezhang <yuezhang@freewheel.tv>

* Update NOTICE copyright year (apache#10834)

the future is now

* Cleanup openssl fixes to keep certs

* Address CVE-2020-8570, suppress CVE-2020-8554 (apache#10826)

* Address CVE-2020-8570, suppress CVE-2020-8554

* Update licenses.yaml

* wget debug

* Suppress CVE-2020-9492 for hadoop-mapreduce-client-core (apache#10847)

* Revert "wget debug"

This reverts commit 5b81c33b4728420e2312b3c919b7de9c1b4da589.

* Add MYSQL_VERSION env variable in integration-tests-imply tests

* Increase heap to 64m for custom node (apache#10846)

* Fix CVE-2021-25646 (apache#10818)

* Add ZK_VERSION env variable

Co-authored-by: Xavier Léauté <xvrl@apache.org>
Co-authored-by: zhangyue19921010 <69956021+zhangyue19921010@users.noreply.github.com>
Co-authored-by: yuezhang <yuezhang@freewheel.tv>
Co-authored-by: Clint Wylie <cwylie@apache.org>
Co-authored-by: Slava Mogilevsky <triggerwoods91@gmail.com>
Co-authored-by: Jihoon Son <jihoonson@apache.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants