Skip to content

[GEP-31] Controller changes to Support Auto InPlaceUpdate strategy #973

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
Merged
unmarshall merged 28 commits into from
Apr 23, 2025
Merged

[GEP-31] Controller changes to Support Auto InPlaceUpdate strategy #973

unmarshall merged 28 commits into from
Apr 23, 2025

Conversation

@acumino
Copy link
Member

@acumino acumino commented Mar 11, 2025 •
edited
Loading

What this PR does / why we need it:
This PR introduces the following changes -

  • Introduces new node condition, 'LastOperations state, and CurrentStatus Phase for in-place update.
  • Controller change for support Auto InPlaceUpdate strategy.
  • Drain logic for in-place updates.
  • Machine health check adaptation for in-place update.
  • New predicate for machine reconciliation so if a node is labeled with any of the in-place update labels, the machine should get reconciled.

Which issue(s) this PR fixes:
Part of #944
Part of gardener/gardener#10219

Special notes for your reviewer:
/invite @aaronfern @elankath @unmarshall

Release note:

Machine Controller Manager now supports a new machine deployment strategy called `InPlaceUpdate`.

@acumino acumino requested a review from a team as a code owner March 11, 2025 08:55
@gardener-robot gardener-robot added needs/review Needs review kind/api-change API change with impact on API users needs/second-opinion Needs second review by someone else labels Mar 11, 2025
@gardener-robot gardener-robot added the size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. label Mar 11, 2025
@acumino acumino changed the title [GEP-31] Controller changes to Support InPlaceUpdate Auto strategy [GEP-31] Controller changes to Support Auto InPlaceUpdate strategy Mar 11, 2025
@gardener-robot-ci-1 gardener-robot-ci-1 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Mar 11, 2025
@gardener-robot-ci-2 gardener-robot-ci-2 added needs/ok-to-test Needs approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Mar 11, 2025
@acumino acumino force-pushed the auto-inplace-update branch from ba1db8b to ef33379 Compare March 11, 2025 09:16
@acumino acumino mentioned this pull request Mar 7, 2025
Closed
12 tasks
@ghost ghost added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Mar 11, 2025
@gardener-robot-ci-2 gardener-robot-ci-2 removed the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Mar 11, 2025
@acumino acumino force-pushed the auto-inplace-update branch from ef33379 to 58637c1 Compare March 11, 2025 10:02
@gardener-robot-ci-1 gardener-robot-ci-1 added reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Mar 11, 2025
aaronfern
aaronfern reviewed Mar 12, 2025
View reviewed changes
Copy link
Member

@aaronfern aaronfern left a comment •
edited
Loading

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 out initial look. PTAL
-- MCM team

aaronfern
aaronfern reviewed Mar 13, 2025
View reviewed changes
Copy link
Member

@aaronfern aaronfern left a comment

Choose a reason for hiding this comment

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

Some more comments. PTAL
Thanks, MCM Team

@gardener-robot
Copy link

gardener-robot commented Mar 14, 2025

@elankath, @unmarshall You have pull request review open invite, please check

aaronfern
aaronfern reviewed Mar 15, 2025
View reviewed changes
takoverflow
takoverflow reviewed Mar 17, 2025
View reviewed changes
Copy link
Member

@takoverflow takoverflow left a comment

Choose a reason for hiding this comment

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

Some minor cosmetic changes and typo fixes, PTAL.

ary1992
ary1992 reviewed Mar 18, 2025
View reviewed changes
aaronfern
aaronfern reviewed Mar 18, 2025
View reviewed changes
Copy link
Member

@aaronfern aaronfern left a comment

Choose a reason for hiding this comment

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

Have some more review comments, please have a look
Thanks, MCM Team

aaronfern
aaronfern reviewed Mar 19, 2025
View reviewed changes
aaronfern
aaronfern reviewed Mar 19, 2025
View reviewed changes
@gardener-robot-ci-1 gardener-robot-ci-1 added reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Mar 20, 2025
acumino and others added 13 commits April 17, 2025 13:30
@acumino @shafeeqes @ary1992
[MCS] Label all oldISs to not upscale for the transferred machine
ebc3be7 
When transferring ownership old machine set should not create new machine for the machine transferred to new machine set

Co-authored-by: Shafeeque E S <shafeeque.e.s@sap.com>
Co-authored-by: Ashish Ranjan Yadav  <ashish.ranjan.yadav@sap.com>
@acumino @shafeeqes @ary1992
[Machine Safety] Calculate maxSurge based on update strategy
cf64bde 
Co-authored-by: Shafeeque E S <shafeeque.e.s@sap.com>
Co-authored-by: Ashish Ranjan Yadav  <ashish.ranjan.yadav@sap.com>
@acumino @shafeeqes @ary1992
[MC] Update node condition based on the in-place update state
28ff779 
Co-Authored-By: Shafeeque E S <shafeeque.e.s@sap.com>
Co-Authored-By: Ashish Ranjan Yadav <ashish.ranjan.yadav@sap.com>
@acumino @shafeeqes @ary1992
[MC] Drain Nodes which has been selected for update
0177cd7 
Co-Authored-By: Shafeeque E S <shafeeque.e.s@sap.com>
Co-Authored-By: Ashish Ranjan Yadav <ashish.ranjan.yadav@sap.com>
@acumino @shafeeqes @ary1992
[MC] Adapt machine health reconciliation
554b390 
Co-Authored-By: Shafeeque E S <shafeeque.e.s@sap.com>
Co-Authored-By: Ashish Ranjan Yadav <ashish.ranjan.yadav@sap.com>
@acumino @shafeeqes @ary1992
[MC] Trigger machine reconciliation on addition of in-place update no…
27d1e32 
…de label

Co-authored-by: Shafeeque E S <shafeeque.e.s@sap.com>
Co-authored-by: Ashish Ranjan Yadav  <ashish.ranjan.yadav@sap.com>
@acumino @ary1992
Nit
0a6e436
@acumino @ary1992
Fix case where there are zero machines in oldMachineSets
470b313
@acumino @ary1992
Address Review
48121ea
@acumino @ary1992
Address Review
a082dd1
@acumino @ary1992
Address Review
32a4132
@ary1992
Address PR review feedback
885760c
@ary1992
Adapt constant name change
6900f38
@ary1992 ary1992 force-pushed the auto-inplace-update branch from 88bd02e to 6900f38 Compare April 21, 2025 06:12
@gardener-robot-ci-1 gardener-robot-ci-1 added reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Apr 21, 2025
unmarshall
unmarshall requested changes Apr 21, 2025
View reviewed changes
@gardener-robot-ci-2 gardener-robot-ci-2 added reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Apr 21, 2025
@acumino acumino requested a review from unmarshall April 21, 2025 15:42
@ghost ghost added reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Apr 23, 2025
@unmarshall unmarshall merged commit 26533e0 into gardener:master Apr 23, 2025
8 checks passed
@gardener-robot gardener-robot added the status/closed Issue is closed (either delivered or triaged) label Apr 23, 2025
@acumino acumino deleted the auto-inplace-update branch April 23, 2025 09:38
@acumino acumino mentioned this pull request Apr 28, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@aaronfern aaronfern aaronfern left review comments

@unmarshall unmarshall unmarshall approved these changes

@elankath elankath Awaiting requested review from elankath

@takoverflow takoverflow Awaiting requested review from takoverflow

+1 more reviewer

@ary1992 ary1992 ary1992 left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

kind/api-change API change with impact on API users needs/changes Needs (more) changes needs/ok-to-test Needs approval for testing (check PR in detail before setting this label because PR is run on CI/CD) needs/rebase Needs git rebase needs/review Needs review needs/second-opinion Needs second review by someone else size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. status/closed Issue is closed (either delivered or triaged)

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

9 participants

@acumino @gardener-robot @aaronfern @unmarshall @elankath @ary1992 @takoverflow @gardener-robot-ci-1 @gardener-robot-ci-2