-
Notifications
You must be signed in to change notification settings - Fork 156
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
Improve initial node deployment creation #6064
Improve initial node deployment creation #6064
Conversation
Skipping CI for Draft Pull Request. |
/test all |
pkg/controller/seed-controller-manager/update/update_controller.go
Outdated
Show resolved
Hide resolved
pkg/controller/seed-controller-manager/update/update_controller.go
Outdated
Show resolved
Hide resolved
pkg/controller/seed-controller-manager/update/update_controller.go
Outdated
Show resolved
Hide resolved
/test pre-kubermatic-test-integration |
/retest |
/approve |
LGTM label has been added. Git tree hash: c56ee00333d06982a39c9155618e8911eab13542
|
/hold |
Since Marcin is OOO, I am taking over. |
e318597
to
172750a
Compare
pkg/controller/seed-controller-manager/initialmachinedeployment/controller.go
Outdated
Show resolved
Hide resolved
pkg/controller/seed-controller-manager/initialmachinedeployment/controller.go
Show resolved
Hide resolved
pkg/controller/seed-controller-manager/initialmachinedeployment/controller.go
Show resolved
Hide resolved
Looks very good. It seems a little bit better to have it in a new controller as it is easier to read code in that way. I have added some remarks/questions. |
7fca65f
to
42411a9
Compare
Added some unit tests as well. This is now ready for review. |
/retest |
/approve |
LGTM label has been added. Git tree hash: 478a0e9d84333aca663d5b5e1057b2d12a7173f9
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: maciaszczykm, moadqassem, xrstf The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
What this PR does / why we need it: Currently the whole cluster and initial node deployment creation process happens in the API. First we create the cluster, then start the background process for initial node deployment creation and return the created cluster to the user. The main issue with this approach is that if the API will crash or if it will restart during the background process run that handles initial node deployment creation, then the data will be lost and the cluster will have no initial deployment added to it. This pull request changes the approach and adds persistence layer to the process.
Which issue(s) this PR fixes (optional, in
fixes #<issue number>(, fixes #<issue_number>, ...)
format, will close the issue(s) when PR gets merged): Fixes #2928.Special notes for your reviewer:
Documentation:
Does this PR introduce a user-facing change?: