From a15be18ec824c032640eda60d11b0d6a5bb70b4f Mon Sep 17 00:00:00 2001 From: Kensei Nakada Date: Wed, 11 Oct 2023 17:23:04 +0900 Subject: [PATCH] modify ContainerResourcePhases when tortoise is initialized or the autoscaling policy changed (#165) * modify ContainerResourcePhases when tortoise is initialized or the autoscaling policy changed * fix by lint --- api/v1beta1/tortoise_types.go | 3 +- api/v1beta1/zz_generated.deepcopy.go | 4 +- .../deletion-no-delete/before/tortoise.yaml | 2 +- .../deletion-policy-all/before/tortoise.yaml | 2 +- .../after/tortoise.yaml | 4 +- .../before/tortoise.yaml | 2 +- .../after/tortoise.yaml | 4 +- .../before/tortoise.yaml | 2 +- .../after/tortoise.yaml | 2 +- .../before/tortoise.yaml | 2 +- .../after/tortoise.yaml | 2 +- .../before/tortoise.yaml | 2 +- .../after/tortoise.yaml | 2 +- .../before/tortoise.yaml | 2 +- .../after/tortoise.yaml | 2 +- .../before/tortoise.yaml | 2 +- .../after/tortoise.yaml | 2 +- .../before/tortoise.yaml | 2 +- .../after/tortoise.yaml | 2 +- .../before/tortoise.yaml | 2 +- .../after/tortoise.yaml | 2 +- .../before/tortoise.yaml | 2 +- .../after/tortoise.yaml | 2 +- .../before/tortoise.yaml | 2 +- .../after/tortoise.yaml | 2 +- .../before/tortoise.yaml | 2 +- controllers/tortoise_controller.go | 32 +-- pkg/hpa/service.go | 70 ++++-- pkg/hpa/service_test.go | 223 +++++++++++++++++- pkg/tortoise/tortoise.go | 16 ++ pkg/tortoise/tortoise_test.go | 99 ++++++++ pkg/vpa/service.go | 38 +++ pkg/vpa/service_test.go | 123 ++++++++++ 33 files changed, 586 insertions(+), 74 deletions(-) create mode 100644 pkg/vpa/service_test.go diff --git a/api/v1beta1/tortoise_types.go b/api/v1beta1/tortoise_types.go index c670a0d8..c0248c50 100644 --- a/api/v1beta1/tortoise_types.go +++ b/api/v1beta1/tortoise_types.go @@ -154,7 +154,7 @@ type ContainerResourcePhases struct { // ContainerName is the name of target container. ContainerName string `json:"containerName" protobuf:"bytes,1,name=containerName"` // ResourcePhases is the phase of each resource of this container. - ResourceePhases map[v1.ResourceName]ContainerResourcePhase `json:"resourcePhases" protobuf:"bytes,2,name=resourcePhases"` + ResourcePhases map[v1.ResourceName]ContainerResourcePhase `json:"resourcePhases" protobuf:"bytes,2,name=resourcePhases"` } type ContainerResourcePhase string @@ -162,6 +162,7 @@ type ContainerResourcePhase string const ( ContainerResourcePhaseGatheringData ContainerResourcePhase = "GatheringData" ContainerResourcePhaseWorking ContainerResourcePhase = "Working" + ContainerResourcePhaseOff ContainerResourcePhase = "Off" ) type TortoisePhase string diff --git a/api/v1beta1/zz_generated.deepcopy.go b/api/v1beta1/zz_generated.deepcopy.go index ec0ba3df..0b24ab9f 100644 --- a/api/v1beta1/zz_generated.deepcopy.go +++ b/api/v1beta1/zz_generated.deepcopy.go @@ -95,8 +95,8 @@ func (in *ContainerRecommendationFromVPA) DeepCopy() *ContainerRecommendationFro // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *ContainerResourcePhases) DeepCopyInto(out *ContainerResourcePhases) { *out = *in - if in.ResourceePhases != nil { - in, out := &in.ResourceePhases, &out.ResourceePhases + if in.ResourcePhases != nil { + in, out := &in.ResourcePhases, &out.ResourcePhases *out = make(map[v1.ResourceName]ContainerResourcePhase, len(*in)) for key, val := range *in { (*out)[key] = val diff --git a/controllers/testdata/deletion-no-delete/before/tortoise.yaml b/controllers/testdata/deletion-no-delete/before/tortoise.yaml index c3ef11ba..928c71fb 100644 --- a/controllers/testdata/deletion-no-delete/before/tortoise.yaml +++ b/controllers/testdata/deletion-no-delete/before/tortoise.yaml @@ -84,7 +84,7 @@ status: role: Monitor tortoisePhase: Working containerResourcePhases: - - containerName: "nginx" + - containerName: "app" resourcePhases: cpu: Working memory: Working diff --git a/controllers/testdata/deletion-policy-all/before/tortoise.yaml b/controllers/testdata/deletion-policy-all/before/tortoise.yaml index b62a9d0f..c3c6575b 100644 --- a/controllers/testdata/deletion-policy-all/before/tortoise.yaml +++ b/controllers/testdata/deletion-policy-all/before/tortoise.yaml @@ -84,7 +84,7 @@ status: role: Monitor tortoisePhase: Working containerResourcePhases: - - containerName: "nginx" + - containerName: "app" resourcePhases: cpu: Working memory: Working diff --git a/controllers/testdata/mutable-autoscalingpolicy-add-another-horizontal/after/tortoise.yaml b/controllers/testdata/mutable-autoscalingpolicy-add-another-horizontal/after/tortoise.yaml index 796883c4..c72fbc84 100644 --- a/controllers/testdata/mutable-autoscalingpolicy-add-another-horizontal/after/tortoise.yaml +++ b/controllers/testdata/mutable-autoscalingpolicy-add-another-horizontal/after/tortoise.yaml @@ -92,11 +92,11 @@ status: role: Monitor tortoisePhase: Working containerResourcePhases: - - containerName: "nginx" + - containerName: "app" resourcePhases: cpu: Working memory: Working - containerName: "istio-proxy" resourcePhases: cpu: Working - memory: Working + memory: GatheringData diff --git a/controllers/testdata/mutable-autoscalingpolicy-add-another-horizontal/before/tortoise.yaml b/controllers/testdata/mutable-autoscalingpolicy-add-another-horizontal/before/tortoise.yaml index 19af0db3..27b042a5 100644 --- a/controllers/testdata/mutable-autoscalingpolicy-add-another-horizontal/before/tortoise.yaml +++ b/controllers/testdata/mutable-autoscalingpolicy-add-another-horizontal/before/tortoise.yaml @@ -83,7 +83,7 @@ status: role: Monitor tortoisePhase: Working containerResourcePhases: - - containerName: "nginx" + - containerName: "app" resourcePhases: cpu: Working memory: Working diff --git a/controllers/testdata/mutable-autoscalingpolicy-no-hpa-and-add-horizontal/after/tortoise.yaml b/controllers/testdata/mutable-autoscalingpolicy-no-hpa-and-add-horizontal/after/tortoise.yaml index f534b8c5..e3a5d034 100644 --- a/controllers/testdata/mutable-autoscalingpolicy-no-hpa-and-add-horizontal/after/tortoise.yaml +++ b/controllers/testdata/mutable-autoscalingpolicy-no-hpa-and-add-horizontal/after/tortoise.yaml @@ -90,9 +90,9 @@ status: role: Monitor tortoisePhase: Working containerResourcePhases: - - containerName: "nginx" + - containerName: "app" resourcePhases: - cpu: Working + cpu: GatheringData memory: Working - containerName: "istio-proxy" resourcePhases: 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 910d6027..3aaeb37e 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 @@ -88,7 +88,7 @@ status: role: Monitor tortoisePhase: Working containerResourcePhases: - - containerName: "nginx" + - containerName: "app" resourcePhases: cpu: Working memory: Working diff --git a/controllers/testdata/mutable-autoscalingpolicy-remove-horizontal/after/tortoise.yaml b/controllers/testdata/mutable-autoscalingpolicy-remove-horizontal/after/tortoise.yaml index 33454c45..e3c7f1ec 100644 --- a/controllers/testdata/mutable-autoscalingpolicy-remove-horizontal/after/tortoise.yaml +++ b/controllers/testdata/mutable-autoscalingpolicy-remove-horizontal/after/tortoise.yaml @@ -89,7 +89,7 @@ status: role: Monitor tortoisePhase: Working containerResourcePhases: - - containerName: "nginx" + - containerName: "app" resourcePhases: cpu: Working memory: Working diff --git a/controllers/testdata/mutable-autoscalingpolicy-remove-horizontal/before/tortoise.yaml b/controllers/testdata/mutable-autoscalingpolicy-remove-horizontal/before/tortoise.yaml index 7a97cd9d..eb30161f 100644 --- a/controllers/testdata/mutable-autoscalingpolicy-remove-horizontal/before/tortoise.yaml +++ b/controllers/testdata/mutable-autoscalingpolicy-remove-horizontal/before/tortoise.yaml @@ -83,7 +83,7 @@ status: role: Monitor tortoisePhase: Working containerResourcePhases: - - containerName: "nginx" + - containerName: "app" resourcePhases: cpu: Working memory: Working diff --git a/controllers/testdata/reconcile-for-the-multiple-containers-pod-all-off/after/tortoise.yaml b/controllers/testdata/reconcile-for-the-multiple-containers-pod-all-off/after/tortoise.yaml index f26dfbe6..b0f92f6b 100644 --- a/controllers/testdata/reconcile-for-the-multiple-containers-pod-all-off/after/tortoise.yaml +++ b/controllers/testdata/reconcile-for-the-multiple-containers-pod-all-off/after/tortoise.yaml @@ -87,7 +87,7 @@ status: role: Monitor tortoisePhase: Working containerResourcePhases: - - containerName: "nginx" + - containerName: "app" resourcePhases: cpu: Working memory: Working diff --git a/controllers/testdata/reconcile-for-the-multiple-containers-pod-all-off/before/tortoise.yaml b/controllers/testdata/reconcile-for-the-multiple-containers-pod-all-off/before/tortoise.yaml index 0a1b5ce8..34c6d87e 100644 --- a/controllers/testdata/reconcile-for-the-multiple-containers-pod-all-off/before/tortoise.yaml +++ b/controllers/testdata/reconcile-for-the-multiple-containers-pod-all-off/before/tortoise.yaml @@ -79,7 +79,7 @@ status: role: Monitor tortoisePhase: Working containerResourcePhases: - - containerName: "nginx" + - containerName: "app" resourcePhases: cpu: Working memory: Working diff --git a/controllers/testdata/reconcile-for-the-multiple-containers-pod-all-vertical/after/tortoise.yaml b/controllers/testdata/reconcile-for-the-multiple-containers-pod-all-vertical/after/tortoise.yaml index 4c11b8b3..b83edaf6 100644 --- a/controllers/testdata/reconcile-for-the-multiple-containers-pod-all-vertical/after/tortoise.yaml +++ b/controllers/testdata/reconcile-for-the-multiple-containers-pod-all-vertical/after/tortoise.yaml @@ -88,7 +88,7 @@ status: role: Monitor tortoisePhase: Working containerResourcePhases: - - containerName: "nginx" + - containerName: "app" resourcePhases: cpu: Working memory: Working diff --git a/controllers/testdata/reconcile-for-the-multiple-containers-pod-all-vertical/before/tortoise.yaml b/controllers/testdata/reconcile-for-the-multiple-containers-pod-all-vertical/before/tortoise.yaml index eb6a5868..002cd103 100644 --- a/controllers/testdata/reconcile-for-the-multiple-containers-pod-all-vertical/before/tortoise.yaml +++ b/controllers/testdata/reconcile-for-the-multiple-containers-pod-all-vertical/before/tortoise.yaml @@ -80,7 +80,7 @@ status: role: Monitor tortoisePhase: Working containerResourcePhases: - - containerName: "nginx" + - containerName: "app" resourcePhases: cpu: Working memory: Working diff --git a/controllers/testdata/reconcile-for-the-multiple-containers-pod-emergency/after/tortoise.yaml b/controllers/testdata/reconcile-for-the-multiple-containers-pod-emergency/after/tortoise.yaml index f9c4fafd..b4e79681 100644 --- a/controllers/testdata/reconcile-for-the-multiple-containers-pod-emergency/after/tortoise.yaml +++ b/controllers/testdata/reconcile-for-the-multiple-containers-pod-emergency/after/tortoise.yaml @@ -92,7 +92,7 @@ status: role: Monitor tortoisePhase: Emergency containerResourcePhases: - - containerName: "nginx" + - containerName: "app" resourcePhases: cpu: Working memory: Working diff --git a/controllers/testdata/reconcile-for-the-multiple-containers-pod-emergency/before/tortoise.yaml b/controllers/testdata/reconcile-for-the-multiple-containers-pod-emergency/before/tortoise.yaml index b6403b24..ba8c8651 100644 --- a/controllers/testdata/reconcile-for-the-multiple-containers-pod-emergency/before/tortoise.yaml +++ b/controllers/testdata/reconcile-for-the-multiple-containers-pod-emergency/before/tortoise.yaml @@ -84,7 +84,7 @@ status: role: Monitor tortoisePhase: Working containerResourcePhases: - - containerName: "nginx" + - containerName: "app" resourcePhases: cpu: Working memory: Working diff --git a/controllers/testdata/reconcile-for-the-multiple-containers-pod-one-off/after/tortoise.yaml b/controllers/testdata/reconcile-for-the-multiple-containers-pod-one-off/after/tortoise.yaml index 799e60c0..4cefdd45 100644 --- a/controllers/testdata/reconcile-for-the-multiple-containers-pod-one-off/after/tortoise.yaml +++ b/controllers/testdata/reconcile-for-the-multiple-containers-pod-one-off/after/tortoise.yaml @@ -90,7 +90,7 @@ status: role: Monitor tortoisePhase: Working containerResourcePhases: - - containerName: "nginx" + - containerName: "app" resourcePhases: cpu: Working memory: Working diff --git a/controllers/testdata/reconcile-for-the-multiple-containers-pod-one-off/before/tortoise.yaml b/controllers/testdata/reconcile-for-the-multiple-containers-pod-one-off/before/tortoise.yaml index 3d6113eb..b3ddfe12 100644 --- a/controllers/testdata/reconcile-for-the-multiple-containers-pod-one-off/before/tortoise.yaml +++ b/controllers/testdata/reconcile-for-the-multiple-containers-pod-one-off/before/tortoise.yaml @@ -83,7 +83,7 @@ status: role: Monitor tortoisePhase: Working containerResourcePhases: - - containerName: "nginx" + - containerName: "app" resourcePhases: cpu: Working memory: Working diff --git a/controllers/testdata/reconcile-for-the-multiple-containers-pod-working/after/tortoise.yaml b/controllers/testdata/reconcile-for-the-multiple-containers-pod-working/after/tortoise.yaml index 68bae4e1..3bb189a9 100644 --- a/controllers/testdata/reconcile-for-the-multiple-containers-pod-working/after/tortoise.yaml +++ b/controllers/testdata/reconcile-for-the-multiple-containers-pod-working/after/tortoise.yaml @@ -91,7 +91,7 @@ status: role: Monitor tortoisePhase: Working containerResourcePhases: - - containerName: "nginx" + - containerName: "app" resourcePhases: cpu: Working memory: Working diff --git a/controllers/testdata/reconcile-for-the-multiple-containers-pod-working/before/tortoise.yaml b/controllers/testdata/reconcile-for-the-multiple-containers-pod-working/before/tortoise.yaml index c7ee5a6b..075e5fe3 100644 --- a/controllers/testdata/reconcile-for-the-multiple-containers-pod-working/before/tortoise.yaml +++ b/controllers/testdata/reconcile-for-the-multiple-containers-pod-working/before/tortoise.yaml @@ -83,7 +83,7 @@ status: role: Monitor tortoisePhase: Working containerResourcePhases: - - containerName: "nginx" + - containerName: "app" resourcePhases: cpu: Working memory: Working diff --git a/controllers/testdata/reconcile-for-the-single-container-pod-dryrun/after/tortoise.yaml b/controllers/testdata/reconcile-for-the-single-container-pod-dryrun/after/tortoise.yaml index b571a8ce..21efea1b 100644 --- a/controllers/testdata/reconcile-for-the-single-container-pod-dryrun/after/tortoise.yaml +++ b/controllers/testdata/reconcile-for-the-single-container-pod-dryrun/after/tortoise.yaml @@ -66,7 +66,7 @@ status: role: Monitor tortoisePhase: Working containerResourcePhases: - - containerName: "nginx" + - containerName: "app" resourcePhases: cpu: Working memory: Working diff --git a/controllers/testdata/reconcile-for-the-single-container-pod-dryrun/before/tortoise.yaml b/controllers/testdata/reconcile-for-the-single-container-pod-dryrun/before/tortoise.yaml index 304eb0ca..c7f220cd 100644 --- a/controllers/testdata/reconcile-for-the-single-container-pod-dryrun/before/tortoise.yaml +++ b/controllers/testdata/reconcile-for-the-single-container-pod-dryrun/before/tortoise.yaml @@ -62,7 +62,7 @@ status: role: Monitor tortoisePhase: Working containerResourcePhases: - - containerName: "nginx" + - containerName: "app" resourcePhases: cpu: Working memory: Working diff --git a/controllers/testdata/reconcile-for-the-single-container-pod-emergency/after/tortoise.yaml b/controllers/testdata/reconcile-for-the-single-container-pod-emergency/after/tortoise.yaml index cdfe52d2..ac881a01 100644 --- a/controllers/testdata/reconcile-for-the-single-container-pod-emergency/after/tortoise.yaml +++ b/controllers/testdata/reconcile-for-the-single-container-pod-emergency/after/tortoise.yaml @@ -66,7 +66,7 @@ status: role: Monitor tortoisePhase: Emergency containerResourcePhases: - - containerName: "nginx" + - containerName: "app" resourcePhases: cpu: Working memory: Working diff --git a/controllers/testdata/reconcile-for-the-single-container-pod-emergency/before/tortoise.yaml b/controllers/testdata/reconcile-for-the-single-container-pod-emergency/before/tortoise.yaml index 4527c364..905704c8 100644 --- a/controllers/testdata/reconcile-for-the-single-container-pod-emergency/before/tortoise.yaml +++ b/controllers/testdata/reconcile-for-the-single-container-pod-emergency/before/tortoise.yaml @@ -62,7 +62,7 @@ status: role: Monitor tortoisePhase: Working containerResourcePhases: - - containerName: "nginx" + - containerName: "app" resourcePhases: cpu: Working memory: Working diff --git a/controllers/testdata/reconcile-for-the-single-container-pod-working/after/tortoise.yaml b/controllers/testdata/reconcile-for-the-single-container-pod-working/after/tortoise.yaml index bc3a573f..d7be922b 100644 --- a/controllers/testdata/reconcile-for-the-single-container-pod-working/after/tortoise.yaml +++ b/controllers/testdata/reconcile-for-the-single-container-pod-working/after/tortoise.yaml @@ -65,7 +65,7 @@ status: role: Monitor tortoisePhase: Working containerResourcePhases: - - containerName: "nginx" + - containerName: "app" resourcePhases: cpu: Working memory: Working diff --git a/controllers/testdata/reconcile-for-the-single-container-pod-working/before/tortoise.yaml b/controllers/testdata/reconcile-for-the-single-container-pod-working/before/tortoise.yaml index fafecbf0..61c454c2 100644 --- a/controllers/testdata/reconcile-for-the-single-container-pod-working/before/tortoise.yaml +++ b/controllers/testdata/reconcile-for-the-single-container-pod-working/before/tortoise.yaml @@ -61,7 +61,7 @@ status: role: Monitor tortoisePhase: Working containerResourcePhases: - - containerName: "nginx" + - containerName: "app" resourcePhases: cpu: Working memory: Working diff --git a/controllers/tortoise_controller.go b/controllers/tortoise_controller.go index 324cd89b..2ad3af0a 100644 --- a/controllers/tortoise_controller.go +++ b/controllers/tortoise_controller.go @@ -134,32 +134,13 @@ func (r *TortoiseReconciler) Reconcile(ctx context.Context, req ctrl.Request) (_ return ctrl.Result{RequeueAfter: r.Interval}, nil } - // TODO: If HPA is modified, we need to change the tortoise phase. - // How to scale during that time? - _, err = r.HpaService.UpdateHPASpecFromTortoiseAutoscalingPolicy(ctx, tortoise) + tortoise, err = r.HpaService.UpdateHPASpecFromTortoiseAutoscalingPolicy(ctx, tortoise, dm) if err != nil { - if !apierrors.IsNotFound(err) { - logger.Error(err, "update HPA spec from Tortoise autoscaling policy", "tortoise", req.NamespacedName) - return ctrl.Result{}, err - } - - logger.V(4).Info("HPA doesn't exist for this tortoise. We may need to recreate hpa", "tortoise", req.NamespacedName) - - // If not found, it's one of: - // - the user don't specify Horizontal in any autoscalingPolicy. - // - In that case, we don't need to create an initial HPA. - // - 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. - // - // In both cases, we just need to call `InitializeHPA` and it handles both cases. - tortoise, err = r.HpaService.InitializeHPA(ctx, tortoise, dm) - if err != nil { - return ctrl.Result{}, err - } + logger.Error(err, "update HPA spec from Tortoise autoscaling policy", "tortoise", req.NamespacedName) + return ctrl.Result{}, err } - vpa, ready, err := r.VpaService.GetTortoiseMonitorVPA(ctx, tortoise) + monitorvpa, ready, err := r.VpaService.GetTortoiseMonitorVPA(ctx, tortoise) if err != nil { logger.Error(err, "failed to get tortoise VPA", "tortoise", req.NamespacedName) return ctrl.Result{}, err @@ -175,6 +156,9 @@ func (r *TortoiseReconciler) Reconcile(ctx context.Context, req ctrl.Request) (_ return ctrl.Result{RequeueAfter: r.Interval}, nil } + // VPA is ready, we mark all Vertical scaling resources as Running. + tortoise = vpa.MakeAllVerticalContainerResourcePhaseWorking(tortoise) + 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) if err != nil { @@ -182,7 +166,7 @@ func (r *TortoiseReconciler) Reconcile(ctx context.Context, req ctrl.Request) (_ return ctrl.Result{}, err } - tortoise = r.TortoiseService.UpdateUpperRecommendation(tortoise, vpa) + tortoise = r.TortoiseService.UpdateUpperRecommendation(tortoise, monitorvpa) tortoise, err = r.RecommenderService.UpdateRecommendations(ctx, tortoise, hpa, dm, now) if err != nil { diff --git a/pkg/hpa/service.go b/pkg/hpa/service.go index 5b29ceaf..a875e9bb 100644 --- a/pkg/hpa/service.go +++ b/pkg/hpa/service.go @@ -140,7 +140,9 @@ type resourceNameAndContainerName struct { // addHPAMetricsFromTortoiseAutoscalingPolicy adds metrics to the HPA based on the autoscaling policy in the tortoise. // Note that it doesn't update the HPA in kube-apiserver, you have to do that after this function. -func (c *Service) addHPAMetricsFromTortoiseAutoscalingPolicy(ctx context.Context, tortoise *autoscalingv1beta1.Tortoise, currenthpa *v2.HorizontalPodAutoscaler) *v2.HorizontalPodAutoscaler { +func (c *Service) addHPAMetricsFromTortoiseAutoscalingPolicy(ctx context.Context, tortoise *autoscalingv1beta1.Tortoise, currenthpa *v2.HorizontalPodAutoscaler) (*v2.HorizontalPodAutoscaler, *autoscalingv1beta1.Tortoise, bool) { + hpaEdited := false + policies := sets.New[string]() horizontalResourceAndContainer := sets.New[resourceNameAndContainerName]() for _, p := range tortoise.Spec.ResourcePolicy { @@ -163,13 +165,13 @@ func (c *Service) addHPAMetricsFromTortoiseAutoscalingPolicy(ctx context.Context needToAddToHPA := horizontalResourceAndContainer.Difference(hpaManagedResourceAndContainer) needToRemoveFromHPA := hpaManagedResourceAndContainer.Difference(horizontalResourceAndContainer) - sortedList := needToAddToHPA.UnsortedList() - sort.SliceStable(sortedList, func(i, j int) bool { - return sortedList[i].containerName < sortedList[j].containerName + sortedNeedToAddToHPA := needToAddToHPA.UnsortedList() + sort.SliceStable(sortedNeedToAddToHPA, func(i, j int) bool { + return sortedNeedToAddToHPA[i].containerName < sortedNeedToAddToHPA[j].containerName }) // add metrics - for _, d := range sortedList { + for _, d := range sortedNeedToAddToHPA { m := v2.MetricSpec{ Type: v2.ContainerResourceMetricSourceType, ContainerResource: &v2.ContainerResourceMetricSource{ @@ -183,6 +185,23 @@ func (c *Service) addHPAMetricsFromTortoiseAutoscalingPolicy(ctx context.Context }, } currenthpa.Spec.Metrics = append(currenthpa.Spec.Metrics, m) + hpaEdited = true + found := false + for i, p := range tortoise.Status.ContainerResourcePhases { + if p.ContainerName == d.containerName { + tortoise.Status.ContainerResourcePhases[i].ResourcePhases[d.rn] = autoscalingv1beta1.ContainerResourcePhaseGatheringData + found = true + break + } + } + if !found { + tortoise.Status.ContainerResourcePhases = append(tortoise.Status.ContainerResourcePhases, autoscalingv1beta1.ContainerResourcePhases{ + ContainerName: d.containerName, + ResourcePhases: map[corev1.ResourceName]autoscalingv1beta1.ContainerResourcePhase{ + d.rn: autoscalingv1beta1.ContainerResourcePhaseGatheringData, + }, + }) + } } // remove metrics @@ -193,12 +212,13 @@ func (c *Service) addHPAMetricsFromTortoiseAutoscalingPolicy(ctx context.Context } if !needToRemoveFromHPA.Has(resourceNameAndContainerName{m.ContainerResource.Name, m.ContainerResource.Container}) { newMetrics = append(newMetrics, m) + hpaEdited = true continue } } currenthpa.Spec.Metrics = newMetrics - return currenthpa + return currenthpa, tortoise, hpaEdited } func (c *Service) CreateHPA(ctx context.Context, tortoise *autoscalingv1beta1.Tortoise, dm *v1.Deployment) (*v2.HorizontalPodAutoscaler, *autoscalingv1beta1.Tortoise, error) { @@ -251,7 +271,7 @@ func (c *Service) CreateHPA(ctx context.Context, tortoise *autoscalingv1beta1.To }, } - hpa = c.addHPAMetricsFromTortoiseAutoscalingPolicy(ctx, tortoise, hpa) + hpa, tortoise, _ = c.addHPAMetricsFromTortoiseAutoscalingPolicy(ctx, tortoise, hpa) tortoise.Status.Targets.HorizontalPodAutoscaler = hpa.Name @@ -319,21 +339,41 @@ func (c *Service) ChangeHPAFromTortoiseRecommendation(tortoise *autoscalingv1bet return hpa, tortoise, nil } -func (c *Service) UpdateHPASpecFromTortoiseAutoscalingPolicy(ctx context.Context, tortoise *autoscalingv1beta1.Tortoise) (*v2.HorizontalPodAutoscaler, error) { +func (c *Service) UpdateHPASpecFromTortoiseAutoscalingPolicy(ctx context.Context, tortoise *autoscalingv1beta1.Tortoise, dm *v1.Deployment) (*autoscalingv1beta1.Tortoise, error) { if !hasHorizontal(tortoise) { err := c.DeleteHPACreatedByTortoise(ctx, tortoise) - if err != nil { - return nil, fmt.Errorf("delete hpa created by tortoise: %w", err) + if err != nil && !apierrors.IsNotFound(err) { + return tortoise, fmt.Errorf("delete hpa created by tortoise: %w", err) } - return nil, nil + // No need to edit container resource phase. + + return tortoise, nil } hpa := &v2.HorizontalPodAutoscaler{} if err := c.c.Get(ctx, types.NamespacedName{Namespace: tortoise.Namespace, Name: tortoise.Status.Targets.HorizontalPodAutoscaler}, hpa); err != nil { - return nil, fmt.Errorf("failed to get hpa on tortoise: %w", err) + if apierrors.IsNotFound(err) { + // If not found, it's one of: + // - 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) + if err != nil { + return tortoise, fmt.Errorf("initialize hpa: %w", err) + } + return tortoise, nil + } + return tortoise, fmt.Errorf("failed to get hpa on tortoise: %w", err) + } + + var newhpa *v2.HorizontalPodAutoscaler + var isHpaEdited bool + newhpa, tortoise, isHpaEdited = c.addHPAMetricsFromTortoiseAutoscalingPolicy(ctx, tortoise, hpa) + if !isHpaEdited { + // User didn't change anything. + return tortoise, nil } - newhpa := c.addHPAMetricsFromTortoiseAutoscalingPolicy(ctx, tortoise, hpa) updateFn := func() error { hpa := &v2.HorizontalPodAutoscaler{} if err := c.c.Get(ctx, types.NamespacedName{Namespace: tortoise.Namespace, Name: tortoise.Status.Targets.HorizontalPodAutoscaler}, hpa); err != nil { @@ -346,10 +386,10 @@ func (c *Service) UpdateHPASpecFromTortoiseAutoscalingPolicy(ctx context.Context } if err := retry.RetryOnConflict(retry.DefaultRetry, updateFn); err != nil { - return nil, err + return tortoise, err } - return hpa, nil + return tortoise, nil } func hasHorizontal(tortoise *autoscalingv1beta1.Tortoise) bool { diff --git a/pkg/hpa/service_test.go b/pkg/hpa/service_test.go index d7d79abf..50a40103 100644 --- a/pkg/hpa/service_test.go +++ b/pkg/hpa/service_test.go @@ -1085,11 +1085,12 @@ func TestService_UpdateHPASpecFromTortoiseAutoscalingPolicy(t *testing.T) { dm *appv1.Deployment } tests := []struct { - name string - initialHPA *v2.HorizontalPodAutoscaler - args args - afterHPA *v2.HorizontalPodAutoscaler - wantErr bool + name string + initialHPA *v2.HorizontalPodAutoscaler + args args + afterHPA *v2.HorizontalPodAutoscaler + wantTortoise *autoscalingv1beta1.Tortoise + wantErr bool }{ { name: "add metrics to existing hpa", @@ -1125,6 +1126,22 @@ func TestService_UpdateHPASpecFromTortoiseAutoscalingPolicy(t *testing.T) { }, }, Status: autoscalingv1beta1.TortoiseStatus{ + ContainerResourcePhases: []autoscalingv1beta1.ContainerResourcePhases{ + { + ContainerName: "app", + ResourcePhases: map[v1.ResourceName]autoscalingv1beta1.ContainerResourcePhase{ + v1.ResourceCPU: autoscalingv1beta1.ContainerResourcePhaseWorking, + v1.ResourceMemory: autoscalingv1beta1.ContainerResourcePhaseWorking, + }, + }, + { + ContainerName: "istio-proxy", + ResourcePhases: map[v1.ResourceName]autoscalingv1beta1.ContainerResourcePhase{ + v1.ResourceCPU: autoscalingv1beta1.ContainerResourcePhaseWorking, + v1.ResourceMemory: autoscalingv1beta1.ContainerResourcePhaseWorking, + }, + }, + }, Targets: autoscalingv1beta1.TargetsStatus{ HorizontalPodAutoscaler: "hpa", }, @@ -1192,6 +1209,58 @@ func TestService_UpdateHPASpecFromTortoiseAutoscalingPolicy(t *testing.T) { }, }, }, + wantTortoise: &autoscalingv1beta1.Tortoise{ + ObjectMeta: metav1.ObjectMeta{ + Name: "tortoise", + Namespace: "default", + }, + Spec: autoscalingv1beta1.TortoiseSpec{ + ResourcePolicy: []autoscalingv1beta1.ContainerResourcePolicy{ + { + ContainerName: "app", + AutoscalingPolicy: map[v1.ResourceName]v1beta1.AutoscalingType{ + v1.ResourceMemory: v1beta1.AutoscalingTypeVertical, + v1.ResourceCPU: v1beta1.AutoscalingTypeHorizontal, + }, + }, + { + ContainerName: "istio-proxy", + AutoscalingPolicy: map[v1.ResourceName]v1beta1.AutoscalingType{ + v1.ResourceMemory: v1beta1.AutoscalingTypeVertical, + v1.ResourceCPU: v1beta1.AutoscalingTypeHorizontal, + }, + }, + }, + TargetRefs: autoscalingv1beta1.TargetRefs{ + HorizontalPodAutoscalerName: pointer.String("existing-hpa"), + ScaleTargetRef: autoscalingv1beta1.CrossVersionObjectReference{ + Kind: "Deployment", + Name: "deployment", + }, + }, + }, + Status: autoscalingv1beta1.TortoiseStatus{ + ContainerResourcePhases: []autoscalingv1beta1.ContainerResourcePhases{ + { + ContainerName: "app", + ResourcePhases: map[v1.ResourceName]autoscalingv1beta1.ContainerResourcePhase{ + v1.ResourceCPU: autoscalingv1beta1.ContainerResourcePhaseWorking, + v1.ResourceMemory: autoscalingv1beta1.ContainerResourcePhaseWorking, + }, + }, + { + ContainerName: "istio-proxy", + ResourcePhases: map[v1.ResourceName]autoscalingv1beta1.ContainerResourcePhase{ + v1.ResourceCPU: autoscalingv1beta1.ContainerResourcePhaseGatheringData, + v1.ResourceMemory: autoscalingv1beta1.ContainerResourcePhaseWorking, + }, + }, + }, + Targets: autoscalingv1beta1.TargetsStatus{ + HorizontalPodAutoscaler: "hpa", + }, + }, + }, afterHPA: &v2.HorizontalPodAutoscaler{ ObjectMeta: metav1.ObjectMeta{ Name: "hpa", @@ -1277,6 +1346,22 @@ func TestService_UpdateHPASpecFromTortoiseAutoscalingPolicy(t *testing.T) { }, }, Status: autoscalingv1beta1.TortoiseStatus{ + ContainerResourcePhases: []autoscalingv1beta1.ContainerResourcePhases{ + { + ContainerName: "app", + ResourcePhases: map[v1.ResourceName]autoscalingv1beta1.ContainerResourcePhase{ + v1.ResourceCPU: autoscalingv1beta1.ContainerResourcePhaseWorking, + v1.ResourceMemory: autoscalingv1beta1.ContainerResourcePhaseWorking, + }, + }, + { + ContainerName: "istio-proxy", + ResourcePhases: map[v1.ResourceName]autoscalingv1beta1.ContainerResourcePhase{ + v1.ResourceCPU: autoscalingv1beta1.ContainerResourcePhaseWorking, + v1.ResourceMemory: autoscalingv1beta1.ContainerResourcePhaseWorking, + }, + }, + }, Targets: autoscalingv1beta1.TargetsStatus{ HorizontalPodAutoscaler: "hpa", }, @@ -1355,6 +1440,58 @@ func TestService_UpdateHPASpecFromTortoiseAutoscalingPolicy(t *testing.T) { }, }, }, + wantTortoise: &autoscalingv1beta1.Tortoise{ + ObjectMeta: metav1.ObjectMeta{ + Name: "tortoise", + Namespace: "default", + }, + Spec: autoscalingv1beta1.TortoiseSpec{ + ResourcePolicy: []autoscalingv1beta1.ContainerResourcePolicy{ + { + ContainerName: "app", + AutoscalingPolicy: map[v1.ResourceName]v1beta1.AutoscalingType{ + v1.ResourceMemory: v1beta1.AutoscalingTypeVertical, + v1.ResourceCPU: v1beta1.AutoscalingTypeVertical, + }, + }, + { + ContainerName: "istio-proxy", + AutoscalingPolicy: map[v1.ResourceName]v1beta1.AutoscalingType{ + v1.ResourceMemory: v1beta1.AutoscalingTypeVertical, + v1.ResourceCPU: v1beta1.AutoscalingTypeHorizontal, + }, + }, + }, + TargetRefs: autoscalingv1beta1.TargetRefs{ + HorizontalPodAutoscalerName: pointer.String("existing-hpa"), + ScaleTargetRef: autoscalingv1beta1.CrossVersionObjectReference{ + Kind: "Deployment", + Name: "deployment", + }, + }, + }, + Status: autoscalingv1beta1.TortoiseStatus{ + ContainerResourcePhases: []autoscalingv1beta1.ContainerResourcePhases{ + { + ContainerName: "app", + ResourcePhases: map[v1.ResourceName]autoscalingv1beta1.ContainerResourcePhase{ + v1.ResourceCPU: autoscalingv1beta1.ContainerResourcePhaseWorking, + v1.ResourceMemory: autoscalingv1beta1.ContainerResourcePhaseWorking, + }, + }, + { + ContainerName: "istio-proxy", + ResourcePhases: map[v1.ResourceName]autoscalingv1beta1.ContainerResourcePhase{ + v1.ResourceCPU: autoscalingv1beta1.ContainerResourcePhaseWorking, + v1.ResourceMemory: autoscalingv1beta1.ContainerResourcePhaseWorking, + }, + }, + }, + Targets: autoscalingv1beta1.TargetsStatus{ + HorizontalPodAutoscaler: "hpa", + }, + }, + }, afterHPA: &v2.HorizontalPodAutoscaler{ ObjectMeta: metav1.ObjectMeta{ Name: "hpa", @@ -1430,6 +1567,22 @@ func TestService_UpdateHPASpecFromTortoiseAutoscalingPolicy(t *testing.T) { }, }, Status: autoscalingv1beta1.TortoiseStatus{ + ContainerResourcePhases: []autoscalingv1beta1.ContainerResourcePhases{ + { + ContainerName: "app", + ResourcePhases: map[v1.ResourceName]autoscalingv1beta1.ContainerResourcePhase{ + v1.ResourceCPU: autoscalingv1beta1.ContainerResourcePhaseWorking, + v1.ResourceMemory: autoscalingv1beta1.ContainerResourcePhaseWorking, + }, + }, + { + ContainerName: "istio-proxy", + ResourcePhases: map[v1.ResourceName]autoscalingv1beta1.ContainerResourcePhase{ + v1.ResourceCPU: autoscalingv1beta1.ContainerResourcePhaseWorking, + v1.ResourceMemory: autoscalingv1beta1.ContainerResourcePhaseWorking, + }, + }, + }, Targets: autoscalingv1beta1.TargetsStatus{ HorizontalPodAutoscaler: "hpa", }, @@ -1511,6 +1664,59 @@ func TestService_UpdateHPASpecFromTortoiseAutoscalingPolicy(t *testing.T) { }, }, }, + wantTortoise: &autoscalingv1beta1.Tortoise{ + ObjectMeta: metav1.ObjectMeta{ + Name: "tortoise", + Namespace: "default", + }, + Spec: autoscalingv1beta1.TortoiseSpec{ + DeletionPolicy: autoscalingv1beta1.DeletionPolicyDeleteAll, + ResourcePolicy: []autoscalingv1beta1.ContainerResourcePolicy{ + { + ContainerName: "app", + AutoscalingPolicy: map[v1.ResourceName]v1beta1.AutoscalingType{ + v1.ResourceMemory: v1beta1.AutoscalingTypeVertical, + v1.ResourceCPU: v1beta1.AutoscalingTypeVertical, + }, + }, + { + ContainerName: "istio-proxy", + AutoscalingPolicy: map[v1.ResourceName]v1beta1.AutoscalingType{ + v1.ResourceMemory: v1beta1.AutoscalingTypeVertical, + v1.ResourceCPU: v1beta1.AutoscalingTypeVertical, + }, + }, + }, + TargetRefs: autoscalingv1beta1.TargetRefs{ + HorizontalPodAutoscalerName: pointer.String("existing-hpa"), + ScaleTargetRef: autoscalingv1beta1.CrossVersionObjectReference{ + Kind: "Deployment", + Name: "deployment", + }, + }, + }, + Status: autoscalingv1beta1.TortoiseStatus{ + ContainerResourcePhases: []autoscalingv1beta1.ContainerResourcePhases{ + { + ContainerName: "app", + ResourcePhases: map[v1.ResourceName]autoscalingv1beta1.ContainerResourcePhase{ + v1.ResourceCPU: autoscalingv1beta1.ContainerResourcePhaseWorking, + v1.ResourceMemory: autoscalingv1beta1.ContainerResourcePhaseWorking, + }, + }, + { + ContainerName: "istio-proxy", + ResourcePhases: map[v1.ResourceName]autoscalingv1beta1.ContainerResourcePhase{ + v1.ResourceCPU: autoscalingv1beta1.ContainerResourcePhaseWorking, + v1.ResourceMemory: autoscalingv1beta1.ContainerResourcePhaseWorking, + }, + }, + }, + Targets: autoscalingv1beta1.TargetsStatus{ + HorizontalPodAutoscaler: "hpa", + }, + }, + }, afterHPA: nil, }, } @@ -1520,11 +1726,16 @@ func TestService_UpdateHPASpecFromTortoiseAutoscalingPolicy(t *testing.T) { if tt.initialHPA != nil { c = New(fake.NewClientBuilder().WithRuntimeObjects(tt.initialHPA).Build(), record.NewFakeRecorder(10), 0.95, 90) } - _, err := c.UpdateHPASpecFromTortoiseAutoscalingPolicy(context.Background(), tt.args.tortoise) + tortoise, err := c.UpdateHPASpecFromTortoiseAutoscalingPolicy(context.Background(), tt.args.tortoise, tt.args.dm) if (err != nil) != tt.wantErr { t.Errorf("Service.UpdateHPASpecFromTortoiseAutoscalingPolicy() error = %v, wantErr %v", err, tt.wantErr) return } + + if d := cmp.Diff(tt.wantTortoise, tortoise); d != "" { + t.Errorf("Service.UpdateHPASpecFromTortoiseAutoscalingPolicy() tortoise diff = %v", d) + } + if tt.afterHPA == nil { // hpa should be removed hpa := &v2.HorizontalPodAutoscaler{} diff --git a/pkg/tortoise/tortoise.go b/pkg/tortoise/tortoise.go index 4c0d3041..1c1e239c 100644 --- a/pkg/tortoise/tortoise.go +++ b/pkg/tortoise/tortoise.go @@ -187,6 +187,22 @@ func (s *Service) initializeTortoise(tortoise *v1beta1.Tortoise, dm *appv1.Deplo } tortoise.Status.Targets.Deployment = dm.Name + for _, p := range tortoise.Spec.ResourcePolicy { + phase := v1beta1.ContainerResourcePhases{ + ContainerName: p.ContainerName, + ResourcePhases: map[corev1.ResourceName]v1beta1.ContainerResourcePhase{}, + } + for rn, policy := range p.AutoscalingPolicy { + if policy == v1beta1.AutoscalingTypeOff { + phase.ResourcePhases[rn] = v1beta1.ContainerResourcePhaseOff + continue + } + + phase.ResourcePhases[rn] = v1beta1.ContainerResourcePhaseGatheringData + } + tortoise.Status.ContainerResourcePhases = append(tortoise.Status.ContainerResourcePhases, phase) + } + return tortoise.DeepCopy() } diff --git a/pkg/tortoise/tortoise_test.go b/pkg/tortoise/tortoise_test.go index add2f740..778df205 100644 --- a/pkg/tortoise/tortoise_test.go +++ b/pkg/tortoise/tortoise_test.go @@ -221,12 +221,31 @@ func TestService_InitializeTortoise(t *testing.T) { want *v1beta1.Tortoise }{ { + name: "weekly minMaxReplicasRoutine", fields: fields{ rangeOfMinMaxReplicasRecommendationHour: 8, minMaxReplicasRoutine: "weekly", timeZone: jst, }, tortoise: &v1beta1.Tortoise{ + Spec: v1beta1.TortoiseSpec{ + ResourcePolicy: []v1beta1.ContainerResourcePolicy{ + { + ContainerName: "app", + AutoscalingPolicy: map[corev1.ResourceName]v1beta1.AutoscalingType{ + corev1.ResourceMemory: v1beta1.AutoscalingTypeVertical, + corev1.ResourceCPU: v1beta1.AutoscalingTypeHorizontal, + }, + }, + { + ContainerName: "istio", + AutoscalingPolicy: map[corev1.ResourceName]v1beta1.AutoscalingType{ + corev1.ResourceMemory: v1beta1.AutoscalingTypeOff, + corev1.ResourceCPU: v1beta1.AutoscalingTypeHorizontal, + }, + }, + }, + }, Status: v1beta1.TortoiseStatus{}, }, deployment: &appv1.Deployment{ @@ -240,12 +259,33 @@ func TestService_InitializeTortoise(t *testing.T) { { Name: "app", }, + { + Name: "istio", + }, }, }, }, }, }, want: &v1beta1.Tortoise{ + Spec: v1beta1.TortoiseSpec{ + ResourcePolicy: []v1beta1.ContainerResourcePolicy{ + { + ContainerName: "app", + AutoscalingPolicy: map[corev1.ResourceName]v1beta1.AutoscalingType{ + corev1.ResourceMemory: v1beta1.AutoscalingTypeVertical, + corev1.ResourceCPU: v1beta1.AutoscalingTypeHorizontal, + }, + }, + { + ContainerName: "istio", + AutoscalingPolicy: map[corev1.ResourceName]v1beta1.AutoscalingType{ + corev1.ResourceMemory: v1beta1.AutoscalingTypeOff, + corev1.ResourceCPU: v1beta1.AutoscalingTypeHorizontal, + }, + }, + }, + }, Status: v1beta1.TortoiseStatus{ TortoisePhase: v1beta1.TortoisePhaseInitializing, Targets: v1beta1.TargetsStatus{Deployment: "deployment"}, @@ -262,6 +302,33 @@ func TestService_InitializeTortoise(t *testing.T) { corev1.ResourceMemory: {}, }, }, + { + ContainerName: "istio", + Recommendation: map[corev1.ResourceName]v1beta1.ResourceQuantity{ + corev1.ResourceCPU: {}, + corev1.ResourceMemory: {}, + }, + MaxRecommendation: map[corev1.ResourceName]v1beta1.ResourceQuantity{ + corev1.ResourceCPU: {}, + corev1.ResourceMemory: {}, + }, + }, + }, + }, + ContainerResourcePhases: []v1beta1.ContainerResourcePhases{ + { + ContainerName: "app", + ResourcePhases: map[corev1.ResourceName]v1beta1.ContainerResourcePhase{ + corev1.ResourceMemory: v1beta1.ContainerResourcePhaseGatheringData, + corev1.ResourceCPU: v1beta1.ContainerResourcePhaseGatheringData, + }, + }, + { + ContainerName: "istio", + ResourcePhases: map[corev1.ResourceName]v1beta1.ContainerResourcePhase{ + corev1.ResourceMemory: v1beta1.ContainerResourcePhaseOff, + corev1.ResourceCPU: v1beta1.ContainerResourcePhaseGatheringData, + }, }, }, Recommendations: v1beta1.Recommendations{ @@ -528,12 +595,24 @@ func TestService_InitializeTortoise(t *testing.T) { }, }, { + name: "daily minMaxReplicasRoutine", fields: fields{ minMaxReplicasRoutine: "daily", rangeOfMinMaxReplicasRecommendationHour: 8, timeZone: jst, }, tortoise: &v1beta1.Tortoise{ + Spec: v1beta1.TortoiseSpec{ + ResourcePolicy: []v1beta1.ContainerResourcePolicy{ + { + ContainerName: "app", + AutoscalingPolicy: map[corev1.ResourceName]v1beta1.AutoscalingType{ + corev1.ResourceMemory: v1beta1.AutoscalingTypeVertical, + corev1.ResourceCPU: v1beta1.AutoscalingTypeHorizontal, + }, + }, + }, + }, Status: v1beta1.TortoiseStatus{}, }, deployment: &appv1.Deployment{ @@ -553,6 +632,17 @@ func TestService_InitializeTortoise(t *testing.T) { }, }, want: &v1beta1.Tortoise{ + Spec: v1beta1.TortoiseSpec{ + ResourcePolicy: []v1beta1.ContainerResourcePolicy{ + { + ContainerName: "app", + AutoscalingPolicy: map[corev1.ResourceName]v1beta1.AutoscalingType{ + corev1.ResourceMemory: v1beta1.AutoscalingTypeVertical, + corev1.ResourceCPU: v1beta1.AutoscalingTypeHorizontal, + }, + }, + }, + }, Status: v1beta1.TortoiseStatus{ TortoisePhase: v1beta1.TortoisePhaseInitializing, Targets: v1beta1.TargetsStatus{Deployment: "deployment"}, @@ -571,6 +661,15 @@ func TestService_InitializeTortoise(t *testing.T) { }, }, }, + ContainerResourcePhases: []v1beta1.ContainerResourcePhases{ + { + ContainerName: "app", + ResourcePhases: map[corev1.ResourceName]v1beta1.ContainerResourcePhase{ + corev1.ResourceCPU: v1beta1.ContainerResourcePhaseGatheringData, + corev1.ResourceMemory: v1beta1.ContainerResourcePhaseGatheringData, + }, + }, + }, Recommendations: v1beta1.Recommendations{ Horizontal: v1beta1.HorizontalRecommendations{ MinReplicas: []v1beta1.ReplicasRecommendation{ diff --git a/pkg/vpa/service.go b/pkg/vpa/service.go index 1d7ef12a..6d3322f6 100644 --- a/pkg/vpa/service.go +++ b/pkg/vpa/service.go @@ -8,6 +8,7 @@ import ( corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/util/sets" v1 "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/apis/autoscaling.k8s.io/v1" "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/client/clientset/versioned" "k8s.io/client-go/rest" @@ -272,3 +273,40 @@ func (c *Service) GetTortoiseMonitorVPA(ctx context.Context, tortoise *autoscali return vpa, false, nil } + +func MakeAllVerticalContainerResourcePhaseWorking(tortoise *autoscalingv1beta1.Tortoise) *autoscalingv1beta1.Tortoise { + verticalResourceAndContainer := sets.New[resourceNameAndContainerName]() + for _, p := range tortoise.Spec.ResourcePolicy { + for rn, ap := range p.AutoscalingPolicy { + if ap == autoscalingv1beta1.AutoscalingTypeVertical { + verticalResourceAndContainer.Insert(resourceNameAndContainerName{rn, p.ContainerName}) + } + } + } + + found := false + for _, d := range verticalResourceAndContainer.UnsortedList() { + for i, p := range tortoise.Status.ContainerResourcePhases { + if p.ContainerName == d.containerName { + tortoise.Status.ContainerResourcePhases[i].ResourcePhases[d.rn] = autoscalingv1beta1.ContainerResourcePhaseWorking + found = true + break + } + } + if !found { + tortoise.Status.ContainerResourcePhases = append(tortoise.Status.ContainerResourcePhases, autoscalingv1beta1.ContainerResourcePhases{ + ContainerName: d.containerName, + ResourcePhases: map[corev1.ResourceName]autoscalingv1beta1.ContainerResourcePhase{ + d.rn: autoscalingv1beta1.ContainerResourcePhaseWorking, + }, + }) + } + } + + return tortoise +} + +type resourceNameAndContainerName struct { + rn corev1.ResourceName + containerName string +} diff --git a/pkg/vpa/service_test.go b/pkg/vpa/service_test.go new file mode 100644 index 00000000..af8810ea --- /dev/null +++ b/pkg/vpa/service_test.go @@ -0,0 +1,123 @@ +package vpa + +import ( + "testing" + + "github.com/google/go-cmp/cmp" + v1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + "github.com/mercari/tortoise/api/v1beta1" + autoscalingv1beta1 "github.com/mercari/tortoise/api/v1beta1" +) + +func TestMakeAllVerticalContainerResourcePhaseRunning(t *testing.T) { + type args struct { + tortoise *autoscalingv1beta1.Tortoise + } + tests := []struct { + name string + args args + want *autoscalingv1beta1.Tortoise + }{ + { + name: "test1", + args: args{ + tortoise: &autoscalingv1beta1.Tortoise{ + ObjectMeta: metav1.ObjectMeta{ + Name: "tortoise", + Namespace: "default", + }, + Spec: autoscalingv1beta1.TortoiseSpec{ + ResourcePolicy: []autoscalingv1beta1.ContainerResourcePolicy{ + { + ContainerName: "app", + AutoscalingPolicy: map[v1.ResourceName]v1beta1.AutoscalingType{ + v1.ResourceMemory: v1beta1.AutoscalingTypeVertical, + v1.ResourceCPU: v1beta1.AutoscalingTypeHorizontal, + }, + }, + { + ContainerName: "istio-proxy", + AutoscalingPolicy: map[v1.ResourceName]v1beta1.AutoscalingType{ + v1.ResourceMemory: v1beta1.AutoscalingTypeVertical, + v1.ResourceCPU: v1beta1.AutoscalingTypeHorizontal, + }, + }, + }, + }, + Status: autoscalingv1beta1.TortoiseStatus{ + ContainerResourcePhases: []autoscalingv1beta1.ContainerResourcePhases{ + { + ContainerName: "app", + ResourcePhases: map[v1.ResourceName]autoscalingv1beta1.ContainerResourcePhase{ + v1.ResourceCPU: autoscalingv1beta1.ContainerResourcePhaseGatheringData, + v1.ResourceMemory: autoscalingv1beta1.ContainerResourcePhaseGatheringData, + }, + }, + { + ContainerName: "istio-proxy", + ResourcePhases: map[v1.ResourceName]autoscalingv1beta1.ContainerResourcePhase{ + v1.ResourceCPU: autoscalingv1beta1.ContainerResourcePhaseGatheringData, + v1.ResourceMemory: autoscalingv1beta1.ContainerResourcePhaseGatheringData, + }, + }, + }, + }, + }, + }, + want: &autoscalingv1beta1.Tortoise{ + ObjectMeta: metav1.ObjectMeta{ + Name: "tortoise", + Namespace: "default", + }, + Spec: autoscalingv1beta1.TortoiseSpec{ + ResourcePolicy: []autoscalingv1beta1.ContainerResourcePolicy{ + { + ContainerName: "app", + AutoscalingPolicy: map[v1.ResourceName]v1beta1.AutoscalingType{ + v1.ResourceMemory: v1beta1.AutoscalingTypeVertical, + v1.ResourceCPU: v1beta1.AutoscalingTypeHorizontal, + }, + }, + { + ContainerName: "istio-proxy", + AutoscalingPolicy: map[v1.ResourceName]v1beta1.AutoscalingType{ + v1.ResourceMemory: v1beta1.AutoscalingTypeVertical, + v1.ResourceCPU: v1beta1.AutoscalingTypeHorizontal, + }, + }, + }, + }, + Status: autoscalingv1beta1.TortoiseStatus{ + ContainerResourcePhases: []autoscalingv1beta1.ContainerResourcePhases{ + { + ContainerName: "app", + ResourcePhases: map[v1.ResourceName]autoscalingv1beta1.ContainerResourcePhase{ + v1.ResourceCPU: autoscalingv1beta1.ContainerResourcePhaseGatheringData, + v1.ResourceMemory: autoscalingv1beta1.ContainerResourcePhaseWorking, + }, + }, + { + ContainerName: "istio-proxy", + ResourcePhases: map[v1.ResourceName]autoscalingv1beta1.ContainerResourcePhase{ + v1.ResourceCPU: autoscalingv1beta1.ContainerResourcePhaseGatheringData, + v1.ResourceMemory: autoscalingv1beta1.ContainerResourcePhaseWorking, + }, + }, + }, + }, + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := MakeAllVerticalContainerResourcePhaseWorking(tt.args.tortoise) + + // use diff to compare + if diff := cmp.Diff(got, tt.want); diff != "" { + t.Fatalf("MakeAllVerticalContainerResourcePhaseRunning() mismatch (-want +got):\n%s", diff) + } + }) + } +}