yashmayya opened a new pull request, #14371: URL: https://github.com/apache/kafka/pull/14371
From https://issues.apache.org/jira/browse/KAFKA-14855: > In the Connect embedded integration testing framework, the [EmbeddedConnectClusterAssertions::assertConnectorAndTasksAreStopped method](https://github.com/apache/kafka/blob/31440b00f3ed8de65f368d41d6cf2efb07ca4a5c/connect/runtime/src/test/java/org/apache/kafka/connect/util/clusters/EmbeddedConnectClusterAssertions.java#L411-L428) is used in several places to verify that a connector has been deleted. (This method may be renamed in an upcoming PR to something like assertConnectorAndTasksAreNotRunning, but apart from that, its usage and semantics will remain unchanged.) However, the [underlying logic for that assertion](https://github.com/apache/kafka/blob/31440b00f3ed8de65f368d41d6cf2efb07ca4a5c/connect/runtime/src/test/java/org/apache/kafka/connect/util/clusters/EmbeddedConnectClusterAssertions.java#L430-L451) doesn't strictly check for deletion (which can be done by verifying that the connector and its tasks no longer appear in the REST API at all), since it also allows for the Connector or tasks to appear in the REST API, but with a state that is not RUNNING. > > This constraint is a bit too lax and may be silently masking issues with our shutdown logic for to-be-deleted connectors. We should try to narrow the criteria for that method so that it fails if the Connector or any of its tasks still appear in the REST API, even with a non-RUNNING state. > > However, we should also be careful to ensure that current uses of that method are not relying on its semantics. If, for some reason, a test case requires the existing semantics, we should evaluate whether it's necessary to continue to rely on those semantics, and if so, probably preserve the existing method so that it can be used wherever applicable (but rewrite all other tests to use the new, stricter method). - All usages of the `assertConnectorAndTasksAreNotRunning` method were to check for connector deletion so the assertion on non-running status of connectors and tasks isn't required. - This patch removes that unnecessary extra assertion and hardens the logic for asserting that a connector is deleted. - Note that `assertConnectorAndTasksAreNotRunning` (now renamed to `assertConnectorDoesNotExist`) only verifies that the connector has been wiped from the status backing store which is done by the leader worker after the post-deletion group rebalance. We don't verify whether the connectors and tasks have actually finished stopping - this is potentially a further improvement that could be made in the future. ### Committer Checklist (excluded from commit message) - [ ] Verify design and implementation - [ ] Verify test coverage and CI build status - [ ] Verify documentation (including upgrade notes) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
