fix(statefulset): delete pod on storage mismatch instead of in-place …#2388
fix(statefulset): delete pod on storage mismatch instead of in-place …#2388shivansh-gohem wants to merge 1 commit intoopenkruise:masterfrom
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
shivansh-gohem
force-pushed
the
fix/statefulset-vct-storage-mismatch
branch
from
4f667f9 to
a433044
Compare
There was a problem hiding this comment.
Pull request overview
Fixes Advanced StatefulSet reconciliation when storageMatches() detects a pod storage layout mismatch (e.g., adding a new VolumeClaimTemplate) by deleting the mismatching pod instead of attempting a forbidden in-place spec.volumes update, and adds a regression unit test for the scenario.
Changes:
- In
processReplica, intercept!storageMatches()and delete the pod to trigger recreation with the correct volumes/PVCs. - Add
TestProcessReplicaStorageMismatchto assert the controller issues a delete rather than an in-place update attempt.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| pkg/controller/statefulset/stateful_set_control.go | Deletes pods on storage mismatch to avoid immutable spec.volumes update failures. |
| pkg/controller/statefulset/stateful_set_control_test.go | Adds a unit test to validate the storage-mismatch deletion behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Delete the pod so it gets recreated; the standard creation path will handle | ||
| // creating any missing PVCs before the new pod is scheduled. | ||
| if !storageMatches(set, replicas[i]) { | ||
| if !isTerminating(replicas[i]) { |
There was a problem hiding this comment.
This storage-mismatch deletion doesn't apply the scaleMaxUnavailable budget (other branches use decreaseAndCheckMaxUnavailable(scaleMaxUnavailable) before deleting/creating). In burst/parallel mode this can delete more pods than allowed, increasing disruption. Add the same max-unavailable check before calling delete and return shouldBreak when the budget is exhausted.
| if !isTerminating(replicas[i]) { | |
| if !isTerminating(replicas[i]) { | |
| // Respect the scaleMaxUnavailable budget before deleting for storage mismatch. | |
| if decreaseAndCheckMaxUnavailable(scaleMaxUnavailable) { | |
| logger.V(4).Info( | |
| "StatefulSet pod is unavailable due to storage mismatch, and break pods scale", | |
| "statefulSet", klog.KObj(set), "pod", klog.KObj(replicas[i])) | |
| // No mutation yet (pod not deleted); signal caller to stop further scaling. | |
| return false, true, nil | |
| } |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #2388 +/- ##
==========================================
+ Coverage 48.66% 48.68% +0.02%
==========================================
Files 324 324
Lines 27920 27935 +15
==========================================
+ Hits 13587 13600 +13
- Misses 12794 12795 +1
- Partials 1539 1540 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
shivansh-gohem
force-pushed
the
fix/statefulset-vct-storage-mismatch
branch
from
a433044 to
c44b839
Compare
…update When a new VolumeClaimTemplate is added to an existing Advanced StatefulSet, processReplica falls through to UpdateStatefulPod which tries to patch spec.volumes — Kubernetes forbids this, causing a repeated Forbidden error loop (bug openkruise#2343). Fix: detect storage mismatch before the identity/retention check and call DeleteStatefulPod so the pod gets recreated on the next reconcile. The standard creation path (CreateStatefulPod) naturally handles PVC creation. Fixes openkruise#2343 Signed-off-by: Shivansh Sahu <sahushivansh142@gmail.com>
shivansh-gohem
force-pushed
the
fix/statefulset-vct-storage-mismatch
branch
from
c44b839 to
ca72080
Compare
|
@shivansh-gohem: GitHub didn't allow me to request PR reviews from the following users: technosophos. Note that only openkruise members and repo collaborators can review this PR, and authors cannot review their own PRs. DetailsIn response to this: Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
What this PR does / why we need it:
When a new
VolumeClaimTemplateis added to an existingAdvanced StatefulSet, the controller correctly detects the storage mismatch viastorageMatches(). However, it previously fell through toUpdateStatefulPod, attempting an in-place update of the pod'sspec.volumes. Becausespec.volumesis immutable on running pods in Kubernetes, the API server rejected this with aForbiddenerror, causing the controller to enter an infiniteFailedUpdateretry loop.This PR fixes the issue by intercepting the storage mismatch in
processReplica. If the storage does not match and the pod is not already terminating, the controller explicitly callsDeleteStatefulPod. By deleting the pod, we rely on the native StatefulSet reconciliation loop (CreateStatefulPod) on the next tick to automatically provision the new PVC and reconstruct the pod from scratch.Which issue(s) this PR fixes:
Fixes #2343
Special notes for the reviewer:
processReplica->CreateStatefulPod->createPersistentVolumeClaimsflow on the subsequent reconcile tick. This keeps the fix minimal and native to standard StatefulSet lifecycle management.!isTerminating(replicas[i])check to ensure the controller does not spamDELETErequests to the API server while waiting for the pod to gracefully terminate.TestProcessReplicaStorageMismatchtostateful_set_control_test.goto ensure this exact path is covered by CI.How to verify it:
go test ./pkg/controller/statefulset/... -run TestProcessReplicaStorageMismatchpasses.kindcluster:VolumeClaimTemplate.VolumeClaimTemplate.kubectl describethat the controller successfully emitsSuccessfulDelete->SuccessfulCreateevents, and both PVCs bind correctly without anyFailedUpdateerrors.