Skip to content

Commit

Permalink
fix: don't validate workload with only resource limits set
Browse files Browse the repository at this point in the history
Signed-off-by: Claudio Netto <[email protected]>
  • Loading branch information
nettoclaudio committed Jul 16, 2023
1 parent a1ae203 commit 203cbb7
Show file tree
Hide file tree
Showing 2 changed files with 67 additions and 6 deletions.
11 changes: 5 additions & 6 deletions apis/keda/v1alpha1/scaledobject_webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -255,19 +255,18 @@ func verifyCPUMemoryScalers(incomingSo *ScaledObject, action string) error {
if conainerName != "" && container.Name != conainerName {
continue
}

if trigger.Type == cpuString {
if container.Resources.Requests == nil ||
container.Resources.Requests.Cpu() == nil ||
container.Resources.Requests.Cpu().AsApproximateFloat64() == 0 {
if container.Resources.Requests.Cpu().AsApproximateFloat64() == 0 &&
container.Resources.Limits.Cpu().AsApproximateFloat64() == 0 {
err := fmt.Errorf("the scaledobject has a cpu trigger but the container %s doesn't have the cpu request defined", container.Name)
scaledobjectlog.Error(err, "validation error")
prommetrics.RecordScaledObjectValidatingErrors(incomingSo.Namespace, action, "missing-requests")
return err
}
} else if trigger.Type == memoryString {
if container.Resources.Requests == nil ||
container.Resources.Requests.Memory() == nil ||
container.Resources.Requests.Memory().AsApproximateFloat64() == 0 {
if container.Resources.Requests.Memory().AsApproximateFloat64() == 0 &&
container.Resources.Limits.Memory().AsApproximateFloat64() == 0 {
err := fmt.Errorf("the scaledobject has a memory trigger but the container %s doesn't have the memory request defined", container.Name)
scaledobjectlog.Error(err, "validation error")
prommetrics.RecordScaledObjectValidatingErrors(incomingSo.Namespace, action, "missing-requests")
Expand Down
62 changes: 62 additions & 0 deletions apis/keda/v1alpha1/scaledobject_webhook_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -486,6 +486,68 @@ var _ = It("should validate so creation when min replicas is > 0 with only cpu s

})

var _ = It("should not validate ScaledObject creation when deployment only provides cpu resource limits", func() {

namespaceName := "only-cpu-resource-limits-set"
namespace := createNamespace(namespaceName)

err := k8sClient.Create(context.Background(), namespace)
Expect(err).ToNot(HaveOccurred())

workload := createDeployment(namespaceName, false, false)
workload.Spec.Template.Spec.Containers[0].Resources.Limits = v1.ResourceList{
v1.ResourceCPU: *resource.NewMilliQuantity(100, resource.DecimalSI),
}

err = k8sClient.Create(context.Background(), workload)
Expect(err).ToNot(HaveOccurred())

so := createScaledObject(soName, namespaceName, workloadName, "apps/v1", "Deployment", false, map[string]string{}, "")
so.Spec.Triggers = []ScaleTriggers{
{
Type: "cpu",
Metadata: map[string]string{
"cpu": "10",
},
},
}

Eventually(func() error {
return k8sClient.Create(context.Background(), so)
}).ShouldNot(HaveOccurred())
})

var _ = It("should not validate ScaledObject creation when deployment only provides memory resource limits", func() {

namespaceName := "only-memory-resource-limits-set"
namespace := createNamespace(namespaceName)

err := k8sClient.Create(context.Background(), namespace)
Expect(err).ToNot(HaveOccurred())

workload := createDeployment(namespaceName, false, false)
workload.Spec.Template.Spec.Containers[0].Resources.Limits = v1.ResourceList{
v1.ResourceMemory: *resource.NewMilliQuantity(1024, resource.DecimalSI),
}

err = k8sClient.Create(context.Background(), workload)
Expect(err).ToNot(HaveOccurred())

so := createScaledObject(soName, namespaceName, workloadName, "apps/v1", "Deployment", false, map[string]string{}, "")
so.Spec.Triggers = []ScaleTriggers{
{
Type: "memory",
Metadata: map[string]string{
"memory": "512Mi",
},
},
}

Eventually(func() error {
return k8sClient.Create(context.Background(), so)
}).ShouldNot(HaveOccurred())
})

var _ = It("should validate the so update if it's removing the finalizer even if it's invalid", func() {

namespaceName := "removing-finalizers"
Expand Down

0 comments on commit 203cbb7

Please sign in to comment.