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

docs(kraft): adds a procedure for zookeeper to kraft migration #9633

Merged
merged 16 commits into from Mar 2, 2024
Merged

docs(kraft): adds a procedure for zookeeper to kraft migration #9633

merged 16 commits into from Mar 2, 2024

Conversation

PaulRMellor
Copy link
Contributor

@PaulRMellor PaulRMellor commented Feb 2, 2024 •
edited

Documentation

Adds a new procedure that describes how to migrate from ZooKeeper-based to KRaft-based Kafka cluster.
Also includes steps to roll back, if required.

Checklist

Please go through this checklist and make sure all applicable tasks have been done

  • Write tests
  • Make sure all tests pass
  • Update documentation
  • Check RBAC rights for Kubernetes / OpenShift roles
  • 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
  • Supply screenshots for visual changes, such as Grafana dashboards

@PaulRMellor
docs(kraft): adds a procedure for zookeeper to kraft migration
5749869 
Signed-off-by: prmellor <pmellor@redhat.com>
@PaulRMellor PaulRMellor added this to the 0.40.0 milestone Feb 2, 2024
@PaulRMellor PaulRMellor self-assigned this Feb 2, 2024
scholzj
scholzj reviewed Feb 2, 2024
View reviewed changes
documentation/assemblies/upgrading/assembly-upgrade-zookeeper.adoc Outdated Show resolved Hide resolved
documentation/modules/configuring/con-config-node-pools.adoc Outdated Show resolved Hide resolved
documentation/modules/managing/con-custom-resources-status.adoc Outdated Show resolved Hide resolved
documentation/modules/deploying/proc-deploy-migrate-kraft.adoc Outdated Show resolved Hide resolved
documentation/modules/deploying/proc-deploy-migrate-kraft.adoc Outdated Show resolved Hide resolved
documentation/modules/deploying/proc-deploy-migrate-kraft.adoc Outdated Show resolved Hide resolved
documentation/modules/deploying/proc-deploy-migrate-kraft.adoc Outdated Show resolved Hide resolved
documentation/modules/deploying/proc-deploy-migrate-kraft.adoc Outdated Show resolved Hide resolved
documentation/modules/deploying/proc-deploy-migrate-kraft.adoc Outdated Show resolved Hide resolved
documentation/modules/deploying/proc-deploy-migrate-kraft.adoc Outdated Show resolved Hide resolved
@PaulRMellor
docs(review): edits to doc from review by JS -- 01
d822f0f 
Signed-off-by: prmellor <pmellor@redhat.com>
ppatierno
ppatierno reviewed Feb 5, 2024
View reviewed changes
documentation/assemblies/upgrading/assembly-upgrade-zookeeper.adoc Outdated Show resolved Hide resolved
documentation/modules/deploying/proc-deploy-migrate-kraft.adoc Outdated Show resolved Hide resolved
documentation/modules/deploying/proc-deploy-migrate-kraft.adoc Outdated Show resolved Hide resolved
documentation/modules/deploying/proc-deploy-migrate-kraft.adoc Outdated Show resolved Hide resolved
documentation/modules/deploying/proc-deploy-migrate-kraft.adoc Outdated Show resolved Hide resolved
documentation/modules/deploying/proc-deploy-migrate-kraft.adoc Outdated Show resolved Hide resolved
documentation/modules/deploying/proc-deploy-migrate-kraft.adoc Outdated Show resolved Hide resolved
documentation/modules/deploying/proc-deploy-migrate-kraft.adoc Show resolved Hide resolved
PaulRMellor and others added 3 commits February 5, 2024 12:22
@PaulRMellor
docs(review): edits to doc from comments by PP -- 01
4fbb2ea 
Signed-off-by: prmellor <pmellor@redhat.com>
@PaulRMellor
docs(review): edits to doc from comments by PP -- 02
006db58 
Signed-off-by: prmellor <pmellor@redhat.com>
@PaulRMellor
Merge branch 'main' into docs_kraft-migration
87baaba 
Signed-off-by: PaulRMellor <47596553+PaulRMellor@users.noreply.github.com>
@ppatierno
Copy link
Member

@PaulRMellor I added the "do-not-merge" label because this one should anyway wait for the implementation in #9480 being merged as well.

@PaulRMellor
docs(review): edits to doc from review by JS -- 02
91ba17c 
Signed-off-by: prmellor <pmellor@redhat.com>
scholzj
scholzj approved these changes Feb 6, 2024
View reviewed changes
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. But it shoud be approved by Paolo as the migration SME.

ppatierno
ppatierno reviewed Feb 7, 2024
View reviewed changes
documentation/assemblies/upgrading/assembly-upgrade-zookeeper.adoc Outdated Show resolved Hide resolved
documentation/modules/deploying/proc-deploy-migrate-kraft.adoc Outdated Show resolved Hide resolved
documentation/modules/deploying/proc-deploy-migrate-kraft.adoc Outdated Show resolved Hide resolved
documentation/modules/deploying/proc-deploy-migrate-kraft.adoc Outdated Show resolved Hide resolved
documentation/modules/deploying/proc-deploy-migrate-kraft.adoc Outdated Show resolved Hide resolved
documentation/modules/deploying/proc-deploy-migrate-kraft.adoc Outdated Show resolved Hide resolved
documentation/modules/deploying/proc-deploy-migrate-kraft.adoc Outdated Show resolved Hide resolved
documentation/modules/deploying/proc-deploy-migrate-kraft.adoc Outdated Show resolved Hide resolved
documentation/modules/deploying/proc-deploy-migrate-kraft.adoc Show resolved Hide resolved
documentation/modules/deploying/proc-deploy-migrate-kraft.adoc Outdated Show resolved Hide resolved
see-quick
see-quick reviewed Feb 7, 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.

first iteration adding a few comments, I would also do the second one to check all procedures you have add here.

documentation/modules/deploying/proc-deploy-migrate-kraft.adoc Outdated Show resolved Hide resolved
documentation/modules/deploying/proc-deploy-migrate-kraft.adoc Outdated Show resolved Hide resolved
@PaulRMellor
docs(review): edits to doc from comments by PP -- 03
2b800e8 
Signed-off-by: prmellor <pmellor@redhat.com>
@PaulRMellor
docs(review): edits to doc from comments by M0 -- 01
56216a6 
Signed-off-by: prmellor <pmellor@redhat.com>
@PaulRMellor
docs(review): edits to doc from comments by PP -- 04
14a55d9 
Signed-off-by: prmellor <pmellor@redhat.com>
@PaulRMellor
docs(review): edist to doc from comments by PP -- 05
9e9f239 
Signed-off-by: prmellor <pmellor@redhat.com>
ppatierno
ppatierno approved these changes Feb 8, 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! Thanks!
As already said we'll not merge this until the corresponding implementation PR #9480 is merged as well.

fvaleri
fvaleri reviewed Feb 8, 2024
View reviewed changes
Copy link
Contributor

@fvaleri fvaleri left a comment

Choose a reason for hiding this comment

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

Thanks. Left some comments.

documentation/modules/deploying/proc-deploy-migrate-kraft.adoc Outdated Show resolved Hide resolved
documentation/modules/deploying/proc-deploy-migrate-kraft.adoc Outdated Show resolved Hide resolved
documentation/modules/deploying/proc-deploy-migrate-kraft.adoc Outdated Show resolved Hide resolved
@ppatierno
Copy link
Member

@PaulRMellor while having an offline chat with @fvaleri it could be worth mentioning that if your initial configuration has the inter broker protocol and/or log message format set, you should remove them at the end of the migration. The brokers and controllers will be rolled one more time to update them.
At the same time even when removing the spec.zookeeper.
There are warnings in the Kafka CR but after the rolling, those are removed.

see-quick
see-quick approved these changes Feb 9, 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.

Just a few comments from me 👍 Looks good

documentation/modules/deploying/proc-deploy-migrate-kraft.adoc Outdated Show resolved Hide resolved
documentation/modules/deploying/proc-deploy-migrate-kraft.adoc Outdated Show resolved Hide resolved
documentation/modules/deploying/proc-deploy-migrate-kraft.adoc Show resolved Hide resolved
documentation/modules/deploying/proc-deploy-migrate-kraft.adoc Outdated Show resolved Hide resolved
documentation/modules/deploying/proc-deploy-migrate-kraft.adoc Outdated Show resolved Hide resolved
documentation/modules/deploying/proc-deploy-migrate-kraft.adoc Show resolved Hide resolved
@PaulRMellor
docs(review): annotations values update
1f44abc 
Signed-off-by: prmellor <pmellor@redhat.com>
ppatierno
ppatierno requested changes Feb 9, 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.

@PaulRMellor We should take all the updates not related to migration out of this PR but having a new one for them. I see you updated annotation values in all other places, but I would move them for a new PR. Let's take this PR for migration only.

@PaulRMellor
docs(review): edits to doc from comments by FV -- 01
2d80e20 
Signed-off-by: prmellor <pmellor@redhat.com>
@PaulRMellor PaulRMellor force-pushed the docs_kraft-migration branch 2 times, most recently from 8c7f5e9 to 2d80e20 Compare February 9, 2024 14:35
@PaulRMellor
docs(review): edits to doc from comments by MO -- 02
3b596a4 
Signed-off-by: prmellor <pmellor@redhat.com>
@PaulRMellor
docs(revert): revert unwanted changes
b9936be 
Signed-off-by: prmellor <pmellor@redhat.com>
ppatierno
ppatierno approved these changes Feb 9, 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. Back to green!

fvaleri
fvaleri approved these changes Feb 9, 2024
View reviewed changes
Copy link
Contributor

@fvaleri fvaleri 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.

@ppatierno
Copy link
Member

@PaulRMellor while having an offline chat with @fvaleri it could be worth mentioning that if your initial configuration has the inter broker protocol and/or log message format set, you should remove them at the end of the migration. The brokers and controllers will be rolled one more time to update them.
At the same time even when removing the spec.zookeeper.
There are warnings in the Kafka CR but after the rolling, those are removed.

@PaulRMellor I don't see addressed the above comment here.

@PaulRMellor
docs(addition): step for removing zookeeper config
a826dea 
Signed-off-by: prmellor <pmellor@redhat.com>
@ppatierno ppatierno mentioned this pull request Feb 22, 2024
Merged
5 tasks
@PaulRMellor
docs(review): edits to doc from review by JS -- 03
fc2be1e 
Signed-off-by: prmellor <pmellor@redhat.com>
@ppatierno ppatierno merged commit e266f0a into strimzi:main Mar 2, 2024
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@scholzj scholzj scholzj approved these changes

@ppatierno ppatierno ppatierno approved these changes

@fvaleri fvaleri fvaleri approved these changes

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

Assignees

@PaulRMellor PaulRMellor

Labels
documentation
Projects
None yet
Milestone
0.40.0
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@PaulRMellor @ppatierno @scholzj @fvaleri @see-quick