diff --git a/pkg/provider/hpa.go b/pkg/provider/hpa.go index c2bf22f..3fef3c0 100644 --- a/pkg/provider/hpa.go +++ b/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 - // whenever the status of the object is updated. We only want to - // compare the metadata and the spec. - a.ObjectMeta.ResourceVersion = "" - b.ObjectMeta.ResourceVersion = "" - return reflect.DeepEqual(a.ObjectMeta, b.ObjectMeta) && reflect.DeepEqual(a.Spec, b.Spec) + return annotationsUpToDate(a.ObjectMeta, b.ObjectMeta, kubectlLastAppliedAnnotation) && reflect.DeepEqual(a.Spec, b.Spec) +} + +// annotationsUpToDate checks whether the annotations of the existing and +// updated resource are up to date. +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 diff --git a/pkg/provider/hpa_test.go b/pkg/provider/hpa_test.go index c7df48e..e6284e9 100644 --- a/pkg/provider/hpa_test.go +++ b/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)) + }) + + } +}