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

Reflect the current roles when rolling Kafka nodes in KafkaRoller #9686

Conversation

scholzj
Copy link
Member

@scholzj scholzj commented Feb 14, 2024

Type of change

  • Enhancement / new feature

Description

This PR updates the KafkaRoller to roll the Kafka nodes based on their current roles and not based on the desired roles. The current roles are taken from the Pod labels. If the pod or the annotation is missing, we assume that the role did not change and use the desired roles (this is what is done today all the time). This fix improves the transitions between architectures and enables moving from dedicated controller nodes to mixed nodes by adding the broker role to the controller nodes. Additional testing will be done in system tests in a separate PR.

This should resolve #9434.

Checklist

  • Write tests
  • Make sure all tests pass
  • Update documentation
  • Try your changes from Pod inside your Kubernetes and OpenShift cluster, not just locally
  • Reference relevant issue(s) and close them after merging
  • Update CHANGELOG.md

@scholzj scholzj added this to the 0.40.0 milestone Feb 14, 2024
@scholzj scholzj force-pushed the reflect-the-current-roles-when-rolling-the-pods-in-KafkaRoller branch from 3861b0d to 4755bcd Compare February 14, 2024 15:27
@scholzj
Copy link
Member Author

scholzj commented Feb 14, 2024

/azp run regression

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@scholzj
Copy link
Member Author

scholzj commented Feb 14, 2024

/azp run kraft-regression

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@scholzj scholzj marked this pull request as ready for review February 14, 2024 22:11
…loses strimzi#9434

Signed-off-by: Jakub Scholz <www@scholzj.com>
@scholzj scholzj force-pushed the reflect-the-current-roles-when-rolling-the-pods-in-KafkaRoller branch from 4755bcd to 22a60c5 Compare February 14, 2024 22:33
Copy link
Contributor

@tinaselenge tinaselenge left a comment

Choose a reason for hiding this comment

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

I left a few minor comments, otherwise LGTM

Signed-off-by: Jakub Scholz <www@scholzj.com>
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 to me. 👍
Couple of suggestions

Signed-off-by: Jakub Scholz <www@scholzj.com>
@scholzj scholzj merged commit 12a40b2 into strimzi:main Feb 15, 2024
13 checks passed
@scholzj scholzj deleted the reflect-the-current-roles-when-rolling-the-pods-in-KafkaRoller branch February 15, 2024 17:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[KRaft] KafkaRoller is strugling to transition controller-only nodes to mixed nodes
6 participants