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

Promote UseKRaft feature gate to beta #9518

Merged
merged 6 commits into from
Jan 12, 2024
Merged

Promote UseKRaft feature gate to beta #9518

merged 6 commits into from
Jan 12, 2024

Conversation

scholzj
Copy link
Member

@scholzj scholzj commented Jan 7, 2024 •
edited
Loading

Type of change

  • Task

Description

This PR provides the initial implementation of the [SP#62|https://github.com/strimzi/proposals/blob/main/062-UseKRaft-feature-gate-promotion.md] that promotes the UseKRaft feature gate to beta and enabled it by default. It includes the following tasks:

  • Promotes the feature gate
  • Makes the Kafka CRD fields not used in KRaft / with Node Pools not required and adds additional validation to the operator
  • Does the initial documentation update

There are some other parts that will be done only in a follow-up PRs. Mainly:

  • Warnings for the old fields being still used with KRaft
  • The validation of the KRaft versus ZooKeeper mode and prevention of unintentional switching between them.

Checklist

  • Write tests
  • Make sure all tests pass
  • Update documentation
  • Try your changes from Pod inside your Kubernetes and OpenShift cluster, not just locally
  • Update CHANGELOG.md

@scholzj scholzj added this to the 0.40.0 milestone Jan 7, 2024
@scholzj
Copy link
Member Author

scholzj commented Jan 8, 2024

/azp run regression

@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@scholzj
Copy link
Member Author

scholzj commented Jan 8, 2024

/azp run kraft-regression

@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@scholzj
Copy link
Member Author

scholzj commented Jan 8, 2024

/azp run upgrade

@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@scholzj scholzj force-pushed the promote-UseKRaft-feature-gate-to-beta branch from e0a528c to c7d5483 Compare January 8, 2024 10:54
@scholzj
Copy link
Member Author

scholzj commented Jan 8, 2024

/azp run regression

@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@scholzj scholzj force-pushed the promote-UseKRaft-feature-gate-to-beta branch from c7d5483 to 7f2b6ec Compare January 8, 2024 19:44
@scholzj
Copy link
Member Author

scholzj commented Jan 8, 2024

/azp run kraft-regression

@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@scholzj
Copy link
Member Author

scholzj commented Jan 9, 2024

/azp run feature-gates-regression

@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@scholzj scholzj force-pushed the promote-UseKRaft-feature-gate-to-beta branch from 7f2b6ec to ed4b84a Compare January 9, 2024 21:14
@scholzj
Copy link
Member Author

scholzj commented Jan 9, 2024

/azp run upgrade

@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@scholzj scholzj marked this pull request as ready for review January 10, 2024 07:54
im-konge
im-konge approved these changes Jan 10, 2024
View reviewed changes
Copy link
Member

@im-konge im-konge 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 :)

see-quick
see-quick approved these changes Jan 10, 2024
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.

LGTM, thanks for this PR 👍 just a nit

packaging/examples/kafka/kraft/README.md Outdated Show resolved Hide resolved
packaging/examples/kafka/kraft/README.md Outdated Show resolved Hide resolved
@scholzj
Review comments MO
99286cd 
Signed-off-by: Jakub Scholz <[email protected]>
ppatierno
ppatierno approved these changes Jan 11, 2024
View reviewed changes
Copy link
Member

@ppatierno ppatierno left a comment

Choose a reason for hiding this comment

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

LGTM.
(now it's on my checking what's the impact on my migration work)

@ppatierno
Copy link
Member

The validation of the KRaft versus ZooKeeper mode and prevention of unintentional switching between them.

Wondering what is this about, because in my migration work, the dedicated state manager (used for migration as well) already checks that you don't switch from disabled to enabled or vice versa. So if you are going to do something now it will brake for sure my logic or your logic will be removed when my logic will be in I guess. It's getting complicated developing this way ...

@scholzj
Copy link
Member Author

scholzj commented Jan 11, 2024

The validation of the KRaft versus ZooKeeper mode and prevention of unintentional switching between them.

Wondering what is this about, because in my migration work, the dedicated state manager (used for migration as well) already checks that you don't switch from disabled to enabled or vice versa. So if you are going to do something now it will brake for sure my logic or your logic will be removed when my logic will be in I guess. It's getting complicated developing this way ...

It is one of the things approved in the proposal for the promotion.

@ppatierno
Copy link
Member

It is one of the things approved in the proposal for the promotion.

The proposal says "It will be implemented as part of the migration work if it is shipped in Strimzi 0.40.0. Or separately if the migration is postponed to Strimzi 0.41.0." . So I guess it should hold on and wait for the migration work to check at which point we'll be when it's time for 0.40.0 but not implementing it now.

PaulRMellor
PaulRMellor approved these changes Jan 11, 2024
View reviewed changes
Copy link
Contributor

@PaulRMellor PaulRMellor left a comment

Choose a reason for hiding this comment

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

Looks good. Just a couple of suggestions.

api/src/main/java/io/strimzi/api/kafka/model/kafka/KafkaClusterSpec.java Outdated Show resolved Hide resolved
api/src/main/java/io/strimzi/api/kafka/model/kafka/KafkaClusterSpec.java Outdated Show resolved Hide resolved
documentation/modules/configuring/con-scaling-kafka-clusters.adoc Outdated Show resolved Hide resolved
documentation/modules/configuring/ref-storage-ephemeral.adoc Outdated Show resolved Hide resolved
documentation/modules/configuring/ref-storage-jbod.adoc Outdated Show resolved Hide resolved
documentation/modules/configuring/ref-storage-persistent.adoc Outdated Show resolved Hide resolved
documentation/modules/overview/con-configuration-points-node-pools.adoc Outdated Show resolved Hide resolved
@scholzj
Copy link
Member Author

scholzj commented Jan 11, 2024

/azp run upgrade

@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@scholzj
Copy link
Member Author

scholzj commented Jan 11, 2024

/azp run regression

@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@scholzj
Review comments PM
28091ce 
Signed-off-by: Jakub Scholz <[email protected]>
@scholzj
Copy link
Member Author

scholzj commented Jan 11, 2024

/azp run regression

@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@scholzj
Copy link
Member Author

scholzj commented Jan 11, 2024

/azp run upgrade

@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@scholzj
Copy link
Member Author

scholzj commented Jan 11, 2024

/azp run kraft-regression

@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@scholzj
Copy link
Member Author

scholzj commented Jan 12, 2024

/azp run feature-gates-regression

@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@scholzj scholzj merged commit 02fb63d into strimzi:main Jan 12, 2024
41 checks passed
@scholzj scholzj deleted the promote-UseKRaft-feature-gate-to-beta branch January 12, 2024 09:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

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

@ppatierno ppatierno ppatierno approved these changes

@PaulRMellor PaulRMellor PaulRMellor approved these changes

@im-konge im-konge im-konge approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
0.40.0
Development

Successfully merging this pull request may close these issues.
5 participants
@scholzj @ppatierno @see-quick @PaulRMellor @im-konge