From a3ca76d1b2b888683b5ed4f031bebe6d6985c74a Mon Sep 17 00:00:00 2001 From: Shane Date: Tue, 19 Nov 2024 17:32:50 +0800 Subject: [PATCH 01/24] Check scaleTarget if exists in webhook Signed-off-by: Shane --- apis/keda/v1alpha1/scaledobject_types.go | 23 +++++++++++++++++++++ apis/keda/v1alpha1/scaledobject_webhook.go | 9 +++++--- controllers/keda/scaledobject_controller.go | 7 +------ 3 files changed, 30 insertions(+), 9 deletions(-) diff --git a/apis/keda/v1alpha1/scaledobject_types.go b/apis/keda/v1alpha1/scaledobject_types.go index 37c5e640963..ab31fe17fed 100644 --- a/apis/keda/v1alpha1/scaledobject_types.go +++ b/apis/keda/v1alpha1/scaledobject_types.go @@ -17,8 +17,12 @@ limitations under the License. package v1alpha1 import ( + "context" "fmt" + "github.com/kedacore/keda/v2/pkg/common/message" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "reflect" + "sigs.k8s.io/controller-runtime/pkg/client" "strconv" autoscalingv2 "k8s.io/api/autoscaling/v2" @@ -254,6 +258,25 @@ func (so *ScaledObject) GetHPAMaxReplicas() int32 { return defaultHPAMaxReplicas } +// CheckScaleTargetRefIfExist checks if scaleTargetRef of ScaledObject exists +func (so *ScaledObject) CheckScaleTargetRefIfExist() error { + soGvkr, err := ParseGVKR(restMapper, so.Spec.ScaleTargetRef.APIVersion, so.Spec.ScaleTargetRef.Kind) + if err != nil { + msg := "Failed to parse Group, Version, Kind, Resource" + scaledobjectlog.Error(err, msg, "apiVersion", so.Spec.ScaleTargetRef.APIVersion, "kind", so.Spec.ScaleTargetRef.Kind) + return err + } + gvkString := soGvkr.GVKString() + unstruct := &unstructured.Unstructured{} + unstruct.SetGroupVersionKind(soGvkr.GroupVersionKind()) + if err := kc.Get(context.Background(), client.ObjectKey{Namespace: so.Namespace, Name: so.Spec.ScaleTargetRef.Name}, unstruct); err != nil { + // resource doesn't exist + scaledobjectlog.Error(err, message.ScaleTargetNotFoundMsg, "resource", gvkString, "name", so.Spec.ScaleTargetRef.Name) + return err + } + return nil +} + // checkReplicaCountBoundsAreValid checks that Idle/Min/Max ReplicaCount defined in ScaledObject are correctly specified // i.e. that Min is not greater than Max or Idle greater or equal to Min func CheckReplicaCountBoundsAreValid(scaledObject *ScaledObject) error { diff --git a/apis/keda/v1alpha1/scaledobject_webhook.go b/apis/keda/v1alpha1/scaledobject_webhook.go index 41bd0355596..c4e0a29699e 100644 --- a/apis/keda/v1alpha1/scaledobject_webhook.go +++ b/apis/keda/v1alpha1/scaledobject_webhook.go @@ -288,8 +288,11 @@ func verifyScaledObjects(incomingSo *ScaledObject, action string, _ bool) error if err != nil { return err } + if err := incomingSo.CheckScaleTargetRefIfExist(); err != nil { + return err + } - incomingSoGckr, err := ParseGVKR(restMapper, incomingSo.Spec.ScaleTargetRef.APIVersion, incomingSo.Spec.ScaleTargetRef.Kind) + incomingSoGvkr, err := ParseGVKR(restMapper, incomingSo.Spec.ScaleTargetRef.APIVersion, incomingSo.Spec.ScaleTargetRef.Kind) if err != nil { scaledobjectlog.Error(err, "Failed to parse Group, Version, Kind, Resource from incoming ScaledObject", "apiVersion", incomingSo.Spec.ScaleTargetRef.APIVersion, "kind", incomingSo.Spec.ScaleTargetRef.Kind) return err @@ -303,13 +306,13 @@ func verifyScaledObjects(incomingSo *ScaledObject, action string, _ bool) error val, _ := json.MarshalIndent(so, "", " ") scaledobjectlog.V(1).Info(fmt.Sprintf("checking scaledobject %s: %v", so.Name, string(val))) - soGckr, err := ParseGVKR(restMapper, so.Spec.ScaleTargetRef.APIVersion, so.Spec.ScaleTargetRef.Kind) + soGvkr, err := ParseGVKR(restMapper, so.Spec.ScaleTargetRef.APIVersion, so.Spec.ScaleTargetRef.Kind) if err != nil { scaledobjectlog.Error(err, "Failed to parse Group, Version, Kind, Resource from ScaledObject", "soName", so.Name, "apiVersion", so.Spec.ScaleTargetRef.APIVersion, "kind", so.Spec.ScaleTargetRef.Kind) return err } - if soGckr.GVKString() == incomingSoGckr.GVKString() && + if soGvkr.GVKString() == incomingSoGvkr.GVKString() && so.Spec.ScaleTargetRef.Name == incomingSo.Spec.ScaleTargetRef.Name { err = fmt.Errorf("the workload '%s' of type '%s' is already managed by the ScaledObject '%s'", so.Spec.ScaleTargetRef.Name, incomingSoGckr.GVKString(), so.Name) scaledobjectlog.Error(err, "validation error") diff --git a/controllers/keda/scaledobject_controller.go b/controllers/keda/scaledobject_controller.go index c480dc380c4..57beeee59e0 100755 --- a/controllers/keda/scaledobject_controller.go +++ b/controllers/keda/scaledobject_controller.go @@ -29,7 +29,6 @@ import ( "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" "k8s.io/client-go/scale" @@ -399,11 +398,7 @@ func (r *ScaledObjectReconciler) checkTargetResourceIsScalable(ctx context.Conte scale, errScale = (r.ScaleClient).Scales(scaledObject.Namespace).Get(ctx, gr, scaledObject.Spec.ScaleTargetRef.Name, metav1.GetOptions{}) if errScale != nil { // not able to get /scale subresource -> let's check if the resource even exist in the cluster - unstruct := &unstructured.Unstructured{} - unstruct.SetGroupVersionKind(gvkr.GroupVersionKind()) - if err := r.Client.Get(ctx, client.ObjectKey{Namespace: scaledObject.Namespace, Name: scaledObject.Spec.ScaleTargetRef.Name}, unstruct); err != nil { - // resource doesn't exist - logger.Error(err, message.ScaleTargetNotFoundMsg, "resource", gvkString, "name", scaledObject.Spec.ScaleTargetRef.Name) + if err := scaledObject.CheckScaleTargetRefIfExist(); err != nil { r.EventEmitter.Emit(scaledObject, scaledObject.Namespace, corev1.EventTypeWarning, eventingv1alpha1.ScaledObjectFailedType, eventreason.ScaledObjectCheckFailed, message.ScaleTargetNotFoundMsg) return gvkr, err } From 7837c594bd34ca0e1bdc644780f5c9ad0fb7b304 Mon Sep 17 00:00:00 2001 From: Shane Date: Tue, 19 Nov 2024 18:15:37 +0800 Subject: [PATCH 02/24] fix some info Signed-off-by: Shane --- apis/keda/v1alpha1/scaledobject_webhook.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apis/keda/v1alpha1/scaledobject_webhook.go b/apis/keda/v1alpha1/scaledobject_webhook.go index c4e0a29699e..a34aa124c54 100644 --- a/apis/keda/v1alpha1/scaledobject_webhook.go +++ b/apis/keda/v1alpha1/scaledobject_webhook.go @@ -314,7 +314,7 @@ func verifyScaledObjects(incomingSo *ScaledObject, action string, _ bool) error if soGvkr.GVKString() == incomingSoGvkr.GVKString() && so.Spec.ScaleTargetRef.Name == incomingSo.Spec.ScaleTargetRef.Name { - err = fmt.Errorf("the workload '%s' of type '%s' is already managed by the ScaledObject '%s'", so.Spec.ScaleTargetRef.Name, incomingSoGckr.GVKString(), so.Name) + err = fmt.Errorf("the workload '%s' of type '%s' is already managed by the ScaledObject '%s'", so.Spec.ScaleTargetRef.Name, incomingSoGvkr.GVKString(), so.Name) scaledobjectlog.Error(err, "validation error") metricscollector.RecordScaledObjectValidatingErrors(incomingSo.Namespace, action, "other-scaled-object") return err From d58a0aa33e5736825282f02e1b170399e2718e15 Mon Sep 17 00:00:00 2001 From: Shane Date: Wed, 20 Nov 2024 09:14:19 +0800 Subject: [PATCH 03/24] change function parameter Signed-off-by: Shane --- apis/keda/v1alpha1/scaledobject_types.go | 4 ++-- apis/keda/v1alpha1/scaledobject_webhook.go | 4 ++-- controllers/keda/scaledobject_controller.go | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/apis/keda/v1alpha1/scaledobject_types.go b/apis/keda/v1alpha1/scaledobject_types.go index ab31fe17fed..30fdb320aeb 100644 --- a/apis/keda/v1alpha1/scaledobject_types.go +++ b/apis/keda/v1alpha1/scaledobject_types.go @@ -259,7 +259,7 @@ func (so *ScaledObject) GetHPAMaxReplicas() int32 { } // CheckScaleTargetRefIfExist checks if scaleTargetRef of ScaledObject exists -func (so *ScaledObject) CheckScaleTargetRefIfExist() error { +func (so *ScaledObject) CheckScaleTargetRefIfExist(ctx context.Context) error { soGvkr, err := ParseGVKR(restMapper, so.Spec.ScaleTargetRef.APIVersion, so.Spec.ScaleTargetRef.Kind) if err != nil { msg := "Failed to parse Group, Version, Kind, Resource" @@ -269,7 +269,7 @@ func (so *ScaledObject) CheckScaleTargetRefIfExist() error { gvkString := soGvkr.GVKString() unstruct := &unstructured.Unstructured{} unstruct.SetGroupVersionKind(soGvkr.GroupVersionKind()) - if err := kc.Get(context.Background(), client.ObjectKey{Namespace: so.Namespace, Name: so.Spec.ScaleTargetRef.Name}, unstruct); err != nil { + if err := kc.Get(ctx, client.ObjectKey{Namespace: so.Namespace, Name: so.Spec.ScaleTargetRef.Name}, unstruct); err != nil { // resource doesn't exist scaledobjectlog.Error(err, message.ScaleTargetNotFoundMsg, "resource", gvkString, "name", so.Spec.ScaleTargetRef.Name) return err diff --git a/apis/keda/v1alpha1/scaledobject_webhook.go b/apis/keda/v1alpha1/scaledobject_webhook.go index a34aa124c54..e43b314e70b 100644 --- a/apis/keda/v1alpha1/scaledobject_webhook.go +++ b/apis/keda/v1alpha1/scaledobject_webhook.go @@ -288,9 +288,9 @@ func verifyScaledObjects(incomingSo *ScaledObject, action string, _ bool) error if err != nil { return err } - if err := incomingSo.CheckScaleTargetRefIfExist(); err != nil { + if err := incomingSo.CheckScaleTargetRefIfExist(context.Background()); err != nil { return err - } + }git incomingSoGvkr, err := ParseGVKR(restMapper, incomingSo.Spec.ScaleTargetRef.APIVersion, incomingSo.Spec.ScaleTargetRef.Kind) if err != nil { diff --git a/controllers/keda/scaledobject_controller.go b/controllers/keda/scaledobject_controller.go index 57beeee59e0..0118364dcc5 100755 --- a/controllers/keda/scaledobject_controller.go +++ b/controllers/keda/scaledobject_controller.go @@ -398,7 +398,7 @@ func (r *ScaledObjectReconciler) checkTargetResourceIsScalable(ctx context.Conte scale, errScale = (r.ScaleClient).Scales(scaledObject.Namespace).Get(ctx, gr, scaledObject.Spec.ScaleTargetRef.Name, metav1.GetOptions{}) if errScale != nil { // not able to get /scale subresource -> let's check if the resource even exist in the cluster - if err := scaledObject.CheckScaleTargetRefIfExist(); err != nil { + if err := scaledObject.CheckScaleTargetRefIfExist(ctx); err != nil { r.EventEmitter.Emit(scaledObject, scaledObject.Namespace, corev1.EventTypeWarning, eventingv1alpha1.ScaledObjectFailedType, eventreason.ScaledObjectCheckFailed, message.ScaleTargetNotFoundMsg) return gvkr, err } From 37e4645277021c885eda83a42f675e82cb3f059c Mon Sep 17 00:00:00 2001 From: Shane Date: Wed, 20 Nov 2024 09:15:49 +0800 Subject: [PATCH 04/24] fix a mistke Signed-off-by: Shane --- apis/keda/v1alpha1/scaledobject_webhook.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apis/keda/v1alpha1/scaledobject_webhook.go b/apis/keda/v1alpha1/scaledobject_webhook.go index e43b314e70b..b5fba489ae7 100644 --- a/apis/keda/v1alpha1/scaledobject_webhook.go +++ b/apis/keda/v1alpha1/scaledobject_webhook.go @@ -290,7 +290,7 @@ func verifyScaledObjects(incomingSo *ScaledObject, action string, _ bool) error } if err := incomingSo.CheckScaleTargetRefIfExist(context.Background()); err != nil { return err - }git + } incomingSoGvkr, err := ParseGVKR(restMapper, incomingSo.Spec.ScaleTargetRef.APIVersion, incomingSo.Spec.ScaleTargetRef.Kind) if err != nil { From 6fa2d55034bac7e6ab34269e917a3f8526297b56 Mon Sep 17 00:00:00 2001 From: Shane Date: Wed, 20 Nov 2024 11:59:55 +0800 Subject: [PATCH 05/24] add CHANGELOG and change import order Signed-off-by: Shane --- CHANGELOG.md | 2 +- apis/keda/v1alpha1/scaledobject_types.go | 5 +++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index dbba7b57a05..ccfef70786b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -58,7 +58,7 @@ To learn more about active deprecations, we recommend checking [GitHub Discussio ### New -- TODO ([#XXX](https://github.com/kedacore/keda/issues/XXX)) +-**General**: Add scaleTargetRef exists check in webhook and revise some variable name ([#6350](https://github.com/kedacore/keda/pull/6350)) #### Experimental diff --git a/apis/keda/v1alpha1/scaledobject_types.go b/apis/keda/v1alpha1/scaledobject_types.go index 30fdb320aeb..9d1ae064fdc 100644 --- a/apis/keda/v1alpha1/scaledobject_types.go +++ b/apis/keda/v1alpha1/scaledobject_types.go @@ -19,11 +19,12 @@ package v1alpha1 import ( "context" "fmt" + "reflect" + "strconv" + "github.com/kedacore/keda/v2/pkg/common/message" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" - "reflect" "sigs.k8s.io/controller-runtime/pkg/client" - "strconv" autoscalingv2 "k8s.io/api/autoscaling/v2" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" From c76c19f363dc707cb7f9749e7cb46c17dc748a36 Mon Sep 17 00:00:00 2001 From: Shane Date: Wed, 20 Nov 2024 12:40:39 +0800 Subject: [PATCH 06/24] change import order using gci Signed-off-by: Shane --- apis/keda/v1alpha1/scaledobject_types.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/apis/keda/v1alpha1/scaledobject_types.go b/apis/keda/v1alpha1/scaledobject_types.go index 9d1ae064fdc..6cc6e97add3 100644 --- a/apis/keda/v1alpha1/scaledobject_types.go +++ b/apis/keda/v1alpha1/scaledobject_types.go @@ -22,12 +22,12 @@ import ( "reflect" "strconv" - "github.com/kedacore/keda/v2/pkg/common/message" + autoscalingv2 "k8s.io/api/autoscaling/v2" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "sigs.k8s.io/controller-runtime/pkg/client" - autoscalingv2 "k8s.io/api/autoscaling/v2" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "github.com/kedacore/keda/v2/pkg/common/message" ) // +genclient From e20ff991161424ad5db0fb1b64485cc4dd9b74e3 Mon Sep 17 00:00:00 2001 From: Shane Date: Wed, 20 Nov 2024 22:31:11 +0800 Subject: [PATCH 07/24] add test and revise old test after forbidden the circumstance scaleTargetRef not found Signed-off-by: Shane --- .../v1alpha1/scaledobject_webhook_test.go | 108 +++++++++++++++++- 1 file changed, 104 insertions(+), 4 deletions(-) diff --git a/apis/keda/v1alpha1/scaledobject_webhook_test.go b/apis/keda/v1alpha1/scaledobject_webhook_test.go index e30404d16c4..95743c26668 100644 --- a/apis/keda/v1alpha1/scaledobject_webhook_test.go +++ b/apis/keda/v1alpha1/scaledobject_webhook_test.go @@ -36,11 +36,17 @@ var _ = It("should validate the so creation when there isn't any hpa", func() { namespaceName := "valid" namespace := createNamespace(namespaceName) + + workload := createDeployment(namespaceName, false, false) + so := createScaledObject(soName, namespaceName, workloadName, "apps/v1", "Deployment", false, map[string]string{}, "") err := k8sClient.Create(context.Background(), namespace) Expect(err).ToNot(HaveOccurred()) + err = k8sClient.Create(context.Background(), workload) + Expect(err).ToNot(HaveOccurred()) + Eventually(func() error { return k8sClient.Create(context.Background(), so) }).ShouldNot(HaveOccurred()) @@ -50,12 +56,23 @@ var _ = It("should validate the so creation when there are other SO for other wo namespaceName := "valid-multiple-so" namespace := createNamespace(namespaceName) + + workload := createDeployment(namespaceName, false, false) + otherWorkload := createDeployment(namespaceName, false, false) + otherWorkload.Spec.Template.Name = "other-workload" + so1 := createScaledObject(soName, namespaceName, workloadName, "apps/v1", "Deployment", false, map[string]string{}, "") so2 := createScaledObject("other-so-name", namespaceName, "other-workload", "apps/v1", "Deployment", false, map[string]string{}, "") err := k8sClient.Create(context.Background(), namespace) Expect(err).ToNot(HaveOccurred()) + err = k8sClient.Create(context.Background(), workload) + Expect(err).ToNot(HaveOccurred()) + + err = k8sClient.Create(context.Background(), otherWorkload) + Expect(err).ToNot(HaveOccurred()) + err = k8sClient.Create(context.Background(), so1) Expect(err).ToNot(HaveOccurred()) @@ -68,12 +85,18 @@ var _ = It("should validate the so creation when there are other HPA for other w namespaceName := "valid-other-hpa" namespace := createNamespace(namespaceName) + + workload := createDeployment(namespaceName, false, false) + so := createScaledObject(soName, namespaceName, workloadName, "apps/v1", "Deployment", false, map[string]string{}, "") hpa := createHpa("other-hpa-name", namespaceName, "other-workload", "apps/v1", "Deployment", nil) err := k8sClient.Create(context.Background(), namespace) Expect(err).ToNot(HaveOccurred()) + err = k8sClient.Create(context.Background(), workload) + Expect(err).ToNot(HaveOccurred()) + err = k8sClient.Create(context.Background(), hpa) Expect(err).ToNot(HaveOccurred()) @@ -87,12 +110,18 @@ var _ = It("should validate the so creation when it's own hpa is already generat hpaName := "test-so-hpa" namespaceName := "own-hpa" namespace := createNamespace(namespaceName) + + workload := createDeployment(namespaceName, false, false) + so := createScaledObject(soName, namespaceName, workloadName, "apps/v1", "Deployment", false, map[string]string{}, "") hpa := createHpa(hpaName, namespaceName, workloadName, "apps/v1", "Deployment", so) err := k8sClient.Create(context.Background(), namespace) Expect(err).ToNot(HaveOccurred()) + err = k8sClient.Create(context.Background(), workload) + Expect(err).ToNot(HaveOccurred()) + err = k8sClient.Create(context.Background(), hpa) Expect(err).ToNot(HaveOccurred()) @@ -106,12 +135,18 @@ var _ = It("should validate the so update when it's own hpa is already generated hpaName := "test-so-hpa" namespaceName := "update-so" namespace := createNamespace(namespaceName) + + workload := createDeployment(namespaceName, false, false) + so := createScaledObject(soName, namespaceName, workloadName, "apps/v1", "Deployment", false, map[string]string{}, "") hpa := createHpa(hpaName, namespaceName, workloadName, "apps/v1", "Deployment", so) err := k8sClient.Create(context.Background(), namespace) Expect(err).ToNot(HaveOccurred()) + err = k8sClient.Create(context.Background(), workload) + Expect(err).ToNot(HaveOccurred()) + err = k8sClient.Create(context.Background(), hpa) Expect(err).ToNot(HaveOccurred()) @@ -129,12 +164,18 @@ var _ = It("shouldn't validate the so creation when there is another unmanaged h hpaName := "test-unmanaged-hpa" namespaceName := "unmanaged-hpa" namespace := createNamespace(namespaceName) + + workload := createDeployment(namespaceName, false, false) + hpa := createHpa(hpaName, namespaceName, workloadName, "apps/v1", "Deployment", nil) so := createScaledObject(soName, namespaceName, workloadName, "apps/v1", "Deployment", false, map[string]string{}, "") err := k8sClient.Create(context.Background(), namespace) Expect(err).ToNot(HaveOccurred()) + err = k8sClient.Create(context.Background(), workload) + Expect(err).ToNot(HaveOccurred()) + err = k8sClient.Create(context.Background(), hpa) Expect(err).ToNot(HaveOccurred()) @@ -146,6 +187,9 @@ var _ = It("shouldn't validate the so creation when there is another unmanaged h var _ = It("shouldn't validate the so creation when the replica counts are wrong", func() { namespaceName := "wrong-replica-count" namespace := createNamespace(namespaceName) + + workload := createDeployment(namespaceName, false, false) + so := createScaledObject(soName, namespaceName, workloadName, "apps/v1", "Deployment", false, map[string]string{}, "") so.Spec.MinReplicaCount = ptr.To[int32](10) so.Spec.MaxReplicaCount = ptr.To[int32](5) @@ -153,6 +197,9 @@ var _ = It("shouldn't validate the so creation when the replica counts are wrong err := k8sClient.Create(context.Background(), namespace) Expect(err).ToNot(HaveOccurred()) + err = k8sClient.Create(context.Background(), workload) + Expect(err).ToNot(HaveOccurred()) + Eventually(func() error { return k8sClient.Create(context.Background(), so) }).Should(HaveOccurred()) @@ -162,6 +209,8 @@ var _ = It("shouldn't validate the so creation when the fallback is wrong", func namespaceName := "wrong-fallback" namespace := createNamespace(namespaceName) + workload := createDeployment(namespaceName, false, false) + so := createScaledObject(soName, namespaceName, workloadName, "apps/v1", "Deployment", false, map[string]string{}, "") so.Spec.Fallback = &Fallback{ FailureThreshold: -1, @@ -171,6 +220,9 @@ var _ = It("shouldn't validate the so creation when the fallback is wrong", func err := k8sClient.Create(context.Background(), namespace) Expect(err).ToNot(HaveOccurred()) + err = k8sClient.Create(context.Background(), workload) + Expect(err).ToNot(HaveOccurred()) + Eventually(func() error { return k8sClient.Create(context.Background(), so) }).Should(HaveOccurred()) @@ -201,12 +253,18 @@ var _ = It("shouldn't validate the so creation when there is another unmanaged h hpaName := "test-unmanaged-hpa-ownership" namespaceName := "unmanaged-hpa-ownership" namespace := createNamespace(namespaceName) + + workload := createDeployment(namespaceName, false, false) + hpa := createHpa(hpaName, namespaceName, workloadName, "apps/v1", "Deployment", nil) so := createScaledObject(soName, namespaceName, workloadName, "apps/v1", "Deployment", false, map[string]string{ScaledObjectTransferHpaOwnershipAnnotation: "true"}, hpaName) err := k8sClient.Create(context.Background(), namespace) Expect(err).ToNot(HaveOccurred()) + err = k8sClient.Create(context.Background(), workload) + Expect(err).ToNot(HaveOccurred()) + err = k8sClient.Create(context.Background(), hpa) Expect(err).ToNot(HaveOccurred()) @@ -220,6 +278,9 @@ var _ = It("shouldn't validate the so creation when hpa has shared-ownership una hpaName := "test-hpa-disabled-validation-by-hpa-ownership" namespaceName := "hpa-ownership" namespace := createNamespace(namespaceName) + + workload := createDeployment(namespaceName, false, false) + hpa := createHpa(hpaName, namespaceName, workloadName, "apps/v1", "Deployment", nil) hpa.ObjectMeta.Annotations = map[string]string{ValidationsHpaOwnershipAnnotation: "false"} so := createScaledObject(soName, namespaceName, workloadName, "apps/v1", "Deployment", false, map[string]string{ScaledObjectTransferHpaOwnershipAnnotation: "false"}, hpaName) @@ -227,6 +288,9 @@ var _ = It("shouldn't validate the so creation when hpa has shared-ownership una err := k8sClient.Create(context.Background(), namespace) Expect(err).ToNot(HaveOccurred()) + err = k8sClient.Create(context.Background(), workload) + Expect(err).ToNot(HaveOccurred()) + err = k8sClient.Create(context.Background(), hpa) Expect(err).ToNot(HaveOccurred()) @@ -240,12 +304,18 @@ var _ = It("shouldn't validate the so creation when there is another so", func() so2Name := "test-so2" namespaceName := "managed-hpa" namespace := createNamespace(namespaceName) + + workload := createDeployment(namespaceName, false, false) + so := createScaledObject(soName, namespaceName, workloadName, "apps/v1", "Deployment", false, map[string]string{}, "") so2 := createScaledObject(so2Name, namespaceName, workloadName, "apps/v1", "Deployment", false, map[string]string{}, "") err := k8sClient.Create(context.Background(), namespace) Expect(err).ToNot(HaveOccurred()) + err = k8sClient.Create(context.Background(), workload) + Expect(err).ToNot(HaveOccurred()) + err = k8sClient.Create(context.Background(), so2) Expect(err).ToNot(HaveOccurred()) @@ -291,29 +361,51 @@ var _ = It("should validate the so creation with cpu and memory when deployment }).ShouldNot(HaveOccurred()) }) -var _ = It("shouldn't validate the creation with cpu and memory when deployment is missing", func() { +var _ = It("shouldn't validate the so creation when deployment is missing", func() { namespaceName := "deployment-missing" namespace := createNamespace(namespaceName) + so := createScaledObject(soName, namespaceName, workloadName, "apps/v1", "Deployment", false, map[string]string{}, "") + + err := k8sClient.Create(context.Background(), namespace) + Expect(err).ToNot(HaveOccurred()) + + Eventually(func() error { + return k8sClient.Create(context.Background(), so, client.DryRunAll) + }).Should(HaveOccurred()) +}) + +var _ = It("should validate the so creation with cpu and memory", func() { + + namespaceName := "deployment-not-missing" + namespace := createNamespace(namespaceName) + workload := createDeployment(namespaceName, true, true) so := createScaledObject(soName, namespaceName, workloadName, "apps/v1", "Deployment", true, map[string]string{}, "") err := k8sClient.Create(context.Background(), namespace) Expect(err).ToNot(HaveOccurred()) + err = k8sClient.Create(context.Background(), workload) + Expect(err).ToNot(HaveOccurred()) + Eventually(func() error { return k8sClient.Create(context.Background(), so) - }).Should(HaveOccurred()) + }).ShouldNot(HaveOccurred()) }) -var _ = It("should validate the creation with cpu and memory when deployment is missing and dry-run is true", func() { +var _ = It("should validate the so creation with cpu and memory when dry-run is true", func() { - namespaceName := "deployment-missing-dry-run" + namespaceName := "deployment-dry-run" namespace := createNamespace(namespaceName) + workload := createDeployment(namespaceName, true, true) so := createScaledObject(soName, namespaceName, workloadName, "apps/v1", "Deployment", true, map[string]string{}, "") err := k8sClient.Create(context.Background(), namespace) Expect(err).ToNot(HaveOccurred()) + err = k8sClient.Create(context.Background(), workload) + Expect(err).ToNot(HaveOccurred()) + Eventually(func() error { return k8sClient.Create(context.Background(), so, client.DryRunAll) }).ShouldNot(HaveOccurred()) @@ -468,11 +560,15 @@ var _ = It("should validate the so creation without cpu and memory when custom r namespaceName := "crd-not-resources" namespace := createNamespace(namespaceName) + workload := createStatefulSet(namespaceName, false, true) so := createScaledObject(soName, namespaceName, workloadName, "custom-api", "StatefulSet", true, map[string]string{}, "") err := k8sClient.Create(context.Background(), namespace) Expect(err).ToNot(HaveOccurred()) + err = k8sClient.Create(context.Background(), workload) + Expect(err).ToNot(HaveOccurred()) + Eventually(func() error { return k8sClient.Create(context.Background(), so) }).ShouldNot(HaveOccurred()) @@ -623,6 +719,7 @@ var _ = It("shouldn't create so when stabilizationWindowSeconds exceeds 3600", f namespaceName := "fail-so-creation" namespace := createNamespace(namespaceName) + workload := createDeployment(namespaceName, false, false) so := createScaledObject(soName, namespaceName, workloadName, "apps/v1", "Deployment", false, map[string]string{}, "") so.Spec.Advanced.HorizontalPodAutoscalerConfig = &HorizontalPodAutoscalerConfig{ Behavior: &v2.HorizontalPodAutoscalerBehavior{ @@ -634,6 +731,9 @@ var _ = It("shouldn't create so when stabilizationWindowSeconds exceeds 3600", f err := k8sClient.Create(context.Background(), namespace) Expect(err).ToNot(HaveOccurred()) + err = k8sClient.Create(context.Background(), workload) + Expect(err).ToNot(HaveOccurred()) + Eventually(func() error { return k8sClient.Create(context.Background(), so) }). From 0fb7542249d7c652f9b9e1cfbf60a76929ec9802 Mon Sep 17 00:00:00 2001 From: Shane Date: Wed, 20 Nov 2024 22:57:05 +0800 Subject: [PATCH 08/24] revise one case Signed-off-by: Shane --- apis/keda/v1alpha1/scaledobject_webhook_test.go | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/apis/keda/v1alpha1/scaledobject_webhook_test.go b/apis/keda/v1alpha1/scaledobject_webhook_test.go index 95743c26668..75edace5250 100644 --- a/apis/keda/v1alpha1/scaledobject_webhook_test.go +++ b/apis/keda/v1alpha1/scaledobject_webhook_test.go @@ -59,6 +59,7 @@ var _ = It("should validate the so creation when there are other SO for other wo workload := createDeployment(namespaceName, false, false) otherWorkload := createDeployment(namespaceName, false, false) + otherWorkload.Name = "other-workload" otherWorkload.Spec.Template.Name = "other-workload" so1 := createScaledObject(soName, namespaceName, workloadName, "apps/v1", "Deployment", false, map[string]string{}, "") @@ -556,22 +557,18 @@ var _ = It("shouldn't validate the so creation with cpu and memory when stateful }).Should(HaveOccurred()) }) -var _ = It("should validate the so creation without cpu and memory when custom resources", func() { +var _ = It("shouldn't validate the so creation without cpu and memory when custom resources without real StatefulSet", func() { namespaceName := "crd-not-resources" namespace := createNamespace(namespaceName) - workload := createStatefulSet(namespaceName, false, true) so := createScaledObject(soName, namespaceName, workloadName, "custom-api", "StatefulSet", true, map[string]string{}, "") err := k8sClient.Create(context.Background(), namespace) Expect(err).ToNot(HaveOccurred()) - err = k8sClient.Create(context.Background(), workload) - Expect(err).ToNot(HaveOccurred()) - Eventually(func() error { return k8sClient.Create(context.Background(), so) - }).ShouldNot(HaveOccurred()) + }).Should(HaveOccurred()) }) var _ = It("should validate so creation when all requirements are met for scaling to zero with cpu scaler", func() { From 22b489d048f77775bf7be00cefaf78020b35b9c9 Mon Sep 17 00:00:00 2001 From: Shane Date: Wed, 20 Nov 2024 23:43:59 +0800 Subject: [PATCH 09/24] revise test case annotation and follow review opinion Signed-off-by: Shane --- CHANGELOG.md | 1 + apis/keda/v1alpha1/scaledobject_types.go | 2 +- apis/keda/v1alpha1/scaledobject_webhook_test.go | 2 +- 3 files changed, 3 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ccfef70786b..850cc1bcffb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -59,6 +59,7 @@ To learn more about active deprecations, we recommend checking [GitHub Discussio ### New -**General**: Add scaleTargetRef exists check in webhook and revise some variable name ([#6350](https://github.com/kedacore/keda/pull/6350)) +- **General**: Introduce new NSQ scaler ([#3281](https://github.com/kedacore/keda/issues/3281)) #### Experimental diff --git a/apis/keda/v1alpha1/scaledobject_types.go b/apis/keda/v1alpha1/scaledobject_types.go index 6cc6e97add3..e10982c4e69 100644 --- a/apis/keda/v1alpha1/scaledobject_types.go +++ b/apis/keda/v1alpha1/scaledobject_types.go @@ -270,7 +270,7 @@ func (so *ScaledObject) CheckScaleTargetRefIfExist(ctx context.Context) error { gvkString := soGvkr.GVKString() unstruct := &unstructured.Unstructured{} unstruct.SetGroupVersionKind(soGvkr.GroupVersionKind()) - if err := kc.Get(ctx, client.ObjectKey{Namespace: so.Namespace, Name: so.Spec.ScaleTargetRef.Name}, unstruct); err != nil { + if err := getFromCacheOrDirect(ctx, client.ObjectKey{Namespace: so.Namespace, Name: so.Spec.ScaleTargetRef.Name}, unstruct); err != nil { // resource doesn't exist scaledobjectlog.Error(err, message.ScaleTargetNotFoundMsg, "resource", gvkString, "name", so.Spec.ScaleTargetRef.Name) return err diff --git a/apis/keda/v1alpha1/scaledobject_webhook_test.go b/apis/keda/v1alpha1/scaledobject_webhook_test.go index 75edace5250..04d10695116 100644 --- a/apis/keda/v1alpha1/scaledobject_webhook_test.go +++ b/apis/keda/v1alpha1/scaledobject_webhook_test.go @@ -557,7 +557,7 @@ var _ = It("shouldn't validate the so creation with cpu and memory when stateful }).Should(HaveOccurred()) }) -var _ = It("shouldn't validate the so creation without cpu and memory when custom resources without real StatefulSet", func() { +var _ = It("shouldn't validate the so creation without cpu and memory when custom resources and real StatefulSet not found", func() { namespaceName := "crd-not-resources" namespace := createNamespace(namespaceName) From c017a92faf9f4bc75abbb15620c87712111741b6 Mon Sep 17 00:00:00 2001 From: Shane Date: Sun, 8 Dec 2024 20:20:14 +0800 Subject: [PATCH 10/24] revise cloudevent test Signed-off-by: Shane --- .../cloudevent_source/cloudevent_source_test.go | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/tests/internals/cloudevent_source/cloudevent_source_test.go b/tests/internals/cloudevent_source/cloudevent_source_test.go index b1001c7c1e3..aa1b87aa440 100644 --- a/tests/internals/cloudevent_source/cloudevent_source_test.go +++ b/tests/internals/cloudevent_source/cloudevent_source_test.go @@ -13,8 +13,6 @@ import ( "github.com/joho/godotenv" "github.com/stretchr/testify/assert" "k8s.io/client-go/kubernetes" - - . "github.com/kedacore/keda/v2/tests/helper" ) const ( @@ -116,7 +114,7 @@ metadata: namespace: {{.TestNamespace}} spec: scaleTargetRef: - name: test + name: {{.DeploymentName}} triggers: - type: kubernetes-workload metadata: @@ -379,6 +377,7 @@ func testErrEventSourceEmitValue(t *testing.T, _ *kubernetes.Clientset, data tem t.Log("--- test emitting eventsource about scaledobject err---") KubectlApplyWithTemplate(t, data, "cloudEventSourceTemplate", ceTemplate) + KubectlApplyWithTemplate(t, data, "deploymentTemplate", deploymentTemplate) KubectlApplyWithTemplate(t, data, "scaledObjectErrTemplate", scaledObjectErrTemplate) // wait 15 seconds to ensure event propagation @@ -430,8 +429,8 @@ func testErrEventSourceEmitValue(t *testing.T, _ *kubernetes.Clientset, data tem func testEventSourceEmitValue(t *testing.T, _ *kubernetes.Clientset, data templateData) { t.Log("--- test emitting eventsource about scaledobject removed---") - KubectlApplyWithTemplate(t, data, "scaledObjectTemplate", scaledObjectTemplate) KubectlApplyWithTemplate(t, data, "deploymentTemplate", deploymentTemplate) + KubectlApplyWithTemplate(t, data, "scaledObjectTemplate", scaledObjectTemplate) // wait 15 seconds to ensure event propagation time.Sleep(5 * time.Second) @@ -483,6 +482,7 @@ func testErrEventSourceExcludeValue(t *testing.T, _ *kubernetes.Clientset, data } KubectlApplyWithTemplate(t, data, "cloudEventSourceWithExcludeTemplate", ceTemplate) + KubectlApplyWithTemplate(t, data, "deploymentTemplate", deploymentTemplate) KubectlApplyWithTemplate(t, data, "scaledObjectErrTemplate", scaledObjectErrTemplate) // wait 15 seconds to ensure event propagation @@ -524,6 +524,7 @@ func testErrEventSourceIncludeValue(t *testing.T, _ *kubernetes.Clientset, data } KubectlApplyWithTemplate(t, data, "cloudEventSourceWithIncludeTemplate", ceTemplate) + KubectlApplyWithTemplate(t, data, "deploymentTemplate", deploymentTemplate) KubectlApplyWithTemplate(t, data, "scaledObjectErrTemplate", scaledObjectErrTemplate) // wait 15 seconds to ensure event propagation From 00bef8e5457a6882fdf9fe92f571467aab8605dd Mon Sep 17 00:00:00 2001 From: Shane Date: Sun, 8 Dec 2024 20:38:37 +0800 Subject: [PATCH 11/24] Update cloudevent_source_test.go Signed-off-by: Shane --- tests/internals/cloudevent_source/cloudevent_source_test.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/internals/cloudevent_source/cloudevent_source_test.go b/tests/internals/cloudevent_source/cloudevent_source_test.go index aa1b87aa440..561f94eb896 100644 --- a/tests/internals/cloudevent_source/cloudevent_source_test.go +++ b/tests/internals/cloudevent_source/cloudevent_source_test.go @@ -13,6 +13,8 @@ import ( "github.com/joho/godotenv" "github.com/stretchr/testify/assert" "k8s.io/client-go/kubernetes" + + . "github.com/kedacore/keda/v2/tests/helper" ) const ( From cf2cc4232d14ce721efbc8e472e28657d4f586d2 Mon Sep 17 00:00:00 2001 From: Shane Date: Tue, 10 Dec 2024 12:46:03 +0800 Subject: [PATCH 12/24] change end-to-end test cases Signed-off-by: Shane --- .../cloudevent_source_test.go | 6 +- tests/internals/events/events_test.go | 9 --- .../scaled_object_validation_test.go | 59 +++++++++++++++---- 3 files changed, 53 insertions(+), 21 deletions(-) diff --git a/tests/internals/cloudevent_source/cloudevent_source_test.go b/tests/internals/cloudevent_source/cloudevent_source_test.go index 561f94eb896..c63e5f266e3 100644 --- a/tests/internals/cloudevent_source/cloudevent_source_test.go +++ b/tests/internals/cloudevent_source/cloudevent_source_test.go @@ -13,8 +13,6 @@ import ( "github.com/joho/godotenv" "github.com/stretchr/testify/assert" "k8s.io/client-go/kubernetes" - - . "github.com/kedacore/keda/v2/tests/helper" ) const ( @@ -426,6 +424,7 @@ func testErrEventSourceEmitValue(t *testing.T, _ *kubernetes.Clientset, data tem } assert.NotEmpty(t, foundEvents) KubectlDeleteWithTemplate(t, data, "cloudEventSourceTemplate", ceTemplate) + KubectlDeleteWithTemplate(t, data, "deploymentTemplate", deploymentTemplate) t.Log("--- testErrEventSourceEmitValuetestErrEventSourceEmitValuer---", "cloud event time", lastCloudEventTime) } @@ -470,6 +469,7 @@ func testEventSourceEmitValue(t *testing.T, _ *kubernetes.Clientset, data templa } } assert.NotEmpty(t, foundEvents) + KubectlDeleteWithTemplate(t, data, "deploymentTemplate", deploymentTemplate) } // tests error events not emitted by @@ -512,6 +512,7 @@ func testErrEventSourceExcludeValue(t *testing.T, _ *kubernetes.Clientset, data } KubectlDeleteWithTemplate(t, data, "cloudEventSourceWithExcludeTemplate", ceTemplate) + KubectlDeleteWithTemplate(t, data, "deploymentTemplate", deploymentTemplate) } // tests error events in include filter @@ -552,6 +553,7 @@ func testErrEventSourceIncludeValue(t *testing.T, _ *kubernetes.Clientset, data } assert.NotEmpty(t, foundEvents) KubectlDeleteWithTemplate(t, data, "cloudEventSourceWithIncludeTemplate", ceTemplate) + KubectlDeleteWithTemplate(t, data, "deploymentTemplate", deploymentTemplate) } // tests error event type when creation diff --git a/tests/internals/events/events_test.go b/tests/internals/events/events_test.go index 3f9122d81fe..7719137d727 100644 --- a/tests/internals/events/events_test.go +++ b/tests/internals/events/events_test.go @@ -13,7 +13,6 @@ import ( "github.com/kedacore/keda/v2/pkg/common/message" "github.com/kedacore/keda/v2/pkg/eventreason" - . "github.com/kedacore/keda/v2/tests/helper" ) const ( @@ -352,14 +351,6 @@ func testNormalEvent(t *testing.T, kc *kubernetes.Clientset, data templateData) KubectlDeleteWithTemplate(t, data, "scaledObjectTemplate", scaledObjectTemplate) } -func testTargetNotFoundErr(t *testing.T, _ *kubernetes.Clientset, data templateData) { - t.Log("--- testing target not found error event ---") - - KubectlApplyWithTemplate(t, data, "scaledObjectTargetErrTemplate", scaledObjectTargetErrTemplate) - checkingEvent(t, testNamespace, scaledObjectTargetNotFoundName, -2, eventreason.ScaledObjectCheckFailed, message.ScaleTargetNotFoundMsg) - checkingEvent(t, testNamespace, scaledObjectTargetNotFoundName, -1, eventreason.ScaledObjectCheckFailed, message.ScaleTargetErrMsg) -} - func testTargetNotSupportEventErr(t *testing.T, _ *kubernetes.Clientset, data templateData) { t.Log("--- testing target not support error event ---") diff --git a/tests/internals/scaled_object_validation/scaled_object_validation_test.go b/tests/internals/scaled_object_validation/scaled_object_validation_test.go index 2af7f6b81d8..bed4557fc84 100644 --- a/tests/internals/scaled_object_validation/scaled_object_validation_test.go +++ b/tests/internals/scaled_object_validation/scaled_object_validation_test.go @@ -8,8 +8,6 @@ import ( "testing" "github.com/stretchr/testify/assert" - - . "github.com/kedacore/keda/v2/tests/helper" ) const ( @@ -18,7 +16,8 @@ const ( var ( testNamespace = fmt.Sprintf("%s-ns", testName) - deploymentName = fmt.Sprintf("%s-deployment", testName) + deployment1Name = fmt.Sprintf("%s-deployment", testName) + deployment2Name = fmt.Sprintf("%s-deployment2", testName) scaledObject1Name = fmt.Sprintf("%s-so1", testName) scaledObject2Name = fmt.Sprintf("%s-so2", testName) emptyTriggersSoName = fmt.Sprintf("%s-so-empty-triggers", testName) @@ -218,18 +217,24 @@ func TestScaledObjectValidations(t *testing.T) { } func testWithNotScaledWorkload(t *testing.T, data templateData) { - t.Log("--- unscaled workload ---") + t.Log("--- scaled workload ---") + err := KubectlApplyWithErrors(t, data, "deploymentTemplate", deploymentTemplate) + assert.Errorf(t, err, "cannot deploy the deployment - %s", err) data.ScaledObjectName = scaledObject1Name err := KubectlApplyWithErrors(t, data, "scaledObjectTemplate", scaledObjectTemplate) assert.NoErrorf(t, err, "cannot deploy the scaledObject - %s", err) KubectlDeleteWithTemplate(t, data, "scaledObjectTemplate", scaledObjectTemplate) + KubectlDeleteWithTemplate(t, data, "deploymentTemplate", deploymentTemplate) } func testScaledWorkloadByOtherScaledObject(t *testing.T, data templateData) { t.Log("--- already scaled workload by other scaledobject---") + err := KubectlApplyWithErrors(t, data, "deploymentTemplate", deploymentTemplate) + assert.Errorf(t, err, "cannot deploy the deployment - %s", err) + data.ScaledObjectName = scaledObject1Name err := KubectlApplyWithErrors(t, data, "scaledObjectTemplate", scaledObjectTemplate) assert.NoErrorf(t, err, "cannot deploy the scaledObject - %s", err) @@ -237,10 +242,11 @@ func testScaledWorkloadByOtherScaledObject(t *testing.T, data templateData) { data.ScaledObjectName = scaledObject2Name err = KubectlApplyWithErrors(t, data, "scaledObjectTemplate", scaledObjectTemplate) assert.Errorf(t, err, "can deploy the scaledObject - %s", err) - assert.Contains(t, err.Error(), fmt.Sprintf("the workload '%s' of type 'apps/v1.Deployment' is already managed by the ScaledObject '%s", deploymentName, scaledObject1Name)) + assert.Contains(t, err.Error(), fmt.Sprintf("the workload '%s' of type 'apps/v1.Deployment' is already managed by the ScaledObject '%s", deployment1Name, scaledObject1Name)) data.ScaledObjectName = scaledObject1Name KubectlDeleteWithTemplate(t, data, "scaledObjectTemplate", scaledObjectTemplate) + KubectlDeleteWithTemplate(t, data, "deploymentTemplate", deploymentTemplate) } func testManagedHpaByOtherScaledObject(t *testing.T, data templateData) { @@ -248,23 +254,36 @@ func testManagedHpaByOtherScaledObject(t *testing.T, data templateData) { data.HpaName = hpaName + err := KubectlApplyWithErrors(t, data, "deploymentTemplate", deploymentTemplate) + assert.Errorf(t, err, "cannot deploy the deployment - %s", err) + data.ScaledObjectName = scaledObject1Name err := KubectlApplyWithErrors(t, data, "scaledObjectTemplate", customHpaScaledObjectTemplate) assert.NoErrorf(t, err, "cannot deploy the scaledObject - %s", err) data.ScaledObjectName = scaledObject2Name - data.DeploymentName = fmt.Sprintf("%s-other-deployment", testName) + data.DeploymentName = deployment2Name + + err := KubectlApplyWithErrors(t, data, "deploymentTemplate", deploymentTemplate) + assert.Errorf(t, err, "cannot deploy the deployment - %s", err) + err = KubectlApplyWithErrors(t, data, "scaledObjectTemplate", customHpaScaledObjectTemplate) assert.Errorf(t, err, "can deploy the scaledObject - %s", err) assert.Contains(t, err.Error(), fmt.Sprintf("the HPA '%s' is already managed by the ScaledObject '%s", hpaName, scaledObject1Name)) data.ScaledObjectName = scaledObject1Name KubectlDeleteWithTemplate(t, data, "scaledObjectTemplate", scaledObjectTemplate) + KubectlDeleteWithTemplate(t, data, "deploymentTemplate", deploymentTemplate) + data.DeploymentName = deployment1Name + KubectlDeleteWithTemplate(t, data, "deploymentTemplate", deploymentTemplate) } func testScaledWorkloadByOtherHpa(t *testing.T, data templateData) { t.Log("--- already scaled workload by other hpa---") + err := KubectlApplyWithErrors(t, data, "deploymentTemplate", deploymentTemplate) + assert.Errorf(t, err, "cannot deploy the deployment - %s", err) + data.HpaName = hpaName err := KubectlApplyWithErrors(t, data, "hpaTemplate", hpaTemplate) assert.NoErrorf(t, err, "cannot deploy the hpa - %s", err) @@ -272,14 +291,18 @@ func testScaledWorkloadByOtherHpa(t *testing.T, data templateData) { data.ScaledObjectName = scaledObject1Name err = KubectlApplyWithErrors(t, data, "scaledObjectTemplate", scaledObjectTemplate) assert.Errorf(t, err, "can deploy the scaledObject - %s", err) - assert.Contains(t, err.Error(), fmt.Sprintf("the workload '%s' of type 'apps/v1.Deployment' is already managed by the hpa '%s", deploymentName, hpaName)) + assert.Contains(t, err.Error(), fmt.Sprintf("the workload '%s' of type 'apps/v1.Deployment' is already managed by the hpa '%s", deployment1Name, hpaName)) KubectlDeleteWithTemplate(t, data, "hpaTemplate", hpaTemplate) + KubectlDeleteWithTemplate(t, data, "deploymentTemplate", deploymentTemplate) } func testScaledWorkloadByOtherHpaWithOwnershipTransfer(t *testing.T, data templateData) { t.Log("--- already scaled workload by other hpa ownership transfer ---") + err := KubectlApplyWithErrors(t, data, "deploymentTemplate", deploymentTemplate) + assert.Errorf(t, err, "cannot deploy the deployment - %s", err) + data.HpaName = ownershipTransferHpaName err := KubectlApplyWithErrors(t, data, "hpaTemplate", hpaTemplate) assert.NoErrorf(t, err, "cannot deploy the hpa - %s", err) @@ -290,24 +313,35 @@ func testScaledWorkloadByOtherHpaWithOwnershipTransfer(t *testing.T, data templa KubectlDeleteWithTemplate(t, data, "hpaTemplate", hpaTemplate) KubectlDeleteWithTemplate(t, data, "ownershipTransferScaledObjectTemplate", ownershipTransferScaledObjectTemplate) + KubectlDeleteWithTemplate(t, data, "deploymentTemplate", deploymentTemplate) } func testMissingCPU(t *testing.T, data templateData) { t.Log("--- missing cpu resource ---") + err := KubectlApplyWithErrors(t, data, "deploymentTemplate", deploymentTemplate) + assert.Errorf(t, err, "cannot deploy the deployment - %s", err) + data.ScaledObjectName = scaledObject1Name err := KubectlApplyWithErrors(t, data, "scaledObjectTemplate", cpuScaledObjectTemplate) assert.Errorf(t, err, "can deploy the scaledObject - %s", err) - assert.Contains(t, err.Error(), fmt.Sprintf("the scaledobject has a cpu trigger but the container %s doesn't have the cpu request defined", deploymentName)) + assert.Contains(t, err.Error(), fmt.Sprintf("the scaledobject has a cpu trigger but the container %s doesn't have the cpu request defined", deployment1Name)) + + KubectlDeleteWithTemplate(t, data, "deploymentTemplate", deploymentTemplate) } func testMissingMemory(t *testing.T, data templateData) { t.Log("--- missing memory resource ---") + err := KubectlApplyWithErrors(t, data, "deploymentTemplate", deploymentTemplate) + assert.Errorf(t, err, "cannot deploy the deployment - %s", err) + data.ScaledObjectName = scaledObject1Name err := KubectlApplyWithErrors(t, data, "scaledObjectTemplate", memoryScaledObjectTemplate) assert.Errorf(t, err, "can deploy the scaledObject - %s", err) - assert.Contains(t, err.Error(), fmt.Sprintf("the scaledobject has a memory trigger but the container %s doesn't have the memory request defined", deploymentName)) + assert.Contains(t, err.Error(), fmt.Sprintf("the scaledobject has a memory trigger but the container %s doesn't have the memory request defined", deployment1Name)) + + KubectlDeleteWithTemplate(t, data, "deploymentTemplate", deploymentTemplate) } func testWorkloadWithOnlyLimits(t *testing.T, data templateData) { @@ -373,15 +407,20 @@ spec: func testTriggersWithEmptyArray(t *testing.T, data templateData) { t.Log("--- triggers with empty array ---") + err := KubectlApplyWithErrors(t, data, "deploymentTemplate", deploymentTemplate) + assert.Errorf(t, err, "cannot deploy the deployment - %s", err) + err := KubectlApplyWithErrors(t, data, "emptyTriggersTemplate", emptyTriggersTemplate) assert.Errorf(t, err, "can deploy the scaledObject - %s", err) assert.Contains(t, err.Error(), "no triggers defined in the ScaledObject/ScaledJob") + + KubectlDeleteWithTemplate(t, data, "deploymentTemplate", deploymentTemplate) } func getTemplateData() (templateData, []Template) { return templateData{ TestNamespace: testNamespace, - DeploymentName: deploymentName, + DeploymentName: deployment1Name, EmptyTriggersSoName: emptyTriggersSoName, }, []Template{ {Name: "deploymentTemplate", Config: deploymentTemplate}, From 2614ea9a8f17ce1faee81d62c89bba5d890861fe Mon Sep 17 00:00:00 2001 From: Shane Date: Tue, 10 Dec 2024 20:19:34 +0800 Subject: [PATCH 13/24] revise test case Signed-off-by: Shane --- apis/keda/v1alpha1/scaledobject_types.go | 3 +- apis/keda/v1alpha1/scaledobject_webhook.go | 2 +- controllers/keda/scaledobject_controller.go | 8 +++- .../cloudevent_source_test.go | 6 ++- tests/internals/events/events_test.go | 1 - .../scaled_object_validation_test.go | 40 +++++++++---------- 6 files changed, 34 insertions(+), 26 deletions(-) diff --git a/apis/keda/v1alpha1/scaledobject_types.go b/apis/keda/v1alpha1/scaledobject_types.go index e10982c4e69..fcae0c17dea 100644 --- a/apis/keda/v1alpha1/scaledobject_types.go +++ b/apis/keda/v1alpha1/scaledobject_types.go @@ -19,6 +19,7 @@ package v1alpha1 import ( "context" "fmt" + "k8s.io/apimachinery/pkg/api/meta" "reflect" "strconv" @@ -260,7 +261,7 @@ func (so *ScaledObject) GetHPAMaxReplicas() int32 { } // CheckScaleTargetRefIfExist checks if scaleTargetRef of ScaledObject exists -func (so *ScaledObject) CheckScaleTargetRefIfExist(ctx context.Context) error { +func (so *ScaledObject) CheckScaleTargetRefIfExist(ctx context.Context, restMapper meta.RESTMapper, getFromCacheOrDirect func(ctx context.Context, key client.ObjectKey, obj client.Object) error) error { soGvkr, err := ParseGVKR(restMapper, so.Spec.ScaleTargetRef.APIVersion, so.Spec.ScaleTargetRef.Kind) if err != nil { msg := "Failed to parse Group, Version, Kind, Resource" diff --git a/apis/keda/v1alpha1/scaledobject_webhook.go b/apis/keda/v1alpha1/scaledobject_webhook.go index b5fba489ae7..5ef06a93709 100644 --- a/apis/keda/v1alpha1/scaledobject_webhook.go +++ b/apis/keda/v1alpha1/scaledobject_webhook.go @@ -288,7 +288,7 @@ func verifyScaledObjects(incomingSo *ScaledObject, action string, _ bool) error if err != nil { return err } - if err := incomingSo.CheckScaleTargetRefIfExist(context.Background()); err != nil { + if err := incomingSo.CheckScaleTargetRefIfExist(context.Background(), restMapper, getFromCacheOrDirect); err != nil { return err } diff --git a/controllers/keda/scaledobject_controller.go b/controllers/keda/scaledobject_controller.go index 0118364dcc5..2b8909e630c 100755 --- a/controllers/keda/scaledobject_controller.go +++ b/controllers/keda/scaledobject_controller.go @@ -398,7 +398,7 @@ func (r *ScaledObjectReconciler) checkTargetResourceIsScalable(ctx context.Conte scale, errScale = (r.ScaleClient).Scales(scaledObject.Namespace).Get(ctx, gr, scaledObject.Spec.ScaleTargetRef.Name, metav1.GetOptions{}) if errScale != nil { // not able to get /scale subresource -> let's check if the resource even exist in the cluster - if err := scaledObject.CheckScaleTargetRefIfExist(ctx); err != nil { + if err := scaledObject.CheckScaleTargetRefIfExist(ctx, r.restMapper, r.getFromCacheOrDirect); err != nil { r.EventEmitter.Emit(scaledObject, scaledObject.Namespace, corev1.EventTypeWarning, eventingv1alpha1.ScaledObjectFailedType, eventreason.ScaledObjectCheckFailed, message.ScaleTargetNotFoundMsg) return gvkr, err } @@ -632,3 +632,9 @@ func (r *ScaledObjectReconciler) updateStatusWithTriggersAndAuthsTypes(ctx conte return kedastatus.UpdateScaledObjectStatus(ctx, r.Client, logger, scaledObject, status) } + +func (r *ScaledObjectReconciler) getFromCacheOrDirect(ctx context.Context, key client.ObjectKey, obj client.Object) error { + // r.Client from controller-runtime here had implemented the logic to choose to perform a cache or live lookup. + err := r.Client.Get(ctx, key, obj, &client.GetOptions{}) + return err +} diff --git a/tests/internals/cloudevent_source/cloudevent_source_test.go b/tests/internals/cloudevent_source/cloudevent_source_test.go index c63e5f266e3..81a32eba899 100644 --- a/tests/internals/cloudevent_source/cloudevent_source_test.go +++ b/tests/internals/cloudevent_source/cloudevent_source_test.go @@ -40,6 +40,7 @@ var ( expectedSubject = fmt.Sprintf("/%s/%s/scaledobject/%s", clusterName, namespace, scaledObjectName) expectedSource = fmt.Sprintf("/%s/keda/keda", clusterName) lastCloudEventTime = time.Now() + test = "test" ) type templateData struct { @@ -114,7 +115,7 @@ metadata: namespace: {{.TestNamespace}} spec: scaleTargetRef: - name: {{.DeploymentName}} + name: test triggers: - type: kubernetes-workload metadata: @@ -377,6 +378,7 @@ func testErrEventSourceEmitValue(t *testing.T, _ *kubernetes.Clientset, data tem t.Log("--- test emitting eventsource about scaledobject err---") KubectlApplyWithTemplate(t, data, "cloudEventSourceTemplate", ceTemplate) + data.DeploymentName = test KubectlApplyWithTemplate(t, data, "deploymentTemplate", deploymentTemplate) KubectlApplyWithTemplate(t, data, "scaledObjectErrTemplate", scaledObjectErrTemplate) @@ -484,6 +486,7 @@ func testErrEventSourceExcludeValue(t *testing.T, _ *kubernetes.Clientset, data } KubectlApplyWithTemplate(t, data, "cloudEventSourceWithExcludeTemplate", ceTemplate) + data.DeploymentName = test KubectlApplyWithTemplate(t, data, "deploymentTemplate", deploymentTemplate) KubectlApplyWithTemplate(t, data, "scaledObjectErrTemplate", scaledObjectErrTemplate) @@ -527,6 +530,7 @@ func testErrEventSourceIncludeValue(t *testing.T, _ *kubernetes.Clientset, data } KubectlApplyWithTemplate(t, data, "cloudEventSourceWithIncludeTemplate", ceTemplate) + data.DeploymentName = test KubectlApplyWithTemplate(t, data, "deploymentTemplate", deploymentTemplate) KubectlApplyWithTemplate(t, data, "scaledObjectErrTemplate", scaledObjectErrTemplate) diff --git a/tests/internals/events/events_test.go b/tests/internals/events/events_test.go index 7719137d727..a1c6f0e0f01 100644 --- a/tests/internals/events/events_test.go +++ b/tests/internals/events/events_test.go @@ -293,7 +293,6 @@ func TestEvents(t *testing.T) { // test scaling testNormalEvent(t, kc, data) - testTargetNotFoundErr(t, kc, data) testTargetNotSupportEventErr(t, kc, data) testScaledJobNormalEvent(t, kc, data) diff --git a/tests/internals/scaled_object_validation/scaled_object_validation_test.go b/tests/internals/scaled_object_validation/scaled_object_validation_test.go index bed4557fc84..17a75df54da 100644 --- a/tests/internals/scaled_object_validation/scaled_object_validation_test.go +++ b/tests/internals/scaled_object_validation/scaled_object_validation_test.go @@ -16,7 +16,7 @@ const ( var ( testNamespace = fmt.Sprintf("%s-ns", testName) - deployment1Name = fmt.Sprintf("%s-deployment", testName) + deployment1Name = fmt.Sprintf("%s-deployment1", testName) deployment2Name = fmt.Sprintf("%s-deployment2", testName) scaledObject1Name = fmt.Sprintf("%s-so1", testName) scaledObject2Name = fmt.Sprintf("%s-so2", testName) @@ -219,10 +219,10 @@ func TestScaledObjectValidations(t *testing.T) { func testWithNotScaledWorkload(t *testing.T, data templateData) { t.Log("--- scaled workload ---") err := KubectlApplyWithErrors(t, data, "deploymentTemplate", deploymentTemplate) - assert.Errorf(t, err, "cannot deploy the deployment - %s", err) + assert.NoErrorf(t, err, "cannot deploy the deployment - %s", err) data.ScaledObjectName = scaledObject1Name - err := KubectlApplyWithErrors(t, data, "scaledObjectTemplate", scaledObjectTemplate) + err = KubectlApplyWithErrors(t, data, "scaledObjectTemplate", scaledObjectTemplate) assert.NoErrorf(t, err, "cannot deploy the scaledObject - %s", err) KubectlDeleteWithTemplate(t, data, "scaledObjectTemplate", scaledObjectTemplate) @@ -233,10 +233,10 @@ func testScaledWorkloadByOtherScaledObject(t *testing.T, data templateData) { t.Log("--- already scaled workload by other scaledobject---") err := KubectlApplyWithErrors(t, data, "deploymentTemplate", deploymentTemplate) - assert.Errorf(t, err, "cannot deploy the deployment - %s", err) + assert.NoErrorf(t, err, "cannot deploy the deployment - %s", err) data.ScaledObjectName = scaledObject1Name - err := KubectlApplyWithErrors(t, data, "scaledObjectTemplate", scaledObjectTemplate) + err = KubectlApplyWithErrors(t, data, "scaledObjectTemplate", scaledObjectTemplate) assert.NoErrorf(t, err, "cannot deploy the scaledObject - %s", err) data.ScaledObjectName = scaledObject2Name @@ -255,17 +255,17 @@ func testManagedHpaByOtherScaledObject(t *testing.T, data templateData) { data.HpaName = hpaName err := KubectlApplyWithErrors(t, data, "deploymentTemplate", deploymentTemplate) - assert.Errorf(t, err, "cannot deploy the deployment - %s", err) + assert.NoErrorf(t, err, "cannot deploy the deployment - %s", err) data.ScaledObjectName = scaledObject1Name - err := KubectlApplyWithErrors(t, data, "scaledObjectTemplate", customHpaScaledObjectTemplate) + err = KubectlApplyWithErrors(t, data, "scaledObjectTemplate", customHpaScaledObjectTemplate) assert.NoErrorf(t, err, "cannot deploy the scaledObject - %s", err) data.ScaledObjectName = scaledObject2Name data.DeploymentName = deployment2Name - err := KubectlApplyWithErrors(t, data, "deploymentTemplate", deploymentTemplate) - assert.Errorf(t, err, "cannot deploy the deployment - %s", err) + err = KubectlApplyWithErrors(t, data, "deploymentTemplate", deploymentTemplate) + assert.NoErrorf(t, err, "cannot deploy the deployment - %s", err) err = KubectlApplyWithErrors(t, data, "scaledObjectTemplate", customHpaScaledObjectTemplate) assert.Errorf(t, err, "can deploy the scaledObject - %s", err) @@ -282,10 +282,10 @@ func testScaledWorkloadByOtherHpa(t *testing.T, data templateData) { t.Log("--- already scaled workload by other hpa---") err := KubectlApplyWithErrors(t, data, "deploymentTemplate", deploymentTemplate) - assert.Errorf(t, err, "cannot deploy the deployment - %s", err) + assert.NoErrorf(t, err, "cannot deploy the deployment - %s", err) data.HpaName = hpaName - err := KubectlApplyWithErrors(t, data, "hpaTemplate", hpaTemplate) + err = KubectlApplyWithErrors(t, data, "hpaTemplate", hpaTemplate) assert.NoErrorf(t, err, "cannot deploy the hpa - %s", err) data.ScaledObjectName = scaledObject1Name @@ -301,10 +301,10 @@ func testScaledWorkloadByOtherHpaWithOwnershipTransfer(t *testing.T, data templa t.Log("--- already scaled workload by other hpa ownership transfer ---") err := KubectlApplyWithErrors(t, data, "deploymentTemplate", deploymentTemplate) - assert.Errorf(t, err, "cannot deploy the deployment - %s", err) + assert.NoErrorf(t, err, "cannot deploy the deployment - %s", err) data.HpaName = ownershipTransferHpaName - err := KubectlApplyWithErrors(t, data, "hpaTemplate", hpaTemplate) + err = KubectlApplyWithErrors(t, data, "hpaTemplate", hpaTemplate) assert.NoErrorf(t, err, "cannot deploy the hpa - %s", err) data.ScaledObjectName = ownershipTransferScaledObjectName @@ -320,10 +320,10 @@ func testMissingCPU(t *testing.T, data templateData) { t.Log("--- missing cpu resource ---") err := KubectlApplyWithErrors(t, data, "deploymentTemplate", deploymentTemplate) - assert.Errorf(t, err, "cannot deploy the deployment - %s", err) + assert.NoErrorf(t, err, "cannot deploy the deployment - %s", err) data.ScaledObjectName = scaledObject1Name - err := KubectlApplyWithErrors(t, data, "scaledObjectTemplate", cpuScaledObjectTemplate) + err = KubectlApplyWithErrors(t, data, "scaledObjectTemplate", cpuScaledObjectTemplate) assert.Errorf(t, err, "can deploy the scaledObject - %s", err) assert.Contains(t, err.Error(), fmt.Sprintf("the scaledobject has a cpu trigger but the container %s doesn't have the cpu request defined", deployment1Name)) @@ -334,10 +334,10 @@ func testMissingMemory(t *testing.T, data templateData) { t.Log("--- missing memory resource ---") err := KubectlApplyWithErrors(t, data, "deploymentTemplate", deploymentTemplate) - assert.Errorf(t, err, "cannot deploy the deployment - %s", err) + assert.NoErrorf(t, err, "cannot deploy the deployment - %s", err) data.ScaledObjectName = scaledObject1Name - err := KubectlApplyWithErrors(t, data, "scaledObjectTemplate", memoryScaledObjectTemplate) + err = KubectlApplyWithErrors(t, data, "scaledObjectTemplate", memoryScaledObjectTemplate) assert.Errorf(t, err, "can deploy the scaledObject - %s", err) assert.Contains(t, err.Error(), fmt.Sprintf("the scaledobject has a memory trigger but the container %s doesn't have the memory request defined", deployment1Name)) @@ -408,13 +408,11 @@ func testTriggersWithEmptyArray(t *testing.T, data templateData) { t.Log("--- triggers with empty array ---") err := KubectlApplyWithErrors(t, data, "deploymentTemplate", deploymentTemplate) - assert.Errorf(t, err, "cannot deploy the deployment - %s", err) + assert.NoErrorf(t, err, "cannot deploy the deployment - %s", err) - err := KubectlApplyWithErrors(t, data, "emptyTriggersTemplate", emptyTriggersTemplate) + err = KubectlApplyWithErrors(t, data, "emptyTriggersTemplate", emptyTriggersTemplate) assert.Errorf(t, err, "can deploy the scaledObject - %s", err) assert.Contains(t, err.Error(), "no triggers defined in the ScaledObject/ScaledJob") - - KubectlDeleteWithTemplate(t, data, "deploymentTemplate", deploymentTemplate) } func getTemplateData() (templateData, []Template) { From 684504567d9a196e5975fa1bbcc715457d85cf1a Mon Sep 17 00:00:00 2001 From: Shane Date: Tue, 10 Dec 2024 20:33:54 +0800 Subject: [PATCH 14/24] Update cloudevent_source_test.go Signed-off-by: Shane --- tests/internals/cloudevent_source/cloudevent_source_test.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/internals/cloudevent_source/cloudevent_source_test.go b/tests/internals/cloudevent_source/cloudevent_source_test.go index 81a32eba899..85101d2f4de 100644 --- a/tests/internals/cloudevent_source/cloudevent_source_test.go +++ b/tests/internals/cloudevent_source/cloudevent_source_test.go @@ -13,6 +13,8 @@ import ( "github.com/joho/godotenv" "github.com/stretchr/testify/assert" "k8s.io/client-go/kubernetes" + + . "github.com/kedacore/keda/v2/tests/helper" ) const ( From 97efe83c907d3823c7af3db75ead5633dd89da05 Mon Sep 17 00:00:00 2001 From: Shane Date: Tue, 10 Dec 2024 20:34:24 +0800 Subject: [PATCH 15/24] Update events_test.go Signed-off-by: Shane --- tests/internals/events/events_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/internals/events/events_test.go b/tests/internals/events/events_test.go index a1c6f0e0f01..24b28b89e63 100644 --- a/tests/internals/events/events_test.go +++ b/tests/internals/events/events_test.go @@ -13,6 +13,7 @@ import ( "github.com/kedacore/keda/v2/pkg/common/message" "github.com/kedacore/keda/v2/pkg/eventreason" + . "github.com/kedacore/keda/v2/tests/helper" ) const ( From fab9b0fa12597c53f2abba731f0e43e108993c75 Mon Sep 17 00:00:00 2001 From: Shane Date: Tue, 10 Dec 2024 20:35:06 +0800 Subject: [PATCH 16/24] Update scaled_object_validation_test.go Signed-off-by: Shane --- .../scaled_object_validation/scaled_object_validation_test.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/internals/scaled_object_validation/scaled_object_validation_test.go b/tests/internals/scaled_object_validation/scaled_object_validation_test.go index 17a75df54da..15379019cbd 100644 --- a/tests/internals/scaled_object_validation/scaled_object_validation_test.go +++ b/tests/internals/scaled_object_validation/scaled_object_validation_test.go @@ -8,6 +8,8 @@ import ( "testing" "github.com/stretchr/testify/assert" + + . "github.com/kedacore/keda/v2/tests/helper" ) const ( From ff3ab9e8c9618bcc73c7b477f1729335474c23b4 Mon Sep 17 00:00:00 2001 From: Shane Date: Tue, 10 Dec 2024 20:44:05 +0800 Subject: [PATCH 17/24] Update events_test.go Signed-off-by: Shane --- tests/internals/events/events_test.go | 16 ---------------- 1 file changed, 16 deletions(-) diff --git a/tests/internals/events/events_test.go b/tests/internals/events/events_test.go index 24b28b89e63..9892939964f 100644 --- a/tests/internals/events/events_test.go +++ b/tests/internals/events/events_test.go @@ -115,22 +115,6 @@ spec: image: nginxinc/nginx-unprivileged:alpine-slim ` - scaledObjectTargetErrTemplate = ` -apiVersion: keda.sh/v1alpha1 -kind: ScaledObject -metadata: - name: {{.ScaledObjectTargetNotFoundName}} - namespace: {{.TestNamespace}} -spec: - scaleTargetRef: - name: no-exist - triggers: - - type: kubernetes-workload - metadata: - podSelector: 'app={{.DeploymentName}}' - value: '1' -` - daemonSetTemplate = ` apiVersion: apps/v1 kind: DaemonSet From 7772090397e6bac1a639f23107758096f6b8a8c7 Mon Sep 17 00:00:00 2001 From: Shane Date: Tue, 10 Dec 2024 21:09:57 +0800 Subject: [PATCH 18/24] change import order Signed-off-by: Shane --- apis/keda/v1alpha1/scaledobject_types.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apis/keda/v1alpha1/scaledobject_types.go b/apis/keda/v1alpha1/scaledobject_types.go index fcae0c17dea..24c466ea821 100644 --- a/apis/keda/v1alpha1/scaledobject_types.go +++ b/apis/keda/v1alpha1/scaledobject_types.go @@ -19,11 +19,11 @@ package v1alpha1 import ( "context" "fmt" - "k8s.io/apimachinery/pkg/api/meta" "reflect" "strconv" autoscalingv2 "k8s.io/api/autoscaling/v2" + "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "sigs.k8s.io/controller-runtime/pkg/client" From ea9dc8e0b1d9e7b65544199fdf059db2ff1de4f4 Mon Sep 17 00:00:00 2001 From: Shane Date: Thu, 12 Dec 2024 17:25:04 +0800 Subject: [PATCH 19/24] change test case Signed-off-by: Shane --- .../cloudevent_source_test.go | 52 +++++++++++++------ tests/internals/events/events_test.go | 4 +- 2 files changed, 39 insertions(+), 17 deletions(-) diff --git a/tests/internals/cloudevent_source/cloudevent_source_test.go b/tests/internals/cloudevent_source/cloudevent_source_test.go index 85101d2f4de..7b471eea8a7 100644 --- a/tests/internals/cloudevent_source/cloudevent_source_test.go +++ b/tests/internals/cloudevent_source/cloudevent_source_test.go @@ -14,7 +14,7 @@ import ( "github.com/stretchr/testify/assert" "k8s.io/client-go/kubernetes" - . "github.com/kedacore/keda/v2/tests/helper" + . "github.com/kedacore/keda/v2/tests/helper" ) const ( @@ -28,6 +28,7 @@ var ( namespace = fmt.Sprintf("%s-ns", testName) scaledObjectName = fmt.Sprintf("%s-so", testName) deploymentName = fmt.Sprintf("%s-d", testName) + daemonsetName = fmt.Sprintf("%s-daemonset", testName) clientName = fmt.Sprintf("%s-client", testName) cloudeventSourceName = fmt.Sprintf("%s-ce", testName) cloudeventSourceErrName = fmt.Sprintf("%s-ce-err", testName) @@ -49,6 +50,7 @@ type templateData struct { TestNamespace string ScaledObject string DeploymentName string + DaemonsetName string ClientName string CloudEventSourceName string CloudeventSourceErrName string @@ -109,7 +111,7 @@ const ( cpu: "500m" ` - scaledObjectErrTemplate = ` + scaledObjectTargetNotSupportTemplate = ` apiVersion: keda.sh/v1alpha1 kind: ScaledObject metadata: @@ -117,13 +119,35 @@ metadata: namespace: {{.TestNamespace}} spec: scaleTargetRef: - name: test + name: {{.DaemonsetName}} + kind: DaemonSet triggers: - type: kubernetes-workload metadata: podSelector: 'pod=testWorkloadDeploymentName' value: '1' - activationValue: '3' +` + + daemonSetTemplate = ` +apiVersion: apps/v1 +kind: DaemonSet +metadata: + name: {{.DaemonsetName}} + namespace: {{.TestNamespace}} + labels: + app: {{.DaemonsetName}} +spec: + selector: + matchLabels: + app: {{.DaemonsetName}} + template: + metadata: + labels: + app: {{.DaemonsetName}} + spec: + containers: + - name: {{.DaemonsetName}} + image: nginxinc/nginx-unprivileged:alpine-slim ` clientTemplate = ` @@ -381,12 +405,12 @@ func testErrEventSourceEmitValue(t *testing.T, _ *kubernetes.Clientset, data tem t.Log("--- test emitting eventsource about scaledobject err---") KubectlApplyWithTemplate(t, data, "cloudEventSourceTemplate", ceTemplate) data.DeploymentName = test - KubectlApplyWithTemplate(t, data, "deploymentTemplate", deploymentTemplate) - KubectlApplyWithTemplate(t, data, "scaledObjectErrTemplate", scaledObjectErrTemplate) + KubectlApplyWithTemplate(t, data, "daemonSetTemplate", daemonSetTemplate) + KubectlApplyWithTemplate(t, data, "scaledObjectTargetNotSupportTemplate", scaledObjectTargetNotSupportTemplate) // wait 15 seconds to ensure event propagation time.Sleep(5 * time.Second) - KubectlDeleteWithTemplate(t, data, "scaledObjectErrTemplate", scaledObjectErrTemplate) + KubectlDeleteWithTemplate(t, data, "scaledObjectTargetNotSupportTemplate", scaledObjectTargetNotSupportTemplate) time.Sleep(10 * time.Second) out, outErr, err := ExecCommandOnSpecificPod(t, clientName, namespace, fmt.Sprintf("curl -X GET %s/getCloudEvent/%s", cloudEventHTTPServiceURL, "ScaledObjectCheckFailed")) @@ -411,7 +435,7 @@ func testErrEventSourceEmitValue(t *testing.T, _ *kubernetes.Clientset, data tem assert.NoError(t, err) assert.Condition(t, func() bool { - if data["message"] == "ScaledObject doesn't have correct scaleTargetRef specification" || data["message"] == "Target resource doesn't exist" { + if data["message"] == "ScaledObject doesn't have correct scaleTargetRef specification" || data["message"] == "Target resource doesn't expose /scale subresource" { return true } return false @@ -428,7 +452,6 @@ func testErrEventSourceEmitValue(t *testing.T, _ *kubernetes.Clientset, data tem } assert.NotEmpty(t, foundEvents) KubectlDeleteWithTemplate(t, data, "cloudEventSourceTemplate", ceTemplate) - KubectlDeleteWithTemplate(t, data, "deploymentTemplate", deploymentTemplate) t.Log("--- testErrEventSourceEmitValuetestErrEventSourceEmitValuer---", "cloud event time", lastCloudEventTime) } @@ -489,8 +512,8 @@ func testErrEventSourceExcludeValue(t *testing.T, _ *kubernetes.Clientset, data KubectlApplyWithTemplate(t, data, "cloudEventSourceWithExcludeTemplate", ceTemplate) data.DeploymentName = test - KubectlApplyWithTemplate(t, data, "deploymentTemplate", deploymentTemplate) - KubectlApplyWithTemplate(t, data, "scaledObjectErrTemplate", scaledObjectErrTemplate) + KubectlApplyWithTemplate(t, data, "daemonSetTemplate", daemonSetTemplate) + KubectlApplyWithTemplate(t, data, "scaledObjectTargetNotSupportTemplate", scaledObjectTargetNotSupportTemplate) // wait 15 seconds to ensure event propagation time.Sleep(15 * time.Second) @@ -517,7 +540,6 @@ func testErrEventSourceExcludeValue(t *testing.T, _ *kubernetes.Clientset, data } KubectlDeleteWithTemplate(t, data, "cloudEventSourceWithExcludeTemplate", ceTemplate) - KubectlDeleteWithTemplate(t, data, "deploymentTemplate", deploymentTemplate) } // tests error events in include filter @@ -533,8 +555,8 @@ func testErrEventSourceIncludeValue(t *testing.T, _ *kubernetes.Clientset, data KubectlApplyWithTemplate(t, data, "cloudEventSourceWithIncludeTemplate", ceTemplate) data.DeploymentName = test - KubectlApplyWithTemplate(t, data, "deploymentTemplate", deploymentTemplate) - KubectlApplyWithTemplate(t, data, "scaledObjectErrTemplate", scaledObjectErrTemplate) + KubectlApplyWithTemplate(t, data, "daemonSetTemplate", daemonSetTemplate) + KubectlApplyWithTemplate(t, data, "scaledObjectTargetNotSupportTemplate", scaledObjectTargetNotSupportTemplate) // wait 15 seconds to ensure event propagation time.Sleep(15 * time.Second) @@ -559,7 +581,6 @@ func testErrEventSourceIncludeValue(t *testing.T, _ *kubernetes.Clientset, data } assert.NotEmpty(t, foundEvents) KubectlDeleteWithTemplate(t, data, "cloudEventSourceWithIncludeTemplate", ceTemplate) - KubectlDeleteWithTemplate(t, data, "deploymentTemplate", deploymentTemplate) } // tests error event type when creation @@ -594,6 +615,7 @@ func getTemplateData() (templateData, []Template) { return templateData{ TestNamespace: namespace, ScaledObject: scaledObjectName, + DaemonsetName: daemonsetName, DeploymentName: deploymentName, ClientName: clientName, CloudEventSourceName: cloudeventSourceName, diff --git a/tests/internals/events/events_test.go b/tests/internals/events/events_test.go index 9892939964f..1f791c64fac 100644 --- a/tests/internals/events/events_test.go +++ b/tests/internals/events/events_test.go @@ -13,7 +13,7 @@ import ( "github.com/kedacore/keda/v2/pkg/common/message" "github.com/kedacore/keda/v2/pkg/eventreason" - . "github.com/kedacore/keda/v2/tests/helper" + . "github.com/kedacore/keda/v2/tests/helper" ) const ( @@ -324,7 +324,7 @@ func testNormalEvent(t *testing.T, kc *kubernetes.Clientset, data templateData) // time.Sleep(2 * time.Second) KubernetesScaleDeployment(t, kc, monitoredDeploymentName, 2, testNamespace) - assert.True(t, WaitForDeploymentReplicaReadyCount(t, kc, deploymentName, testNamespace, 2, 60, 1), + assert.True(t, WaitForDeploymentReplicaReadyCount(t, kc, monitoredDeploymentName, testNamespace, 2, 60, 1), "replica count should be 2 after 1 minute") checkingEvent(t, testNamespace, scaledObjectName, 0, eventreason.KEDAScalersStarted, fmt.Sprintf(message.ScalerIsBuiltMsg, "kubernetes-workload")) checkingEvent(t, testNamespace, scaledObjectName, 1, eventreason.KEDAScalersStarted, message.ScalerStartMsg) From 109026338e3d2af420cc2f93c725e8652e6255d1 Mon Sep 17 00:00:00 2001 From: Shane Date: Thu, 12 Dec 2024 17:32:40 +0800 Subject: [PATCH 20/24] change format Signed-off-by: Shane --- tests/internals/cloudevent_source/cloudevent_source_test.go | 2 +- tests/internals/events/events_test.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/internals/cloudevent_source/cloudevent_source_test.go b/tests/internals/cloudevent_source/cloudevent_source_test.go index 7b471eea8a7..5b0ca00beb6 100644 --- a/tests/internals/cloudevent_source/cloudevent_source_test.go +++ b/tests/internals/cloudevent_source/cloudevent_source_test.go @@ -14,7 +14,7 @@ import ( "github.com/stretchr/testify/assert" "k8s.io/client-go/kubernetes" - . "github.com/kedacore/keda/v2/tests/helper" + . "github.com/kedacore/keda/v2/tests/helper" ) const ( diff --git a/tests/internals/events/events_test.go b/tests/internals/events/events_test.go index 1f791c64fac..6aed5d4a1d6 100644 --- a/tests/internals/events/events_test.go +++ b/tests/internals/events/events_test.go @@ -13,7 +13,7 @@ import ( "github.com/kedacore/keda/v2/pkg/common/message" "github.com/kedacore/keda/v2/pkg/eventreason" - . "github.com/kedacore/keda/v2/tests/helper" + . "github.com/kedacore/keda/v2/tests/helper" ) const ( From a1a41ab87b6932dd8c7e0406e038c75771c857f5 Mon Sep 17 00:00:00 2001 From: Shane Date: Thu, 12 Dec 2024 17:33:56 +0800 Subject: [PATCH 21/24] Update events_test.go Signed-off-by: Shane --- tests/internals/events/events_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/internals/events/events_test.go b/tests/internals/events/events_test.go index 6aed5d4a1d6..1f791c64fac 100644 --- a/tests/internals/events/events_test.go +++ b/tests/internals/events/events_test.go @@ -13,7 +13,7 @@ import ( "github.com/kedacore/keda/v2/pkg/common/message" "github.com/kedacore/keda/v2/pkg/eventreason" - . "github.com/kedacore/keda/v2/tests/helper" + . "github.com/kedacore/keda/v2/tests/helper" ) const ( From d8ab20337140993f105dd61bc5c14b081db7b37e Mon Sep 17 00:00:00 2001 From: Shane Date: Thu, 12 Dec 2024 17:34:47 +0800 Subject: [PATCH 22/24] Update cloudevent_source_test.go Signed-off-by: Shane --- tests/internals/cloudevent_source/cloudevent_source_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/internals/cloudevent_source/cloudevent_source_test.go b/tests/internals/cloudevent_source/cloudevent_source_test.go index 5b0ca00beb6..ccc84c85f4d 100644 --- a/tests/internals/cloudevent_source/cloudevent_source_test.go +++ b/tests/internals/cloudevent_source/cloudevent_source_test.go @@ -14,7 +14,7 @@ import ( "github.com/stretchr/testify/assert" "k8s.io/client-go/kubernetes" - . "github.com/kedacore/keda/v2/tests/helper" + . "github.com/kedacore/keda/v2/tests/helper" ) const ( From 3ca522fa542d54c0feaa2027a33e1f6bdde5e551 Mon Sep 17 00:00:00 2001 From: Shane Date: Thu, 12 Dec 2024 17:35:21 +0800 Subject: [PATCH 23/24] Update events_test.go Signed-off-by: Shane From 4260f77f2f08481070f1ae7b81dd6bf979922bde Mon Sep 17 00:00:00 2001 From: Shane Date: Thu, 12 Dec 2024 17:36:20 +0800 Subject: [PATCH 24/24] Update events_test.go Signed-off-by: Shane --- tests/internals/events/events_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/internals/events/events_test.go b/tests/internals/events/events_test.go index 1f791c64fac..ecf58a48d35 100644 --- a/tests/internals/events/events_test.go +++ b/tests/internals/events/events_test.go @@ -13,7 +13,7 @@ import ( "github.com/kedacore/keda/v2/pkg/common/message" "github.com/kedacore/keda/v2/pkg/eventreason" - . "github.com/kedacore/keda/v2/tests/helper" + . "github.com/kedacore/keda/v2/tests/helper" ) const (