Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add KEDAScalersInfo to display important information #6330

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@ New deprecation(s):
### Other

- **General**: Bump newrelic-client-go deps to 2.51.2 (latest) ([#6325](https://github.com/kedacore/keda/pull/6325))
- **General**: New eventreason KEDAScalersInfo to display important information ([#6328](https://github.com/kedacore/keda/pull/6328))


## v2.16.0
Expand Down
3 changes: 3 additions & 0 deletions pkg/eventreason/eventreason.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,9 @@ const (
// ScaledJobDeleted is for event when ScaledJob is deleted
ScaledJobDeleted = "ScaledJobDeleted"

// KEDAScalersInfo is for event when Scaler has additional info
KEDAScalersInfo = "KEDAScalerInfo"

// KEDAScalersStarted is for event when scalers watch started for ScaledObject or ScaledJob
KEDAScalersStarted = "KEDAScalersStarted"

Expand Down
13 changes: 7 additions & 6 deletions pkg/scalers/cpu_memory_scaler.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ type cpuMemoryScaler struct {
}

type cpuMemoryMetadata struct {
Type string `keda:"name=type, order=triggerMetadata, enum=Utilization;AverageValue, optional"`
Type string `keda:"name=type, order=triggerMetadata, enum=Utilization;AverageValue, optional, deprecatedAnnounce=The 'type' setting is DEPRECATED and will be removed in v2.18 - Use 'metricType' instead."`
Value string `keda:"name=value, order=triggerMetadata"`
ContainerName string `keda:"name=containerName, order=triggerMetadata, optional"`
AverageValue *resource.Quantity
Expand All @@ -33,19 +33,21 @@ type cpuMemoryMetadata struct {
func NewCPUMemoryScaler(resourceName v1.ResourceName, config *scalersconfig.ScalerConfig) (Scaler, error) {
logger := InitializeLogger(config, "cpu_memory_scaler")

meta, err := parseResourceMetadata(config, logger)
meta, err := parseResourceMetadata(config)
if err != nil {
return nil, fmt.Errorf("error parsing %s metadata: %w", resourceName, err)
}

return &cpuMemoryScaler{
scaler := &cpuMemoryScaler{
metadata: meta,
resourceName: resourceName,
logger: logger,
}, nil
}

return scaler, nil
}

func parseResourceMetadata(config *scalersconfig.ScalerConfig, logger logr.Logger) (cpuMemoryMetadata, error) {
func parseResourceMetadata(config *scalersconfig.ScalerConfig) (cpuMemoryMetadata, error) {
meta := cpuMemoryMetadata{}
err := config.TypedConfig(&meta)
if err != nil {
Expand All @@ -58,7 +60,6 @@ func parseResourceMetadata(config *scalersconfig.ScalerConfig, logger logr.Logge

// This is deprecated and can be removed later
if meta.Type != "" {
logger.Info("The 'type' setting is DEPRECATED and will be removed in v2.18 - Use 'metricType' instead.")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: does it make sense to keep both events as well as log messages regarding deprecated fields?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it make sense to keep both

switch meta.Type {
case "AverageValue":
meta.MetricType = v2.AverageValueMetricType
Expand Down
4 changes: 1 addition & 3 deletions pkg/scalers/cpu_memory_scaler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import (
"context"
"testing"

"github.com/go-logr/logr"
"github.com/stretchr/testify/assert"
v2 "k8s.io/api/autoscaling/v2"
v1 "k8s.io/api/core/v1"
Expand Down Expand Up @@ -43,13 +42,12 @@ var testCPUMemoryMetadata = []parseCPUMemoryMetadataTestData{
}

func TestCPUMemoryParseMetadata(t *testing.T) {
logger := logr.Discard()
for i, testData := range testCPUMemoryMetadata {
config := &scalersconfig.ScalerConfig{
TriggerMetadata: testData.metadata,
MetricType: testData.metricType,
}
_, err := parseResourceMetadata(config, logger)
_, err := parseResourceMetadata(config)
if err != nil && !testData.isError {
t.Errorf("Test case %d: Expected success but got error: %v", i, err)
}
Expand Down
9 changes: 5 additions & 4 deletions pkg/scalers/ibmmq_scaler.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ type ibmmqMetadata struct {
Username string `keda:"name=username, order=authParams;resolvedEnv;triggerMetadata"`
Password string `keda:"name=password, order=authParams;resolvedEnv;triggerMetadata"`
UnsafeSsl bool `keda:"name=unsafeSsl, order=triggerMetadata, default=false"`
TLS bool `keda:"name=tls, order=triggerMetadata, default=false"` // , deprecated=use unsafeSsl instead
TLS bool `keda:"name=tls, order=triggerMetadata, default=false, deprecatedAnnounce=The 'tls' setting is DEPRECATED and will be removed in v2.18 - Use 'unsafeSsl' instead"`
CA string `keda:"name=ca, order=authParams, optional"`
Cert string `keda:"name=cert, order=authParams, optional"`
Key string `keda:"name=key, order=authParams, optional"`
Expand Down Expand Up @@ -92,7 +92,6 @@ func NewIBMMQScaler(config *scalersconfig.ScalerConfig) (Scaler, error) {

// TODO: DEPRECATED to be removed in v2.18
if meta.TLS {
logger.Info("The 'tls' setting is DEPRECATED and will be removed in v2.18 - Use 'unsafeSsl' instead")
meta.UnsafeSsl = meta.TLS
}

Expand All @@ -106,12 +105,14 @@ func NewIBMMQScaler(config *scalersconfig.ScalerConfig) (Scaler, error) {
httpClient.Transport = kedautil.CreateHTTPTransportWithTLSConfig(tlsConfig)
}

return &ibmmqScaler{
scaler := &ibmmqScaler{
metricType: metricType,
metadata: meta,
httpClient: httpClient,
logger: logger,
}, nil
}

return scaler, nil
}

func (s *ibmmqScaler) Close(context.Context) error {
Expand Down
8 changes: 8 additions & 0 deletions pkg/scalers/scalersconfig/scalersconfig.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ import (
"time"

v2 "k8s.io/api/autoscaling/v2"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/client-go/tools/record"

kedav1alpha1 "github.com/kedacore/keda/v2/apis/keda/v1alpha1"
)
Expand Down Expand Up @@ -68,4 +70,10 @@ type ScalerConfig struct {

// When we use the scaler for composite scaler, we shouldn't require the value because it'll be ignored
AsMetricSource bool

// For events
Recorder record.EventRecorder

// ScaledObjct
ScaledObject runtime.Object
}
38 changes: 29 additions & 9 deletions pkg/scalers/scalersconfig/typed_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,9 @@ import (

"golang.org/x/exp/maps"
"golang.org/x/exp/slices"
corev1 "k8s.io/api/core/v1"

"github.com/kedacore/keda/v2/pkg/eventreason"
)

// CustomValidator is an interface that can be implemented to validate the configuration of the typed config
Expand Down Expand Up @@ -67,15 +70,16 @@ const (

// field tag parameters
const (
optionalTag = "optional"
deprecatedTag = "deprecated"
defaultTag = "default"
orderTag = "order"
nameTag = "name"
enumTag = "enum"
exclusiveSetTag = "exclusiveSet"
rangeTag = "range"
separatorTag = "separator"
optionalTag = "optional"
deprecatedTag = "deprecated"
deprecatedAnnounceTag = "deprecatedAnnounce"
defaultTag = "default"
orderTag = "order"
nameTag = "name"
enumTag = "enum"
exclusiveSetTag = "exclusiveSet"
rangeTag = "range"
separatorTag = "separator"
)

// Params is a struct that represents the parameter list that can be used in the keda tag
Expand All @@ -101,6 +105,10 @@ type Params struct {
// as an error and the DeprecatedMessage should be returned to the user
Deprecated string

// DeprecatedAnnounce is the 'deprecatedAnnounce' tag parameter, if set this will trigger
// an info event with the deprecation message
DeprecatedAnnounce string

// Enum is the 'enum' tag parameter defining the list of possible values for the parameter
Enum []string

Expand Down Expand Up @@ -195,6 +203,12 @@ func (sc *ScalerConfig) setValue(field reflect.Value, params Params) error {
if exists && params.IsDeprecated() {
return fmt.Errorf("parameter %q is deprecated%v", params.Name(), params.DeprecatedMessage())
}
if exists && params.DeprecatedAnnounce != "" {
if sc.Recorder != nil {
fmt.Print(params.DeprecatedAnnounce)
sc.Recorder.Event(sc.ScaledObject, corev1.EventTypeNormal, eventreason.KEDAScalersInfo, params.DeprecatedAnnounce)
}
}
if !exists && params.Default != "" {
exists = true
valFromConfig = params.Default
Expand Down Expand Up @@ -480,6 +494,12 @@ func paramsFromTag(tag string, field reflect.StructField) (Params, error) {
} else {
params.Deprecated = strings.TrimSpace(tsplit[1])
}
case deprecatedAnnounceTag:
if len(tsplit) == 1 {
params.DeprecatedAnnounce = deprecatedAnnounceTag
} else {
params.DeprecatedAnnounce = strings.TrimSpace(tsplit[1])
}
case defaultTag:
if len(tsplit) > 1 {
params.Default = strings.TrimSpace(tsplit[1])
Expand Down
48 changes: 48 additions & 0 deletions pkg/scalers/scalersconfig/typed_config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
"testing"

. "github.com/onsi/gomega"
"k8s.io/apimachinery/pkg/runtime"
)

// TestBasicTypedConfig tests the basic types for typed config
Expand Down Expand Up @@ -583,3 +584,50 @@ func TestMultiName(t *testing.T) {
Expect(err).To(BeNil())
Expect(ts.Property).To(Equal("bbb"))
}

// TestDeprecatedAnnounce tests the deprecatedAnnounce tag
func TestDeprecatedAnnounce(t *testing.T) {
RegisterTestingT(t)

// Create a mock recorder to capture the event
mockRecorder := &MockEventRecorder{}

sc := &ScalerConfig{
TriggerMetadata: map[string]string{
"oldParam": "value1",
},
Recorder: mockRecorder,
}

type testStruct struct {
OldParam string `keda:"name=oldParam, order=triggerMetadata, deprecatedAnnounce=This parameter is deprecated. Use newParam instead"`
}

ts := testStruct{}
err := sc.TypedConfig(&ts)
Expect(err).To(BeNil())
Expect(ts.OldParam).To(Equal("value1"))

// Verify that the deprecation event was recorded
Expect(mockRecorder.EventCalled).To(BeTrue())
Expect(mockRecorder.Message).To(Equal("This parameter is deprecated. Use newParam instead"))
}

// MockEventRecorder is a mock implementation of record.EventRecorder
type MockEventRecorder struct {
EventCalled bool
Message string
}

func (m *MockEventRecorder) Event(object runtime.Object, eventtype, reason, message string) {
m.EventCalled = true
m.Message = message
}

func (m *MockEventRecorder) Eventf(object runtime.Object, eventtype, reason, messageFmt string, args ...interface{}) {
// Not needed
}

func (m *MockEventRecorder) AnnotatedEventf(object runtime.Object, annotations map[string]string, eventtype, reason, messageFmt string, args ...interface{}) {
// Not needed
}
2 changes: 2 additions & 0 deletions pkg/scaling/scalers_builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,8 @@ func (h *scaleHandler) buildScalers(ctx context.Context, withTriggers *kedav1alp
TriggerIndex: triggerIndex,
MetricType: trigger.MetricType,
AsMetricSource: asMetricSource,
ScaledObject: withTriggers,
Recorder: h.recorder,
TriggerUniqueKey: fmt.Sprintf("%s-%s-%s-%d", withTriggers.Kind, withTriggers.Namespace, withTriggers.Name, triggerIndex),
}

Expand Down
Loading