Skip to content

[GEP-31] Adapt Shoot reconciler for in-place update #11843

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
merged 8 commits into from
Apr 18, 2025
Merged

[GEP-31] Adapt Shoot reconciler for in-place update #11843

merged 8 commits into from
Apr 18, 2025

Conversation

shafeeqes
Copy link
Contributor

@shafeeqes shafeeqes commented Apr 8, 2025 •
edited
Loading

How to categorize this PR?

/area control-plane delivery open-source scalability
/kind enhancement

What this PR does / why we need it:

  • Introduce a new field in the Shoot status API to keep track of pending in-place updates.
    • Adapt shoot reconciler to add pending worker pools to the Shoot status in the start of the reconciliation by fetching the current worker status.
    • Deny making changes to the same worker pool/ triggering a credential rotation during an ongoing update.

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

Special notes for your reviewer:
Initially part of #11713

Release note:

NONE

@gardener-prow gardener-prow bot added area/control-plane Control plane related area/delivery Delivery related area/open-source Open Source (community, enablement, contributions, conferences, CNCF, etc.) related area/scalability Scalability related kind/enhancement Enhancement, improvement, extension labels Apr 8, 2025
@gardener-prow gardener-prow bot requested a review from LucaBernstein April 8, 2025 18:13
@gardener-prow gardener-prow bot added the cla: yes Indicates the PR's author has signed the cla-assistant.io CLA. label Apr 8, 2025
@gardener-prow gardener-prow bot requested a review from tobschli April 8, 2025 18:13
@gardener-prow gardener-prow bot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Apr 8, 2025
This was referenced Apr 8, 2025
Merged
Open
@shafeeqes shafeeqes force-pushed the inplace-updates-worker-2 branch from ca6bd6e to ef1711f Compare April 9, 2025 07:52
@shafeeqes shafeeqes mentioned this pull request Apr 9, 2025
Merged
@shafeeqes shafeeqes force-pushed the inplace-updates-worker-2 branch 2 times, most recently from ef37022 to f343a3e Compare April 9, 2025 08:11
rfranzke
rfranzke reviewed Apr 15, 2025
View reviewed changes
Copy link
Member

@rfranzke rfranzke left a comment

Choose a reason for hiding this comment

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

Can you please also add a new e2e test that covers this in-place update logic? Also in the light of credentials rotation. Thank you!

@shafeeqes
Copy link
Contributor Author

Can you please also add a new e2e test that covers this in-place update logic? Also in the light of credentials rotation. Thank you!

It's on the list of tasks in #10219, We didn't add it in this PR since it needs a release of MCM with the controller changes.

@shafeeqes shafeeqes requested a review from rfranzke April 16, 2025 06:29
@rfranzke
Copy link
Member

/lgtm
/approve

@gardener-prow gardener-prow bot added the lgtm Indicates that a PR is ready to be merged. label Apr 16, 2025
@gardener-prow Gardener Prow
Copy link
Contributor

gardener-prow bot commented Apr 16, 2025

LGTM label has been added.

Git tree hash: 3152d5a277a663e46463fcaaea5180bc49bbfc5a

@gardener-prow gardener-prow bot added approved Indicates a PR has been approved by an approver from all required OWNERS files. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Apr 16, 2025
@shafeeqes shafeeqes force-pushed the inplace-updates-worker-2 branch from 7b82649 to 3333aee Compare April 16, 2025 09:43
@gardener-prow gardener-prow bot removed lgtm Indicates that a PR is ready to be merged. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Apr 16, 2025
@shafeeqes
Copy link
Contributor Author

/hold until #11713 is merged

@gardener-prow gardener-prow bot added do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Apr 16, 2025
shafeeqes and others added 7 commits April 17, 2025 19:52
@shafeeqes @ary1992 @acumino
Adapt Shoot status API
f3888a9 
Co-Authored-By: Ashish Ranjan Yadav <ashish.ranjan.yadav@sap.com>
Co-Authored-By: Sonu Kumar Singh <sonu.kumar.singh02@sap.com>
@shafeeqes @ary1992 @acumino
Deny updating the worker pool or credential rotation if there is an o…
a0844b0 
…ngoing in-place update

Co-Authored-By: Ashish Ranjan Yadav <ashish.ranjan.yadav@sap.com>
Co-Authored-By: Sonu Kumar Singh <sonu.kumar.singh02@sap.com>
@shafeeqes @ary1992 @acumino
Adapt shoot reconciler to update Shoot status with pending workers ba…
c83e453 
…sed on worker pool hash

Co-Authored-By: Ashish Ranjan Yadav <ashish.ranjan.yadav@sap.com>
Co-Authored-By: Sonu Kumar Singh <sonu.kumar.singh02@sap.com>
@shafeeqes @ary1992 @acumino
Handle hibernation case and updating credentials rotation phase
a02bac0 
Co-Authored-By: Ashish Ranjan Yadav <ashish.ranjan.yadav@sap.com>
Co-Authored-By: Sonu Kumar Singh <sonu.kumar.singh02@sap.com>
@shafeeqes @ary1992 @acumino
Run make generate
bc30429 
Co-Authored-By: Ashish Ranjan Yadav <ashish.ranjan.yadav@sap.com>
Co-Authored-By: Sonu Kumar Singh <sonu.kumar.singh02@sap.com>
@shafeeqes
Use gardener.cloud/operation annotation for force-in-place-update
b14c220
@shafeeqes
Address PR review feedback from rfranzke
9a3b641
@shafeeqes shafeeqes force-pushed the inplace-updates-worker-2 branch from 3333aee to 9a3b641 Compare April 17, 2025 14:24
@gardener-prow gardener-prow bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 17, 2025
@shafeeqes
Copy link
Contributor Author

/unhold

@gardener-prow gardener-prow bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Apr 17, 2025
acumino
acumino requested changes Apr 17, 2025
View reviewed changes
@shafeeqes shafeeqes force-pushed the inplace-updates-worker-2 branch from 3540424 to c270497 Compare April 18, 2025 04:44
acumino
acumino reviewed Apr 18, 2025
View reviewed changes
Copy link
Member

@acumino acumino left a comment

Choose a reason for hiding this comment

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

/lgtm
/approve

@gardener-prow gardener-prow bot added the lgtm Indicates that a PR is ready to be merged. label Apr 18, 2025
@gardener-prow Gardener Prow
Copy link
Contributor

gardener-prow bot commented Apr 18, 2025

LGTM label has been added.

Git tree hash: b72fa2bb9317c9aac00c52cfcf589f56a13642b7

@gardener-prow Gardener Prow
Copy link
Contributor

gardener-prow bot commented Apr 18, 2025

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: acumino, rfranzke

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@gardener-prow gardener-prow bot merged commit d75823c into gardener:master Apr 18, 2025
19 checks passed
@shafeeqes shafeeqes deleted the inplace-updates-worker-2 branch April 18, 2025 07:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@acumino acumino acumino requested changes

@LucaBernstein LucaBernstein Awaiting requested review from LucaBernstein

@tobschli tobschli Awaiting requested review from tobschli

@rfranzke rfranzke Awaiting requested review from rfranzke

Assignees

@rfranzke rfranzke

@acumino acumino

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. area/control-plane Control plane related area/delivery Delivery related area/open-source Open Source (community, enablement, contributions, conferences, CNCF, etc.) related area/scalability Scalability related cla: yes Indicates the PR's author has signed the cla-assistant.io CLA. kind/enhancement Enhancement, improvement, extension lgtm Indicates that a PR is ready to be merged. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@shafeeqes @rfranzke @acumino