From e3c408aca9e0cebdc7049548328fd6a31af39495 Mon Sep 17 00:00:00 2001 From: Kensei Nakada Date: Tue, 6 Feb 2024 13:43:32 +0800 Subject: [PATCH] calculate the recommendation based on a correct resource request (#313) --- .../after/tortoise.yaml | 4 +- .../after/tortoise.yaml | 8 +- .../after/tortoise.yaml | 8 +- .../after/tortoise.yaml | 8 +- .../after/tortoise.yaml | 4 +- .../after/tortoise.yaml | 8 +- .../after/tortoise.yaml | 6 +- .../before/tortoise.yaml | 6 +- .../after/tortoise.yaml | 6 +- .../after/tortoise.yaml | 4 +- .../after/tortoise.yaml | 4 +- .../after/tortoise.yaml | 6 +- .../after/tortoise.yaml | 2 +- .../after/tortoise.yaml | 2 +- .../before/tortoise.yaml | 2 +- .../after/tortoise.yaml | 2 +- .../after/tortoise.yaml | 2 +- .../after/tortoise.yaml | 2 +- .../after/tortoise.yaml | 2 +- .../after/tortoise.yaml | 2 +- controllers/tortoise_controller.go | 21 +- pkg/deployment/service.go | 1 + pkg/vpa/service.go | 15 +- pkg/vpa/service_test.go | 313 +++++++++++++++++- 24 files changed, 373 insertions(+), 65 deletions(-) diff --git a/controllers/testdata/mutable-autoscalingpolicy-add-another-horizontal/after/tortoise.yaml b/controllers/testdata/mutable-autoscalingpolicy-add-another-horizontal/after/tortoise.yaml index c2292a24..f769c311 100644 --- a/controllers/testdata/mutable-autoscalingpolicy-add-another-horizontal/after/tortoise.yaml +++ b/controllers/testdata/mutable-autoscalingpolicy-add-another-horizontal/after/tortoise.yaml @@ -21,8 +21,8 @@ status: containerResourceRequests: - containerName: "app" resource: - cpu: "10" - memory: 10Gi + cpu: "6" + memory: 3Gi - containerName: "istio-proxy" resource: cpu: "4" diff --git a/controllers/testdata/mutable-autoscalingpolicy-no-hpa-and-add-horizontal/after/tortoise.yaml b/controllers/testdata/mutable-autoscalingpolicy-no-hpa-and-add-horizontal/after/tortoise.yaml index 43192a4e..b1637996 100644 --- a/controllers/testdata/mutable-autoscalingpolicy-no-hpa-and-add-horizontal/after/tortoise.yaml +++ b/controllers/testdata/mutable-autoscalingpolicy-no-hpa-and-add-horizontal/after/tortoise.yaml @@ -21,12 +21,12 @@ status: containerResourceRequests: - containerName: "app" resource: - cpu: "10" - memory: 10Gi + cpu: "6" + memory: 3Gi - containerName: "istio-proxy" resource: - cpu: "4" - memory: 4Gi + cpu: "3" + memory: 3Gi tortoiseConditions: - message: "HPA target utilization is updated" reason: HPATargetUtilizationUpdated diff --git a/controllers/testdata/mutable-autoscalingpolicy-remove-horizontal-2/after/tortoise.yaml b/controllers/testdata/mutable-autoscalingpolicy-remove-horizontal-2/after/tortoise.yaml index 8a7cce1e..946ab225 100644 --- a/controllers/testdata/mutable-autoscalingpolicy-remove-horizontal-2/after/tortoise.yaml +++ b/controllers/testdata/mutable-autoscalingpolicy-remove-horizontal-2/after/tortoise.yaml @@ -31,12 +31,12 @@ status: containerResourceRequests: - containerName: "app" resource: - cpu: "10" - memory: 10Gi + cpu: "3" + memory: 3Gi - containerName: "istio-proxy" resource: - cpu: "4" - memory: 4Gi + cpu: "3" + memory: 3Gi containerRecommendationFromVPA: - containerName: app maxRecommendation: diff --git a/controllers/testdata/mutable-autoscalingpolicy-remove-horizontal/after/tortoise.yaml b/controllers/testdata/mutable-autoscalingpolicy-remove-horizontal/after/tortoise.yaml index e58ae326..a753ddf5 100644 --- a/controllers/testdata/mutable-autoscalingpolicy-remove-horizontal/after/tortoise.yaml +++ b/controllers/testdata/mutable-autoscalingpolicy-remove-horizontal/after/tortoise.yaml @@ -30,12 +30,12 @@ status: containerResourceRequests: - containerName: "app" resource: - cpu: "10" - memory: 10Gi + cpu: "3" + memory: 3Gi - containerName: "istio-proxy" resource: - cpu: "4" - memory: 4Gi + cpu: "3" + memory: 3Gi containerRecommendationFromVPA: - containerName: app maxRecommendation: diff --git a/controllers/testdata/reconcile-for-the-istio-enabled-pod-working/after/tortoise.yaml b/controllers/testdata/reconcile-for-the-istio-enabled-pod-working/after/tortoise.yaml index 4fd05fc4..3b4a3735 100644 --- a/controllers/testdata/reconcile-for-the-istio-enabled-pod-working/after/tortoise.yaml +++ b/controllers/testdata/reconcile-for-the-istio-enabled-pod-working/after/tortoise.yaml @@ -22,11 +22,11 @@ status: - containerName: "app" resource: cpu: "10" - memory: 10Gi + memory: 3Gi - containerName: "istio-proxy" resource: cpu: "4" - memory: 4Gi + memory: 3Gi tortoiseConditions: - message: "HPA target utilization is updated" reason: HPATargetUtilizationUpdated diff --git a/controllers/testdata/reconcile-for-the-multiple-containers-pod-all-vertical/after/tortoise.yaml b/controllers/testdata/reconcile-for-the-multiple-containers-pod-all-vertical/after/tortoise.yaml index 2e5ff66b..6bc6106d 100644 --- a/controllers/testdata/reconcile-for-the-multiple-containers-pod-all-vertical/after/tortoise.yaml +++ b/controllers/testdata/reconcile-for-the-multiple-containers-pod-all-vertical/after/tortoise.yaml @@ -21,12 +21,12 @@ status: containerResourceRequests: - containerName: "app" resource: - cpu: "10" - memory: 10Gi + cpu: "3" + memory: 3Gi - containerName: "istio-proxy" resource: - cpu: "4" - memory: 4Gi + cpu: "3" + memory: 3Gi containerRecommendationFromVPA: - containerName: app maxRecommendation: diff --git a/controllers/testdata/reconcile-for-the-multiple-containers-pod-during-emergency/after/tortoise.yaml b/controllers/testdata/reconcile-for-the-multiple-containers-pod-during-emergency/after/tortoise.yaml index c75e02f6..81260ee3 100644 --- a/controllers/testdata/reconcile-for-the-multiple-containers-pod-during-emergency/after/tortoise.yaml +++ b/controllers/testdata/reconcile-for-the-multiple-containers-pod-during-emergency/after/tortoise.yaml @@ -22,12 +22,12 @@ status: containerResourceRequests: - containerName: "app" resource: - cpu: "10" - memory: 10Gi + cpu: "6" + memory: 3Gi - containerName: "istio-proxy" resource: cpu: "4" - memory: 4Gi + memory: 3Gi tortoiseConditions: - message: "HPA target utilization is updated" reason: HPATargetUtilizationUpdated diff --git a/controllers/testdata/reconcile-for-the-multiple-containers-pod-during-emergency/before/tortoise.yaml b/controllers/testdata/reconcile-for-the-multiple-containers-pod-during-emergency/before/tortoise.yaml index c75e02f6..81260ee3 100644 --- a/controllers/testdata/reconcile-for-the-multiple-containers-pod-during-emergency/before/tortoise.yaml +++ b/controllers/testdata/reconcile-for-the-multiple-containers-pod-during-emergency/before/tortoise.yaml @@ -22,12 +22,12 @@ status: containerResourceRequests: - containerName: "app" resource: - cpu: "10" - memory: 10Gi + cpu: "6" + memory: 3Gi - containerName: "istio-proxy" resource: cpu: "4" - memory: 4Gi + memory: 3Gi tortoiseConditions: - message: "HPA target utilization is updated" reason: HPATargetUtilizationUpdated diff --git a/controllers/testdata/reconcile-for-the-multiple-containers-pod-emergency-started/after/tortoise.yaml b/controllers/testdata/reconcile-for-the-multiple-containers-pod-emergency-started/after/tortoise.yaml index c75e02f6..81260ee3 100644 --- a/controllers/testdata/reconcile-for-the-multiple-containers-pod-emergency-started/after/tortoise.yaml +++ b/controllers/testdata/reconcile-for-the-multiple-containers-pod-emergency-started/after/tortoise.yaml @@ -22,12 +22,12 @@ status: containerResourceRequests: - containerName: "app" resource: - cpu: "10" - memory: 10Gi + cpu: "6" + memory: 3Gi - containerName: "istio-proxy" resource: cpu: "4" - memory: 4Gi + memory: 3Gi tortoiseConditions: - message: "HPA target utilization is updated" reason: HPATargetUtilizationUpdated diff --git a/controllers/testdata/reconcile-for-the-multiple-containers-pod-one-off/after/tortoise.yaml b/controllers/testdata/reconcile-for-the-multiple-containers-pod-one-off/after/tortoise.yaml index c267180d..8ea704e5 100644 --- a/controllers/testdata/reconcile-for-the-multiple-containers-pod-one-off/after/tortoise.yaml +++ b/controllers/testdata/reconcile-for-the-multiple-containers-pod-one-off/after/tortoise.yaml @@ -21,8 +21,8 @@ status: containerResourceRequests: - containerName: "app" resource: - cpu: "10" - memory: 10Gi + cpu: "6" + memory: 3Gi - containerName: "istio-proxy" resource: cpu: "4" diff --git a/controllers/testdata/reconcile-for-the-multiple-containers-pod-suggested-too-small/after/tortoise.yaml b/controllers/testdata/reconcile-for-the-multiple-containers-pod-suggested-too-small/after/tortoise.yaml index 8bcba0f7..64c55ebb 100644 --- a/controllers/testdata/reconcile-for-the-multiple-containers-pod-suggested-too-small/after/tortoise.yaml +++ b/controllers/testdata/reconcile-for-the-multiple-containers-pod-suggested-too-small/after/tortoise.yaml @@ -22,11 +22,11 @@ status: - containerName: "app" resource: cpu: "4" - memory: 4Gi + memory: 10Mi - containerName: "istio-proxy" resource: cpu: "100m" - memory: "100Mi" + memory: "11Mi" tortoiseConditions: - message: "HPA target utilization is updated" reason: HPATargetUtilizationUpdated diff --git a/controllers/testdata/reconcile-for-the-multiple-containers-pod-working/after/tortoise.yaml b/controllers/testdata/reconcile-for-the-multiple-containers-pod-working/after/tortoise.yaml index 4fd05fc4..9009fbf6 100644 --- a/controllers/testdata/reconcile-for-the-multiple-containers-pod-working/after/tortoise.yaml +++ b/controllers/testdata/reconcile-for-the-multiple-containers-pod-working/after/tortoise.yaml @@ -21,12 +21,12 @@ status: containerResourceRequests: - containerName: "app" resource: - cpu: "10" - memory: 10Gi + cpu: "6" + memory: 3Gi - containerName: "istio-proxy" resource: cpu: "4" - memory: 4Gi + memory: 3Gi tortoiseConditions: - message: "HPA target utilization is updated" reason: HPATargetUtilizationUpdated diff --git a/controllers/testdata/reconcile-for-the-single-container-pod-backtonormal/after/tortoise.yaml b/controllers/testdata/reconcile-for-the-single-container-pod-backtonormal/after/tortoise.yaml index 33d84004..cce85da3 100644 --- a/controllers/testdata/reconcile-for-the-single-container-pod-backtonormal/after/tortoise.yaml +++ b/controllers/testdata/reconcile-for-the-single-container-pod-backtonormal/after/tortoise.yaml @@ -19,7 +19,7 @@ status: - containerName: "app" resource: cpu: "4" - memory: 4Gi + memory: 3Gi tortoiseConditions: - message: "HPA target utilization is updated" reason: HPATargetUtilizationUpdated diff --git a/controllers/testdata/reconcile-for-the-single-container-pod-during-emergency/after/tortoise.yaml b/controllers/testdata/reconcile-for-the-single-container-pod-during-emergency/after/tortoise.yaml index 537862a0..ae0a9967 100644 --- a/controllers/testdata/reconcile-for-the-single-container-pod-during-emergency/after/tortoise.yaml +++ b/controllers/testdata/reconcile-for-the-single-container-pod-during-emergency/after/tortoise.yaml @@ -19,7 +19,7 @@ status: - containerName: "app" resource: cpu: "4" - memory: 4Gi + memory: 3Gi tortoiseConditions: - message: "HPA target utilization is updated" reason: HPATargetUtilizationUpdated diff --git a/controllers/testdata/reconcile-for-the-single-container-pod-during-emergency/before/tortoise.yaml b/controllers/testdata/reconcile-for-the-single-container-pod-during-emergency/before/tortoise.yaml index 0c902140..f57c3d2b 100644 --- a/controllers/testdata/reconcile-for-the-single-container-pod-during-emergency/before/tortoise.yaml +++ b/controllers/testdata/reconcile-for-the-single-container-pod-during-emergency/before/tortoise.yaml @@ -19,7 +19,7 @@ status: - containerName: "app" resource: cpu: "4" - memory: 4Gi + memory: 3Gi tortoiseConditions: - message: "HPA target utilization is updated" reason: HPATargetUtilizationUpdated diff --git a/controllers/testdata/reconcile-for-the-single-container-pod-emergency-started/after/tortoise.yaml b/controllers/testdata/reconcile-for-the-single-container-pod-emergency-started/after/tortoise.yaml index 24267f27..f0fe0ee4 100644 --- a/controllers/testdata/reconcile-for-the-single-container-pod-emergency-started/after/tortoise.yaml +++ b/controllers/testdata/reconcile-for-the-single-container-pod-emergency-started/after/tortoise.yaml @@ -19,7 +19,7 @@ status: - containerName: "app" resource: cpu: "4" - memory: 4Gi + memory: 3Gi tortoiseConditions: - message: "HPA target utilization is updated" reason: HPATargetUtilizationUpdated diff --git a/controllers/testdata/reconcile-for-the-single-container-pod-gathering-data-finished/after/tortoise.yaml b/controllers/testdata/reconcile-for-the-single-container-pod-gathering-data-finished/after/tortoise.yaml index 75a76fe8..553dfd31 100644 --- a/controllers/testdata/reconcile-for-the-single-container-pod-gathering-data-finished/after/tortoise.yaml +++ b/controllers/testdata/reconcile-for-the-single-container-pod-gathering-data-finished/after/tortoise.yaml @@ -18,7 +18,7 @@ status: - containerName: "app" resource: cpu: "4" - memory: 4Gi + memory: 3Gi tortoiseConditions: - message: "HPA target utilization is updated" reason: HPATargetUtilizationUpdated diff --git a/controllers/testdata/reconcile-for-the-single-container-pod-hpa-changed/after/tortoise.yaml b/controllers/testdata/reconcile-for-the-single-container-pod-hpa-changed/after/tortoise.yaml index d3f13765..0d6e34ac 100644 --- a/controllers/testdata/reconcile-for-the-single-container-pod-hpa-changed/after/tortoise.yaml +++ b/controllers/testdata/reconcile-for-the-single-container-pod-hpa-changed/after/tortoise.yaml @@ -18,7 +18,7 @@ status: containerResourceRequests: - containerName: "app" resource: - cpu: "4" + cpu: "3" memory: 4Gi tortoiseConditions: - message: "HPA target utilization is updated" diff --git a/controllers/testdata/reconcile-for-the-single-container-pod-partly-working/after/tortoise.yaml b/controllers/testdata/reconcile-for-the-single-container-pod-partly-working/after/tortoise.yaml index 46e192e5..4becdc49 100644 --- a/controllers/testdata/reconcile-for-the-single-container-pod-partly-working/after/tortoise.yaml +++ b/controllers/testdata/reconcile-for-the-single-container-pod-partly-working/after/tortoise.yaml @@ -18,7 +18,7 @@ status: - containerName: "app" resource: cpu: "4" - memory: 4Gi + memory: 3Gi tortoiseConditions: - message: "HPA target utilization is updated" reason: HPATargetUtilizationUpdated diff --git a/controllers/testdata/reconcile-for-the-single-container-pod-working/after/tortoise.yaml b/controllers/testdata/reconcile-for-the-single-container-pod-working/after/tortoise.yaml index 75a76fe8..553dfd31 100644 --- a/controllers/testdata/reconcile-for-the-single-container-pod-working/after/tortoise.yaml +++ b/controllers/testdata/reconcile-for-the-single-container-pod-working/after/tortoise.yaml @@ -18,7 +18,7 @@ status: - containerName: "app" resource: cpu: "4" - memory: 4Gi + memory: 3Gi tortoiseConditions: - message: "HPA target utilization is updated" reason: HPATargetUtilizationUpdated diff --git a/controllers/tortoise_controller.go b/controllers/tortoise_controller.go index 75a4fb1e..0cb8163d 100644 --- a/controllers/tortoise_controller.go +++ b/controllers/tortoise_controller.go @@ -139,16 +139,17 @@ func (r *TortoiseReconciler) Reconcile(ctx context.Context, req ctrl.Request) (_ return ctrl.Result{}, err } currentReplicaNum := dm.Status.Replicas - acr, err := r.DeploymentService.GetResourceRequests(dm) - if err != nil { - logger.Error(err, "failed to get resource requests in deployment", "tortoise", req.NamespacedName, "deployment", klog.KObj(dm)) - return ctrl.Result{}, err - } - - tortoise.Status.Conditions.ContainerResourceRequests = acr - // === Finish the part depending on deployment === - // From here, we shouldn't use `dm` anymore. + if tortoise.Spec.UpdateMode == autoscalingv1beta3.UpdateModeOff || tortoise.Status.Conditions.ContainerResourceRequests == nil { + // If the update mode is off, we have to update ContainerResourceRequests from the deployment directly. + // If it's not off, ContainerResourceRequests should be updated in UpdateVPAFromTortoiseRecommendation in the last reconciliation. + acr, err := r.DeploymentService.GetResourceRequests(dm) + if err != nil { + logger.Error(err, "failed to get resource requests in deployment", "tortoise", req.NamespacedName, "deployment", klog.KObj(dm)) + return ctrl.Result{}, err + } + tortoise.Status.Conditions.ContainerResourceRequests = acr + } hpa, err := r.HpaService.GetHPAOnTortoiseSpec(ctx, tortoise) if err != nil { @@ -236,7 +237,7 @@ func (r *TortoiseReconciler) Reconcile(ctx context.Context, req ctrl.Request) (_ return ctrl.Result{}, err } - _, updated, err := r.VpaService.UpdateVPAFromTortoiseRecommendation(ctx, tortoise, currentReplicaNum, now) + _, tortoise, updated, err := r.VpaService.UpdateVPAFromTortoiseRecommendation(ctx, tortoise, currentReplicaNum, now) if err != nil { logger.Error(err, "update VPA based on the recommendation in tortoise", "tortoise", req.NamespacedName) return ctrl.Result{}, err diff --git a/pkg/deployment/service.go b/pkg/deployment/service.go index 08e4e9bf..1d1731b5 100644 --- a/pkg/deployment/service.go +++ b/pkg/deployment/service.go @@ -57,6 +57,7 @@ func (c *Service) RolloutRestart(ctx context.Context, dm *v1.Deployment, tortois return nil } +// GetResourceRequests returns the resource requests of the containers in the deployment. func (c *Service) GetResourceRequests(dm *v1.Deployment) ([]autoscalingv1beta3.ContainerResourceRequests, error) { actualContainerResource := []autoscalingv1beta3.ContainerResourceRequests{} for _, c := range dm.Spec.Template.Spec.Containers { diff --git a/pkg/vpa/service.go b/pkg/vpa/service.go index 19a159f8..1fc128a6 100644 --- a/pkg/vpa/service.go +++ b/pkg/vpa/service.go @@ -232,7 +232,7 @@ func (c *Service) CreateTortoiseMonitorVPA(ctx context.Context, tortoise *autosc // UpdateVPAFromTortoiseRecommendation updates VPA with the recommendation from Tortoise. // In the second return value, it returns true if the Pods should be updated with new resources. -func (c *Service) UpdateVPAFromTortoiseRecommendation(ctx context.Context, tortoise *autoscalingv1beta3.Tortoise, replica int32, now time.Time) (*v1.VerticalPodAutoscaler, bool, error) { +func (c *Service) UpdateVPAFromTortoiseRecommendation(ctx context.Context, tortoise *autoscalingv1beta3.Tortoise, replica int32, now time.Time) (*v1.VerticalPodAutoscaler, *autoscalingv1beta3.Tortoise, bool, error) { newVPA := &v1.VerticalPodAutoscaler{} // At the end of this function, this will be true: // - if UpdateMode is Off, this will be always false. @@ -250,6 +250,7 @@ func (c *Service) UpdateVPAFromTortoiseRecommendation(ctx context.Context, torto } newVPA = oldVPA.DeepCopy() newRecommendations := make([]v1.RecommendedContainerResources, 0, len(tortoise.Status.Recommendations.Vertical.ContainerResourceRecommendation)) + newRequests := make([]autoscalingv1beta3.ContainerResourceRequests, 0, len(tortoise.Status.Recommendations.Vertical.ContainerResourceRecommendation)) for _, r := range tortoise.Status.Recommendations.Vertical.ContainerResourceRecommendation { if !metricsRecorded { // only record metrics once in every reconcile loop. @@ -272,6 +273,10 @@ func (c *Service) UpdateVPAFromTortoiseRecommendation(ctx context.Context, torto UpperBound: r.RecommendedResource, UncappedTarget: r.RecommendedResource, }) + newRequests = append(newRequests, autoscalingv1beta3.ContainerResourceRequests{ + ContainerName: r.ContainerName, + Resource: r.RecommendedResource, + }) } metricsRecorded = true if tortoise.Spec.UpdateMode == autoscalingv1beta3.UpdateModeOff { @@ -316,6 +321,10 @@ func (c *Service) UpdateVPAFromTortoiseRecommendation(ctx context.Context, torto } } + // The recommendation will be applied to VPA and the deployment will be restarted with the new resources. + // Update the request recorded in the status, which will be used in the next reconcile loop. + tortoise.Status.Conditions.ContainerResourceRequests = newRequests + newVPA.Status.Conditions = []v1.VerticalPodAutoscalerCondition{ { Type: v1.RecommendationProvided, @@ -359,7 +368,7 @@ func (c *Service) UpdateVPAFromTortoiseRecommendation(ctx context.Context, torto } if err := retry.RetryOnConflict(retry.DefaultRetry, updateFn); err != nil { - return newVPA, podShouldBeUpdatedWithNewResource, fmt.Errorf("update VPA status: %w", err) + return newVPA, tortoise, podShouldBeUpdatedWithNewResource, fmt.Errorf("update VPA status: %w", err) } if tortoise.Spec.UpdateMode != autoscalingv1beta3.UpdateModeOff && podShouldBeUpdatedWithNewResource { @@ -378,7 +387,7 @@ func (c *Service) UpdateVPAFromTortoiseRecommendation(ctx context.Context, torto } } - return newVPA, podShouldBeUpdatedWithNewResource, nil + return newVPA, tortoise, podShouldBeUpdatedWithNewResource, nil } func recommendationIncreaseAnyResource(oldVPA, newVPA *v1.VerticalPodAutoscaler) bool { diff --git a/pkg/vpa/service_test.go b/pkg/vpa/service_test.go index c1717b4e..47670a0d 100644 --- a/pkg/vpa/service_test.go +++ b/pkg/vpa/service_test.go @@ -379,11 +379,25 @@ func TestService_UpdateVPAFromTortoiseRecommendation(t *testing.T) { initialVPA *vpav1.VerticalPodAutoscaler tortoise *autoscalingv1beta3.Tortoise want *vpav1.VerticalPodAutoscaler + wantTortoise *autoscalingv1beta3.Tortoise wantPodShouldBeUpdatedWithNewResource bool wantErr bool }{ { name: "VPA is modified at the first time (tortoise is Auto)", + initialVPA: &vpav1.VerticalPodAutoscaler{ + ObjectMeta: metav1.ObjectMeta{ + Name: tortoiseUpdaterVPANamePrefix + "tortoise", + Namespace: "default", + }, + Spec: vpav1.VerticalPodAutoscalerSpec{ + UpdatePolicy: &vpav1.PodUpdatePolicy{ + UpdateMode: ptr.To(vpav1.UpdateModeInitial), + }, + }, + // No recommendation yet. + Status: vpav1.VerticalPodAutoscalerStatus{}, + }, tortoise: &autoscalingv1beta3.Tortoise{ ObjectMeta: metav1.ObjectMeta{ Name: "tortoise", @@ -415,18 +429,54 @@ func TestService_UpdateVPAFromTortoiseRecommendation(t *testing.T) { }, }, }, - initialVPA: &vpav1.VerticalPodAutoscaler{ + wantTortoise: &autoscalingv1beta3.Tortoise{ ObjectMeta: metav1.ObjectMeta{ - Name: tortoiseUpdaterVPANamePrefix + "tortoise", + Name: "tortoise", Namespace: "default", }, - Spec: vpav1.VerticalPodAutoscalerSpec{ - UpdatePolicy: &vpav1.PodUpdatePolicy{ - UpdateMode: ptr.To(vpav1.UpdateModeInitial), + Spec: autoscalingv1beta3.TortoiseSpec{ + UpdateMode: autoscalingv1beta3.UpdateModeAuto, + }, + Status: autoscalingv1beta3.TortoiseStatus{ + Conditions: autoscalingv1beta3.Conditions{ + ContainerResourceRequests: []autoscalingv1beta3.ContainerResourceRequests{ + { + ContainerName: "app", + Resource: v1.ResourceList{ + v1.ResourceMemory: resource.MustParse("1Gi"), + v1.ResourceCPU: resource.MustParse("1"), + }, + }, + { + ContainerName: "sidecar", + Resource: v1.ResourceList{ + v1.ResourceMemory: resource.MustParse("1Gi"), + v1.ResourceCPU: resource.MustParse("1"), + }, + }, + }, + }, + Recommendations: autoscalingv1beta3.Recommendations{ + Vertical: autoscalingv1beta3.VerticalRecommendations{ + ContainerResourceRecommendation: []autoscalingv1beta3.RecommendedContainerResources{ + { + ContainerName: "app", + RecommendedResource: v1.ResourceList{ + v1.ResourceMemory: resource.MustParse("1Gi"), + v1.ResourceCPU: resource.MustParse("1"), + }, + }, + { + ContainerName: "sidecar", + RecommendedResource: v1.ResourceList{ + v1.ResourceMemory: resource.MustParse("1Gi"), + v1.ResourceCPU: resource.MustParse("1"), + }, + }, + }, + }, }, }, - // No recommendation yet. - Status: vpav1.VerticalPodAutoscalerStatus{}, }, // It should be true because the VPA has got a first recommendation. wantPodShouldBeUpdatedWithNewResource: true, @@ -590,6 +640,38 @@ func TestService_UpdateVPAFromTortoiseRecommendation(t *testing.T) { }, }, }, + wantTortoise: &autoscalingv1beta3.Tortoise{ + ObjectMeta: metav1.ObjectMeta{ + Name: "tortoise", + Namespace: "default", + }, + Spec: autoscalingv1beta3.TortoiseSpec{ + UpdateMode: autoscalingv1beta3.UpdateModeAuto, + }, + Status: autoscalingv1beta3.TortoiseStatus{ + Recommendations: autoscalingv1beta3.Recommendations{ + Vertical: autoscalingv1beta3.VerticalRecommendations{ + ContainerResourceRecommendation: []autoscalingv1beta3.RecommendedContainerResources{ + { + ContainerName: "app", + RecommendedResource: v1.ResourceList{ + v1.ResourceMemory: resource.MustParse("1Gi"), + v1.ResourceCPU: resource.MustParse("1"), + }, + }, + { + ContainerName: "sidecar", + RecommendedResource: v1.ResourceList{ + v1.ResourceMemory: resource.MustParse("1Gi"), + v1.ResourceCPU: resource.MustParse("1"), + }, + }, + }, + }, + }, + // ContainerResourceRequests isn't updated because we won't apply it. + }, + }, wantPodShouldBeUpdatedWithNewResource: false, want: &vpav1.VerticalPodAutoscaler{ ObjectMeta: metav1.ObjectMeta{ @@ -750,6 +832,55 @@ func TestService_UpdateVPAFromTortoiseRecommendation(t *testing.T) { }, }, }, + wantTortoise: &autoscalingv1beta3.Tortoise{ + ObjectMeta: metav1.ObjectMeta{ + Name: "tortoise", + Namespace: "default", + }, + Spec: autoscalingv1beta3.TortoiseSpec{ + UpdateMode: autoscalingv1beta3.UpdateModeAuto, + }, + Status: autoscalingv1beta3.TortoiseStatus{ + Conditions: autoscalingv1beta3.Conditions{ + ContainerResourceRequests: []autoscalingv1beta3.ContainerResourceRequests{ + { + ContainerName: "app", + Resource: v1.ResourceList{ + v1.ResourceMemory: resource.MustParse("1Gi"), + v1.ResourceCPU: resource.MustParse("1"), + }, + }, + { + ContainerName: "sidecar", + Resource: v1.ResourceList{ + v1.ResourceMemory: resource.MustParse("1Gi"), + v1.ResourceCPU: resource.MustParse("1"), + }, + }, + }, + }, + Recommendations: autoscalingv1beta3.Recommendations{ + Vertical: autoscalingv1beta3.VerticalRecommendations{ + ContainerResourceRecommendation: []autoscalingv1beta3.RecommendedContainerResources{ + { + ContainerName: "app", + RecommendedResource: v1.ResourceList{ + v1.ResourceMemory: resource.MustParse("1Gi"), + v1.ResourceCPU: resource.MustParse("1"), + }, + }, + { + ContainerName: "sidecar", + RecommendedResource: v1.ResourceList{ + v1.ResourceMemory: resource.MustParse("1Gi"), + v1.ResourceCPU: resource.MustParse("1"), + }, + }, + }, + }, + }, + }, + }, wantPodShouldBeUpdatedWithNewResource: true, want: &vpav1.VerticalPodAutoscaler{ ObjectMeta: metav1.ObjectMeta{ @@ -903,6 +1034,38 @@ func TestService_UpdateVPAFromTortoiseRecommendation(t *testing.T) { }, }, }, + wantTortoise: &autoscalingv1beta3.Tortoise{ + ObjectMeta: metav1.ObjectMeta{ + Name: "tortoise", + Namespace: "default", + }, + Spec: autoscalingv1beta3.TortoiseSpec{ + UpdateMode: autoscalingv1beta3.UpdateModeAuto, + }, + Status: autoscalingv1beta3.TortoiseStatus{ + Recommendations: autoscalingv1beta3.Recommendations{ + Vertical: autoscalingv1beta3.VerticalRecommendations{ + ContainerResourceRecommendation: []autoscalingv1beta3.RecommendedContainerResources{ + { + ContainerName: "app", + RecommendedResource: v1.ResourceList{ + v1.ResourceMemory: resource.MustParse("1Gi"), + v1.ResourceCPU: resource.MustParse("1"), + }, + }, + { + ContainerName: "sidecar", + RecommendedResource: v1.ResourceList{ + v1.ResourceMemory: resource.MustParse("1Gi"), + v1.ResourceCPU: resource.MustParse("1"), + }, + }, + }, + }, + }, + // ContainerResourceRequests isn't updated because we won't apply it. + }, + }, wantPodShouldBeUpdatedWithNewResource: false, want: &vpav1.VerticalPodAutoscaler{ ObjectMeta: metav1.ObjectMeta{ @@ -1022,6 +1185,38 @@ func TestService_UpdateVPAFromTortoiseRecommendation(t *testing.T) { Recommendation: &vpav1.RecommendedPodResources{}, }, }, + wantTortoise: &autoscalingv1beta3.Tortoise{ + ObjectMeta: metav1.ObjectMeta{ + Name: "tortoise", + Namespace: "default", + }, + Spec: autoscalingv1beta3.TortoiseSpec{ + UpdateMode: autoscalingv1beta3.UpdateModeOff, + }, + Status: autoscalingv1beta3.TortoiseStatus{ + Recommendations: autoscalingv1beta3.Recommendations{ + Vertical: autoscalingv1beta3.VerticalRecommendations{ + ContainerResourceRecommendation: []autoscalingv1beta3.RecommendedContainerResources{ + { + ContainerName: "app", + RecommendedResource: v1.ResourceList{ + v1.ResourceMemory: resource.MustParse("1Gi"), + v1.ResourceCPU: resource.MustParse("1"), + }, + }, + { + ContainerName: "sidecar", + RecommendedResource: v1.ResourceList{ + v1.ResourceMemory: resource.MustParse("1Gi"), + v1.ResourceCPU: resource.MustParse("1"), + }, + }, + }, + }, + }, + }, + // ContainerResourceRequests isn't updated because it's dry-run. + }, }, { name: "the recommendation on VPA is removed when tortoise is Off mode (= probably this tortoise is changed back from auto to off)", @@ -1098,6 +1293,55 @@ func TestService_UpdateVPAFromTortoiseRecommendation(t *testing.T) { Recommendation: &vpav1.RecommendedPodResources{}, }, }, + wantTortoise: &autoscalingv1beta3.Tortoise{ + ObjectMeta: metav1.ObjectMeta{ + Name: "tortoise", + Namespace: "default", + }, + Spec: autoscalingv1beta3.TortoiseSpec{ + UpdateMode: autoscalingv1beta3.UpdateModeOff, + }, + Status: autoscalingv1beta3.TortoiseStatus{ + Recommendations: autoscalingv1beta3.Recommendations{ + Vertical: autoscalingv1beta3.VerticalRecommendations{ + ContainerResourceRecommendation: []autoscalingv1beta3.RecommendedContainerResources{ + { + ContainerName: "app", + RecommendedResource: v1.ResourceList{ + v1.ResourceMemory: resource.MustParse("1Gi"), + v1.ResourceCPU: resource.MustParse("1"), + }, + }, + { + ContainerName: "sidecar", + RecommendedResource: v1.ResourceList{ + v1.ResourceMemory: resource.MustParse("1Gi"), + v1.ResourceCPU: resource.MustParse("1"), + }, + }, + }, + }, + }, + Conditions: autoscalingv1beta3.Conditions{ + ContainerResourceRequests: []autoscalingv1beta3.ContainerResourceRequests{ + { + ContainerName: "app", + Resource: v1.ResourceList{ + v1.ResourceMemory: resource.MustParse("1Gi"), + v1.ResourceCPU: resource.MustParse("1"), + }, + }, + { + ContainerName: "sidecar", + Resource: v1.ResourceList{ + v1.ResourceMemory: resource.MustParse("1Gi"), + v1.ResourceCPU: resource.MustParse("1"), + }, + }, + }, + }, + }, + }, }, { name: "Tortoise's mode changed: Auto → Off → Auto", @@ -1154,6 +1398,55 @@ func TestService_UpdateVPAFromTortoiseRecommendation(t *testing.T) { }, }, }, + wantTortoise: &autoscalingv1beta3.Tortoise{ + ObjectMeta: metav1.ObjectMeta{ + Name: "tortoise", + Namespace: "default", + }, + Spec: autoscalingv1beta3.TortoiseSpec{ + UpdateMode: autoscalingv1beta3.UpdateModeOff, + }, + Status: autoscalingv1beta3.TortoiseStatus{ + Recommendations: autoscalingv1beta3.Recommendations{ + Vertical: autoscalingv1beta3.VerticalRecommendations{ + ContainerResourceRecommendation: []autoscalingv1beta3.RecommendedContainerResources{ + { + ContainerName: "app", + RecommendedResource: v1.ResourceList{ + v1.ResourceMemory: resource.MustParse("1Gi"), + v1.ResourceCPU: resource.MustParse("1"), + }, + }, + { + ContainerName: "sidecar", + RecommendedResource: v1.ResourceList{ + v1.ResourceMemory: resource.MustParse("1Gi"), + v1.ResourceCPU: resource.MustParse("1"), + }, + }, + }, + }, + }, + Conditions: autoscalingv1beta3.Conditions{ + ContainerResourceRequests: []autoscalingv1beta3.ContainerResourceRequests{ + { + ContainerName: "app", + Resource: v1.ResourceList{ + v1.ResourceMemory: resource.MustParse("1Gi"), + v1.ResourceCPU: resource.MustParse("1"), + }, + }, + { + ContainerName: "sidecar", + Resource: v1.ResourceList{ + v1.ResourceMemory: resource.MustParse("1Gi"), + v1.ResourceCPU: resource.MustParse("1"), + }, + }, + }, + }, + }, + }, // It should be true because the VPA has got a first recommendation. wantPodShouldBeUpdatedWithNewResource: true, want: &vpav1.VerticalPodAutoscaler{ @@ -1228,7 +1521,7 @@ func TestService_UpdateVPAFromTortoiseRecommendation(t *testing.T) { recorder: record.NewFakeRecorder(10), } - got, updated, err := c.UpdateVPAFromTortoiseRecommendation(context.Background(), tt.tortoise, 10, now) + got, gotTortoise, updated, err := c.UpdateVPAFromTortoiseRecommendation(context.Background(), tt.tortoise, 10, now) if (err != nil) != tt.wantErr { t.Errorf("Service.UpdateVPAFromTortoiseRecommendation() error = %v, wantErr %v", err, tt.wantErr) return @@ -1242,6 +1535,10 @@ func TestService_UpdateVPAFromTortoiseRecommendation(t *testing.T) { if diff := cmp.Diff(got, tt.want); diff != "" { t.Errorf("Service.UpdateVPAFromTortoiseRecommendation() mismatch (-want +got):\n%s", diff) } + + if diff := cmp.Diff(gotTortoise, tt.tortoise); diff != "" { + t.Errorf("Service.UpdateVPAFromTortoiseRecommendation() mismatch (-want +got):\n%s", diff) + } }) } }