Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[ST] Remove rest of the Zookeeper occurences from STs, together with other unused code (UTO, NodePools/KRaft checks) #11041

Merged
merged 1 commit into from
Jan 17, 2025
Merged

[ST] Remove rest of the Zookeeper occurences from STs, together with other unused code (UTO, NodePools/KRaft checks) #11041

merged 1 commit into from
Jan 17, 2025

Conversation

im-konge
Copy link
Member

@im-konge im-konge commented Jan 15, 2025 •
edited
Loading

Type of change

  • Refactoring

Description

This PR (as the last PR from the serious of "ZK removal from STs" PRs) is deleting rest of the ZK code in the STs, together with env variables and functionality which is not needed anymore -> related to UTO/BTO (as the UTO is the only Topic Operator supported for a while), checks for NodePools or KRaft usage (as both are enabled by default). Additionally, the NodePoolsConverter is deleted by this PR as well, as it is not needed anymore -> for the mixed roles mode we decided to not go with the way of rewriting whole test-suite and all tests, but just pick scenarios that are good to be checked with mixed roles and write few tests for them. The functionality of switching the NodePool roles (mixed/separate) wasn't used anyway. Finally, the methods from KafkaTemplates were cleaned up as well, as we are not differentiating between the persistent or ephemeral Kafka -> everything is handled in the NodePools.

Checklist

  • Make sure all tests pass

@im-konge im-konge force-pushed the remove-other-zk-np-stuff branch from a77b9bb to 3412d1d Compare January 15, 2025 11:38
@im-konge im-konge added this to the 0.46.0 milestone Jan 15, 2025
@im-konge im-konge self-assigned this Jan 15, 2025
@im-konge
Copy link
Member Author

/azp run regression

@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@im-konge
Copy link
Member Author

/azp run upgrade

@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@im-konge im-konge force-pushed the remove-other-zk-np-stuff branch from 3412d1d to e68c709 Compare January 15, 2025 16:20
@im-konge
remove ZK stuff from STs
a395fdc 
Signed-off-by: Lukas Kral <[email protected]>

[ST] Remove the rest of ZK stuff

Signed-off-by: Lukas Kral <[email protected]>

remove rest of ZK stuff, UTO etc.

Signed-off-by: Lukas Kral <[email protected]>

update STs docs

Signed-off-by: Lukas Kral <[email protected]>

changes to kafkautils

Signed-off-by: Lukas Kral <[email protected]>
@im-konge im-konge force-pushed the remove-other-zk-np-stuff branch from e68c709 to a395fdc Compare January 15, 2025 18:09
@im-konge im-konge changed the title [ST] Remove rest of the Zookeeper things from STs [ST] Remove rest of the Zookeeper occurences from STs, together with other unused code (UTO, NodePools/KRaft checks) Jan 15, 2025
@im-konge im-konge marked this pull request as ready for review January 15, 2025 18:10
@im-konge im-konge requested a review from a team January 15, 2025 18:10
@im-konge
Copy link
Member Author

/azp run regression

@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@im-konge
Copy link
Member Author

/azp run feature-gates-regression

@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@im-konge
Copy link
Member Author

/azp run upgrade

@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Frawless
Frawless approved these changes Jan 16, 2025
View reviewed changes
Copy link
Member

@Frawless Frawless left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume that the tests works withe very install type on every env.

@im-konge
Copy link
Member Author

/azp list

@azure-pipelines Azure Pipelines
Copy link

CI/CD Pipelines for this repository:

@im-konge
Copy link
Member Author

/azp run helm-acceptance

@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@im-konge
Copy link
Member Author

@strimzi-ci run tests --cluster-type=ocp --profile=all --groups=olm_acceptance_upgrade --install-type=olm --cluster-version=4.17 --env=OLM_OPERATOR_VERSION=6.6.6;OLM_SOURCE_NAME=strimzi-source;

@strimzi-ci
Copy link

▶️ Build started - check Jenkins for more info. ▶️

@strimzi-ci
Copy link

❗ Systemtests Failed (no tests results are present) ❗

jankalinic
jankalinic approved these changes Jan 16, 2025
View reviewed changes
Copy link
Contributor

@jankalinic jankalinic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While nothing stands out, i can approve with the assumption of all the tests passing.

@im-konge
Copy link
Member Author

@strimzi-ci run tests --cluster-type=ocp --profile=olm_acceptance_upgrade --install-type=olm --cluster-version=4.17 --env=OLM_OPERATOR_VERSION=6.6.6;OLM_SOURCE_NAME=strimzi-source;

@strimzi-ci
Copy link

▶️ Build started - check Jenkins for more info. ▶️

@strimzi-ci
Copy link

❌ Test Summary ❌

TEST_PROFILE: olm_acceptance_upgrade
GROUPS:
TEST_CASE:
TOTAL: 25
PASS: 24
FAIL: 1
SKIP: 0
BUILD_NUMBER: 20
OCP_VERSION: 4.17
BUILD_IMAGES: false
FIPS_ENABLED: false
PARALLEL_COUNT: 5
EXCLUDED_GROUPS: loadbalancer,nodeport,olm
ENV_VARIABLES: OLM_OPERATOR_VERSION=6.6.6;OLM_SOURCE_NAME=strimzi-source;

❗ Test Failures ❗

  • io.strimzi.systemtest.security.oauth.OauthTlsST

Re-run command:
@strimzi-ci run tests --profile=olm_acceptance_upgrade --testcase=io.strimzi.systemtest.security.oauth.OauthTlsST

@im-konge
Copy link
Member Author

There is just one test-failure for OAuthTlsST, which is not related to this PR (it fails to deploy Postgres). It will be handled in different PR or directly in CI.

see-quick
see-quick approved these changes Jan 17, 2025
View reviewed changes
Copy link
Member

@see-quick see-quick left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, looks very good! 👍 🔥

@im-konge im-konge merged commit c1b20f7 into strimzi:main Jan 17, 2025
35 of 36 checks passed
@im-konge im-konge deleted the remove-other-zk-np-stuff branch January 17, 2025 11:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jankalinic jankalinic jankalinic approved these changes

@see-quick see-quick see-quick approved these changes

@Frawless Frawless Frawless approved these changes

Assignees

@im-konge im-konge

Labels
System tests
Projects
None yet
Milestone
0.46.0
Development

Successfully merging this pull request may close these issues.
5 participants
@im-konge @strimzi-ci @Frawless @see-quick @jankalinic