diff --git a/pkg/scalers/cpu_memory_scaler.go b/pkg/scalers/cpu_memory_scaler.go index da5119f3ec0..ce845414966 100644 --- a/pkg/scalers/cpu_memory_scaler.go +++ b/pkg/scalers/cpu_memory_scaler.go @@ -15,25 +15,38 @@ import ( ) type cpuMemoryScaler struct { - metadata *cpuMemoryMetadata + metadata cpuMemoryMetadata resourceName v1.ResourceName logger logr.Logger } type cpuMemoryMetadata struct { - Type v2.MetricTargetType + Type string `keda:"name=type, order=triggerMetadata, enum=Utilization;AverageValue, optional"` + Value string `keda:"name=value, order=triggerMetadata"` + ContainerName string `keda:"name=containerName, order=triggerMetadata, optional"` AverageValue *resource.Quantity AverageUtilization *int32 - ContainerName string + MetricType v2.MetricTargetType +} + +func (m *cpuMemoryMetadata) Validate() error { + return nil } // NewCPUMemoryScaler creates a new cpuMemoryScaler func NewCPUMemoryScaler(resourceName v1.ResourceName, config *scalersconfig.ScalerConfig) (Scaler, error) { logger := InitializeLogger(config, "cpu_memory_scaler") - meta, parseErr := parseResourceMetadata(config, logger) - if parseErr != nil { - return nil, fmt.Errorf("error parsing %s metadata: %w", resourceName, parseErr) + meta, err := parseResourceMetadata(config, logger) + if err != nil { + return nil, fmt.Errorf("error parsing %s metadata: %w", resourceName, err) + } + + if err := meta.Validate(); err != nil { + if meta.MetricType == "" { + return nil, fmt.Errorf("metricType is required") + } + return nil, fmt.Errorf("validation error: %w", err) } return &cpuMemoryScaler{ @@ -43,48 +56,56 @@ func NewCPUMemoryScaler(resourceName v1.ResourceName, config *scalersconfig.Scal }, nil } -func parseResourceMetadata(config *scalersconfig.ScalerConfig, logger logr.Logger) (*cpuMemoryMetadata, error) { - meta := &cpuMemoryMetadata{} - var value string - var ok bool - value, ok = config.TriggerMetadata["type"] - switch { - case ok && value != "" && config.MetricType != "": - return nil, fmt.Errorf("only one of trigger.metadata.type or trigger.metricType should be defined") - case ok && value != "": - logger.V(0).Info("trigger.metadata.type is deprecated in favor of trigger.metricType") - meta.Type = v2.MetricTargetType(value) - case config.MetricType != "": - meta.Type = config.MetricType - default: - return nil, fmt.Errorf("no type given in neither trigger.metadata.type or trigger.metricType") +func parseResourceMetadata(config *scalersconfig.ScalerConfig, logger logr.Logger) (cpuMemoryMetadata, error) { + meta := cpuMemoryMetadata{} + err := config.TypedConfig(&meta) + if err != nil { + return meta, err + } + + if config.MetricType != "" { + meta.MetricType = config.MetricType } - if value, ok = config.TriggerMetadata["value"]; !ok || value == "" { - return nil, fmt.Errorf("no value given") + // 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.") + switch meta.Type { + case "AverageValue": + meta.MetricType = v2.AverageValueMetricType + case "Utilization": + meta.MetricType = v2.UtilizationMetricType + default: + return meta, fmt.Errorf("unknown metric type: %s, allowed values are 'Utilization' or 'AverageValue'", meta.Type) + } } - switch meta.Type { + + switch meta.MetricType { case v2.AverageValueMetricType: - averageValueQuantity := resource.MustParse(value) + averageValueQuantity := resource.MustParse(meta.Value) meta.AverageValue = &averageValueQuantity case v2.UtilizationMetricType: - valueNum, err := strconv.ParseInt(value, 10, 32) + utilizationNum, err := parseUtilization(meta.Value) if err != nil { - return nil, err + return meta, err } - utilizationNum := int32(valueNum) - meta.AverageUtilization = &utilizationNum + meta.AverageUtilization = utilizationNum default: - return nil, fmt.Errorf("unsupported metric type, allowed values are 'Utilization' or 'AverageValue'") - } - - if value, ok = config.TriggerMetadata["containerName"]; ok && value != "" { - meta.ContainerName = value + return meta, fmt.Errorf("unknown metric type: %s, allowed values are 'Utilization' or 'AverageValue'", string(meta.MetricType)) } return meta, nil } +func parseUtilization(value string) (*int32, error) { + valueNum, err := strconv.ParseInt(value, 10, 32) + if err != nil { + return nil, err + } + utilizationNum := int32(valueNum) + return &utilizationNum, nil +} + // Close no need for cpuMemory scaler func (s *cpuMemoryScaler) Close(context.Context) error { return nil @@ -92,13 +113,14 @@ func (s *cpuMemoryScaler) Close(context.Context) error { // GetMetricSpecForScaling returns the metric spec for the HPA func (s *cpuMemoryScaler) GetMetricSpecForScaling(context.Context) []v2.MetricSpec { - var metricSpec v2.MetricSpec + metricType := s.metadata.MetricType + var metricSpec v2.MetricSpec if s.metadata.ContainerName != "" { containerCPUMemoryMetric := &v2.ContainerResourceMetricSource{ Name: s.resourceName, Target: v2.MetricTarget{ - Type: s.metadata.Type, + Type: metricType, AverageUtilization: s.metadata.AverageUtilization, AverageValue: s.metadata.AverageValue, }, @@ -109,7 +131,7 @@ func (s *cpuMemoryScaler) GetMetricSpecForScaling(context.Context) []v2.MetricSp cpuMemoryMetric := &v2.ResourceMetricSource{ Name: s.resourceName, Target: v2.MetricTarget{ - Type: s.metadata.Type, + Type: metricType, AverageUtilization: s.metadata.AverageUtilization, AverageValue: s.metadata.AverageValue, }, diff --git a/pkg/scalers/cpu_memory_scaler_test.go b/pkg/scalers/cpu_memory_scaler_test.go index 81f7ea9df9a..78f662de247 100644 --- a/pkg/scalers/cpu_memory_scaler_test.go +++ b/pkg/scalers/cpu_memory_scaler_test.go @@ -18,7 +18,6 @@ type parseCPUMemoryMetadataTestData struct { isError bool } -// A complete valid metadata example for reference var validCPUMemoryMetadata = map[string]string{ "type": "Utilization", "value": "50", @@ -44,17 +43,18 @@ var testCPUMemoryMetadata = []parseCPUMemoryMetadataTestData{ } func TestCPUMemoryParseMetadata(t *testing.T) { - for _, testData := range testCPUMemoryMetadata { + logger := logr.Discard() + for i, testData := range testCPUMemoryMetadata { config := &scalersconfig.ScalerConfig{ TriggerMetadata: testData.metadata, MetricType: testData.metricType, } - _, err := parseResourceMetadata(config, logr.Discard()) + _, err := parseResourceMetadata(config, logger) if err != nil && !testData.isError { - t.Error("Expected success but got error", err) + t.Errorf("Test case %d: Expected success but got error: %v", i, err) } if testData.isError && err == nil { - t.Error("Expected error but got success") + t.Errorf("Test case %d: Expected error but got success", i) } } }