autoscaler: populate status, conditions, and observedGeneration for homogeneous and heterogeneous policies - #1470
Conversation
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
|
[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 |
|
/cc @YaoZengzeng @LiZhenCheng9527 @hzxuzhonghu @git-malu This PR resolves issue #1469 by implementing status update helpers (updateHomogeneousPolicyStatus and updateHeterogeneousPolicyStatus) for HomogeneousTarget and HeterogeneousTarget policies in AutoscaleController. It ensures .status (ObservedGeneration, Conditions, HomogeneousStatus, HeterogeneousStatus) is accurately populated and updated with conflict retry handling. |
5565073 to
acd5ed5
Compare
…omogeneous and heterogeneous policies The AutoscaleController previously only updated AutoscalingPolicy status for DisaggregatedTarget policies (via updateDisaggregatedPolicyStatus). For HomogeneousTarget and HeterogeneousTarget policies, doScale and doOptimize scaled target ModelServing instances but omitted updating the policy status, leaving status unpopulated in production. This change introduces updateHomogeneousPolicyStatus and updateHeterogeneousPolicyStatus in AutoscaleController: - Populates Status.ObservedGeneration to match the policy's generation. - Populates Status.Conditions (Ready and TargetFound) to reflect reconciliation outcome and target resolution. - Populates Status.HomogeneousStatus (CurrentReplicas, DesiredReplicas, Mode, LastScaleTime) for HomogeneousTarget policies. - Populates Status.HeterogeneousStatus (per-target CurrentReplicas, DesiredReplicas, Mode, LastScaleTime) for HeterogeneousTarget policies. - Uses retry.RetryOnConflict with a fallback read strategy to prevent status update conflicts. Unit tests in autoscale_controller_test.go verify status generation, condition transitions, and target lookup failures for both policy types. Signed-off-by: bhuvan-somisetty <somisettybhuvan5@gmail.com>
acd5ed5 to
fe05072
Compare
…omogeneous and heterogeneous policies The AutoscaleController previously only updated AutoscalingPolicy status for DisaggregatedTarget policies (via updateDisaggregatedPolicyStatus). For HomogeneousTarget and HeterogeneousTarget policies, doScale and doOptimize scaled target ModelServing instances but omitted updating the policy status, leaving status unpopulated in production. This change introduces updateHomogeneousPolicyStatus and updateHeterogeneousPolicyStatus in AutoscaleController: - Populates Status.ObservedGeneration to match the policy's generation. - Populates Status.Conditions (Ready and TargetFound) to reflect reconciliation outcome and target resolution. - Populates Status.HomogeneousStatus (CurrentReplicas, DesiredReplicas, Mode, LastScaleTime) for HomogeneousTarget policies. - Populates Status.HeterogeneousStatus (per-target CurrentReplicas, DesiredReplicas, Mode, LastScaleTime) for HeterogeneousTarget policies. - Uses retry.RetryOnConflict with a fallback read strategy to prevent status update conflicts. Unit tests in autoscale_controller_test.go verify status generation, condition transitions, and target lookup failures for both policy types. Signed-off-by: bhuvan-somisetty <somisettybhuvan5@gmail.com>
|
cc @YaoZengzeng @LiZhenCheng9527 @hzxuzhonghu @git-malu @FAUST-BENCHOU Hello maintainers, when you get a chance please take a look at this PR. It populates status fields (ObservedGeneration, Conditions, HomogeneousStatus, and HeterogeneousStatus) for homogeneous and heterogeneous autoscaling policies so their status is no longer left empty in production. All tests are passing. |
|
hi @LiZhenCheng9527 PTAL , whenever u get chance |
What type of PR is this?
/kind bug
What this PR does / why we need it:
AutoscaleControllerpreviously only updatedAutoscalingPolicycustom resource status forDisaggregatedTargetpolicies (viaupdateDisaggregatedPolicyStatus). ForHomogeneousTargetandHeterogeneousTargetpolicies,doScaleanddoOptimizeexecuted the replica scaling patches onModelServinginstances but omitted updating.statuson theAutoscalingPolicycustom resource entirely.As a result,
AutoscalingPolicyresources using homogeneous or heterogeneous scaling targets remained with empty status blocks (status: {}), leavingobservedGeneration,conditions(Ready,TargetFound),homogeneousStatus, andheterogeneousStatusunpopulated.This PR adds status update helpers and integrates them into
doScaleanddoOptimize:updateHomogeneousPolicyStatus: WritesObservedGeneration,Conditions(Ready,TargetFound), andHomogeneousStatus(current/desired replicas, mode, last scale time).updateHeterogeneousPolicyStatus: WritesObservedGeneration,Conditions(Ready,TargetFound), andHeterogeneousStatus(per-target current/desired replicas, mode, last scale time).retry.RetryOnConflictwith a fallback API read strategy to handle status update conflicts cleanly.Which issue(s) this PR fixes:
Fixes #1469
Bug evidence (required for bug-related PRs):
Source path inspection:
doScaleinpkg/autoscaler/controller/autoscale_controller.goscaled target replicas but had no status update call.doOptimizeinpkg/autoscaler/controller/autoscale_controller.goscaled target replicas across heterogeneous backends but had no status update call.AutoscalingPolicyStatusAPI definition inpkg/apis/workload/v1alpha1/autoscalingpolicy_types.goexplicitly definesHomogeneousStatusandHeterogeneousStatus, but no corresponding status reconciliation functions existed inAutoscaleController.Reproduction:
AutoscalingPolicywith ahomogeneousTargetorheterogeneousTarget.AutoscaleControllerreconciles and scales targetModelServinginstances.kubectl get asp <name> -o yaml—.statusis completely empty.Special notes for your reviewer:
pkg/autoscaler/controller/autoscale_controller_test.go:TestUpdateHomogeneousPolicyStatus_SuccessTestUpdateHomogeneousPolicyStatus_TargetNotFoundTestUpdateHeterogeneousPolicyStatus_SuccessTestUpdateHeterogeneousPolicyStatus_TargetNotFoundpkg/autoscaler/controllerpass cleanly.Does this PR introduce a user-facing change?: