Repository: mercari/tortoise
Pull request #285 (2024-01-18 09:47:24)

add log for deployment replacement
<!--  
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/deployment/service.go b/pkg/deployment/service.go
index bacf468..c4220af 100644
--- a/pkg/deployment/service.go
+++ b/pkg/deployment/service.go
@@ -11,6 +11,7 @@ import (
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/client-go/tools/record"
 	"sigs.k8s.io/controller-runtime/pkg/client"
+	"sigs.k8s.io/controller-runtime/pkg/log"
 
 	"github.com/mercari/tortoise/api/v1beta3"
 	autoscalingv1beta3 "github.com/mercari/tortoise/api/v1beta3"
@@ -53,6 +54,7 @@ func (c *Service) RolloutRestart(ctx context.Context, dm *v1.Deployment, tortois
 	}
 
 	c.recorder.Event(tortoise, corev1.EventTypeNormal, event.RestartDeployment, "Deployment is restarted to apply the recommendation from Tortoise")
+	log.FromContext(ctx).Info("Deployment is restarted to apply the recommendation from Tortoise", "tortoise", tortoise)
 
 	return nil
 }
diff --git a/pkg/vpa/service.go b/pkg/vpa/service.go
index 99f1df1..b272aba 100644
--- a/pkg/vpa/service.go
+++ b/pkg/vpa/service.go
@@ -275,8 +275,8 @@ func (c *Service) UpdateVPAFromTortoiseRecommendation(ctx context.Context, torto
 		metricsRecorded = true
 		retVPA = vpa
 		if tortoise.Spec.UpdateMode == autoscalingv1beta3.UpdateModeOff {
-			// don't update vpa if update mode is off. (= dryrun)
-			return nil
+			// remove recommendation if UpdateMode is Off so that VPA won't update the Pod.
+			newRecommendations = nil
 		}
 
 		if vpa.Status.Recommendation == nil {


Test patch:
diff --git a/controllers/testdata/reconcile-for-the-single-container-pod-dryrun/after/vpa-Updater.yaml b/controllers/testdata/reconcile-for-the-single-container-pod-dryrun/after/vpa-Updater.yaml
index 3e41bc7..08e993a 100644
--- a/controllers/testdata/reconcile-for-the-single-container-pod-dryrun/after/vpa-Updater.yaml
+++ b/controllers/testdata/reconcile-for-the-single-container-pod-dryrun/after/vpa-Updater.yaml
@@ -16,4 +16,6 @@ spec:
     name: mercari-app
   updatePolicy:
     updateMode: Initial
-status: {}
+    minReplicas: 9
+status: 
+  recommendation: {}
diff --git a/pkg/vpa/service_test.go b/pkg/vpa/service_test.go
index 1e087f2..84ef9d6 100644
--- a/pkg/vpa/service_test.go
+++ b/pkg/vpa/service_test.go
@@ -669,7 +669,6 @@ func TestService_UpdateVPAFromTortoiseRecommendation(t *testing.T) {
 					Name:      tortoiseUpdaterVPANamePrefix + "tortoise",
 					Namespace: "default",
 				},
-				Status: vpav1.VerticalPodAutoscalerStatus{},
 			},
 			wantUpdated: false,
 			want: &vpav1.VerticalPodAutoscaler{
@@ -677,7 +676,84 @@ func TestService_UpdateVPAFromTortoiseRecommendation(t *testing.T) {
 					Name:      tortoiseUpdaterVPANamePrefix + "tortoise",
 					Namespace: "default",
 				},
-				Status: vpav1.VerticalPodAutoscalerStatus{},
+				Spec: vpav1.VerticalPodAutoscalerSpec{
+					UpdatePolicy: &vpav1.PodUpdatePolicy{
+						UpdateMode:  ptr.To(vpav1.UpdateModeInitial),
+						MinReplicas: ptr.To[int32](9),
+					},
+				},
+				Status: vpav1.VerticalPodAutoscalerStatus{
+					Recommendation: &vpav1.RecommendedPodResources{},
+				},
+			},
+		},
+		{
+			name: "the recommendation on VPA is removed when tortoise is Off mode (= probably this tortoise is changed back from auto to off)",
+			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{
+					Recommendation: &vpav1.RecommendedPodResources{
+						ContainerRecommendations: []vpav1.RecommendedContainerResources{
+							// This will be removed.
+							{
+								ContainerName: "app",
+								Target: v1.ResourceList{
+									v1.ResourceMemory: resource.MustParse("1Gi"),
+								},
+							},
+						},
+					},
+				},
+			},
+			wantUpdated: true,
+			want: &vpav1.VerticalPodAutoscaler{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      tortoiseUpdaterVPANamePrefix + "tortoise",
+					Namespace: "default",
+				},
+				Spec: vpav1.VerticalPodAutoscalerSpec{
+					UpdatePolicy: &vpav1.PodUpdatePolicy{
+						UpdateMode:  ptr.To(vpav1.UpdateModeInitial),
+						MinReplicas: ptr.To[int32](9),
+					},
+				},
+				Status: vpav1.VerticalPodAutoscalerStatus{
+					Recommendation: &vpav1.RecommendedPodResources{},
+				},
 			},
 		},
 	}