Add docs, tests, and move namespaced to metricsQuery

docs/externalmetrics.md

@@ -0,0 +1,84 @@
+External Metrics
+===========
+
+It's possible to configure [Autoscaling on metrics not related to Kubernetes objects](Autoscaling on metrics not related to Kubernetes objects) in Kubernetes. This is done with a special `External Metrics` system. Using external metrics in Kubernetes with the adapter requires you to configure special `external` rules in the configuration.
+
+The configuration for `external` metrics rules is almost identical to the normal `rules`:
+
+```yaml
+external:
+- seriesQuery: '{__name__="queue_consumer_lag",name!=""}'
+  metricsQuery: sum(<<.Series>>{<<.LabelMatchers>>}) by (name)
+  resources:
+    overrides: { namespace: {resource: "namespace"} }
+```
+
+Namespacing
+-----------
+
+All Kubernetes Horizontal Pod Autoscaler (HPA) resources are namespaced. And when you create an HPA that
+references an external metric the adapter will automatically add a `namespace` label to the `seriesQuery` you have configured.
+
+This is done because the External Merics API Specification *requires* a namespace component in the URL:
+
+```shell
+kubectl get --raw "/apis/external.metrics.k8s.io/v1beta1/namespaces/default/queue_consumer_lag"
+```
+
+Cross-Namespace or No Namespace Queries
+---------------------------------------
+
+A semi-common scenario is to have a `workload` in one namespace that needs to scale based on a metric from a different namespace. This is normally not
+possible with `external` rules because the `namespace` label is set to match that of the source `workload`.
+
+However, you can explicitly disable the automatic add of the HPA namepace to the query, and instead opt to not set a namespace at all, or to target a different namespace.
+
+This is done by setting `namespaced: false` the `resources` section of the `external` rule:
+
+```yaml
+# rules: ...
+
+external:
+- seriesQuery: '{__name__="queue_depth",name!=""}'
+  metricsQuery: sum(<<.Series>>{<<.LabelMatchers>>}) by (name)
+  resources:
+    namespaced: false
+```
+
+Given the `external` rules defined above any `External` metric query for `queue_depth` will simply ignore the source `namespace` of the HPA. This allows you to explicilty not put a namespace into an external query, or to set the namespace to one that might be different from that of the HPA.
+
+```yaml
+apiVersion: autoscaling/v1
+kind: HorizontalPodAutoscaler
+metadata:
+  name: external-queue-scaler
+  # the HPA and scaleTargetRef must exist in a namespace
+  namespace: default
+  annotations:
+    # The "External" metric below targets a metricName that has namespaced=false
+    # and this allows the metric to explicitly query a different
+    # namespace than that of the HPA and scaleTargetRef
+    autoscaling.alpha.kubernetes.io/metrics: |
+      [
+        {
+            "type": "External",
+            "external": {
+                "metricName": "queue_depth",
+                "metricSelector": {
+                    "matchLabels": {
+                        "namespace": "queue",
+                        "name": "my-sample-queue"
+                    }
+                },
+                "targetAverageValue": "50"
+            }
+        }
+      ]
+spec:
+  maxReplicas: 5
+  minReplicas: 1
+  scaleTargetRef:
+    apiVersion: apps/v1
+    kind: Deployment
+    name: my-app
+```

docs/sample-config.yaml

@@ -65,5 +65,18 @@ rules:
       brand: {group: "cheese.io", resource: "brand"}
   metricQuery: 'count(cheddar{sharp="true"})'
 
+# external rules are not tied to a Kubernetes resource and can reference any metric
+# https://kubernetes.io/docs/tasks/run-application/horizontal-pod-autoscale-walkthrough/#autoscaling-on-metrics-not-related-to-kubernetes-objects
+external:
+- seriesQuery: '{__name__="queue_consumer_lag",name!=""}'
+  metricsQuery: sum(<<.Series>>{<<.LabelMatchers>>}) by (name)
+- seriesQuery: '{__name__="queue_depth",topic!=""}'
+  metricsQuery: sum(<<.Series>>{<<.LabelMatchers>>}) by (name)
+  # Kubernetes metric queries include a namespace in the query by default
+  # but you can explicitly disable namespaces if needed with "namespaced: false"
+  # this is useful if you have an HPA with an external metric in namespace A
+  # but want to query for metrics from namespace B
+  namespaced: false
+
 # TODO: should we be able to map to a constant instance of a resource
 # (e.g. `resources: {constant: [{resource: "namespace", name: "kube-system"}}]`)?

pkg/naming/metric_namer.go

@@ -102,15 +102,10 @@ type metricNamer struct {
 	nameMatches    *regexp.Regexp
 	nameAs         string
 	seriesMatchers []*ReMatcher
-	namespaced     *bool
 
 	ResourceConverter
 }
 
-func (n *metricNamer) isNamespaced() bool {
-	return (n.namespaced != nil) && *n.namespaced
-}
-
 // queryTemplateArgs are the arguments for the metrics query template.
 func (n *metricNamer) FilterSeries(initialSeries []prom.Series) []prom.Series {
 	if len(n.seriesMatchers) == 0 {
@@ -136,10 +131,6 @@ func (n *metricNamer) QueryForSeries(series string, resource schema.GroupResourc
 }
 
 func (n *metricNamer) QueryForExternalSeries(series string, namespace string, metricSelector labels.Selector) (prom.Selector, error) {
-	if !n.isNamespaced() {
-		namespace = ""
-	}
-
 	return n.metricsQuery.BuildExternal(series, namespace, "", []string{}, metricSelector)
 }
 
@@ -162,7 +153,13 @@ func NamersFromConfig(cfg []config.DiscoveryRule, mapper apimeta.RESTMapper) ([]
 			return nil, err
 		}
 
-		metricsQuery, err := NewMetricsQuery(rule.MetricsQuery, resConv)
+		// queries are namespaced by default unless the rule specifically disables it
+		namespaced := true
+		if rule.Resources.Namespaced != nil {
+			namespaced = *rule.Resources.Namespaced
+		}
+
+		metricsQuery, err := NewMetricsQuery(rule.MetricsQuery, resConv, namespaced)
 		if err != nil {
 			return nil, fmt.Errorf("unable to construct metrics query associated with series query %q: %v", rule.SeriesQuery, err)
 		}
@@ -214,7 +211,6 @@ func NamersFromConfig(cfg []config.DiscoveryRule, mapper apimeta.RESTMapper) ([]
 			nameMatches:       nameMatches,
 			nameAs:            nameAs,
 			seriesMatchers:    seriesMatchers,
-			namespaced:        rule.Resources.Namespaced,
 			ResourceConverter: resConv,
 		}
 

pkg/naming/metrics_query.go

@@ -50,7 +50,7 @@ type MetricsQuery interface {
 // - LabelMatchersByName: the raw map-form of the above matchers
 // - GroupBy: the group-by clause to use for the resources in the query (stringified)
 // - GroupBySlice: the raw slice form of the above group-by clause
-func NewMetricsQuery(queryTemplate string, resourceConverter ResourceConverter) (MetricsQuery, error) {
+func NewMetricsQuery(queryTemplate string, resourceConverter ResourceConverter, namespaced bool) (MetricsQuery, error) {
 	templ, err := template.New("metrics-query").Delims("<<", ">>").Parse(queryTemplate)
 	if err != nil {
 		return nil, fmt.Errorf("unable to parse metrics query template %q: %v", queryTemplate, err)
@@ -59,6 +59,7 @@ func NewMetricsQuery(queryTemplate string, resourceConverter ResourceConverter)
 	return &metricsQuery{
 		resConverter: resourceConverter,
 		template:     templ,
+		namespaced:   namespaced,
 	}, nil
 }
 
@@ -68,6 +69,7 @@ func NewMetricsQuery(queryTemplate string, resourceConverter ResourceConverter)
 type metricsQuery struct {
 	resConverter ResourceConverter
 	template     *template.Template
+	namespaced   bool
 }
 
 // queryTemplateArgs contains the arguments for the template used in metricsQuery.
@@ -88,7 +90,7 @@ type queryPart struct {
 func (q *metricsQuery) Build(series string, resource schema.GroupResource, namespace string, extraGroupBy []string, metricSelector labels.Selector, names ...string) (prom.Selector, error) {
 	queryParts := q.createQueryPartsFromSelector(metricSelector)
 
-	if namespace != "" {
+	if q.namespaced && namespace != "" {
 		namespaceLbl, err := q.resConverter.LabelForResource(NsGroupResource)
 		if err != nil {
 			return "", err
@@ -150,7 +152,7 @@ func (q *metricsQuery) BuildExternal(seriesName string, namespace string, groupB
 	// Build up the query parts from the selector.
 	queryParts = append(queryParts, q.createQueryPartsFromSelector(metricSelector)...)
 
-	if namespace != "" {
+	if q.namespaced && namespace != "" {
 		namespaceLbl, err := q.resConverter.LabelForResource(NsGroupResource)
 		if err != nil {
 			return "", err

pkg/naming/metrics_query_test.go

@@ -76,7 +76,15 @@ func checks(cs ...checkFunc) checkFunc {
 
 func TestBuildSelector(t *testing.T) {
 	mustNewQuery := func(queryTemplate string, namespaced bool) MetricsQuery {
-		mq, err := NewMetricsQuery(queryTemplate, &resourceConverterMock{namespaced})
+		mq, err := NewMetricsQuery(queryTemplate, &resourceConverterMock{namespaced}, true)
+		if err != nil {
+			t.Fatal(err)
+		}
+		return mq
+	}
+
+	mustNewNonNamespacedQuery := func(queryTemplate string, namespaced bool) MetricsQuery {
+		mq, err := NewMetricsQuery(queryTemplate, &resourceConverterMock{namespaced}, false)
 		if err != nil {
 			t.Fatal(err)
 		}
@@ -204,6 +212,21 @@ func TestBuildSelector(t *testing.T) {
 			),
 		},
 
+		{
+			name: "multiple LabelValuesByName values with namespace disabled",
+
+			mq:             mustNewNonNamespacedQuery(`<<index .LabelValuesByName "namespaces">> <<index .LabelValuesByName "resource">>`, true),
+			metricSelector: labels.NewSelector(),
+			resource:       schema.GroupResource{Group: "group", Resource: "resource"},
+			namespace:      "default",
+			names:          []string{"bar", "baz"},
+
+			check: checks(
+				hasError(nil),
+				hasSelector(" bar|baz"),
+			),
+		},
+
 		{
 			name: "single GroupBy value",
 
@@ -272,7 +295,15 @@ func TestBuildSelector(t *testing.T) {
 
 func TestBuildExternalSelector(t *testing.T) {
 	mustNewQuery := func(queryTemplate string) MetricsQuery {
-		mq, err := NewMetricsQuery(queryTemplate, &resourceConverterMock{true})
+		mq, err := NewMetricsQuery(queryTemplate, &resourceConverterMock{true}, true)
+		if err != nil {
+			t.Fatal(err)
+		}
+		return mq
+	}
+
+	mustNewNonNamespacedQuery := func(queryTemplate string) MetricsQuery {
+		mq, err := NewMetricsQuery(queryTemplate, &resourceConverterMock{true}, false)
 		if err != nil {
 			t.Fatal(err)
 		}
@@ -348,6 +379,19 @@ func TestBuildExternalSelector(t *testing.T) {
 				hasSelector("default [foo bar]"),
 			),
 		},
+		{
+			name: "multiple GroupBySlice values with namespace disabled",
+
+			mq:             mustNewNonNamespacedQuery(`<<index .LabelValuesByName "namespaces">> <<.GroupBySlice>>`),
+			namespace:      "default",
+			groupBySlice:   []string{"foo", "bar"},
+			metricSelector: labels.NewSelector(),
+
+			check: checks(
+				hasError(nil),
+				hasSelector(" [foo bar]"),
+			),
+		},
 		{
 			name: "single LabelMatchers value",
 

pkg/resourceprovider/provider.go

@@ -54,11 +54,11 @@ func newResourceQuery(cfg config.ResourceRule, mapper apimeta.RESTMapper) (resou
 		return resourceQuery{}, fmt.Errorf("unable to construct label-resource converter: %v", err)
 	}
 
-	contQuery, err := naming.NewMetricsQuery(cfg.ContainerQuery, converter)
+	contQuery, err := naming.NewMetricsQuery(cfg.ContainerQuery, converter, true)
 	if err != nil {
 		return resourceQuery{}, fmt.Errorf("unable to construct container metrics query: %v", err)
 	}
-	nodeQuery, err := naming.NewMetricsQuery(cfg.NodeQuery, converter)
+	nodeQuery, err := naming.NewMetricsQuery(cfg.NodeQuery, converter, true)
 	if err != nil {
 		return resourceQuery{}, fmt.Errorf("unable to construct node metrics query: %v", err)
 	}