Merge branch 'main' into region_in_aws_config_cache

Signed-off-by: Maksymilian Boguń <[email protected]>

CHANGELOG.md

@@ -72,6 +72,7 @@ Here is an overview of all new **experimental** features:
 
 ### Fixes
 
+- **General**: Paused ScaledObject count is reported correctly after operator restart ([#6321](https://github.com/kedacore/keda/issues/6321))
 - **AWS Scalers**: Add AWS region to the AWS Config Cache key ([#6128](https://github.com/kedacore/keda/issues/6128))
 
 ### Deprecations
@@ -88,7 +89,8 @@ New deprecation(s):
 
 ### Other
 
-- TODO ([#XXX](https://github.com/kedacore/keda/issues/XXX))
+- **General**: Bump newrelic-client-go deps to 2.51.2 (latest) ([#6325](https://github.com/kedacore/keda/pull/6325))
+
 
 ## v2.16.0
 

controllers/keda/scaledobject_controller.go

@@ -209,6 +209,8 @@ func (r *ScaledObjectReconciler) Reconcile(ctx context.Context, req ctrl.Request
 		conditions.SetFallbackCondition(metav1.ConditionFalse, "NoFallbackFound", "No fallbacks are active on this scaled object")
 	}
 
+	metricscollector.RecordScaledObjectPaused(scaledObject.Namespace, scaledObject.Name, conditions.GetPausedCondition().Status == metav1.ConditionTrue)
+
 	if err := kedastatus.SetStatusConditions(ctx, r.Client, reqLogger, scaledObject, &conditions); err != nil {
 		r.EventEmitter.Emit(scaledObject, req.NamespacedName.Namespace, corev1.EventTypeWarning, eventingv1alpha1.ScaledObjectFailedType, eventreason.ScaledObjectUpdateFailed, err.Error())
 		return ctrl.Result{}, err
@@ -246,12 +248,10 @@ func (r *ScaledObjectReconciler) reconcileScaledObject(ctx context.Context, logg
 				return msg, err
 			}
 			conditions.SetPausedCondition(metav1.ConditionTrue, kedav1alpha1.ScaledObjectConditionPausedReason, msg)
-			metricscollector.RecordScaledObjectPaused(scaledObject.Namespace, scaledObject.Name, true)
 			return msg, nil
 		}
 	} else if conditions.GetPausedCondition().Status == metav1.ConditionTrue {
 		conditions.SetPausedCondition(metav1.ConditionFalse, "ScaledObjectUnpaused", "pause annotation removed for ScaledObject")
-		metricscollector.RecordScaledObjectPaused(scaledObject.Namespace, scaledObject.Name, false)
 	}
 
 	// Check scale target Name is specified

go.mod

@@ -66,7 +66,7 @@ require (
 	github.com/microsoft/ApplicationInsights-Go v0.4.4
 	github.com/microsoft/azure-devops-go-api/azuredevops v1.0.0-b5
 	github.com/mitchellh/hashstructure v1.1.0
-	github.com/newrelic/newrelic-client-go v1.1.0
+	github.com/newrelic/newrelic-client-go/v2 v2.51.2
 	github.com/onsi/ginkgo/v2 v2.21.0
 	github.com/onsi/gomega v1.35.1
 	github.com/open-policy-agent/cert-controller v0.12.0

go.sum

@@ -2173,6 +2173,8 @@ github.com/neelance/astrewrite v0.0.0-20160511093645-99348263ae86/go.mod h1:kHJE
 github.com/neelance/sourcemap v0.0.0-20200213170602-2833bce08e4c/go.mod h1:Qr6/a/Q4r9LP1IltGz7tA7iOK1WonHEYhu1HRBA7ZiM=
 github.com/newrelic/newrelic-client-go v1.1.0 h1:aflNjzQ21c+2GwBVh+UbAf9lznkRfCcVABoc5UM4IXw=
 github.com/newrelic/newrelic-client-go v1.1.0/go.mod h1:RYMXt7hgYw7nzuXIGd2BH0F1AivgWw7WrBhNBQZEB4k=
+github.com/newrelic/newrelic-client-go/v2 v2.51.2 h1:Xf+M0NuZuIuxqG48zYoqyIdQL514j2J1c+kNVYajcYI=
+github.com/newrelic/newrelic-client-go/v2 v2.51.2/go.mod h1:+RRjI3nDGWT3kLm9Oi3QxpBm70uu8q1upEHBVWCZFpo=
 github.com/nxadm/tail v1.4.8 h1:nPr65rt6Y5JFSKQO7qToXr7pePgD6Gwiw05lkbyAQTE=
 github.com/nxadm/tail v1.4.8/go.mod h1:+ncqLTQzXmGhMZNUePPaPqPvBxHAIsmXswZKocGu+AU=
 github.com/oapi-codegen/runtime v1.1.1 h1:EXLHh0DXIJnWhdRPN2w4MXAzFyE4CskzhNLUmtpMYro=

pkg/scalers/newrelic_scaler.go

@@ -3,12 +3,10 @@ package scalers
 import (
 	"context"
 	"fmt"
-	"log"
-	"strconv"
 
 	"github.com/go-logr/logr"
-	"github.com/newrelic/newrelic-client-go/newrelic"
-	"github.com/newrelic/newrelic-client-go/pkg/nrdb"
+	"github.com/newrelic/newrelic-client-go/v2/newrelic"
+	"github.com/newrelic/newrelic-client-go/v2/pkg/nrdb"
 	v2 "k8s.io/api/autoscaling/v2"
 	"k8s.io/metrics/pkg/apis/external_metrics"
 
@@ -17,31 +15,25 @@ import (
 )
 
 const (
-	account           = "account"
-	queryKeyParamater = "queryKey"
-	regionParameter   = "region"
-	nrql              = "nrql"
-	threshold         = "threshold"
-	noDataError       = "noDataError"
-	scalerName        = "new-relic"
+	scalerName = "new-relic"
 )
 
 type newrelicScaler struct {
 	metricType v2.MetricTargetType
-	metadata   *newrelicMetadata
+	metadata   newrelicMetadata
 	nrClient   *newrelic.NewRelic
 	logger     logr.Logger
 }
 
 type newrelicMetadata struct {
-	account             int
-	region              string
-	queryKey            string
-	noDataError         bool
-	nrql                string
-	threshold           float64
-	activationThreshold float64
-	triggerIndex        int
+	Account             int     `keda:"name=account,             order=authParams;triggerMetadata"`
+	Region              string  `keda:"name=region,              order=authParams;triggerMetadata, default=US"`
+	QueryKey            string  `keda:"name=queryKey,            order=authParams;triggerMetadata"`
+	NoDataError         bool    `keda:"name=noDataError,         order=triggerMetadata, default=false"`
+	NRQL                string  `keda:"name=nrql,                order=triggerMetadata"`
+	Threshold           float64 `keda:"name=threshold,           order=triggerMetadata"`
+	ActivationThreshold float64 `keda:"name=activationThreshold, order=triggerMetadata, default=0"`
+	TriggerIndex        int
 }
 
 func NewNewRelicScaler(config *scalersconfig.ScalerConfig) (Scaler, error) {
@@ -52,128 +44,69 @@ func NewNewRelicScaler(config *scalersconfig.ScalerConfig) (Scaler, error) {
 
 	logger := InitializeLogger(config, fmt.Sprintf("%s_scaler", scalerName))
 
-	meta, err := parseNewRelicMetadata(config, logger)
+	meta, err := parseNewRelicMetadata(config)
 	if err != nil {
 		return nil, fmt.Errorf("error parsing %s metadata: %w", scalerName, err)
 	}
 
 	nrClient, err := newrelic.New(
-		newrelic.ConfigPersonalAPIKey(meta.queryKey),
-		newrelic.ConfigRegion(meta.region))
-
+		newrelic.ConfigPersonalAPIKey(meta.QueryKey),
+		newrelic.ConfigRegion(meta.Region))
 	if err != nil {
-		log.Fatal("error initializing client:", err)
+		return nil, fmt.Errorf("error initializing client: %w", err)
 	}
 
-	logMsg := fmt.Sprintf("Initializing New Relic Scaler (account %d in region %s)", meta.account, meta.region)
-
-	logger.Info(logMsg)
+	logger.Info(fmt.Sprintf("Initializing New Relic Scaler (account %d in region %s)", meta.Account, meta.Region))
 
 	return &newrelicScaler{
 		metricType: metricType,
 		metadata:   meta,
 		nrClient:   nrClient,
-		logger:     logger}, nil
+		logger:     logger,
+	}, nil
 }
 
-func parseNewRelicMetadata(config *scalersconfig.ScalerConfig, logger logr.Logger) (*newrelicMetadata, error) {
+func parseNewRelicMetadata(config *scalersconfig.ScalerConfig) (newrelicMetadata, error) {
 	meta := newrelicMetadata{}
-	var err error
-
-	val, err := GetFromAuthOrMeta(config, account)
-	if err != nil {
-		return nil, err
-	}
-
-	t, err := strconv.Atoi(val)
-	if err != nil {
-		return nil, fmt.Errorf("error parsing %s: %w", account, err)
-	}
-	meta.account = t
-
-	if val, ok := config.TriggerMetadata[nrql]; ok && val != "" {
-		meta.nrql = val
-	} else {
-		return nil, fmt.Errorf("no %s given", nrql)
-	}
-
-	queryKey, err := GetFromAuthOrMeta(config, queryKeyParamater)
-	if err != nil {
-		return nil, err
-	}
-	meta.queryKey = queryKey
-
-	region, err := GetFromAuthOrMeta(config, regionParameter)
+	err := config.TypedConfig(&meta)
 	if err != nil {
-		region = "US"
-		logger.Info("Using default 'US' region")
+		return meta, fmt.Errorf("error parsing newrelic metadata: %w", err)
 	}
-	meta.region = region
 
-	if val, ok := config.TriggerMetadata[threshold]; ok && val != "" {
-		t, err := strconv.ParseFloat(val, 64)
-		if err != nil {
-			return nil, fmt.Errorf("error parsing %s", threshold)
-		}
-		meta.threshold = t
-	} else {
-		if config.AsMetricSource {
-			meta.threshold = 0
-		} else {
-			return nil, fmt.Errorf("missing %s value", threshold)
-		}
+	if config.AsMetricSource {
+		meta.Threshold = 0
 	}
 
-	meta.activationThreshold = 0
-	if val, ok := config.TriggerMetadata["activationThreshold"]; ok {
-		activationThreshold, err := strconv.ParseFloat(val, 64)
-		if err != nil {
-			return nil, fmt.Errorf("queryValue parsing error %w", err)
-		}
-		meta.activationThreshold = activationThreshold
-	}
-
-	// If Query Return an Empty Data , shall we treat it as an error or not
-	// default is NO error is returned when query result is empty/no data
-	if val, ok := config.TriggerMetadata[noDataError]; ok {
-		noDataError, err := strconv.ParseBool(val)
-		if err != nil {
-			return nil, fmt.Errorf("noDataError has invalid value")
-		}
-		meta.noDataError = noDataError
-	} else {
-		meta.noDataError = false
-	}
-	meta.triggerIndex = config.TriggerIndex
-	return &meta, nil
+	meta.TriggerIndex = config.TriggerIndex
+	return meta, nil
 }
 
 func (s *newrelicScaler) Close(context.Context) error {
 	return nil
 }
 
 func (s *newrelicScaler) executeNewRelicQuery(ctx context.Context) (float64, error) {
-	nrdbQuery := nrdb.NRQL(s.metadata.nrql)
-	resp, err := s.nrClient.Nrdb.QueryWithContext(ctx, s.metadata.account, nrdbQuery)
+	nrdbQuery := nrdb.NRQL(s.metadata.NRQL)
+	resp, err := s.nrClient.Nrdb.QueryWithContext(ctx, s.metadata.Account, nrdbQuery)
 	if err != nil {
-		return 0, fmt.Errorf("error running NRQL %s (%s)", s.metadata.nrql, err.Error())
+		return 0, fmt.Errorf("error running NRQL %s: %w", s.metadata.NRQL, err)
 	}
-	// Check for empty results set, as New Relic lib does not report these as errors
+
 	if len(resp.Results) == 0 {
-		if s.metadata.noDataError {
-			return 0, fmt.Errorf("query return no results %s", s.metadata.nrql)
+		if s.metadata.NoDataError {
+			return 0, fmt.Errorf("query returned no results: %s", s.metadata.NRQL)
 		}
 		return 0, nil
 	}
 	// Only use the first result from the query, as the query should not be multi row
 	for _, v := range resp.Results[0] {
-		val, ok := v.(float64)
-		if ok {
+		if val, ok := v.(float64); ok {
 			return val, nil
 		}
 	}
-	if s.metadata.noDataError {
-		return 0, fmt.Errorf("query return no results %s", s.metadata.nrql)
+
+	if s.metadata.NoDataError {
+		return 0, fmt.Errorf("query returned no numeric results: %s", s.metadata.NRQL)
 	}
 	return 0, nil
 }
@@ -186,21 +119,17 @@ func (s *newrelicScaler) GetMetricsAndActivity(ctx context.Context, metricName s
 	}
 
 	metric := GenerateMetricInMili(metricName, val)
-
-	return []external_metrics.ExternalMetricValue{metric}, val > s.metadata.activationThreshold, nil
+	return []external_metrics.ExternalMetricValue{metric}, val > s.metadata.ActivationThreshold, nil
 }
 
 func (s *newrelicScaler) GetMetricSpecForScaling(context.Context) []v2.MetricSpec {
 	metricName := kedautil.NormalizeString(scalerName)
-
 	externalMetric := &v2.ExternalMetricSource{
 		Metric: v2.MetricIdentifier{
-			Name: GenerateMetricNameWithIndex(s.metadata.triggerIndex, metricName),
+			Name: GenerateMetricNameWithIndex(s.metadata.TriggerIndex, metricName),
 		},
-		Target: GetMetricTargetMili(s.metricType, s.metadata.threshold),
-	}
-	metricSpec := v2.MetricSpec{
-		External: externalMetric, Type: externalMetricType,
+		Target: GetMetricTargetMili(s.metricType, s.metadata.Threshold),
 	}
+	metricSpec := v2.MetricSpec{External: externalMetric, Type: externalMetricType}
 	return []v2.MetricSpec{metricSpec}
 }

pkg/scalers/newrelic_scaler_test.go

@@ -6,6 +6,7 @@ import (
 	"testing"
 
 	"github.com/go-logr/logr"
+	v2 "k8s.io/api/autoscaling/v2"
 
 	"github.com/kedacore/keda/v2/pkg/scalers/scalersconfig"
 )
@@ -26,7 +27,7 @@ var testNewRelicMetadata = []parseNewRelicMetadataTestData{
 	{map[string]string{}, map[string]string{}, true},
 	// all properly formed
 	{map[string]string{"account": "0", "threshold": "100", "queryKey": "somekey", "nrql": "SELECT average(cpuUsedCores) as result FROM K8sContainerSample WHERE containerName='coredns'"}, map[string]string{}, false},
-	// all properly formed
+	// all properly formed with region and activationThreshold
 	{map[string]string{"account": "0", "region": "EU", "threshold": "100", "activationThreshold": "20", "queryKey": "somekey", "nrql": "SELECT average(cpuUsedCores) as result FROM K8sContainerSample WHERE containerName='coredns'"}, map[string]string{}, false},
 	// account passed via auth params
 	{map[string]string{"region": "EU", "threshold": "100", "queryKey": "somekey", "nrql": "SELECT average(cpuUsedCores) as result FROM K8sContainerSample WHERE containerName='coredns'"}, map[string]string{"account": "0"}, false},
@@ -48,7 +49,7 @@ var testNewRelicMetadata = []parseNewRelicMetadataTestData{
 	{map[string]string{"account": "0", "threshold": "100", "queryKey": "somekey"}, map[string]string{}, true},
 	// noDataError invalid value
 	{map[string]string{"account": "0", "threshold": "100", "queryKey": "somekey", "noDataError": "invalid", "nrql": "SELECT average(cpuUsedCores) as result FROM K8sContainerSample WHERE containerName='coredns'"}, map[string]string{}, true},
-	// noDataError valid value
+	// noDataError valid values
 	{map[string]string{"account": "0", "threshold": "100", "queryKey": "somekey", "noDataError": "true", "nrql": "SELECT average(cpuUsedCores) as result FROM K8sContainerSample WHERE containerName='coredns'"}, map[string]string{}, false},
 	{map[string]string{"account": "0", "threshold": "100", "queryKey": "somekey", "noDataError": "false", "nrql": "SELECT average(cpuUsedCores) as result FROM K8sContainerSample WHERE containerName='coredns'"}, map[string]string{}, false},
 	{map[string]string{"account": "0", "threshold": "100", "queryKey": "somekey", "noDataError": "0", "nrql": "SELECT average(cpuUsedCores) as result FROM K8sContainerSample WHERE containerName='coredns'"}, map[string]string{}, false},
@@ -61,27 +62,39 @@ var newrelicMetricIdentifiers = []newrelicMetricIdentifier{
 }
 
 func TestNewRelicParseMetadata(t *testing.T) {
-	for _, testData := range testNewRelicMetadata {
-		_, err := parseNewRelicMetadata(&scalersconfig.ScalerConfig{TriggerMetadata: testData.metadata, AuthParams: testData.authParams}, logr.Discard())
-		if err != nil && !testData.isError {
-			fmt.Printf("X: %s", testData.metadata)
-			t.Error("Expected success but got error", err)
-		}
-		if testData.isError && err == nil {
-			fmt.Printf("X: %s", testData.metadata)
-			t.Error("Expected error but got success")
-		}
+	for i, testData := range testNewRelicMetadata {
+		t.Run(fmt.Sprintf("test case %d", i), func(t *testing.T) {
+			_, err := parseNewRelicMetadata(&scalersconfig.ScalerConfig{
+				TriggerMetadata: testData.metadata,
+				AuthParams:      testData.authParams,
+			})
+			if err != nil && !testData.isError {
+				t.Errorf("Test case %d: Expected success but got error: %v\nMetadata: %v\nAuthParams: %v",
+					i, err, testData.metadata, testData.authParams)
+			}
+			if testData.isError && err == nil {
+				t.Errorf("Test case %d: Expected error but got success\nMetadata: %v\nAuthParams: %v",
+					i, testData.metadata, testData.authParams)
+			}
+		})
 	}
 }
+
 func TestNewRelicGetMetricSpecForScaling(t *testing.T) {
 	for _, testData := range newrelicMetricIdentifiers {
-		meta, err := parseNewRelicMetadata(&scalersconfig.ScalerConfig{TriggerMetadata: testData.metadataTestData.metadata, AuthParams: testData.metadataTestData.authParams, TriggerIndex: testData.triggerIndex}, logr.Discard())
+		meta, err := parseNewRelicMetadata(&scalersconfig.ScalerConfig{
+			TriggerMetadata: testData.metadataTestData.metadata,
+			AuthParams:      testData.metadataTestData.authParams,
+			TriggerIndex:    testData.triggerIndex,
+		})
 		if err != nil {
 			t.Fatal("Could not parse metadata:", err)
 		}
 		mockNewRelicScaler := newrelicScaler{
-			metadata: meta,
-			nrClient: nil,
+			metadata:   meta,
+			nrClient:   nil,
+			logger:     logr.Discard(),
+			metricType: v2.AverageValueMetricType,
 		}
 
 		metricSpec := mockNewRelicScaler.GetMetricSpecForScaling(context.Background())

pkg/scalers/scalersconfig/typed_config.go

@@ -396,6 +396,14 @@ func setConfigValueHelper(params Params, valFromConfig string, field reflect.Val
 	if field.Kind() == reflect.Slice {
 		return setConfigValueSlice(params, valFromConfig, field)
 	}
+	if field.Kind() == reflect.Bool {
+		boolVal, err := strconv.ParseBool(valFromConfig)
+		if err != nil {
+			return fmt.Errorf("unable to parse boolean value %q: %w", valFromConfig, err)
+		}
+		field.SetBool(boolVal)
+		return nil
+	}
 	if field.CanInterface() {
 		ifc := reflect.New(field.Type()).Interface()
 		if err := json.Unmarshal([]byte(valFromConfig), &ifc); err != nil {