KAFKA-5647: Use KafkaZkClient in ReassignPartitionsCommand and PreferredReplicaLeaderElectionCommand

[omkreddy]

• Use KafkaZkClient in ReassignPartitionsCommand
• Use KafkaZkClient in PreferredReplicaLeaderElectionCommand
• Updated test classes to use new methods
• All existing tests should pass

Committer Checklist (excluded from commit message)

• Verify design and implementation
• Verify test coverage and CI build status
• Verify documentation (including upgrade notes)

[omkreddy]

@junrao @ijuma

1. In previous PRs, I have updated TopicCommand and ConfigCommand. In this PR, I am updating ReassignPartitionsCommand, ReassignPartitionsCommand. Since these changes breaks existing users code, I am going to add a upgrade note. Do you see any other issues with these changes?

2. I have not updated ConsumerGroupCommand methods. These are deprecated old consumer related methods.

3. Should we replace all the usages of TopicAndPartition with TopicPartition class from ReassignPartitionsCommand.

[ijuma]

Thanks for the PR. Since we are in the process of converting some of these tools to AdminClient, maybe we can do that instead? We have a PR from @tombentley for PreferredReplicaLeaderElectionCommand and he was also working on one for ReassignPartitionsCommand .

[tedyu]

Do we need line 28 above ?

[omkreddy]

Yes, we need the import

[omkreddy]

@ijuma It makes sense to avoid multiple iterations of these classes. We can deprecate and later remove ZK based implementations. I will leave it to you and @junrao to decide the scope/need of this PR.

[omkreddy]

@junrao your thoughts on this?

[junrao]

@omkreddy : Thanks for the patch. The plan in KIP-183 is to deprecate the zookeeper option in PreferredReplicaCommand and not remove the zookeeper code. So, it's probably still useful to get this patch in so that we can remove the old ZkClient dependency sooner. A few minor comments below. Also, it's probably better to replace all the usages of TopicAndPartition with TopicPartition class from ReassignPartitionsCommand.

[junrao]

Could we add the return type?

[junrao]

Could we just use createRecursive to create the full path?

[junrao]

Could we just use createRecursive to create the full path?

[ijuma]

@junrao, what is your plan for removing ZkUtils and AdminUtils? Probably not before 2.0, right? So, it seems like it would be removed at the same time as the ZK-based parts of PreferredReplicaCommand. We can do this PR, but it doesn't seem like it helps us remove ZkUtils faster.

[junrao]

@ijuma : Yes, we probably can't remove ZkUtils and AdminUtils any time soon. I am just not sure when we can deprecate the zookeeper option in the tools. We probably don't want to keep using the old ZK utils there.

[omkreddy]

@junrao Thanks for the review. Updated the PR with review comments. Pls take a look.

Will update remaining test classes in another PR.

[omkreddy]

@junrao pinging for review

[omkreddy]

retest this please

[omkreddy]

@junrao pinging for review

[junrao]

@omkreddy : Thanks for the patch. Sorry for the delay. LGTM.