From 2e261c4dcfb71f85b1234dda4c9eb14fe23f2f93 Mon Sep 17 00:00:00 2001 From: randytqwjp Date: Thu, 12 Dec 2024 14:53:00 +0900 Subject: [PATCH] refactor tortoisephase change into tortoise service and write unit tests --- .../deletion-no-delete/before/hpa.yaml | 19 +++ .../deletion-policy-all/before/hpa.yaml | 19 +++ .../after/hpa.yaml | 20 ++- .../before/hpa.yaml | 19 +++ .../after/hpa.yaml | 20 ++- .../before/hpa.yaml | 19 +++ .../before/hpa.yaml | 19 +++ .../after/hpa.yaml | 20 ++- .../before/hpa.yaml | 19 +++ .../after/hpa.yaml | 20 ++- .../before/hpa.yaml | 19 +++ .../after/hpa.yaml | 20 ++- .../before/hpa.yaml | 19 +++ .../after/hpa.yaml | 15 ++- .../before/hpa.yaml | 14 +++ .../after/hpa.yaml | 20 ++- .../before/hpa.yaml | 19 +++ .../after/hpa.yaml | 20 ++- .../before/hpa.yaml | 19 +++ .../after/hpa.yaml | 15 ++- .../before/hpa.yaml | 14 +++ .../after/hpa.yaml | 15 ++- .../before/hpa.yaml | 14 +++ .../after/hpa.yaml | 15 ++- .../before/hpa.yaml | 14 +++ .../after/hpa.yaml | 15 ++- .../before/hpa.yaml | 14 +++ .../after/hpa.yaml | 15 ++- .../before/hpa.yaml | 14 +++ .../after/hpa.yaml | 15 ++- .../before/hpa.yaml | 14 +++ .../after/hpa.yaml | 15 ++- .../before/hpa.yaml | 14 +++ .../after/hpa.yaml | 15 ++- .../before/hpa.yaml | 14 +++ .../after/hpa.yaml | 15 ++- .../before/hpa.yaml | 14 +++ .../after/hpa.yaml | 15 ++- .../before/hpa.yaml | 14 +++ .../after/hpa.yaml | 15 ++- .../before/hpa.yaml | 14 +++ internal/controller/tortoise_controller.go | 18 ++- pkg/hpa/service.go | 45 ++++--- pkg/hpa/service_test.go | 115 +++++++++++++++++- pkg/tortoise/tortoise.go | 8 ++ pkg/tortoise/tortoise_test.go | 108 ++++++++++++++++ 46 files changed, 905 insertions(+), 67 deletions(-) diff --git a/internal/controller/testdata/deletion-no-delete/before/hpa.yaml b/internal/controller/testdata/deletion-no-delete/before/hpa.yaml index 54cef9df..9bbc7c95 100644 --- a/internal/controller/testdata/deletion-no-delete/before/hpa.yaml +++ b/internal/controller/testdata/deletion-no-delete/before/hpa.yaml @@ -3,6 +3,25 @@ metadata: tortoise.autoscaling.mercari.com/managed-by-tortoise: "true" name: tortoise-hpa-mercari namespace: default +status: + conditions: + - status: "True" + type: AbleToScale + message: "recommended size matches current size" + - status: "True" + type: ScalingActive + message: "the HPA was able to compute the replica count" + currentMetrics: + - containerResource: + container: app + name: cpu + current: + value: 3 + - containerResource: + container: istio-proxy + name: cpu + current: + value: 3 spec: behavior: scaleDown: diff --git a/internal/controller/testdata/deletion-policy-all/before/hpa.yaml b/internal/controller/testdata/deletion-policy-all/before/hpa.yaml index 54cef9df..9bbc7c95 100644 --- a/internal/controller/testdata/deletion-policy-all/before/hpa.yaml +++ b/internal/controller/testdata/deletion-policy-all/before/hpa.yaml @@ -3,6 +3,25 @@ metadata: tortoise.autoscaling.mercari.com/managed-by-tortoise: "true" name: tortoise-hpa-mercari namespace: default +status: + conditions: + - status: "True" + type: AbleToScale + message: "recommended size matches current size" + - status: "True" + type: ScalingActive + message: "the HPA was able to compute the replica count" + currentMetrics: + - containerResource: + container: app + name: cpu + current: + value: 3 + - containerResource: + container: istio-proxy + name: cpu + current: + value: 3 spec: behavior: scaleDown: diff --git a/internal/controller/testdata/mutable-autoscalingpolicy-add-another-horizontal/after/hpa.yaml b/internal/controller/testdata/mutable-autoscalingpolicy-add-another-horizontal/after/hpa.yaml index 99305dee..7c627ee7 100644 --- a/internal/controller/testdata/mutable-autoscalingpolicy-add-another-horizontal/after/hpa.yaml +++ b/internal/controller/testdata/mutable-autoscalingpolicy-add-another-horizontal/after/hpa.yaml @@ -47,5 +47,21 @@ spec: kind: Deployment name: mercari-app status: - currentMetrics: null - desiredReplicas: 0 + conditions: + - status: "True" + type: AbleToScale + message: "recommended size matches current size" + - status: "True" + type: ScalingActive + message: "the HPA was able to compute the replica count" + currentMetrics: + - containerResource: + container: app + name: cpu + current: + value: 3 + - containerResource: + container: istio-proxy + name: cpu + current: + value: 3 diff --git a/internal/controller/testdata/mutable-autoscalingpolicy-add-another-horizontal/before/hpa.yaml b/internal/controller/testdata/mutable-autoscalingpolicy-add-another-horizontal/before/hpa.yaml index 54cef9df..9bbc7c95 100644 --- a/internal/controller/testdata/mutable-autoscalingpolicy-add-another-horizontal/before/hpa.yaml +++ b/internal/controller/testdata/mutable-autoscalingpolicy-add-another-horizontal/before/hpa.yaml @@ -3,6 +3,25 @@ metadata: tortoise.autoscaling.mercari.com/managed-by-tortoise: "true" name: tortoise-hpa-mercari namespace: default +status: + conditions: + - status: "True" + type: AbleToScale + message: "recommended size matches current size" + - status: "True" + type: ScalingActive + message: "the HPA was able to compute the replica count" + currentMetrics: + - containerResource: + container: app + name: cpu + current: + value: 3 + - containerResource: + container: istio-proxy + name: cpu + current: + value: 3 spec: behavior: scaleDown: diff --git a/internal/controller/testdata/mutable-autoscalingpolicy-remove-horizontal-2/after/hpa.yaml b/internal/controller/testdata/mutable-autoscalingpolicy-remove-horizontal-2/after/hpa.yaml index acce44a3..28c7019f 100644 --- a/internal/controller/testdata/mutable-autoscalingpolicy-remove-horizontal-2/after/hpa.yaml +++ b/internal/controller/testdata/mutable-autoscalingpolicy-remove-horizontal-2/after/hpa.yaml @@ -32,5 +32,21 @@ spec: kind: Deployment name: mercari-app status: - currentMetrics: null - desiredReplicas: 0 + conditions: + - status: "True" + type: AbleToScale + message: "recommended size matches current size" + - status: "True" + type: ScalingActive + message: "the HPA was able to compute the replica count" + currentMetrics: + - containerResource: + container: app + name: cpu + current: + value: 3 + - containerResource: + container: istio-proxy + name: cpu + current: + value: 3 diff --git a/internal/controller/testdata/mutable-autoscalingpolicy-remove-horizontal-2/before/hpa.yaml b/internal/controller/testdata/mutable-autoscalingpolicy-remove-horizontal-2/before/hpa.yaml index 21aec690..0c317754 100644 --- a/internal/controller/testdata/mutable-autoscalingpolicy-remove-horizontal-2/before/hpa.yaml +++ b/internal/controller/testdata/mutable-autoscalingpolicy-remove-horizontal-2/before/hpa.yaml @@ -3,6 +3,25 @@ metadata: tortoise.autoscaling.mercari.com/managed-by-tortoise: "true" name: tortoise-hpa-mercari namespace: default +status: + conditions: + - status: "True" + type: AbleToScale + message: "recommended size matches current size" + - status: "True" + type: ScalingActive + message: "the HPA was able to compute the replica count" + currentMetrics: + - containerResource: + container: app + name: cpu + current: + value: 3 + - containerResource: + container: istio-proxy + name: cpu + current: + value: 3 spec: behavior: scaleDown: diff --git a/internal/controller/testdata/mutable-autoscalingpolicy-remove-horizontal/before/hpa.yaml b/internal/controller/testdata/mutable-autoscalingpolicy-remove-horizontal/before/hpa.yaml index 54cef9df..9bbc7c95 100644 --- a/internal/controller/testdata/mutable-autoscalingpolicy-remove-horizontal/before/hpa.yaml +++ b/internal/controller/testdata/mutable-autoscalingpolicy-remove-horizontal/before/hpa.yaml @@ -3,6 +3,25 @@ metadata: tortoise.autoscaling.mercari.com/managed-by-tortoise: "true" name: tortoise-hpa-mercari namespace: default +status: + conditions: + - status: "True" + type: AbleToScale + message: "recommended size matches current size" + - status: "True" + type: ScalingActive + message: "the HPA was able to compute the replica count" + currentMetrics: + - containerResource: + container: app + name: cpu + current: + value: 3 + - containerResource: + container: istio-proxy + name: cpu + current: + value: 3 spec: behavior: scaleDown: diff --git a/internal/controller/testdata/reconcile-for-the-istio-enabled-pod-working/after/hpa.yaml b/internal/controller/testdata/reconcile-for-the-istio-enabled-pod-working/after/hpa.yaml index 836c01e8..2d976100 100644 --- a/internal/controller/testdata/reconcile-for-the-istio-enabled-pod-working/after/hpa.yaml +++ b/internal/controller/testdata/reconcile-for-the-istio-enabled-pod-working/after/hpa.yaml @@ -40,5 +40,21 @@ spec: kind: Deployment name: mercari-app status: - currentMetrics: null - desiredReplicas: 0 + conditions: + - status: "True" + type: AbleToScale + message: "recommended size matches current size" + - status: "True" + type: ScalingActive + message: "the HPA was able to compute the replica count" + currentMetrics: + - containerResource: + container: app + name: cpu + current: + value: 3 + - containerResource: + container: istio-proxy + name: cpu + current: + value: 3 diff --git a/internal/controller/testdata/reconcile-for-the-istio-enabled-pod-working/before/hpa.yaml b/internal/controller/testdata/reconcile-for-the-istio-enabled-pod-working/before/hpa.yaml index 54cef9df..9bbc7c95 100644 --- a/internal/controller/testdata/reconcile-for-the-istio-enabled-pod-working/before/hpa.yaml +++ b/internal/controller/testdata/reconcile-for-the-istio-enabled-pod-working/before/hpa.yaml @@ -3,6 +3,25 @@ metadata: tortoise.autoscaling.mercari.com/managed-by-tortoise: "true" name: tortoise-hpa-mercari namespace: default +status: + conditions: + - status: "True" + type: AbleToScale + message: "recommended size matches current size" + - status: "True" + type: ScalingActive + message: "the HPA was able to compute the replica count" + currentMetrics: + - containerResource: + container: app + name: cpu + current: + value: 3 + - containerResource: + container: istio-proxy + name: cpu + current: + value: 3 spec: behavior: scaleDown: diff --git a/internal/controller/testdata/reconcile-for-the-multiple-containers-pod-during-emergency/after/hpa.yaml b/internal/controller/testdata/reconcile-for-the-multiple-containers-pod-during-emergency/after/hpa.yaml index 0cb536c2..a102d9c1 100644 --- a/internal/controller/testdata/reconcile-for-the-multiple-containers-pod-during-emergency/after/hpa.yaml +++ b/internal/controller/testdata/reconcile-for-the-multiple-containers-pod-during-emergency/after/hpa.yaml @@ -40,5 +40,21 @@ spec: kind: Deployment name: mercari-app status: - currentMetrics: null - desiredReplicas: 0 + conditions: + - status: "True" + type: AbleToScale + message: "recommended size matches current size" + - status: "True" + type: ScalingActive + message: "the HPA was able to compute the replica count" + currentMetrics: + - containerResource: + container: app + name: cpu + current: + value: 3 + - containerResource: + container: istio-proxy + name: cpu + current: + value: 3 diff --git a/internal/controller/testdata/reconcile-for-the-multiple-containers-pod-during-emergency/before/hpa.yaml b/internal/controller/testdata/reconcile-for-the-multiple-containers-pod-during-emergency/before/hpa.yaml index aeeabf2b..3a70606b 100644 --- a/internal/controller/testdata/reconcile-for-the-multiple-containers-pod-during-emergency/before/hpa.yaml +++ b/internal/controller/testdata/reconcile-for-the-multiple-containers-pod-during-emergency/before/hpa.yaml @@ -3,6 +3,25 @@ metadata: tortoise.autoscaling.mercari.com/managed-by-tortoise: "true" name: tortoise-hpa-mercari namespace: default +status: + conditions: + - status: "True" + type: AbleToScale + message: "recommended size matches current size" + - status: "True" + type: ScalingActive + message: "the HPA was able to compute the replica count" + currentMetrics: + - containerResource: + container: app + name: cpu + current: + value: 3 + - containerResource: + container: istio-proxy + name: cpu + current: + value: 3 spec: behavior: scaleDown: diff --git a/internal/controller/testdata/reconcile-for-the-multiple-containers-pod-emergency-started/after/hpa.yaml b/internal/controller/testdata/reconcile-for-the-multiple-containers-pod-emergency-started/after/hpa.yaml index 0cb536c2..a102d9c1 100644 --- a/internal/controller/testdata/reconcile-for-the-multiple-containers-pod-emergency-started/after/hpa.yaml +++ b/internal/controller/testdata/reconcile-for-the-multiple-containers-pod-emergency-started/after/hpa.yaml @@ -40,5 +40,21 @@ spec: kind: Deployment name: mercari-app status: - currentMetrics: null - desiredReplicas: 0 + conditions: + - status: "True" + type: AbleToScale + message: "recommended size matches current size" + - status: "True" + type: ScalingActive + message: "the HPA was able to compute the replica count" + currentMetrics: + - containerResource: + container: app + name: cpu + current: + value: 3 + - containerResource: + container: istio-proxy + name: cpu + current: + value: 3 diff --git a/internal/controller/testdata/reconcile-for-the-multiple-containers-pod-emergency-started/before/hpa.yaml b/internal/controller/testdata/reconcile-for-the-multiple-containers-pod-emergency-started/before/hpa.yaml index d1d756fd..fe5f26b0 100644 --- a/internal/controller/testdata/reconcile-for-the-multiple-containers-pod-emergency-started/before/hpa.yaml +++ b/internal/controller/testdata/reconcile-for-the-multiple-containers-pod-emergency-started/before/hpa.yaml @@ -3,6 +3,25 @@ metadata: tortoise.autoscaling.mercari.com/managed-by-tortoise: "true" name: tortoise-hpa-mercari namespace: default +status: + conditions: + - status: "True" + type: AbleToScale + message: "recommended size matches current size" + - status: "True" + type: ScalingActive + message: "the HPA was able to compute the replica count" + currentMetrics: + - containerResource: + container: app + name: cpu + current: + value: 3 + - containerResource: + container: istio-proxy + name: cpu + current: + value: 3 spec: behavior: scaleDown: diff --git a/internal/controller/testdata/reconcile-for-the-multiple-containers-pod-one-off/after/hpa.yaml b/internal/controller/testdata/reconcile-for-the-multiple-containers-pod-one-off/after/hpa.yaml index 0eb6b7e6..bfc7d4d1 100644 --- a/internal/controller/testdata/reconcile-for-the-multiple-containers-pod-one-off/after/hpa.yaml +++ b/internal/controller/testdata/reconcile-for-the-multiple-containers-pod-one-off/after/hpa.yaml @@ -33,5 +33,16 @@ spec: kind: Deployment name: mercari-app status: - currentMetrics: null - desiredReplicas: 0 + conditions: + - status: "True" + type: AbleToScale + message: "recommended size matches current size" + - status: "True" + type: ScalingActive + message: "the HPA was able to compute the replica count" + currentMetrics: + - containerResource: + container: app + name: cpu + current: + value: 3 diff --git a/internal/controller/testdata/reconcile-for-the-multiple-containers-pod-one-off/before/hpa.yaml b/internal/controller/testdata/reconcile-for-the-multiple-containers-pod-one-off/before/hpa.yaml index 98160014..bdd7234f 100644 --- a/internal/controller/testdata/reconcile-for-the-multiple-containers-pod-one-off/before/hpa.yaml +++ b/internal/controller/testdata/reconcile-for-the-multiple-containers-pod-one-off/before/hpa.yaml @@ -3,6 +3,20 @@ metadata: tortoise.autoscaling.mercari.com/managed-by-tortoise: "true" name: tortoise-hpa-mercari namespace: default +status: + conditions: + - status: "True" + type: AbleToScale + message: "recommended size matches current size" + - status: "True" + type: ScalingActive + message: "the HPA was able to compute the replica count" + currentMetrics: + - containerResource: + container: app + name: cpu + current: + value: 3 spec: behavior: scaleDown: diff --git a/internal/controller/testdata/reconcile-for-the-multiple-containers-pod-suggested-too-small/after/hpa.yaml b/internal/controller/testdata/reconcile-for-the-multiple-containers-pod-suggested-too-small/after/hpa.yaml index 47e4e117..3dbd5407 100644 --- a/internal/controller/testdata/reconcile-for-the-multiple-containers-pod-suggested-too-small/after/hpa.yaml +++ b/internal/controller/testdata/reconcile-for-the-multiple-containers-pod-suggested-too-small/after/hpa.yaml @@ -40,5 +40,21 @@ spec: kind: Deployment name: mercari-app status: - currentMetrics: null - desiredReplicas: 0 + conditions: + - status: "True" + type: AbleToScale + message: "recommended size matches current size" + - status: "True" + type: ScalingActive + message: "the HPA was able to compute the replica count" + currentMetrics: + - containerResource: + container: app + name: cpu + current: + value: 3 + - containerResource: + container: istio-proxy + name: cpu + current: + value: 3 \ No newline at end of file diff --git a/internal/controller/testdata/reconcile-for-the-multiple-containers-pod-suggested-too-small/before/hpa.yaml b/internal/controller/testdata/reconcile-for-the-multiple-containers-pod-suggested-too-small/before/hpa.yaml index 4f9031a6..9dd43455 100644 --- a/internal/controller/testdata/reconcile-for-the-multiple-containers-pod-suggested-too-small/before/hpa.yaml +++ b/internal/controller/testdata/reconcile-for-the-multiple-containers-pod-suggested-too-small/before/hpa.yaml @@ -3,6 +3,25 @@ metadata: tortoise.autoscaling.mercari.com/managed-by-tortoise: "true" name: tortoise-hpa-mercari namespace: default +status: + conditions: + - status: "True" + type: AbleToScale + message: "recommended size matches current size" + - status: "True" + type: ScalingActive + message: "the HPA was able to compute the replica count" + currentMetrics: + - containerResource: + container: app + name: cpu + current: + value: 3 + - containerResource: + container: istio-proxy + name: cpu + current: + value: 3 spec: behavior: scaleDown: diff --git a/internal/controller/testdata/reconcile-for-the-multiple-containers-pod-working/after/hpa.yaml b/internal/controller/testdata/reconcile-for-the-multiple-containers-pod-working/after/hpa.yaml index 836c01e8..2d976100 100644 --- a/internal/controller/testdata/reconcile-for-the-multiple-containers-pod-working/after/hpa.yaml +++ b/internal/controller/testdata/reconcile-for-the-multiple-containers-pod-working/after/hpa.yaml @@ -40,5 +40,21 @@ spec: kind: Deployment name: mercari-app status: - currentMetrics: null - desiredReplicas: 0 + conditions: + - status: "True" + type: AbleToScale + message: "recommended size matches current size" + - status: "True" + type: ScalingActive + message: "the HPA was able to compute the replica count" + currentMetrics: + - containerResource: + container: app + name: cpu + current: + value: 3 + - containerResource: + container: istio-proxy + name: cpu + current: + value: 3 diff --git a/internal/controller/testdata/reconcile-for-the-multiple-containers-pod-working/before/hpa.yaml b/internal/controller/testdata/reconcile-for-the-multiple-containers-pod-working/before/hpa.yaml index 54cef9df..9bbc7c95 100644 --- a/internal/controller/testdata/reconcile-for-the-multiple-containers-pod-working/before/hpa.yaml +++ b/internal/controller/testdata/reconcile-for-the-multiple-containers-pod-working/before/hpa.yaml @@ -3,6 +3,25 @@ metadata: tortoise.autoscaling.mercari.com/managed-by-tortoise: "true" name: tortoise-hpa-mercari namespace: default +status: + conditions: + - status: "True" + type: AbleToScale + message: "recommended size matches current size" + - status: "True" + type: ScalingActive + message: "the HPA was able to compute the replica count" + currentMetrics: + - containerResource: + container: app + name: cpu + current: + value: 3 + - containerResource: + container: istio-proxy + name: cpu + current: + value: 3 spec: behavior: scaleDown: diff --git a/internal/controller/testdata/reconcile-for-the-single-container-pod-backtonormal/after/hpa.yaml b/internal/controller/testdata/reconcile-for-the-single-container-pod-backtonormal/after/hpa.yaml index 1b871784..87aabcd3 100644 --- a/internal/controller/testdata/reconcile-for-the-single-container-pod-backtonormal/after/hpa.yaml +++ b/internal/controller/testdata/reconcile-for-the-single-container-pod-backtonormal/after/hpa.yaml @@ -33,5 +33,16 @@ spec: kind: Deployment name: mercari-app status: - currentMetrics: null - desiredReplicas: 0 + conditions: + - status: "True" + type: AbleToScale + message: "recommended size matches current size" + - status: "True" + type: ScalingActive + message: "the HPA was able to compute the replica count" + currentMetrics: + - containerResource: + container: app + name: cpu + current: + value: 3 diff --git a/internal/controller/testdata/reconcile-for-the-single-container-pod-backtonormal/before/hpa.yaml b/internal/controller/testdata/reconcile-for-the-single-container-pod-backtonormal/before/hpa.yaml index 9c2126fa..e5c83a34 100644 --- a/internal/controller/testdata/reconcile-for-the-single-container-pod-backtonormal/before/hpa.yaml +++ b/internal/controller/testdata/reconcile-for-the-single-container-pod-backtonormal/before/hpa.yaml @@ -3,6 +3,20 @@ metadata: tortoise.autoscaling.mercari.com/managed-by-tortoise: "true" name: tortoise-hpa-mercari namespace: default +status: + conditions: + - status: "True" + type: AbleToScale + message: "recommended size matches current size" + - status: "True" + type: ScalingActive + message: "the HPA was able to compute the replica count" + currentMetrics: + - containerResource: + container: app + name: cpu + current: + value: 3 spec: behavior: scaleDown: diff --git a/internal/controller/testdata/reconcile-for-the-single-container-pod-dryrun/after/hpa.yaml b/internal/controller/testdata/reconcile-for-the-single-container-pod-dryrun/after/hpa.yaml index 2924abbc..a0719772 100644 --- a/internal/controller/testdata/reconcile-for-the-single-container-pod-dryrun/after/hpa.yaml +++ b/internal/controller/testdata/reconcile-for-the-single-container-pod-dryrun/after/hpa.yaml @@ -33,5 +33,16 @@ spec: kind: Deployment name: mercari-app status: - currentMetrics: null - desiredReplicas: 0 + conditions: + - status: "True" + type: AbleToScale + message: "recommended size matches current size" + - status: "True" + type: ScalingActive + message: "the HPA was able to compute the replica count" + currentMetrics: + - containerResource: + container: app + name: cpu + current: + value: 3 \ No newline at end of file diff --git a/internal/controller/testdata/reconcile-for-the-single-container-pod-dryrun/before/hpa.yaml b/internal/controller/testdata/reconcile-for-the-single-container-pod-dryrun/before/hpa.yaml index 98160014..bdd7234f 100644 --- a/internal/controller/testdata/reconcile-for-the-single-container-pod-dryrun/before/hpa.yaml +++ b/internal/controller/testdata/reconcile-for-the-single-container-pod-dryrun/before/hpa.yaml @@ -3,6 +3,20 @@ metadata: tortoise.autoscaling.mercari.com/managed-by-tortoise: "true" name: tortoise-hpa-mercari namespace: default +status: + conditions: + - status: "True" + type: AbleToScale + message: "recommended size matches current size" + - status: "True" + type: ScalingActive + message: "the HPA was able to compute the replica count" + currentMetrics: + - containerResource: + container: app + name: cpu + current: + value: 3 spec: behavior: scaleDown: diff --git a/internal/controller/testdata/reconcile-for-the-single-container-pod-during-emergency/after/hpa.yaml b/internal/controller/testdata/reconcile-for-the-single-container-pod-during-emergency/after/hpa.yaml index 16758a47..b3fee935 100644 --- a/internal/controller/testdata/reconcile-for-the-single-container-pod-during-emergency/after/hpa.yaml +++ b/internal/controller/testdata/reconcile-for-the-single-container-pod-during-emergency/after/hpa.yaml @@ -33,5 +33,16 @@ spec: kind: Deployment name: mercari-app status: - currentMetrics: null - desiredReplicas: 0 + conditions: + - status: "True" + type: AbleToScale + message: "recommended size matches current size" + - status: "True" + type: ScalingActive + message: "the HPA was able to compute the replica count" + currentMetrics: + - containerResource: + container: app + name: cpu + current: + value: 3 diff --git a/internal/controller/testdata/reconcile-for-the-single-container-pod-during-emergency/before/hpa.yaml b/internal/controller/testdata/reconcile-for-the-single-container-pod-during-emergency/before/hpa.yaml index 087cff6f..7836cac4 100644 --- a/internal/controller/testdata/reconcile-for-the-single-container-pod-during-emergency/before/hpa.yaml +++ b/internal/controller/testdata/reconcile-for-the-single-container-pod-during-emergency/before/hpa.yaml @@ -3,6 +3,20 @@ metadata: tortoise.autoscaling.mercari.com/managed-by-tortoise: "true" name: tortoise-hpa-mercari namespace: default +status: + conditions: + - status: "True" + type: AbleToScale + message: "recommended size matches current size" + - status: "True" + type: ScalingActive + message: "the HPA was able to compute the replica count" + currentMetrics: + - containerResource: + container: app + name: cpu + current: + value: 3 spec: behavior: scaleDown: diff --git a/internal/controller/testdata/reconcile-for-the-single-container-pod-emergency-started/after/hpa.yaml b/internal/controller/testdata/reconcile-for-the-single-container-pod-emergency-started/after/hpa.yaml index dc35b0ab..b1b60f03 100644 --- a/internal/controller/testdata/reconcile-for-the-single-container-pod-emergency-started/after/hpa.yaml +++ b/internal/controller/testdata/reconcile-for-the-single-container-pod-emergency-started/after/hpa.yaml @@ -33,5 +33,16 @@ spec: kind: Deployment name: mercari-app status: - currentMetrics: null - desiredReplicas: 0 + conditions: + - status: "True" + type: AbleToScale + message: "recommended size matches current size" + - status: "True" + type: ScalingActive + message: "the HPA was able to compute the replica count" + currentMetrics: + - containerResource: + container: app + name: cpu + current: + value: 3 diff --git a/internal/controller/testdata/reconcile-for-the-single-container-pod-emergency-started/before/hpa.yaml b/internal/controller/testdata/reconcile-for-the-single-container-pod-emergency-started/before/hpa.yaml index bdf08d5d..2e1bdfb5 100644 --- a/internal/controller/testdata/reconcile-for-the-single-container-pod-emergency-started/before/hpa.yaml +++ b/internal/controller/testdata/reconcile-for-the-single-container-pod-emergency-started/before/hpa.yaml @@ -3,6 +3,20 @@ metadata: tortoise.autoscaling.mercari.com/managed-by-tortoise: "true" name: tortoise-hpa-mercari namespace: default +status: + conditions: + - status: "True" + type: AbleToScale + message: "recommended size matches current size" + - status: "True" + type: ScalingActive + message: "the HPA was able to compute the replica count" + currentMetrics: + - containerResource: + container: app + name: cpu + current: + value: 3 spec: behavior: scaleDown: diff --git a/internal/controller/testdata/reconcile-for-the-single-container-pod-gathering-data-finished/after/hpa.yaml b/internal/controller/testdata/reconcile-for-the-single-container-pod-gathering-data-finished/after/hpa.yaml index 7c9ab2d5..450a8cd5 100644 --- a/internal/controller/testdata/reconcile-for-the-single-container-pod-gathering-data-finished/after/hpa.yaml +++ b/internal/controller/testdata/reconcile-for-the-single-container-pod-gathering-data-finished/after/hpa.yaml @@ -33,5 +33,16 @@ spec: kind: Deployment name: mercari-app status: - currentMetrics: null - desiredReplicas: 0 + conditions: + - status: "True" + type: AbleToScale + message: "recommended size matches current size" + - status: "True" + type: ScalingActive + message: "the HPA was able to compute the replica count" + currentMetrics: + - containerResource: + container: app + name: cpu + current: + value: 3 diff --git a/internal/controller/testdata/reconcile-for-the-single-container-pod-gathering-data-finished/before/hpa.yaml b/internal/controller/testdata/reconcile-for-the-single-container-pod-gathering-data-finished/before/hpa.yaml index 98160014..bdd7234f 100644 --- a/internal/controller/testdata/reconcile-for-the-single-container-pod-gathering-data-finished/before/hpa.yaml +++ b/internal/controller/testdata/reconcile-for-the-single-container-pod-gathering-data-finished/before/hpa.yaml @@ -3,6 +3,20 @@ metadata: tortoise.autoscaling.mercari.com/managed-by-tortoise: "true" name: tortoise-hpa-mercari namespace: default +status: + conditions: + - status: "True" + type: AbleToScale + message: "recommended size matches current size" + - status: "True" + type: ScalingActive + message: "the HPA was able to compute the replica count" + currentMetrics: + - containerResource: + container: app + name: cpu + current: + value: 3 spec: behavior: scaleDown: diff --git a/internal/controller/testdata/reconcile-for-the-single-container-pod-gathering-data/after/hpa.yaml b/internal/controller/testdata/reconcile-for-the-single-container-pod-gathering-data/after/hpa.yaml index 2924abbc..a0c6bcf1 100644 --- a/internal/controller/testdata/reconcile-for-the-single-container-pod-gathering-data/after/hpa.yaml +++ b/internal/controller/testdata/reconcile-for-the-single-container-pod-gathering-data/after/hpa.yaml @@ -33,5 +33,16 @@ spec: kind: Deployment name: mercari-app status: - currentMetrics: null - desiredReplicas: 0 + conditions: + - status: "True" + type: AbleToScale + message: "recommended size matches current size" + - status: "True" + type: ScalingActive + message: "the HPA was able to compute the replica count" + currentMetrics: + - containerResource: + container: app + name: cpu + current: + value: 3 diff --git a/internal/controller/testdata/reconcile-for-the-single-container-pod-gathering-data/before/hpa.yaml b/internal/controller/testdata/reconcile-for-the-single-container-pod-gathering-data/before/hpa.yaml index 98160014..bdd7234f 100644 --- a/internal/controller/testdata/reconcile-for-the-single-container-pod-gathering-data/before/hpa.yaml +++ b/internal/controller/testdata/reconcile-for-the-single-container-pod-gathering-data/before/hpa.yaml @@ -3,6 +3,20 @@ metadata: tortoise.autoscaling.mercari.com/managed-by-tortoise: "true" name: tortoise-hpa-mercari namespace: default +status: + conditions: + - status: "True" + type: AbleToScale + message: "recommended size matches current size" + - status: "True" + type: ScalingActive + message: "the HPA was able to compute the replica count" + currentMetrics: + - containerResource: + container: app + name: cpu + current: + value: 3 spec: behavior: scaleDown: diff --git a/internal/controller/testdata/reconcile-for-the-single-container-pod-hpa-changed/after/hpa.yaml b/internal/controller/testdata/reconcile-for-the-single-container-pod-hpa-changed/after/hpa.yaml index d4accc33..cf3a7e97 100644 --- a/internal/controller/testdata/reconcile-for-the-single-container-pod-hpa-changed/after/hpa.yaml +++ b/internal/controller/testdata/reconcile-for-the-single-container-pod-hpa-changed/after/hpa.yaml @@ -40,5 +40,16 @@ spec: kind: Deployment name: mercari-app status: - currentMetrics: null - desiredReplicas: 0 + conditions: + - status: "True" + type: AbleToScale + message: "recommended size matches current size" + - status: "True" + type: ScalingActive + message: "the HPA was able to compute the replica count" + currentMetrics: + - containerResource: + container: app + name: cpu + current: + value: 3 diff --git a/internal/controller/testdata/reconcile-for-the-single-container-pod-hpa-changed/before/hpa.yaml b/internal/controller/testdata/reconcile-for-the-single-container-pod-hpa-changed/before/hpa.yaml index 63f804f9..9b68a123 100644 --- a/internal/controller/testdata/reconcile-for-the-single-container-pod-hpa-changed/before/hpa.yaml +++ b/internal/controller/testdata/reconcile-for-the-single-container-pod-hpa-changed/before/hpa.yaml @@ -3,6 +3,20 @@ metadata: tortoise.autoscaling.mercari.com/managed-by-tortoise: "true" name: tortoise-hpa-mercari namespace: default +status: + conditions: + - status: "True" + type: AbleToScale + message: "recommended size matches current size" + - status: "True" + type: ScalingActive + message: "the HPA was able to compute the replica count" + currentMetrics: + - containerResource: + container: app + name: cpu + current: + value: 3 spec: behavior: scaleDown: diff --git a/internal/controller/testdata/reconcile-for-the-single-container-pod-initializing/after/hpa.yaml b/internal/controller/testdata/reconcile-for-the-single-container-pod-initializing/after/hpa.yaml index 2924abbc..a0c6bcf1 100644 --- a/internal/controller/testdata/reconcile-for-the-single-container-pod-initializing/after/hpa.yaml +++ b/internal/controller/testdata/reconcile-for-the-single-container-pod-initializing/after/hpa.yaml @@ -33,5 +33,16 @@ spec: kind: Deployment name: mercari-app status: - currentMetrics: null - desiredReplicas: 0 + conditions: + - status: "True" + type: AbleToScale + message: "recommended size matches current size" + - status: "True" + type: ScalingActive + message: "the HPA was able to compute the replica count" + currentMetrics: + - containerResource: + container: app + name: cpu + current: + value: 3 diff --git a/internal/controller/testdata/reconcile-for-the-single-container-pod-initializing/before/hpa.yaml b/internal/controller/testdata/reconcile-for-the-single-container-pod-initializing/before/hpa.yaml index 98160014..bdd7234f 100644 --- a/internal/controller/testdata/reconcile-for-the-single-container-pod-initializing/before/hpa.yaml +++ b/internal/controller/testdata/reconcile-for-the-single-container-pod-initializing/before/hpa.yaml @@ -3,6 +3,20 @@ metadata: tortoise.autoscaling.mercari.com/managed-by-tortoise: "true" name: tortoise-hpa-mercari namespace: default +status: + conditions: + - status: "True" + type: AbleToScale + message: "recommended size matches current size" + - status: "True" + type: ScalingActive + message: "the HPA was able to compute the replica count" + currentMetrics: + - containerResource: + container: app + name: cpu + current: + value: 3 spec: behavior: scaleDown: diff --git a/internal/controller/testdata/reconcile-for-the-single-container-pod-partly-working/after/hpa.yaml b/internal/controller/testdata/reconcile-for-the-single-container-pod-partly-working/after/hpa.yaml index 2924abbc..a0c6bcf1 100644 --- a/internal/controller/testdata/reconcile-for-the-single-container-pod-partly-working/after/hpa.yaml +++ b/internal/controller/testdata/reconcile-for-the-single-container-pod-partly-working/after/hpa.yaml @@ -33,5 +33,16 @@ spec: kind: Deployment name: mercari-app status: - currentMetrics: null - desiredReplicas: 0 + conditions: + - status: "True" + type: AbleToScale + message: "recommended size matches current size" + - status: "True" + type: ScalingActive + message: "the HPA was able to compute the replica count" + currentMetrics: + - containerResource: + container: app + name: cpu + current: + value: 3 diff --git a/internal/controller/testdata/reconcile-for-the-single-container-pod-partly-working/before/hpa.yaml b/internal/controller/testdata/reconcile-for-the-single-container-pod-partly-working/before/hpa.yaml index 98160014..bdd7234f 100644 --- a/internal/controller/testdata/reconcile-for-the-single-container-pod-partly-working/before/hpa.yaml +++ b/internal/controller/testdata/reconcile-for-the-single-container-pod-partly-working/before/hpa.yaml @@ -3,6 +3,20 @@ metadata: tortoise.autoscaling.mercari.com/managed-by-tortoise: "true" name: tortoise-hpa-mercari namespace: default +status: + conditions: + - status: "True" + type: AbleToScale + message: "recommended size matches current size" + - status: "True" + type: ScalingActive + message: "the HPA was able to compute the replica count" + currentMetrics: + - containerResource: + container: app + name: cpu + current: + value: 3 spec: behavior: scaleDown: diff --git a/internal/controller/testdata/reconcile-for-the-single-container-pod-too-big/after/hpa.yaml b/internal/controller/testdata/reconcile-for-the-single-container-pod-too-big/after/hpa.yaml index b8db9d51..8ccddef9 100644 --- a/internal/controller/testdata/reconcile-for-the-single-container-pod-too-big/after/hpa.yaml +++ b/internal/controller/testdata/reconcile-for-the-single-container-pod-too-big/after/hpa.yaml @@ -40,5 +40,16 @@ spec: kind: Deployment name: mercari-app status: - currentMetrics: null - desiredReplicas: 0 + conditions: + - status: "True" + type: AbleToScale + message: "recommended size matches current size" + - status: "True" + type: ScalingActive + message: "the HPA was able to compute the replica count" + currentMetrics: + - containerResource: + container: app + name: cpu + current: + value: 3 diff --git a/internal/controller/testdata/reconcile-for-the-single-container-pod-too-big/before/hpa.yaml b/internal/controller/testdata/reconcile-for-the-single-container-pod-too-big/before/hpa.yaml index 8f992d7e..663e7e30 100644 --- a/internal/controller/testdata/reconcile-for-the-single-container-pod-too-big/before/hpa.yaml +++ b/internal/controller/testdata/reconcile-for-the-single-container-pod-too-big/before/hpa.yaml @@ -3,6 +3,20 @@ metadata: tortoise.autoscaling.mercari.com/managed-by-tortoise: "true" name: tortoise-hpa-mercari namespace: default +status: + conditions: + - status: "True" + type: AbleToScale + message: "recommended size matches current size" + - status: "True" + type: ScalingActive + message: "the HPA was able to compute the replica count" + currentMetrics: + - containerResource: + container: app + name: cpu + current: + value: 3 spec: behavior: scaleDown: diff --git a/internal/controller/testdata/reconcile-for-the-single-container-pod-working/after/hpa.yaml b/internal/controller/testdata/reconcile-for-the-single-container-pod-working/after/hpa.yaml index 0ea1203f..7c1e45ef 100644 --- a/internal/controller/testdata/reconcile-for-the-single-container-pod-working/after/hpa.yaml +++ b/internal/controller/testdata/reconcile-for-the-single-container-pod-working/after/hpa.yaml @@ -40,5 +40,16 @@ spec: kind: Deployment name: mercari-app status: - currentMetrics: null - desiredReplicas: 0 + conditions: + - status: "True" + type: AbleToScale + message: "recommended size matches current size" + - status: "True" + type: ScalingActive + message: "the HPA was able to compute the replica count" + currentMetrics: + - containerResource: + container: app + name: cpu + current: + value: 3 diff --git a/internal/controller/testdata/reconcile-for-the-single-container-pod-working/before/hpa.yaml b/internal/controller/testdata/reconcile-for-the-single-container-pod-working/before/hpa.yaml index 8f992d7e..663e7e30 100644 --- a/internal/controller/testdata/reconcile-for-the-single-container-pod-working/before/hpa.yaml +++ b/internal/controller/testdata/reconcile-for-the-single-container-pod-working/before/hpa.yaml @@ -3,6 +3,20 @@ metadata: tortoise.autoscaling.mercari.com/managed-by-tortoise: "true" name: tortoise-hpa-mercari namespace: default +status: + conditions: + - status: "True" + type: AbleToScale + message: "recommended size matches current size" + - status: "True" + type: ScalingActive + message: "the HPA was able to compute the replica count" + currentMetrics: + - containerResource: + container: app + name: cpu + current: + value: 3 spec: behavior: scaleDown: diff --git a/internal/controller/tortoise_controller.go b/internal/controller/tortoise_controller.go index 8c518430..3afbcc41 100644 --- a/internal/controller/tortoise_controller.go +++ b/internal/controller/tortoise_controller.go @@ -90,6 +90,7 @@ var ( func (r *TortoiseReconciler) Reconcile(ctx context.Context, req ctrl.Request) (_ ctrl.Result, reterr error) { logger := log.FromContext(ctx) now := time.Now() + hpaCreated := false if onlyTestNow != nil { now = *onlyTestNow } @@ -203,7 +204,7 @@ func (r *TortoiseReconciler) Reconcile(ctx context.Context, req ctrl.Request) (_ return ctrl.Result{}, fmt.Errorf("add finalizer: %w", err) } - tortoise, err = r.HpaService.UpdateHPASpecFromTortoiseAutoscalingPolicy(ctx, tortoise, hpa, currentDesiredReplicaNum, now) + tortoise, hpaCreated, err = r.HpaService.UpdateHPASpecFromTortoiseAutoscalingPolicy(ctx, tortoise, hpa, currentDesiredReplicaNum, now) if err != nil { logger.Error(err, "update HPA spec from Tortoise autoscaling policy", "tortoise", req.NamespacedName) return ctrl.Result{}, err @@ -239,9 +240,18 @@ func (r *TortoiseReconciler) Reconcile(ctx context.Context, req ctrl.Request) (_ logger.Error(err, "failed to get HPA", "tortoise", req.NamespacedName) return ctrl.Result{}, err } - scalingActive := r.HpaService.CheckHpaMetricStatus(ctx, hpa) - if !scalingActive && tortoise.Spec.UpdateMode == autoscalingv1beta3.UpdateModeAuto && tortoise.Status.TortoisePhase == autoscalingv1beta3.TortoisePhaseWorking { - tortoise.Status.TortoisePhase = v1beta3.TortoisePhaseEmergency + scalingActive, err := r.HpaService.CheckHpaMetricStatus(ctx, hpa) + if err != nil { + if tortoise.Status.TortoisePhase == autoscalingv1beta3.TortoisePhaseWorking && hpaCreated == false { + logger.Error(err, "HPA status abnormal", "tortoise", req.NamespacedName) + return ctrl.Result{}, err + } + err = nil + } + tortoise, err = r.TortoiseService.UpdateTortoisePhaseIfHPAIsUnhealthy(ctx, scalingActive, tortoise) + if err != nil { + logger.Error(err, "Tortoise could not switch to emergency mode", "tortoise", req.NamespacedName) + return ctrl.Result{}, err } tortoise = r.TortoiseService.UpdateContainerRecommendationFromVPA(tortoise, monitorvpa, now) diff --git a/pkg/hpa/service.go b/pkg/hpa/service.go index 06ee77f3..78bdaf2d 100644 --- a/pkg/hpa/service.go +++ b/pkg/hpa/service.go @@ -498,10 +498,11 @@ func (c *Service) UpdateHPASpecFromTortoiseAutoscalingPolicy( givenHPA *v2.HorizontalPodAutoscaler, replicaNum int32, now time.Time, -) (*autoscalingv1beta3.Tortoise, error) { +) (*autoscalingv1beta3.Tortoise, bool, error) { + hpaCreated := false if tortoise.Spec.UpdateMode == autoscalingv1beta3.UpdateModeOff { // When UpdateMode is Off, we don't update HPA. - return tortoise, nil + return tortoise, hpaCreated, nil } if !HasHorizontal(tortoise) { @@ -509,21 +510,21 @@ func (c *Service) UpdateHPASpecFromTortoiseAutoscalingPolicy( // HPA should be created by Tortoise, which can be deleted. err := c.DeleteHPACreatedByTortoise(ctx, tortoise) if err != nil && !apierrors.IsNotFound(err) { - return tortoise, fmt.Errorf("delete hpa created by tortoise: %w", err) + return tortoise, hpaCreated, fmt.Errorf("delete hpa created by tortoise: %w", err) } c.recorder.Event(tortoise, corev1.EventTypeNormal, event.HPADeleted, fmt.Sprintf("Deleted a HPA %s/%s because tortoise has no resource to scale horizontally", tortoise.Namespace, tortoise.Status.Targets.HorizontalPodAutoscaler)) } else { // We cannot delete the HPA because it's specified by the user. err := c.disableHPA(ctx, tortoise, replicaNum) if err != nil { - return tortoise, fmt.Errorf("disable hpa: %w", err) + return tortoise, hpaCreated, fmt.Errorf("disable hpa: %w", err) } c.recorder.Event(tortoise, corev1.EventTypeNormal, event.HPADisabled, fmt.Sprintf("Disabled a HPA %s/%s because tortoise has no resource to scale horizontally", tortoise.Namespace, tortoise.Status.Targets.HorizontalPodAutoscaler)) } // No need to edit container resource phase. - return tortoise, nil + return tortoise, hpaCreated, nil } hpa := &v2.HorizontalPodAutoscaler{} @@ -535,14 +536,15 @@ func (c *Service) UpdateHPASpecFromTortoiseAutoscalingPolicy( // - In that case, we need to create an initial HPA or give an annotation to existing HPA. tortoise, err = c.InitializeHPA(ctx, tortoise, replicaNum, now) if err != nil { - return tortoise, fmt.Errorf("initialize hpa: %w", err) + return tortoise, hpaCreated, fmt.Errorf("initialize hpa: %w", err) } + hpaCreated = true c.recorder.Event(tortoise, corev1.EventTypeNormal, event.HPACreated, fmt.Sprintf("Initialized a HPA %s/%s because tortoise has resource to scale horizontally", tortoise.Namespace, tortoise.Status.Targets.HorizontalPodAutoscaler)) - return tortoise, nil + return tortoise, hpaCreated, nil } - return tortoise, fmt.Errorf("failed to get hpa on tortoise: %w", err) + return tortoise, hpaCreated, fmt.Errorf("failed to get hpa on tortoise: %w", err) } var newhpa *v2.HorizontalPodAutoscaler @@ -550,7 +552,7 @@ func (c *Service) UpdateHPASpecFromTortoiseAutoscalingPolicy( newhpa, tortoise, isHpaEdited = c.syncHPAMetricsWithTortoiseAutoscalingPolicy(ctx, tortoise, hpa, now) if !isHpaEdited { // User didn't change anything. - return tortoise, nil + return tortoise, hpaCreated, nil } retryNumber := -1 @@ -569,12 +571,12 @@ func (c *Service) UpdateHPASpecFromTortoiseAutoscalingPolicy( } if err := retry.RetryOnConflict(retry.DefaultRetry, updateFn); err != nil { - return tortoise, fmt.Errorf("update hpa: %w (%v times retried)", err, replicaNum) + return tortoise, hpaCreated, fmt.Errorf("update hpa: %w (%v times retried)", err, replicaNum) } c.recorder.Event(tortoise, corev1.EventTypeNormal, event.HPAUpdated, fmt.Sprintf("Updated a HPA %s/%s because the autoscaling policy is changed in the tortoise", tortoise.Namespace, tortoise.Status.Targets.HorizontalPodAutoscaler)) - return tortoise, nil + return tortoise, hpaCreated, nil } func HasHorizontal(tortoise *autoscalingv1beta3.Tortoise) bool { @@ -767,27 +769,24 @@ func (c *Service) excludeExternalMetric(ctx context.Context, hpa *v2.HorizontalP return newHPA } -func (c *Service) CheckHpaMetricStatus(ctx context.Context, currenthpa *v2.HorizontalPodAutoscaler) bool { +func (c *Service) CheckHpaMetricStatus(ctx context.Context, currenthpa *v2.HorizontalPodAutoscaler) (bool, error) { //currenthpa = currenthpa.DeepCopy() logger := log.FromContext(ctx) if currenthpa == nil { logger.Info("empty HPA passed into status check, ignore") - return true + return true, nil } if reflect.DeepEqual(currenthpa.Status, v2.HorizontalPodAutoscalerStatus{}) { - logger.Info("HPA empty status, switch to emergency mode") - return false + return false, fmt.Errorf("HPA empty status, switch to emergency mode") } if currenthpa.Status.Conditions == nil { - logger.Info("HPA empty conditions, switch to emergency mode") - return false + return false, fmt.Errorf("HPA empty conditions, switch to emergency mode") } if currenthpa.Status.CurrentMetrics == nil { - logger.Info("HPA no metrics, switch to emergency mode") - return false + return false, fmt.Errorf("HPA no metrics, switch to emergency mode") } conditions := currenthpa.Status.Conditions currentMetrics := currenthpa.Status.CurrentMetrics @@ -797,7 +796,7 @@ func (c *Service) CheckHpaMetricStatus(ctx context.Context, currenthpa *v2.Horiz if condition.Type == "ScalingActive" && condition.Status == "False" && condition.Reason == "FailedGetResourceMetric" { //switch to Emergency mode since no metrics logger.Info("HPA failed to get resource metrics, switch to emergency mode") - return false + return false, nil } } } @@ -806,14 +805,14 @@ func (c *Service) CheckHpaMetricStatus(ctx context.Context, currenthpa *v2.Horiz for _, currentMetric := range currentMetrics { if !currentMetric.ContainerResource.Current.Value.IsZero() { //Can still get metrics for some containers, scale based on those - return true + return true, nil } } logger.Info("HPA all metrics return 0, switch to emergency mode") - return false + return false, nil } logger.Info("HPA status check passed") - return true + return true, nil } diff --git a/pkg/hpa/service_test.go b/pkg/hpa/service_test.go index 734fb7a5..c7780820 100644 --- a/pkg/hpa/service_test.go +++ b/pkg/hpa/service_test.go @@ -4636,7 +4636,7 @@ func TestService_UpdateHPASpecFromTortoiseAutoscalingPolicy(t *testing.T) { // givenHPA is only non-nil when the tortoise has a reference to an existing HPA givenHPA = tt.initialHPA } - tortoise, err := c.UpdateHPASpecFromTortoiseAutoscalingPolicy(context.Background(), tt.args.tortoise, givenHPA, tt.args.replicaNum, time.Now()) + tortoise, _, err := c.UpdateHPASpecFromTortoiseAutoscalingPolicy(context.Background(), tt.args.tortoise, givenHPA, tt.args.replicaNum, time.Now()) if (err != nil) != tt.wantErr { t.Errorf("Service.UpdateHPASpecFromTortoiseAutoscalingPolicy() error = %v, wantErr %v", err, tt.wantErr) return @@ -4890,6 +4890,114 @@ func TestService_CheckHpaMetricStatus(t *testing.T) { }, result: false, }, + { + name: "HPA working normally", + HPA: &v2.HorizontalPodAutoscaler{ + ObjectMeta: metav1.ObjectMeta{ + Name: "existing-hpa", + Namespace: "default", + }, + Status: v2.HorizontalPodAutoscalerStatus{ + Conditions: []v2.HorizontalPodAutoscalerCondition{ + { + Status: "True", + Type: v2.AbleToScale, + Message: "recommended size matches current size", + }, + { + Status: "True", + Type: v2.ScalingActive, + Message: "the HPA was able to successfully calculate a replica count from cpu container resource utilization (percentage of request)", + }, + { + Status: "False", + Type: v2.ScalingLimited, + Message: "the desired count is within the acceptable range", + }, + }, + CurrentMetrics: []v2.MetricStatus{ + { + Type: "ContainerResource", + ContainerResource: &v2.ContainerResourceMetricStatus{ + Container: "app", + Current: v2.MetricValueStatus{ + AverageUtilization: ptr.To[int32](70), + AverageValue: resource.NewQuantity(5, resource.DecimalSI), + Value: resource.NewQuantity(5, resource.DecimalSI), + }, + Name: v1.ResourceCPU, + }, + }, + { + Type: "ContainerResource", + ContainerResource: &v2.ContainerResourceMetricStatus{ + Container: "istio-proxy", + Current: v2.MetricValueStatus{ + AverageUtilization: ptr.To[int32](70), + AverageValue: resource.NewQuantity(5, resource.DecimalSI), + Value: resource.NewQuantity(5, resource.DecimalSI), + }, + Name: v1.ResourceCPU, + }, + }, + }, + }, + Spec: v2.HorizontalPodAutoscalerSpec{ + MinReplicas: ptrInt32(3), + MaxReplicas: 1000, + Metrics: []v2.MetricSpec{ + { + Type: v2.ContainerResourceMetricSourceType, + ContainerResource: &v2.ContainerResourceMetricSource{ + Name: v1.ResourceCPU, + Container: "app", + Target: v2.MetricTarget{ + AverageUtilization: ptr.To[int32](70), + Type: v2.UtilizationMetricType, + }, + }, + }, + { + Type: v2.ContainerResourceMetricSourceType, + ContainerResource: &v2.ContainerResourceMetricSource{ + Name: v1.ResourceCPU, + Container: "istio-proxy", + Target: v2.MetricTarget{ + AverageUtilization: ptr.To[int32](70), + Type: v2.UtilizationMetricType, + }, + }, + }, + }, + ScaleTargetRef: v2.CrossVersionObjectReference{ + Kind: "Deployment", + Name: "deployment", + APIVersion: "apps/v1", + }, + Behavior: &v2.HorizontalPodAutoscalerBehavior{ + ScaleUp: &v2.HPAScalingRules{ + Policies: []v2.HPAScalingPolicy{ + { + Type: v2.PercentScalingPolicy, + Value: 100, + PeriodSeconds: 60, + }, + }, + }, + ScaleDown: &v2.HPAScalingRules{ + Policies: []v2.HPAScalingPolicy{ + { + Type: v2.PercentScalingPolicy, + Value: 2, + PeriodSeconds: 90, + }, + }, + }, + }, + }, + }, + result: true, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -4897,7 +5005,10 @@ func TestService_CheckHpaMetricStatus(t *testing.T) { if err != nil { t.Fatalf("New() error = %v", err) } - status := c.CheckHpaMetricStatus(context.Background(), tt.HPA) + status, err := c.CheckHpaMetricStatus(context.Background(), tt.HPA) + if err != nil { + t.Fatalf("New() error = %v", err) + } if status != tt.result { t.Errorf("Service.checkHpaMetricStatus() status test: %s failed", tt.name) return diff --git a/pkg/tortoise/tortoise.go b/pkg/tortoise/tortoise.go index ef1e0490..1e77874a 100644 --- a/pkg/tortoise/tortoise.go +++ b/pkg/tortoise/tortoise.go @@ -816,3 +816,11 @@ func recommendationIncreaseAnyResource(oldTortoise, newTortoise *v1beta3.Tortois return false } + +func (c *Service) UpdateTortoisePhaseIfHPAIsUnhealthy(ctx context.Context, scalingActive bool, tortoise *v1beta3.Tortoise) (*v1beta3.Tortoise, error) { + if !scalingActive && tortoise.Spec.UpdateMode == v1beta3.UpdateModeAuto && tortoise.Status.TortoisePhase == v1beta3.TortoisePhaseWorking { + //switch to Emergency mode since no metrics + tortoise.Status.TortoisePhase = v1beta3.TortoisePhaseEmergency + } + return tortoise, nil +} diff --git a/pkg/tortoise/tortoise_test.go b/pkg/tortoise/tortoise_test.go index b90e26f1..ef416375 100644 --- a/pkg/tortoise/tortoise_test.go +++ b/pkg/tortoise/tortoise_test.go @@ -4605,3 +4605,111 @@ func TestService_UpdateResourceRequest(t *testing.T) { }) } } + +func TestService_UpdateTortoisePhaseIfHPAIsUnhealthy(t *testing.T) { + type args struct { + t *v1beta3.Tortoise + scalingActive bool + } + tests := []struct { + name string + args args + wantTortoise *v1beta3.Tortoise + }{ + { + name: "healthy HPA tortoise working", + args: args{ + t: &v1beta3.Tortoise{ + ObjectMeta: metav1.ObjectMeta{Name: "t", Namespace: "test"}, + Status: v1beta3.TortoiseStatus{ + TortoisePhase: v1beta3.TortoisePhaseWorking, + }, + Spec: v1beta3.TortoiseSpec{ + UpdateMode: v1beta3.UpdateModeAuto, + }, + }, + scalingActive: true, + }, + wantTortoise: &v1beta3.Tortoise{ + ObjectMeta: metav1.ObjectMeta{Name: "t", Namespace: "test", ResourceVersion: "1"}, + Status: v1beta3.TortoiseStatus{ + TortoisePhase: v1beta3.TortoisePhaseWorking, + }, + Spec: v1beta3.TortoiseSpec{ + UpdateMode: v1beta3.UpdateModeAuto, + }, + }, + }, + { + name: "unhealthy HPA tortoise working", + args: args{ + t: &v1beta3.Tortoise{ + ObjectMeta: metav1.ObjectMeta{Name: "t", Namespace: "test"}, + Status: v1beta3.TortoiseStatus{ + TortoisePhase: v1beta3.TortoisePhaseWorking, + }, + Spec: v1beta3.TortoiseSpec{ + UpdateMode: v1beta3.UpdateModeAuto, + }, + }, + scalingActive: false, + }, + wantTortoise: &v1beta3.Tortoise{ + ObjectMeta: metav1.ObjectMeta{Name: "t", Namespace: "test", ResourceVersion: "1"}, + Status: v1beta3.TortoiseStatus{ + TortoisePhase: v1beta3.TortoisePhaseEmergency, + }, + Spec: v1beta3.TortoiseSpec{ + UpdateMode: v1beta3.UpdateModeAuto, + }, + }, + }, + { + name: "unhealthy HPA tortoise off", + args: args{ + t: &v1beta3.Tortoise{ + ObjectMeta: metav1.ObjectMeta{Name: "t", Namespace: "test"}, + Status: v1beta3.TortoiseStatus{ + TortoisePhase: v1beta3.TortoisePhaseWorking, + }, + Spec: v1beta3.TortoiseSpec{ + UpdateMode: v1beta3.UpdateModeOff, + }, + }, + scalingActive: false, + }, + wantTortoise: &v1beta3.Tortoise{ + ObjectMeta: metav1.ObjectMeta{Name: "t", Namespace: "test", ResourceVersion: "1"}, + Status: v1beta3.TortoiseStatus{ + TortoisePhase: v1beta3.TortoisePhaseWorking, + }, + Spec: v1beta3.TortoiseSpec{ + UpdateMode: v1beta3.UpdateModeOff, + }, + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + scheme := runtime.NewScheme() + err := v1beta3.AddToScheme(scheme) + if err != nil { + t.Fatalf("failed to add to scheme: %v", err) + } + c := fake.NewClientBuilder().WithScheme(scheme).Build() + err = c.Create(context.Background(), tt.args.t) + if err != nil { + t.Fatalf("create tortoise: %v", err) + } + s := &Service{ + c: c, + lastTimeUpdateTortoise: make(map[client.ObjectKey]time.Time), + } + + s.UpdateTortoisePhaseIfHPAIsUnhealthy(context.Background(), tt.args.scalingActive, tt.args.t) + if d := cmp.Diff(tt.args.t, tt.wantTortoise); d != "" { + t.Errorf("UpdateTortoiseStatus() diff = %v", d) + } + }) + } +}