KAFKA-16206 Fix unnecessary topic config deletion during ZK migration

[ahuang98]

Committer Checklist (excluded from commit message)

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

[rondagostino]

@ahuang98 I still saw lots of test failures, and when I checked them locally I actually was able to reproduce the below failures in KRaftMigrationDriverTest. Can you take a look to see if you also agree these fail locally and then, assuming you agree, investigate as to why?

testTopicDualWriteDelta()
testTopicDualWriteSnapshot()
testControllerFailover()

What I don't understand is why these would start to fail locally on this branch when they don't fail on trunk. We haven't touched anything except test files here -- and not KRaftMigrationDriverTest.

Ah, I see why. Those 3 tests use org.apache.kafka.image.TopicsImageTest.DELTA1_RECORDS, which we have changed. So KRaftMigrationDriverTest will need some adjusting.

[ahuang98]

Latest test run has 14 "new failing" which are all timeout related/unrelated - https://ci-builds.apache.org/blue/organizations/jenkins/Kafka%2Fkafka-pr/detail/PR-14206/17/tests

[ahuang98]

Is there a downside to having deleteTopic in ZkTopicMigrationClient not delete configs? Otherwise changing the logging level seems okay to me. [Edit] David explained offline that even if there are no configs, we still need to clean up the topic’s config znode.

[ahuang98]

@mimaison tagging you again just in case you have time to take a look @mumrah I've added the logging change in the meantime - are we okay merging in the next week if we don't get a response?

[ahuang98]

@mimaison the PR was originally meant to introduce additional test changes, I believe @mumrah renamed it after I added the migration fix. I'll move the tests out and apply your suggestions, thanks!

[ahuang98]

@mimaison @mumrah I moved unrelated test changes over to https://github.com/apache/kafka/pull/15373. The latest commit 1c1eb7d addresses the comments left here (mostly imports)

[mimaison]

There's quite a few failures related to ZkMigration in the last CI run: https://ci-builds.apache.org/job/Kafka/job/kafka-pr/job/PR-14206/22/testReport/

[ahuang98]

Thanks @mimaison, looks like this is because I split out the TopicsImageTest changes into a separate PR. The migration tests depend on the DELTA1_RECORDS defined there so I moved over changes from TopicsImageTest as well.

[ahuang98]

@mimaison could you help merge this if this looks good to you? :)