handle custom istio injection (#356)

api/v1beta3/tortoise_types.go

@@ -54,10 +54,10 @@ type TortoiseSpec struct {
 	// ResourcePolicy contains the policy how each resource is updated.
 	// +optional
 	ResourcePolicy []ContainerResourcePolicy `json:"resourcePolicy,omitempty" protobuf:"bytes,3,opt,name=resourcePolicy"`
-	// DeletionPolicy is the policy how the controller deletes associated HPA and VPAs when tortoise is removed.
-	// If "DeleteAll", tortoise deletes all associated HPA and VPAs, created by tortoise. If the associated HPA is not created by tortoise,
+	// DeletionPolicy is the policy how the controller deletes associated HPA and VPA when tortoise is removed.
+	// If "DeleteAll", tortoise deletes all associated HPA and VPA, created by tortoise. If the associated HPA is not created by tortoise,
 	// which is associated by spec.targetRefs.horizontalPodAutoscalerName, tortoise never delete the HPA.
-	// If "NoDelete", tortoise doesn't delete any associated HPA and VPAs.
+	// If "NoDelete", tortoise doesn't delete any associated HPA and VPA.
 	//
 	// "NoDelete" is the default value.
 	// +optional
@@ -215,7 +215,7 @@ const (
 type TortoisePhase string
 
 const (
-	// TortoisePhaseInitializing means tortoise is just created and initializing some components (HPA and VPAs),
+	// TortoisePhaseInitializing means tortoise is just created and initializing some components (HPA and VPA),
 	// and wait for those components to be ready.
 	TortoisePhaseInitializing TortoisePhase = "Initializing"
 	// TortoisePhaseGatheringData means tortoise is now gathering data and cannot make the accurate recommendations.

config/crd/bases/autoscaling.mercari.com_tortoises.yaml

@@ -93,12 +93,12 @@ spec:
                 type: array
               deletionPolicy:
                 description: "DeletionPolicy is the policy how the controller deletes
-                  associated HPA and VPAs when tortoise is removed. If \"DeleteAll\",
-                  tortoise deletes all associated HPA and VPAs, created by tortoise.
+                  associated HPA and VPA when tortoise is removed. If \"DeleteAll\",
+                  tortoise deletes all associated HPA and VPA, created by tortoise.
                   If the associated HPA is not created by tortoise, which is associated
                   by spec.targetRefs.horizontalPodAutoscalerName, tortoise never delete
                   the HPA. If \"NoDelete\", tortoise doesn't delete any associated
-                  HPA and VPAs. \n \"NoDelete\" is the default value."
+                  HPA and VPA. \n \"NoDelete\" is the default value."
                 enum:
                 - DeleteAll
                 - NoDelete

controllers/testdata/reconcile-for-the-istio-enabled-pod-working/after/deployment.yaml

@@ -19,6 +19,8 @@ spec:
         app: mercari
     spec:
       containers:
+      - name: istio-proxy 
+        image: auto
       - image: awesome-mercari-app-image
         name: app
         resources:

controllers/testdata/reconcile-for-the-istio-enabled-pod-working/before/deployment.yaml

@@ -16,6 +16,8 @@ spec:
         app: mercari
     spec:
       containers:
+      - name: istio-proxy # will be ignored.
+        image: auto
       - image: awesome-mercari-app-image
         name: app
         resources:

controllers/tortoise_controller.go

@@ -101,7 +101,7 @@ func (r *TortoiseReconciler) Reconcile(ctx context.Context, req ctrl.Request) (_
 		// Tortoise is deleted by user and waiting for finalizer.
 		logger.Info("tortoise is deleted", "tortoise", req.NamespacedName)
 		if err := r.deleteVPAAndHPA(ctx, tortoise, now); err != nil {
-			return ctrl.Result{}, fmt.Errorf("delete VPAs and HPA: %w", err)
+			return ctrl.Result{}, fmt.Errorf("delete VPA and HPA: %w", err)
 		}
 		if err := r.TortoiseService.RemoveFinalizer(ctx, tortoise); err != nil {
 			return ctrl.Result{}, fmt.Errorf("remove finalizer: %w", err)
@@ -181,7 +181,7 @@ func (r *TortoiseReconciler) Reconcile(ctx context.Context, req ctrl.Request) (_
 		logger.Info("initializing tortoise", "tortoise", req.NamespacedName)
 		// need to initialize HPA and VPA.
 		if err := r.initializeVPAAndHPA(ctx, tortoise, currentReplicaNum, now); err != nil {
-			return ctrl.Result{}, fmt.Errorf("initialize VPAs and HPA: %w", err)
+			return ctrl.Result{}, fmt.Errorf("initialize VPA and HPA: %w", err)
 		}
 
 		return ctrl.Result{RequeueAfter: r.Interval}, nil
@@ -245,7 +245,7 @@ func (r *TortoiseReconciler) Reconcile(ctx context.Context, req ctrl.Request) (_
 	}
 
 	if tortoise.Status.TortoisePhase == autoscalingv1beta3.TortoisePhaseGatheringData {
-		logger.Info("tortoise is GatheringData phase; skip applying the recommendation to HPA or VPAs")
+		logger.Info("tortoise is GatheringData phase; skip applying the recommendation to HPA or VPA")
 		return ctrl.Result{RequeueAfter: r.Interval}, nil
 	}
 

controllers/tortoise_controller_test.go

@@ -470,7 +470,7 @@ var _ = Describe("Test TortoiseController", func() {
 		})
 	})
 	Context("DeletionPolicy is handled correctly", func() {
-		It("[DeletionPolicy = DeleteAll] delete HPA and VPAs when Tortoise is deleted", func() {
+		It("[DeletionPolicy = DeleteAll] delete HPA and VPA when Tortoise is deleted", func() {
 			resource := initializeResourcesFromFiles(ctx, k8sClient, "testdata/deletion-policy-all/before")
 			stopFunc = startController(ctx)
 
@@ -491,7 +491,7 @@ var _ = Describe("Test TortoiseController", func() {
 				g.Expect(apierrors.IsNotFound(err)).To(Equal(true))
 			}).Should(Succeed())
 		})
-		It("[DeletionPolicy = NoDelete] do not delete HPA and VPAs when Tortoise is deleted", func() {
+		It("[DeletionPolicy = NoDelete] do not delete HPA and VPA when Tortoise is deleted", func() {
 			resource := initializeResourcesFromFiles(ctx, k8sClient, "testdata/deletion-no-delete/before")
 			stopFunc = startController(ctx)
 

docs/user-guide.md

@@ -131,12 +131,12 @@ spec:
   deletionPolicy: "DeleteAll"
 ```
 
-DeletionPolicy is the policy how the controller deletes associated HPA and VPAs when tortoise is removed.
+DeletionPolicy is the policy how the controller deletes associated HPA and VPA when tortoise is removed.
 
-- `DeleteAll`: tortoise deletes all associated HPA and VPAs, created by tortoise. 
+- `DeleteAll`: tortoise deletes all associated HPA and VPA, created by tortoise. 
 But, if the associated HPA is not created by tortoise, that is associated by `spec.targetRefs.horizontalPodAutoscalerName`, 
 tortoise doesn't delete the HPA even with `DeleteAll`.
-- `NoDelete`(default): tortoise doesn't delete any associated HPA and VPAs.
+- `NoDelete`(default): tortoise doesn't delete any associated HPA and VPA.
 
 ### `.spec.ResourcePolicy`
 

manifests/crd/apiextensions.k8s.io_v1_customresourcedefinition_tortoises.autoscaling.mercari.com.yaml

@@ -68,7 +68,7 @@ spec:
                   type: object
                 type: array
               deletionPolicy:
-                description: "DeletionPolicy is the policy how the controller deletes associated HPA and VPAs when tortoise is removed. If \"DeleteAll\", tortoise deletes all associated HPA and VPAs, created by tortoise. If the associated HPA is not created by tortoise, which is associated by spec.targetRefs.horizontalPodAutoscalerName, tortoise never delete the HPA. If \"NoDelete\", tortoise doesn't delete any associated HPA and VPAs. \n \"NoDelete\" is the default value."
+                description: "DeletionPolicy is the policy how the controller deletes associated HPA and VPA when tortoise is removed. If \"DeleteAll\", tortoise deletes all associated HPA and VPA, created by tortoise. If the associated HPA is not created by tortoise, which is associated by spec.targetRefs.horizontalPodAutoscalerName, tortoise never delete the HPA. If \"NoDelete\", tortoise doesn't delete any associated HPA and VPA. \n \"NoDelete\" is the default value."
                 enum:
                 - DeleteAll
                 - NoDelete

manifests/default/apiextensions.k8s.io_v1_customresourcedefinition_tortoises.autoscaling.mercari.com.yaml

@@ -68,7 +68,7 @@ spec:
                   type: object
                 type: array
               deletionPolicy:
-                description: "DeletionPolicy is the policy how the controller deletes associated HPA and VPAs when tortoise is removed. If \"DeleteAll\", tortoise deletes all associated HPA and VPAs, created by tortoise. If the associated HPA is not created by tortoise, which is associated by spec.targetRefs.horizontalPodAutoscalerName, tortoise never delete the HPA. If \"NoDelete\", tortoise doesn't delete any associated HPA and VPAs. \n \"NoDelete\" is the default value."
+                description: "DeletionPolicy is the policy how the controller deletes associated HPA and VPA when tortoise is removed. If \"DeleteAll\", tortoise deletes all associated HPA and VPA, created by tortoise. If the associated HPA is not created by tortoise, which is associated by spec.targetRefs.horizontalPodAutoscalerName, tortoise never delete the HPA. If \"NoDelete\", tortoise doesn't delete any associated HPA and VPA. \n \"NoDelete\" is the default value."
                 enum:
                 - DeleteAll
                 - NoDelete

pkg/deployment/service.go

@@ -62,7 +62,9 @@ func (c *Service) RolloutRestart(ctx context.Context, dm *v1.Deployment, tortois
 // 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 {
+
+	istioProxyIndex := -1
+	for i, c := range dm.Spec.Template.Spec.Containers {
 		rcr := autoscalingv1beta3.ContainerResourceRequests{
 			ContainerName: c.Name,
 			Resource:      corev1.ResourceList{},
@@ -71,6 +73,9 @@ func (c *Service) GetResourceRequests(dm *v1.Deployment) ([]autoscalingv1beta3.C
 			rcr.Resource[name] = r
 		}
 		actualContainerResource = append(actualContainerResource, rcr)
+		if c.Name == "istio-proxy" {
+			istioProxyIndex = i
+		}
 	}
 
 	if dm.Spec.Template.Annotations != nil {
@@ -95,15 +100,24 @@ func (c *Service) GetResourceRequests(dm *v1.Deployment) ([]autoscalingv1beta3.C
 			if err != nil {
 				return nil, fmt.Errorf("parse Memory request of istio sidecar: %w", err)
 			}
-			// If the deployment has the sidecar injection annotation, the Pods will have the sidecar container in addition.
-			actualContainerResource = append(actualContainerResource, v1beta3.ContainerResourceRequests{
-				ContainerName: "istio-proxy",
-				Resource: corev1.ResourceList{
-					corev1.ResourceCPU:    cpu,
-					corev1.ResourceMemory: memory,
-				},
-			})
+
+			if istioProxyIndex == -1 {
+				// If the deployment has the sidecar injection annotation, the Pods will have the sidecar container in addition.
+				actualContainerResource = append(actualContainerResource, v1beta3.ContainerResourceRequests{
+					ContainerName: "istio-proxy",
+					Resource: corev1.ResourceList{
+						corev1.ResourceCPU:    cpu,
+						corev1.ResourceMemory: memory,
+					},
+				})
+			} else {
+				// the deployment has the sidecar injection annotation and it's using the custom injection:
+				// https://istio.io/latest/docs/setup/additional-setup/sidecar-injection/#customizing-injection
+				actualContainerResource[istioProxyIndex].Resource[corev1.ResourceCPU] = cpu
+				actualContainerResource[istioProxyIndex].Resource[corev1.ResourceMemory] = memory
+			}
 		}
 	}
+
 	return actualContainerResource, nil
 }

pkg/vpa/service.go

@@ -146,7 +146,7 @@ func (c *Service) DisableTortoiseUpdaterVPA(ctx context.Context, tortoise *autos
 	return nil
 }
 
-// UpdateVPAContainerResourcePolicy is update VPAs to have appropriate container policies based on tortoises' resource policy.
+// UpdateVPAContainerResourcePolicy is update VPA to have appropriate container policies based on tortoises' resource policy.
 func (c *Service) UpdateVPAContainerResourcePolicy(ctx context.Context, tortoise *autoscalingv1beta3.Tortoise, vpa *v1.VerticalPodAutoscaler) (*v1.VerticalPodAutoscaler, error) {
 	retVPA := &v1.VerticalPodAutoscaler{}
 	var err error