Skip to content

Commit efbc308

Browse files
y-rabiemittalvaibhav1
y-rabie
authored and
mittalvaibhav1
committed
fix!: fix CheckFallbackValid logic (kedacore#6538)
Signed-off-by: Youssef Rabie <[email protected]>
1 parent c7604f9 commit efbc308

File tree

3 files changed

+198
-11
lines changed

3 files changed

+198
-11
lines changed

‎CHANGELOG.md

+2-1
Original file line numberDiff line numberDiff line change
@@ -97,8 +97,10 @@ Here is an overview of all new **experimental** features:
9797
- **General**: Fix CVE-2025-29786 ([#6637](https://github.com/kedacore/keda/issues/6637))
9898
- **General**: Fix CVE-2025-30204 ([#6641](https://github.com/kedacore/keda/pull/6641))
9999
- **General**: Fix event text when deactivation fails ([#6469](https://github.com/kedacore/keda/issues/6469))
100+
- **General**: Fix fallback validation check bug ([#6407](https://github.com/kedacore/keda/pull/6407))
100101
- **General**: Make sure the exposed metrics (from KEDA operator) are updated when there is a change to triggers ([#6618](https://github.com/kedacore/keda/pull/6618))
101102
- **General**: Paused ScaledObject count is reported correctly after operator restart ([#6321](https://github.com/kedacore/keda/issues/6321))
103+
- **General**: Reiterate fix (after [#6407](https://github.com/kedacore/keda/pull/6407)) for fallback validation in admission webhook. ([#6538](https://github.com/kedacore/keda/pull/6538))
102104
- **General**: ScaledJobs ready status set to true when recoverred problem ([#6329](https://github.com/kedacore/keda/pull/6329))
103105
- **AWS Scalers**: Add AWS region to the AWS Config Cache key ([#6128](https://github.com/kedacore/keda/issues/6128))
104106
- **External Scaler**: Support server TLS without custom CA ([#6606](https://github.com/kedacore/keda/pull/6606))
@@ -125,7 +127,6 @@ New deprecation(s):
125127
### Other
126128

127129
- **General**: Add debug logs tracking validation of ScaledObjects on webhook ([#6498](https://github.com/kedacore/keda/pull/6498))
128-
- **General**: Fix fallback validation check bug ([#6407](https://github.com/kedacore/keda/pull/6407))
129130
- **General**: New eventreason KEDAScalersInfo to display important information ([#6328](https://github.com/kedacore/keda/issues/6328))
130131
- **Apache Kafka Scaler**: Remove unused awsEndpoint in Apache Kafka scaler ([#6627](https://github.com/kedacore/keda/pull/6627))
131132
- **External Scalers**: Allow `float64` values in externalmetrics' `MetricValue` & `TargetSize`. The old fields are still there because of backward compatibility. ([#5159](https://github.com/kedacore/keda/issues/5159))

‎apis/keda/v1alpha1/scaledobject_types.go

+18-8
Original file line numberDiff line numberDiff line change
@@ -23,11 +23,8 @@ import (
2323

2424
autoscalingv2 "k8s.io/api/autoscaling/v2"
2525
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
26-
logf "sigs.k8s.io/controller-runtime/pkg/log"
2726
)
2827

29-
var scaledobjecttypeslog = logf.Log.WithName("scaledobject-types")
30-
3128
// +genclient
3229
// +kubebuilder:object:root=true
3330
// +kubebuilder:subresource:status
@@ -297,12 +294,25 @@ func CheckFallbackValid(scaledObject *ScaledObject) error {
297294
scaledObject.Spec.Fallback.FailureThreshold, scaledObject.Spec.Fallback.Replicas)
298295
}
299296

300-
for _, trigger := range scaledObject.Spec.Triggers {
301-
if trigger.Type == cpuString || trigger.Type == memoryString {
302-
scaledobjecttypeslog.Error(nil, fmt.Sprintf("type is %s , but fallback it is not supported by the CPU & memory scalers", trigger.Type))
297+
if scaledObject.IsUsingModifiers() {
298+
if scaledObject.Spec.Advanced.ScalingModifiers.MetricType != autoscalingv2.AverageValueMetricType {
299+
return fmt.Errorf("when using ScalingModifiers, ScaledObject.Spec.Advanced.ScalingModifiers.MetricType must be AverageValue to have fallback enabled")
300+
}
301+
} else {
302+
fallbackValid := false
303+
for _, trigger := range scaledObject.Spec.Triggers {
304+
if trigger.Type == cpuString || trigger.Type == memoryString {
305+
continue
306+
}
307+
// If at least one trigger is of the type `AverageValue`, then having fallback is valid.
308+
if trigger.MetricType == autoscalingv2.AverageValueMetricType {
309+
fallbackValid = true
310+
break
311+
}
303312
}
304-
if trigger.MetricType != autoscalingv2.AverageValueMetricType {
305-
return fmt.Errorf("MetricType=%s, but fallback can only be enabled for triggers with metric of type AverageValue", trigger.MetricType)
313+
314+
if !fallbackValid {
315+
return fmt.Errorf("at least one trigger (that is not cpu or memory) has to have the `AverageValue` type for the fallback to be enabled")
306316
}
307317
}
308318
return nil

‎apis/keda/v1alpha1/scaledobject_webhook_test.go

+178-2
Original file line numberDiff line numberDiff line change
@@ -176,8 +176,8 @@ var _ = It("shouldn't validate the so creation when the fallback is wrong", func
176176
}).Should(HaveOccurred())
177177
})
178178

179-
var _ = It("should validate the so creation When the fallback are configured and the scaler is either CPU or memory.", func() {
180-
namespaceName := "right-fallback-cpu-memory"
179+
var _ = It("shouldn't validate the so creation when the fallback is configured and only cpu/memory triggers are used.", func() {
180+
namespaceName := "wrong-fallback-cpu-memory"
181181
namespace := createNamespace(namespaceName)
182182
workload := createDeployment(namespaceName, true, true)
183183
so := createScaledObject(soName, namespaceName, workloadName, "apps/v1", "Deployment", true, map[string]string{}, "")
@@ -194,6 +194,182 @@ var _ = It("should validate the so creation When the fallback are configured and
194194
err = k8sClient.Create(context.Background(), workload)
195195
Expect(err).ToNot(HaveOccurred())
196196

197+
Eventually(func() error {
198+
return k8sClient.Create(context.Background(), so)
199+
}).Should(HaveOccurred())
200+
})
201+
202+
var _ = It("should validate the so creation when the fallback is configured, and at least one trigger (besides cpu/memory) has metricType == AverageValue.", func() {
203+
namespaceName := "right-fallback-at-least-one-averagevalue"
204+
namespace := createNamespace(namespaceName)
205+
workload := createDeployment(namespaceName, true, true)
206+
// Create ScaledObject with cpu and memory triggers.
207+
so := createScaledObject(soName, namespaceName, workloadName, "apps/v1", "Deployment", true, map[string]string{}, "")
208+
209+
triggers := []ScaleTriggers{
210+
{
211+
Type: "kubernetes-workload",
212+
Name: "workload_trig_1",
213+
Metadata: map[string]string{
214+
"podSelector": "pod=workload-test",
215+
"value": "1",
216+
},
217+
MetricType: v2.ValueMetricType,
218+
},
219+
{
220+
Type: "kubernetes-workload",
221+
Name: "workload_trig_2",
222+
Metadata: map[string]string{
223+
"podSelector": "pod=workload-test",
224+
"value": "1",
225+
},
226+
MetricType: v2.AverageValueMetricType,
227+
},
228+
}
229+
// Append other triggers to the SO, one of them with metricType=AverageValue.
230+
so.Spec.Triggers = append(so.Spec.Triggers, triggers...)
231+
so.Spec.Fallback = &Fallback{
232+
FailureThreshold: 3,
233+
Replicas: 6,
234+
}
235+
236+
err := k8sClient.Create(context.Background(), namespace)
237+
Expect(err).ToNot(HaveOccurred())
238+
239+
err = k8sClient.Create(context.Background(), workload)
240+
Expect(err).ToNot(HaveOccurred())
241+
242+
Eventually(func() error {
243+
return k8sClient.Create(context.Background(), so)
244+
}).ShouldNot(HaveOccurred())
245+
})
246+
247+
var _ = It("shouldn't validate the so creation when the fallback is configured, and NO trigger (besides cpu/memory) has metricType == AverageValue.", func() {
248+
namespaceName := "wrong-fallback-none-averagevalue"
249+
namespace := createNamespace(namespaceName)
250+
workload := createDeployment(namespaceName, true, true)
251+
// Create ScaledObject with cpu and memory triggers.
252+
so := createScaledObject(soName, namespaceName, workloadName, "apps/v1", "Deployment", true, map[string]string{}, "")
253+
254+
triggers := []ScaleTriggers{
255+
{
256+
Type: "kubernetes-workload",
257+
Name: "workload_trig_1",
258+
Metadata: map[string]string{
259+
"podSelector": "pod=workload-test",
260+
"value": "1",
261+
},
262+
MetricType: v2.ValueMetricType,
263+
},
264+
{
265+
Type: "kubernetes-workload",
266+
Name: "workload_trig_2",
267+
Metadata: map[string]string{
268+
"podSelector": "pod=workload-test",
269+
"value": "1",
270+
},
271+
MetricType: v2.ValueMetricType,
272+
},
273+
}
274+
// Append other triggers to the SO, none of them with metricType=AverageValue.
275+
so.Spec.Triggers = append(so.Spec.Triggers, triggers...)
276+
so.Spec.Fallback = &Fallback{
277+
FailureThreshold: 3,
278+
Replicas: 6,
279+
}
280+
281+
err := k8sClient.Create(context.Background(), namespace)
282+
Expect(err).ToNot(HaveOccurred())
283+
284+
err = k8sClient.Create(context.Background(), workload)
285+
Expect(err).ToNot(HaveOccurred())
286+
287+
Eventually(func() error {
288+
return k8sClient.Create(context.Background(), so)
289+
}).Should(HaveOccurred())
290+
})
291+
292+
var _ = It("shouldn't validate the so creation when the fallback is configured, and the so uses ScalingModifiers with its metricType != AverageValue.", func() {
293+
namespaceName := "wrong-fallback-scalingmodifier"
294+
namespace := createNamespace(namespaceName)
295+
workload := createDeployment(namespaceName, true, true)
296+
297+
triggers := []ScaleTriggers{
298+
{
299+
Type: "cron",
300+
Name: "cron_trig",
301+
Metadata: map[string]string{
302+
"timezone": "UTC",
303+
"start": "0 * * * *",
304+
"end": "1 * * * *",
305+
"desiredReplicas": "1",
306+
},
307+
},
308+
{
309+
Type: "kubernetes-workload",
310+
Name: "workload_trig",
311+
Metadata: map[string]string{
312+
"podSelector": "pod=workload-test",
313+
"value": "1",
314+
},
315+
},
316+
}
317+
sm := ScalingModifiers{Target: "2", Formula: "workload_trig + cron_trig", MetricType: v2.ValueMetricType}
318+
so := createScaledObjectScalingModifiers(namespaceName, sm, triggers)
319+
so.Spec.Fallback = &Fallback{
320+
FailureThreshold: 3,
321+
Replicas: 6,
322+
}
323+
324+
err := k8sClient.Create(context.Background(), namespace)
325+
Expect(err).ToNot(HaveOccurred())
326+
327+
err = k8sClient.Create(context.Background(), workload)
328+
Expect(err).ToNot(HaveOccurred())
329+
330+
Eventually(func() error {
331+
return k8sClient.Create(context.Background(), so)
332+
}).Should(HaveOccurred())
333+
})
334+
335+
var _ = It("should validate the so creation when the fallback is configured, and the so uses ScalingModifiers with its metricType == AverageValue.", func() {
336+
namespaceName := "right-fallback-scalingmodifier"
337+
namespace := createNamespace(namespaceName)
338+
workload := createDeployment(namespaceName, true, true)
339+
340+
triggers := []ScaleTriggers{
341+
{
342+
Type: "cron",
343+
Name: "cron_trig",
344+
Metadata: map[string]string{
345+
"timezone": "UTC",
346+
"start": "0 * * * *",
347+
"end": "1 * * * *",
348+
"desiredReplicas": "1",
349+
},
350+
},
351+
{
352+
Type: "kubernetes-workload",
353+
Name: "workload_trig",
354+
Metadata: map[string]string{
355+
"podSelector": "pod=workload-test",
356+
"value": "1",
357+
},
358+
},
359+
}
360+
sm := ScalingModifiers{Target: "2", Formula: "workload_trig + cron_trig", MetricType: v2.AverageValueMetricType}
361+
so := createScaledObjectScalingModifiers(namespaceName, sm, triggers)
362+
so.Spec.Fallback = &Fallback{
363+
FailureThreshold: 3,
364+
Replicas: 6,
365+
}
366+
367+
err := k8sClient.Create(context.Background(), namespace)
368+
Expect(err).ToNot(HaveOccurred())
369+
370+
err = k8sClient.Create(context.Background(), workload)
371+
Expect(err).ToNot(HaveOccurred())
372+
197373
Eventually(func() error {
198374
return k8sClient.Create(context.Background(), so)
199375
}).ShouldNot(HaveOccurred())

0 commit comments

Comments
 (0)