From 24c4a42a72970e9c1f1fdf264cf61d3fd0a93e12 Mon Sep 17 00:00:00 2001 From: Shane Date: Tue, 10 Dec 2024 20:19:34 +0800 Subject: [PATCH] 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) {