KAFKA-8509; Add downgrade system test #7724
Conversation
|
I'm changing this test case to use EndToEndTest instead of ProduceConsumeValidateTest. The advantage is that this allows me to assert committed offsets in the consumer, which makes the test case a stronger assertion about compatibility. I will push this change tomorrow. |
| @parametrize(kafka_version=str(LATEST_0_8_2), compression_types=["none"]) | ||
| @parametrize(kafka_version=str(LATEST_0_8_2), compression_types=["snappy"]) | ||
| def test_upgrade_and_downgrade(self, kafka_version, compression_types, security_protocol="PLAINTEXT"): | ||
| @parametrize(version=str(LATEST_2_3), compression_types=["none"]) |
There was a problem hiding this comment.
Could some of these be replaced with @matrix?
There was a problem hiding this comment.
Possibly. Let me consider it.
| @parametrize(version=str(LATEST_0_10_1), compression_types=["gzip"]) | ||
| @parametrize(version=str(LATEST_0_10_0), compression_types=["none"]) | ||
| @parametrize(version=str(LATEST_0_10_0), compression_types=["lz4"]) | ||
| @parametrize(version=str(LATEST_0_9), compression_types=["none"], security_protocol="SASL_SSL") |
There was a problem hiding this comment.
Why not PLAINTEXT here as well? Was there something specific to SASL_SSL for this version?
There was a problem hiding this comment.
Inherited from the upgrade test. It does seem useful to have some tests which validate the auth flows. For each version, I was considering doing one test with PLAINTEXT/no compression and one with SASL_SSL/some compression. How does that sound?
There was a problem hiding this comment.
If it doesn't add too much to the runtime, I think it would be good to include some more cases like you suggest.
There was a problem hiding this comment.
Do we want to support all these versions? I'd vote for only testing 1.0 and newer. 1.0 was released roughly 2 years ago.
There was a problem hiding this comment.
Given that 1.0 was released 2 years ago, I'd even go with 1.1 as the minimum version.
There was a problem hiding this comment.
Sounds good to me.
| @@ -103,13 +103,14 @@ def stop_producer_and_consumer(self): | |||
| self.producer.stop() | |||
| self.consumer.wait() | |||
|
|
|||
| def run_produce_consume_validate(self, core_test_action=None, *args): | |||
| """Top-level template for simple produce/consume/validate tests.""" | |||
| def run_multistep_produce_consume_validate(self, core_test_actions, *args): | |||
There was a problem hiding this comment.
Are these changes still needed now that the downgrade test is extending EndToEndTest?
There was a problem hiding this comment.
Thanks, I'll revert these changes.
| (self.reset_policy, self.group_id, self.topic, node.group_instance_id, self.kafka.bootstrap_servers(self.security_config.security_protocol), | ||
|
|
||
| if node.version >= V_2_3_0 and node.group_instance_id: | ||
| cmd += "--group-instance-id %s" % node.group_instance_id |
There was a problem hiding this comment.
nit: may need to add space before --group-instance-id as previous if condition adds verbse w/o a trailing space.
hachikuji
force-pushed
the
KAFKA-8509
branch
from
94134ec to
5898d09
Compare
| def test_upgrade_and_downgrade(self, version, compression_types, security_protocol="PLAINTEXT"): | ||
| """Test upgrade and downgrade of Kafka cluster from old versions to the current version | ||
|
|
||
| `version` is the Kafka version to upgrade from and dowrade back to |
The --enable-autocommit argument is a flag. It does not take a parameter. This was broken in #7724. Reviewers: Ismael Juma <[email protected]>, Manikumar Reddy <[email protected]>
This patch adds a basic downgrade system test. It verifies that producing and consuming continues to work before and after the downgrade.
Committer Checklist (excluded from commit message)