Repository: mercari/tortoise
Pull request #264 (2024-01-09 03:38:37)

add the unit test for UpdateVPAFromTortoiseRecommendation
<!--  
Thanks for sending a pull request!  
Please read the CLA carefully before submitting your contribution to Mercari. 
Under any circumstances, by submitting your contribution, you are deemed to accept and agree to be bound by the terms and conditions of the CLA.
https://www.mercari.com/cla/

Also, please read the contributor guide, which contains some general rules and suggestions:
https://github.com/mercari/tortoise/blob/main/docs/contributor-guide.md
-->

#### What this PR does / why we need it:

ssia

#### Which issue(s) this PR fixes:

<!--
Before implementing any feature changes, please raise the issue and discuss what you propose with maintainers.
-->

Fixes #

#### Special notes for your reviewer:



Patch:
diff --git a/pkg/vpa/service.go b/pkg/vpa/service.go
index 97f6d5c..2c69c5b 100644
--- a/pkg/vpa/service.go
+++ b/pkg/vpa/service.go
@@ -35,16 +35,16 @@ func New(c *rest.Config, recorder record.EventRecorder) (*Service, error) {
 	return &Service{c: cli, recorder: recorder}, nil
 }
 
-const TortoiseMonitorVPANamePrefix = "tortoise-monitor-"
-const TortoiseUpdaterVPANamePrefix = "tortoise-updater-"
-const TortoiseVPARecommenderName = "tortoise-controller"
+const tortoiseMonitorVPANamePrefix = "tortoise-monitor-"
+const tortoiseUpdaterVPANamePrefix = "tortoise-updater-"
+const tortoiseVPARecommenderName = "tortoise-controller"
 
 func TortoiseMonitorVPAName(tortoiseName string) string {
-	return TortoiseMonitorVPANamePrefix + tortoiseName
+	return tortoiseMonitorVPANamePrefix + tortoiseName
 }
 
 func TortoiseUpdaterVPAName(tortoiseName string) string {
-	return TortoiseUpdaterVPANamePrefix + tortoiseName
+	return tortoiseUpdaterVPANamePrefix + tortoiseName
 }
 
 func (c *Service) DeleteTortoiseMonitorVPA(ctx context.Context, tortoise *autoscalingv1beta3.Tortoise) error {
@@ -113,7 +113,7 @@ func (c *Service) CreateTortoiseUpdaterVPA(ctx context.Context, tortoise *autosc
 		Spec: v1.VerticalPodAutoscalerSpec{
 			Recommenders: []*v1.VerticalPodAutoscalerRecommenderSelector{
 				{
-					Name: TortoiseVPARecommenderName, // This VPA is managed by Tortoise.
+					Name: tortoiseVPARecommenderName, // This VPA is managed by Tortoise.
 				},
 			},
 			TargetRef: &autoscaling.CrossVersionObjectReference{
@@ -244,15 +244,16 @@ func (c *Service) UpdateVPAFromTortoiseRecommendation(ctx context.Context, torto
 				UncappedTarget: r.RecommendedResource,
 			})
 		}
+		if tortoise.Spec.UpdateMode == autoscalingv1beta3.UpdateModeOff {
+			// don't update status if update mode is off. (= dryrun)
+			retVPA = vpa
+			return nil
+		}
 		if vpa.Status.Recommendation == nil {
 			vpa.Status.Recommendation = &v1.RecommendedPodResources{}
 		}
 		vpa.Status.Recommendation.ContainerRecommendations = newRecommendations
 		retVPA = vpa
-		if tortoise.Spec.UpdateMode == autoscalingv1beta3.UpdateModeOff {
-			// don't update status if update mode is off. (= dryrun)
-			return nil
-		}
 		retVPA, err = c.c.AutoscalingV1().VerticalPodAutoscalers(vpa.Namespace).UpdateStatus(ctx, vpa, metav1.UpdateOptions{})
 		if err != nil {
 			if apierrors.IsNotFound(err) {


Test patch:
diff --git a/pkg/vpa/service_test.go b/pkg/vpa/service_test.go
index 2c9e6ea..07685e0 100644
--- a/pkg/vpa/service_test.go
+++ b/pkg/vpa/service_test.go
@@ -1,13 +1,18 @@
 package vpa
 
 import (
+	"context"
 	"testing"
 	"time"
 
 	"github.com/google/go-cmp/cmp"
 	"github.com/google/go-cmp/cmp/cmpopts"
 	v1 "k8s.io/api/core/v1"
+	"k8s.io/apimachinery/pkg/api/resource"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	vpav1 "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/apis/autoscaling.k8s.io/v1"
+	"k8s.io/autoscaler/vertical-pod-autoscaler/pkg/client/clientset/versioned/fake"
+	"k8s.io/client-go/tools/record"
 
 	"github.com/mercari/tortoise/api/v1beta3"
 	autoscalingv1beta3 "github.com/mercari/tortoise/api/v1beta3"
@@ -135,3 +140,170 @@ func TestMakeAllVerticalContainerResourcePhaseRunning(t *testing.T) {
 		})
 	}
 }
+
+func TestService_UpdateVPAFromTortoiseRecommendation(t *testing.T) {
+	tests := []struct {
+		name       string
+		initialVPA *vpav1.VerticalPodAutoscaler
+		tortoise   *autoscalingv1beta3.Tortoise
+		want       *vpav1.VerticalPodAutoscaler
+		wantErr    bool
+	}{
+		{
+			name: "VPA is modified when tortoise is Auto mode",
+			tortoise: &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"),
+									},
+								},
+							},
+						},
+					},
+				},
+			},
+			initialVPA: &vpav1.VerticalPodAutoscaler{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      tortoiseUpdaterVPANamePrefix + "tortoise",
+					Namespace: "default",
+				},
+				Status: vpav1.VerticalPodAutoscalerStatus{},
+			},
+			want: &vpav1.VerticalPodAutoscaler{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      tortoiseUpdaterVPANamePrefix + "tortoise",
+					Namespace: "default",
+				},
+				Status: vpav1.VerticalPodAutoscalerStatus{
+					Recommendation: &vpav1.RecommendedPodResources{
+						ContainerRecommendations: []vpav1.RecommendedContainerResources{
+							{
+								ContainerName: "app",
+								Target: v1.ResourceList{
+									v1.ResourceMemory: resource.MustParse("1Gi"),
+									v1.ResourceCPU:    resource.MustParse("1"),
+								},
+								LowerBound: v1.ResourceList{
+									v1.ResourceMemory: resource.MustParse("1Gi"),
+									v1.ResourceCPU:    resource.MustParse("1"),
+								},
+								UpperBound: v1.ResourceList{
+									v1.ResourceMemory: resource.MustParse("1Gi"),
+									v1.ResourceCPU:    resource.MustParse("1"),
+								},
+								UncappedTarget: v1.ResourceList{
+									v1.ResourceMemory: resource.MustParse("1Gi"),
+									v1.ResourceCPU:    resource.MustParse("1"),
+								},
+							},
+							{
+								ContainerName: "sidecar",
+								Target: v1.ResourceList{
+									v1.ResourceMemory: resource.MustParse("1Gi"),
+									v1.ResourceCPU:    resource.MustParse("1"),
+								},
+								LowerBound: v1.ResourceList{
+									v1.ResourceMemory: resource.MustParse("1Gi"),
+									v1.ResourceCPU:    resource.MustParse("1"),
+								},
+								UpperBound: v1.ResourceList{
+									v1.ResourceMemory: resource.MustParse("1Gi"),
+									v1.ResourceCPU:    resource.MustParse("1"),
+								},
+								UncappedTarget: v1.ResourceList{
+									v1.ResourceMemory: resource.MustParse("1Gi"),
+									v1.ResourceCPU:    resource.MustParse("1"),
+								},
+							},
+						},
+					},
+				},
+			},
+		},
+		{
+			name: "VPA is NOT modified when tortoise is Off mode",
+			tortoise: &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"),
+									},
+								},
+							},
+						},
+					},
+				},
+			},
+			initialVPA: &vpav1.VerticalPodAutoscaler{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      tortoiseUpdaterVPANamePrefix + "tortoise",
+					Namespace: "default",
+				},
+				Status: vpav1.VerticalPodAutoscalerStatus{},
+			},
+			want: &vpav1.VerticalPodAutoscaler{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      tortoiseUpdaterVPANamePrefix + "tortoise",
+					Namespace: "default",
+				},
+				Status: vpav1.VerticalPodAutoscalerStatus{},
+			},
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			c := &Service{
+				c:        fake.NewSimpleClientset(tt.initialVPA),
+				recorder: record.NewFakeRecorder(10),
+			}
+
+			got, err := c.UpdateVPAFromTortoiseRecommendation(context.Background(), tt.tortoise)
+			if (err != nil) != tt.wantErr {
+				t.Errorf("Service.UpdateVPAFromTortoiseRecommendation() error = %v, wantErr %v", err, tt.wantErr)
+				return
+			}
+			if diff := cmp.Diff(got, tt.want); diff != "" {
+				t.Errorf("Service.UpdateVPAFromTortoiseRecommendation() mismatch (-want +got):\n%s", diff)
+			}
+		})
+	}
+}