Skip to content

MINOR: add support for kafka 2.4 and 2.5 to downgrade test #8518

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.

Sign up for GitHub

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

Closed
lbradstreet wants to merge 2 commits into from
Closed

MINOR: add support for kafka 2.4 and 2.5 to downgrade test #8518

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d48bbb3
MINOR: add support for kafka 2.4 and 2.5 to downgrade test
lbradstreet Apr 19, 2020
353b922
Avoid static membership where broken
lbradstreet Apr 20, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 9 additions & 2 deletions tests/kafkatest/tests/core/downgrade_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
from kafkatest.services.zookeeper import ZookeeperService
from kafkatest.tests.end_to_end import EndToEndTest
from kafkatest.utils import is_int
from kafkatest.version import LATEST_0_9, LATEST_0_10, LATEST_0_10_0, LATEST_0_10_1, LATEST_0_10_2, LATEST_0_11_0, LATEST_1_0, LATEST_1_1, LATEST_2_0, LATEST_2_1, LATEST_2_2, LATEST_2_3, V_0_9_0_0, V_0_11_0_0, DEV_BRANCH, KafkaVersion
from kafkatest.version import LATEST_0_9, LATEST_0_10, LATEST_0_10_0, LATEST_0_10_1, LATEST_0_10_2, LATEST_0_11_0, LATEST_1_0, LATEST_1_1, LATEST_2_0, LATEST_2_1, LATEST_2_2, LATEST_2_3, LATEST_2_4, LATEST_2_5, V_0_9_0_0, V_0_11_0_0, DEV_BRANCH, KafkaVersion

class TestDowngrade(EndToEndTest):

Expand Down Expand Up @@ -67,11 +67,18 @@ def setup_services(self, kafka_version, compression_types, security_protocol):
version=kafka_version)
self.producer.start()

static_membership = kafka_version == DEV_BRANCH or kafka_version >= LATEST_2_3

@abbccdda abbccdda Apr 20, 2020

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Why does static membership make a difference here?

@lbradstreet lbradstreet Apr 20, 2020 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good question, without it the verifiable consumer fails with:

usage: verifiable-consumer [-h]                                                                                                                                                                                                                              
                           --broker-list HOST1:PORT1[,HOST2:PORT2[...]]                                                                                                                                                                                      
                           --topic TOPIC --group-id GROUP_ID                                                                                                                                                                                                 
                           --group-instance-id GROUP_INSTANCE_ID                                                                                                                                                                                             
                           [--max-messages MAX-MESSAGES]                                                                                                                                                                                                     
                           [--session-timeout TIMEOUT_MS] [--verbose]                                                                                                                                                                                        
                           [--enable-autocommit]                                                                                                                                                                                                             
                           [--reset-policy RESETPOLICY]                                                                                                                                                                                                      
                           [--assignment-strategy ASSIGNMENTSTRATEGY]                                                                                                                                                                                        
                           [--consumer.config CONFIG_FILE]                                                                                                                                                                                                   
verifiable-consumer: error: argument --group-instance-id is required

I assumed that this check was added for a reason, however maybe it's being to strict or buggy?

@lbradstreet lbradstreet Apr 20, 2020

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I believe it's due to 2.4 accidentally requiring --group-instance-id and in this test we end up using this version. See 0f995ba

@lbradstreet lbradstreet Apr 20, 2020

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I could force static throttling to be used when the version is 2.4 or 2.3 and leave a comment if that works for you.

@lbradstreet lbradstreet Apr 20, 2020

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@abbccdda I changed the code to add a static_membership parameter to the test matrix. We simply avoid this for 2.4 where this is broken due to the command line validation in that release.

@lbradstreet lbradstreet Apr 24, 2020

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@abbccdda friendly ping on this

@abbccdda abbccdda Apr 28, 2020

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I see, makes sense.

self.create_consumer(log_level="DEBUG",
version=kafka_version)
version=kafka_version,
static_membership=static_membership)

self.consumer.start()

@cluster(num_nodes=7)
@parametrize(version=str(LATEST_2_5), compression_types=["none"])
@parametrize(version=str(LATEST_2_5), compression_types=["zstd"], security_protocol="SASL_SSL")
@parametrize(version=str(LATEST_2_4), compression_types=["none"])
@parametrize(version=str(LATEST_2_4), compression_types=["zstd"], security_protocol="SASL_SSL")
@parametrize(version=str(LATEST_2_3), compression_types=["none"])
@parametrize(version=str(LATEST_2_3), compression_types=["zstd"], security_protocol="SASL_SSL")
@parametrize(version=str(LATEST_2_2), compression_types=["none"])
Expand Down