Check for events to ensure invalid HPAs are ignored

Signed-off-by: Mikkel Oscar Lyderik Larsen <mikkel.larsen@zalando.de>

go.mod

@@ -4,7 +4,6 @@ require (
 	github.com/NYTimes/gziphandler v1.0.1 // indirect
 	github.com/aws/aws-sdk-go v1.37.20
 	github.com/go-openapi/spec v0.20.3
-	github.com/hashicorp/go-multierror v1.0.0
 	github.com/influxdata/influxdb-client-go v0.2.0
 	github.com/influxdata/line-protocol v0.0.0-20201012155213-5f565037cbc9 // indirect
 	github.com/kubernetes-sigs/custom-metrics-apiserver v0.0.0-20201216091021-1b9fa998bbaa

go.sum

@@ -290,12 +290,10 @@ github.com/hashicorp/consul/api v1.1.0/go.mod h1:VmuI/Lkw1nC05EYQWNKwWGbkg+FbDBt
 github.com/hashicorp/consul/api v1.3.0/go.mod h1:MmDNSzIMUjNpY/mQ398R4bk2FnqQLoPndWW5VkKPlCE=
 github.com/hashicorp/consul/sdk v0.1.1/go.mod h1:VKf9jXwCTEY1QZP2MOLRhb5i/I/ssyNV1vwHyQBF0x8=
 github.com/hashicorp/consul/sdk v0.3.0/go.mod h1:VKf9jXwCTEY1QZP2MOLRhb5i/I/ssyNV1vwHyQBF0x8=
-github.com/hashicorp/errwrap v1.0.0 h1:hLrqtEDnRye3+sgx6z4qVLNuviH3MR5aQ0ykNJa/UYA=
 github.com/hashicorp/errwrap v1.0.0/go.mod h1:YH+1FKiLXxHSkmPseP+kNlulaMuP3n2brvKWEqk/Jc4=
 github.com/hashicorp/go-cleanhttp v0.5.1/go.mod h1:JpRdi6/HCYpAwUzNwuwqhbovhLtngrth3wmdIIUrZ80=
 github.com/hashicorp/go-immutable-radix v1.0.0/go.mod h1:0y9vanUI8NX6FsYoO3zeMjhV/C5i9g4Q3DwcSNZ4P60=
 github.com/hashicorp/go-msgpack v0.5.3/go.mod h1:ahLV/dePpqEmjfWmKiqvPkv/twdG7iPBM1vqhUKIvfM=
-github.com/hashicorp/go-multierror v1.0.0 h1:iVjPR7a6H0tWELX5NxNe7bYopibicUzc7uPribsnS6o=
 github.com/hashicorp/go-multierror v1.0.0/go.mod h1:dHtQlpGsu+cZNNAkkCN/P3hoUDHhCYQXV3UM06sGGrk=
 github.com/hashicorp/go-rootcerts v1.0.0/go.mod h1:K6zTfqpRlCUIjkwsN4Z+hiSfzSTQa6eBIzfwKfwNnHU=
 github.com/hashicorp/go-sockaddr v1.0.0/go.mod h1:7Xibr9yA9JjQq1JpNB2Vw7kxv8xerXegt+ozgdvDeDU=

pkg/provider/hpa.go

@@ -7,7 +7,6 @@ import (
 	"sync"
 	"time"
 
-	multierror "github.com/hashicorp/go-multierror"
 	"github.com/kubernetes-sigs/custom-metrics-apiserver/pkg/provider"
 	"github.com/prometheus/client_golang/prometheus"
 	"github.com/prometheus/client_golang/prometheus/promauto"
@@ -130,9 +129,6 @@ func (p *HPAProvider) updateHPAs() error {
 
 	newHPAs := 0
 
-	// Multi errors object used in loops where they have to continue but in the end an error should be reported
-	var errs error
-
 	for _, hpa := range hpas.Items {
 		hpa := *hpa.DeepCopy()
 		resourceRef := resourceReference{
@@ -153,7 +149,6 @@ func (p *HPAProvider) updateHPAs() error {
 			metricConfigs, err := collector.ParseHPAMetrics(&hpa)
 			if err != nil {
 				p.logger.Errorf("Failed to parse HPA metrics: %v", err)
-				errs = multierror.Append(errs, err)
 				continue
 			}
 
@@ -170,7 +165,6 @@ func (p *HPAProvider) updateHPAs() error {
 					// 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)
-						errs = multierror.Append(errs, err)
 					}
 
 					cache = false
@@ -204,7 +198,7 @@ func (p *HPAProvider) updateHPAs() error {
 	p.logger.Infof("Found %d new/updated HPA(s)", newHPAs)
 	p.hpaCache = newHPACache
 
-	return errs
+	return nil
 }
 
 // equalHPA returns true if two HPAs are identical (apart from their status).

pkg/provider/hpa_test.go

@@ -2,6 +2,7 @@ package provider
 
 import (
 	"context"
+	"fmt"
 	"testing"
 	"time"
 
@@ -10,6 +11,7 @@ import (
 	autoscaling "k8s.io/api/autoscaling/v2beta2"
 	"k8s.io/apimachinery/pkg/api/resource"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/client-go/kubernetes/fake"
 )
 
@@ -29,6 +31,33 @@ func (c mockCollector) Interval() time.Duration {
 	return 1 * time.Second
 }
 
+type event struct {
+	Object    runtime.Object
+	EventType string
+	Reason    string
+	Message   string
+}
+
+type mockEventRecorder struct {
+	Events []event
+}
+
+func (r *mockEventRecorder) Event(object runtime.Object, eventtype, reason, message string) {
+	r.Events = append(r.Events, event{
+		Object:    object,
+		EventType: eventtype,
+		Reason:    reason,
+		Message:   message,
+	})
+}
+
+func (r *mockEventRecorder) Eventf(object runtime.Object, eventtype, reason, messageFmt string, args ...interface{}) {
+	r.Event(object, eventtype, reason, fmt.Sprintf(messageFmt, args...))
+}
+
+func (r *mockEventRecorder) AnnotatedEventf(object runtime.Object, annotations map[string]string, eventtype, reason, messageFmt string, args ...interface{}) {
+}
+
 func TestUpdateHPAs(t *testing.T) {
 	value := resource.MustParse("1k")
 
@@ -141,9 +170,26 @@ func TestUpdateHPAsDisregardingIncompatibleHPA(t *testing.T) {
 	err = collectorFactory.RegisterPodsCollector("", mockCollectorPlugin{})
 	require.NoError(t, err)
 
+	eventRecorder := &mockEventRecorder{}
 	provider := NewHPAProvider(fakeClient, 1*time.Second, 1*time.Second, collectorFactory, true)
+	provider.recorder = eventRecorder
 	provider.collectorScheduler = NewCollectorScheduler(context.Background(), provider.metricSink)
 
 	err = provider.updateHPAs()
 	require.NoError(t, err)
+
+	// we don't expect any events if disregardIncompatibleHPAs=true
+	require.Len(t, eventRecorder.Events, 0)
+
+	// check for events when disregardIncompatibleHPAs=false
+	eventRecorder = &mockEventRecorder{}
+	provider = NewHPAProvider(fakeClient, 1*time.Second, 1*time.Second, collectorFactory, false)
+	provider.recorder = eventRecorder
+	provider.collectorScheduler = NewCollectorScheduler(context.Background(), provider.metricSink)
+
+	err = provider.updateHPAs()
+	require.NoError(t, err)
+
+	// we expect an event when disregardIncompatibleHPAs=false
+	require.Len(t, eventRecorder.Events, 1)
 }