-
Notifications
You must be signed in to change notification settings - Fork 15.1k
KAFKA-2846: Add Ducktape test for kafka-consumer-groups #555
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,105 @@ | ||
| # Licensed to the Apache Software Foundation (ASF) under one or more | ||
| # contributor license agreements. See the NOTICE file distributed with | ||
| # this work for additional information regarding copyright ownership. | ||
| # The ASF licenses this file to You under the Apache License, Version 2.0 | ||
| # (the "License"); you may not use this file except in compliance with | ||
| # the License. You may obtain a copy of the License at | ||
| # | ||
| # http://www.apache.org/licenses/LICENSE-2.0 | ||
| # | ||
| # Unless required by applicable law or agreed to in writing, software | ||
| # distributed under the License is distributed on an "AS IS" BASIS, | ||
| # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| # See the License for the specific language governing permissions and | ||
| # limitations under the License. | ||
|
|
||
|
|
||
| from ducktape.utils.util import wait_until | ||
| from ducktape.tests.test import Test | ||
| from ducktape.mark import matrix | ||
|
|
||
| from kafkatest.services.zookeeper import ZookeeperService | ||
| from kafkatest.services.kafka import KafkaService | ||
| from kafkatest.services.console_consumer import ConsoleConsumer | ||
| from kafkatest.services.security.security_config import SecurityConfig | ||
|
|
||
| import os | ||
|
|
||
| TOPIC = "topic-consumer-group-command" | ||
|
|
||
| class ConsumerGroupCommandTest(Test): | ||
| """ | ||
| Tests ConsumerGroupCommand | ||
| """ | ||
| # Root directory for persistent output | ||
| PERSISTENT_ROOT = "/mnt/consumer_group_command" | ||
| COMMAND_CONFIG_FILE = os.path.join(PERSISTENT_ROOT, "command.properties") | ||
|
|
||
| def __init__(self, test_context): | ||
| super(ConsumerGroupCommandTest, self).__init__(test_context) | ||
| self.num_zk = 1 | ||
| self.num_brokers = 1 | ||
| self.topics = { | ||
| TOPIC: {'partitions': 1, 'replication-factor': 1} | ||
| } | ||
| self.zk = ZookeeperService(test_context, self.num_zk) | ||
|
|
||
| def setUp(self): | ||
| self.zk.start() | ||
|
|
||
| def start_kafka(self, security_protocol, interbroker_security_protocol): | ||
| self.kafka = KafkaService( | ||
| self.test_context, self.num_brokers, | ||
| self.zk, security_protocol=security_protocol, | ||
| interbroker_security_protocol=interbroker_security_protocol, topics=self.topics) | ||
| self.kafka.start() | ||
|
|
||
| def start_consumer(self, security_protocol): | ||
| enable_new_consumer = security_protocol == SecurityConfig.SSL | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why is this the condition for enabling the new consumer? I know you require it for SSL, but why not always use new consumer? Or make the choice of old vs new consumer explicit in the different tests you're running?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. My reasoning is explained in my previous response. Pasting here for reference. Below are the combinations for ConsumerGroupCommand. I chose to test following combinations and I think it is reasonable. |
||
| self.consumer = ConsoleConsumer(self.test_context, num_nodes=self.num_brokers, kafka=self.kafka, topic=TOPIC, | ||
| consumer_timeout_ms=None, new_consumer=enable_new_consumer) | ||
| self.consumer.start() | ||
|
|
||
| def setup_and_verify(self, security_protocol, group=None): | ||
| self.start_kafka(security_protocol, security_protocol) | ||
| self.start_consumer(security_protocol) | ||
| consumer_node = self.consumer.nodes[0] | ||
| wait_until(lambda: self.consumer.alive(consumer_node), | ||
| timeout_sec=10, backoff_sec=.2, err_msg="Consumer was too slow to start") | ||
| kafka_node = self.kafka.nodes[0] | ||
| if security_protocol is not SecurityConfig.PLAINTEXT: | ||
| prop_file = str(self.kafka.security_config.client_config()) | ||
| self.logger.debug(prop_file) | ||
| kafka_node.account.ssh("mkdir -p %s" % self.PERSISTENT_ROOT, allow_fail=False) | ||
| kafka_node.account.create_file(self.COMMAND_CONFIG_FILE, prop_file) | ||
|
|
||
| # Verify ConsumerGroupCommand lists expected consumer groups | ||
| enable_new_consumer = security_protocol != SecurityConfig.PLAINTEXT | ||
| command_config_file = None | ||
| if enable_new_consumer: | ||
| command_config_file = self.COMMAND_CONFIG_FILE | ||
|
|
||
| if group: | ||
| wait_until(lambda: ("%s, topic-consumer-group-command, 0," % group) in self.kafka.describe_consumer_group(group=group, node=kafka_node, new_consumer=enable_new_consumer, command_config=command_config_file), timeout_sec=10, | ||
| err_msg="Timed out waiting to list expected consumer groups.") | ||
| else: | ||
| wait_until(lambda: "test-consumer-group" in self.kafka.list_consumer_groups(node=kafka_node, new_consumer=enable_new_consumer, command_config=command_config_file), timeout_sec=10, | ||
| err_msg="Timed out waiting to list expected consumer groups.") | ||
|
|
||
| self.consumer.stop() | ||
|
|
||
| @matrix(security_protocol=['PLAINTEXT', 'SSL']) | ||
| def test_list_consumer_groups(self, security_protocol='PLAINTEXT'): | ||
| """ | ||
| Tests if ConsumerGroupCommand is listing correct consumer groups | ||
| :return: None | ||
| """ | ||
| self.setup_and_verify(security_protocol) | ||
|
|
||
| @matrix(security_protocol=['PLAINTEXT', 'SSL']) | ||
| def test_describe_consumer_group(self, security_protocol='PLAINTEXT'): | ||
| """ | ||
| Tests if ConsumerGroupCommand is describing a consumer group correctly | ||
| :return: None | ||
| """ | ||
| self.setup_and_verify(security_protocol, group="test-consumer-group") | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like there's a lot of redundant code between
list_consumer_groupsanddescribe_consumer_groupsince almost all of it is for setting up thekafka-consumer-groups.shcommand. What about making that a common, fairly generic method and making each of these thin wrappers around it? For example, the generic common version might do all of this initial command setup, just accepting a list of extra argumets (i.e. empty in list_groups, just the group name in describe). Only thing I'm not sure about making generic is the way you're processing the output (which is the unfortunate result of not having something like JSON output for these commands).There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure I can do that.
True, Kafka-313 would have addressed that, but it never got traction.