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 |
2609cb1 to
27a345c
Compare
|
This change integrates PVC (Persistent Volume Claim) autoscaling capability into the AWS Gardener extension by adding a new dependency and implementing a webhook that automatically sets default cooldown periods for PVC autoscaler resources in shoot clusters. Walkthrough
The webhook ensures that when PVC autoscaler resources are created without explicit cooldown durations, they automatically receive a 6-hour default cooldown period to prevent excessive scaling operations. Model: claude-sonnet-4-20250514 | Prompt Tokens: 4342 | Completion Tokens: 190 |
plkokanov
left a comment
There was a problem hiding this comment.
The provider-aws extension can be installed on all types of seeds (gcp, azure, etc) depending on the required DNSRecord type and the cloud provider of the hosted shoots. We have to make sure that the aws-specific PVCA mutation does not happen in such cases.
You can check how this is done here, although I'm not sure if there's a better way to do it. For instance, the seedprovider webhook has a namespace selector that only selects shoot control plane namespaces labeled with seed.gardener.cloud/provider=aws. One thing to note is that the PVCA webhook also has to select resources from the garden namespace.
| // | ||
| // SPDX-License-Identifier: Apache-2.0 | ||
|
|
||
| package shootpvca |
There was a problem hiding this comment.
This is a webhook that runs in the seed and targets resources in the garden and shoot control plane namespaces. I think having shoot in the name is confusing and should be changed also for other occurrences in these files.
| return nil, err | ||
| } | ||
|
|
||
| // This webhook should apply to all namespaces in the shoot cluster. |
There was a problem hiding this comment.
This webhook should have nothing to do with namespaces in the shoot cluster.
| for i := range pvca.Spec.VolumePolicies { | ||
| if pvca.Spec.VolumePolicies[i].ScaleUp != nil && | ||
| pvca.Spec.VolumePolicies[i].ScaleUp.CooldownDuration == nil { | ||
| pvca.Spec.VolumePolicies[i].ScaleUp.CooldownDuration = defaultCooldownDuration.DeepCopy() | ||
| } | ||
| } |
There was a problem hiding this comment.
We always have to ensure that the CooldownDuration is at least 6 hours, meaning that if it was already set to something smaller, we should overwrite it.
| "sigs.k8s.io/controller-runtime/pkg/client" | ||
| ) | ||
|
|
||
| var defaultCooldownDuration = &metav1.Duration{Duration: 6 * time.Hour} |
There was a problem hiding this comment.
Nit:
| var defaultCooldownDuration = &metav1.Duration{Duration: 6 * time.Hour} | |
| const minCooldownDuration = 6 * time.Hour |
Then something like
pvca.Spec.VolumePolicies[i].ScaleUp.CooldownDuration = &metav1.Duration{Duration: minCooldownDuration}|
@plkokanov: adding LGTM is restricted to approvers and reviewers in OWNERS files. 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-sigs/prow repository. |
|
PR needs rebase. DetailsInstructions 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-sigs/prow repository. |
How to categorize this PR?
/area storage
/kind enhancement
/platform aws
What this PR does / why we need it:
TBD
Which issue(s) this PR fixes:
Fixes #
Special notes for your reviewer:
Release note: