Unverified Commit 663551be authored by Kubernetes Submit Queue's avatar Kubernetes Submit Queue Committed by GitHub

Merge pull request #67252 from jbartosik/metric-sanitization

Automatic merge from submit-queue (batch tested with PRs 66916, 67252, 67794, 67619, 67328). If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>. Fix HPA sample sanitization **What this PR does / why we need it**: @mwielgus pointed out a case when HPA fails as a result of my changes to HPA algorithm: - Have pods that use a lot of CPU during initilization, become ready right after they initialize, - Trigger a scale up, - When new pods become ready will will count their usage (even though it's not related to any work that needs doing), - This triggers another scale up, even though existing pods can handle work, no problem. The fix is: - Use all samples for non-cpu metrics. - Only use CPU samples if: - Pod is ready and was started more than 2 minutes ago, or - Pod is unready and last readiness change happened more than 10s after it was started. Reasoning behind this in: https://docs.google.com/document/d/1UdtYedhmCxjaJIQi6hwJMY0eHQQKxlVD8lSHZC1BPOA/edit **Which issue(s) this PR fixes** *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close the issue(s) when PR gets merged)*: **Special notes for your reviewer**: **Release note**: ```release-note Replace scale up forbidden window with disregarding CPU samples collected when pod was initializing. ```
parents a697d71c 4fd6a168
...@@ -93,6 +93,8 @@ API rule violation: names_match,k8s.io/kubernetes/pkg/apis/componentconfig/v1alp ...@@ -93,6 +93,8 @@ API rule violation: names_match,k8s.io/kubernetes/pkg/apis/componentconfig/v1alp
API rule violation: names_match,k8s.io/kubernetes/pkg/apis/componentconfig/v1alpha1,HPAControllerConfiguration,HorizontalPodAutoscalerDownscaleForbiddenWindow API rule violation: names_match,k8s.io/kubernetes/pkg/apis/componentconfig/v1alpha1,HPAControllerConfiguration,HorizontalPodAutoscalerDownscaleForbiddenWindow
API rule violation: names_match,k8s.io/kubernetes/pkg/apis/componentconfig/v1alpha1,HPAControllerConfiguration,HorizontalPodAutoscalerTolerance API rule violation: names_match,k8s.io/kubernetes/pkg/apis/componentconfig/v1alpha1,HPAControllerConfiguration,HorizontalPodAutoscalerTolerance
API rule violation: names_match,k8s.io/kubernetes/pkg/apis/componentconfig/v1alpha1,HPAControllerConfiguration,HorizontalPodAutoscalerUseRESTClients API rule violation: names_match,k8s.io/kubernetes/pkg/apis/componentconfig/v1alpha1,HPAControllerConfiguration,HorizontalPodAutoscalerUseRESTClients
API rule violation: names_match,k8s.io/kubernetes/pkg/apis/componentconfig/v1alpha1,HPAControllerConfiguration,HorizontalPodAutoscalerCPUTaintPeriod
API rule violation: names_match,k8s.io/kubernetes/pkg/apis/componentconfig/v1alpha1,HPAControllerConfiguration,HorizontalPodAutoscalerInitialReadinessDelay
API rule violation: names_match,k8s.io/kubernetes/pkg/apis/componentconfig/v1alpha1,JobControllerConfiguration,ConcurrentJobSyncs API rule violation: names_match,k8s.io/kubernetes/pkg/apis/componentconfig/v1alpha1,JobControllerConfiguration,ConcurrentJobSyncs
API rule violation: names_match,k8s.io/kubernetes/pkg/apis/componentconfig/v1alpha1,KubeCloudSharedConfiguration,Port API rule violation: names_match,k8s.io/kubernetes/pkg/apis/componentconfig/v1alpha1,KubeCloudSharedConfiguration,Port
API rule violation: names_match,k8s.io/kubernetes/pkg/apis/componentconfig/v1alpha1,KubeCloudSharedConfiguration,Address API rule violation: names_match,k8s.io/kubernetes/pkg/apis/componentconfig/v1alpha1,KubeCloudSharedConfiguration,Address
......
...@@ -84,6 +84,8 @@ func startHPAControllerWithMetricsClient(ctx ControllerContext, metricsClient me ...@@ -84,6 +84,8 @@ func startHPAControllerWithMetricsClient(ctx ControllerContext, metricsClient me
metricsClient, metricsClient,
hpaClient.CoreV1(), hpaClient.CoreV1(),
ctx.ComponentConfig.HPAController.HorizontalPodAutoscalerTolerance, ctx.ComponentConfig.HPAController.HorizontalPodAutoscalerTolerance,
ctx.ComponentConfig.HPAController.HorizontalPodAutoscalerCPUTaintPeriod.Duration,
ctx.ComponentConfig.HPAController.HorizontalPodAutoscalerInitialReadinessDelay.Duration,
) )
go podautoscaler.NewHorizontalController( go podautoscaler.NewHorizontalController(
hpaClient.CoreV1(), hpaClient.CoreV1(),
......
...@@ -30,6 +30,8 @@ type HPAControllerOptions struct { ...@@ -30,6 +30,8 @@ type HPAControllerOptions struct {
HorizontalPodAutoscalerDownscaleForbiddenWindow metav1.Duration HorizontalPodAutoscalerDownscaleForbiddenWindow metav1.Duration
HorizontalPodAutoscalerUpscaleForbiddenWindow metav1.Duration HorizontalPodAutoscalerUpscaleForbiddenWindow metav1.Duration
HorizontalPodAutoscalerSyncPeriod metav1.Duration HorizontalPodAutoscalerSyncPeriod metav1.Duration
HorizontalPodAutoscalerCPUTaintPeriod metav1.Duration
HorizontalPodAutoscalerInitialReadinessDelay metav1.Duration
} }
// AddFlags adds flags related to HPAController for controller manager to the specified FlagSet. // AddFlags adds flags related to HPAController for controller manager to the specified FlagSet.
...@@ -44,6 +46,8 @@ func (o *HPAControllerOptions) AddFlags(fs *pflag.FlagSet) { ...@@ -44,6 +46,8 @@ func (o *HPAControllerOptions) AddFlags(fs *pflag.FlagSet) {
fs.DurationVar(&o.HorizontalPodAutoscalerDownscaleForbiddenWindow.Duration, "horizontal-pod-autoscaler-downscale-delay", o.HorizontalPodAutoscalerDownscaleForbiddenWindow.Duration, "The period since last downscale, before another downscale can be performed in horizontal pod autoscaler.") fs.DurationVar(&o.HorizontalPodAutoscalerDownscaleForbiddenWindow.Duration, "horizontal-pod-autoscaler-downscale-delay", o.HorizontalPodAutoscalerDownscaleForbiddenWindow.Duration, "The period since last downscale, before another downscale can be performed in horizontal pod autoscaler.")
fs.Float64Var(&o.HorizontalPodAutoscalerTolerance, "horizontal-pod-autoscaler-tolerance", o.HorizontalPodAutoscalerTolerance, "The minimum change (from 1.0) in the desired-to-actual metrics ratio for the horizontal pod autoscaler to consider scaling.") fs.Float64Var(&o.HorizontalPodAutoscalerTolerance, "horizontal-pod-autoscaler-tolerance", o.HorizontalPodAutoscalerTolerance, "The minimum change (from 1.0) in the desired-to-actual metrics ratio for the horizontal pod autoscaler to consider scaling.")
fs.BoolVar(&o.HorizontalPodAutoscalerUseRESTClients, "horizontal-pod-autoscaler-use-rest-clients", o.HorizontalPodAutoscalerUseRESTClients, "If set to true, causes the horizontal pod autoscaler controller to use REST clients through the kube-aggregator, instead of using the legacy metrics client through the API server proxy. This is required for custom metrics support in the horizontal pod autoscaler.") fs.BoolVar(&o.HorizontalPodAutoscalerUseRESTClients, "horizontal-pod-autoscaler-use-rest-clients", o.HorizontalPodAutoscalerUseRESTClients, "If set to true, causes the horizontal pod autoscaler controller to use REST clients through the kube-aggregator, instead of using the legacy metrics client through the API server proxy. This is required for custom metrics support in the horizontal pod autoscaler.")
fs.DurationVar(&o.HorizontalPodAutoscalerCPUTaintPeriod.Duration, "horizontal-pod-autoscaler-cpu-taint-period", o.HorizontalPodAutoscalerCPUTaintPeriod.Duration, "The period after pod start for which CPU samples are considered tainted by initialization.")
fs.DurationVar(&o.HorizontalPodAutoscalerInitialReadinessDelay.Duration, "horizontal-pod-autoscaler-initial-readiness-delay", o.HorizontalPodAutoscalerInitialReadinessDelay.Duration, "The period after pod start during which readiness changes will be treated as initial readiness.")
} }
// ApplyTo fills up HPAController config with options. // ApplyTo fills up HPAController config with options.
......
...@@ -134,6 +134,8 @@ func NewKubeControllerManagerOptions() (*KubeControllerManagerOptions, error) { ...@@ -134,6 +134,8 @@ func NewKubeControllerManagerOptions() (*KubeControllerManagerOptions, error) {
HorizontalPodAutoscalerSyncPeriod: componentConfig.HPAController.HorizontalPodAutoscalerSyncPeriod, HorizontalPodAutoscalerSyncPeriod: componentConfig.HPAController.HorizontalPodAutoscalerSyncPeriod,
HorizontalPodAutoscalerUpscaleForbiddenWindow: componentConfig.HPAController.HorizontalPodAutoscalerUpscaleForbiddenWindow, HorizontalPodAutoscalerUpscaleForbiddenWindow: componentConfig.HPAController.HorizontalPodAutoscalerUpscaleForbiddenWindow,
HorizontalPodAutoscalerDownscaleForbiddenWindow: componentConfig.HPAController.HorizontalPodAutoscalerDownscaleForbiddenWindow, HorizontalPodAutoscalerDownscaleForbiddenWindow: componentConfig.HPAController.HorizontalPodAutoscalerDownscaleForbiddenWindow,
HorizontalPodAutoscalerCPUTaintPeriod: componentConfig.HPAController.HorizontalPodAutoscalerCPUTaintPeriod,
HorizontalPodAutoscalerInitialReadinessDelay: componentConfig.HPAController.HorizontalPodAutoscalerInitialReadinessDelay,
HorizontalPodAutoscalerTolerance: componentConfig.HPAController.HorizontalPodAutoscalerTolerance, HorizontalPodAutoscalerTolerance: componentConfig.HPAController.HorizontalPodAutoscalerTolerance,
HorizontalPodAutoscalerUseRESTClients: componentConfig.HPAController.HorizontalPodAutoscalerUseRESTClients, HorizontalPodAutoscalerUseRESTClients: componentConfig.HPAController.HorizontalPodAutoscalerUseRESTClients,
}, },
......
...@@ -73,6 +73,8 @@ func TestAddFlags(t *testing.T) { ...@@ -73,6 +73,8 @@ func TestAddFlags(t *testing.T) {
"--horizontal-pod-autoscaler-downscale-delay=2m", "--horizontal-pod-autoscaler-downscale-delay=2m",
"--horizontal-pod-autoscaler-sync-period=45s", "--horizontal-pod-autoscaler-sync-period=45s",
"--horizontal-pod-autoscaler-upscale-delay=1m", "--horizontal-pod-autoscaler-upscale-delay=1m",
"--horizontal-pod-autoscaler-cpu-taint-period=90s",
"--horizontal-pod-autoscaler-initial-readiness-delay=50s",
"--http2-max-streams-per-connection=47", "--http2-max-streams-per-connection=47",
"--kube-api-burst=100", "--kube-api-burst=100",
"--kube-api-content-type=application/json", "--kube-api-content-type=application/json",
...@@ -185,6 +187,8 @@ func TestAddFlags(t *testing.T) { ...@@ -185,6 +187,8 @@ func TestAddFlags(t *testing.T) {
HorizontalPodAutoscalerSyncPeriod: metav1.Duration{Duration: 45 * time.Second}, HorizontalPodAutoscalerSyncPeriod: metav1.Duration{Duration: 45 * time.Second},
HorizontalPodAutoscalerUpscaleForbiddenWindow: metav1.Duration{Duration: 1 * time.Minute}, HorizontalPodAutoscalerUpscaleForbiddenWindow: metav1.Duration{Duration: 1 * time.Minute},
HorizontalPodAutoscalerDownscaleForbiddenWindow: metav1.Duration{Duration: 2 * time.Minute}, HorizontalPodAutoscalerDownscaleForbiddenWindow: metav1.Duration{Duration: 2 * time.Minute},
HorizontalPodAutoscalerCPUTaintPeriod: metav1.Duration{Duration: 90 * time.Second},
HorizontalPodAutoscalerInitialReadinessDelay: metav1.Duration{Duration: 50 * time.Second},
HorizontalPodAutoscalerTolerance: 0.1, HorizontalPodAutoscalerTolerance: 0.1,
HorizontalPodAutoscalerUseRESTClients: true, HorizontalPodAutoscalerUseRESTClients: true,
}, },
......
...@@ -271,6 +271,14 @@ type HPAControllerConfiguration struct { ...@@ -271,6 +271,14 @@ type HPAControllerConfiguration struct {
// through the kube-aggregator when enabled, instead of using the legacy metrics client // through the kube-aggregator when enabled, instead of using the legacy metrics client
// through the API server proxy. // through the API server proxy.
HorizontalPodAutoscalerUseRESTClients bool HorizontalPodAutoscalerUseRESTClients bool
// HorizontalPodAutoscalerCPUTaintPeriod is period after pod start for which HPA will consider CPU
// samples from the pod contaminated by initialization and disregard them.
HorizontalPodAutoscalerCPUTaintPeriod metav1.Duration
// HorizontalPodAutoscalerInitialReadinessDelay is period after pod start during which readiness
// changes are treated as readiness being set for the first time. The only effect of this is that
// HPA will disregard CPU samples from unready pods that had last readiness change during that
// period.
HorizontalPodAutoscalerInitialReadinessDelay metav1.Duration
} }
type JobControllerConfiguration struct { type JobControllerConfiguration struct {
......
...@@ -89,6 +89,14 @@ func SetDefaults_KubeControllerManagerConfiguration(obj *KubeControllerManagerCo ...@@ -89,6 +89,14 @@ func SetDefaults_KubeControllerManagerConfiguration(obj *KubeControllerManagerCo
if obj.HPAController.HorizontalPodAutoscalerUpscaleForbiddenWindow == zero { if obj.HPAController.HorizontalPodAutoscalerUpscaleForbiddenWindow == zero {
obj.HPAController.HorizontalPodAutoscalerUpscaleForbiddenWindow = metav1.Duration{Duration: 3 * time.Minute} obj.HPAController.HorizontalPodAutoscalerUpscaleForbiddenWindow = metav1.Duration{Duration: 3 * time.Minute}
} }
if obj.HPAController.HorizontalPodAutoscalerCPUTaintPeriod == zero {
// Assuming CPU is collected every minute and initialization takes another minute HPA should
// disregard samples from first two minutes as contaminated by initialization.
obj.HPAController.HorizontalPodAutoscalerCPUTaintPeriod = metav1.Duration{Duration: time.Minute}
}
if obj.HPAController.HorizontalPodAutoscalerInitialReadinessDelay == zero {
obj.HPAController.HorizontalPodAutoscalerInitialReadinessDelay = metav1.Duration{Duration: 30 * time.Second}
}
if obj.HPAController.HorizontalPodAutoscalerDownscaleForbiddenWindow == zero { if obj.HPAController.HorizontalPodAutoscalerDownscaleForbiddenWindow == zero {
obj.HPAController.HorizontalPodAutoscalerDownscaleForbiddenWindow = metav1.Duration{Duration: 5 * time.Minute} obj.HPAController.HorizontalPodAutoscalerDownscaleForbiddenWindow = metav1.Duration{Duration: 5 * time.Minute}
} }
......
...@@ -320,6 +320,14 @@ type HPAControllerConfiguration struct { ...@@ -320,6 +320,14 @@ type HPAControllerConfiguration struct {
// through the kube-aggregator when enabled, instead of using the legacy metrics client // through the kube-aggregator when enabled, instead of using the legacy metrics client
// through the API server proxy. // through the API server proxy.
HorizontalPodAutoscalerUseRESTClients *bool HorizontalPodAutoscalerUseRESTClients *bool
// HorizontalPodAutoscalerCPUTaintPeriod is period after pod start for which HPA will consider CPU
// samples from the pod contaminated by initialization and disregard them.
HorizontalPodAutoscalerCPUTaintPeriod metav1.Duration
// HorizontalPodAutoscalerInitialReadinessDelay is period after pod start during which readiness
// changes are treated as readiness being set for the first time. The only effect of this is that
// HPA will disregard CPU samples from unready pods that had last readiness change during that
// period.
HorizontalPodAutoscalerInitialReadinessDelay metav1.Duration
} }
type JobControllerConfiguration struct { type JobControllerConfiguration struct {
......
...@@ -606,6 +606,8 @@ func autoConvert_v1alpha1_HPAControllerConfiguration_To_componentconfig_HPAContr ...@@ -606,6 +606,8 @@ func autoConvert_v1alpha1_HPAControllerConfiguration_To_componentconfig_HPAContr
if err := v1.Convert_Pointer_bool_To_bool(&in.HorizontalPodAutoscalerUseRESTClients, &out.HorizontalPodAutoscalerUseRESTClients, s); err != nil { if err := v1.Convert_Pointer_bool_To_bool(&in.HorizontalPodAutoscalerUseRESTClients, &out.HorizontalPodAutoscalerUseRESTClients, s); err != nil {
return err return err
} }
out.HorizontalPodAutoscalerCPUTaintPeriod = in.HorizontalPodAutoscalerCPUTaintPeriod
out.HorizontalPodAutoscalerInitialReadinessDelay = in.HorizontalPodAutoscalerInitialReadinessDelay
return nil return nil
} }
...@@ -622,6 +624,8 @@ func autoConvert_componentconfig_HPAControllerConfiguration_To_v1alpha1_HPAContr ...@@ -622,6 +624,8 @@ func autoConvert_componentconfig_HPAControllerConfiguration_To_v1alpha1_HPAContr
if err := v1.Convert_bool_To_Pointer_bool(&in.HorizontalPodAutoscalerUseRESTClients, &out.HorizontalPodAutoscalerUseRESTClients, s); err != nil { if err := v1.Convert_bool_To_Pointer_bool(&in.HorizontalPodAutoscalerUseRESTClients, &out.HorizontalPodAutoscalerUseRESTClients, s); err != nil {
return err return err
} }
out.HorizontalPodAutoscalerCPUTaintPeriod = in.HorizontalPodAutoscalerCPUTaintPeriod
out.HorizontalPodAutoscalerInitialReadinessDelay = in.HorizontalPodAutoscalerInitialReadinessDelay
return nil return nil
} }
......
...@@ -242,6 +242,8 @@ func (in *HPAControllerConfiguration) DeepCopyInto(out *HPAControllerConfigurati ...@@ -242,6 +242,8 @@ func (in *HPAControllerConfiguration) DeepCopyInto(out *HPAControllerConfigurati
*out = new(bool) *out = new(bool)
**out = **in **out = **in
} }
out.HorizontalPodAutoscalerCPUTaintPeriod = in.HorizontalPodAutoscalerCPUTaintPeriod
out.HorizontalPodAutoscalerInitialReadinessDelay = in.HorizontalPodAutoscalerInitialReadinessDelay
return return
} }
......
...@@ -232,6 +232,8 @@ func (in *HPAControllerConfiguration) DeepCopyInto(out *HPAControllerConfigurati ...@@ -232,6 +232,8 @@ func (in *HPAControllerConfiguration) DeepCopyInto(out *HPAControllerConfigurati
out.HorizontalPodAutoscalerSyncPeriod = in.HorizontalPodAutoscalerSyncPeriod out.HorizontalPodAutoscalerSyncPeriod = in.HorizontalPodAutoscalerSyncPeriod
out.HorizontalPodAutoscalerUpscaleForbiddenWindow = in.HorizontalPodAutoscalerUpscaleForbiddenWindow out.HorizontalPodAutoscalerUpscaleForbiddenWindow = in.HorizontalPodAutoscalerUpscaleForbiddenWindow
out.HorizontalPodAutoscalerDownscaleForbiddenWindow = in.HorizontalPodAutoscalerDownscaleForbiddenWindow out.HorizontalPodAutoscalerDownscaleForbiddenWindow = in.HorizontalPodAutoscalerDownscaleForbiddenWindow
out.HorizontalPodAutoscalerCPUTaintPeriod = in.HorizontalPodAutoscalerCPUTaintPeriod
out.HorizontalPodAutoscalerInitialReadinessDelay = in.HorizontalPodAutoscalerInitialReadinessDelay
return return
} }
......
...@@ -73,6 +73,7 @@ go_test( ...@@ -73,6 +73,7 @@ go_test(
"//staging/src/k8s.io/apimachinery/pkg/labels:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/labels:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/runtime:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/runtime:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/runtime/schema:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/runtime/schema:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/util/sets:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/watch:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/watch:go_default_library",
"//staging/src/k8s.io/client-go/informers:go_default_library", "//staging/src/k8s.io/client-go/informers:go_default_library",
"//staging/src/k8s.io/client-go/kubernetes/fake:go_default_library", "//staging/src/k8s.io/client-go/kubernetes/fake:go_default_library",
......
...@@ -103,6 +103,7 @@ type testCase struct { ...@@ -103,6 +103,7 @@ type testCase struct {
reportedLevels []uint64 reportedLevels []uint64
reportedCPURequests []resource.Quantity reportedCPURequests []resource.Quantity
reportedPodReadiness []v1.ConditionStatus reportedPodReadiness []v1.ConditionStatus
reportedPodStartTime []metav1.Time
reportedPodPhase []v1.PodPhase reportedPodPhase []v1.PodPhase
scaleUpdated bool scaleUpdated bool
statusUpdated bool statusUpdated bool
...@@ -261,6 +262,10 @@ func (tc *testCase) prepareTestClient(t *testing.T) (*fake.Clientset, *metricsfa ...@@ -261,6 +262,10 @@ func (tc *testCase) prepareTestClient(t *testing.T) (*fake.Clientset, *metricsfa
if tc.reportedPodReadiness != nil { if tc.reportedPodReadiness != nil {
podReadiness = tc.reportedPodReadiness[i] podReadiness = tc.reportedPodReadiness[i]
} }
var podStartTime metav1.Time
if tc.reportedPodStartTime != nil {
podStartTime = tc.reportedPodStartTime[i]
}
podPhase := v1.PodRunning podPhase := v1.PodRunning
if tc.reportedPodPhase != nil { if tc.reportedPodPhase != nil {
...@@ -283,6 +288,7 @@ func (tc *testCase) prepareTestClient(t *testing.T) (*fake.Clientset, *metricsfa ...@@ -283,6 +288,7 @@ func (tc *testCase) prepareTestClient(t *testing.T) (*fake.Clientset, *metricsfa
Status: podReadiness, Status: podReadiness,
}, },
}, },
StartTime: &podStartTime,
}, },
ObjectMeta: metav1.ObjectMeta{ ObjectMeta: metav1.ObjectMeta{
Name: podName, Name: podName,
...@@ -636,11 +642,7 @@ func (tc *testCase) setupController(t *testing.T) (*HorizontalController, inform ...@@ -636,11 +642,7 @@ func (tc *testCase) setupController(t *testing.T) (*HorizontalController, inform
return true, obj, nil return true, obj, nil
}) })
replicaCalc := &ReplicaCalculator{ replicaCalc := NewReplicaCalculator(metricsClient, testClient.Core(), defaultTestingTolerance, defaultTestingCpuTaintAfterStart, defaultTestingDelayOfInitialReadinessStatus)
metricsClient: metricsClient,
podsGetter: testClient.Core(),
tolerance: defaultTestingTolerance,
}
informerFactory := informers.NewSharedInformerFactory(testClient, controller.NoResyncPeriodFunc()) informerFactory := informers.NewSharedInformerFactory(testClient, controller.NoResyncPeriodFunc())
defaultDownscaleForbiddenWindow := 5 * time.Minute defaultDownscaleForbiddenWindow := 5 * time.Minute
...@@ -660,6 +662,14 @@ func (tc *testCase) setupController(t *testing.T) (*HorizontalController, inform ...@@ -660,6 +662,14 @@ func (tc *testCase) setupController(t *testing.T) (*HorizontalController, inform
return hpaController, informerFactory return hpaController, informerFactory
} }
func hotCpuCreationTime() metav1.Time {
return metav1.Time{Time: time.Now()}
}
func coolCpuCreationTime() metav1.Time {
return metav1.Time{Time: time.Now().Add(-3 * time.Minute)}
}
func (tc *testCase) runTestWithController(t *testing.T, hpaController *HorizontalController, informerFactory informers.SharedInformerFactory) { func (tc *testCase) runTestWithController(t *testing.T, hpaController *HorizontalController, informerFactory informers.SharedInformerFactory) {
stop := make(chan struct{}) stop := make(chan struct{})
defer close(stop) defer close(stop)
...@@ -716,6 +726,23 @@ func TestScaleUpUnreadyLessScale(t *testing.T) { ...@@ -716,6 +726,23 @@ func TestScaleUpUnreadyLessScale(t *testing.T) {
tc.runTest(t) tc.runTest(t)
} }
func TestScaleUpHotCpuLessScale(t *testing.T) {
tc := testCase{
minReplicas: 2,
maxReplicas: 6,
initialReplicas: 3,
expectedDesiredReplicas: 4,
CPUTarget: 30,
CPUCurrent: 60,
verifyCPUCurrent: true,
reportedLevels: []uint64{300, 500, 700},
reportedCPURequests: []resource.Quantity{resource.MustParse("1.0"), resource.MustParse("1.0"), resource.MustParse("1.0")},
reportedPodStartTime: []metav1.Time{hotCpuCreationTime(), coolCpuCreationTime(), coolCpuCreationTime()},
useMetricsAPI: true,
}
tc.runTest(t)
}
func TestScaleUpUnreadyNoScale(t *testing.T) { func TestScaleUpUnreadyNoScale(t *testing.T) {
tc := testCase{ tc := testCase{
minReplicas: 2, minReplicas: 2,
...@@ -738,6 +765,29 @@ func TestScaleUpUnreadyNoScale(t *testing.T) { ...@@ -738,6 +765,29 @@ func TestScaleUpUnreadyNoScale(t *testing.T) {
tc.runTest(t) tc.runTest(t)
} }
func TestScaleUpHotCpuNoScale(t *testing.T) {
tc := testCase{
minReplicas: 2,
maxReplicas: 6,
initialReplicas: 3,
expectedDesiredReplicas: 3,
CPUTarget: 30,
CPUCurrent: 40,
verifyCPUCurrent: true,
reportedLevels: []uint64{400, 500, 700},
reportedCPURequests: []resource.Quantity{resource.MustParse("1.0"), resource.MustParse("1.0"), resource.MustParse("1.0")},
reportedPodReadiness: []v1.ConditionStatus{v1.ConditionTrue, v1.ConditionFalse, v1.ConditionFalse},
reportedPodStartTime: []metav1.Time{coolCpuCreationTime(), hotCpuCreationTime(), hotCpuCreationTime()},
useMetricsAPI: true,
expectedConditions: statusOkWithOverrides(autoscalingv2.HorizontalPodAutoscalerCondition{
Type: autoscalingv2.AbleToScale,
Status: v1.ConditionTrue,
Reason: "ReadyForNewScale",
}),
}
tc.runTest(t)
}
func TestScaleUpIgnoresFailedPods(t *testing.T) { func TestScaleUpIgnoresFailedPods(t *testing.T) {
tc := testCase{ tc := testCase{
minReplicas: 2, minReplicas: 2,
...@@ -818,12 +868,12 @@ func TestScaleUpCM(t *testing.T) { ...@@ -818,12 +868,12 @@ func TestScaleUpCM(t *testing.T) {
tc.runTest(t) tc.runTest(t)
} }
func TestScaleUpCMUnreadyLessScale(t *testing.T) { func TestScaleUpCMUnreadyAndHotCpuNoLessScale(t *testing.T) {
tc := testCase{ tc := testCase{
minReplicas: 2, minReplicas: 2,
maxReplicas: 6, maxReplicas: 6,
initialReplicas: 3, initialReplicas: 3,
expectedDesiredReplicas: 4, expectedDesiredReplicas: 6,
CPUTarget: 0, CPUTarget: 0,
metricsTarget: []autoscalingv2.MetricSpec{ metricsTarget: []autoscalingv2.MetricSpec{
{ {
...@@ -836,17 +886,18 @@ func TestScaleUpCMUnreadyLessScale(t *testing.T) { ...@@ -836,17 +886,18 @@ func TestScaleUpCMUnreadyLessScale(t *testing.T) {
}, },
reportedLevels: []uint64{50000, 10000, 30000}, reportedLevels: []uint64{50000, 10000, 30000},
reportedPodReadiness: []v1.ConditionStatus{v1.ConditionTrue, v1.ConditionTrue, v1.ConditionFalse}, reportedPodReadiness: []v1.ConditionStatus{v1.ConditionTrue, v1.ConditionTrue, v1.ConditionFalse},
reportedPodStartTime: []metav1.Time{coolCpuCreationTime(), coolCpuCreationTime(), hotCpuCreationTime()},
reportedCPURequests: []resource.Quantity{resource.MustParse("1.0"), resource.MustParse("1.0"), resource.MustParse("1.0")}, reportedCPURequests: []resource.Quantity{resource.MustParse("1.0"), resource.MustParse("1.0"), resource.MustParse("1.0")},
} }
tc.runTest(t) tc.runTest(t)
} }
func TestScaleUpCMUnreadyNoScaleWouldScaleDown(t *testing.T) { func TestScaleUpCMUnreadyandCpuHot(t *testing.T) {
tc := testCase{ tc := testCase{
minReplicas: 2, minReplicas: 2,
maxReplicas: 6, maxReplicas: 6,
initialReplicas: 3, initialReplicas: 3,
expectedDesiredReplicas: 3, expectedDesiredReplicas: 6,
CPUTarget: 0, CPUTarget: 0,
metricsTarget: []autoscalingv2.MetricSpec{ metricsTarget: []autoscalingv2.MetricSpec{
{ {
...@@ -859,11 +910,48 @@ func TestScaleUpCMUnreadyNoScaleWouldScaleDown(t *testing.T) { ...@@ -859,11 +910,48 @@ func TestScaleUpCMUnreadyNoScaleWouldScaleDown(t *testing.T) {
}, },
reportedLevels: []uint64{50000, 15000, 30000}, reportedLevels: []uint64{50000, 15000, 30000},
reportedPodReadiness: []v1.ConditionStatus{v1.ConditionFalse, v1.ConditionTrue, v1.ConditionFalse}, reportedPodReadiness: []v1.ConditionStatus{v1.ConditionFalse, v1.ConditionTrue, v1.ConditionFalse},
reportedPodStartTime: []metav1.Time{hotCpuCreationTime(), coolCpuCreationTime(), hotCpuCreationTime()},
reportedCPURequests: []resource.Quantity{resource.MustParse("1.0"), resource.MustParse("1.0"), resource.MustParse("1.0")}, reportedCPURequests: []resource.Quantity{resource.MustParse("1.0"), resource.MustParse("1.0"), resource.MustParse("1.0")},
expectedConditions: statusOkWithOverrides(autoscalingv2.HorizontalPodAutoscalerCondition{ expectedConditions: statusOkWithOverrides(autoscalingv2.HorizontalPodAutoscalerCondition{
Type: autoscalingv2.AbleToScale, Type: autoscalingv2.AbleToScale,
Status: v1.ConditionTrue, Status: v1.ConditionTrue,
Reason: "ReadyForNewScale", Reason: "SucceededRescale",
}, autoscalingv2.HorizontalPodAutoscalerCondition{
Type: autoscalingv2.ScalingLimited,
Status: v1.ConditionTrue,
Reason: "TooManyReplicas",
}),
}
tc.runTest(t)
}
func TestScaleUpHotCpuNoScaleWouldScaleDown(t *testing.T) {
tc := testCase{
minReplicas: 2,
maxReplicas: 6,
initialReplicas: 3,
expectedDesiredReplicas: 6,
CPUTarget: 0,
metricsTarget: []autoscalingv2.MetricSpec{
{
Type: autoscalingv2.PodsMetricSourceType,
Pods: &autoscalingv2.PodsMetricSource{
MetricName: "qps",
TargetAverageValue: resource.MustParse("15.0"),
},
},
},
reportedLevels: []uint64{50000, 15000, 30000},
reportedCPURequests: []resource.Quantity{resource.MustParse("1.0"), resource.MustParse("1.0"), resource.MustParse("1.0")},
reportedPodStartTime: []metav1.Time{hotCpuCreationTime(), coolCpuCreationTime(), hotCpuCreationTime()},
expectedConditions: statusOkWithOverrides(autoscalingv2.HorizontalPodAutoscalerCondition{
Type: autoscalingv2.AbleToScale,
Status: v1.ConditionTrue,
Reason: "SucceededRescale",
}, autoscalingv2.HorizontalPodAutoscalerCondition{
Type: autoscalingv2.ScalingLimited,
Status: v1.ConditionTrue,
Reason: "TooManyReplicas",
}), }),
} }
tc.runTest(t) tc.runTest(t)
...@@ -1043,7 +1131,7 @@ func TestScaleDownPerPodCMExternal(t *testing.T) { ...@@ -1043,7 +1131,7 @@ func TestScaleDownPerPodCMExternal(t *testing.T) {
tc.runTest(t) tc.runTest(t)
} }
func TestScaleDownIgnoresUnreadyPods(t *testing.T) { func TestScaleDownIncludeUnreadyPods(t *testing.T) {
tc := testCase{ tc := testCase{
minReplicas: 2, minReplicas: 2,
maxReplicas: 6, maxReplicas: 6,
...@@ -1060,6 +1148,23 @@ func TestScaleDownIgnoresUnreadyPods(t *testing.T) { ...@@ -1060,6 +1148,23 @@ func TestScaleDownIgnoresUnreadyPods(t *testing.T) {
tc.runTest(t) tc.runTest(t)
} }
func TestScaleDownIgnoreHotCpuPods(t *testing.T) {
tc := testCase{
minReplicas: 2,
maxReplicas: 6,
initialReplicas: 5,
expectedDesiredReplicas: 2,
CPUTarget: 50,
CPUCurrent: 30,
verifyCPUCurrent: true,
reportedLevels: []uint64{100, 300, 500, 250, 250},
reportedCPURequests: []resource.Quantity{resource.MustParse("1.0"), resource.MustParse("1.0"), resource.MustParse("1.0"), resource.MustParse("1.0"), resource.MustParse("1.0")},
useMetricsAPI: true,
reportedPodStartTime: []metav1.Time{coolCpuCreationTime(), coolCpuCreationTime(), coolCpuCreationTime(), hotCpuCreationTime(), hotCpuCreationTime()},
}
tc.runTest(t)
}
func TestScaleDownIgnoresFailedPods(t *testing.T) { func TestScaleDownIgnoresFailedPods(t *testing.T) {
tc := testCase{ tc := testCase{
minReplicas: 2, minReplicas: 2,
...@@ -1975,7 +2080,7 @@ func TestAvoidUncessaryUpdates(t *testing.T) { ...@@ -1975,7 +2080,7 @@ func TestAvoidUncessaryUpdates(t *testing.T) {
verifyCPUCurrent: true, verifyCPUCurrent: true,
reportedLevels: []uint64{400, 500, 700}, reportedLevels: []uint64{400, 500, 700},
reportedCPURequests: []resource.Quantity{resource.MustParse("1.0"), resource.MustParse("1.0"), resource.MustParse("1.0")}, reportedCPURequests: []resource.Quantity{resource.MustParse("1.0"), resource.MustParse("1.0"), resource.MustParse("1.0")},
reportedPodReadiness: []v1.ConditionStatus{v1.ConditionTrue, v1.ConditionFalse, v1.ConditionFalse}, reportedPodStartTime: []metav1.Time{coolCpuCreationTime(), hotCpuCreationTime(), hotCpuCreationTime()},
useMetricsAPI: true, useMetricsAPI: true,
} }
testClient, _, _, _, _ := tc.prepareTestClient(t) testClient, _, _, _, _ := tc.prepareTestClient(t)
......
...@@ -222,7 +222,8 @@ func (tc *legacyTestCase) prepareTestClient(t *testing.T) (*fake.Clientset, *sca ...@@ -222,7 +222,8 @@ func (tc *legacyTestCase) prepareTestClient(t *testing.T) (*fake.Clientset, *sca
podName := fmt.Sprintf("%s-%d", podNamePrefix, i) podName := fmt.Sprintf("%s-%d", podNamePrefix, i)
pod := v1.Pod{ pod := v1.Pod{
Status: v1.PodStatus{ Status: v1.PodStatus{
Phase: v1.PodRunning, StartTime: &metav1.Time{Time: time.Now().Add(-3 * time.Minute)},
Phase: v1.PodRunning,
Conditions: []v1.PodCondition{ Conditions: []v1.PodCondition{
{ {
Type: v1.PodReady, Type: v1.PodReady,
...@@ -484,11 +485,7 @@ func (tc *legacyTestCase) runTest(t *testing.T) { ...@@ -484,11 +485,7 @@ func (tc *legacyTestCase) runTest(t *testing.T) {
return true, obj, nil return true, obj, nil
}) })
replicaCalc := &ReplicaCalculator{ replicaCalc := NewReplicaCalculator(metricsClient, testClient.Core(), defaultTestingTolerance, defaultTestingCpuTaintAfterStart, defaultTestingDelayOfInitialReadinessStatus)
metricsClient: metricsClient,
podsGetter: testClient.Core(),
tolerance: defaultTestingTolerance,
}
informerFactory := informers.NewSharedInformerFactory(testClient, controller.NoResyncPeriodFunc()) informerFactory := informers.NewSharedInformerFactory(testClient, controller.NoResyncPeriodFunc())
defaultDownscaleForbiddenWindow := 5 * time.Minute defaultDownscaleForbiddenWindow := 5 * time.Minute
...@@ -545,8 +542,7 @@ func TestLegacyScaleUpUnreadyLessScale(t *testing.T) { ...@@ -545,8 +542,7 @@ func TestLegacyScaleUpUnreadyLessScale(t *testing.T) {
initialReplicas: 3, initialReplicas: 3,
desiredReplicas: 4, desiredReplicas: 4,
CPUTarget: 30, CPUTarget: 30,
CPUCurrent: 60, verifyCPUCurrent: false,
verifyCPUCurrent: true,
reportedLevels: []uint64{300, 500, 700}, reportedLevels: []uint64{300, 500, 700},
reportedCPURequests: []resource.Quantity{resource.MustParse("1.0"), resource.MustParse("1.0"), resource.MustParse("1.0")}, reportedCPURequests: []resource.Quantity{resource.MustParse("1.0"), resource.MustParse("1.0"), resource.MustParse("1.0")},
reportedPodReadiness: []v1.ConditionStatus{v1.ConditionFalse, v1.ConditionTrue, v1.ConditionTrue}, reportedPodReadiness: []v1.ConditionStatus{v1.ConditionFalse, v1.ConditionTrue, v1.ConditionTrue},
...@@ -634,12 +630,12 @@ func TestLegacyScaleUpCM(t *testing.T) { ...@@ -634,12 +630,12 @@ func TestLegacyScaleUpCM(t *testing.T) {
tc.runTest(t) tc.runTest(t)
} }
func TestLegacyScaleUpCMUnreadyLessScale(t *testing.T) { func TestLegacyScaleUpCMUnreadyNoLessScale(t *testing.T) {
tc := legacyTestCase{ tc := legacyTestCase{
minReplicas: 2, minReplicas: 2,
maxReplicas: 6, maxReplicas: 6,
initialReplicas: 3, initialReplicas: 3,
desiredReplicas: 4, desiredReplicas: 6,
CPUTarget: 0, CPUTarget: 0,
metricsTarget: []autoscalingv2.MetricSpec{ metricsTarget: []autoscalingv2.MetricSpec{
{ {
...@@ -662,7 +658,7 @@ func TestLegacyScaleUpCMUnreadyNoScaleWouldScaleDown(t *testing.T) { ...@@ -662,7 +658,7 @@ func TestLegacyScaleUpCMUnreadyNoScaleWouldScaleDown(t *testing.T) {
minReplicas: 2, minReplicas: 2,
maxReplicas: 6, maxReplicas: 6,
initialReplicas: 3, initialReplicas: 3,
desiredReplicas: 3, desiredReplicas: 6,
CPUTarget: 0, CPUTarget: 0,
metricsTarget: []autoscalingv2.MetricSpec{ metricsTarget: []autoscalingv2.MetricSpec{
{ {
......
...@@ -67,7 +67,8 @@ func (tc *legacyReplicaCalcTestCase) prepareTestClient(t *testing.T) *fake.Clien ...@@ -67,7 +67,8 @@ func (tc *legacyReplicaCalcTestCase) prepareTestClient(t *testing.T) *fake.Clien
podName := fmt.Sprintf("%s-%d", podNamePrefix, i) podName := fmt.Sprintf("%s-%d", podNamePrefix, i)
pod := v1.Pod{ pod := v1.Pod{
Status: v1.PodStatus{ Status: v1.PodStatus{
Phase: v1.PodRunning, Phase: v1.PodRunning,
StartTime: &metav1.Time{Time: time.Now().Add(-3 * time.Minute)},
Conditions: []v1.PodCondition{ Conditions: []v1.PodCondition{
{ {
Type: v1.PodReady, Type: v1.PodReady,
...@@ -185,11 +186,7 @@ func (tc *legacyReplicaCalcTestCase) runTest(t *testing.T) { ...@@ -185,11 +186,7 @@ func (tc *legacyReplicaCalcTestCase) runTest(t *testing.T) {
testClient := tc.prepareTestClient(t) testClient := tc.prepareTestClient(t)
metricsClient := metrics.NewHeapsterMetricsClient(testClient, metrics.DefaultHeapsterNamespace, metrics.DefaultHeapsterScheme, metrics.DefaultHeapsterService, metrics.DefaultHeapsterPort) metricsClient := metrics.NewHeapsterMetricsClient(testClient, metrics.DefaultHeapsterNamespace, metrics.DefaultHeapsterScheme, metrics.DefaultHeapsterService, metrics.DefaultHeapsterPort)
replicaCalc := &ReplicaCalculator{ replicaCalc := NewReplicaCalculator(metricsClient, testClient.Core(), defaultTestingTolerance, defaultTestingCpuTaintAfterStart, defaultTestingDelayOfInitialReadinessStatus)
metricsClient: metricsClient,
podsGetter: testClient.Core(),
tolerance: defaultTestingTolerance,
}
selector, err := metav1.LabelSelectorAsSelector(&metav1.LabelSelector{ selector, err := metav1.LabelSelectorAsSelector(&metav1.LabelSelector{
MatchLabels: map[string]string{"name": podNamePrefix}, MatchLabels: map[string]string{"name": podNamePrefix},
...@@ -310,10 +307,10 @@ func TestLegacyReplicaCalcScaleUpCM(t *testing.T) { ...@@ -310,10 +307,10 @@ func TestLegacyReplicaCalcScaleUpCM(t *testing.T) {
tc.runTest(t) tc.runTest(t)
} }
func TestLegacyReplicaCalcScaleUpCMUnreadyLessScale(t *testing.T) { func TestLegacyReplicaCalcScaleUpCMUnreadyNoLessScale(t *testing.T) {
tc := legacyReplicaCalcTestCase{ tc := legacyReplicaCalcTestCase{
currentReplicas: 3, currentReplicas: 3,
expectedReplicas: 4, expectedReplicas: 6,
podReadiness: []v1.ConditionStatus{v1.ConditionTrue, v1.ConditionTrue, v1.ConditionFalse}, podReadiness: []v1.ConditionStatus{v1.ConditionTrue, v1.ConditionTrue, v1.ConditionFalse},
metric: &metricInfo{ metric: &metricInfo{
name: "qps", name: "qps",
...@@ -325,16 +322,16 @@ func TestLegacyReplicaCalcScaleUpCMUnreadyLessScale(t *testing.T) { ...@@ -325,16 +322,16 @@ func TestLegacyReplicaCalcScaleUpCMUnreadyLessScale(t *testing.T) {
tc.runTest(t) tc.runTest(t)
} }
func TestLegacyReplicaCalcScaleUpCMUnreadyNoScaleWouldScaleDown(t *testing.T) { func TestLegacyReplicaCalcScaleUpCMUnreadyScale(t *testing.T) {
tc := legacyReplicaCalcTestCase{ tc := legacyReplicaCalcTestCase{
currentReplicas: 3, currentReplicas: 3,
expectedReplicas: 3, expectedReplicas: 7,
podReadiness: []v1.ConditionStatus{v1.ConditionFalse, v1.ConditionTrue, v1.ConditionFalse}, podReadiness: []v1.ConditionStatus{v1.ConditionFalse, v1.ConditionTrue, v1.ConditionFalse},
metric: &metricInfo{ metric: &metricInfo{
name: "qps", name: "qps",
levels: []int64{50000, 15000, 30000}, levels: []int64{50000, 15000, 30000},
targetUtilization: 15000, targetUtilization: 15000,
expectedUtilization: 15000, expectedUtilization: 31666,
}, },
} }
tc.runTest(t) tc.runTest(t)
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment