KAFKA-5112: Update compatibility system tests to include 0.10.2.1 #2701

ijuma · 2017-03-17T13:19:30Z

No description provided.

ijuma · 2017-03-17T13:19:54Z

Review by @hachikuji, @ewencp, @cmccabe.

ijuma · 2017-03-17T13:24:27Z

System tests run: https://jenkins.confluent.io/job/system-test-kafka-branch-builder-2/204/ (URL will work once the run starts, it's currently queued).

asfbot · 2017-03-17T14:04:57Z

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/kafka-pr-jdk7-scala2.10/2247/
Test PASSed (JDK 7 and Scala 2.10).

asfbot · 2017-03-17T14:10:05Z

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/kafka-pr-jdk8-scala2.12/2247/
Test PASSed (JDK 8 and Scala 2.12).

asfbot · 2017-03-17T14:36:45Z

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/kafka-pr-jdk8-scala2.11/2250/
Test PASSed (JDK 8 and Scala 2.11).

ewencp

@ijuma tests/kafkatest/sanity_checks/test_verifiable_producer.py and tests/kafkatest/tests/streams/streams_broker_compatibility_test.py seem like they might be missing from this patch.

Although the former might be a candidate to be deleted (along with other sanity checks) since they're effectively subsets of other tests anyway and so probably cost more test time than they are worth in debugging value.

ewencp · 2017-03-21T20:56:07Z

tests/docker/Dockerfile

@@ -27,5 +27,6 @@ RUN mkdir -p "/opt/kafka-0.8.2.2" && curl -s "${MIRROR}kafka/0.8.2.2/kafka_2.10-
 RUN mkdir -p "/opt/kafka-0.9.0.1" && curl -s "${MIRROR}kafka/0.9.0.1/kafka_2.10-0.9.0.1.tgz" | tar xz --strip-components=1 -C "/opt/kafka-0.9.0.1"
 RUN mkdir -p "/opt/kafka-0.10.0.1" && curl -s "${MIRROR}kafka/0.10.0.1/kafka_2.10-0.10.0.1.tgz" | tar xz --strip-components=1 -C "/opt/kafka-0.10.0.1"
 RUN mkdir -p "/opt/kafka-0.10.1.1" && curl -s "${MIRROR}kafka/0.10.1.1/kafka_2.10-0.10.1.1.tgz" | tar xz --strip-components=1 -C "/opt/kafka-0.10.1.1"
+RUN mkdir -p "/opt/kafka-0.10.2.0" && curl -s "${MIRROR}kafka/0.10.2.0/kafka_2.10-0.10.2.0.tgz" | tar xz --strip-components=1 -C "/opt/kafka-0.10.2.0"


I am wondering based on how many places require modification if we should centralize the version info into a single location, e.g. a json file that everything else can rely on and code like this could be turned into a loop. Seems like these version additions would require touching a lot less stuff that way.

That sounds good. OK if I file a JIRA for it?

ewencp · 2017-03-21T20:57:55Z

tests/kafkatest/tests/core/compatibility_test_new_broker_test.py

@@ -46,12 +45,12 @@ def setUp(self):

    @cluster(num_nodes=6)
    @parametrize(producer_version=str(LATEST_0_8_2), consumer_version=str(LATEST_0_8_2), compression_types=["none"], new_consumer=False, timestamp_type=None)
-    @parametrize(producer_version=str(LATEST_0_8_2), consumer_version=str(LATEST_0_9), compression_types=["none"], new_consumer=False, timestamp_type=None)


I'm a fan of removing unneeded parameterizations, but is this ok to remove just because we don't think there's anything between 0.8 and 0.9 clients that would be affected by running against an 0.10 broker?

It seems a bit odd that we have so many combinations for 0.8.2 and 0.9 while we have very few for each of the 0.10.x versions. Note that we a few cases where the consumer version is LATEST_0_9.

Yeah, I guess I just don't know what the strategy is for this test in terms of how we are choosing the combinations of producer/consumer version. Which makes it hard to review a patch adding/removing parameterizations :)

I can't discern a pattern apart from "have some coverage for all the versions" at the moment. Starting from a clean slate, if we want to be completely distrustful of all parts of the system, we'd want to test every possible producer/consumer combination. But we test same version producer/consumer pairs in many other tests, so we could drop those perhaps. So, this test would be about testing producer/consumers with different versions. The numbers will grow pretty quickly if we don't drop support for older versions though.

What do you think?

I think the exhaustive approach is good when we can do things like label tests for infrequent/must-pass-for-final-release, but kinda wasteful nightly.

I just lost track of how we decided what to add/remove/modify from these. If we're convinced we have sufficient coverage, then I'm fine with the modifications.

ijuma · 2017-04-06T01:45:06Z

I'll update this PR to also take into account that we now have 3 message format versions (upgrade_test.py and message_format_change_test.py need to be updated).

asfbot · 2017-05-31T19:57:16Z

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/kafka-pr-jdk7-scala2.11/4664/
Test PASSed (JDK 7 and Scala 2.11).

asfbot · 2017-05-31T20:00:42Z

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/kafka-pr-jdk8-scala2.12/4648/
Test PASSed (JDK 8 and Scala 2.12).

asfbot · 2017-06-03T03:23:51Z

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/kafka-pr-jdk7-scala2.11/4848/
Test PASSed (JDK 7 and Scala 2.11).

asfbot · 2017-06-03T03:50:50Z

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/kafka-pr-jdk8-scala2.12/4832/
Test PASSed (JDK 8 and Scala 2.12).

hachikuji · 2017-06-03T06:01:50Z

tests/kafkatest/tests/core/upgrade_test.py

@@ -62,11 +62,16 @@ def perform_upgrade(self, from_kafka_version, to_message_format_version=None):
            self.kafka.start_node(node)

    @cluster(num_nodes=6)
+    @parametrize(from_kafka_version=str(LATEST_0_10_2), to_message_format_version=str(LATEST_0_9), compression_types=["none"])


Discussed offline. This one looks a little funny. We shouldn't be able to downgrade the message format.

The test actually passes. I guess it makes sense. If the clients understand the new format, then it works regardless of whether the broker/topic config has an older version.

hachikuji

Left one comment, but the rest LGTM (provided tests are passing). One improvement which we could consider separately is testing upgrades from a newer kafka version running an older message format.

asfbot · 2017-06-05T16:03:27Z

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/kafka-pr-jdk7-scala2.11/4922/
Test PASSed (JDK 7 and Scala 2.11).

asfbot · 2017-06-05T16:34:31Z

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/kafka-pr-jdk8-scala2.12/4906/
Test PASSed (JDK 8 and Scala 2.12).

hachikuji · 2017-06-06T02:03:00Z

Thanks for the updates. LGTM

ijuma · 2017-06-06T02:23:13Z

Since the branch builder is out of action, I ran the modified system tests via AWS and they passed. Merging to trunk and 0.11.0.

Also update message format tests now that we have a third message format. Finally, set group.initial.rebalance.delay.ms=100. Author: Ismael Juma <ismael@juma.me.uk> Reviewers: Ewen Cheslack-Postava <ewen@confluent.io>, Jason Gustafson <jason@confluent.io> Closes #2701 from ijuma/update-upgrade-tests-for-0.11 (cherry picked from commit b119d04) Signed-off-by: Ismael Juma <ismael@juma.me.uk>

ewencp reviewed Mar 21, 2017

View reviewed changes

ijuma mentioned this pull request Apr 23, 2017

KAFKA-5112: Trunk compatibility tests should test against 0.10.2 #2893

Closed

ijuma force-pushed the update-upgrade-tests-for-0.11 branch from 36ab9fa to f9388bc Compare May 31, 2017 18:30

ijuma changed the title ~~MINOR: Update compatibility system tests to include 0.10.2.0~~ MINOR: Update compatibility system tests to include 0.10.2.1 May 31, 2017

ijuma changed the title ~~MINOR: Update compatibility system tests to include 0.10.2.1~~ KAFKA-5112: Update compatibility system tests to include 0.10.2.1 May 31, 2017

ijuma force-pushed the update-upgrade-tests-for-0.11 branch from 72b14b0 to 9e38fed Compare June 3, 2017 02:18

hachikuji reviewed Jun 3, 2017

View reviewed changes

hachikuji approved these changes Jun 3, 2017

View reviewed changes

ijuma added 3 commits June 5, 2017 12:36

MINOR: Update compatibility system tests to include 0.10.2.1

c016218

Add more message format version parameterizations

25f748d

A few fixes and improvements to message_format_change_test.py

43f5466

ijuma force-pushed the update-upgrade-tests-for-0.11 branch from 9e38fed to 43f5466 Compare June 5, 2017 15:04

Set group.initial.rebalance.delay.ms to 100 ms

6248642

asfgit closed this in b119d04 Jun 6, 2017

ijuma deleted the update-upgrade-tests-for-0.11 branch June 18, 2017 09:26

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

KAFKA-5112: Update compatibility system tests to include 0.10.2.1 #2701

KAFKA-5112: Update compatibility system tests to include 0.10.2.1 #2701

ijuma commented Mar 17, 2017

ijuma commented Mar 17, 2017

ijuma commented Mar 17, 2017 •

edited

Loading

asfbot commented Mar 17, 2017

asfbot commented Mar 17, 2017

asfbot commented Mar 17, 2017

ewencp left a comment

ewencp Mar 21, 2017

ijuma Mar 22, 2017

ewencp Mar 22, 2017

ewencp Mar 21, 2017

ijuma Mar 22, 2017 •

edited

Loading

ewencp Mar 22, 2017

ijuma Mar 23, 2017

ewencp Mar 28, 2017

ijuma commented Apr 6, 2017

asfbot commented May 31, 2017

asfbot commented May 31, 2017

asfbot commented Jun 3, 2017

asfbot commented Jun 3, 2017

hachikuji Jun 3, 2017

ijuma Jun 6, 2017

hachikuji left a comment

asfbot commented Jun 5, 2017

asfbot commented Jun 5, 2017

hachikuji commented Jun 6, 2017

ijuma commented Jun 6, 2017

KAFKA-5112: Update compatibility system tests to include 0.10.2.1 #2701

KAFKA-5112: Update compatibility system tests to include 0.10.2.1 #2701

Conversation

ijuma commented Mar 17, 2017

ijuma commented Mar 17, 2017

ijuma commented Mar 17, 2017 • edited Loading

asfbot commented Mar 17, 2017

asfbot commented Mar 17, 2017

asfbot commented Mar 17, 2017

ewencp left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ijuma Mar 22, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ijuma commented Apr 6, 2017

asfbot commented May 31, 2017

asfbot commented May 31, 2017

asfbot commented Jun 3, 2017

asfbot commented Jun 3, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hachikuji left a comment

Choose a reason for hiding this comment

asfbot commented Jun 5, 2017

asfbot commented Jun 5, 2017

hachikuji commented Jun 6, 2017

ijuma commented Jun 6, 2017

ijuma commented Mar 17, 2017 •

edited

Loading

ijuma Mar 22, 2017 •

edited

Loading