Fix the check for HPA changed

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

pkg/provider/hpa.go

@@ -25,6 +25,10 @@ import (
 	"github.com/zalando-incubator/kube-metrics-adapter/pkg/recorder"
 )
 
+const (
+	kubectlLastAppliedAnnotation = "kubectl.kubernetes.io/last-applied-configuration"
+)
+
 var (
 	// CollectionSuccesses is the total number of successful collections.
 	CollectionSuccesses = promauto.NewCounter(prometheus.CounterOpts{
@@ -120,7 +124,7 @@ func (p *HPAProvider) Run(ctx context.Context) {
 // updateHPAs discovers all HPA resources and sets up metric collectors for new
 // HPAs.
 func (p *HPAProvider) updateHPAs() error {
-	p.logger.Info("Looking for HPAs")
+	p.logger.Debug("Looking for HPAs")
 
 	hpas, err := p.client.AutoscalingV2().HorizontalPodAutoscalers(metav1.NamespaceAll).List(context.TODO(), metav1.ListOptions{})
 	if err != nil {
@@ -144,7 +148,7 @@ func (p *HPAProvider) updateHPAs() error {
 			// if the hpa has changed then remove the previous
 			// scheduled collector.
 			if hpaUpdated {
-				p.logger.Infof("Removing previously scheduled metrics collector: %s", resourceRef)
+				p.logger.Infof("Removing previously scheduled metrics collector as HPA changed: %s", resourceRef)
 				p.collectorScheduler.Remove(resourceRef)
 			}
 
@@ -197,7 +201,12 @@ func (p *HPAProvider) updateHPAs() error {
 		p.collectorScheduler.Remove(ref)
 	}
 
-	p.logger.Infof("Found %d new/updated HPA(s)", newHPAs)
+	if newHPAs > 0 {
+		p.logger.Infof("Found %d new/updated HPA(s)", newHPAs)
+	} else {
+		p.logger.Debug("No new/updated HPAs found")
+	}
+
 	p.hpaCache = newHPACache
 
 	return nil
@@ -205,12 +214,38 @@ func (p *HPAProvider) updateHPAs() error {
 
 // equalHPA returns true if two HPAs are identical (apart from their status).
 func equalHPA(a, b autoscalingv2.HorizontalPodAutoscaler) bool {
-	// reset resource version to not compare it since this will change
+	return annotationsUpToDate(a.ObjectMeta, b.ObjectMeta, kubectlLastAppliedAnnotation) && reflect.DeepEqual(a.Spec, b.Spec)
-	// whenever the status of the object is updated. We only want to
+}
-	// compare the metadata and the spec.
+
-	a.ObjectMeta.ResourceVersion = ""
+// annotationsUpToDate checks whether the annotations of the existing and
-	b.ObjectMeta.ResourceVersion = ""
+// updated resource are up to date.
-	return reflect.DeepEqual(a.ObjectMeta, b.ObjectMeta) && reflect.DeepEqual(a.Spec, b.Spec)
+func annotationsUpToDate(updated, existing metav1.ObjectMeta, ignoredAnnotations ...string) bool {
+	if len(updated.Annotations) != len(existing.Annotations) {
+		return false
+	}
+
+	for k, v := range updated.Annotations {
+		isIgnored := false
+		for _, ignored := range ignoredAnnotations {
+			if k == ignored {
+				isIgnored = true
+				break
+			}
+		}
+
+		if isIgnored {
+			continue
+		}
+
+		existingValue, ok := existing.GetAnnotations()[k]
+		if ok && existingValue == v {
+			continue
+		}
+
+		return false
+	}
+
+	return true
 }
 
 // collectMetrics collects all metrics from collectors and manages a central

pkg/provider/hpa_test.go

@@ -193,3 +193,172 @@ func TestUpdateHPAsDisregardingIncompatibleHPA(t *testing.T) {
 	// we expect an event when disregardIncompatibleHPAs=false
 	require.Len(t, eventRecorder.Events, 1)
 }
+
+func TestEqualHPA(t *testing.T) {
+	for _, tc := range []struct {
+		name  string
+		hpa1  autoscaling.HorizontalPodAutoscaler
+		hpa2  autoscaling.HorizontalPodAutoscaler
+		equal bool
+	}{
+		{
+			name: "Identical HPAs are equal",
+			hpa1: autoscaling.HorizontalPodAutoscaler{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "hpa1",
+					Namespace: "default",
+				},
+				Spec: autoscaling.HorizontalPodAutoscalerSpec{
+					ScaleTargetRef: autoscaling.CrossVersionObjectReference{
+						Kind:       "Deployment",
+						Name:       "app",
+						APIVersion: "apps/v1",
+					},
+					MinReplicas: &[]int32{1}[0],
+					MaxReplicas: 10,
+				},
+			},
+			hpa2: autoscaling.HorizontalPodAutoscaler{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "hpa1",
+					Namespace: "default",
+				},
+				Spec: autoscaling.HorizontalPodAutoscalerSpec{
+					ScaleTargetRef: autoscaling.CrossVersionObjectReference{
+						Kind:       "Deployment",
+						Name:       "app",
+						APIVersion: "apps/v1",
+					},
+					MinReplicas: &[]int32{1}[0],
+					MaxReplicas: 10,
+				},
+			},
+			equal: true,
+		},
+		{
+			name: "Only kubectl-last-applied diff on HPAs are equal",
+			hpa1: autoscaling.HorizontalPodAutoscaler{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "hpa1",
+					Namespace: "default",
+					Annotations: map[string]string{
+						kubectlLastAppliedAnnotation: "old value",
+					},
+				},
+				Spec: autoscaling.HorizontalPodAutoscalerSpec{
+					ScaleTargetRef: autoscaling.CrossVersionObjectReference{
+						Kind:       "Deployment",
+						Name:       "app",
+						APIVersion: "apps/v1",
+					},
+					MinReplicas: &[]int32{1}[0],
+					MaxReplicas: 10,
+				},
+			},
+			hpa2: autoscaling.HorizontalPodAutoscaler{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "hpa1",
+					Namespace: "default",
+					Annotations: map[string]string{
+						kubectlLastAppliedAnnotation: "new value",
+					},
+				},
+				Spec: autoscaling.HorizontalPodAutoscalerSpec{
+					ScaleTargetRef: autoscaling.CrossVersionObjectReference{
+						Kind:       "Deployment",
+						Name:       "app",
+						APIVersion: "apps/v1",
+					},
+					MinReplicas: &[]int32{1}[0],
+					MaxReplicas: 10,
+				},
+			},
+			equal: true,
+		},
+		{
+			name: "diff in annotations are not equal",
+			hpa1: autoscaling.HorizontalPodAutoscaler{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "hpa1",
+					Namespace: "default",
+					Annotations: map[string]string{
+						"annotation": "old value",
+					},
+				},
+				Spec: autoscaling.HorizontalPodAutoscalerSpec{
+					ScaleTargetRef: autoscaling.CrossVersionObjectReference{
+						Kind:       "Deployment",
+						Name:       "app",
+						APIVersion: "apps/v1",
+					},
+					MinReplicas: &[]int32{1}[0],
+					MaxReplicas: 10,
+				},
+			},
+			hpa2: autoscaling.HorizontalPodAutoscaler{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "hpa1",
+					Namespace: "default",
+					Annotations: map[string]string{
+						"annotation": "new value",
+					},
+				},
+				Spec: autoscaling.HorizontalPodAutoscalerSpec{
+					ScaleTargetRef: autoscaling.CrossVersionObjectReference{
+						Kind:       "Deployment",
+						Name:       "app",
+						APIVersion: "apps/v1",
+					},
+					MinReplicas: &[]int32{1}[0],
+					MaxReplicas: 10,
+				},
+			},
+			equal: false,
+		},
+		{
+			name: "diff in labels are equal",
+			hpa1: autoscaling.HorizontalPodAutoscaler{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "hpa1",
+					Namespace: "default",
+					Labels: map[string]string{
+						"label": "old-value",
+					},
+				},
+				Spec: autoscaling.HorizontalPodAutoscalerSpec{
+					ScaleTargetRef: autoscaling.CrossVersionObjectReference{
+						Kind:       "Deployment",
+						Name:       "app",
+						APIVersion: "apps/v1",
+					},
+					MinReplicas: &[]int32{1}[0],
+					MaxReplicas: 10,
+				},
+			},
+			hpa2: autoscaling.HorizontalPodAutoscaler{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "hpa1",
+					Namespace: "default",
+					Labels: map[string]string{
+						"label": "new-value",
+					},
+				},
+				Spec: autoscaling.HorizontalPodAutoscalerSpec{
+					ScaleTargetRef: autoscaling.CrossVersionObjectReference{
+						Kind:       "Deployment",
+						Name:       "app",
+						APIVersion: "apps/v1",
+					},
+					MinReplicas: &[]int32{1}[0],
+					MaxReplicas: 10,
+				},
+			},
+			equal: true,
+		},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			require.Equal(t, tc.equal, equalHPA(tc.hpa1, tc.hpa2))
+		})
+
+	}
+}