diff --git a/api/v1beta2/tortoise_types.go b/api/v1beta2/tortoise_types.go index db84a1d5..798f2e3e 100644 --- a/api/v1beta2/tortoise_types.go +++ b/api/v1beta2/tortoise_types.go @@ -231,6 +231,7 @@ type RecommendedContainerResources struct { // ContainerName is the name of target container. ContainerName string `json:"containerName" protobuf:"bytes,1,name=containerName"` // RecommendedResource is the recommendation calculated by the tortoise. + // // If AutoscalingPolicy is vertical, it's the same value as the VPA suggests. // If AutoscalingPolicy is horizontal, it's basically the same value as the current resource request. // But, when the number of replicas are too small or too large, @@ -324,7 +325,7 @@ type ContainerRecommendationFromVPA struct { // MaxRecommendation is the max recommendation value from VPA in a certain period (1 week). // Tortoise generates all recommendation based on this MaxRecommendation. MaxRecommendation map[v1.ResourceName]ResourceQuantity `json:"maxRecommendation" protobuf:"bytes,2,name=maxRecommendation"` - // Recommendation is the latest recommendation value from VPA. + // Recommendation is the recommendation value from VPA that the tortoise controller observed in the last reconciliation.. Recommendation map[v1.ResourceName]ResourceQuantity `json:"recommendation" protobuf:"bytes,3,name=recommendation"` } diff --git a/api/v1beta2/tortoise_webhook.go b/api/v1beta2/tortoise_webhook.go index 88854086..66f0be04 100644 --- a/api/v1beta2/tortoise_webhook.go +++ b/api/v1beta2/tortoise_webhook.go @@ -40,6 +40,8 @@ import ( logf "sigs.k8s.io/controller-runtime/pkg/log" "sigs.k8s.io/controller-runtime/pkg/webhook" "sigs.k8s.io/controller-runtime/pkg/webhook/admission" + + "github.com/mercari/tortoise/pkg/annotation" ) // log is for logging in this package. @@ -83,8 +85,18 @@ func (r *Tortoise) Default() { return } - if len(d.Spec.Template.Spec.Containers) != len(r.Spec.ResourcePolicy) { - for _, c := range d.Spec.Template.Spec.Containers { + containers := d.Spec.Template.Spec.DeepCopy().Containers + if d.Annotations != nil { + if v, ok := d.Annotations[annotation.IstioSidecarInjectionAnnotation]; ok && v == "true" { + // If the deployment has the sidecar injection annotation, the Pods will have the sidecar container in addition. + containers = append(d.Spec.Template.Spec.Containers, v1.Container{ + Name: "istio-proxy", + }) + } + } + + if len(containers) != len(r.Spec.ResourcePolicy) { + for _, c := range containers { policyExist := false for _, p := range r.Spec.ResourcePolicy { if c.Name == p.ContainerName { diff --git a/config/crd/bases/autoscaling.mercari.com_tortoises.yaml b/config/crd/bases/autoscaling.mercari.com_tortoises.yaml index b4aefd13..233c4732 100644 --- a/config/crd/bases/autoscaling.mercari.com_tortoises.yaml +++ b/config/crd/bases/autoscaling.mercari.com_tortoises.yaml @@ -956,8 +956,9 @@ spec: format: date-time type: string type: object - description: Recommendation is the latest recommendation - value from VPA. + description: Recommendation is the recommendation value + from VPA that the tortoise controller observed in the + last reconciliation.. type: object required: - containerName @@ -1132,15 +1133,15 @@ spec: - type: string pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$ x-kubernetes-int-or-string: true - description: RecommendedResource is the recommendation - calculated by the tortoise. If AutoscalingPolicy is - vertical, it's the same value as the VPA suggests. + description: "RecommendedResource is the recommendation + calculated by the tortoise. \n If AutoscalingPolicy + is vertical, it's the same value as the VPA suggests. If AutoscalingPolicy is horizontal, it's basically the same value as the current resource request. But, when the number of replicas are too small or too large, tortoise may try to increase/decrease the amount of resources given to the container, so that the number - of replicas won't be very small or very large. + of replicas won't be very small or very large." type: object containerName: description: ContainerName is the name of target container. diff --git a/controllers/testdata/mutable-autoscalingpolicy-no-hpa-and-add-horizontal/after/vpa-Monitor.yaml b/controllers/testdata/mutable-autoscalingpolicy-no-hpa-and-add-horizontal/after/vpa-Monitor.yaml index 73acfb27..a167d409 100644 --- a/controllers/testdata/mutable-autoscalingpolicy-no-hpa-and-add-horizontal/after/vpa-Monitor.yaml +++ b/controllers/testdata/mutable-autoscalingpolicy-no-hpa-and-add-horizontal/after/vpa-Monitor.yaml @@ -40,5 +40,5 @@ status: cpu: "3" memory: 3Gi upperBound: - cpu: "5" - memory: 5Gi + cpu: "350m" + memory: 3.5Gi diff --git a/controllers/testdata/mutable-autoscalingpolicy-no-hpa-and-add-horizontal/before/tortoise.yaml b/controllers/testdata/mutable-autoscalingpolicy-no-hpa-and-add-horizontal/before/tortoise.yaml index 829f024c..99295f14 100644 --- a/controllers/testdata/mutable-autoscalingpolicy-no-hpa-and-add-horizontal/before/tortoise.yaml +++ b/controllers/testdata/mutable-autoscalingpolicy-no-hpa-and-add-horizontal/before/tortoise.yaml @@ -22,32 +22,32 @@ status: - containerName: app maxRecommendation: cpu: - quantity: "3" + quantity: "10" updatedAt: null memory: - quantity: 3Gi + quantity: 10Gi updatedAt: null recommendation: cpu: - quantity: "3" + quantity: "10" updatedAt: null memory: - quantity: 3Gi + quantity: 10Gi updatedAt: null - containerName: istio-proxy maxRecommendation: cpu: - quantity: "3" + quantity: "4" updatedAt: null memory: - quantity: 3Gi + quantity: 4Gi updatedAt: null recommendation: cpu: - quantity: "3" + quantity: "4" updatedAt: null memory: - quantity: 3Gi + quantity: 4Gi updatedAt: null recommendations: horizontal: @@ -71,12 +71,12 @@ status: vertical: containerResourceRecommendation: - RecommendedResource: - cpu: "3" - memory: 3Gi + cpu: "10" + memory: 10Gi containerName: app - RecommendedResource: - cpu: "3" - memory: 3Gi + cpu: "4" + memory: 4Gi containerName: istio-proxy targets: verticalPodAutoscalers: diff --git a/controllers/testdata/mutable-autoscalingpolicy-no-hpa-and-add-horizontal/before/vpa-Monitor.yaml b/controllers/testdata/mutable-autoscalingpolicy-no-hpa-and-add-horizontal/before/vpa-Monitor.yaml index 73acfb27..a167d409 100644 --- a/controllers/testdata/mutable-autoscalingpolicy-no-hpa-and-add-horizontal/before/vpa-Monitor.yaml +++ b/controllers/testdata/mutable-autoscalingpolicy-no-hpa-and-add-horizontal/before/vpa-Monitor.yaml @@ -40,5 +40,5 @@ status: cpu: "3" memory: 3Gi upperBound: - cpu: "5" - memory: 5Gi + cpu: "350m" + memory: 3.5Gi diff --git a/controllers/testdata/reconcile-for-the-istio-enabled-pod-working/after/hpa.yaml b/controllers/testdata/reconcile-for-the-istio-enabled-pod-working/after/hpa.yaml new file mode 100644 index 00000000..b760bbc8 --- /dev/null +++ b/controllers/testdata/reconcile-for-the-istio-enabled-pod-working/after/hpa.yaml @@ -0,0 +1,45 @@ +metadata: + annotations: + tortoise.autoscaling.mercari.com/managed-by-tortoise: "true" + tortoises.autoscaling.mercari.com/tortoise-name: mercari + name: tortoise-hpa-mercari + namespace: default +spec: + behavior: + scaleDown: + policies: + - periodSeconds: 90 + type: Percent + value: 2 + selectPolicy: Max + scaleUp: + policies: + - periodSeconds: 60 + type: Percent + value: 100 + selectPolicy: Max + stabilizationWindowSeconds: 0 + maxReplicas: 20 + metrics: + - containerResource: + container: app + name: cpu + target: + averageUtilization: 50 + type: Utilization + type: ContainerResource + - containerResource: + container: istio-proxy + name: cpu + target: + averageUtilization: 75 + type: Utilization + type: ContainerResource + minReplicas: 5 + scaleTargetRef: + apiVersion: apps/v1 + kind: Deployment + name: mercari-app +status: + currentMetrics: null + desiredReplicas: 0 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 new file mode 100644 index 00000000..0aba545c --- /dev/null +++ b/controllers/testdata/reconcile-for-the-istio-enabled-pod-working/after/tortoise.yaml @@ -0,0 +1,108 @@ +metadata: + name: mercari + namespace: default +spec: + resourcePolicy: + - autoscalingPolicy: + cpu: Horizontal + memory: Vertical + containerName: app + - autoscalingPolicy: + cpu: Horizontal + memory: Vertical + containerName: istio-proxy + targetRefs: + scaleTargetRef: + apiVersion: apps/v1 + kind: Deployment + name: mercari-app +status: + conditions: + tortoiseConditions: + - message: "HPA target utilization is updated" + reason: HPATargetUtilizationUpdated + status: "True" + type: HPATargetUtilizationUpdated + containerRecommendationFromVPA: + - containerName: app + maxRecommendation: + cpu: + quantity: "3" + updatedAt: null + memory: + quantity: 3Gi + updatedAt: null + recommendation: + cpu: + quantity: "3" + updatedAt: null + memory: + quantity: 3Gi + updatedAt: null + - containerName: istio-proxy + maxRecommendation: + cpu: + quantity: "3" + updatedAt: null + memory: + quantity: 3Gi + updatedAt: null + recommendation: + cpu: + quantity: "3" + updatedAt: null + memory: + quantity: 3Gi + updatedAt: null + recommendations: + horizontal: + maxReplicas: + - from: 0 + timezone: Local + to: 24 + updatedAt: "2023-10-06T01:15:46Z" + value: 20 + minReplicas: + - from: 0 + timezone: Local + to: 24 + updatedAt: "2023-10-06T01:15:46Z" + value: 5 + targetUtilizations: + - containerName: app + targetUtilization: + cpu: 50 + - containerName: istio-proxy + targetUtilization: + cpu: 75 + vertical: + containerResourceRecommendation: + - RecommendedResource: + cpu: "6" + memory: 3Gi + containerName: app + - RecommendedResource: + cpu: "4" + memory: 3Gi + containerName: istio-proxy + targets: + horizontalPodAutoscaler: tortoise-hpa-mercari + verticalPodAutoscalers: + - name: tortoise-updater-mercari + role: Updater + - name: tortoise-monitor-mercari + role: Monitor + tortoisePhase: Working + containerResourcePhases: + - containerName: "app" + resourcePhases: + cpu: + phase: Working + memory: + phase: Working + - containerName: "istio-proxy" + resourcePhases: + cpu: + phase: Working + memory: + phase: Working diff --git a/controllers/testdata/reconcile-for-the-istio-enabled-pod-working/after/vpa-Monitor.yaml b/controllers/testdata/reconcile-for-the-istio-enabled-pod-working/after/vpa-Monitor.yaml new file mode 100644 index 00000000..73acfb27 --- /dev/null +++ b/controllers/testdata/reconcile-for-the-istio-enabled-pod-working/after/vpa-Monitor.yaml @@ -0,0 +1,44 @@ +metadata: + annotations: + tortoise.autoscaling.mercari.com/managed-by-tortoise: "true" + tortoises.autoscaling.mercari.com/tortoise-name: mercari + name: tortoise-monitor-mercari + namespace: default +spec: + resourcePolicy: + containerPolicies: + - containerName: app + - containerName: istio-proxy + targetRef: + apiVersion: apps/v1 + kind: Deployment + name: mercari-app + updatePolicy: + updateMode: "Off" +status: + conditions: + - lastTransitionTime: null + status: "True" + type: RecommendationProvided + recommendation: + containerRecommendations: + - containerName: app + lowerBound: + cpu: "3" + memory: 3Gi + target: + cpu: "3" + memory: 3Gi + upperBound: + cpu: "5" + memory: 5Gi + - containerName: istio-proxy + lowerBound: + cpu: "3" + memory: 3Gi + target: + cpu: "3" + memory: 3Gi + upperBound: + cpu: "5" + memory: 5Gi diff --git a/controllers/testdata/reconcile-for-the-istio-enabled-pod-working/after/vpa-Updater.yaml b/controllers/testdata/reconcile-for-the-istio-enabled-pod-working/after/vpa-Updater.yaml new file mode 100644 index 00000000..07b36468 --- /dev/null +++ b/controllers/testdata/reconcile-for-the-istio-enabled-pod-working/after/vpa-Updater.yaml @@ -0,0 +1,48 @@ +metadata: + annotations: + tortoise.autoscaling.mercari.com/managed-by-tortoise: "true" + tortoises.autoscaling.mercari.com/tortoise-name: mercari + name: tortoise-updater-mercari + namespace: default +spec: + recommenders: + - name: tortoise-controller + resourcePolicy: + containerPolicies: + - containerName: app + - containerName: istio-proxy + targetRef: + apiVersion: apps/v1 + kind: Deployment + name: mercari-app + updatePolicy: + updateMode: Auto +status: + recommendation: + containerRecommendations: + - containerName: app + lowerBound: + cpu: "6" + memory: 3Gi + target: + cpu: "6" + memory: 3Gi + uncappedTarget: + cpu: "6" + memory: 3Gi + upperBound: + cpu: "6" + memory: 3Gi + - containerName: istio-proxy + lowerBound: + cpu: "4" + memory: 3Gi + target: + cpu: "4" + memory: 3Gi + uncappedTarget: + cpu: "4" + memory: 3Gi + upperBound: + cpu: "4" + memory: 3Gi diff --git a/controllers/testdata/reconcile-for-the-istio-enabled-pod-working/before/deployment.yaml b/controllers/testdata/reconcile-for-the-istio-enabled-pod-working/before/deployment.yaml new file mode 100644 index 00000000..a655a363 --- /dev/null +++ b/controllers/testdata/reconcile-for-the-istio-enabled-pod-working/before/deployment.yaml @@ -0,0 +1,27 @@ +metadata: + name: mercari-app + namespace: default + annotations: + sidecar.istio.io/inject: "true" + sidecar.istio.io/proxyCPU: "4" + sidecar.istio.io/proxyMemory: "4Gi" +spec: + selector: + matchLabels: + app: mercari + strategy: {} + template: + metadata: + creationTimestamp: null + labels: + app: mercari + spec: + containers: + - image: awesome-mercari-app-image + name: app + resources: + requests: + cpu: "10" + memory: 10Gi +status: + replicas: 10 diff --git a/controllers/testdata/reconcile-for-the-istio-enabled-pod-working/before/hpa.yaml b/controllers/testdata/reconcile-for-the-istio-enabled-pod-working/before/hpa.yaml new file mode 100644 index 00000000..d4c64cb0 --- /dev/null +++ b/controllers/testdata/reconcile-for-the-istio-enabled-pod-working/before/hpa.yaml @@ -0,0 +1,45 @@ +metadata: + annotations: + tortoise.autoscaling.mercari.com/managed-by-tortoise: "true" + tortoises.autoscaling.mercari.com/tortoise-name: mercari + name: tortoise-hpa-mercari + namespace: default +spec: + behavior: + scaleDown: + policies: + - periodSeconds: 90 + type: Percent + value: 2 + selectPolicy: Max + scaleUp: + policies: + - periodSeconds: 60 + type: Percent + value: 100 + selectPolicy: Max + stabilizationWindowSeconds: 0 + maxReplicas: 100 + metrics: + - containerResource: + container: app + name: cpu + target: + averageUtilization: 50 + type: Utilization + type: ContainerResource + - containerResource: + container: istio-proxy + name: cpu + target: + averageUtilization: 50 + type: Utilization + type: ContainerResource + minReplicas: 1 + scaleTargetRef: + apiVersion: apps/v1 + kind: Deployment + name: mercari-app +status: + currentMetrics: null + desiredReplicas: 0 diff --git a/controllers/testdata/reconcile-for-the-istio-enabled-pod-working/before/tortoise.yaml b/controllers/testdata/reconcile-for-the-istio-enabled-pod-working/before/tortoise.yaml new file mode 100644 index 00000000..1545cf80 --- /dev/null +++ b/controllers/testdata/reconcile-for-the-istio-enabled-pod-working/before/tortoise.yaml @@ -0,0 +1,93 @@ +metadata: + name: mercari + namespace: default +spec: + resourcePolicy: + - autoscalingPolicy: + cpu: Horizontal + memory: Vertical + containerName: app + - autoscalingPolicy: + cpu: Horizontal + memory: Vertical + containerName: istio-proxy + targetRefs: + scaleTargetRef: + apiVersion: apps/v1 + kind: Deployment + name: mercari-app +status: + conditions: + containerRecommendationFromVPA: + - containerName: app + maxRecommendation: + cpu: + quantity: "0" + updatedAt: null + memory: + quantity: "0" + updatedAt: null + recommendation: + cpu: + quantity: "0" + updatedAt: null + memory: + quantity: "0" + updatedAt: null + - containerName: istio-proxy + maxRecommendation: + cpu: + quantity: "0" + updatedAt: null + memory: + quantity: "0" + updatedAt: null + recommendation: + cpu: + quantity: "0" + updatedAt: null + memory: + quantity: "0" + updatedAt: null + recommendations: + horizontal: + maxReplicas: + - from: 0 + timezone: Local + to: 24 + updatedAt: "2023-10-06T01:15:46Z" + value: 15 + minReplicas: + - from: 0 + timezone: Local + to: 24 + updatedAt: "2023-10-06T01:15:46Z" + value: 3 + targetUtilizations: + - containerName: app + targetUtilization: + cpu: 50 + - containerName: istio-proxy + targetUtilization: + cpu: 50 + targets: + horizontalPodAutoscaler: tortoise-hpa-mercari + verticalPodAutoscalers: + - name: tortoise-updater-mercari + role: Updater + - name: tortoise-monitor-mercari + role: Monitor + tortoisePhase: Working + containerResourcePhases: + - containerName: "app" + resourcePhases: + cpu: + phase: Working + memory: + phase: Working + - containerName: "istio-proxy" + resourcePhases: + cpu: + phase: Working + memory: + phase: Working diff --git a/controllers/testdata/reconcile-for-the-istio-enabled-pod-working/before/vpa-Monitor.yaml b/controllers/testdata/reconcile-for-the-istio-enabled-pod-working/before/vpa-Monitor.yaml new file mode 100644 index 00000000..73acfb27 --- /dev/null +++ b/controllers/testdata/reconcile-for-the-istio-enabled-pod-working/before/vpa-Monitor.yaml @@ -0,0 +1,44 @@ +metadata: + annotations: + tortoise.autoscaling.mercari.com/managed-by-tortoise: "true" + tortoises.autoscaling.mercari.com/tortoise-name: mercari + name: tortoise-monitor-mercari + namespace: default +spec: + resourcePolicy: + containerPolicies: + - containerName: app + - containerName: istio-proxy + targetRef: + apiVersion: apps/v1 + kind: Deployment + name: mercari-app + updatePolicy: + updateMode: "Off" +status: + conditions: + - lastTransitionTime: null + status: "True" + type: RecommendationProvided + recommendation: + containerRecommendations: + - containerName: app + lowerBound: + cpu: "3" + memory: 3Gi + target: + cpu: "3" + memory: 3Gi + upperBound: + cpu: "5" + memory: 5Gi + - containerName: istio-proxy + lowerBound: + cpu: "3" + memory: 3Gi + target: + cpu: "3" + memory: 3Gi + upperBound: + cpu: "5" + memory: 5Gi diff --git a/controllers/testdata/reconcile-for-the-istio-enabled-pod-working/before/vpa-Updater.yaml b/controllers/testdata/reconcile-for-the-istio-enabled-pod-working/before/vpa-Updater.yaml new file mode 100644 index 00000000..28c9ee17 --- /dev/null +++ b/controllers/testdata/reconcile-for-the-istio-enabled-pod-working/before/vpa-Updater.yaml @@ -0,0 +1,20 @@ +metadata: + annotations: + tortoise.autoscaling.mercari.com/managed-by-tortoise: "true" + tortoises.autoscaling.mercari.com/tortoise-name: mercari + name: tortoise-updater-mercari + namespace: default +spec: + recommenders: + - name: tortoise-controller + resourcePolicy: + containerPolicies: + - containerName: app + - containerName: istio-proxy + targetRef: + apiVersion: apps/v1 + kind: Deployment + name: mercari-app + updatePolicy: + updateMode: Auto +status: {} diff --git a/controllers/tortoise_controller.go b/controllers/tortoise_controller.go index 0507c60f..96acc594 100644 --- a/controllers/tortoise_controller.go +++ b/controllers/tortoise_controller.go @@ -30,15 +30,17 @@ import ( "fmt" "time" - v1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/api/resource" "k8s.io/apimachinery/pkg/runtime" "k8s.io/client-go/tools/record" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/log" + "github.com/mercari/tortoise/api/v1beta2" autoscalingv1beta2 "github.com/mercari/tortoise/api/v1beta2" + "github.com/mercari/tortoise/pkg/annotation" "github.com/mercari/tortoise/pkg/deployment" "github.com/mercari/tortoise/pkg/hpa" "github.com/mercari/tortoise/pkg/recommender" @@ -58,6 +60,12 @@ type TortoiseReconciler struct { TortoiseService *tortoise.Service RecommenderService *recommender.Service EventRecorder record.EventRecorder + + // TODO: the following fields should be removed after we stop depending on deployment. + // IstioSidecarProxyDefaultCPU is the default CPU resource request of the istio sidecar proxy + IstioSidecarProxyDefaultCPU string + // IstioSidecarProxyDefaultMemory is the default Memory resource request of the istio sidecar proxy + IstioSidecarProxyDefaultMemory string } //+kubebuilder:rbac:groups=autoscaling.mercari.com,resources=tortoises,verbs=get;list;watch;create;update;patch;delete @@ -122,24 +130,80 @@ func (r *TortoiseReconciler) Reconcile(ctx context.Context, req ctrl.Request) (_ return ctrl.Result{RequeueAfter: requeueAfter}, nil } + // TODO: stop depending on deployment. + // https://github.com/mercari/tortoise/issues/129 + // + // Currently, we don't depend on the deployment on almost all cases, + // but we need to get the number of replicas from it + we need to take resource requests of each container when initializing tortoises. + // We should be able to eventually remove this dependency by using the number of replicas from scale subresource. dm, err := r.DeploymentService.GetDeploymentOnTortoise(ctx, tortoise) if err != nil { logger.Error(err, "failed to get deployment", "tortoise", req.NamespacedName) return ctrl.Result{}, err } + currentReplicaNum := dm.Status.Replicas + + if tortoise.Status.TortoisePhase == autoscalingv1beta2.TortoisePhaseInitializing || + tortoise.Status.Recommendations.Vertical.ContainerResourceRecommendation == nil /* only the integration test */ { + for _, c := range dm.Spec.Template.Spec.Containers { + rcr := v1beta2.RecommendedContainerResources{ + ContainerName: c.Name, + RecommendedResource: corev1.ResourceList{}, + } + for name, r := range c.Resources.Requests { + rcr.RecommendedResource[name] = r + } + tortoise.Status.Recommendations.Vertical.ContainerResourceRecommendation = append(tortoise.Status.Recommendations.Vertical.ContainerResourceRecommendation, rcr) + } + + if dm.Annotations != nil { + if v, ok := dm.Annotations[annotation.IstioSidecarInjectionAnnotation]; ok && v == "true" { + // Istio sidecar injection is enabled. + // Because the istio container spec is not in the deployment spec, we need to get it from the deployment's annotation. + + cpuReq, ok := dm.Annotations[annotation.IstioSidecarProxyCPUAnnotation] + if !ok { + cpuReq = r.IstioSidecarProxyDefaultCPU + } + cpu, err := resource.ParseQuantity(cpuReq) + if err != nil { + return ctrl.Result{}, fmt.Errorf("parse CPU request of istio sidecar: %w", err) + } + + memoryReq, ok := dm.Annotations[annotation.IstioSidecarProxyMemoryAnnotation] + if !ok { + memoryReq = r.IstioSidecarProxyDefaultMemory + } + memory, err := resource.ParseQuantity(memoryReq) + if err != nil { + return ctrl.Result{}, 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. + tortoise.Status.Recommendations.Vertical.ContainerResourceRecommendation = append(tortoise.Status.Recommendations.Vertical.ContainerResourceRecommendation, v1beta2.RecommendedContainerResources{ + ContainerName: "istio-proxy", + RecommendedResource: corev1.ResourceList{ + corev1.ResourceCPU: cpu, + corev1.ResourceMemory: memory, + }, + }) + } + } + } + // === Finish the part depending on deployment === + // From here, we shouldn't use `dm` anymore. - tortoise = r.TortoiseService.UpdateTortoisePhase(tortoise, dm, now) + tortoise = r.TortoiseService.UpdateTortoisePhase(tortoise, now) if tortoise.Status.TortoisePhase == autoscalingv1beta2.TortoisePhaseInitializing { logger.V(4).Info("initializing tortoise", "tortoise", req.NamespacedName) // need to initialize HPA and VPA. - if err := r.initializeVPAAndHPA(ctx, tortoise, dm, now); err != nil { + if err := r.initializeVPAAndHPA(ctx, tortoise, currentReplicaNum, now); err != nil { return ctrl.Result{}, fmt.Errorf("initialize VPAs and HPA: %w", err) } return ctrl.Result{RequeueAfter: r.Interval}, nil } - tortoise, err = r.HpaService.UpdateHPASpecFromTortoiseAutoscalingPolicy(ctx, tortoise, dm, now) + tortoise, err = r.HpaService.UpdateHPASpecFromTortoiseAutoscalingPolicy(ctx, tortoise, currentReplicaNum, now) if err != nil { logger.Error(err, "update HPA spec from Tortoise autoscaling policy", "tortoise", req.NamespacedName) return ctrl.Result{}, err @@ -162,7 +226,7 @@ func (r *TortoiseReconciler) Reconcile(ctx context.Context, req ctrl.Request) (_ } // VPA is ready, we mark all Vertical scaling resources as Running. - tortoise = vpa.MakeAllVerticalContainerResourcePhaseWorking(tortoise, now) + tortoise = vpa.SetAllVerticalContainerResourcePhaseWorking(tortoise, now) logger.V(4).Info("VPA created by tortoise is ready, proceeding to generate the recommendation", "tortoise", req.NamespacedName) hpa, err := r.HpaService.GetHPAOnTortoise(ctx, tortoise) @@ -173,7 +237,7 @@ func (r *TortoiseReconciler) Reconcile(ctx context.Context, req ctrl.Request) (_ tortoise = r.TortoiseService.UpdateUpperRecommendation(tortoise, monitorvpa) - tortoise, err = r.RecommenderService.UpdateRecommendations(ctx, tortoise, hpa, dm, now) + tortoise, err = r.RecommenderService.UpdateRecommendations(ctx, tortoise, hpa, currentReplicaNum, now) if err != nil { logger.Error(err, "update recommendation in tortoise", "tortoise", req.NamespacedName) return ctrl.Result{}, err @@ -239,9 +303,9 @@ func (r *TortoiseReconciler) deleteVPAAndHPA(ctx context.Context, tortoise *auto return nil } -func (r *TortoiseReconciler) initializeVPAAndHPA(ctx context.Context, tortoise *autoscalingv1beta2.Tortoise, dm *v1.Deployment, now time.Time) error { +func (r *TortoiseReconciler) initializeVPAAndHPA(ctx context.Context, tortoise *autoscalingv1beta2.Tortoise, replicaNum int32, now time.Time) error { // need to initialize HPA and VPA. - tortoise, err := r.HpaService.InitializeHPA(ctx, tortoise, dm, now) + tortoise, err := r.HpaService.InitializeHPA(ctx, tortoise, replicaNum, now) if err != nil { return err } diff --git a/main.go b/main.go index 976f08eb..7920ccf2 100644 --- a/main.go +++ b/main.go @@ -137,14 +137,16 @@ func main() { hpaService := hpa.New(mgr.GetClient(), eventRecorder, config.ReplicaReductionFactor, config.UpperTargetResourceUtilization, config.TortoiseHPATargetUtilizationMaxIncrease, config.TortoiseHPATargetUtilizationUpdateInterval) if err = (&controllers.TortoiseReconciler{ - Scheme: mgr.GetScheme(), - HpaService: hpaService, - VpaService: vpaClient, - DeploymentService: deployment.New(mgr.GetClient()), - RecommenderService: recommender.New(config.TTLHoursOfMinMaxReplicasRecommendation, config.MaxReplicasFactor, config.MinReplicasFactor, config.UpperTargetResourceUtilization, config.MinimumMinReplicas, config.PreferredReplicaNumUpperLimit, config.MaximumCPUCores, config.MaximumMemoryBytes), - TortoiseService: tortoiseService, - Interval: config.TortoiseUpdateInterval, - EventRecorder: eventRecorder, + Scheme: mgr.GetScheme(), + HpaService: hpaService, + VpaService: vpaClient, + DeploymentService: deployment.New(mgr.GetClient()), + RecommenderService: recommender.New(config.TTLHoursOfMinMaxReplicasRecommendation, config.MaxReplicasFactor, config.MinReplicasFactor, config.UpperTargetResourceUtilization, config.MinimumMinReplicas, config.PreferredReplicaNumUpperLimit, config.MaximumCPUCores, config.MaximumMemoryBytes), + TortoiseService: tortoiseService, + Interval: config.TortoiseUpdateInterval, + EventRecorder: eventRecorder, + IstioSidecarProxyDefaultCPU: config.IstioSidecarProxyDefaultCPU, + IstioSidecarProxyDefaultMemory: config.IstioSidecarProxyDefaultMemory, }).SetupWithManager(mgr); err != nil { setupLog.Error(err, "unable to create controller", "controller", "Tortoise") os.Exit(1) diff --git a/manifests/crd/apiextensions.k8s.io_v1_customresourcedefinition_tortoises.autoscaling.mercari.com.yaml b/manifests/crd/apiextensions.k8s.io_v1_customresourcedefinition_tortoises.autoscaling.mercari.com.yaml index 82bf2229..ae6c4fc1 100644 --- a/manifests/crd/apiextensions.k8s.io_v1_customresourcedefinition_tortoises.autoscaling.mercari.com.yaml +++ b/manifests/crd/apiextensions.k8s.io_v1_customresourcedefinition_tortoises.autoscaling.mercari.com.yaml @@ -776,7 +776,7 @@ spec: format: date-time type: string type: object - description: Recommendation is the latest recommendation value from VPA. + description: Recommendation is the recommendation value from VPA that the tortoise controller observed in the last reconciliation.. type: object required: - containerName @@ -934,7 +934,7 @@ spec: - type: string pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$ x-kubernetes-int-or-string: true - description: RecommendedResource is the recommendation calculated by the tortoise. If AutoscalingPolicy is vertical, it's the same value as the VPA suggests. If AutoscalingPolicy is horizontal, it's basically the same value as the current resource request. But, when the number of replicas are too small or too large, tortoise may try to increase/decrease the amount of resources given to the container, so that the number of replicas won't be very small or very large. + description: "RecommendedResource is the recommendation calculated by the tortoise. \n If AutoscalingPolicy is vertical, it's the same value as the VPA suggests. If AutoscalingPolicy is horizontal, it's basically the same value as the current resource request. But, when the number of replicas are too small or too large, tortoise may try to increase/decrease the amount of resources given to the container, so that the number of replicas won't be very small or very large." type: object containerName: description: ContainerName is the name of target container. diff --git a/manifests/default/apiextensions.k8s.io_v1_customresourcedefinition_tortoises.autoscaling.mercari.com.yaml b/manifests/default/apiextensions.k8s.io_v1_customresourcedefinition_tortoises.autoscaling.mercari.com.yaml index d40e7908..97817e38 100644 --- a/manifests/default/apiextensions.k8s.io_v1_customresourcedefinition_tortoises.autoscaling.mercari.com.yaml +++ b/manifests/default/apiextensions.k8s.io_v1_customresourcedefinition_tortoises.autoscaling.mercari.com.yaml @@ -776,7 +776,7 @@ spec: format: date-time type: string type: object - description: Recommendation is the latest recommendation value from VPA. + description: Recommendation is the recommendation value from VPA that the tortoise controller observed in the last reconciliation.. type: object required: - containerName @@ -934,7 +934,7 @@ spec: - type: string pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$ x-kubernetes-int-or-string: true - description: RecommendedResource is the recommendation calculated by the tortoise. If AutoscalingPolicy is vertical, it's the same value as the VPA suggests. If AutoscalingPolicy is horizontal, it's basically the same value as the current resource request. But, when the number of replicas are too small or too large, tortoise may try to increase/decrease the amount of resources given to the container, so that the number of replicas won't be very small or very large. + description: "RecommendedResource is the recommendation calculated by the tortoise. \n If AutoscalingPolicy is vertical, it's the same value as the VPA suggests. If AutoscalingPolicy is horizontal, it's basically the same value as the current resource request. But, when the number of replicas are too small or too large, tortoise may try to increase/decrease the amount of resources given to the container, so that the number of replicas won't be very small or very large." type: object containerName: description: ContainerName is the name of target container. diff --git a/pkg/annotation/external_annotation.go b/pkg/annotation/external_annotation.go new file mode 100644 index 00000000..633c3d56 --- /dev/null +++ b/pkg/annotation/external_annotation.go @@ -0,0 +1,9 @@ +package annotation + +const ( + // IstioSidecarInjectionAnnotation - If this annotation is set to "true", it means that the sidecar injection is enabled. + IstioSidecarInjectionAnnotation = "sidecar.istio.io/inject" + + IstioSidecarProxyCPUAnnotation = "sidecar.istio.io/proxyCPU" + IstioSidecarProxyMemoryAnnotation = "sidecar.istio.io/proxyMemory" +) diff --git a/pkg/config/config.go b/pkg/config/config.go index 22ec5810..69d0410a 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -39,6 +39,13 @@ type Config struct { TortoiseHPATargetUtilizationMaxIncrease int `yaml:"TortoiseHPATargetUtilizationMaxIncrease"` // TortoiseHPATargetUtilizationUpdateInterval is the interval of updating target utilization of each HPA (default: 1h) TortoiseHPATargetUtilizationUpdateInterval time.Duration `yaml:"TortoiseHPATargetUtilizationUpdateInterval"` + + // TODO: the following fields should be removed after we stop depending on deployment. + // So, we don't put them in the documentation. + // IstioSidecarProxyDefaultCPU is the default CPU resource request of the istio sidecar proxy (default: 100m) + IstioSidecarProxyDefaultCPU string + // IstioSidecarProxyDefaultMemory is the default Memory resource request of the istio sidecar proxy (default: 200Mi) + IstioSidecarProxyDefaultMemory string } // ParseConfig parses the config file (yaml) and returns Config. @@ -59,6 +66,8 @@ func ParseConfig(path string) (*Config, error) { TortoiseUpdateInterval: 15 * time.Second, TortoiseHPATargetUtilizationMaxIncrease: 5, TortoiseHPATargetUtilizationUpdateInterval: time.Hour, + IstioSidecarProxyDefaultCPU: "100m", + IstioSidecarProxyDefaultMemory: "200Mi", } if path == "" { return config, nil diff --git a/pkg/config/config_test.go b/pkg/config/config_test.go index 6afb148a..b4d2f9b2 100644 --- a/pkg/config/config_test.go +++ b/pkg/config/config_test.go @@ -37,6 +37,8 @@ func TestParseConfig(t *testing.T) { TortoiseUpdateInterval: 1 * time.Hour, TortoiseHPATargetUtilizationMaxIncrease: 10, TortoiseHPATargetUtilizationUpdateInterval: 3 * time.Hour, + IstioSidecarProxyDefaultCPU: "100m", + IstioSidecarProxyDefaultMemory: "200Mi", }, }, { @@ -60,6 +62,8 @@ func TestParseConfig(t *testing.T) { TortoiseUpdateInterval: 15 * time.Second, TortoiseHPATargetUtilizationMaxIncrease: 5, TortoiseHPATargetUtilizationUpdateInterval: 1 * time.Hour, + IstioSidecarProxyDefaultCPU: "100m", + IstioSidecarProxyDefaultMemory: "200Mi", }, }, { @@ -90,6 +94,8 @@ func TestParseConfig(t *testing.T) { TortoiseUpdateInterval: 15 * time.Second, TortoiseHPATargetUtilizationMaxIncrease: 5, TortoiseHPATargetUtilizationUpdateInterval: time.Hour, + IstioSidecarProxyDefaultCPU: "100m", + IstioSidecarProxyDefaultMemory: "200Mi", }, }, } diff --git a/pkg/hpa/service.go b/pkg/hpa/service.go index f05e653e..f9d2c5cc 100644 --- a/pkg/hpa/service.go +++ b/pkg/hpa/service.go @@ -8,7 +8,6 @@ import ( "sort" "time" - v1 "k8s.io/api/apps/v1" v2 "k8s.io/api/autoscaling/v2" corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" @@ -47,7 +46,7 @@ func New(c client.Client, recorder record.EventRecorder, replicaReductionFactor } } -func (c *Service) InitializeHPA(ctx context.Context, tortoise *autoscalingv1beta2.Tortoise, dm *v1.Deployment, now time.Time) (*autoscalingv1beta2.Tortoise, error) { +func (c *Service) InitializeHPA(ctx context.Context, tortoise *autoscalingv1beta2.Tortoise, replicaNum int32, now time.Time) (*autoscalingv1beta2.Tortoise, error) { logger := log.FromContext(ctx) // if all policy is off or Vertical, we don't need HPA. if !HasHorizontal(tortoise) { @@ -71,7 +70,7 @@ func (c *Service) InitializeHPA(ctx context.Context, tortoise *autoscalingv1beta logger.V(4).Info("no existing HPA specified, creating HPA") // create default HPA. - _, tortoise, err := c.CreateHPA(ctx, tortoise, dm, now) + _, tortoise, err := c.CreateHPA(ctx, tortoise, replicaNum, now) if err != nil { return tortoise, fmt.Errorf("create hpa: %w", err) } @@ -232,7 +231,7 @@ func (c *Service) addHPAMetricsFromTortoiseAutoscalingPolicy(ctx context.Context return currenthpa, tortoise, hpaEdited } -func (c *Service) CreateHPA(ctx context.Context, tortoise *autoscalingv1beta2.Tortoise, dm *v1.Deployment, now time.Time) (*v2.HorizontalPodAutoscaler, *autoscalingv1beta2.Tortoise, error) { +func (c *Service) CreateHPA(ctx context.Context, tortoise *autoscalingv1beta2.Tortoise, replicaNum int32, now time.Time) (*v2.HorizontalPodAutoscaler, *autoscalingv1beta2.Tortoise, error) { if !HasHorizontal(tortoise) { // no need to create HPA return nil, tortoise, nil @@ -257,8 +256,8 @@ func (c *Service) CreateHPA(ctx context.Context, tortoise *autoscalingv1beta2.To Name: tortoise.Spec.TargetRefs.ScaleTargetRef.Name, APIVersion: tortoise.Spec.TargetRefs.ScaleTargetRef.APIVersion, }, - MinReplicas: pointer.Int32(int32(math.Ceil(float64(dm.Status.Replicas) / 2.0))), - MaxReplicas: dm.Status.Replicas * 2, + MinReplicas: pointer.Int32(int32(math.Ceil(float64(replicaNum) / 2.0))), + MaxReplicas: replicaNum * 2, Behavior: &v2.HorizontalPodAutoscalerBehavior{ ScaleUp: &v2.HPAScalingRules{ Policies: []v2.HPAScalingPolicy{ @@ -427,7 +426,7 @@ func (c *Service) ChangeHPAFromTortoiseRecommendation(tortoise *autoscalingv1bet return hpa, tortoise, nil } -func (c *Service) UpdateHPASpecFromTortoiseAutoscalingPolicy(ctx context.Context, tortoise *autoscalingv1beta2.Tortoise, dm *v1.Deployment, now time.Time) (*autoscalingv1beta2.Tortoise, error) { +func (c *Service) UpdateHPASpecFromTortoiseAutoscalingPolicy(ctx context.Context, tortoise *autoscalingv1beta2.Tortoise, replicaNum int32, now time.Time) (*autoscalingv1beta2.Tortoise, error) { if tortoise.Spec.UpdateMode == autoscalingv1beta2.UpdateModeOff { // When UpdateMode is Off, we don't update HPA. return tortoise, nil @@ -452,7 +451,7 @@ func (c *Service) UpdateHPASpecFromTortoiseAutoscalingPolicy(ctx context.Context // - the user didn't specify Horizontal in any autoscalingPolicy previously, // but just updated tortoise to have Horizontal in some. // - In that case, we need to create an initial HPA. - tortoise, err = c.InitializeHPA(ctx, tortoise, dm, now) + tortoise, err = c.InitializeHPA(ctx, tortoise, replicaNum, now) if err != nil { return tortoise, fmt.Errorf("initialize hpa: %w", err) } diff --git a/pkg/hpa/service_test.go b/pkg/hpa/service_test.go index eb863fa8..c371a9b2 100644 --- a/pkg/hpa/service_test.go +++ b/pkg/hpa/service_test.go @@ -7,7 +7,6 @@ import ( "github.com/google/go-cmp/cmp" "github.com/google/go-cmp/cmp/cmpopts" - appv1 "k8s.io/api/apps/v1" v2 "k8s.io/api/autoscaling/v2" v1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" @@ -1533,8 +1532,8 @@ func ptrInt32(i int32) *int32 { func TestService_InitializeHPA(t *testing.T) { type args struct { - tortoise *autoscalingv1beta2.Tortoise - dm *appv1.Deployment + tortoise *autoscalingv1beta2.Tortoise + replicaNum int32 } tests := []struct { name string @@ -1577,30 +1576,7 @@ func TestService_InitializeHPA(t *testing.T) { }, }, }, - dm: &appv1.Deployment{ - ObjectMeta: metav1.ObjectMeta{ - Name: "deployment", - Namespace: "default", - }, - Spec: appv1.DeploymentSpec{ - Replicas: pointer.Int32(4), - Template: v1.PodTemplateSpec{ - Spec: v1.PodSpec{ - Containers: []v1.Container{ - { - Name: "app", - }, - { - Name: "istio-proxy", - }, - }, - }, - }, - }, - Status: appv1.DeploymentStatus{ - Replicas: 4, - }, - }, + replicaNum: 4, }, afterHPA: &v2.HorizontalPodAutoscaler{ ObjectMeta: metav1.ObjectMeta{ @@ -1693,27 +1669,7 @@ func TestService_InitializeHPA(t *testing.T) { }, }, }, - dm: &appv1.Deployment{ - ObjectMeta: metav1.ObjectMeta{ - Name: "deployment", - Namespace: "default", - }, - Spec: appv1.DeploymentSpec{ - Replicas: pointer.Int32(4), - Template: v1.PodTemplateSpec{ - Spec: v1.PodSpec{ - Containers: []v1.Container{ - { - Name: "app", - }, - }, - }, - }, - }, - Status: appv1.DeploymentStatus{ - Replicas: 4, - }, - }, + replicaNum: 4, }, initialHPA: &v2.HorizontalPodAutoscaler{ ObjectMeta: metav1.ObjectMeta{ @@ -1780,7 +1736,7 @@ func TestService_InitializeHPA(t *testing.T) { if tt.initialHPA != nil { c = New(fake.NewClientBuilder().WithRuntimeObjects(tt.initialHPA).Build(), record.NewFakeRecorder(10), 0.95, 90, 100, time.Hour) } - _, err := c.InitializeHPA(context.Background(), tt.args.tortoise, tt.args.dm, time.Now()) + _, err := c.InitializeHPA(context.Background(), tt.args.tortoise, tt.args.replicaNum, time.Now()) if (err != nil) != tt.wantErr { t.Errorf("Service.InitializeHPA() error = %v, wantErr %v", err, tt.wantErr) } @@ -1799,8 +1755,8 @@ func TestService_InitializeHPA(t *testing.T) { func TestService_UpdateHPASpecFromTortoiseAutoscalingPolicy(t *testing.T) { type args struct { - tortoise *autoscalingv1beta2.Tortoise - dm *appv1.Deployment + tortoise *autoscalingv1beta2.Tortoise + replicaNum int32 } tests := []struct { name string @@ -1873,27 +1829,7 @@ func TestService_UpdateHPASpecFromTortoiseAutoscalingPolicy(t *testing.T) { }, }, }, - dm: &appv1.Deployment{ - ObjectMeta: metav1.ObjectMeta{ - Name: "deployment", - Namespace: "default", - }, - Spec: appv1.DeploymentSpec{ - Replicas: pointer.Int32(4), - Template: v1.PodTemplateSpec{ - Spec: v1.PodSpec{ - Containers: []v1.Container{ - { - Name: "app", - }, - }, - }, - }, - }, - Status: appv1.DeploymentStatus{ - Replicas: 4, - }, - }, + replicaNum: 4, }, initialHPA: &v2.HorizontalPodAutoscaler{ ObjectMeta: metav1.ObjectMeta{ @@ -2114,27 +2050,7 @@ func TestService_UpdateHPASpecFromTortoiseAutoscalingPolicy(t *testing.T) { }, }, }, - dm: &appv1.Deployment{ - ObjectMeta: metav1.ObjectMeta{ - Name: "deployment", - Namespace: "default", - }, - Spec: appv1.DeploymentSpec{ - Replicas: pointer.Int32(4), - Template: v1.PodTemplateSpec{ - Spec: v1.PodSpec{ - Containers: []v1.Container{ - { - Name: "app", - }, - }, - }, - }, - }, - Status: appv1.DeploymentStatus{ - Replicas: 4, - }, - }, + replicaNum: 4, }, initialHPA: &v2.HorizontalPodAutoscaler{ ObjectMeta: metav1.ObjectMeta{ @@ -2356,27 +2272,7 @@ func TestService_UpdateHPASpecFromTortoiseAutoscalingPolicy(t *testing.T) { }, }, }, - dm: &appv1.Deployment{ - ObjectMeta: metav1.ObjectMeta{ - Name: "deployment", - Namespace: "default", - }, - Spec: appv1.DeploymentSpec{ - Replicas: pointer.Int32(4), - Template: v1.PodTemplateSpec{ - Spec: v1.PodSpec{ - Containers: []v1.Container{ - { - Name: "app", - }, - }, - }, - }, - }, - Status: appv1.DeploymentStatus{ - Replicas: 4, - }, - }, + replicaNum: 4, }, initialHPA: &v2.HorizontalPodAutoscaler{ ObjectMeta: metav1.ObjectMeta{ @@ -2502,7 +2398,7 @@ func TestService_UpdateHPASpecFromTortoiseAutoscalingPolicy(t *testing.T) { if tt.initialHPA != nil { c = New(fake.NewClientBuilder().WithRuntimeObjects(tt.initialHPA).Build(), record.NewFakeRecorder(10), 0.95, 90, 100, time.Hour) } - tortoise, err := c.UpdateHPASpecFromTortoiseAutoscalingPolicy(context.Background(), tt.args.tortoise, tt.args.dm, time.Now()) + tortoise, err := c.UpdateHPASpecFromTortoiseAutoscalingPolicy(context.Background(), tt.args.tortoise, tt.args.replicaNum, time.Now()) if (err != nil) != tt.wantErr { t.Errorf("Service.UpdateHPASpecFromTortoiseAutoscalingPolicy() error = %v, wantErr %v", err, tt.wantErr) return diff --git a/pkg/recommender/recommender.go b/pkg/recommender/recommender.go index 35a4826b..e06fdee2 100644 --- a/pkg/recommender/recommender.go +++ b/pkg/recommender/recommender.go @@ -7,7 +7,6 @@ import ( "math" "time" - v1 "k8s.io/api/apps/v1" v2 "k8s.io/api/autoscaling/v2" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/resource" @@ -55,12 +54,13 @@ func New( } } -func (s *Service) updateVPARecommendation(tortoise *v1beta2.Tortoise, deployment *v1.Deployment, hpa *v2.HorizontalPodAutoscaler) (*v1beta2.Tortoise, error) { +func (s *Service) updateVPARecommendation(tortoise *v1beta2.Tortoise, hpa *v2.HorizontalPodAutoscaler, replicaNum int32) (*v1beta2.Tortoise, error) { requestMap := map[string]map[corev1.ResourceName]resource.Quantity{} - for _, c := range deployment.Spec.Template.Spec.Containers { - requestMap[c.Name] = map[corev1.ResourceName]resource.Quantity{} - for rn, q := range c.Resources.Requests { - requestMap[c.Name][rn] = q + // This ContainerResourceRecommendationkshould be the current resource requests. + for _, r := range tortoise.Status.Recommendations.Vertical.ContainerResourceRecommendation { + requestMap[r.ContainerName] = map[corev1.ResourceName]resource.Quantity{} + for resourcename, value := range r.RecommendedResource { + requestMap[r.ContainerName][resourcename] = value } } @@ -99,7 +99,7 @@ func (s *Service) updateVPARecommendation(tortoise *v1beta2.Tortoise, deployment if !ok { return nil, fmt.Errorf("no %s recommendation from VPA for the container %s", k, r.ContainerName) } - newSize, err := s.calculateBestNewSize(p, r.ContainerName, recom, k, hpa, deployment, req, r.MinAllocatedResources) + newSize, err := s.calculateBestNewSize(p, r.ContainerName, recom, k, hpa, replicaNum, req, r.MinAllocatedResources, len(requestMap) > 1) if err != nil { return nil, err } @@ -117,23 +117,23 @@ func (s *Service) updateVPARecommendation(tortoise *v1beta2.Tortoise, deployment // calculateBestNewSize calculates the best new resource request based on the current replica number and the recommended resource request. // Even if the autoscaling policy is Horizontal, this function may suggest the vertical scaling, see comments in the function. -func (s *Service) calculateBestNewSize(p v1beta2.AutoscalingType, containerName string, recommendedResourceRequest resource.Quantity, k corev1.ResourceName, hpa *v2.HorizontalPodAutoscaler, deployment *v1.Deployment, resourceRequest resource.Quantity, minAllocatedResources corev1.ResourceList) (int64, error) { +func (s *Service) calculateBestNewSize(p v1beta2.AutoscalingType, containerName string, recommendedResourceRequest resource.Quantity, k corev1.ResourceName, hpa *v2.HorizontalPodAutoscaler, replicaNum int32, resourceRequest resource.Quantity, minAllocatedResources corev1.ResourceList, isMultipleContainersPod bool) (int64, error) { // When the current replica num is more than or equal to the preferredReplicaNumUpperLimit, // make the container size bigger (just multiple by 1.1) so that the replica number will be descreased. - if deployment.Status.Replicas >= s.preferredReplicaNumUpperLimit { + if replicaNum >= s.preferredReplicaNumUpperLimit { // We keep increasing the size until we hit the maxResourceSize. newSize := int64(float64(resourceRequest.MilliValue()) * 1.1) return s.justifyNewSizeByMaxMin(newSize, k, resourceRequest, minAllocatedResources), nil } - if deployment.Status.Replicas <= s.minimumMinReplicas || p == v1beta2.AutoscalingTypeVertical { + if replicaNum <= s.minimumMinReplicas || p == v1beta2.AutoscalingTypeVertical { // It's the simplest case. // The user configures Vertical on this container's resource. This is just vertical scaling. newSize := recommendedResourceRequest.MilliValue() return s.justifyNewSizeByMaxMin(newSize, k, resourceRequest, minAllocatedResources), nil } - if deployment.Status.Replicas <= s.minimumMinReplicas || p == v1beta2.AutoscalingTypeVertical { + if replicaNum <= s.minimumMinReplicas || p == v1beta2.AutoscalingTypeVertical { // The current replica number is less than or equal to the minimumMinReplicas. // The replica number is too small and hits the minReplicas. // So, the resource utilization might be super low because HPA cannot scale down further. @@ -155,7 +155,7 @@ func (s *Service) calculateBestNewSize(p v1beta2.AutoscalingType, containerName } upperUtilization := math.Ceil((float64(recommendedResourceRequest.MilliValue()) / float64(resourceRequest.MilliValue())) * 100) - if targetUtilizationValue > int32(upperUtilization) && len(deployment.Spec.Template.Spec.Containers) >= 2 { + if targetUtilizationValue > int32(upperUtilization) && isMultipleContainersPod { // upperUtilization is less than targetUtilizationValue, which seems weird in normal cases. // In this case, most likely the container size is unbalanced. (= we need multi-container specific optimization) // So, for example, when app:istio use the resource in the ratio of 1:5, but the resource request is 1:1, @@ -188,13 +188,13 @@ func (s *Service) justifyNewSizeByMaxMin(newSize int64, k corev1.ResourceName, r return newSize } -func (s *Service) updateHPARecommendation(ctx context.Context, tortoise *v1beta2.Tortoise, hpa *v2.HorizontalPodAutoscaler, deployment *v1.Deployment, now time.Time) (*v1beta2.Tortoise, error) { +func (s *Service) updateHPARecommendation(ctx context.Context, tortoise *v1beta2.Tortoise, hpa *v2.HorizontalPodAutoscaler, replicaNum int32, now time.Time) (*v1beta2.Tortoise, error) { var err error - tortoise, err = s.updateHPATargetUtilizationRecommendations(ctx, tortoise, hpa, deployment) + tortoise, err = s.updateHPATargetUtilizationRecommendations(ctx, tortoise, hpa) if err != nil { return tortoise, fmt.Errorf("update HPA target utilization recommendations: %w", err) } - tortoise, err = s.updateHPAMinMaxReplicasRecommendations(tortoise, deployment, now) + tortoise, err = s.updateHPAMinMaxReplicasRecommendations(tortoise, replicaNum, now) if err != nil { return tortoise, err } @@ -202,13 +202,13 @@ func (s *Service) updateHPARecommendation(ctx context.Context, tortoise *v1beta2 return tortoise, nil } -func (s *Service) UpdateRecommendations(ctx context.Context, tortoise *v1beta2.Tortoise, hpa *v2.HorizontalPodAutoscaler, deployment *v1.Deployment, now time.Time) (*v1beta2.Tortoise, error) { +func (s *Service) UpdateRecommendations(ctx context.Context, tortoise *v1beta2.Tortoise, hpa *v2.HorizontalPodAutoscaler, replicaNum int32, now time.Time) (*v1beta2.Tortoise, error) { var err error - tortoise, err = s.updateHPARecommendation(ctx, tortoise, hpa, deployment, now) + tortoise, err = s.updateHPARecommendation(ctx, tortoise, hpa, replicaNum, now) if err != nil { return tortoise, fmt.Errorf("update HPA recommendations: %w", err) } - tortoise, err = s.updateVPARecommendation(tortoise, deployment, hpa) + tortoise, err = s.updateVPARecommendation(tortoise, hpa, replicaNum) if err != nil { return tortoise, fmt.Errorf("update VPA recommendations: %w", err) } @@ -216,8 +216,8 @@ func (s *Service) UpdateRecommendations(ctx context.Context, tortoise *v1beta2.T return tortoise, nil } -func (s *Service) updateHPAMinMaxReplicasRecommendations(tortoise *v1beta2.Tortoise, deployment *v1.Deployment, now time.Time) (*v1beta2.Tortoise, error) { - currentReplicaNum := float64(deployment.Status.Replicas) +func (s *Service) updateHPAMinMaxReplicasRecommendations(tortoise *v1beta2.Tortoise, replicaNum int32, now time.Time) (*v1beta2.Tortoise, error) { + currentReplicaNum := float64(replicaNum) min, err := s.updateMaxMinReplicasRecommendation(int32(math.Ceil(currentReplicaNum*s.minReplicasFactor)), tortoise.Status.Recommendations.Horizontal.MinReplicas, now, s.minimumMinReplicas) if err != nil { return tortoise, fmt.Errorf("update MinReplicas recommendation: %w", err) @@ -262,13 +262,15 @@ func (s *Service) updateMaxMinReplicasRecommendation(value int32, recommendation return recommendations, nil } -func (s *Service) updateHPATargetUtilizationRecommendations(ctx context.Context, tortoise *v1beta2.Tortoise, hpa *v2.HorizontalPodAutoscaler, deployment *v1.Deployment) (*v1beta2.Tortoise, error) { +func (s *Service) updateHPATargetUtilizationRecommendations(ctx context.Context, tortoise *v1beta2.Tortoise, hpa *v2.HorizontalPodAutoscaler) (*v1beta2.Tortoise, error) { logger := log.FromContext(ctx) + requestMap := map[string]map[corev1.ResourceName]resource.Quantity{} - for _, c := range deployment.Spec.Template.Spec.Containers { - requestMap[c.Name] = map[corev1.ResourceName]resource.Quantity{} - for rn, q := range c.Resources.Requests { - requestMap[c.Name][rn] = q + // This ContainerResourceRecommendationkshould be the current resource requests. + for _, r := range tortoise.Status.Recommendations.Vertical.ContainerResourceRecommendation { + requestMap[r.ContainerName] = map[corev1.ResourceName]resource.Quantity{} + for resourcename, value := range r.RecommendedResource { + requestMap[r.ContainerName][resourcename] = value } } diff --git a/pkg/recommender/recommender_test.go b/pkg/recommender/recommender_test.go index 3b79d62b..45ac9652 100644 --- a/pkg/recommender/recommender_test.go +++ b/pkg/recommender/recommender_test.go @@ -7,7 +7,6 @@ import ( "github.com/google/go-cmp/cmp" "github.com/google/go-cmp/cmp/cmpopts" - v1 "k8s.io/api/apps/v1" v2 "k8s.io/api/autoscaling/v2" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/resource" @@ -15,13 +14,13 @@ import ( "k8s.io/utils/pointer" "github.com/mercari/tortoise/api/v1beta2" + "github.com/mercari/tortoise/pkg/utils" ) func TestUpdateRecommendation(t *testing.T) { type args struct { - tortoise *v1beta2.Tortoise - hpa *v2.HorizontalPodAutoscaler - deployment *v1.Deployment + tortoise *v1beta2.Tortoise + hpa *v2.HorizontalPodAutoscaler } tests := []struct { name string @@ -78,6 +77,26 @@ func TestUpdateRecommendation(t *testing.T) { }, }, }, + Recommendations: v1beta2.Recommendations{ + Vertical: v1beta2.VerticalRecommendations{ + ContainerResourceRecommendation: []v1beta2.RecommendedContainerResources{ + { + ContainerName: "app", + RecommendedResource: corev1.ResourceList{ + corev1.ResourceMemory: resource.MustParse("5Gi"), + corev1.ResourceCPU: resource.MustParse("5"), + }, + }, + { + ContainerName: "istio-proxy", + RecommendedResource: corev1.ResourceList{ + corev1.ResourceMemory: resource.MustParse("1Gi"), + corev1.ResourceCPU: resource.MustParse("1"), + }, + }, + }, + }, + }, }, }, hpa: &v2.HorizontalPodAutoscaler{ @@ -123,34 +142,6 @@ func TestUpdateRecommendation(t *testing.T) { }, Status: v2.HorizontalPodAutoscalerStatus{}, }, - deployment: &v1.Deployment{ - Spec: v1.DeploymentSpec{ - Template: corev1.PodTemplateSpec{ - Spec: corev1.PodSpec{ - Containers: []corev1.Container{ - { - Name: "app", - Resources: corev1.ResourceRequirements{ - Requests: corev1.ResourceList{ - corev1.ResourceMemory: resource.MustParse("5Gi"), - corev1.ResourceCPU: resource.MustParse("5"), - }, - }, - }, - { - Name: "istio-proxy", - Resources: corev1.ResourceRequirements{ - Requests: corev1.ResourceList{ - corev1.ResourceMemory: resource.MustParse("1Gi"), - corev1.ResourceCPU: resource.MustParse("1"), - }, - }, - }, - }, - }, - }, - }, - }, }, want: &v1beta2.Tortoise{ Spec: v1beta2.TortoiseSpec{ @@ -189,6 +180,24 @@ func TestUpdateRecommendation(t *testing.T) { }, }, }, + Vertical: v1beta2.VerticalRecommendations{ + ContainerResourceRecommendation: []v1beta2.RecommendedContainerResources{ + { + ContainerName: "app", + RecommendedResource: corev1.ResourceList{ + corev1.ResourceMemory: resource.MustParse("5Gi"), + corev1.ResourceCPU: resource.MustParse("5"), + }, + }, + { + ContainerName: "istio-proxy", + RecommendedResource: corev1.ResourceList{ + corev1.ResourceMemory: resource.MustParse("1Gi"), + corev1.ResourceCPU: resource.MustParse("1"), + }, + }, + }, + }, }, Conditions: v1beta2.Conditions{ ContainerRecommendationFromVPA: []v1beta2.ContainerRecommendationFromVPA{ @@ -268,6 +277,26 @@ func TestUpdateRecommendation(t *testing.T) { }, }, }, + Recommendations: v1beta2.Recommendations{ + Vertical: v1beta2.VerticalRecommendations{ + ContainerResourceRecommendation: []v1beta2.RecommendedContainerResources{ + { + ContainerName: "app", + RecommendedResource: corev1.ResourceList{ + corev1.ResourceMemory: resource.MustParse("5Gi"), + corev1.ResourceCPU: resource.MustParse("5"), + }, + }, + { + ContainerName: "istio-proxy", + RecommendedResource: corev1.ResourceList{ + corev1.ResourceMemory: resource.MustParse("1Gi"), + corev1.ResourceCPU: resource.MustParse("1"), + }, + }, + }, + }, + }, }, }, hpa: &v2.HorizontalPodAutoscaler{ @@ -303,34 +332,6 @@ func TestUpdateRecommendation(t *testing.T) { }, Status: v2.HorizontalPodAutoscalerStatus{}, }, - deployment: &v1.Deployment{ - Spec: v1.DeploymentSpec{ - Template: corev1.PodTemplateSpec{ - Spec: corev1.PodSpec{ - Containers: []corev1.Container{ - { - Name: "app", - Resources: corev1.ResourceRequirements{ - Requests: corev1.ResourceList{ - corev1.ResourceMemory: resource.MustParse("5Gi"), - corev1.ResourceCPU: resource.MustParse("5"), - }, - }, - }, - { - Name: "istio-proxy", - Resources: corev1.ResourceRequirements{ - Requests: corev1.ResourceList{ - corev1.ResourceMemory: resource.MustParse("1Gi"), - corev1.ResourceCPU: resource.MustParse("1"), - }, - }, - }, - }, - }, - }, - }, - }, }, want: &v1beta2.Tortoise{ Spec: v1beta2.TortoiseSpec{ @@ -353,6 +354,24 @@ func TestUpdateRecommendation(t *testing.T) { }, Status: v1beta2.TortoiseStatus{ Recommendations: v1beta2.Recommendations{ + Vertical: v1beta2.VerticalRecommendations{ + ContainerResourceRecommendation: []v1beta2.RecommendedContainerResources{ + { + ContainerName: "app", + RecommendedResource: corev1.ResourceList{ + corev1.ResourceMemory: resource.MustParse("5Gi"), + corev1.ResourceCPU: resource.MustParse("5"), + }, + }, + { + ContainerName: "istio-proxy", + RecommendedResource: corev1.ResourceList{ + corev1.ResourceMemory: resource.MustParse("1Gi"), + corev1.ResourceCPU: resource.MustParse("1"), + }, + }, + }, + }, Horizontal: v1beta2.HorizontalRecommendations{ TargetUtilizations: []v1beta2.HPATargetUtilizationRecommendationPerContainer{ { @@ -446,6 +465,26 @@ func TestUpdateRecommendation(t *testing.T) { }, }, }, + Recommendations: v1beta2.Recommendations{ + Vertical: v1beta2.VerticalRecommendations{ + ContainerResourceRecommendation: []v1beta2.RecommendedContainerResources{ + { + ContainerName: "app", + RecommendedResource: corev1.ResourceList{ + corev1.ResourceMemory: resource.MustParse("5Gi"), + corev1.ResourceCPU: resource.MustParse("5"), + }, + }, + { + ContainerName: "istio-proxy", + RecommendedResource: corev1.ResourceList{ + corev1.ResourceMemory: resource.MustParse("1Gi"), + corev1.ResourceCPU: resource.MustParse("1"), + }, + }, + }, + }, + }, }, }, hpa: &v2.HorizontalPodAutoscaler{ @@ -482,34 +521,6 @@ func TestUpdateRecommendation(t *testing.T) { }, Status: v2.HorizontalPodAutoscalerStatus{}, }, - deployment: &v1.Deployment{ - Spec: v1.DeploymentSpec{ - Template: corev1.PodTemplateSpec{ - Spec: corev1.PodSpec{ - Containers: []corev1.Container{ - { - Name: "app", - Resources: corev1.ResourceRequirements{ - Requests: corev1.ResourceList{ - corev1.ResourceMemory: resource.MustParse("5Gi"), - corev1.ResourceCPU: resource.MustParse("5"), - }, - }, - }, - { - Name: "istio-proxy", - Resources: corev1.ResourceRequirements{ - Requests: corev1.ResourceList{ - corev1.ResourceMemory: resource.MustParse("1Gi"), - corev1.ResourceCPU: resource.MustParse("1"), - }, - }, - }, - }, - }, - }, - }, - }, }, wantErr: true, }, @@ -517,7 +528,7 @@ func TestUpdateRecommendation(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { s := New(24*30, 2.0, 0.5, 90, 3, 30, "10", "10Gi") - got, err := s.updateHPATargetUtilizationRecommendations(context.Background(), tt.args.tortoise, tt.args.hpa, tt.args.deployment) + got, err := s.updateHPATargetUtilizationRecommendations(context.Background(), tt.args.tortoise, tt.args.hpa) if (err != nil) != tt.wantErr { t.Errorf("updateHPATargetUtilizationRecommendations() error = %v, wantErr %v", err, tt.wantErr) return @@ -544,7 +555,7 @@ func Test_updateHPAMinMaxReplicasRecommendations(t *testing.T) { } type args struct { tortoise *v1beta2.Tortoise - deployment *v1.Deployment + replicaNum int32 now time.Time } tests := []struct { @@ -584,12 +595,8 @@ func Test_updateHPAMinMaxReplicasRecommendations(t *testing.T) { }, }, }, - deployment: &v1.Deployment{ - Status: v1.DeploymentStatus{ - Replicas: 10, - }, - }, - now: time.Date(2023, 3, 19, 0, 0, 0, 0, jst), + replicaNum: 10, + now: time.Date(2023, 3, 19, 0, 0, 0, 0, jst), }, want: &v1beta2.Tortoise{ Status: v1beta2.TortoiseStatus{ @@ -634,12 +641,8 @@ func Test_updateHPAMinMaxReplicasRecommendations(t *testing.T) { }, }, }, - deployment: &v1.Deployment{ - Status: v1.DeploymentStatus{ - Replicas: 5, - }, - }, - now: time.Date(2023, 3, 19, 0, 0, 0, 0, jst), + replicaNum: 5, + now: time.Date(2023, 3, 19, 0, 0, 0, 0, jst), }, want: &v1beta2.Tortoise{ Status: v1beta2.TortoiseStatus{ @@ -684,12 +687,8 @@ func Test_updateHPAMinMaxReplicasRecommendations(t *testing.T) { }, }, }, - deployment: &v1.Deployment{ - Status: v1.DeploymentStatus{ - Replicas: 5, - }, - }, - now: time.Date(2023, 3, 19, 0, 0, 0, 0, jst), + replicaNum: 5, + now: time.Date(2023, 3, 19, 0, 0, 0, 0, jst), }, want: &v1beta2.Tortoise{ Status: v1beta2.TortoiseStatus{ @@ -754,12 +753,8 @@ func Test_updateHPAMinMaxReplicasRecommendations(t *testing.T) { }, }, }, - deployment: &v1.Deployment{ - Status: v1.DeploymentStatus{ - Replicas: 10, - }, - }, - now: time.Date(2023, 3, 19, 0, 0, 0, 0, jst), + replicaNum: 10, + now: time.Date(2023, 3, 19, 0, 0, 0, 0, jst), }, want: &v1beta2.Tortoise{ Status: v1beta2.TortoiseStatus{ @@ -822,12 +817,8 @@ func Test_updateHPAMinMaxReplicasRecommendations(t *testing.T) { }, }, }, - deployment: &v1.Deployment{ - Status: v1.DeploymentStatus{ - Replicas: 1, - }, - }, - now: time.Date(2023, 3, 19, 0, 0, 0, 0, jst), + replicaNum: 1, + now: time.Date(2023, 3, 19, 0, 0, 0, 0, jst), }, want: &v1beta2.Tortoise{ Status: v1beta2.TortoiseStatus{ @@ -863,7 +854,7 @@ func Test_updateHPAMinMaxReplicasRecommendations(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { s := New(24*30, 2.0, 0.5, 90, 3, 30, "10", "10Gi") - got, err := s.updateHPAMinMaxReplicasRecommendations(tt.args.tortoise, tt.args.deployment, tt.args.now) + got, err := s.updateHPAMinMaxReplicasRecommendations(tt.args.tortoise, tt.args.replicaNum, tt.args.now) if (err != nil) != tt.wantErr { t.Errorf("updateHPAMinMaxReplicasRecommendations() error = %v, wantErr %v", err, tt.wantErr) return @@ -883,8 +874,8 @@ func TestService_UpdateVPARecommendation(t *testing.T) { } type args struct { tortoise *v1beta2.Tortoise - deployment *v1.Deployment hpa *v2.HorizontalPodAutoscaler + replicaNum int32 } tests := []struct { name string @@ -900,17 +891,52 @@ func TestService_UpdateVPARecommendation(t *testing.T) { suggestedResourceSizeAtMax: createResourceList("1000m", "1Gi"), }, args: args{ - tortoise: createTortoiseWithCondition(map[corev1.ResourceName]v1beta2.ResourceQuantity{ - corev1.ResourceCPU: { - Quantity: resource.MustParse("500m"), + tortoise: createTortoise().AddCondition( + v1beta2.ContainerRecommendationFromVPA{ + ContainerName: "test-container", + Recommendation: map[corev1.ResourceName]v1beta2.ResourceQuantity{ + corev1.ResourceCPU: { + Quantity: resource.MustParse("500m"), + }, + corev1.ResourceMemory: { + Quantity: resource.MustParse("500Mi"), + }, + }, }, - corev1.ResourceMemory: { - Quantity: resource.MustParse("500Mi"), + ).SetRecommendations(v1beta2.Recommendations{ + Vertical: v1beta2.VerticalRecommendations{ + ContainerResourceRecommendation: []v1beta2.RecommendedContainerResources{ + { + ContainerName: "test-container", + RecommendedResource: createResourceList("500m", "500Mi"), + }, + }, }, - }), - deployment: createDeployment(4, "500m", "500Mi"), + }).Build(), + replicaNum: 4, }, - want: createTortoiseWithVPARecommendation("550m", "550Mi"), + want: createTortoise().AddCondition( + v1beta2.ContainerRecommendationFromVPA{ + ContainerName: "test-container", + Recommendation: map[corev1.ResourceName]v1beta2.ResourceQuantity{ + corev1.ResourceCPU: { + Quantity: resource.MustParse("500m"), + }, + corev1.ResourceMemory: { + Quantity: resource.MustParse("500Mi"), + }, + }, + }, + ).SetRecommendations(v1beta2.Recommendations{ + Vertical: v1beta2.VerticalRecommendations{ + ContainerResourceRecommendation: []v1beta2.RecommendedContainerResources{ + { + ContainerName: "test-container", + RecommendedResource: createResourceList("550m", "550Mi"), + }, + }, + }, + }).Build(), wantErr: false, }, { @@ -920,17 +946,53 @@ func TestService_UpdateVPARecommendation(t *testing.T) { suggestedResourceSizeAtMax: createResourceList("1000m", "1Gi"), }, args: args{ - tortoise: createTortoiseWithCondition(map[corev1.ResourceName]v1beta2.ResourceQuantity{ - corev1.ResourceCPU: { - Quantity: resource.MustParse("1500m"), + tortoise: createTortoise().AddCondition( + v1beta2.ContainerRecommendationFromVPA{ + ContainerName: "test-container", + Recommendation: map[corev1.ResourceName]v1beta2.ResourceQuantity{ + corev1.ResourceCPU: { + Quantity: resource.MustParse("1500m"), + }, + corev1.ResourceMemory: { + Quantity: resource.MustParse("1.5Gi"), + }, + }, }, - corev1.ResourceMemory: { - Quantity: resource.MustParse("1.5Gi"), + ).SetRecommendations(v1beta2.Recommendations{ + Vertical: v1beta2.VerticalRecommendations{ + ContainerResourceRecommendation: []v1beta2.RecommendedContainerResources{ + { + ContainerName: "test-container", + RecommendedResource: createResourceList("1500m", "1.5Gi"), + }, + }, }, - }), - deployment: createDeployment(4, "1500m", "1.5Gi"), + }).Build(), + replicaNum: 4, }, - want: createTortoiseWithVPARecommendation("1500m", "1.5Gi"), + want: createTortoise().AddCondition( + // Unchanged! + v1beta2.ContainerRecommendationFromVPA{ + ContainerName: "test-container", + Recommendation: map[corev1.ResourceName]v1beta2.ResourceQuantity{ + corev1.ResourceCPU: { + Quantity: resource.MustParse("1500m"), + }, + corev1.ResourceMemory: { + Quantity: resource.MustParse("1.5Gi"), + }, + }, + }, + ).SetRecommendations(v1beta2.Recommendations{ + Vertical: v1beta2.VerticalRecommendations{ + ContainerResourceRecommendation: []v1beta2.RecommendedContainerResources{ + { + ContainerName: "test-container", + RecommendedResource: createResourceList("1500m", "1.5Gi"), + }, + }, + }, + }).Build(), wantErr: false, }, { @@ -941,37 +1003,107 @@ func TestService_UpdateVPARecommendation(t *testing.T) { minimumMinReplicas: 3, }, args: args{ - tortoise: createTortoiseWithCondition(map[corev1.ResourceName]v1beta2.ResourceQuantity{ - corev1.ResourceCPU: { - Quantity: resource.MustParse("120m"), + tortoise: createTortoise().AddCondition( + v1beta2.ContainerRecommendationFromVPA{ + ContainerName: "test-container", + Recommendation: map[corev1.ResourceName]v1beta2.ResourceQuantity{ + corev1.ResourceCPU: { + Quantity: resource.MustParse("120m"), + }, + corev1.ResourceMemory: { + Quantity: resource.MustParse("120Mi"), + }, + }, }, - corev1.ResourceMemory: { - Quantity: resource.MustParse("120Mi"), + ).SetRecommendations(v1beta2.Recommendations{ + Vertical: v1beta2.VerticalRecommendations{ + ContainerResourceRecommendation: []v1beta2.RecommendedContainerResources{ + { + ContainerName: "test-container", + RecommendedResource: createResourceList("130m", "130Mi"), + }, + }, }, - }), - deployment: createDeployment(3, "130m", "130Mi"), + }).Build(), + replicaNum: 3, }, - want: createTortoiseWithVPARecommendation("120m", "120Mi"), + want: createTortoise().AddCondition( + v1beta2.ContainerRecommendationFromVPA{ + ContainerName: "test-container", + Recommendation: map[corev1.ResourceName]v1beta2.ResourceQuantity{ + corev1.ResourceCPU: { + Quantity: resource.MustParse("120m"), + }, + corev1.ResourceMemory: { + Quantity: resource.MustParse("120Mi"), + }, + }, + }, + ).SetRecommendations(v1beta2.Recommendations{ + Vertical: v1beta2.VerticalRecommendations{ + ContainerResourceRecommendation: []v1beta2.RecommendedContainerResources{ + { + ContainerName: "test-container", + RecommendedResource: createResourceList("120m", "120Mi"), + }, + }, + }, + }).Build(), wantErr: false, }, { - name: "reduced resources based on VPA recommendation", + name: "vertical only: reduced resources based on VPA recommendation", fields: fields{ preferredReplicaNumUpperLimit: 6, suggestedResourceSizeAtMax: createResourceList("1000m", "1Gi"), }, args: args{ - tortoise: createVerticalTortoiseWithCondition(map[corev1.ResourceName]v1beta2.ResourceQuantity{ - corev1.ResourceCPU: { - Quantity: resource.MustParse("120m"), + tortoise: createVerticalTortoise().AddCondition( + v1beta2.ContainerRecommendationFromVPA{ + ContainerName: "test-container", + Recommendation: map[corev1.ResourceName]v1beta2.ResourceQuantity{ + corev1.ResourceCPU: { + Quantity: resource.MustParse("120m"), + }, + corev1.ResourceMemory: { + Quantity: resource.MustParse("120Mi"), + }, + }, }, - corev1.ResourceMemory: { - Quantity: resource.MustParse("120Mi"), + ).SetRecommendations(v1beta2.Recommendations{ + Vertical: v1beta2.VerticalRecommendations{ + ContainerResourceRecommendation: []v1beta2.RecommendedContainerResources{ + { + ContainerName: "test-container", + RecommendedResource: createResourceList("130m", "130Mi"), + }, + }, }, - }), - deployment: createDeployment(3, "130m", "130Mi"), + }).Build(), + replicaNum: 3, }, - want: createVerticalTortoiseWithVPARecommendation("120m", "120Mi"), + want: createVerticalTortoise().AddCondition( + v1beta2.ContainerRecommendationFromVPA{ + ContainerName: "test-container", + Recommendation: map[corev1.ResourceName]v1beta2.ResourceQuantity{ + corev1.ResourceCPU: { + Quantity: resource.MustParse("120m"), + }, + corev1.ResourceMemory: { + Quantity: resource.MustParse("120Mi"), + }, + }, + }, + ).SetRecommendations(v1beta2.Recommendations{ + Vertical: v1beta2.VerticalRecommendations{ + ContainerResourceRecommendation: []v1beta2.RecommendedContainerResources{ + { + ContainerName: "test-container", + RecommendedResource: createResourceList("120m", "120Mi"), + }, + }, + }, + }).Build(), wantErr: false, }, { @@ -981,24 +1113,45 @@ func TestService_UpdateVPARecommendation(t *testing.T) { suggestedResourceSizeAtMax: createResourceList("10000m", "100Gi"), }, args: args{ - tortoise: createTortoiseWithMultipleContainersWithCondition( - map[corev1.ResourceName]v1beta2.ResourceQuantity{ - corev1.ResourceCPU: { - Quantity: resource.MustParse("80m"), - }, - corev1.ResourceMemory: { - Quantity: resource.MustParse("9Gi"), + tortoise: createTortoiseWithMultipleContainers().AddCondition( + v1beta2.ContainerRecommendationFromVPA{ + ContainerName: "test-container", + Recommendation: map[corev1.ResourceName]v1beta2.ResourceQuantity{ + corev1.ResourceCPU: { + Quantity: resource.MustParse("80m"), + }, + corev1.ResourceMemory: { + Quantity: resource.MustParse("9Gi"), + }, }, }, - map[corev1.ResourceName]v1beta2.ResourceQuantity{ - corev1.ResourceCPU: { - Quantity: resource.MustParse("800m"), + ).AddCondition( + v1beta2.ContainerRecommendationFromVPA{ + ContainerName: "test-container2", + Recommendation: map[corev1.ResourceName]v1beta2.ResourceQuantity{ + corev1.ResourceCPU: { + Quantity: resource.MustParse("800m"), + }, + corev1.ResourceMemory: { + Quantity: resource.MustParse("0.7Gi"), + }, }, - corev1.ResourceMemory: { - Quantity: resource.MustParse("0.7Gi"), + }, + ).SetRecommendations(v1beta2.Recommendations{ + Vertical: v1beta2.VerticalRecommendations{ + ContainerResourceRecommendation: []v1beta2.RecommendedContainerResources{ + { + ContainerName: "test-container", + RecommendedResource: createResourceList("1000m", "10Gi"), + }, + { + ContainerName: "test-container2", + RecommendedResource: createResourceList("1000m", "10Gi"), + }, }, - }), - deployment: createMultipleContainersDeployment(5, "1000m", "1000m", "10Gi", "10Gi"), + }, + }).Build(), + replicaNum: 5, hpa: &v2.HorizontalPodAutoscaler{ Spec: v2.HorizontalPodAutoscalerSpec{ Metrics: []v2.MetricSpec{ @@ -1046,7 +1199,44 @@ func TestService_UpdateVPARecommendation(t *testing.T) { }, }, }, - want: createTortoiseWithMultipleContainersWithVPARecommendation("100m", "1000m", "10Gi", "1Gi"), + want: createTortoiseWithMultipleContainers().AddCondition( + v1beta2.ContainerRecommendationFromVPA{ + ContainerName: "test-container", + Recommendation: map[corev1.ResourceName]v1beta2.ResourceQuantity{ + corev1.ResourceCPU: { + Quantity: resource.MustParse("80m"), + }, + corev1.ResourceMemory: { + Quantity: resource.MustParse("9Gi"), + }, + }, + }, + ).AddCondition( + v1beta2.ContainerRecommendationFromVPA{ + ContainerName: "test-container2", + Recommendation: map[corev1.ResourceName]v1beta2.ResourceQuantity{ + corev1.ResourceCPU: { + Quantity: resource.MustParse("800m"), + }, + corev1.ResourceMemory: { + Quantity: resource.MustParse("0.7Gi"), + }, + }, + }, + ).SetRecommendations(v1beta2.Recommendations{ + Vertical: v1beta2.VerticalRecommendations{ + ContainerResourceRecommendation: []v1beta2.RecommendedContainerResources{ + { + ContainerName: "test-container", + RecommendedResource: createResourceList("100m", "10Gi"), + }, + { + ContainerName: "test-container2", + RecommendedResource: createResourceList("1000m", "1Gi"), + }, + }, + }, + }).Build(), wantErr: false, }, } @@ -1057,12 +1247,12 @@ func TestService_UpdateVPARecommendation(t *testing.T) { preferredReplicaNumUpperLimit: tt.fields.preferredReplicaNumUpperLimit, maxResourceSize: tt.fields.suggestedResourceSizeAtMax, } - got, err := s.updateVPARecommendation(tt.args.tortoise, tt.args.deployment, tt.args.hpa) + got, err := s.updateVPARecommendation(tt.args.tortoise, tt.args.hpa, tt.args.replicaNum) if (err != nil) != tt.wantErr { t.Errorf("updateVPARecommendation() error = %v, wantErr %v", err, tt.wantErr) return } - if d := cmp.Diff(got, tt.want, cmpopts.IgnoreTypes(metav1.Time{}, v1beta2.Conditions{})); d != "" { + if d := cmp.Diff(got, tt.want, cmpopts.IgnoreTypes(metav1.Time{})); d != "" { t.Errorf("updateVPARecommendation() diff = %s", d) } }) @@ -1076,191 +1266,43 @@ func createResourceList(cpu, memory string) corev1.ResourceList { corev1.ResourceMemory: resource.MustParse(memory), } } -func createVerticalTortoise() *v1beta2.Tortoise { - return &v1beta2.Tortoise{ - Spec: v1beta2.TortoiseSpec{ - ResourcePolicy: []v1beta2.ContainerResourcePolicy{ - { - ContainerName: "test-container", - AutoscalingPolicy: map[corev1.ResourceName]v1beta2.AutoscalingType{ - corev1.ResourceCPU: v1beta2.AutoscalingTypeVertical, - corev1.ResourceMemory: v1beta2.AutoscalingTypeVertical, - }, - MinAllocatedResources: createResourceList("100m", "100Mi"), - }, - }, - }, - } -} -func createVerticalTortoiseWithCondition(vpaRecommendation map[corev1.ResourceName]v1beta2.ResourceQuantity) *v1beta2.Tortoise { - tortoise := createVerticalTortoise() - tortoise.Status.Conditions.ContainerRecommendationFromVPA = []v1beta2.ContainerRecommendationFromVPA{ - { - ContainerName: "test-container", - Recommendation: vpaRecommendation, - }, - } - return tortoise -} -func createTortoise() *v1beta2.Tortoise { - return &v1beta2.Tortoise{ - Spec: v1beta2.TortoiseSpec{ - ResourcePolicy: []v1beta2.ContainerResourcePolicy{ - { - ContainerName: "test-container", - AutoscalingPolicy: map[corev1.ResourceName]v1beta2.AutoscalingType{ - corev1.ResourceCPU: v1beta2.AutoscalingTypeHorizontal, - corev1.ResourceMemory: v1beta2.AutoscalingTypeHorizontal, - }, - MinAllocatedResources: createResourceList("100m", "100Mi"), - }, - }, - }, - } -} -func createDeployment(replicas int32, cpu, memory string) *v1.Deployment { - return &v1.Deployment{ - Status: v1.DeploymentStatus{ - Replicas: replicas, - }, - Spec: v1.DeploymentSpec{ - Template: corev1.PodTemplateSpec{ - Spec: corev1.PodSpec{ - Containers: []corev1.Container{ - { - Name: "test-container", - Resources: corev1.ResourceRequirements{ - Requests: createResourceList(cpu, memory), - }, - }, - }, - }, - }, - }, - } -} -func createTortoiseWithMultipleContainers() *v1beta2.Tortoise { - return &v1beta2.Tortoise{ - Spec: v1beta2.TortoiseSpec{ - ResourcePolicy: []v1beta2.ContainerResourcePolicy{ - { - ContainerName: "test-container", - AutoscalingPolicy: map[corev1.ResourceName]v1beta2.AutoscalingType{ - corev1.ResourceCPU: v1beta2.AutoscalingTypeHorizontal, - corev1.ResourceMemory: v1beta2.AutoscalingTypeHorizontal, - }, - MinAllocatedResources: createResourceList("100m", "100Mi"), - }, - { - ContainerName: "test-container2", - AutoscalingPolicy: map[corev1.ResourceName]v1beta2.AutoscalingType{ - corev1.ResourceCPU: v1beta2.AutoscalingTypeHorizontal, - corev1.ResourceMemory: v1beta2.AutoscalingTypeHorizontal, - }, - MinAllocatedResources: createResourceList("100m", "100Mi"), - }, - }, - }, - } -} -func createMultipleContainersDeployment(replicas int32, cpu1, cpu2, memory1, memory2 string) *v1.Deployment { - return &v1.Deployment{ - Status: v1.DeploymentStatus{ - Replicas: replicas, - }, - Spec: v1.DeploymentSpec{ - Template: corev1.PodTemplateSpec{ - Spec: corev1.PodSpec{ - Containers: []corev1.Container{ - { - Name: "test-container", - Resources: corev1.ResourceRequirements{ - Requests: createResourceList(cpu1, memory1), - }, - }, - { - Name: "test-container2", - Resources: corev1.ResourceRequirements{ - Requests: createResourceList(cpu2, memory2), - }, - }, - }, - }, - }, - }, - } -} - -func createTortoiseWithMultipleContainersWithCondition(vpaRecommendation1, vpaRecommendation2 map[corev1.ResourceName]v1beta2.ResourceQuantity) *v1beta2.Tortoise { - tortoise := createTortoiseWithMultipleContainers() - tortoise.Status.Conditions.ContainerRecommendationFromVPA = []v1beta2.ContainerRecommendationFromVPA{ - { - ContainerName: "test-container", - Recommendation: vpaRecommendation1, +func createVerticalTortoise() *utils.TortoiseBuilder { + return utils.NewTortoiseBuilder().AddResourcePolicy(v1beta2.ContainerResourcePolicy{ + ContainerName: "test-container", + AutoscalingPolicy: map[corev1.ResourceName]v1beta2.AutoscalingType{ + corev1.ResourceCPU: v1beta2.AutoscalingTypeVertical, + corev1.ResourceMemory: v1beta2.AutoscalingTypeVertical, }, - { - ContainerName: "test-container2", - Recommendation: vpaRecommendation2, - }, - } - return tortoise + MinAllocatedResources: createResourceList("100m", "100Mi"), + }) } - -func createTortoiseWithCondition(vpaRecommendation map[corev1.ResourceName]v1beta2.ResourceQuantity) *v1beta2.Tortoise { - tortoise := createTortoise() - tortoise.Status.Conditions.ContainerRecommendationFromVPA = []v1beta2.ContainerRecommendationFromVPA{ - { - ContainerName: "test-container", - Recommendation: vpaRecommendation, +func createTortoise() *utils.TortoiseBuilder { + b := utils.NewTortoiseBuilder() + // Build the above tortoise via builder + b.AddResourcePolicy(v1beta2.ContainerResourcePolicy{ + ContainerName: "test-container", + AutoscalingPolicy: map[corev1.ResourceName]v1beta2.AutoscalingType{ + corev1.ResourceCPU: v1beta2.AutoscalingTypeHorizontal, + corev1.ResourceMemory: v1beta2.AutoscalingTypeHorizontal, }, - } - return tortoise + MinAllocatedResources: createResourceList("100m", "100Mi"), + }) + return b } - -func createTortoiseWithVPARecommendation(cpu, memory string) *v1beta2.Tortoise { - tortoise := createTortoise() - tortoise.Status.Recommendations = v1beta2.Recommendations{ - Vertical: v1beta2.VerticalRecommendations{ - ContainerResourceRecommendation: []v1beta2.RecommendedContainerResources{ - { - ContainerName: "test-container", - RecommendedResource: createResourceList(cpu, memory), - }, - }, +func createTortoiseWithMultipleContainers() *utils.TortoiseBuilder { + return utils.NewTortoiseBuilder().AddResourcePolicy(v1beta2.ContainerResourcePolicy{ + ContainerName: "test-container", + AutoscalingPolicy: map[corev1.ResourceName]v1beta2.AutoscalingType{ + corev1.ResourceCPU: v1beta2.AutoscalingTypeHorizontal, + corev1.ResourceMemory: v1beta2.AutoscalingTypeHorizontal, }, - } - return tortoise -} - -func createVerticalTortoiseWithVPARecommendation(cpu, memory string) *v1beta2.Tortoise { - tortoise := createVerticalTortoise() - tortoise.Status.Recommendations = v1beta2.Recommendations{ - Vertical: v1beta2.VerticalRecommendations{ - ContainerResourceRecommendation: []v1beta2.RecommendedContainerResources{ - { - ContainerName: "test-container", - RecommendedResource: createResourceList(cpu, memory), - }, - }, - }, - } - return tortoise -} -func createTortoiseWithMultipleContainersWithVPARecommendation(cpu1, cpu2, memory1, memory2 string) *v1beta2.Tortoise { - tortoise := createTortoiseWithMultipleContainers() - tortoise.Status.Recommendations = v1beta2.Recommendations{ - Vertical: v1beta2.VerticalRecommendations{ - ContainerResourceRecommendation: []v1beta2.RecommendedContainerResources{ - { - ContainerName: "test-container", - RecommendedResource: createResourceList(cpu1, memory1), - }, - { - ContainerName: "test-container2", - RecommendedResource: createResourceList(cpu2, memory2), - }, - }, + MinAllocatedResources: createResourceList("100m", "100Mi"), + }).AddResourcePolicy(v1beta2.ContainerResourcePolicy{ + ContainerName: "test-container2", + AutoscalingPolicy: map[corev1.ResourceName]v1beta2.AutoscalingType{ + corev1.ResourceCPU: v1beta2.AutoscalingTypeHorizontal, + corev1.ResourceMemory: v1beta2.AutoscalingTypeHorizontal, }, - } - return tortoise + MinAllocatedResources: createResourceList("100m", "100Mi"), + }) } diff --git a/pkg/tortoise/tortoise.go b/pkg/tortoise/tortoise.go index c7f0e8c0..7b7f756e 100644 --- a/pkg/tortoise/tortoise.go +++ b/pkg/tortoise/tortoise.go @@ -6,7 +6,6 @@ import ( "sync" "time" - appv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -75,10 +74,10 @@ func (s *Service) ShouldReconcileTortoiseNow(tortoise *v1beta2.Tortoise, now tim return false, lastTime.Add(s.tortoiseUpdateInterval).Sub(now) } -func (s *Service) UpdateTortoisePhase(tortoise *v1beta2.Tortoise, dm *appv1.Deployment, now time.Time) *v1beta2.Tortoise { +func (s *Service) UpdateTortoisePhase(tortoise *v1beta2.Tortoise, now time.Time) *v1beta2.Tortoise { switch tortoise.Status.TortoisePhase { case "": - tortoise = s.initializeTortoise(tortoise, dm, now) + tortoise = s.initializeTortoise(tortoise, now) r := "1 week" if s.minMaxReplicasRoutine == "daily" { r = "1 day" @@ -211,14 +210,14 @@ func (s *Service) initializeMinMaxReplicas(tortoise *v1beta2.Tortoise) *v1beta2. return tortoise } -func (s *Service) initializeTortoise(tortoise *v1beta2.Tortoise, dm *appv1.Deployment, now time.Time) *v1beta2.Tortoise { +func (s *Service) initializeTortoise(tortoise *v1beta2.Tortoise, now time.Time) *v1beta2.Tortoise { tortoise = s.initializeMinMaxReplicas(tortoise) tortoise.Status.TortoisePhase = v1beta2.TortoisePhaseInitializing - tortoise.Status.Conditions.ContainerRecommendationFromVPA = make([]v1beta2.ContainerRecommendationFromVPA, len(dm.Spec.Template.Spec.Containers)) - for i, c := range dm.Spec.Template.Spec.Containers { + tortoise.Status.Conditions.ContainerRecommendationFromVPA = make([]v1beta2.ContainerRecommendationFromVPA, len(tortoise.Spec.ResourcePolicy)) + for i, c := range tortoise.Spec.ResourcePolicy { tortoise.Status.Conditions.ContainerRecommendationFromVPA[i] = v1beta2.ContainerRecommendationFromVPA{ - ContainerName: c.Name, + ContainerName: c.ContainerName, Recommendation: map[corev1.ResourceName]v1beta2.ResourceQuantity{ corev1.ResourceCPU: {}, corev1.ResourceMemory: {}, diff --git a/pkg/tortoise/tortoise_test.go b/pkg/tortoise/tortoise_test.go index 294a2608..6e99347f 100644 --- a/pkg/tortoise/tortoise_test.go +++ b/pkg/tortoise/tortoise_test.go @@ -729,7 +729,7 @@ func TestService_InitializeTortoise(t *testing.T) { minMaxReplicasRoutine: tt.fields.minMaxReplicasRoutine, timeZone: tt.fields.timeZone, } - got := s.initializeTortoise(tt.tortoise, tt.deployment, time.Now()) + got := s.initializeTortoise(tt.tortoise, time.Now()) if d := cmp.Diff(got, tt.want, cmpopts.IgnoreTypes(metav1.Time{})); d != "" { t.Errorf("initializeTortoise() diff = %v", d) } diff --git a/pkg/vpa/service.go b/pkg/vpa/service.go index 19f00e80..f9c99efb 100644 --- a/pkg/vpa/service.go +++ b/pkg/vpa/service.go @@ -283,7 +283,7 @@ func (c *Service) GetTortoiseMonitorVPA(ctx context.Context, tortoise *autoscali return vpa, false, nil } -func MakeAllVerticalContainerResourcePhaseWorking(tortoise *autoscalingv1beta2.Tortoise, now time.Time) *autoscalingv1beta2.Tortoise { +func SetAllVerticalContainerResourcePhaseWorking(tortoise *autoscalingv1beta2.Tortoise, now time.Time) *autoscalingv1beta2.Tortoise { verticalResourceAndContainer := sets.New[resourceNameAndContainerName]() for _, p := range tortoise.Spec.ResourcePolicy { for rn, ap := range p.AutoscalingPolicy { diff --git a/pkg/vpa/service_test.go b/pkg/vpa/service_test.go index 4d152d06..3849150c 100644 --- a/pkg/vpa/service_test.go +++ b/pkg/vpa/service_test.go @@ -130,7 +130,7 @@ func TestMakeAllVerticalContainerResourcePhaseRunning(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got := MakeAllVerticalContainerResourcePhaseWorking(tt.args.tortoise, time.Now()) + got := SetAllVerticalContainerResourcePhaseWorking(tt.args.tortoise, time.Now()) // use diff to compare if diff := cmp.Diff(got, tt.want, cmpopts.IgnoreTypes(metav1.Time{})); diff != "" {