Fix validation error when specifying multiple metrics

[homme]

This fixes an issue where the loop iterator variable for queries references the last specified MetricStat in the case of multiple metrics. The issue is evident in the following ExternalMetric:

apiVersion: metrics.aws/v1alpha1
kind: ExternalMetric
metadata:
  creationTimestamp: null
  name: rds-db-test
  namespace: default
spec:
  name: rds-db-test
  queries:
  - id: rds_db_test
    metricStat:
      metric:
        dimensions:
        - name: DBInstanceIdentifier
          value: foobar
        metricName: CPUUtilization
        namespace: AWS/RDS
      period: 600
      stat: Average
      unit: Percent
    returnData: false
  - expression: AVG(METRICS())
    id: rds_db_avg_test
    returnData: true
  resource:
    resource: deployment

This produces the following error:

kubectl get --raw "/apis/external.metrics.k8s.io/v1beta1/namespaces/default/rds-db-test"
Error from server (BadRequest): InvalidParameter: 3 validation error(s) found.
- minimum field value of 1, GetMetricDataInput.MetricDataQueries[0].MetricStat.Period.
- minimum field size of 1, GetMetricDataInput.MetricDataQueries[0].MetricStat.Metric.MetricName.
- minimum field size of 1, GetMetricDataInput.MetricDataQueries[0].MetricStat.Metric.Namespace.

This can also be replicated locally with the following program:

package main

import (
	"flag"
	"fmt"
	"log"

	"github.com/awslabs/k8s-cloudwatch-adapter/pkg/aws"
	"github.com/awslabs/k8s-cloudwatch-adapter/pkg/config"
)

func main() {
	flag.Parse()
	c := aws.NewCloudWatchClient()
	queries := []config.MetricDataQuery{
		{
			ID: "rds_db_test",
			MetricStat: config.MetricStat{
				Metric: config.Metric{
					Dimensions: []config.Dimension{
						{
							Name:  "DBInstanceIdentifier",
							Value: "foobar",
						},
					},
					MetricName: "CPUUtilization",
					Namespace:  "AWS/RDS",
				},
				Period: 600,
				Stat:   "Average",
				Unit:   "Percent",
			},
			ReturnData: false,
		},
		{
			ID:         "rds_db_avg_test",
			Expression: "AVG(METRICS())",
			ReturnData: true,
		},
	}
	r, err := c.Query(queries)
	if err != nil {
		log.Fatal(err)
	}
	fmt.Println(r)
}

Running this produces the following validation error without the fix:

E0807 13:22:22.033542   18945 client.go:98] err: InvalidParameter: 3 validation error(s) found.
- minimum field value of 1, GetMetricDataInput.MetricDataQueries[0].MetricStat.Period.
- minimum field size of 1, GetMetricDataInput.MetricDataQueries[0].MetricStat.Metric.MetricName.
- minimum field size of 1, GetMetricDataInput.MetricDataQueries[0].MetricStat.Metric.Namespace.

The fix allows it to run as expected without any error.

Description of changes:

• Data is copied when looping over queries (q := q)
• The loop is refactored to remove redundant logic when handling expressions.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

[homme]

Hmmm, I've built and deployed a version of k8s-cloudwatch-adapter with this fix and am still getting the same errors, so this needs further investigation...

[homme]

Looking into this further I found and fixed where duplicate logic needed to be updated: the adapter now works as expected in the cluster.

@chankh would it make sense to re-use the MetricSeriesSpec struct as the type for MetricsDiscoveryConfig.Series? We can then de-duplicate the query construction logic.