From d886fe414e46377aec3951cd52756678c2ca0467 Mon Sep 17 00:00:00 2001 From: Marco Voelz Date: Wed, 27 Nov 2024 10:37:58 +0100 Subject: [PATCH 1/4] fix all ErrorS without keys --- .../pkg/recommender/checkpoint/checkpoint_writer.go | 4 ++-- .../pkg/recommender/input/cluster_feeder.go | 6 +++--- .../pkg/target/controller_fetcher/controller_fetcher.go | 4 ++-- .../pkg/updater/eviction/pods_eviction_restriction.go | 4 ++-- 4 files changed, 9 insertions(+), 9 deletions(-) diff --git a/vertical-pod-autoscaler/pkg/recommender/checkpoint/checkpoint_writer.go b/vertical-pod-autoscaler/pkg/recommender/checkpoint/checkpoint_writer.go index e7ef346b8507..b0e023a64ddd 100644 --- a/vertical-pod-autoscaler/pkg/recommender/checkpoint/checkpoint_writer.go +++ b/vertical-pod-autoscaler/pkg/recommender/checkpoint/checkpoint_writer.go @@ -108,9 +108,9 @@ func (writer *checkpointWriter) StoreCheckpoints(ctx context.Context, now time.T } err = api_util.CreateOrUpdateVpaCheckpoint(writer.vpaCheckpointClient.VerticalPodAutoscalerCheckpoints(vpa.ID.Namespace), &vpaCheckpoint) if err != nil { - klog.ErrorS(err, "Cannot save checkpoint for VPA", klog.KRef(vpa.ID.Namespace, vpaCheckpoint.Spec.VPAObjectName), "container", vpaCheckpoint.Spec.ContainerName) + klog.ErrorS(err, "Cannot save checkpoint for VPA", "vpa", klog.KRef(vpa.ID.Namespace, vpaCheckpoint.Spec.VPAObjectName), "container", vpaCheckpoint.Spec.ContainerName) } else { - klog.V(3).InfoS("Saved checkpoint for VPA", klog.KRef(vpa.ID.Namespace, vpaCheckpoint.Spec.VPAObjectName), "container", vpaCheckpoint.Spec.ContainerName) + klog.V(3).InfoS("Saved checkpoint for VPA", "vpa", klog.KRef(vpa.ID.Namespace, vpaCheckpoint.Spec.VPAObjectName), "container", vpaCheckpoint.Spec.ContainerName) vpa.CheckpointWritten = now } minCheckpoints-- diff --git a/vertical-pod-autoscaler/pkg/recommender/input/cluster_feeder.go b/vertical-pod-autoscaler/pkg/recommender/input/cluster_feeder.go index 45473cc762bd..5fef7df5deb4 100644 --- a/vertical-pod-autoscaler/pkg/recommender/input/cluster_feeder.go +++ b/vertical-pod-autoscaler/pkg/recommender/input/cluster_feeder.go @@ -262,7 +262,7 @@ func (feeder *clusterStateFeeder) InitFromCheckpoints() { } for _, checkpoint := range checkpointList.Items { - klog.V(3).InfoS("Loading checkpoint for VPA", klog.KRef(checkpoint.ObjectMeta.Namespace, checkpoint.Spec.VPAObjectName), "container", checkpoint.Spec.ContainerName) + klog.V(3).InfoS("Loading checkpoint for VPA", "checkpoint", klog.KRef(checkpoint.ObjectMeta.Namespace, checkpoint.Spec.VPAObjectName), "container", checkpoint.Spec.ContainerName) err = feeder.setVpaCheckpoint(&checkpoint) if err != nil { klog.ErrorS(err, "Error while loading checkpoint") @@ -294,9 +294,9 @@ func (feeder *clusterStateFeeder) GarbageCollectCheckpoints() { if !exists { err = feeder.vpaCheckpointClient.VerticalPodAutoscalerCheckpoints(namespace).Delete(context.TODO(), checkpoint.Name, metav1.DeleteOptions{}) if err == nil { - klog.V(3).InfoS("Orphaned VPA checkpoint cleanup - deleting", klog.KRef(namespace, checkpoint.Name)) + klog.V(3).InfoS("Orphaned VPA checkpoint cleanup - deleting", "checkpoint", klog.KRef(namespace, checkpoint.Name)) } else { - klog.ErrorS(err, "Orphaned VPA checkpoint cleanup - error deleting", klog.KRef(namespace, checkpoint.Name)) + klog.ErrorS(err, "Orphaned VPA checkpoint cleanup - error deleting", "checkpoint", klog.KRef(namespace, checkpoint.Name)) } } } diff --git a/vertical-pod-autoscaler/pkg/target/controller_fetcher/controller_fetcher.go b/vertical-pod-autoscaler/pkg/target/controller_fetcher/controller_fetcher.go index 933f2680c50f..be7036ded386 100644 --- a/vertical-pod-autoscaler/pkg/target/controller_fetcher/controller_fetcher.go +++ b/vertical-pod-autoscaler/pkg/target/controller_fetcher/controller_fetcher.go @@ -261,7 +261,7 @@ func (f *controllerFetcher) isWellKnownOrScalable(ctx context.Context, key *Cont //if not well known check if it supports scaling groupKind, err := key.groupKind() if err != nil { - klog.ErrorS(err, "Could not find groupKind", klog.KRef(key.Namespace, key.Name)) + klog.ErrorS(err, "Could not find groupKind", "object", klog.KRef(key.Namespace, key.Name)) return false } @@ -271,7 +271,7 @@ func (f *controllerFetcher) isWellKnownOrScalable(ctx context.Context, key *Cont mappings, err := f.mapper.RESTMappings(groupKind) if err != nil { - klog.ErrorS(err, "Could not find mappings", klog.KRef(key.Namespace, key.Name)) + klog.ErrorS(err, "Could not find mappings", "object", klog.KRef(key.Namespace, key.Name)) return false } diff --git a/vertical-pod-autoscaler/pkg/updater/eviction/pods_eviction_restriction.go b/vertical-pod-autoscaler/pkg/updater/eviction/pods_eviction_restriction.go index b2bc51886dce..6fa4009d73f7 100644 --- a/vertical-pod-autoscaler/pkg/updater/eviction/pods_eviction_restriction.go +++ b/vertical-pod-autoscaler/pkg/updater/eviction/pods_eviction_restriction.go @@ -140,7 +140,7 @@ func (e *podsEvictionRestrictionImpl) Evict(podToEvict *apiv1.Pod, vpa *vpa_type } err := e.client.CoreV1().Pods(podToEvict.Namespace).EvictV1(context.TODO(), eviction) if err != nil { - klog.ErrorS(err, "Failed to evict pod", klog.KObj(podToEvict)) + klog.ErrorS(err, "Failed to evict pod", "pod", klog.KObj(podToEvict)) return err } eventRecorder.Event(podToEvict, apiv1.EventTypeNormal, "EvictedByVPA", @@ -237,7 +237,7 @@ func (f *podsEvictionRestrictionFactoryImpl) NewPodsEvictionRestriction(pods []* var err error configured, err = f.getReplicaCount(creator) if err != nil { - klog.ErrorS(err, "Failed to obtain replication info", "kind", creator.Kind, klog.KRef(creator.Namespace, creator.Name)) + klog.ErrorS(err, "Failed to obtain replication info", "kind", creator.Kind, "object", klog.KRef(creator.Namespace, creator.Name)) continue } } From d791391e22f50ed37e381507528f44a51055137c Mon Sep 17 00:00:00 2001 From: Marco Voelz Date: Wed, 27 Nov 2024 10:44:02 +0100 Subject: [PATCH 2/4] fix all InfoS without keys --- .../pkg/updater/eviction/pods_eviction_restriction.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/vertical-pod-autoscaler/pkg/updater/eviction/pods_eviction_restriction.go b/vertical-pod-autoscaler/pkg/updater/eviction/pods_eviction_restriction.go index 6fa4009d73f7..e79f79e54c57 100644 --- a/vertical-pod-autoscaler/pkg/updater/eviction/pods_eviction_restriction.go +++ b/vertical-pod-autoscaler/pkg/updater/eviction/pods_eviction_restriction.go @@ -225,7 +225,7 @@ func (f *podsEvictionRestrictionFactoryImpl) NewPodsEvictionRestriction(pods []* for creator, replicas := range livePods { actual := len(replicas) if actual < required { - klog.V(2).InfoS("Too few replicas", "kind", creator.Kind, klog.KRef(creator.Namespace, creator.Name), "livePods", actual, "requiredPods", required, "globalMinReplicas", f.minReplicas) + klog.V(2).InfoS("Too few replicas", "kind", creator.Kind, "object", klog.KRef(creator.Namespace, creator.Name), "livePods", actual, "requiredPods", required, "globalMinReplicas", f.minReplicas) continue } From 3b054a0f53e6d54e3721b85aa51366de9f80717e Mon Sep 17 00:00:00 2001 From: Marco Voelz Date: Wed, 27 Nov 2024 11:41:27 +0100 Subject: [PATCH 3/4] Update vertical-pod-autoscaler/pkg/target/controller_fetcher/controller_fetcher.go Co-authored-by: Ismail Alidzhikov <9372594+ialidzhikov@users.noreply.github.com> --- .../pkg/target/controller_fetcher/controller_fetcher.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/vertical-pod-autoscaler/pkg/target/controller_fetcher/controller_fetcher.go b/vertical-pod-autoscaler/pkg/target/controller_fetcher/controller_fetcher.go index be7036ded386..080451329df4 100644 --- a/vertical-pod-autoscaler/pkg/target/controller_fetcher/controller_fetcher.go +++ b/vertical-pod-autoscaler/pkg/target/controller_fetcher/controller_fetcher.go @@ -271,7 +271,7 @@ func (f *controllerFetcher) isWellKnownOrScalable(ctx context.Context, key *Cont mappings, err := f.mapper.RESTMappings(groupKind) if err != nil { - klog.ErrorS(err, "Could not find mappings", "object", klog.KRef(key.Namespace, key.Name)) + klog.ErrorS(err, "Could not find mappings", "groupKind", groupKind, "object", klog.KRef(key.Namespace, key.Name)) return false } From 0d3e262f4b3b45bb2efe19106201e23e074b2d71 Mon Sep 17 00:00:00 2001 From: Marco Voelz Date: Wed, 27 Nov 2024 11:41:56 +0100 Subject: [PATCH 4/4] apply review suggestions --- .../pkg/recommender/routines/recommender.go | 2 +- vertical-pod-autoscaler/pkg/utils/metrics/quality/quality.go | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/vertical-pod-autoscaler/pkg/recommender/routines/recommender.go b/vertical-pod-autoscaler/pkg/recommender/routines/recommender.go index 2ded58817525..1f456a80c18b 100644 --- a/vertical-pod-autoscaler/pkg/recommender/routines/recommender.go +++ b/vertical-pod-autoscaler/pkg/recommender/routines/recommender.go @@ -111,7 +111,7 @@ func (r *recommender) UpdateVPAs() { if klog.V(4).Enabled() { pods := r.clusterState.GetMatchingPods(vpa) if len(pods) != vpa.PodCount { - klog.ErrorS(nil, "ClusterState pod count and matching pods disagree for VPA", "vpa", klog.KRef(vpa.ID.Namespace, vpa.ID.VpaName), "podCount", vpa.PodCount, "MatchingPods", pods) + klog.ErrorS(nil, "ClusterState pod count and matching pods disagree for VPA", "vpa", klog.KRef(vpa.ID.Namespace, vpa.ID.VpaName), "podCount", vpa.PodCount, "matchingPods", pods) } klog.InfoS("VPA dump", "vpa", vpa, "hasMatchingPods", hasMatchingPods, "podCount", vpa.PodCount, "matchingPods", pods) } diff --git a/vertical-pod-autoscaler/pkg/utils/metrics/quality/quality.go b/vertical-pod-autoscaler/pkg/utils/metrics/quality/quality.go index 3566911f6227..eff116ba9169 100644 --- a/vertical-pod-autoscaler/pkg/utils/metrics/quality/quality.go +++ b/vertical-pod-autoscaler/pkg/utils/metrics/quality/quality.go @@ -204,7 +204,7 @@ func ObserveRecommendationChange(previous, current corev1.ResourceList, updateMo } // This is not really expected thus a warning. if current == nil { - klog.V(0).InfoS("Current recommendation is nil", "update_mode", updateMode, "size", vpaSize) + klog.V(0).InfoS("Current recommendation is nil", "updateMode", updateMode, "size", vpaSize) return } @@ -217,7 +217,7 @@ func ObserveRecommendationChange(previous, current corev1.ResourceList, updateMo log2 := metrics.GetVpaSizeLog2(vpaSize) relativeRecommendationChange.WithLabelValues(updateModeToString(updateMode), string(resource), strconv.Itoa(log2)).Observe(diff) } else { - klog.V(0).InfoS("Cannot compare as old recommendation is 0", "resource", resource, "vpa_mode", updateMode, "size", vpaSize) + klog.V(0).InfoS("Cannot compare as old recommendation is 0", "resource", resource, "vpaMode", updateMode, "size", vpaSize) } } }