search

strimzi / strimzi-kafka-operator

Apache Kafka® running on Kubernetes
https://strimzi.io/
Apache License 2.0
4.86k stars 1.3k forks source link

[system test] make Recovery tests run only on KRaft mode only #10637

Closed see-quick closed 3 days ago

see-quick commented 1 month ago

Type of change

Description

This PR changes our recovery tests so they can run with KRaft mode.

Checklist

scholzj commented 1 month ago

Why would you want to run them in ZooKeeper mode only?

see-quick commented 1 month ago

Why would you want to run them in ZooKeeper mode only?

Well, some of the test cases do not work with KRaft here (e.g., testTopicNotAvailable, testTopicAvailable and more...). I don't have any strong opinion on this but for now, I think having these tests run only at least ZK mode is okay I think. Maybe @henryZrncik or @im-konge knows more about this.

scholzj commented 1 month ago

Why would you want to run them in ZooKeeper mode only?

Well, some of the test cases do not work with KRaft here (e.g., testTopicNotAvailable, testTopicAvailable and more...). I don't have any strong opinion on this but for now, I think having these tests run only at least ZK mode is okay I think. Maybe @henryZrncik or @im-konge knows more about this.

So, should it be fixed? Extended? Deleted? ZooKeeper will be gone soon and any ZooKeeper only tests will be deleted with it.

see-quick commented 1 month ago

Why would you want to run them in ZooKeeper mode only?

Well, some of the test cases do not work with KRaft here (e.g., testTopicNotAvailable, testTopicAvailable and more...). I don't have any strong opinion on this but for now, I think having these tests run only at least ZK mode is okay I think. Maybe @henryZrncik or @im-konge knows more about this.

So, should it be fixed? Extended? Deleted? ZooKeeper will be gone soon and any ZooKeeper only tests will be deleted with it.

I mean if those tests could run also on KRaft maybe we should update them...I think @henryZrncik and @fvaleri were trying to somehow fix it but I am not sure if that's possible (it was something related to UTO).

scholzj commented 1 month ago

Why would you want to run them in ZooKeeper mode only?

Well, some of the test cases do not work with KRaft here (e.g., testTopicNotAvailable, testTopicAvailable and more...). I don't have any strong opinion on this but for now, I think having these tests run only at least ZK mode is okay I think. Maybe @henryZrncik or @im-konge knows more about this.

So, should it be fixed? Extended? Deleted? ZooKeeper will be gone soon and any ZooKeeper only tests will be deleted with it.

I mean if those tests could run also on KRaft maybe we should update them...I think @henryZrncik and @fvaleri were trying to somehow fix it but I am not sure if that's possible (it was something related to UTO).

Right, but we need to understand what exactly the problem is. UTO is now used everywhere. So not sure why would that make a difference Zoo versus KRaft.

see-quick commented 1 month ago

Thanks for PR!

IMHO Changes in the RecoveryST would need a few more steps to run correctly.

Regarding NamespaceDeletionRecovery as this tests represent steps done in documentation it would be nice to keep these tests (referencing to removing zookeeper related tests) it would be probably nice to find out what is the cause of problem in kraft after you resolved problem with incorrect zookeeper configuration.

Thanks for investigating this problem. So let's do the following:

  1. Remove ZK-only tests, which can't be run as KRAft I think we have 2 of them (i.e., testRecoveryFromZookeeperServiceDeletion and testRecoveryFromZookeeperHeadlessServiceDeletion).

And in the case of testTopicNotAvailable, I am unsure if that's a possible bug in KRaft? 🤔 I will double-check this one.

So in summary I think making those tests KRaft-only compatible would be better than just ZK (because of the mentioned issue that ZK will be removed soon).

see-quick commented 1 week ago

I have updated the tests to match the recovery procedure. The tests are passing now with such a change.

see-quick commented 1 week ago

@strimzi-ci run tests --cluster-type=ocp --cluster-version=4.17 --install-type=bundle --profile=recovery

strimzi-ci commented 1 week ago

:arrow_forward: Build started - check Jenkins for more info. :arrow_forward:

see-quick commented 1 week ago

@strimzi-ci run tests --cluster-type=ocp --install-type=bundle --profile=recovery

strimzi-ci commented 1 week ago

:arrow_forward: Build started - check Jenkins for more info. :arrow_forward:

strimzi-ci commented 1 week ago

:heavy_check_mark: Test Summary :heavy_check_mark:

TEST_PROFILE: recovery GROUPS: TEST_CASE: TOTAL: 2 PASS: 2 FAIL: 0 SKIP: 0 BUILD_NUMBER: 7 OCP_VERSION: 4.17 BUILD_IMAGES: false FIPS_ENABLED: false PARALLEL_COUNT: 5 EXCLUDED_GROUPS: loadbalancer,nodeport,olm

see-quick commented 5 days ago

@strimzi-ci run tests --cluster-type=ocp --install-type=bundle --profile=recovery

strimzi-ci commented 5 days ago

:arrow_forward: Build started - check Jenkins for more info. :arrow_forward:

see-quick commented 5 days ago

@strimzi-ci run tests --cluster-type=ocp --install-type=bundle --profile=recovery

strimzi-ci commented 5 days ago

:arrow_forward: Build started - check Jenkins for more info. :arrow_forward:

see-quick commented 4 days ago

@strimzi-ci run tests --cluster-type=ocp --install-type=bundle --profile=recovery

strimzi-ci commented 4 days ago

:arrow_forward: Build started - check Jenkins for more info. :arrow_forward:

strimzi-ci commented 4 days ago

:x: Test Summary :x:

TEST_PROFILE: recovery GROUPS: TEST_CASE: TOTAL: 2 PASS: 0 FAIL: 2 SKIP: 0 BUILD_NUMBER: 10 OCP_VERSION: 4.17 BUILD_IMAGES: false FIPS_ENABLED: false PARALLEL_COUNT: 5 EXCLUDED_GROUPS: loadbalancer,nodeport,olm

:heavy_exclamation_mark: Test Failures :heavy_exclamation_mark:

Re-run command: @strimzi-ci run tests --profile=recovery --testcase=io.strimzi.systemtest.operators.NamespaceDeletionRecoveryST#testTopicNotAvailable

see-quick commented 4 days ago

@strimzi-ci run tests --cluster-type=ocp --install-type=bundle --profile=recovery

strimzi-ci commented 4 days ago

:arrow_forward: Build started - check Jenkins for more info. :arrow_forward:

strimzi-ci commented 3 days ago

:heavy_check_mark: Test Summary :heavy_check_mark:

TEST_PROFILE: recovery GROUPS: TEST_CASE: TOTAL: 2 PASS: 2 FAIL: 0 SKIP: 0 BUILD_NUMBER: 11 OCP_VERSION: 4.17 BUILD_IMAGES: false FIPS_ENABLED: false PARALLEL_COUNT: 5 EXCLUDED_GROUPS: loadbalancer,nodeport,olm

see-quick commented 3 days ago

@scholzj @fvaleri Do you have anything to add here? If not I will merge this today/tomorrow.

scholzj commented 3 days ago

@scholzj @fvaleri Do you have anything to add here? If not I will merge this today/tomorrow.

I'm still not sure I understand why we are removing the ZooKepeer test one release too early. But I'm not going to block it 🤷.

fvaleri commented 3 days ago

@see-quick sorry, I didn't have time to have another pass, but feel free to move on. Thanks.