-
Notifications
You must be signed in to change notification settings - Fork 39.4k
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
StatefulSet: fix kube-controller-manager panic due to StatefulSetPersistentVolumeClaimRetentionPolicy being nil #113358
StatefulSet: fix kube-controller-manager panic due to StatefulSetPersistentVolumeClaimRetentionPolicy being nil #113358
Conversation
/sig apps |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mengjiao-liu can you add an unit test to prevent regressing in the future?
/assign @mattcary |
That looks to be all the bare references to the policy, so this should fix the update problem. I'm not sure of how to add a unit test for covering a cluster update across the feature gate being enabled. Maybe change runTestOverPVCRetentionPolicies to add a case with a nil policy? |
Okay. I've added this case that StatefulSetPersistentVolumeClaimRetentionPolicy is nil in the runTestOverPVCRetentionPolicies to test. |
90245d7
to
dcf11af
Compare
Thanks! A minor nit then I think this is good to go. |
…istentVolumeClaimRetentionPolicy being nil
dcf11af
to
b974069
Compare
/lgtm /priority now We'll need someone to improve this. |
@mattcary: The label(s) In 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. |
/priority important-now |
@mattcary: The label(s) In 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. |
/priority critical-urgent |
Please take a look.@janetkuo @soltysh @smarterclayton |
/priority important-soon |
/approve |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: mengjiao-liu, smarterclayton 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 |
/triage accepted |
What type of PR is this?
/kind bug
What this PR does / why we need it:
When we start the feature gate only in kube-controller-manager (actually need to start this feature gate in kube-apiserver too, but maybe haven't started or forgot), kube-controller-manager shouldn't cause a panic.
#113319 (comment)
Fix a bug that causes the kube-controller-manager component to panic when the value of
StatefulSetPersistentVolumeClaimRetentionPolicy
is nil.Which issue(s) this PR fixes:
Fixes #113319
Special notes for your reviewer:
Does this PR introduce a user-facing change?
Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.: