Permit disregarding incompatible HPAs (#95)

* This commit adds a --disregard-incompatible-hpas that makes the HPA
provider stop erroring out when a collector cannot be created for a
metric in a HPA. Useful when kube-metrics-adapter runs alongside another
metrics provider. Fixes issue #94.

Signed-off-by: Tomás Pinho <me@tomaspinho.com>

* Make tests pass

Signed-off-by: Tomás Pinho <me@tomaspinho.com>

* Wraps the Plugin Not Found error in a new type that can be checked by the caller of a function to determine if its contents should be logged or added as an event to the HPA, when this HPA is incompatible.
The disregardIncompatibleHPAs is now targetting only the log or addition of the same event.

Signed-off-by: Tomás Pinho <me@tomaspinho.com>

* Invert if expression to select when we should log
CreateNewMetricsCollector errors: don't log when both conditions are true - it's not a PluginNotFoundError
and disregardIncompatibleHPAs flag is set to true. This way, if an error
is NOT PluginNotFoundError it will always be logged, and when it IS
PluginNotFoundError it will only be logged when
disregardIncompatibleHPAs is false.

Signed-off-by: Tomás Pinho <me@tomaspinho.com>

* Remove redundant "whether to"

Signed-off-by: Tomás Pinho <me@tomaspinho.com>

* Add test case for updating HPAs via HPA Provider while disregarding
incompatible HPAs.

Signed-off-by: Tomás Pinho <me@tomaspinho.com>

pkg/collector/collector.go

@@ -46,6 +46,14 @@ type CollectorPlugin interface {
 	NewCollector(hpa *autoscalingv2.HorizontalPodAutoscaler, config *MetricConfig, interval time.Duration) (Collector, error)
 }
 
+type PluginNotFoundError struct {
+	metricTypeName MetricTypeName
+}
+
+func (p *PluginNotFoundError) Error() string {
+	return fmt.Sprintf("no plugin found for %s", p.metricTypeName)
+}
+
 func (c *CollectorFactory) RegisterPodsCollector(metricCollector string, plugin CollectorPlugin) error {
 	if metricCollector == "" {
 		c.podsPlugins.Any = plugin
@@ -139,7 +147,7 @@ func (c *CollectorFactory) NewCollector(hpa *autoscalingv2.HorizontalPodAutoscal
 		}
 	}
 
-	return nil, fmt.Errorf("no plugin found for %s", config.MetricTypeName)
+	return nil, &PluginNotFoundError{metricTypeName: config.MetricTypeName}
 }
 
 type MetricTypeName struct {

pkg/provider/hpa.go

@@ -2,6 +2,7 @@ package provider
 
 import (
 	"context"
+	"errors"
 	"reflect"
 	"sync"
 	"time"
@@ -49,16 +50,17 @@ var (
 // HPAProvider is a base provider for initializing metric collectors based on
 // HPA resources.
 type HPAProvider struct {
-	client             kubernetes.Interface
-	interval           time.Duration
-	collectorScheduler *CollectorScheduler
-	collectorInterval  time.Duration
-	metricSink         chan metricCollection
-	hpaCache           map[resourceReference]autoscalingv2.HorizontalPodAutoscaler
-	metricStore        *MetricStore
-	collectorFactory   *collector.CollectorFactory
-	recorder           kube_record.EventRecorder
-	logger             *log.Entry
+	client                    kubernetes.Interface
+	interval                  time.Duration
+	collectorScheduler        *CollectorScheduler
+	collectorInterval         time.Duration
+	metricSink                chan metricCollection
+	hpaCache                  map[resourceReference]autoscalingv2.HorizontalPodAutoscaler
+	metricStore               *MetricStore
+	collectorFactory          *collector.CollectorFactory
+	recorder                  kube_record.EventRecorder
+	logger                    *log.Entry
+	disregardIncompatibleHPAs bool
 }
 
 // metricCollection is a container for sending collected metrics across a
@@ -69,7 +71,7 @@ type metricCollection struct {
 }
 
 // NewHPAProvider initializes a new HPAProvider.
-func NewHPAProvider(client kubernetes.Interface, interval, collectorInterval time.Duration, collectorFactory *collector.CollectorFactory) *HPAProvider {
+func NewHPAProvider(client kubernetes.Interface, interval, collectorInterval time.Duration, collectorFactory *collector.CollectorFactory, disregardIncompatibleHPAs bool) *HPAProvider {
 	metricsc := make(chan metricCollection)
 
 	return &HPAProvider{
@@ -80,9 +82,10 @@ func NewHPAProvider(client kubernetes.Interface, interval, collectorInterval tim
 		metricStore: NewMetricStore(func() time.Time {
 			return time.Now().UTC().Add(15 * time.Minute)
 		}),
-		collectorFactory: collectorFactory,
-		recorder:         recorder.CreateEventRecorder(client),
-		logger:           log.WithFields(log.Fields{"provider": "hpa"}),
+		collectorFactory:          collectorFactory,
+		recorder:                  recorder.CreateEventRecorder(client),
+		logger:                    log.WithFields(log.Fields{"provider": "hpa"}),
+		disregardIncompatibleHPAs: disregardIncompatibleHPAs,
 	}
 }
 
@@ -154,15 +157,20 @@ func (p *HPAProvider) updateHPAs() error {
 					interval = p.collectorInterval
 				}
 
-				collector, err := p.collectorFactory.NewCollector(&hpa, config, interval)
+				c, err := p.collectorFactory.NewCollector(&hpa, config, interval)
 				if err != nil {
-					p.recorder.Eventf(&hpa, apiv1.EventTypeWarning, "CreateNewMetricsCollector", "Failed to create new metrics collector: %v", err)
+
+					// Only log when it's not a PluginNotFoundError AND flag disregardIncompatibleHPAs is true
+					if !(errors.Is(err, &collector.PluginNotFoundError{}) && p.disregardIncompatibleHPAs) {
+						p.recorder.Eventf(&hpa, apiv1.EventTypeWarning, "CreateNewMetricsCollector", "Failed to create new metrics collector: %v", err)
+					}
+
 					cache = false
 					continue
 				}
 
-				p.logger.Infof("Adding new metrics collector: %T", collector)
-				p.collectorScheduler.Add(resourceRef, config.MetricTypeName, collector)
+				p.logger.Infof("Adding new metrics collector: %T", c)
+				p.collectorScheduler.Add(resourceRef, config.MetricTypeName, c)
 			}
 			newHPAs++
 

pkg/provider/hpa_test.go

@@ -77,7 +77,7 @@ func TestUpdateHPAs(t *testing.T) {
 	err = collectorFactory.RegisterPodsCollector("", mockCollectorPlugin{})
 	require.NoError(t, err)
 
-	provider := NewHPAProvider(fakeClient, 1*time.Second, 1*time.Second, collectorFactory)
+	provider := NewHPAProvider(fakeClient, 1*time.Second, 1*time.Second, collectorFactory, false)
 	provider.collectorScheduler = NewCollectorScheduler(context.Background(), provider.metricSink)
 
 	err = provider.updateHPAs()
@@ -94,3 +94,51 @@ func TestUpdateHPAs(t *testing.T) {
 
 	require.Len(t, provider.collectorScheduler.table, 1)
 }
+
+func TestUpdateHPAsDisregardingIncompatibleHPA(t *testing.T) {
+	// Test HPAProvider with disregardIncompatibleHPAs = true
+
+	value := resource.MustParse("1k")
+
+	hpa := &autoscalingv1.HorizontalPodAutoscaler{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:        "hpa1",
+			Namespace:   "default",
+			Annotations: map[string]string{},
+		},
+		Spec: autoscalingv1.HorizontalPodAutoscalerSpec{
+			ScaleTargetRef: autoscalingv1.CrossVersionObjectReference{
+				Kind:       "Deployment",
+				Name:       "app",
+				APIVersion: "apps/v1",
+			},
+			MinReplicas: &[]int32{1}[0],
+			MaxReplicas: 10,
+			Metrics: []autoscalingv1.MetricSpec{
+				{
+					Type: autoscalingv1.ExternalMetricSourceType,
+					External: &autoscalingv1.ExternalMetricSource{
+						MetricName:         "some-other-metric",
+						TargetAverageValue: &value,
+					},
+				},
+			},
+		},
+	}
+
+	fakeClient := fake.NewSimpleClientset()
+
+	var err error
+	_, err = fakeClient.AutoscalingV2beta1().HorizontalPodAutoscalers("default").Create(hpa)
+	require.NoError(t, err)
+
+	collectorFactory := collector.NewCollectorFactory()
+	err = collectorFactory.RegisterPodsCollector("", mockCollectorPlugin{})
+	require.NoError(t, err)
+
+	provider := NewHPAProvider(fakeClient, 1*time.Second, 1*time.Second, collectorFactory, true)
+	provider.collectorScheduler = NewCollectorScheduler(context.Background(), provider.metricSink)
+
+	err = provider.updateHPAs()
+	require.NoError(t, err)
+}

pkg/server/start.go

@@ -110,7 +110,8 @@ func NewCommandStartAdapterServer(stopCh <-chan struct{}) *cobra.Command {
 		"whether to enable AWS external metrics")
 	flags.StringSliceVar(&o.AWSRegions, "aws-region", o.AWSRegions, "the AWS regions which should be monitored. eg: eu-central, eu-west-1")
 	flags.StringVar(&o.MetricsAddress, "metrics-address", o.MetricsAddress, "The address where to serve prometheus metrics")
-
+	flags.BoolVar(&o.DisregardIncompatibleHPAs, "disregard-incompatible-hpas", o.DisregardIncompatibleHPAs, ""+
+		"disregard failing to create collectors for incompatible HPAs")
 	return cmd
 }
 
@@ -228,7 +229,7 @@ func (o AdapterServerOptions) RunCustomMetricsAdapterServer(stopCh <-chan struct
 		collectorFactory.RegisterExternalCollector([]string{collector.AWSSQSQueueLengthMetric}, collector.NewAWSCollectorPlugin(awsSessions))
 	}
 
-	hpaProvider := provider.NewHPAProvider(client, 30*time.Second, 1*time.Minute, collectorFactory)
+	hpaProvider := provider.NewHPAProvider(client, 30*time.Second, 1*time.Minute, collectorFactory, o.DisregardIncompatibleHPAs)
 
 	go hpaProvider.Run(ctx)
 
@@ -332,4 +333,7 @@ type AdapterServerOptions struct {
 	MetricsAddress string
 	// SkipperBackendWeightAnnotation is the annotation on the ingress indicating the backend weights
 	SkipperBackendWeightAnnotation []string
+	// Whether to disregard failing to create collectors for incompatible HPAs - such as when using
+	// kube-metrics-adapter beside another Metrics Provider
+	DisregardIncompatibleHPAs bool
 }