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

Use docker-compose to run the functional tests #1695

Merged
merged 7 commits into from
Jun 24, 2020

Conversation

KJTsanaktsidis
Copy link

@KJTsanaktsidis KJTsanaktsidis commented May 10, 2020

CC @bai @dnwe - I took a stab at changing how the functional tests run as we discussed in #1661. Let me know what you think!

This PR makes a few changes:

  • Puts the functional tests behind a separate build tag. Running go test runs the unit tests, and running go test -tags=integration will run all tests, including the functional tests
  • Replaces the vagrant test setup mechanism with a docker-compose file.
  • The functional tests will automatically start/stop the docker-compose test environment if TOXIPROXY_ADDR isn't set
  • Changed the github actions config to use the docker-compose set up as well.

Open questions:

  • This is using the confluent platform docker images (confluentinc/cp-{kafka,zookeeper}), which have a different versioning scheme to the upstream apache kafka project. This leads to some pretty ugly mapping code to deduce the kafka version from the confluent platform version, and vice versa. Unfortunately this seems to be the "msot official" kafka image I could find. Open to other suggestions on what we should use though.
  • This changed the API for test cases to interact with the test environment a bit (access to the toxiproxies, kafka address, etc is done via the FunctionalTestEnv now).
  • I did experiment with just using the Docker Engine API directly for spinning up the containers instead of using docker-compose. It lead to a lot of extra code though so I think compose was ultimately a better solution.
  • Maybe it doesn't matter, but we could run docker-compose up in the "install dependancies" part of the github action workflow, instead of relying on it to be automatically invoked as part of "run test suite"

KJ Tsanaktsidis added 4 commits May 10, 2020 16:14
* Functional tests are skipped except when the `functional` build tag
  is passed to the go test command (i.e. go test -tags=functional)
* If TOXIPROXY_ADDR is not set when the functional tests are being run,
  it will use docker-compose to automatically spin up a
  zookeeper/kafka/toxiproxy environment suitab le for running the tests.
  This requies a working Docker and for the docker-compose command line
  tool to be installed.
* If TOXIPROXY_ADDR and KAFKA_VERSION are set, then the tests will not
  spin up any docker infrastructure and will instead rely on whatever
  kafka broker is behind the specified toxiproxy.
@KJTsanaktsidis KJTsanaktsidis requested a review from bai as a code owner May 10, 2020 06:41
@dnwe
Copy link
Collaborator

dnwe commented May 10, 2020

@KJTsanaktsidis this looks good

RE: the docker images and version mapping, I wonder if we should just use a Java base image for the containers and volume mount in the extracted Kafka tgz and the server.properties we were generating? I remember we had some custom (short) timeouts etc. to make the toxiproxy tests complete more quickly etc. so it might be easier to manage our own Kafka within the container

@KJTsanaktsidis
Copy link
Author

I remember we had some custom (short) timeouts etc. to make the toxiproxy tests complete more quickly

Actually I replicated those configs in the compose file - the confluent kafka image transforms environment variables of the form KAFKA_* to server properties.

use a Java base image for the containers and volume mount in the extracted Kafka tgz

This could work but makes it much more difficult to run the test environment by hand (to persist it across runs when iterating on them for example). We could however build our own Kafka images from a Dockerfile - I think docker-compose might even be smart enough to build them once the first time someone writes a functional test and use the same image for subsequent runs.

@KJTsanaktsidis KJTsanaktsidis force-pushed the ktsanaktsidis/docker_compose_tests branch from 509158a to a9e0b4f Compare May 10, 2020 23:01
.gitignore Show resolved Hide resolved
@KJTsanaktsidis
Copy link
Author

@dnwe @d1egoaz @bai just wondering if there was still an appetite to have the functional tests be run like this?

Copy link
Contributor

@bai bai left a comment

Choose a reason for hiding this comment

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

I'll let @d1egoaz re-review this one but LGTM.

@KJTsanaktsidis
Copy link
Author

sorry, I just looked at this a second time since I was on the page and noticed there was a spot where I'd hardcoded kafka 2.5.0 for debugging purposes 🤦 Fixed and pushed.

@bai bai merged commit ad70aac into IBM:master Jun 24, 2020
dnwe added a commit that referenced this pull request May 7, 2021
One of the pieces of #1695 was to use a small jar for publishing
uncommited messages as part of the functional test. Replacing that with
the native Sarama transactional producer

Fixes #1733

Co-authored-by: KJTsanaktsidis <[email protected]>
dnwe added a commit that referenced this pull request Sep 13, 2021
One of the pieces of #1695 was to use a small jar for publishing
uncommited messages as part of the functional test. Replacing that with
the native Sarama transactional producer

Fixes #1733

Co-authored-by: KJTsanaktsidis <[email protected]>
dnwe added a commit that referenced this pull request Feb 8, 2022
One of the pieces of #1695 was to use a small jar for publishing
uncommited messages as part of the functional test. Replacing that with
the native Sarama transactional producer

Fixes #1733

Co-authored-by: KJTsanaktsidis <[email protected]>
dnwe added a commit that referenced this pull request Feb 8, 2022
One of the pieces of #1695 was to use a small jar for publishing
uncommited messages as part of the functional test. Replacing that with
the native Sarama transactional producer

Fixes #1733

Co-authored-by: KJTsanaktsidis <[email protected]>
dnwe added a commit that referenced this pull request Feb 8, 2022
One of the pieces of #1695 was to use a small jar for publishing
uncommited messages as part of the functional test. Replacing that with
the native Sarama transactional producer

Fixes #1733

Co-authored-by: KJTsanaktsidis <[email protected]>
dnwe added a commit that referenced this pull request Feb 8, 2022
One of the pieces of #1695 was to use a small jar for publishing
uncommited messages as part of the functional test. Replacing that with
the native Sarama transactional producer

Fixes #1733

Co-authored-by: KJTsanaktsidis <[email protected]>
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.

4 participants