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.

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

[ST] KRaft to KRaft upgrades/downgrades in STs #9442

Merged
merged 4 commits into from
Dec 8, 2023

Conversation

im-konge
Copy link
Member

@im-konge im-konge commented Dec 6, 2023

Type of change

  • Enhancement

Description

This PR adds STs for KRaft to KRaft upgrades/downgrades.
I decided to make a copy of our current tests that we have in our suite and create new package kraft for the KRaft to KRaft STs. As part of that, I moved the "regular" tests into the regular package.

As for the regular tests, for KRaft upgrades/downgrades I'm using the AbstractKRaftUpgradeST class, which extends the AbstractUpgradeST. To keep the methods same as much as possible, I moved some parts to separate methods, which are then overriden in the AbstractKRaftUpgradeST. So even if we are executing some method in the AbstractUpgradeST, if there is some method in the AbstractKRaftUpgradeST that overrides it, the test uses that one.

KRaftKafkaUpgradeDowngradeST works as expected, it does testing of just Kafka upgrade/downgrade. The two classes that are testing the Strimzi & Kafka upgrades/downgrades are implemented as well, the KRaftStrimziUpgradeST works, but the KRaftStrimziDowngradeST is currently disabled, as 0.38.0 version of Strimzi doesn't support KRaft to KRaft upgrades/downgrades, so that scenario will not currently work. But after 0.39.0 is released, it should be enabled -> and from my testing the tests inside the downgrade suite should work.

As part of this PR I'm changing few classes regarding Kafka version (adding the metadataVersion), loaders for the upgrade/downgrade tests (possibility to specify "always enabled" FG -> used for KRaft, UTO, and KafkaNodePools FGs),
and also adding some helper methods for the NodePools creation to the templates or LabelSelector for getting Pods for particular NodePool.

Finally, I'm adding these STs to AZP and updating our pom.xml with kraft_upgrade profile + adding KRaftKafkaUpgradeDowngradeST to azp_kafka_upgrade profile.

Checklist

  • Write tests
  • Make sure all tests pass

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.

Just a few nits :) but this is a huge amount of work! Good Job! 👍

Signed-off-by: Lukas Kral <lukywill16@gmail.com>

finish the upgrade/downgrade and kafka upgrade/downgrade

Signed-off-by: Lukas Kral <lukywill16@gmail.com>

AZP, MO's comments

Signed-off-by: Lukas Kral <lukywill16@gmail.com>
Signed-off-by: Lukas Kral <lukywill16@gmail.com>
@im-konge im-konge self-assigned this Dec 7, 2023
@im-konge im-konge added this to the 0.39.0 milestone Dec 7, 2023
@im-konge im-konge marked this pull request as ready for review December 7, 2023 08:50
Signed-off-by: Lukas Kral <lukywill16@gmail.com>
@im-konge
Copy link
Member Author

im-konge commented Dec 7, 2023

/azp run upgrade

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Signed-off-by: Lukas Kral <lukywill16@gmail.com>
@im-konge im-konge requested a review from scholzj December 7, 2023 21:22
@im-konge
Copy link
Member Author

im-konge commented Dec 7, 2023

/azp run upgrade

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Copy link
Member

@scholzj scholzj left a comment

Choose a reason for hiding this comment

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

LGTM assuming the tests pass. Should be reviewed by SMEs.

Copy link
Contributor

@henryZrncik henryZrncik left a comment

Choose a reason for hiding this comment

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

great.

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.

LGTM. Thanks for this PR @im-konge ! 💯

@see-quick see-quick merged commit f71526f into strimzi:main Dec 8, 2023
17 checks passed
@im-konge im-konge deleted the kraft-to-kraft-st branch December 8, 2023 16:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants