From 23467747bddf189001c41e2d4fe9a518f256ee37 Mon Sep 17 00:00:00 2001 From: Jennifer Chen Date: Fri, 1 Nov 2024 17:28:49 -0400 Subject: [PATCH 01/16] Add annotations and local recommendations to pod autoscaler internal --- .../workload/model/pod_autoscaler.go | 46 ++++++++++++++++++- 1 file changed, 44 insertions(+), 2 deletions(-) diff --git a/pkg/clusteragent/autoscaling/workload/model/pod_autoscaler.go b/pkg/clusteragent/autoscaling/workload/model/pod_autoscaler.go index 88a80f958093b..321a7840b5f37 100644 --- a/pkg/clusteragent/autoscaling/workload/model/pod_autoscaler.go +++ b/pkg/clusteragent/autoscaling/workload/model/pod_autoscaler.go @@ -38,6 +38,9 @@ type PodAutoscalerInternal struct { // name is the name of the PodAutoscaler name string + // annotations are the annotations of the PodAutoscaler + annotations map[string]string + // creationTimestamp is the time when the kubernetes object was created // creationTimestamp is stored in .DatadogPodAutoscaler.CreationTimestamp creationTimestamp time.Time @@ -72,6 +75,9 @@ type PodAutoscalerInternal struct { // verticalLastActionError is the last error encountered on vertical scaling verticalLastActionError error + // localLastRecommendations is a record of the past local recommendations + localLastRecommendations []HorizontalScalingValues + // currentReplicas is the current number of PODs for the targetRef currentReplicas *int32 @@ -100,8 +106,9 @@ type PodAutoscalerInternal struct { // NewPodAutoscalerInternal creates a new PodAutoscalerInternal from a Kubernetes CR func NewPodAutoscalerInternal(podAutoscaler *datadoghq.DatadogPodAutoscaler) PodAutoscalerInternal { pai := PodAutoscalerInternal{ - namespace: podAutoscaler.Namespace, - name: podAutoscaler.Name, + namespace: podAutoscaler.Namespace, + name: podAutoscaler.Name, + annotations: podAutoscaler.Annotations, } pai.UpdateFromPodAutoscaler(podAutoscaler) pai.UpdateFromStatus(&podAutoscaler.Status) @@ -128,6 +135,7 @@ func NewPodAutoscalerFromSettings(ns, name string, podAutoscalerSpec *datadoghq. func (p *PodAutoscalerInternal) UpdateFromPodAutoscaler(podAutoscaler *datadoghq.DatadogPodAutoscaler) { p.creationTimestamp = podAutoscaler.CreationTimestamp.Time p.generation = podAutoscaler.Generation + p.annotations = podAutoscaler.Annotations p.spec = podAutoscaler.Spec.DeepCopy() // Reset the target GVK as it might have changed // Resolving the target GVK is done in the controller sync to ensure proper sync and error handling @@ -295,6 +303,11 @@ func (p *PodAutoscalerInternal) Name() string { return p.name } +// Annotation returns the annotations on the PodAutoscaler +func (p *PodAutoscalerInternal) Annotations() map[string]string { + return p.annotations +} + // ID returns the functional identifier of the PodAutoscaler func (p *PodAutoscalerInternal) ID() string { return p.namespace + "/" + p.name @@ -345,6 +358,11 @@ func (p *PodAutoscalerInternal) VerticalLastActionError() error { return p.verticalLastActionError } +// LocalLastRecommendations returns the last local recommendations +func (p *PodAutoscalerInternal) LocalLastRecommendations() []HorizontalScalingValues { + return p.localLastRecommendations +} + // CurrentReplicas returns the current number of PODs for the targetRef func (p *PodAutoscalerInternal) CurrentReplicas() *int32 { return p.currentReplicas @@ -542,6 +560,30 @@ func addHorizontalAction(currentTime time.Time, retention time.Duration, actions return actions } +func addHorizontalRecommendation(currentTime time.Time, retention time.Duration, recommendations []HorizontalScalingValues, recommendation *HorizontalScalingValues) []HorizontalScalingValues { + if retention == 0 { + recommendations = recommendations[:0] + recommendations = append(recommendations, *recommendation) + return recommendations + } + + // Find oldest event index to keep + cutoffTime := currentTime.Add(-retention) + cutoffIndex := 0 + for i, recommendation := range recommendations { + // The first event after the cutoff time is the oldest event to keep + if recommendation.Timestamp.After(cutoffTime) { + cutoffIndex = i + break + } + } + + // We are basically removing space from the array until we reallocate + recommendations = recommendations[cutoffIndex:] + recommendations = append(recommendations, *recommendation) + return recommendations +} + func newConditionFromError(trueOnError bool, currentTime metav1.Time, err error, conditionType datadoghq.DatadogPodAutoscalerConditionType, existingConditions map[datadoghq.DatadogPodAutoscalerConditionType]*datadoghq.DatadogPodAutoscalerCondition) datadoghq.DatadogPodAutoscalerCondition { var condition corev1.ConditionStatus From bf4505181e13a732f66c9eb9290db617d5443b07 Mon Sep 17 00:00:00 2001 From: Jennifer Chen Date: Mon, 4 Nov 2024 17:09:00 -0500 Subject: [PATCH 02/16] Extend scaling values with local recommendations --- .../workload/model/pod_autoscaler.go | 14 +++++++----- .../autoscaling/workload/model/rc_schema.go | 22 +++++++++++++++++++ 2 files changed, 30 insertions(+), 6 deletions(-) diff --git a/pkg/clusteragent/autoscaling/workload/model/pod_autoscaler.go b/pkg/clusteragent/autoscaling/workload/model/pod_autoscaler.go index 321a7840b5f37..34eea50c0b428 100644 --- a/pkg/clusteragent/autoscaling/workload/model/pod_autoscaler.go +++ b/pkg/clusteragent/autoscaling/workload/model/pod_autoscaler.go @@ -158,7 +158,14 @@ func (p *PodAutoscalerInternal) UpdateFromSettings(podAutoscalerSpec *datadoghq. // UpdateFromValues updates the PodAutoscalerInternal from a new scaling values func (p *PodAutoscalerInternal) UpdateFromValues(scalingValues ScalingValues) { - p.scalingValues = scalingValues + p.scalingValues.HorizontalError = scalingValues.HorizontalError + p.scalingValues.Horizontal = scalingValues.Horizontal + p.scalingValues.VerticalError = scalingValues.VerticalError + p.scalingValues.Vertical = scalingValues.Vertical +} + +func (p *PodAutoscalerInternal) UpdateFromLocalValues(localScalingValues *LocalScalingValues) { + p.scalingValues.Local = localScalingValues } // RemoveValues clears autoscaling values data from the PodAutoscalerInternal as we stopped autoscaling @@ -358,11 +365,6 @@ func (p *PodAutoscalerInternal) VerticalLastActionError() error { return p.verticalLastActionError } -// LocalLastRecommendations returns the last local recommendations -func (p *PodAutoscalerInternal) LocalLastRecommendations() []HorizontalScalingValues { - return p.localLastRecommendations -} - // CurrentReplicas returns the current number of PODs for the targetRef func (p *PodAutoscalerInternal) CurrentReplicas() *int32 { return p.currentReplicas diff --git a/pkg/clusteragent/autoscaling/workload/model/rc_schema.go b/pkg/clusteragent/autoscaling/workload/model/rc_schema.go index 6c7b4bfef0139..95c0f7d49d8b6 100644 --- a/pkg/clusteragent/autoscaling/workload/model/rc_schema.go +++ b/pkg/clusteragent/autoscaling/workload/model/rc_schema.go @@ -52,6 +52,10 @@ type ScalingValues struct { VerticalError error Vertical *VerticalScalingValues + // LocalError refers to an error encountered locally while computing horizontal scaling values + LocalError error + Local *LocalScalingValues + // Error refers to a general error encountered by Datadog while computing the scaling values Error error } @@ -83,6 +87,24 @@ type VerticalScalingValues struct { ContainerResources []datadoghq.DatadogPodAutoscalerContainerResources } +// LocalScalingValues holds the local scaling values for a target +type LocalScalingValues struct { + // Source is the source of the value + Source datadoghq.DatadogPodAutoscalerValueSource + + // Timestamp is the time at which the data was generated + Timestamp time.Time + + // Replicas is the desired number of replicas for the target + Replicas int32 + + // LowerBoundReplicas is the number of replicas based on lowerBound input + LowerBoundReplicas int32 + + // UpperBoundReplicas is the number of replicas based on upperBound input + UpperBoundReplicas int32 +} + // SumCPUMemoryRequests sums the CPU and memory requests of all containers func (v *VerticalScalingValues) SumCPUMemoryRequests() (cpu, memory resource.Quantity) { for _, container := range v.ContainerResources { From c54a91ed1677cccc9e1320b7f1fd593f76cd4a81 Mon Sep 17 00:00:00 2001 From: Jennifer Chen Date: Mon, 4 Nov 2024 17:40:13 -0500 Subject: [PATCH 03/16] Track previous horizontal recommendations --- pkg/clusteragent/autoscaling/workload/model/pod_autoscaler.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/clusteragent/autoscaling/workload/model/pod_autoscaler.go b/pkg/clusteragent/autoscaling/workload/model/pod_autoscaler.go index 34eea50c0b428..f087573cf05cd 100644 --- a/pkg/clusteragent/autoscaling/workload/model/pod_autoscaler.go +++ b/pkg/clusteragent/autoscaling/workload/model/pod_autoscaler.go @@ -75,8 +75,8 @@ type PodAutoscalerInternal struct { // verticalLastActionError is the last error encountered on vertical scaling verticalLastActionError error - // localLastRecommendations is a record of the past local recommendations - localLastRecommendations []HorizontalScalingValues + // horizontalLastRecommendations is a record of the past horizontal recommendations + horizontalLastRecommendations []HorizontalScalingValues // currentReplicas is the current number of PODs for the targetRef currentReplicas *int32 From 0ca33babe48ac33fd6d9523dfc5db8418014aea6 Mon Sep 17 00:00:00 2001 From: Jennifer Chen Date: Tue, 5 Nov 2024 15:54:54 +0000 Subject: [PATCH 04/16] Fix linter errors --- .../autoscaling/workload/model/pod_autoscaler.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/pkg/clusteragent/autoscaling/workload/model/pod_autoscaler.go b/pkg/clusteragent/autoscaling/workload/model/pod_autoscaler.go index f087573cf05cd..601b6cf397c3e 100644 --- a/pkg/clusteragent/autoscaling/workload/model/pod_autoscaler.go +++ b/pkg/clusteragent/autoscaling/workload/model/pod_autoscaler.go @@ -76,6 +76,7 @@ type PodAutoscalerInternal struct { verticalLastActionError error // horizontalLastRecommendations is a record of the past horizontal recommendations + // nolint: unused horizontalLastRecommendations []HorizontalScalingValues // currentReplicas is the current number of PODs for the targetRef @@ -164,6 +165,7 @@ func (p *PodAutoscalerInternal) UpdateFromValues(scalingValues ScalingValues) { p.scalingValues.Vertical = scalingValues.Vertical } +// UpdateFromLocalValues updates the PodAutoscalerInternal from new local scaling values func (p *PodAutoscalerInternal) UpdateFromLocalValues(localScalingValues *LocalScalingValues) { p.scalingValues.Local = localScalingValues } @@ -310,7 +312,7 @@ func (p *PodAutoscalerInternal) Name() string { return p.name } -// Annotation returns the annotations on the PodAutoscaler +// Annotations returns the annotations on the PodAutoscaler func (p *PodAutoscalerInternal) Annotations() map[string]string { return p.annotations } @@ -562,6 +564,7 @@ func addHorizontalAction(currentTime time.Time, retention time.Duration, actions return actions } +// nolint: unused func addHorizontalRecommendation(currentTime time.Time, retention time.Duration, recommendations []HorizontalScalingValues, recommendation *HorizontalScalingValues) []HorizontalScalingValues { if retention == 0 { recommendations = recommendations[:0] From 2b491acdb5446de602d74f3c438cb2376b3b0325 Mon Sep 17 00:00:00 2001 From: Jennifer Chen Date: Tue, 5 Nov 2024 19:38:10 +0000 Subject: [PATCH 05/16] fixup! Track previous horizontal recommendations --- .../workload/model/pod_autoscaler.go | 29 ------------------- 1 file changed, 29 deletions(-) diff --git a/pkg/clusteragent/autoscaling/workload/model/pod_autoscaler.go b/pkg/clusteragent/autoscaling/workload/model/pod_autoscaler.go index 601b6cf397c3e..e9f114f40da73 100644 --- a/pkg/clusteragent/autoscaling/workload/model/pod_autoscaler.go +++ b/pkg/clusteragent/autoscaling/workload/model/pod_autoscaler.go @@ -75,10 +75,6 @@ type PodAutoscalerInternal struct { // verticalLastActionError is the last error encountered on vertical scaling verticalLastActionError error - // horizontalLastRecommendations is a record of the past horizontal recommendations - // nolint: unused - horizontalLastRecommendations []HorizontalScalingValues - // currentReplicas is the current number of PODs for the targetRef currentReplicas *int32 @@ -564,31 +560,6 @@ func addHorizontalAction(currentTime time.Time, retention time.Duration, actions return actions } -// nolint: unused -func addHorizontalRecommendation(currentTime time.Time, retention time.Duration, recommendations []HorizontalScalingValues, recommendation *HorizontalScalingValues) []HorizontalScalingValues { - if retention == 0 { - recommendations = recommendations[:0] - recommendations = append(recommendations, *recommendation) - return recommendations - } - - // Find oldest event index to keep - cutoffTime := currentTime.Add(-retention) - cutoffIndex := 0 - for i, recommendation := range recommendations { - // The first event after the cutoff time is the oldest event to keep - if recommendation.Timestamp.After(cutoffTime) { - cutoffIndex = i - break - } - } - - // We are basically removing space from the array until we reallocate - recommendations = recommendations[cutoffIndex:] - recommendations = append(recommendations, *recommendation) - return recommendations -} - func newConditionFromError(trueOnError bool, currentTime metav1.Time, err error, conditionType datadoghq.DatadogPodAutoscalerConditionType, existingConditions map[datadoghq.DatadogPodAutoscalerConditionType]*datadoghq.DatadogPodAutoscalerCondition) datadoghq.DatadogPodAutoscalerCondition { var condition corev1.ConditionStatus From 1957a34c736e6247f031a8cd0759a9a21e428bf2 Mon Sep 17 00:00:00 2001 From: Jennifer Chen Date: Tue, 5 Nov 2024 20:55:31 +0000 Subject: [PATCH 06/16] Define bool to determine when local fallback active --- .../workload/model/pod_autoscaler.go | 80 ++++++++++++------ .../autoscaling/workload/model/rc_schema.go | 84 ------------------- .../workload/model/recommendations.go | 74 ++++++++++++++++ 3 files changed, 128 insertions(+), 110 deletions(-) create mode 100644 pkg/clusteragent/autoscaling/workload/model/recommendations.go diff --git a/pkg/clusteragent/autoscaling/workload/model/pod_autoscaler.go b/pkg/clusteragent/autoscaling/workload/model/pod_autoscaler.go index e9f114f40da73..86f823176d768 100644 --- a/pkg/clusteragent/autoscaling/workload/model/pod_autoscaler.go +++ b/pkg/clusteragent/autoscaling/workload/model/pod_autoscaler.go @@ -56,9 +56,15 @@ type PodAutoscalerInternal struct { // (only if owner == remote) settingsTimestamp time.Time - // scalingValues represents the current target scaling values (retrieved from RC) + // isLocalFallbackActive is true if the PodAutoscaler is using local fallback + isLocalFallbackActive bool + + // scalingValues represents the main scaling values scalingValues ScalingValues + // localScalingValues represents the local scaling values + localScalingValues ScalingValues + // horizontalLastActions is the last horizontal action successfully taken horizontalLastActions []datadoghq.DatadogPodAutoscalerHorizontalAction @@ -155,15 +161,13 @@ func (p *PodAutoscalerInternal) UpdateFromSettings(podAutoscalerSpec *datadoghq. // UpdateFromValues updates the PodAutoscalerInternal from a new scaling values func (p *PodAutoscalerInternal) UpdateFromValues(scalingValues ScalingValues) { - p.scalingValues.HorizontalError = scalingValues.HorizontalError - p.scalingValues.Horizontal = scalingValues.Horizontal - p.scalingValues.VerticalError = scalingValues.VerticalError - p.scalingValues.Vertical = scalingValues.Vertical + p.scalingValues = scalingValues } // UpdateFromLocalValues updates the PodAutoscalerInternal from new local scaling values -func (p *PodAutoscalerInternal) UpdateFromLocalValues(localScalingValues *LocalScalingValues) { - p.scalingValues.Local = localScalingValues +// nolint:unused +func (p *PodAutoscalerInternal) UpdateFromLocalValues(localScalingValues ScalingValues) { + p.localScalingValues = localScalingValues } // RemoveValues clears autoscaling values data from the PodAutoscalerInternal as we stopped autoscaling @@ -171,6 +175,12 @@ func (p *PodAutoscalerInternal) RemoveValues() { p.scalingValues = ScalingValues{} } +// RemoveLocalValues clears local autoscaling values data from the PodAutoscalerInternal as we stopped autoscaling +// nolint:unused +func (p *PodAutoscalerInternal) RemoveLocalValues() { + p.localScalingValues = ScalingValues{} +} + // UpdateFromHorizontalAction updates the PodAutoscalerInternal from a new horizontal action func (p *PodAutoscalerInternal) UpdateFromHorizontalAction(action *datadoghq.DatadogPodAutoscalerHorizontalAction, err error) { if err != nil { @@ -236,13 +246,16 @@ func (p *PodAutoscalerInternal) SetDeleted() { // UpdateFromStatus updates the PodAutoscalerInternal from an existing status. // It assumes the PodAutoscalerInternal is empty so it's not emptying existing data. func (p *PodAutoscalerInternal) UpdateFromStatus(status *datadoghq.DatadogPodAutoscalerStatus) { + activeScalingValues := p.getActiveScalingValues() + if status.Horizontal != nil { if status.Horizontal.Target != nil { - p.scalingValues.Horizontal = &HorizontalScalingValues{ + horizontalScalingValues := &HorizontalScalingValues{ Source: status.Horizontal.Target.Source, Timestamp: status.Horizontal.Target.GeneratedAt.Time, Replicas: status.Horizontal.Target.Replicas, } + activeScalingValues.Horizontal = horizontalScalingValues } if len(status.Horizontal.LastActions) > 0 { @@ -252,12 +265,13 @@ func (p *PodAutoscalerInternal) UpdateFromStatus(status *datadoghq.DatadogPodAut if status.Vertical != nil { if status.Vertical.Target != nil { - p.scalingValues.Vertical = &VerticalScalingValues{ + verticalScalingValues := &VerticalScalingValues{ Source: status.Vertical.Target.Source, Timestamp: status.Vertical.Target.GeneratedAt.Time, ContainerResources: status.Vertical.Target.DesiredResources, ResourcesHash: status.Vertical.Target.Version, } + activeScalingValues.Vertical = verticalScalingValues } p.verticalLastAction = status.Vertical.LastAction @@ -276,13 +290,13 @@ func (p *PodAutoscalerInternal) UpdateFromStatus(status *datadoghq.DatadogPodAut // We're restoring this to error as it's the most generic p.error = errors.New(cond.Reason) case cond.Type == datadoghq.DatadogPodAutoscalerHorizontalAbleToRecommendCondition && cond.Status == corev1.ConditionFalse: - p.scalingValues.HorizontalError = errors.New(cond.Reason) + activeScalingValues.HorizontalError = errors.New(cond.Reason) case cond.Type == datadoghq.DatadogPodAutoscalerHorizontalAbleToScaleCondition && cond.Status == corev1.ConditionFalse: p.horizontalLastActionError = errors.New(cond.Reason) case cond.Type == datadoghq.DatadogPodAutoscalerHorizontalScalingLimitedCondition && cond.Status == corev1.ConditionTrue: p.horizontalLastLimitReason = cond.Reason case cond.Type == datadoghq.DatadogPodAutoscalerVerticalAbleToRecommendCondition && cond.Status == corev1.ConditionFalse: - p.scalingValues.VerticalError = errors.New(cond.Reason) + activeScalingValues.VerticalError = errors.New(cond.Reason) case cond.Type == datadoghq.DatadogPodAutoscalerVerticalAbleToApply && cond.Status == corev1.ConditionFalse: p.verticalLastActionError = errors.New(cond.Reason) } @@ -343,6 +357,11 @@ func (p *PodAutoscalerInternal) ScalingValues() ScalingValues { return p.scalingValues } +// LocalScalingValues returns the local scaling values of the PodAutoscaler +func (p *PodAutoscalerInternal) LocalScalingValues() ScalingValues { + return p.localScalingValues +} + // HorizontalLastActions returns the last horizontal actions taken func (p *PodAutoscalerInternal) HorizontalLastActions() []datadoghq.DatadogPodAutoscalerHorizontalAction { return p.horizontalLastActions @@ -415,13 +434,15 @@ func (p *PodAutoscalerInternal) BuildStatus(currentTime metav1.Time, currentStat status.CurrentReplicas = p.currentReplicas } + activeScalingValues := p.getActiveScalingValues() + // Produce Horizontal status only if we have a desired number of replicas - if p.scalingValues.Horizontal != nil { + if activeScalingValues.Horizontal != nil { status.Horizontal = &datadoghq.DatadogPodAutoscalerHorizontalStatus{ Target: &datadoghq.DatadogPodAutoscalerHorizontalTargetStatus{ - Source: p.scalingValues.Horizontal.Source, - GeneratedAt: metav1.NewTime(p.scalingValues.Horizontal.Timestamp), - Replicas: p.scalingValues.Horizontal.Replicas, + Source: activeScalingValues.Horizontal.Source, + GeneratedAt: metav1.NewTime(activeScalingValues.Horizontal.Timestamp), + Replicas: activeScalingValues.Horizontal.Replicas, }, } @@ -436,15 +457,15 @@ func (p *PodAutoscalerInternal) BuildStatus(currentTime metav1.Time, currentStat } // Produce Vertical status only if we have a desired container resources - if p.scalingValues.Vertical != nil { - cpuReqSum, memReqSum := p.scalingValues.Vertical.SumCPUMemoryRequests() + if activeScalingValues.Vertical != nil { + cpuReqSum, memReqSum := activeScalingValues.Vertical.SumCPUMemoryRequests() status.Vertical = &datadoghq.DatadogPodAutoscalerVerticalStatus{ Target: &datadoghq.DatadogPodAutoscalerVerticalTargetStatus{ - Source: p.scalingValues.Vertical.Source, - GeneratedAt: metav1.NewTime(p.scalingValues.Vertical.Timestamp), - Version: p.scalingValues.Vertical.ResourcesHash, - DesiredResources: p.scalingValues.Vertical.ContainerResources, + Source: activeScalingValues.Vertical.Source, + GeneratedAt: metav1.NewTime(activeScalingValues.Vertical.Timestamp), + Version: activeScalingValues.Vertical.ResourcesHash, + DesiredResources: activeScalingValues.Vertical.ContainerResources, Scaled: p.scaledReplicas, PODCPURequest: cpuReqSum, PODMemoryRequest: memReqSum, @@ -476,7 +497,7 @@ func (p *PodAutoscalerInternal) BuildStatus(currentTime metav1.Time, currentStat // Building global error condition globalError := p.error if p.error == nil { - globalError = p.scalingValues.Error + globalError = activeScalingValues.Error } status.Conditions = append(status.Conditions, newConditionFromError(true, currentTime, globalError, datadoghq.DatadogPodAutoscalerErrorCondition, existingConditions)) @@ -489,16 +510,16 @@ func (p *PodAutoscalerInternal) BuildStatus(currentTime metav1.Time, currentStat // Building errors related to compute recommendations var horizontalAbleToRecommend datadoghq.DatadogPodAutoscalerCondition - if p.scalingValues.HorizontalError != nil || p.scalingValues.Horizontal != nil { - horizontalAbleToRecommend = newConditionFromError(false, currentTime, p.scalingValues.HorizontalError, datadoghq.DatadogPodAutoscalerHorizontalAbleToRecommendCondition, existingConditions) + if activeScalingValues.HorizontalError != nil || activeScalingValues.Horizontal != nil { + horizontalAbleToRecommend = newConditionFromError(false, currentTime, activeScalingValues.HorizontalError, datadoghq.DatadogPodAutoscalerHorizontalAbleToRecommendCondition, existingConditions) } else { horizontalAbleToRecommend = newCondition(corev1.ConditionUnknown, "", currentTime, datadoghq.DatadogPodAutoscalerHorizontalAbleToRecommendCondition, existingConditions) } status.Conditions = append(status.Conditions, horizontalAbleToRecommend) var verticalAbleToRecommend datadoghq.DatadogPodAutoscalerCondition - if p.scalingValues.VerticalError != nil || p.scalingValues.Vertical != nil { - verticalAbleToRecommend = newConditionFromError(false, currentTime, p.scalingValues.VerticalError, datadoghq.DatadogPodAutoscalerVerticalAbleToRecommendCondition, existingConditions) + if activeScalingValues.VerticalError != nil || activeScalingValues.Vertical != nil { + verticalAbleToRecommend = newConditionFromError(false, currentTime, activeScalingValues.VerticalError, datadoghq.DatadogPodAutoscalerVerticalAbleToRecommendCondition, existingConditions) } else { verticalAbleToRecommend = newCondition(corev1.ConditionUnknown, "", currentTime, datadoghq.DatadogPodAutoscalerVerticalAbleToRecommendCondition, existingConditions) } @@ -536,6 +557,13 @@ func (p *PodAutoscalerInternal) BuildStatus(currentTime metav1.Time, currentStat } // Private helpers +func (p *PodAutoscalerInternal) getActiveScalingValues() *ScalingValues { + if p.isLocalFallbackActive { + return &p.localScalingValues + } + return &p.scalingValues +} + func addHorizontalAction(currentTime time.Time, retention time.Duration, actions []datadoghq.DatadogPodAutoscalerHorizontalAction, action *datadoghq.DatadogPodAutoscalerHorizontalAction) []datadoghq.DatadogPodAutoscalerHorizontalAction { if retention == 0 { actions = actions[:0] diff --git a/pkg/clusteragent/autoscaling/workload/model/rc_schema.go b/pkg/clusteragent/autoscaling/workload/model/rc_schema.go index 95c0f7d49d8b6..7303833be965f 100644 --- a/pkg/clusteragent/autoscaling/workload/model/rc_schema.go +++ b/pkg/clusteragent/autoscaling/workload/model/rc_schema.go @@ -8,10 +8,6 @@ package model import ( - "time" - - "k8s.io/apimachinery/pkg/api/resource" - kubeAutoscaling "github.com/DataDog/agent-payload/v5/autoscaling/kubernetes" datadoghq "github.com/DataDog/datadog-operator/apis/datadoghq/v1alpha1" ) @@ -41,83 +37,3 @@ type AutoscalingSettings struct { // Spec is the full spec of the PodAutoscaler Spec *datadoghq.DatadogPodAutoscalerSpec `json:"spec"` } - -// ScalingValues represents the scaling values (horizontal and vertical) for a target -type ScalingValues struct { - // HorizontalError refers to an error encountered by Datadog while computing the horizontal scaling values - HorizontalError error - Horizontal *HorizontalScalingValues - - // VerticalError refers to an error encountered by Datadog while computing the vertical scaling values - VerticalError error - Vertical *VerticalScalingValues - - // LocalError refers to an error encountered locally while computing horizontal scaling values - LocalError error - Local *LocalScalingValues - - // Error refers to a general error encountered by Datadog while computing the scaling values - Error error -} - -// HorizontalScalingValues holds the horizontal scaling values for a target -type HorizontalScalingValues struct { - // Source is the source of the value - Source datadoghq.DatadogPodAutoscalerValueSource - - // Timestamp is the time at which the data was generated - Timestamp time.Time - - // Replicas is the desired number of replicas for the target - Replicas int32 -} - -// VerticalScalingValues holds the vertical scaling values for a target -type VerticalScalingValues struct { - // Source is the source of the value - Source datadoghq.DatadogPodAutoscalerValueSource - - // Timestamp is the time at which the data was generated - Timestamp time.Time - - // ResourcesHash is the hash of containerResources - ResourcesHash string - - // ContainerResources holds the resources for a container - ContainerResources []datadoghq.DatadogPodAutoscalerContainerResources -} - -// LocalScalingValues holds the local scaling values for a target -type LocalScalingValues struct { - // Source is the source of the value - Source datadoghq.DatadogPodAutoscalerValueSource - - // Timestamp is the time at which the data was generated - Timestamp time.Time - - // Replicas is the desired number of replicas for the target - Replicas int32 - - // LowerBoundReplicas is the number of replicas based on lowerBound input - LowerBoundReplicas int32 - - // UpperBoundReplicas is the number of replicas based on upperBound input - UpperBoundReplicas int32 -} - -// SumCPUMemoryRequests sums the CPU and memory requests of all containers -func (v *VerticalScalingValues) SumCPUMemoryRequests() (cpu, memory resource.Quantity) { - for _, container := range v.ContainerResources { - cpuReq := container.Requests.Cpu() - if cpuReq != nil { - cpu.Add(*cpuReq) - } - - memoryReq := container.Requests.Memory() - if memoryReq != nil { - memory.Add(*memoryReq) - } - } - - return -} diff --git a/pkg/clusteragent/autoscaling/workload/model/recommendations.go b/pkg/clusteragent/autoscaling/workload/model/recommendations.go new file mode 100644 index 0000000000000..71259007c53af --- /dev/null +++ b/pkg/clusteragent/autoscaling/workload/model/recommendations.go @@ -0,0 +1,74 @@ +// Unless explicitly stated otherwise all files in this repository are licensed +// under the Apache License Version 2.0. +// This product includes software developed at Datadog (https://www.datadoghq.com/). +// Copyright 2024-present Datadog, Inc. + +//go:build kubeapiserver + +package model + +import ( + "time" + + "k8s.io/apimachinery/pkg/api/resource" + + datadoghq "github.com/DataDog/datadog-operator/apis/datadoghq/v1alpha1" +) + +// ScalingValues represents the scaling values (horizontal and vertical) for a target +type ScalingValues struct { + // HorizontalError refers to an error encountered by Datadog while computing the horizontal scaling values + HorizontalError error + Horizontal *HorizontalScalingValues + + // VerticalError refers to an error encountered by Datadog while computing the vertical scaling values + VerticalError error + Vertical *VerticalScalingValues + + // Error refers to a general error encountered by Datadog while computing the scaling values + Error error +} + +// HorizontalScalingValues holds the horizontal scaling values for a target +type HorizontalScalingValues struct { + // Source is the source of the value + Source datadoghq.DatadogPodAutoscalerValueSource + + // Timestamp is the time at which the data was generated + Timestamp time.Time + + // Replicas is the desired number of replicas for the target + Replicas int32 +} + +// VerticalScalingValues holds the vertical scaling values for a target +type VerticalScalingValues struct { + // Source is the source of the value + Source datadoghq.DatadogPodAutoscalerValueSource + + // Timestamp is the time at which the data was generated + Timestamp time.Time + + // ResourcesHash is the hash of containerResources + ResourcesHash string + + // ContainerResources holds the resources for a container + ContainerResources []datadoghq.DatadogPodAutoscalerContainerResources +} + +// SumCPUMemoryRequests sums the CPU and memory requests of all containers +func (v *VerticalScalingValues) SumCPUMemoryRequests() (cpu, memory resource.Quantity) { + for _, container := range v.ContainerResources { + cpuReq := container.Requests.Cpu() + if cpuReq != nil { + cpu.Add(*cpuReq) + } + + memoryReq := container.Requests.Memory() + if memoryReq != nil { + memory.Add(*memoryReq) + } + } + + return +} From f1305ffcbce1153679e573160a7c7d9e6bafcd87 Mon Sep 17 00:00:00 2001 From: Jennifer Chen Date: Tue, 5 Nov 2024 21:03:56 +0000 Subject: [PATCH 07/16] Check target source prior to updating DPAI from existing status --- .../autoscaling/workload/model/pod_autoscaler.go | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/pkg/clusteragent/autoscaling/workload/model/pod_autoscaler.go b/pkg/clusteragent/autoscaling/workload/model/pod_autoscaler.go index 86f823176d768..47c9dceab60fd 100644 --- a/pkg/clusteragent/autoscaling/workload/model/pod_autoscaler.go +++ b/pkg/clusteragent/autoscaling/workload/model/pod_autoscaler.go @@ -246,10 +246,13 @@ func (p *PodAutoscalerInternal) SetDeleted() { // UpdateFromStatus updates the PodAutoscalerInternal from an existing status. // It assumes the PodAutoscalerInternal is empty so it's not emptying existing data. func (p *PodAutoscalerInternal) UpdateFromStatus(status *datadoghq.DatadogPodAutoscalerStatus) { - activeScalingValues := p.getActiveScalingValues() - + activeScalingValues := &p.scalingValues if status.Horizontal != nil { if status.Horizontal.Target != nil { + if status.Horizontal.Target.Source == "Local" { + activeScalingValues = &p.localScalingValues + } + horizontalScalingValues := &HorizontalScalingValues{ Source: status.Horizontal.Target.Source, Timestamp: status.Horizontal.Target.GeneratedAt.Time, From 3e72df57a60ac245bcab836e0d23b994b9eedf1b Mon Sep 17 00:00:00 2001 From: Jennifer Chen Date: Wed, 6 Nov 2024 13:12:03 -0500 Subject: [PATCH 08/16] Define annotation structure and parse directly --- .../autoscaling/workload/model/metadata.go | 26 ++++++++ .../workload/model/metadata_test.go | 64 +++++++++++++++++++ .../workload/model/pod_autoscaler.go | 8 +-- 3 files changed, 94 insertions(+), 4 deletions(-) create mode 100644 pkg/clusteragent/autoscaling/workload/model/metadata.go create mode 100644 pkg/clusteragent/autoscaling/workload/model/metadata_test.go diff --git a/pkg/clusteragent/autoscaling/workload/model/metadata.go b/pkg/clusteragent/autoscaling/workload/model/metadata.go new file mode 100644 index 0000000000000..a2aa0f7814791 --- /dev/null +++ b/pkg/clusteragent/autoscaling/workload/model/metadata.go @@ -0,0 +1,26 @@ +// Unless explicitly stated otherwise all files in this repository are licensed +// under the Apache License Version 2.0. +// This product includes software developed at Datadog (https://www.datadoghq.com/). +// Copyright 2024-present Datadog, Inc. + +//go:build kubeapiserver + +package model + +const ( + AnnotationsURLKey = "autoscaling.datadoghq.com/url" + AnnotationsFallbackURLKey = "autoscaling.datadoghq.com/fallback-url" +) + +// Annotations represents the relevant annotations on a DatadogPodAutoscaler object +type Annotations struct { + Endpoint string + FallbackEndpoint string +} + +func ParseAnnotations(annotations map[string]string) Annotations { + return Annotations{ + Endpoint: annotations[AnnotationsURLKey], + FallbackEndpoint: annotations[AnnotationsFallbackURLKey], + } +} diff --git a/pkg/clusteragent/autoscaling/workload/model/metadata_test.go b/pkg/clusteragent/autoscaling/workload/model/metadata_test.go new file mode 100644 index 0000000000000..f2839c9d22132 --- /dev/null +++ b/pkg/clusteragent/autoscaling/workload/model/metadata_test.go @@ -0,0 +1,64 @@ +// Unless explicitly stated otherwise all files in this repository are licensed +// under the Apache License Version 2.0. +// This product includes software developed at Datadog (https://www.datadoghq.com/). +// Copyright 2024-present Datadog, Inc. + +//go:build kubeapiserver && test + +package model + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestParseAnnotation(t *testing.T) { + tests := []struct { + name string + annotations map[string]string + expected Annotations + }{ + { + name: "Empty annotations", + annotations: map[string]string{}, + expected: Annotations{}, + }, + { + name: "URL annotation", + annotations: map[string]string{ + AnnotationsURLKey: "localhost:8080/test", + }, + expected: Annotations{ + Endpoint: "localhost:8080/test", + }, + }, + { + name: "Fallback annotation", + annotations: map[string]string{ + AnnotationsFallbackURLKey: "localhost:8080/fallback", + }, + expected: Annotations{ + FallbackEndpoint: "localhost:8080/fallback", + }, + }, + { + name: "URL and Fallback annotation", + annotations: map[string]string{ + AnnotationsURLKey: "localhost:8080/test", + AnnotationsFallbackURLKey: "localhost:8080/fallback", + }, + expected: Annotations{ + Endpoint: "localhost:8080/test", + FallbackEndpoint: "localhost:8080/fallback", + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + parsedAnnotation := ParseAnnotations(tt.annotations) + assert.Equal(t, tt.expected, parsedAnnotation) + }) + } +} diff --git a/pkg/clusteragent/autoscaling/workload/model/pod_autoscaler.go b/pkg/clusteragent/autoscaling/workload/model/pod_autoscaler.go index 47c9dceab60fd..875713e8343e7 100644 --- a/pkg/clusteragent/autoscaling/workload/model/pod_autoscaler.go +++ b/pkg/clusteragent/autoscaling/workload/model/pod_autoscaler.go @@ -39,7 +39,7 @@ type PodAutoscalerInternal struct { name string // annotations are the annotations of the PodAutoscaler - annotations map[string]string + annotations Annotations // creationTimestamp is the time when the kubernetes object was created // creationTimestamp is stored in .DatadogPodAutoscaler.CreationTimestamp @@ -111,7 +111,7 @@ func NewPodAutoscalerInternal(podAutoscaler *datadoghq.DatadogPodAutoscaler) Pod pai := PodAutoscalerInternal{ namespace: podAutoscaler.Namespace, name: podAutoscaler.Name, - annotations: podAutoscaler.Annotations, + annotations: ParseAnnotations(podAutoscaler.Annotations), } pai.UpdateFromPodAutoscaler(podAutoscaler) pai.UpdateFromStatus(&podAutoscaler.Status) @@ -138,7 +138,7 @@ func NewPodAutoscalerFromSettings(ns, name string, podAutoscalerSpec *datadoghq. func (p *PodAutoscalerInternal) UpdateFromPodAutoscaler(podAutoscaler *datadoghq.DatadogPodAutoscaler) { p.creationTimestamp = podAutoscaler.CreationTimestamp.Time p.generation = podAutoscaler.Generation - p.annotations = podAutoscaler.Annotations + p.annotations = ParseAnnotations(podAutoscaler.Annotations) p.spec = podAutoscaler.Spec.DeepCopy() // Reset the target GVK as it might have changed // Resolving the target GVK is done in the controller sync to ensure proper sync and error handling @@ -326,7 +326,7 @@ func (p *PodAutoscalerInternal) Name() string { } // Annotations returns the annotations on the PodAutoscaler -func (p *PodAutoscalerInternal) Annotations() map[string]string { +func (p *PodAutoscalerInternal) Annotations() Annotations { return p.annotations } From 70ed026fb8c9a85758874dd548876ad0bf7a1216 Mon Sep 17 00:00:00 2001 From: Jennifer Chen Date: Wed, 6 Nov 2024 13:33:23 -0500 Subject: [PATCH 09/16] Use scalingValues to represent active scaling values --- .../autoscaling/workload/model/metadata.go | 16 ++- .../workload/model/metadata_test.go | 8 +- .../workload/model/pod_autoscaler.go | 99 +++++++++---------- 3 files changed, 70 insertions(+), 53 deletions(-) diff --git a/pkg/clusteragent/autoscaling/workload/model/metadata.go b/pkg/clusteragent/autoscaling/workload/model/metadata.go index a2aa0f7814791..9c135b0004b1a 100644 --- a/pkg/clusteragent/autoscaling/workload/model/metadata.go +++ b/pkg/clusteragent/autoscaling/workload/model/metadata.go @@ -7,20 +7,34 @@ package model +import "encoding/json" + +// exported for testing purposes const ( AnnotationsURLKey = "autoscaling.datadoghq.com/url" AnnotationsFallbackURLKey = "autoscaling.datadoghq.com/fallback-url" + AnnotationsSettingsKey = "autoscaling.datadoghq.com/settings" ) // Annotations represents the relevant annotations on a DatadogPodAutoscaler object type Annotations struct { Endpoint string FallbackEndpoint string + Settings map[string]string } +// ParseAnnotations extracts the relevant autoscaling annotations from a kubernetes annotation map func ParseAnnotations(annotations map[string]string) Annotations { - return Annotations{ + annotation := Annotations{ Endpoint: annotations[AnnotationsURLKey], FallbackEndpoint: annotations[AnnotationsFallbackURLKey], } + + settings := map[string]string{} + err := json.Unmarshal([]byte(annotations[AnnotationsSettingsKey]), &settings) + if err == nil && len(settings) > 0 { + annotation.Settings = settings + } + + return annotation } diff --git a/pkg/clusteragent/autoscaling/workload/model/metadata_test.go b/pkg/clusteragent/autoscaling/workload/model/metadata_test.go index f2839c9d22132..bae520d19c64e 100644 --- a/pkg/clusteragent/autoscaling/workload/model/metadata_test.go +++ b/pkg/clusteragent/autoscaling/workload/model/metadata_test.go @@ -36,21 +36,27 @@ func TestParseAnnotation(t *testing.T) { { name: "Fallback annotation", annotations: map[string]string{ + AnnotationsURLKey: "localhost:8080/test", AnnotationsFallbackURLKey: "localhost:8080/fallback", }, expected: Annotations{ + Endpoint: "localhost:8080/test", FallbackEndpoint: "localhost:8080/fallback", }, }, { - name: "URL and Fallback annotation", + name: "Settings annotation", annotations: map[string]string{ AnnotationsURLKey: "localhost:8080/test", AnnotationsFallbackURLKey: "localhost:8080/fallback", + AnnotationsSettingsKey: `{"key": "value"}`, }, expected: Annotations{ Endpoint: "localhost:8080/test", FallbackEndpoint: "localhost:8080/fallback", + Settings: map[string]string{ + "key": "value", + }, }, }, } diff --git a/pkg/clusteragent/autoscaling/workload/model/pod_autoscaler.go b/pkg/clusteragent/autoscaling/workload/model/pod_autoscaler.go index 875713e8343e7..ab49b0c872b8e 100644 --- a/pkg/clusteragent/autoscaling/workload/model/pod_autoscaler.go +++ b/pkg/clusteragent/autoscaling/workload/model/pod_autoscaler.go @@ -56,14 +56,14 @@ type PodAutoscalerInternal struct { // (only if owner == remote) settingsTimestamp time.Time - // isLocalFallbackActive is true if the PodAutoscaler is using local fallback - isLocalFallbackActive bool - - // scalingValues represents the main scaling values + // scalingValues represents the active scaling values that should be used scalingValues ScalingValues - // localScalingValues represents the local scaling values - localScalingValues ScalingValues + // mainScalingValues represents the scaling values retrieved from the product + mainScalingValues ScalingValues + + // fallbackScalingValues represents the scaling values retrieved from the fallback + fallbackScalingValues ScalingValues // horizontalLastActions is the last horizontal action successfully taken horizontalLastActions []datadoghq.DatadogPodAutoscalerHorizontalAction @@ -159,15 +159,19 @@ func (p *PodAutoscalerInternal) UpdateFromSettings(podAutoscalerSpec *datadoghq. p.horizontalEventsRetention = getHorizontalEventsRetention(podAutoscalerSpec.Policy, longestScalingRulePeriodAllowed) } -// UpdateFromValues updates the PodAutoscalerInternal from a new scaling values +// UpdateFromValues updates the PodAutoscalerInternal scaling values func (p *PodAutoscalerInternal) UpdateFromValues(scalingValues ScalingValues) { p.scalingValues = scalingValues } +// UpdateFromMainValues updates the PodAutoscalerInternal from new main scaling values +func (p *PodAutoscalerInternal) UpdateFromMainValues(mainScalingValues ScalingValues) { + p.mainScalingValues = mainScalingValues +} + // UpdateFromLocalValues updates the PodAutoscalerInternal from new local scaling values -// nolint:unused -func (p *PodAutoscalerInternal) UpdateFromLocalValues(localScalingValues ScalingValues) { - p.localScalingValues = localScalingValues +func (p *PodAutoscalerInternal) UpdateFromLocalValues(fallbackScalingValues ScalingValues) { + p.fallbackScalingValues = fallbackScalingValues } // RemoveValues clears autoscaling values data from the PodAutoscalerInternal as we stopped autoscaling @@ -175,10 +179,14 @@ func (p *PodAutoscalerInternal) RemoveValues() { p.scalingValues = ScalingValues{} } +// RemoveMainValues clears main autoscaling values data from the PodAutoscalerInternal as we stopped autoscaling +func (p *PodAutoscalerInternal) RemoveMainValues() { + p.mainScalingValues = ScalingValues{} +} + // RemoveLocalValues clears local autoscaling values data from the PodAutoscalerInternal as we stopped autoscaling -// nolint:unused func (p *PodAutoscalerInternal) RemoveLocalValues() { - p.localScalingValues = ScalingValues{} + p.fallbackScalingValues = ScalingValues{} } // UpdateFromHorizontalAction updates the PodAutoscalerInternal from a new horizontal action @@ -246,19 +254,13 @@ func (p *PodAutoscalerInternal) SetDeleted() { // UpdateFromStatus updates the PodAutoscalerInternal from an existing status. // It assumes the PodAutoscalerInternal is empty so it's not emptying existing data. func (p *PodAutoscalerInternal) UpdateFromStatus(status *datadoghq.DatadogPodAutoscalerStatus) { - activeScalingValues := &p.scalingValues if status.Horizontal != nil { if status.Horizontal.Target != nil { - if status.Horizontal.Target.Source == "Local" { - activeScalingValues = &p.localScalingValues - } - - horizontalScalingValues := &HorizontalScalingValues{ + p.scalingValues.Horizontal = &HorizontalScalingValues{ Source: status.Horizontal.Target.Source, Timestamp: status.Horizontal.Target.GeneratedAt.Time, Replicas: status.Horizontal.Target.Replicas, } - activeScalingValues.Horizontal = horizontalScalingValues } if len(status.Horizontal.LastActions) > 0 { @@ -268,13 +270,12 @@ func (p *PodAutoscalerInternal) UpdateFromStatus(status *datadoghq.DatadogPodAut if status.Vertical != nil { if status.Vertical.Target != nil { - verticalScalingValues := &VerticalScalingValues{ + p.scalingValues.Vertical = &VerticalScalingValues{ Source: status.Vertical.Target.Source, Timestamp: status.Vertical.Target.GeneratedAt.Time, ContainerResources: status.Vertical.Target.DesiredResources, ResourcesHash: status.Vertical.Target.Version, } - activeScalingValues.Vertical = verticalScalingValues } p.verticalLastAction = status.Vertical.LastAction @@ -293,13 +294,13 @@ func (p *PodAutoscalerInternal) UpdateFromStatus(status *datadoghq.DatadogPodAut // We're restoring this to error as it's the most generic p.error = errors.New(cond.Reason) case cond.Type == datadoghq.DatadogPodAutoscalerHorizontalAbleToRecommendCondition && cond.Status == corev1.ConditionFalse: - activeScalingValues.HorizontalError = errors.New(cond.Reason) + p.scalingValues.HorizontalError = errors.New(cond.Reason) case cond.Type == datadoghq.DatadogPodAutoscalerHorizontalAbleToScaleCondition && cond.Status == corev1.ConditionFalse: p.horizontalLastActionError = errors.New(cond.Reason) case cond.Type == datadoghq.DatadogPodAutoscalerHorizontalScalingLimitedCondition && cond.Status == corev1.ConditionTrue: p.horizontalLastLimitReason = cond.Reason case cond.Type == datadoghq.DatadogPodAutoscalerVerticalAbleToRecommendCondition && cond.Status == corev1.ConditionFalse: - activeScalingValues.VerticalError = errors.New(cond.Reason) + p.scalingValues.VerticalError = errors.New(cond.Reason) case cond.Type == datadoghq.DatadogPodAutoscalerVerticalAbleToApply && cond.Status == corev1.ConditionFalse: p.verticalLastActionError = errors.New(cond.Reason) } @@ -355,14 +356,19 @@ func (p *PodAutoscalerInternal) CreationTimestamp() time.Time { return p.creationTimestamp } -// ScalingValues returns the scaling values of the PodAutoscaler +// ScalingValues returns a pointer to the active scaling values of the PodAutoscaler func (p *PodAutoscalerInternal) ScalingValues() ScalingValues { return p.scalingValues } -// LocalScalingValues returns the local scaling values of the PodAutoscaler -func (p *PodAutoscalerInternal) LocalScalingValues() ScalingValues { - return p.localScalingValues +// MainScalingValues returns the main scaling values of the PodAutoscaler +func (p *PodAutoscalerInternal) MainScalingValues() ScalingValues { + return p.mainScalingValues +} + +// FallbackScalingValues returns the fallback scaling values of the PodAutoscaler +func (p *PodAutoscalerInternal) FallbackScalingValues() ScalingValues { + return p.fallbackScalingValues } // HorizontalLastActions returns the last horizontal actions taken @@ -437,15 +443,13 @@ func (p *PodAutoscalerInternal) BuildStatus(currentTime metav1.Time, currentStat status.CurrentReplicas = p.currentReplicas } - activeScalingValues := p.getActiveScalingValues() - // Produce Horizontal status only if we have a desired number of replicas - if activeScalingValues.Horizontal != nil { + if p.scalingValues.Horizontal != nil { status.Horizontal = &datadoghq.DatadogPodAutoscalerHorizontalStatus{ Target: &datadoghq.DatadogPodAutoscalerHorizontalTargetStatus{ - Source: activeScalingValues.Horizontal.Source, - GeneratedAt: metav1.NewTime(activeScalingValues.Horizontal.Timestamp), - Replicas: activeScalingValues.Horizontal.Replicas, + Source: p.scalingValues.Horizontal.Source, + GeneratedAt: metav1.NewTime(p.scalingValues.Horizontal.Timestamp), + Replicas: p.scalingValues.Horizontal.Replicas, }, } @@ -460,15 +464,15 @@ func (p *PodAutoscalerInternal) BuildStatus(currentTime metav1.Time, currentStat } // Produce Vertical status only if we have a desired container resources - if activeScalingValues.Vertical != nil { - cpuReqSum, memReqSum := activeScalingValues.Vertical.SumCPUMemoryRequests() + if p.scalingValues.Vertical != nil { + cpuReqSum, memReqSum := p.scalingValues.Vertical.SumCPUMemoryRequests() status.Vertical = &datadoghq.DatadogPodAutoscalerVerticalStatus{ Target: &datadoghq.DatadogPodAutoscalerVerticalTargetStatus{ - Source: activeScalingValues.Vertical.Source, - GeneratedAt: metav1.NewTime(activeScalingValues.Vertical.Timestamp), - Version: activeScalingValues.Vertical.ResourcesHash, - DesiredResources: activeScalingValues.Vertical.ContainerResources, + Source: p.scalingValues.Vertical.Source, + GeneratedAt: metav1.NewTime(p.scalingValues.Vertical.Timestamp), + Version: p.scalingValues.Vertical.ResourcesHash, + DesiredResources: p.scalingValues.Vertical.ContainerResources, Scaled: p.scaledReplicas, PODCPURequest: cpuReqSum, PODMemoryRequest: memReqSum, @@ -500,7 +504,7 @@ func (p *PodAutoscalerInternal) BuildStatus(currentTime metav1.Time, currentStat // Building global error condition globalError := p.error if p.error == nil { - globalError = activeScalingValues.Error + globalError = p.scalingValues.Error } status.Conditions = append(status.Conditions, newConditionFromError(true, currentTime, globalError, datadoghq.DatadogPodAutoscalerErrorCondition, existingConditions)) @@ -513,16 +517,16 @@ func (p *PodAutoscalerInternal) BuildStatus(currentTime metav1.Time, currentStat // Building errors related to compute recommendations var horizontalAbleToRecommend datadoghq.DatadogPodAutoscalerCondition - if activeScalingValues.HorizontalError != nil || activeScalingValues.Horizontal != nil { - horizontalAbleToRecommend = newConditionFromError(false, currentTime, activeScalingValues.HorizontalError, datadoghq.DatadogPodAutoscalerHorizontalAbleToRecommendCondition, existingConditions) + if p.scalingValues.HorizontalError != nil || p.scalingValues.Horizontal != nil { + horizontalAbleToRecommend = newConditionFromError(false, currentTime, p.scalingValues.HorizontalError, datadoghq.DatadogPodAutoscalerHorizontalAbleToRecommendCondition, existingConditions) } else { horizontalAbleToRecommend = newCondition(corev1.ConditionUnknown, "", currentTime, datadoghq.DatadogPodAutoscalerHorizontalAbleToRecommendCondition, existingConditions) } status.Conditions = append(status.Conditions, horizontalAbleToRecommend) var verticalAbleToRecommend datadoghq.DatadogPodAutoscalerCondition - if activeScalingValues.VerticalError != nil || activeScalingValues.Vertical != nil { - verticalAbleToRecommend = newConditionFromError(false, currentTime, activeScalingValues.VerticalError, datadoghq.DatadogPodAutoscalerVerticalAbleToRecommendCondition, existingConditions) + if p.scalingValues.VerticalError != nil || p.scalingValues.Vertical != nil { + verticalAbleToRecommend = newConditionFromError(false, currentTime, p.scalingValues.VerticalError, datadoghq.DatadogPodAutoscalerVerticalAbleToRecommendCondition, existingConditions) } else { verticalAbleToRecommend = newCondition(corev1.ConditionUnknown, "", currentTime, datadoghq.DatadogPodAutoscalerVerticalAbleToRecommendCondition, existingConditions) } @@ -560,13 +564,6 @@ func (p *PodAutoscalerInternal) BuildStatus(currentTime metav1.Time, currentStat } // Private helpers -func (p *PodAutoscalerInternal) getActiveScalingValues() *ScalingValues { - if p.isLocalFallbackActive { - return &p.localScalingValues - } - return &p.scalingValues -} - func addHorizontalAction(currentTime time.Time, retention time.Duration, actions []datadoghq.DatadogPodAutoscalerHorizontalAction, action *datadoghq.DatadogPodAutoscalerHorizontalAction) []datadoghq.DatadogPodAutoscalerHorizontalAction { if retention == 0 { actions = actions[:0] From 3afc0595cb2f4ae44a35265007e78b3646e86480 Mon Sep 17 00:00:00 2001 From: Jennifer Chen Date: Wed, 4 Dec 2024 13:45:22 -0500 Subject: [PATCH 10/16] Update function description comments --- pkg/clusteragent/autoscaling/workload/model/pod_autoscaler.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/clusteragent/autoscaling/workload/model/pod_autoscaler.go b/pkg/clusteragent/autoscaling/workload/model/pod_autoscaler.go index ab49b0c872b8e..e93ac2be349e7 100644 --- a/pkg/clusteragent/autoscaling/workload/model/pod_autoscaler.go +++ b/pkg/clusteragent/autoscaling/workload/model/pod_autoscaler.go @@ -59,7 +59,7 @@ type PodAutoscalerInternal struct { // scalingValues represents the active scaling values that should be used scalingValues ScalingValues - // mainScalingValues represents the scaling values retrieved from the product + // mainScalingValues represents the scaling values retrieved from the main recommender (product, optionally a custom endpoint) mainScalingValues ScalingValues // fallbackScalingValues represents the scaling values retrieved from the fallback @@ -356,7 +356,7 @@ func (p *PodAutoscalerInternal) CreationTimestamp() time.Time { return p.creationTimestamp } -// ScalingValues returns a pointer to the active scaling values of the PodAutoscaler +// ScalingValues returns the scaling values of the PodAutoscaler func (p *PodAutoscalerInternal) ScalingValues() ScalingValues { return p.scalingValues } From 5757b59ccad9cc97067ce0257ae8a2463601c64f Mon Sep 17 00:00:00 2001 From: Jennifer Chen Date: Thu, 5 Dec 2024 18:47:57 +0000 Subject: [PATCH 11/16] Parse annotations to new custom recommender config type --- .../autoscaling/workload/model/metadata.go | 40 ----------- .../workload/model/metadata_test.go | 70 ------------------- .../workload/model/pod_autoscaler.go | 43 ++++++++---- .../workload/model/pod_autoscaler_test.go | 46 ++++++++++++ .../workload/model/recommendations.go | 5 ++ 5 files changed, 81 insertions(+), 123 deletions(-) delete mode 100644 pkg/clusteragent/autoscaling/workload/model/metadata.go delete mode 100644 pkg/clusteragent/autoscaling/workload/model/metadata_test.go diff --git a/pkg/clusteragent/autoscaling/workload/model/metadata.go b/pkg/clusteragent/autoscaling/workload/model/metadata.go deleted file mode 100644 index 9c135b0004b1a..0000000000000 --- a/pkg/clusteragent/autoscaling/workload/model/metadata.go +++ /dev/null @@ -1,40 +0,0 @@ -// Unless explicitly stated otherwise all files in this repository are licensed -// under the Apache License Version 2.0. -// This product includes software developed at Datadog (https://www.datadoghq.com/). -// Copyright 2024-present Datadog, Inc. - -//go:build kubeapiserver - -package model - -import "encoding/json" - -// exported for testing purposes -const ( - AnnotationsURLKey = "autoscaling.datadoghq.com/url" - AnnotationsFallbackURLKey = "autoscaling.datadoghq.com/fallback-url" - AnnotationsSettingsKey = "autoscaling.datadoghq.com/settings" -) - -// Annotations represents the relevant annotations on a DatadogPodAutoscaler object -type Annotations struct { - Endpoint string - FallbackEndpoint string - Settings map[string]string -} - -// ParseAnnotations extracts the relevant autoscaling annotations from a kubernetes annotation map -func ParseAnnotations(annotations map[string]string) Annotations { - annotation := Annotations{ - Endpoint: annotations[AnnotationsURLKey], - FallbackEndpoint: annotations[AnnotationsFallbackURLKey], - } - - settings := map[string]string{} - err := json.Unmarshal([]byte(annotations[AnnotationsSettingsKey]), &settings) - if err == nil && len(settings) > 0 { - annotation.Settings = settings - } - - return annotation -} diff --git a/pkg/clusteragent/autoscaling/workload/model/metadata_test.go b/pkg/clusteragent/autoscaling/workload/model/metadata_test.go deleted file mode 100644 index bae520d19c64e..0000000000000 --- a/pkg/clusteragent/autoscaling/workload/model/metadata_test.go +++ /dev/null @@ -1,70 +0,0 @@ -// Unless explicitly stated otherwise all files in this repository are licensed -// under the Apache License Version 2.0. -// This product includes software developed at Datadog (https://www.datadoghq.com/). -// Copyright 2024-present Datadog, Inc. - -//go:build kubeapiserver && test - -package model - -import ( - "testing" - - "github.com/stretchr/testify/assert" -) - -func TestParseAnnotation(t *testing.T) { - tests := []struct { - name string - annotations map[string]string - expected Annotations - }{ - { - name: "Empty annotations", - annotations: map[string]string{}, - expected: Annotations{}, - }, - { - name: "URL annotation", - annotations: map[string]string{ - AnnotationsURLKey: "localhost:8080/test", - }, - expected: Annotations{ - Endpoint: "localhost:8080/test", - }, - }, - { - name: "Fallback annotation", - annotations: map[string]string{ - AnnotationsURLKey: "localhost:8080/test", - AnnotationsFallbackURLKey: "localhost:8080/fallback", - }, - expected: Annotations{ - Endpoint: "localhost:8080/test", - FallbackEndpoint: "localhost:8080/fallback", - }, - }, - { - name: "Settings annotation", - annotations: map[string]string{ - AnnotationsURLKey: "localhost:8080/test", - AnnotationsFallbackURLKey: "localhost:8080/fallback", - AnnotationsSettingsKey: `{"key": "value"}`, - }, - expected: Annotations{ - Endpoint: "localhost:8080/test", - FallbackEndpoint: "localhost:8080/fallback", - Settings: map[string]string{ - "key": "value", - }, - }, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - parsedAnnotation := ParseAnnotations(tt.annotations) - assert.Equal(t, tt.expected, parsedAnnotation) - }) - } -} diff --git a/pkg/clusteragent/autoscaling/workload/model/pod_autoscaler.go b/pkg/clusteragent/autoscaling/workload/model/pod_autoscaler.go index e93ac2be349e7..cf4fd335bd11f 100644 --- a/pkg/clusteragent/autoscaling/workload/model/pod_autoscaler.go +++ b/pkg/clusteragent/autoscaling/workload/model/pod_autoscaler.go @@ -8,14 +8,17 @@ package model import ( + "encoding/json" "errors" "fmt" "slices" "time" - "github.com/DataDog/datadog-agent/pkg/util/pointer" datadoghq "github.com/DataDog/datadog-operator/apis/datadoghq/v1alpha1" + "github.com/DataDog/datadog-agent/pkg/util/log" + "github.com/DataDog/datadog-agent/pkg/util/pointer" + corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime/schema" @@ -28,6 +31,9 @@ const ( // statusRetainedActions is the number of horizontal actions kept in status statusRetainedActions = 5 + + // AnnotationsConfigurationKey is the key used to store custom recommender configuration in annotations + AnnotationsConfigurationKey = "autoscaling.datadoghq.com/custom-recommender" ) // PodAutoscalerInternal holds the necessary data to work with the `DatadogPodAutoscaler` CRD. @@ -38,9 +44,6 @@ type PodAutoscalerInternal struct { // name is the name of the PodAutoscaler name string - // annotations are the annotations of the PodAutoscaler - annotations Annotations - // creationTimestamp is the time when the kubernetes object was created // creationTimestamp is stored in .DatadogPodAutoscaler.CreationTimestamp creationTimestamp time.Time @@ -104,14 +107,18 @@ type PodAutoscalerInternal struct { // horizontalEventsRetention is the time to keep horizontal events in memory // based on scale policies horizontalEventsRetention time.Duration + + // customRecommenderConfiguration holds the configuration for custom recommenders, + // Parsed from annotations on the autoscaler + customRecommenderConfiguration *CustomRecommenderConfiguration } // NewPodAutoscalerInternal creates a new PodAutoscalerInternal from a Kubernetes CR func NewPodAutoscalerInternal(podAutoscaler *datadoghq.DatadogPodAutoscaler) PodAutoscalerInternal { pai := PodAutoscalerInternal{ - namespace: podAutoscaler.Namespace, - name: podAutoscaler.Name, - annotations: ParseAnnotations(podAutoscaler.Annotations), + namespace: podAutoscaler.Namespace, + name: podAutoscaler.Name, + customRecommenderConfiguration: parseCustomConfigurationAnnotation(podAutoscaler.Annotations), } pai.UpdateFromPodAutoscaler(podAutoscaler) pai.UpdateFromStatus(&podAutoscaler.Status) @@ -138,7 +145,7 @@ func NewPodAutoscalerFromSettings(ns, name string, podAutoscalerSpec *datadoghq. func (p *PodAutoscalerInternal) UpdateFromPodAutoscaler(podAutoscaler *datadoghq.DatadogPodAutoscaler) { p.creationTimestamp = podAutoscaler.CreationTimestamp.Time p.generation = podAutoscaler.Generation - p.annotations = ParseAnnotations(podAutoscaler.Annotations) + p.customRecommenderConfiguration = parseCustomConfigurationAnnotation(podAutoscaler.Annotations) p.spec = podAutoscaler.Spec.DeepCopy() // Reset the target GVK as it might have changed // Resolving the target GVK is done in the controller sync to ensure proper sync and error handling @@ -326,11 +333,6 @@ func (p *PodAutoscalerInternal) Name() string { return p.name } -// Annotations returns the annotations on the PodAutoscaler -func (p *PodAutoscalerInternal) Annotations() Annotations { - return p.annotations -} - // ID returns the functional identifier of the PodAutoscaler func (p *PodAutoscalerInternal) ID() string { return p.namespace + "/" + p.name @@ -430,6 +432,11 @@ func (p *PodAutoscalerInternal) TargetGVK() (schema.GroupVersionKind, error) { return p.targetGVK, nil } +// CustomRecommenderConfiguration returns the configuration set on the autoscaler for a customer recommender +func (p *PodAutoscalerInternal) CustomRecommenderConfiguration() *CustomRecommenderConfiguration { + return p.customRecommenderConfiguration +} + // // Helpers // @@ -664,3 +671,13 @@ func getLongestScalingRulesPeriod(rules []datadoghq.DatadogPodAutoscalerScalingR return longest } + +func parseCustomConfigurationAnnotation(annotations map[string]string) *CustomRecommenderConfiguration { + customConfiguration := CustomRecommenderConfiguration{} + + if err := json.Unmarshal([]byte(annotations[AnnotationsConfigurationKey]), &customConfiguration); err != nil { + log.Debugf("Failed to parse annotations for custom recommender configuration: %v", err) + } + + return &customConfiguration +} diff --git a/pkg/clusteragent/autoscaling/workload/model/pod_autoscaler_test.go b/pkg/clusteragent/autoscaling/workload/model/pod_autoscaler_test.go index 5ee7be12228cb..d4de0ee0f0ced 100644 --- a/pkg/clusteragent/autoscaling/workload/model/pod_autoscaler_test.go +++ b/pkg/clusteragent/autoscaling/workload/model/pod_autoscaler_test.go @@ -76,3 +76,49 @@ func TestAddHorizontalAction(t *testing.T) { *addedAction2, }, horizontalLastActions) } + +func TestParseCustomConfigurationAnnotation(t *testing.T) { + tests := []struct { + name string + annotations map[string]string + expected CustomRecommenderConfiguration + }{ + { + name: "Empty annotations", + annotations: map[string]string{}, + expected: CustomRecommenderConfiguration{}, + }, + { + name: "URL annotation", + annotations: map[string]string{ + AnnotationsConfigurationKey: "{\"endpoint\": \"localhost:8080/test\"}", + }, + expected: CustomRecommenderConfiguration{ + Endpoint: "localhost:8080/test", + }, + }, + { + name: "Settings annotation", + annotations: map[string]string{ + AnnotationsConfigurationKey: "{\"endpoint\": \"localhost:8080/test\", \"settings\": {\"key\": \"value\", \"number\": 1, \"bool\": true, \"array\": [1, 2, 3], \"object\": {\"key\": \"value\"}}}", + }, + expected: CustomRecommenderConfiguration{ + Endpoint: "localhost:8080/test", + Settings: map[string]any{ + "key": "value", + "number": 1.0, + "bool": true, + "array": []interface{}{1.0, 2.0, 3.0}, + "object": map[string]interface{}{"key": "value"}, + }, + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + customConfiguration := *parseCustomConfigurationAnnotation(tt.annotations) + assert.Equal(t, tt.expected, customConfiguration) + }) + } +} diff --git a/pkg/clusteragent/autoscaling/workload/model/recommendations.go b/pkg/clusteragent/autoscaling/workload/model/recommendations.go index 71259007c53af..7ec05c1441799 100644 --- a/pkg/clusteragent/autoscaling/workload/model/recommendations.go +++ b/pkg/clusteragent/autoscaling/workload/model/recommendations.go @@ -56,6 +56,11 @@ type VerticalScalingValues struct { ContainerResources []datadoghq.DatadogPodAutoscalerContainerResources } +type CustomRecommenderConfiguration struct { + Endpoint string + Settings map[string]any +} + // SumCPUMemoryRequests sums the CPU and memory requests of all containers func (v *VerticalScalingValues) SumCPUMemoryRequests() (cpu, memory resource.Quantity) { for _, container := range v.ContainerResources { From 92263a1df7708689f5a8666b4f7010416e1b2877 Mon Sep 17 00:00:00 2001 From: Jennifer Chen Date: Thu, 5 Dec 2024 18:49:47 +0000 Subject: [PATCH 12/16] Rename type CustomRecommenderConfiguration to RecommenderConfiguration --- .../autoscaling/workload/model/pod_autoscaler.go | 8 ++++---- .../autoscaling/workload/model/pod_autoscaler_test.go | 8 ++++---- .../autoscaling/workload/model/recommendations.go | 2 +- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/pkg/clusteragent/autoscaling/workload/model/pod_autoscaler.go b/pkg/clusteragent/autoscaling/workload/model/pod_autoscaler.go index cf4fd335bd11f..40b39f693e2a0 100644 --- a/pkg/clusteragent/autoscaling/workload/model/pod_autoscaler.go +++ b/pkg/clusteragent/autoscaling/workload/model/pod_autoscaler.go @@ -110,7 +110,7 @@ type PodAutoscalerInternal struct { // customRecommenderConfiguration holds the configuration for custom recommenders, // Parsed from annotations on the autoscaler - customRecommenderConfiguration *CustomRecommenderConfiguration + customRecommenderConfiguration *RecommenderConfiguration } // NewPodAutoscalerInternal creates a new PodAutoscalerInternal from a Kubernetes CR @@ -433,7 +433,7 @@ func (p *PodAutoscalerInternal) TargetGVK() (schema.GroupVersionKind, error) { } // CustomRecommenderConfiguration returns the configuration set on the autoscaler for a customer recommender -func (p *PodAutoscalerInternal) CustomRecommenderConfiguration() *CustomRecommenderConfiguration { +func (p *PodAutoscalerInternal) CustomRecommenderConfiguration() *RecommenderConfiguration { return p.customRecommenderConfiguration } @@ -672,8 +672,8 @@ func getLongestScalingRulesPeriod(rules []datadoghq.DatadogPodAutoscalerScalingR return longest } -func parseCustomConfigurationAnnotation(annotations map[string]string) *CustomRecommenderConfiguration { - customConfiguration := CustomRecommenderConfiguration{} +func parseCustomConfigurationAnnotation(annotations map[string]string) *RecommenderConfiguration { + customConfiguration := RecommenderConfiguration{} if err := json.Unmarshal([]byte(annotations[AnnotationsConfigurationKey]), &customConfiguration); err != nil { log.Debugf("Failed to parse annotations for custom recommender configuration: %v", err) diff --git a/pkg/clusteragent/autoscaling/workload/model/pod_autoscaler_test.go b/pkg/clusteragent/autoscaling/workload/model/pod_autoscaler_test.go index d4de0ee0f0ced..d183e232006d8 100644 --- a/pkg/clusteragent/autoscaling/workload/model/pod_autoscaler_test.go +++ b/pkg/clusteragent/autoscaling/workload/model/pod_autoscaler_test.go @@ -81,19 +81,19 @@ func TestParseCustomConfigurationAnnotation(t *testing.T) { tests := []struct { name string annotations map[string]string - expected CustomRecommenderConfiguration + expected RecommenderConfiguration }{ { name: "Empty annotations", annotations: map[string]string{}, - expected: CustomRecommenderConfiguration{}, + expected: RecommenderConfiguration{}, }, { name: "URL annotation", annotations: map[string]string{ AnnotationsConfigurationKey: "{\"endpoint\": \"localhost:8080/test\"}", }, - expected: CustomRecommenderConfiguration{ + expected: RecommenderConfiguration{ Endpoint: "localhost:8080/test", }, }, @@ -102,7 +102,7 @@ func TestParseCustomConfigurationAnnotation(t *testing.T) { annotations: map[string]string{ AnnotationsConfigurationKey: "{\"endpoint\": \"localhost:8080/test\", \"settings\": {\"key\": \"value\", \"number\": 1, \"bool\": true, \"array\": [1, 2, 3], \"object\": {\"key\": \"value\"}}}", }, - expected: CustomRecommenderConfiguration{ + expected: RecommenderConfiguration{ Endpoint: "localhost:8080/test", Settings: map[string]any{ "key": "value", diff --git a/pkg/clusteragent/autoscaling/workload/model/recommendations.go b/pkg/clusteragent/autoscaling/workload/model/recommendations.go index 7ec05c1441799..e0164cd9d9e08 100644 --- a/pkg/clusteragent/autoscaling/workload/model/recommendations.go +++ b/pkg/clusteragent/autoscaling/workload/model/recommendations.go @@ -56,7 +56,7 @@ type VerticalScalingValues struct { ContainerResources []datadoghq.DatadogPodAutoscalerContainerResources } -type CustomRecommenderConfiguration struct { +type RecommenderConfiguration struct { Endpoint string Settings map[string]any } From b69bcfbbfec93a9aabf84e8c6d22ba8d0a7247dd Mon Sep 17 00:00:00 2001 From: Jennifer Chen Date: Thu, 5 Dec 2024 19:33:36 +0000 Subject: [PATCH 13/16] Add recommender configuration type comment --- pkg/clusteragent/autoscaling/workload/model/recommendations.go | 1 + 1 file changed, 1 insertion(+) diff --git a/pkg/clusteragent/autoscaling/workload/model/recommendations.go b/pkg/clusteragent/autoscaling/workload/model/recommendations.go index e0164cd9d9e08..09efad941e31b 100644 --- a/pkg/clusteragent/autoscaling/workload/model/recommendations.go +++ b/pkg/clusteragent/autoscaling/workload/model/recommendations.go @@ -56,6 +56,7 @@ type VerticalScalingValues struct { ContainerResources []datadoghq.DatadogPodAutoscalerContainerResources } +// RecommenderConfiguration holds the configuration for a custom recommender type RecommenderConfiguration struct { Endpoint string Settings map[string]any From 8424eb9bc322b8372cc880bc9cdf7ce759d20df8 Mon Sep 17 00:00:00 2001 From: Jennifer Chen Date: Mon, 9 Dec 2024 09:44:08 +0000 Subject: [PATCH 14/16] fixup! Merge remote-tracking branch 'origin/main' into jenn/CASCL-102_adapt-dpai-local-fallback --- pkg/clusteragent/autoscaling/workload/model/pod_autoscaler.go | 1 - pkg/clusteragent/autoscaling/workload/model/recommendations.go | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/pkg/clusteragent/autoscaling/workload/model/pod_autoscaler.go b/pkg/clusteragent/autoscaling/workload/model/pod_autoscaler.go index 26f718fef9868..22a5f98b75642 100644 --- a/pkg/clusteragent/autoscaling/workload/model/pod_autoscaler.go +++ b/pkg/clusteragent/autoscaling/workload/model/pod_autoscaler.go @@ -14,7 +14,6 @@ import ( "slices" "time" - "github.com/DataDog/datadog-agent/pkg/util/pointer" datadoghq "github.com/DataDog/datadog-operator/api/datadoghq/v1alpha1" "github.com/DataDog/datadog-agent/pkg/util/log" diff --git a/pkg/clusteragent/autoscaling/workload/model/recommendations.go b/pkg/clusteragent/autoscaling/workload/model/recommendations.go index 09efad941e31b..5ab1057e93488 100644 --- a/pkg/clusteragent/autoscaling/workload/model/recommendations.go +++ b/pkg/clusteragent/autoscaling/workload/model/recommendations.go @@ -12,7 +12,7 @@ import ( "k8s.io/apimachinery/pkg/api/resource" - datadoghq "github.com/DataDog/datadog-operator/apis/datadoghq/v1alpha1" + datadoghq "github.com/DataDog/datadog-operator/api/datadoghq/v1alpha1" ) // ScalingValues represents the scaling values (horizontal and vertical) for a target From 767066efaf061a30867cf46c5c036ed6a680fc87 Mon Sep 17 00:00:00 2001 From: Jennifer Chen Date: Mon, 9 Dec 2024 12:46:15 +0000 Subject: [PATCH 15/16] Add custom recommender configuration to fake DPAI for tests --- .../autoscaling/workload/controller_test.go | 9 ++- .../model/pod_autoscaler_test_utils.go | 76 ++++++++++--------- 2 files changed, 44 insertions(+), 41 deletions(-) diff --git a/pkg/clusteragent/autoscaling/workload/controller_test.go b/pkg/clusteragent/autoscaling/workload/controller_test.go index e07be01363cb5..8d45825119994 100755 --- a/pkg/clusteragent/autoscaling/workload/controller_test.go +++ b/pkg/clusteragent/autoscaling/workload/controller_test.go @@ -121,10 +121,11 @@ func TestLeaderCreateDeleteLocal(t *testing.T) { // Check internal store content expectedDPAInternal := model.FakePodAutoscalerInternal{ - Namespace: "default", - Name: "dpa-0", - Generation: 1, - Spec: &dpaSpec, + Namespace: "default", + Name: "dpa-0", + Generation: 1, + Spec: &dpaSpec, + CustomRecommenderConfiguration: &model.RecommenderConfiguration{}, } dpaInternal, found := f.store.Get("default/dpa-0") assert.True(t, found) diff --git a/pkg/clusteragent/autoscaling/workload/model/pod_autoscaler_test_utils.go b/pkg/clusteragent/autoscaling/workload/model/pod_autoscaler_test_utils.go index c05b15b20a9cc..5649d62cdd93d 100644 --- a/pkg/clusteragent/autoscaling/workload/model/pod_autoscaler_test_utils.go +++ b/pkg/clusteragent/autoscaling/workload/model/pod_autoscaler_test_utils.go @@ -24,47 +24,49 @@ import ( // FakePodAutoscalerInternal is a fake PodAutoscalerInternal object. type FakePodAutoscalerInternal struct { - Namespace string - Name string - Generation int64 - Spec *datadoghq.DatadogPodAutoscalerSpec - SettingsTimestamp time.Time - CreationTimestamp time.Time - ScalingValues ScalingValues - HorizontalLastActions []datadoghq.DatadogPodAutoscalerHorizontalAction - HorizontalLastLimitReason string - HorizontalLastActionError error - HorizontalEventsRetention time.Duration - VerticalLastAction *datadoghq.DatadogPodAutoscalerVerticalAction - VerticalLastActionError error - CurrentReplicas *int32 - ScaledReplicas *int32 - Error error - Deleted bool - TargetGVK schema.GroupVersionKind + Namespace string + Name string + Generation int64 + Spec *datadoghq.DatadogPodAutoscalerSpec + SettingsTimestamp time.Time + CreationTimestamp time.Time + ScalingValues ScalingValues + HorizontalLastActions []datadoghq.DatadogPodAutoscalerHorizontalAction + HorizontalLastLimitReason string + HorizontalLastActionError error + HorizontalEventsRetention time.Duration + VerticalLastAction *datadoghq.DatadogPodAutoscalerVerticalAction + VerticalLastActionError error + CurrentReplicas *int32 + ScaledReplicas *int32 + Error error + Deleted bool + TargetGVK schema.GroupVersionKind + CustomRecommenderConfiguration *RecommenderConfiguration } // Build creates a PodAutoscalerInternal object from the FakePodAutoscalerInternal. func (f FakePodAutoscalerInternal) Build() PodAutoscalerInternal { return PodAutoscalerInternal{ - namespace: f.Namespace, - name: f.Name, - generation: f.Generation, - spec: f.Spec, - settingsTimestamp: f.SettingsTimestamp, - creationTimestamp: f.CreationTimestamp, - scalingValues: f.ScalingValues, - horizontalLastActions: f.HorizontalLastActions, - horizontalLastLimitReason: f.HorizontalLastLimitReason, - horizontalLastActionError: f.HorizontalLastActionError, - horizontalEventsRetention: f.HorizontalEventsRetention, - verticalLastAction: f.VerticalLastAction, - verticalLastActionError: f.VerticalLastActionError, - currentReplicas: f.CurrentReplicas, - scaledReplicas: f.ScaledReplicas, - error: f.Error, - deleted: f.Deleted, - targetGVK: f.TargetGVK, + namespace: f.Namespace, + name: f.Name, + generation: f.Generation, + spec: f.Spec, + settingsTimestamp: f.SettingsTimestamp, + creationTimestamp: f.CreationTimestamp, + scalingValues: f.ScalingValues, + horizontalLastActions: f.HorizontalLastActions, + horizontalLastLimitReason: f.HorizontalLastLimitReason, + horizontalLastActionError: f.HorizontalLastActionError, + horizontalEventsRetention: f.HorizontalEventsRetention, + verticalLastAction: f.VerticalLastAction, + verticalLastActionError: f.VerticalLastActionError, + currentReplicas: f.CurrentReplicas, + scaledReplicas: f.ScaledReplicas, + error: f.Error, + deleted: f.Deleted, + targetGVK: f.TargetGVK, + customRecommenderConfiguration: f.CustomRecommenderConfiguration, } } @@ -110,7 +112,7 @@ func ComparePodAutoscalers(expected any, actual any) string { if fake, ok := x.(FakePodAutoscalerInternal); ok { return fake.Build() } - panic("filer failed - unexpected type") + panic("filter failed - unexpected type") }), ), cmp.FilterValues( From 5dd8695e8e65547034ac405adc09669913ebd670 Mon Sep 17 00:00:00 2001 From: Jennifer Chen Date: Mon, 9 Dec 2024 13:43:00 +0000 Subject: [PATCH 16/16] Set custom recommender configuration once --- .../autoscaling/workload/model/pod_autoscaler.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/pkg/clusteragent/autoscaling/workload/model/pod_autoscaler.go b/pkg/clusteragent/autoscaling/workload/model/pod_autoscaler.go index 22a5f98b75642..e34ea792d107c 100644 --- a/pkg/clusteragent/autoscaling/workload/model/pod_autoscaler.go +++ b/pkg/clusteragent/autoscaling/workload/model/pod_autoscaler.go @@ -116,9 +116,8 @@ type PodAutoscalerInternal struct { // NewPodAutoscalerInternal creates a new PodAutoscalerInternal from a Kubernetes CR func NewPodAutoscalerInternal(podAutoscaler *datadoghq.DatadogPodAutoscaler) PodAutoscalerInternal { pai := PodAutoscalerInternal{ - namespace: podAutoscaler.Namespace, - name: podAutoscaler.Name, - customRecommenderConfiguration: parseCustomConfigurationAnnotation(podAutoscaler.Annotations), + namespace: podAutoscaler.Namespace, + name: podAutoscaler.Name, } pai.UpdateFromPodAutoscaler(podAutoscaler) pai.UpdateFromStatus(&podAutoscaler.Status) @@ -145,13 +144,14 @@ func NewPodAutoscalerFromSettings(ns, name string, podAutoscalerSpec *datadoghq. func (p *PodAutoscalerInternal) UpdateFromPodAutoscaler(podAutoscaler *datadoghq.DatadogPodAutoscaler) { p.creationTimestamp = podAutoscaler.CreationTimestamp.Time p.generation = podAutoscaler.Generation - p.customRecommenderConfiguration = parseCustomConfigurationAnnotation(podAutoscaler.Annotations) p.spec = podAutoscaler.Spec.DeepCopy() // Reset the target GVK as it might have changed // Resolving the target GVK is done in the controller sync to ensure proper sync and error handling p.targetGVK = schema.GroupVersionKind{} // Compute the horizontal events retention again in case .Spec.Policy has changed p.horizontalEventsRetention = getHorizontalEventsRetention(podAutoscaler.Spec.Policy, longestScalingRulePeriodAllowed) + // Compute recommender configuration again in case .Annotations has changed + p.customRecommenderConfiguration = parseCustomConfigurationAnnotation(podAutoscaler.Annotations) } // UpdateFromSettings updates the PodAutoscalerInternal from a new settings