feat(poolautoscaler): add PoolAutoscaler CRD with capacity-based and cron scaling#625
feat(poolautoscaler): add PoolAutoscaler CRD with capacity-based and cron scaling#625chrisliu1995 wants to merge 3 commits into
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 |
…cron scaling Add PoolAutoscaler controller that automatically scales SandboxSet replicas based on claim demand. Key components: - PoolAutoscaler CRD type with capacityPolicy and cronPolicies - Capacity controller with observation window (rolling samples) and cooldown-based stabilization window for scale up/down - Configurable observation window and sampling interval via CLI flags (--observation-window-seconds, --sampling-interval-seconds) - Cron scaling policies for time-based scaling rules - Validating webhook for PoolAutoscaler spec validation - Feature gate for controller registration - Generated clientset, informers, listers, CRD manifest, RBAC, and webhook configuration The observation window averages available/status replicas over a configurable time period (default 15s) before computing desired replicas, preventing thrashing from transient fluctuations. The cooldown model uses lastScaleUpAt/lastScaleDownAt timestamps with first-action exemption for responsive initial scaling. Signed-off-by: ChrisLiu <chrisliu1995@163.com>
e1c9b82 to
07ab971
Compare
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #625 +/- ##
==========================================
+ Coverage 80.00% 80.23% +0.23%
==========================================
Files 225 230 +5
Lines 17645 18171 +526
==========================================
+ Hits 14117 14580 +463
- Misses 2953 2998 +45
- Partials 575 593 +18
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
…vert Signed-off-by: ChrisLiu <chrisliu1995@163.com>
… and coverage gaps Signed-off-by: ChrisLiu <chrisliu1995@163.com>
| // MinReplicas is the lower limit for the number of replicas to which the autoscaler | ||
| // can scale down. It defaults to 0 pods. | ||
| // +optional | ||
| MinReplicas *int32 `json:"minReplicas,omitempty"` |
There was a problem hiding this comment.
Is the pointer necessary here? The default value of 0 seems sufficient.
|
|
||
| // CronPolicies is a list of potential cron scaling policies which can be used during scaling. | ||
| // +optional | ||
| CronPolicies []CronScalingPolicy `json:"cronPolicies,omitempty"` |
There was a problem hiding this comment.
Are CronPolicies and CapacityPolicy mutually exclusive? If both are set simultaneously, would the implementation be complex and the logic unclear?
If not well thought through, I suggest not supporting both at the same time for now — this can be enforced via webhook validation. Additionally, if the controller detects both are set, it should log a warning. CronPolicies should take higher priority.
| // aggregatedValues returns the average available and statusReplicas | ||
| // from samples within the observation window, along with the sample count. | ||
| // Returns ok=false if no samples exist. | ||
| func (m *capacityMonitor) aggregatedValues() (avgAvailable, avgStatusReplicas int32, sampleCount int, ok bool) { |
There was a problem hiding this comment.
avgStatusReplicas -> avgReplicas
| var sumAvailable, sumStatus int64 | ||
| for _, s := range m.samples { | ||
| sumAvailable += int64(s.available) | ||
| sumStatus += int64(s.statusReplicas) |
There was a problem hiding this comment.
sumStatus looks odd, how about sumReplicas?
| monitor.addSampleIfDue(rawAvailable, rawStatusReplicas, now) | ||
| monitor.pruneSamples(now) | ||
|
|
||
| avgAvail, avgStatus, sampleCount, ok := monitor.aggregatedValues() |
There was a problem hiding this comment.
Consider adding debug logs here to print all samples for easier troubleshooting.
| // +kubebuilder:rbac:groups=agents.kruise.io,resources=sandboxsets/scale,verbs=get;update;patch | ||
|
|
||
| func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { | ||
| logger := logf.FromContext(ctx).WithValues("poolautoscaler", req.NamespacedName) |
| rawStatusReplicas := sbs.Status.Replicas | ||
| rawAvailable := sbs.Status.AvailableReplicas | ||
|
|
||
| avgAvailable, avgStatusReplicas := r.observeAndAggregate(ctx, pa, rawAvailable, rawStatusReplicas) |
There was a problem hiding this comment.
I think rawAvailable is not an appropriate name here — it should be specReplicas.
|
|
||
| // updateStatus updates the PoolAutoscaler status fields. | ||
| func (r *Reconciler) updateStatus(ctx context.Context, pa *agentsv1alpha1.PoolAutoscaler, currentReplicas, desiredReplicas, available int32, appliedCronPolicies []agentsv1alpha1.CronScalingPolicyStatus, suspended bool) error { | ||
| patch := client.MergeFrom(pa.DeepCopy()) |
There was a problem hiding this comment.
Check whether the status actually changed before patching, to avoid unnecessary Patch requests.
| if pa.Spec.MinReplicas != nil { | ||
| minReplicas = *pa.Spec.MinReplicas | ||
| } | ||
| if desiredReplicas == pa.Spec.MaxReplicas && desiredReplicas == minReplicas { |
There was a problem hiding this comment.
Does this mean minReplicas == maxReplicas? This seems overly complex — the webhook validation should enforce that minReplicas != maxReplicas.
| } | ||
|
|
||
| // setCondition updates or appends a condition in the PoolAutoscaler status. | ||
| func setCondition(pa *agentsv1alpha1.PoolAutoscaler, condition metav1.Condition) { |
There was a problem hiding this comment.
Implementation reference: https://github.com/openkruise/agents/blob/master/pkg/utils/utils.go#L50
Ⅰ. Describe what this PR does
Add PoolAutoscaler CRD and controller that automatically scales SandboxSet replicas based on claim demand. Key components:
api/v1alpha1/poolautoscaler_types.go): Defines spec withscaleTargetRef,minReplicas/maxReplicas,capacityPolicy(targetAvailable, tolerance, scaleUp/scaleDown stabilization), andcronPoliciesfor time-based scaling.pkg/controller/poolautoscaler/capacity.go): Implements observation window with rolling samples (averages available/statusReplicas over a configurable period) and cooldown-based stabilization window. The observation window prevents thrashing from transient fluctuations; the cooldown model useslastScaleUpAt/lastScaleDownAttimestamps with first-action exemption for responsive initial scaling.--observation-window-seconds(default 15) and--sampling-interval-seconds(default 5).pkg/controller/poolautoscaler/cron.go): Supports time-based scaling rules with timezone, start/end time, and recurrence patterns.pkg/webhook/poolautoscaler/): Validates PoolAutoscaler spec for targetRef, replica bounds, and capacity policy configuration.pkg/features/features.go):PoolAutoscalerControllergate for controller registration.Ⅱ. Does this pull request fix one issue?
NONE
Ⅲ. Describe how to verify it
--observation-window-seconds=15 --sampling-interval-seconds=5, the controller averages 3 samples before computing desired replicas.Ⅳ. Special notes for reviews
capacityMonitorstruct with a mutex-protectedsamplesslice. Samples are pruned to the window size on each reconcile. TheaggregatedValues()method returnssampleCountto avoid lock-free reads of the samples slice.lastScaleUpAt/lastScaleDownAttimestamps are set after each scale action. First scale action is exempt from cooldown (IsZero()check) for responsive initial scaling. Subsequent actions must wait for the configured stabilization window to elapse.recordScaleActiondoes NOT clear samples after scaling — the observation window continues to accumulate, providing smooth transitions.RequeueAfterfollowssamplingIntervalSeconds(default 5s) rather than a fixed interval, tying reconcile frequency to the observation granularity.