From 13b6d909ab41711734a6a891d4ae7c9655bb6f17 Mon Sep 17 00:00:00 2001 From: Omer Aplatony Date: Wed, 18 Sep 2024 14:47:22 +0300 Subject: [PATCH 01/18] Refactor logging in recommender, updater, and target components Signed-off-by: Omer Aplatony --- .../pkg/recommender/main.go | 7 +++--- .../pkg/recommender/model/cluster.go | 6 ++--- .../pkg/recommender/model/types.go | 4 ++-- .../routines/capping_post_processor.go | 2 +- .../pkg/recommender/routines/recommender.go | 19 ++++++--------- .../controller_cache_storage.go | 4 ++-- .../controller_fetcher/controller_fetcher.go | 10 ++++---- .../eviction/pods_eviction_restriction.go | 13 ++++------- .../pkg/updater/logic/updater.go | 16 ++++++------- vertical-pod-autoscaler/pkg/updater/main.go | 4 ++-- .../updater/priority/priority_processor.go | 3 +-- .../priority/update_priority_calculator.go | 23 +++++++++---------- .../pkg/utils/status/status_updater.go | 2 +- vertical-pod-autoscaler/pkg/utils/vpa/api.go | 8 +++---- .../pkg/utils/vpa/capping.go | 2 +- 15 files changed, 56 insertions(+), 67 deletions(-) diff --git a/vertical-pod-autoscaler/pkg/recommender/main.go b/vertical-pod-autoscaler/pkg/recommender/main.go index 7a73d5816d79..d764cc1bf8cc 100644 --- a/vertical-pod-autoscaler/pkg/recommender/main.go +++ b/vertical-pod-autoscaler/pkg/recommender/main.go @@ -121,8 +121,7 @@ func main() { componentbaseoptions.BindLeaderElectionFlags(&leaderElection, pflag.CommandLine) kube_flag.InitFlags() - klog.V(1).Infof("Vertical Pod Autoscaler %s Recommender: %v", common.VerticalPodAutoscalerVersion, *recommenderName) - + klog.V(1).InfoS("Vertical Pod Autoscaler Recommender", "version", common.VerticalPodAutoscalerVersion, "recommenderName", *recommenderName) if len(*vpaObjectNamespace) > 0 && len(*ignoredVpaObjectNamespaces) > 0 { klog.Fatalf("--vpa-object-namespace and --ignored-vpa-object-namespaces are mutually exclusive and can't be set together.") } @@ -223,10 +222,10 @@ func run(healthCheck *metrics.HealthCheck) { resourceMetrics[apiv1.ResourceMemory] = *externalMemoryMetric } externalClientOptions := &input_metrics.ExternalClientOptions{ResourceMetrics: resourceMetrics, ContainerNameLabel: *ctrNameLabel} - klog.V(1).Infof("Using External Metrics: %+v", externalClientOptions) + klog.V(1).InfoS("Using External Metrics", "options", externalClientOptions) source = input_metrics.NewExternalClient(config, clusterState, *externalClientOptions) } else { - klog.V(1).Infof("Using Metrics Server.") + klog.V(1).InfoS("Using Metrics Server") source = input_metrics.NewPodMetricsesSource(resourceclient.NewForConfigOrDie(config)) } diff --git a/vertical-pod-autoscaler/pkg/recommender/model/cluster.go b/vertical-pod-autoscaler/pkg/recommender/model/cluster.go index ff60d4cf0a5b..8fd2fd8f030e 100644 --- a/vertical-pod-autoscaler/pkg/recommender/model/cluster.go +++ b/vertical-pod-autoscaler/pkg/recommender/model/cluster.go @@ -362,19 +362,19 @@ func (cluster *ClusterState) findOrCreateAggregateContainerState(containerID Con // 2) The last sample is too old to give meaningful recommendation (>8 days), // 3) There are no samples and the aggregate state was created >8 days ago. func (cluster *ClusterState) garbageCollectAggregateCollectionStates(ctx context.Context, now time.Time, controllerFetcher controllerfetcher.ControllerFetcher) { - klog.V(1).Info("Garbage collection of AggregateCollectionStates triggered") + klog.V(1).InfoS("Garbage collection of AggregateCollectionStates triggered") keysToDelete := make([]AggregateStateKey, 0) contributiveKeys := cluster.getContributiveAggregateStateKeys(ctx, controllerFetcher) for key, aggregateContainerState := range cluster.aggregateStateMap { isKeyContributive := contributiveKeys[key] if !isKeyContributive && aggregateContainerState.isEmpty() { keysToDelete = append(keysToDelete, key) - klog.V(1).Infof("Removing empty and not contributive AggregateCollectionState for %+v", key) + klog.V(1).InfoS("Removing empty and not contributive AggregateCollectionState", "key", key) continue } if aggregateContainerState.isExpired(now) { keysToDelete = append(keysToDelete, key) - klog.V(1).Infof("Removing expired AggregateCollectionState for %+v", key) + klog.V(1).InfoS("Removing expired AggregateCollectionState", "key", key) } } for _, key := range keysToDelete { diff --git a/vertical-pod-autoscaler/pkg/recommender/model/types.go b/vertical-pod-autoscaler/pkg/recommender/model/types.go index 85263c60ea1b..dcbacfec82ef 100644 --- a/vertical-pod-autoscaler/pkg/recommender/model/types.go +++ b/vertical-pod-autoscaler/pkg/recommender/model/types.go @@ -92,7 +92,7 @@ func ResourcesAsResourceList(resources Resources) apiv1.ResourceList { newKey = apiv1.ResourceMemory quantity = QuantityFromMemoryAmount(resourceAmount) default: - klog.Errorf("Cannot translate %v resource name", key) + klog.ErrorS(nil, "Cannot translate resource name", "resourceName", key) continue } result[newKey] = quantity @@ -110,7 +110,7 @@ func ResourceNamesApiToModel(resources []apiv1.ResourceName) *[]ResourceName { case apiv1.ResourceMemory: result = append(result, ResourceMemory) default: - klog.Errorf("Cannot translate %v resource name", resource) + klog.ErrorS(nil, "Cannot traslate resource name", "resourceName", resource) continue } } diff --git a/vertical-pod-autoscaler/pkg/recommender/routines/capping_post_processor.go b/vertical-pod-autoscaler/pkg/recommender/routines/capping_post_processor.go index 684028d9c187..851a1c4c5575 100644 --- a/vertical-pod-autoscaler/pkg/recommender/routines/capping_post_processor.go +++ b/vertical-pod-autoscaler/pkg/recommender/routines/capping_post_processor.go @@ -34,7 +34,7 @@ func (c CappingPostProcessor) Process(vpa *vpa_types.VerticalPodAutoscaler, reco // TODO: maybe rename the vpa_utils.ApplyVPAPolicy to something that mention that it is doing capping only cappedRecommendation, err := vpa_utils.ApplyVPAPolicy(recommendation, vpa.Spec.ResourcePolicy) if err != nil { - klog.Errorf("Failed to apply policy for VPA %s: %v", klog.KObj(vpa), err) + klog.ErrorS(err, "Failed to apply policy for VPA", "vpa", klog.KObj(vpa)) return recommendation } return cappedRecommendation diff --git a/vertical-pod-autoscaler/pkg/recommender/routines/recommender.go b/vertical-pod-autoscaler/pkg/recommender/routines/recommender.go index 4a87b21cf778..eb1cf36392c3 100644 --- a/vertical-pod-autoscaler/pkg/recommender/routines/recommender.go +++ b/vertical-pod-autoscaler/pkg/recommender/routines/recommender.go @@ -109,15 +109,11 @@ func (r *recommender) UpdateVPAs() { if err := r.clusterState.RecordRecommendation(vpa, time.Now()); err != nil { klog.Warningf("%v", err) if klog.V(4).Enabled() { - klog.Infof("VPA dump") - klog.Infof("%+v", vpa) - klog.Infof("HasMatchingPods: %v", hasMatchingPods) - klog.Infof("PodCount: %v", vpa.PodCount) pods := r.clusterState.GetMatchingPods(vpa) - klog.Infof("MatchingPods: %+v", pods) if len(pods) != vpa.PodCount { - klog.Errorf("ClusterState pod count and matching pods disagree for VPA %s", klog.KRef(vpa.ID.Namespace, vpa.ID.VpaName)) + 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) } } cnt.Add(vpa) @@ -125,8 +121,7 @@ func (r *recommender) UpdateVPAs() { _, err := vpa_utils.UpdateVpaStatusIfNeeded( r.vpaClient.VerticalPodAutoscalers(vpa.ID.Namespace), vpa.ID.VpaName, vpa.AsStatus(), &observedVpa.Status) if err != nil { - klog.Errorf( - "Cannot update VPA %s object. Reason: %+v", klog.KRef(vpa.ID.Namespace, vpa.ID.VpaName), err) + klog.ErrorS(err, "Cannot update VPA", "vpa", klog.KRef(vpa.ID.Namespace, vpa.ID.VpaName)) } } } @@ -150,7 +145,7 @@ func (r *recommender) RunOnce() { ctx := context.Background() - klog.V(3).Infof("Recommender Run") + klog.V(3).InfoS("Recommender Run") r.clusterStateFeeder.LoadVPAs(ctx) timer.ObserveStep("LoadVPAs") @@ -160,7 +155,7 @@ func (r *recommender) RunOnce() { r.clusterStateFeeder.LoadRealTimeMetrics() timer.ObserveStep("LoadMetrics") - klog.V(3).Infof("ClusterState is tracking %v PodStates and %v VPAs", len(r.clusterState.Pods), len(r.clusterState.Vpas)) + klog.V(3).InfoS("ClusterState is tracking", "pods", len(r.clusterState.Pods), "vpas", len(r.clusterState.Vpas)) r.UpdateVPAs() timer.ObserveStep("UpdateVPAs") @@ -172,7 +167,7 @@ func (r *recommender) RunOnce() { r.clusterState.RateLimitedGarbageCollectAggregateCollectionStates(ctx, time.Now(), r.controllerFetcher) timer.ObserveStep("GarbageCollect") - klog.V(3).Infof("ClusterState is tracking %d aggregated container states", r.clusterState.StateMapSize()) + klog.V(3).InfoS("ClusterState is tracking", "aggregateContainerStates", r.clusterState.StateMapSize()) } // RecommenderFactory makes instances of Recommender. @@ -207,6 +202,6 @@ func (c RecommenderFactory) Make() Recommender { lastAggregateContainerStateGC: time.Now(), lastCheckpointGC: time.Now(), } - klog.V(3).Infof("New Recommender created %+v", recommender) + klog.V(3).InfoS("New Recommender created", "recommender", recommender) return recommender } diff --git a/vertical-pod-autoscaler/pkg/target/controller_fetcher/controller_cache_storage.go b/vertical-pod-autoscaler/pkg/target/controller_fetcher/controller_cache_storage.go index 06ecb9b5d47c..0a2730237a54 100644 --- a/vertical-pod-autoscaler/pkg/target/controller_fetcher/controller_cache_storage.go +++ b/vertical-pod-autoscaler/pkg/target/controller_fetcher/controller_cache_storage.go @@ -132,7 +132,7 @@ func (cc *controllerCacheStorage) Insert(namespace string, groupResource schema. // Removes entries which we didn't read in a while from the cache. func (cc *controllerCacheStorage) RemoveExpired() { - klog.V(5).Info("Removing entries from controllerCacheStorage") + klog.V(5).InfoS("Removing entries from controllerCacheStorage") cc.mux.Lock() defer cc.mux.Unlock() now := now() @@ -143,7 +143,7 @@ func (cc *controllerCacheStorage) RemoveExpired() { delete(cc.cache, k) } } - klog.V(5).Infof("Removed %d entries from controllerCacheStorage", removed) + klog.V(5).InfoS("Removed entries from controllerCacheStorage", "removed", removed) } // Returns a list of keys for which values need to be refreshed 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 13199c13f52c..68163712b498 100644 --- a/vertical-pod-autoscaler/pkg/target/controller_fetcher/controller_fetcher.go +++ b/vertical-pod-autoscaler/pkg/target/controller_fetcher/controller_fetcher.go @@ -92,12 +92,12 @@ func (f *controllerFetcher) periodicallyRefreshCache(ctx context.Context, period return case <-time.After(period): keysToRefresh := f.scaleSubresourceCacheStorage.GetKeysToRefresh() - klog.V(5).Info("Starting to refresh entries in controllerFetchers scaleSubresourceCacheStorage") + klog.V(5).InfoS("Starting to refresh entries in controllerFetchers scaleSubresourceCacheStorage") for _, item := range keysToRefresh { scale, err := f.scaleNamespacer.Scales(item.namespace).Get(context.TODO(), item.groupResource, item.name, metav1.GetOptions{}) f.scaleSubresourceCacheStorage.Refresh(item.namespace, item.groupResource, item.name, scale, err) } - klog.V(5).Infof("Finished refreshing %d entries in controllerFetchers scaleSubresourceCacheStorage", len(keysToRefresh)) + klog.V(5).InfoS("Finished refreshing entries in controllerFetchers scaleSubresourceCacheStorage", "refreshed", len(keysToRefresh)) f.scaleSubresourceCacheStorage.RemoveExpired() } } @@ -138,7 +138,7 @@ func NewControllerFetcher(config *rest.Config, kubeClient kube_client.Interface, if !synced { klog.Warningf("Could not sync cache for %s: %v", kind, err) } else { - klog.Infof("Initial sync of %s completed", kind) + klog.InfoS("Initial sync completed", "kind", kind) } } @@ -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.Errorf("Could not find groupKind for %s/%s: %v", key.Namespace, key.Name, err) + klog.ErrorS(err, "Could not find groupKind", "namespace", key.Namespace, "name", 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.Errorf("Could not find mappings for %s: %v", groupKind, err) + klog.ErrorS(err, "Could not find mappings", "namespace", key.Namespace, "name", 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 2ef319a96f2f..0d75bc55f653 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, eventRecorder } err := e.client.CoreV1().Pods(podToEvict.Namespace).EvictV1(context.TODO(), eviction) if err != nil { - klog.Errorf("failed to evict pod %s/%s, error: %v", podToEvict.Namespace, podToEvict.Name, err) + klog.ErrorS(err, "Failed to evict pod", "namespace", podToEvict.Namespace, "name", podToEvict.Name) return err } eventRecorder.Event(podToEvict, apiv1.EventTypeNormal, "EvictedByVPA", @@ -199,7 +199,7 @@ func (f *podsEvictionRestrictionFactoryImpl) NewPodsEvictionRestriction(pods []* for _, pod := range pods { creator, err := getPodReplicaCreator(pod) if err != nil { - klog.Errorf("failed to obtain replication info for pod %s: %v", klog.KObj(pod), err) + klog.ErrorS(err, "Failed to obtain replication info for pod", "pod", klog.KObj(pod)) continue } if creator == nil { @@ -216,15 +216,13 @@ func (f *podsEvictionRestrictionFactoryImpl) NewPodsEvictionRestriction(pods []* required := f.minReplicas if vpa.Spec.UpdatePolicy != nil && vpa.Spec.UpdatePolicy.MinReplicas != nil { required = int(*vpa.Spec.UpdatePolicy.MinReplicas) - klog.V(3).Infof("overriding minReplicas from global %v to per-VPA %v for VPA %s", - f.minReplicas, required, klog.KObj(vpa)) + klog.V(3).InfoS("Overriding minReplicas from global to per-VPA value", "globalMinReplicas", f.minReplicas, "vpaMinReplicas", required, "vpa", klog.KObj(vpa)) } for creator, replicas := range livePods { actual := len(replicas) if actual < required { - klog.V(2).Infof("too few replicas for %v %v/%v. Found %v live pods, needs %v (global %v)", - creator.Kind, creator.Namespace, creator.Name, actual, required, f.minReplicas) + klog.V(2).InfoS("Too few replicas", "kind", creator.Kind, "namespace", creator.Namespace, "name", creator.Name, "livePods", actual, "requiredPods", required, "globalMinReplicas", f.minReplicas) continue } @@ -236,8 +234,7 @@ func (f *podsEvictionRestrictionFactoryImpl) NewPodsEvictionRestriction(pods []* var err error configured, err = f.getReplicaCount(creator) if err != nil { - klog.Errorf("failed to obtain replication info for %v %v/%v. %v", - creator.Kind, creator.Namespace, creator.Name, err) + klog.ErrorS(err, "Failed to obtain replication info", "kind", creator.Kind, "namespace", creator.Namespace, "name", creator.Name) continue } } diff --git a/vertical-pod-autoscaler/pkg/updater/logic/updater.go b/vertical-pod-autoscaler/pkg/updater/logic/updater.go index 638d3623bed5..89c587c7b656 100644 --- a/vertical-pod-autoscaler/pkg/updater/logic/updater.go +++ b/vertical-pod-autoscaler/pkg/updater/logic/updater.go @@ -122,7 +122,7 @@ func (u *updater) RunOnce(ctx context.Context) { if u.useAdmissionControllerStatus { isValid, err := u.statusValidator.IsStatusValid(ctx, status.AdmissionControllerStatusTimeout) if err != nil { - klog.Errorf("Error getting Admission Controller status: %v. Skipping eviction loop", err) + klog.ErrorS(err, "Error getting Admission Controller status. Skipping eviction loop") return } if !isValid { @@ -142,17 +142,17 @@ func (u *updater) RunOnce(ctx context.Context) { for _, vpa := range vpaList { if slices.Contains(u.ignoredNamespaces, vpa.Namespace) { - klog.V(3).Infof("skipping VPA object %s in namespace %s as namespace is ignored", vpa.Name, vpa.Namespace) + klog.V(3).InfoS("Skipping VPA object in ignored namespace", "vpa", klog.KObj(vpa), "namespace", vpa.Namespace) continue } if vpa_api_util.GetUpdateMode(vpa) != vpa_types.UpdateModeRecreate && vpa_api_util.GetUpdateMode(vpa) != vpa_types.UpdateModeAuto { - klog.V(3).Infof("skipping VPA object %s because its mode is not \"Recreate\" or \"Auto\"", klog.KObj(vpa)) + klog.V(3).InfoS("Skipping VPA object because its mode is not \"Recreate\" or \"Auto\"", "vpa", klog.KObj(vpa)) continue } selector, err := u.selectorFetcher.Fetch(ctx, vpa) if err != nil { - klog.V(3).Infof("skipping VPA object %s because we cannot fetch selector", klog.KObj(vpa)) + klog.V(3).InfoS("Skipping VPA object because we cannot fetch selector", "vpa", klog.KObj(vpa)) continue } @@ -172,7 +172,7 @@ func (u *updater) RunOnce(ctx context.Context) { podsList, err := u.podLister.List(labels.Everything()) if err != nil { - klog.Errorf("failed to get pods list: %v", err) + klog.ErrorS(err, "Failed to get pods list") return } timer.ObserveStep("ListPods") @@ -225,7 +225,7 @@ func (u *updater) RunOnce(ctx context.Context) { klog.Warningf("evicting pod %s failed: %v", klog.KObj(pod), err) return } - klog.V(2).Infof("evicting pod %s", klog.KObj(pod)) + klog.V(2).InfoS("Evicting pod", "pod", klog.KObj(pod)) evictErr := evictionLimiter.Evict(pod, u.eventRecorder) if evictErr != nil { klog.Warningf("evicting pod %s failed: %v", klog.KObj(pod), evictErr) @@ -251,7 +251,7 @@ func getRateLimiter(evictionRateLimit float64, evictionRateLimitBurst int) *rate // As a special case if the rate is set to rate.Inf, the burst rate is ignored // see https://github.com/golang/time/blob/master/rate/rate.go#L37 evictionRateLimiter = rate.NewLimiter(rate.Inf, 0) - klog.V(1).Info("Rate limit disabled") + klog.V(1).InfoS("Rate limit disabled") } else { evictionRateLimiter = rate.NewLimiter(rate.Limit(evictionRateLimit), evictionRateLimitBurst) } @@ -308,7 +308,7 @@ func newPodLister(kubeClient kube_client.Interface, namespace string) v1lister.P func newEventRecorder(kubeClient kube_client.Interface) record.EventRecorder { eventBroadcaster := record.NewBroadcaster() - eventBroadcaster.StartLogging(klog.V(4).Infof) + eventBroadcaster.StartLogging(klog.V(4).InfoS) if _, isFake := kubeClient.(*fake.Clientset); !isFake { eventBroadcaster.StartRecordingToSink(&clientv1.EventSinkImpl{Interface: clientv1.New(kubeClient.CoreV1().RESTClient()).Events("")}) } diff --git a/vertical-pod-autoscaler/pkg/updater/main.go b/vertical-pod-autoscaler/pkg/updater/main.go index 3a72faad8607..b9aa840b9f77 100644 --- a/vertical-pod-autoscaler/pkg/updater/main.go +++ b/vertical-pod-autoscaler/pkg/updater/main.go @@ -92,7 +92,7 @@ func main() { componentbaseoptions.BindLeaderElectionFlags(&leaderElection, pflag.CommandLine) kube_flag.InitFlags() - klog.V(1).Infof("Vertical Pod Autoscaler %s Updater", common.VerticalPodAutoscalerVersion) + klog.V(1).InfoS("Vertical Pod Autoscaler Updater", "version", common.VerticalPodAutoscalerVersion) if len(*vpaObjectNamespace) > 0 && len(*ignoredVpaObjectNamespaces) > 0 { klog.Fatalf("--vpa-object-namespace and --ignored-vpa-object-namespaces are mutually exclusive and can't be set together.") @@ -174,7 +174,7 @@ func run(healthCheck *metrics.HealthCheck) { var limitRangeCalculator limitrange.LimitRangeCalculator limitRangeCalculator, err := limitrange.NewLimitsRangeCalculator(factory) if err != nil { - klog.Errorf("Failed to create limitRangeCalculator, falling back to not checking limits. Error message: %s", err) + klog.ErrorS(err, "Failed to create limitRangeCalculator, falling back to not checking limits") limitRangeCalculator = limitrange.NewNoopLimitsCalculator() } admissionControllerStatusNamespace := status.AdmissionControllerStatusNamespace diff --git a/vertical-pod-autoscaler/pkg/updater/priority/priority_processor.go b/vertical-pod-autoscaler/pkg/updater/priority/priority_processor.go index 49c2b49d3e03..14c933af903d 100644 --- a/vertical-pod-autoscaler/pkg/updater/priority/priority_processor.go +++ b/vertical-pod-autoscaler/pkg/updater/priority/priority_processor.go @@ -53,8 +53,7 @@ func (*defaultPriorityProcessor) GetUpdatePriority(pod *apiv1.Pod, _ *vpa_types. for _, podContainer := range pod.Spec.Containers { if hasObservedContainers && !vpaContainerSet.Has(podContainer.Name) { - klog.V(4).Infof("Not listed in %s:%s. Skipping container %s priority calculations", - annotations.VpaObservedContainersLabel, pod.GetAnnotations()[annotations.VpaObservedContainersLabel], podContainer.Name) + klog.V(4).InfoS("Not listed in VPA observed containers label. Skipping container priority calculations", "label", annotations.VpaObservedContainersLabel, "observedContainers", pod.GetAnnotations()[annotations.VpaObservedContainersLabel], "containerName", podContainer.Name) continue } recommendedRequest := vpa_api_util.GetRecommendationForContainer(podContainer.Name, recommendation) diff --git a/vertical-pod-autoscaler/pkg/updater/priority/update_priority_calculator.go b/vertical-pod-autoscaler/pkg/updater/priority/update_priority_calculator.go index 00ac6326b063..3eae0adcc0c3 100644 --- a/vertical-pod-autoscaler/pkg/updater/priority/update_priority_calculator.go +++ b/vertical-pod-autoscaler/pkg/updater/priority/update_priority_calculator.go @@ -84,7 +84,7 @@ func NewUpdatePriorityCalculator(vpa *vpa_types.VerticalPodAutoscaler, func (calc *UpdatePriorityCalculator) AddPod(pod *apiv1.Pod, now time.Time) { processedRecommendation, _, err := calc.recommendationProcessor.Apply(calc.vpa.Status.Recommendation, calc.vpa.Spec.ResourcePolicy, calc.vpa.Status.Conditions, pod) if err != nil { - klog.V(2).Infof("cannot process recommendation for pod %s: %v", klog.KObj(pod), err) + klog.V(2).InfoS("Cannot process recommendation for pod", "pod", klog.KObj(pod), "error", err) return } @@ -98,15 +98,14 @@ func (calc *UpdatePriorityCalculator) AddPod(pod *apiv1.Pod, now time.Time) { if hasObservedContainers && !vpaContainerSet.Has(cs.Name) { // Containers not observed by Admission Controller are not supported // by the quick OOM logic. - klog.V(4).Infof("Not listed in %s:%s. Skipping container %s quick OOM calculations", - annotations.VpaObservedContainersLabel, pod.GetAnnotations()[annotations.VpaObservedContainersLabel], cs.Name) + klog.V(4).InfoS("Not listed in VPA observed containers label. Skipping container quick OOM calculations", "label", annotations.VpaObservedContainersLabel, "observedContainers", pod.GetAnnotations()[annotations.VpaObservedContainersLabel], "containerName", cs.Name) continue } crp := vpa_api_util.GetContainerResourcePolicy(cs.Name, calc.vpa.Spec.ResourcePolicy) if crp != nil && crp.Mode != nil && *crp.Mode == vpa_types.ContainerScalingModeOff { // Containers with ContainerScalingModeOff are not considered // during the quick OOM calculation. - klog.V(4).Infof("Container with ContainerScalingModeOff. Skipping container %s quick OOM calculations", cs.Name) + klog.V(4).InfoS("Container with ContainerScalingModeOff. Skipping container quick OOM calculations", "containerName", cs.Name) continue } terminationState := &cs.LastTerminationState @@ -114,7 +113,7 @@ func (calc *UpdatePriorityCalculator) AddPod(pod *apiv1.Pod, now time.Time) { terminationState.Terminated.Reason == "OOMKilled" && terminationState.Terminated.FinishedAt.Time.Sub(terminationState.Terminated.StartedAt.Time) < *evictAfterOOMThreshold { quickOOM = true - klog.V(2).Infof("quick OOM detected in pod %s, container %v", klog.KObj(pod), cs.Name) + klog.V(2).InfoS("Quick OOM detected in pod", "pod", klog.KObj(pod), "containerName", cs.Name) } } @@ -125,25 +124,25 @@ func (calc *UpdatePriorityCalculator) AddPod(pod *apiv1.Pod, now time.Time) { if !updatePriority.OutsideRecommendedRange && !quickOOM { if pod.Status.StartTime == nil { // TODO: Set proper condition on the VPA. - klog.V(4).Infof("not updating pod %s, missing field pod.Status.StartTime", klog.KObj(pod)) + klog.V(4).InfoS("Not updating pod, missing field pod.Status.StartTime", "pod", klog.KObj(pod)) return } if now.Before(pod.Status.StartTime.Add(*podLifetimeUpdateThreshold)) { - klog.V(4).Infof("not updating a short-lived pod %s, request within recommended range", klog.KObj(pod)) + klog.V(4).InfoS("Not updating a short-lived pod, request within recommended range", "pod", klog.KObj(pod)) return } if updatePriority.ResourceDiff < calc.config.MinChangePriority { - klog.V(4).Infof("not updating pod %s, resource diff too low: %v", klog.KObj(pod), updatePriority) + klog.V(4).InfoS("Not updating pod, resource diff too low", "pod", klog.KObj(pod), "updatePriority", updatePriority) return } } // If the pod has quick OOMed then evict only if the resources will change if quickOOM && updatePriority.ResourceDiff == 0 { - klog.V(4).Infof("not updating pod %s because resource would not change", klog.KObj(pod)) + klog.V(4).InfoS("Not updating pod because resource would not change", "pod", klog.KObj(pod)) return } - klog.V(2).Infof("pod accepted for update %s with priority %v - processed recommendations:\n%v", klog.KObj(pod), updatePriority.ResourceDiff, calc.GetProcessedRecommendationTargets(processedRecommendation)) + klog.V(2).InfoS("Pod accepted for update", "pod", klog.KObj(pod), "updatePriority", updatePriority.ResourceDiff, "processedRecommendations", calc.GetProcessedRecommendationTargets(processedRecommendation)) calc.pods = append(calc.pods, prioritizedPod{ pod: pod, priority: updatePriority, @@ -159,7 +158,7 @@ func (calc *UpdatePriorityCalculator) GetSortedPods(admission PodEvictionAdmissi if admission.Admit(podPrio.pod, podPrio.recommendation) { result = append(result, podPrio.pod) } else { - klog.V(2).Infof("pod removed from update queue by PodEvictionAdmission: %v", podPrio.pod.Name) + klog.V(2).InfoS("Pod removed from update queue by PodEvictionAdmission", "podName", podPrio.pod.Name) } } @@ -200,7 +199,7 @@ func parseVpaObservedContainers(pod *apiv1.Pod) (bool, sets.String) { vpaContainerSet := sets.NewString() if hasObservedContainers { if containers, err := annotations.ParseVpaObservedContainersValue(observedContainers); err != nil { - klog.Errorf("Vpa annotation %s failed to parse: %v", observedContainers, err) + klog.ErrorS(err, "VPA annotation failed to parse", "annotation", observedContainers) hasObservedContainers = false } else { vpaContainerSet.Insert(containers...) diff --git a/vertical-pod-autoscaler/pkg/utils/status/status_updater.go b/vertical-pod-autoscaler/pkg/utils/status/status_updater.go index 34ab9557c24c..f56131d52b36 100644 --- a/vertical-pod-autoscaler/pkg/utils/status/status_updater.go +++ b/vertical-pod-autoscaler/pkg/utils/status/status_updater.go @@ -70,6 +70,6 @@ func (su *Updater) updateStatus() { defer cancel() if err := su.client.UpdateStatus(ctx); err != nil { - klog.Errorf("Status update by %s failed: %v", su.client.holderIdentity, err) + klog.ErrorS(err, "Status update failed", "holderIdentity", su.client.holderIdentity) } } diff --git a/vertical-pod-autoscaler/pkg/utils/vpa/api.go b/vertical-pod-autoscaler/pkg/utils/vpa/api.go index 819502bc5406..303def859c70 100644 --- a/vertical-pod-autoscaler/pkg/utils/vpa/api.go +++ b/vertical-pod-autoscaler/pkg/utils/vpa/api.go @@ -54,7 +54,7 @@ type patchRecord struct { func patchVpaStatus(vpaClient vpa_api.VerticalPodAutoscalerInterface, vpaName string, patches []patchRecord) (result *vpa_types.VerticalPodAutoscaler, err error) { bytes, err := json.Marshal(patches) if err != nil { - klog.Errorf("Cannot marshal VPA status patches %+v. Reason: %+v", patches, err) + klog.ErrorS(err, "Cannot marshal VPA status patches", "patches", patches) return } @@ -89,9 +89,9 @@ func NewVpasLister(vpaClient *vpa_clientset.Clientset, stopChannel <-chan struct vpaLister := vpa_lister.NewVerticalPodAutoscalerLister(indexer) go controller.Run(stopChannel) if !cache.WaitForCacheSync(make(chan struct{}), controller.HasSynced) { - klog.Fatalf("Failed to sync VPA cache during initialization") + klog.ErrorS(nil, "Failed to sync VPA cache during initialization") } else { - klog.Info("Initial VPA synced successfully") + klog.InfoS("Initial VPA synced successfully") } return vpaLister } @@ -150,7 +150,7 @@ func GetControllingVPAForPod(ctx context.Context, pod *core.Pod, vpas []*VpaWith } parentController, err := ctrlFetcher.FindTopMostWellKnownOrScalable(ctx, k) if err != nil { - klog.Errorf("fail to get pod controller: pod=%s err=%s", klog.KObj(pod), err.Error()) + klog.ErrorS(err, "Failed to get pod controller", "pod", klog.KObj(pod)) return nil } if parentController == nil { diff --git a/vertical-pod-autoscaler/pkg/utils/vpa/capping.go b/vertical-pod-autoscaler/pkg/utils/vpa/capping.go index ad2578b175a6..0999c2cc4c58 100644 --- a/vertical-pod-autoscaler/pkg/utils/vpa/capping.go +++ b/vertical-pod-autoscaler/pkg/utils/vpa/capping.go @@ -76,7 +76,7 @@ func (c *cappingRecommendationProcessor) Apply( container := getContainer(containerRecommendation.ContainerName, pod) if container == nil { - klog.V(2).Infof("no matching Container found for recommendation %s", containerRecommendation.ContainerName) + klog.V(2).InfoS("No matching Container found for recommendation", "containerName", containerRecommendation.ContainerName) continue } From 6680809c1d4e8835ccd2dd7b5071d0a0852b5325 Mon Sep 17 00:00:00 2001 From: Omer Aplatony Date: Thu, 26 Sep 2024 11:27:53 +0300 Subject: [PATCH 02/18] Update vertical-pod-autoscaler/pkg/recommender/routines/recommender.go Co-authored-by: Adrian Moisey --- vertical-pod-autoscaler/pkg/recommender/routines/recommender.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/vertical-pod-autoscaler/pkg/recommender/routines/recommender.go b/vertical-pod-autoscaler/pkg/recommender/routines/recommender.go index eb1cf36392c3..87f1943dbbf6 100644 --- a/vertical-pod-autoscaler/pkg/recommender/routines/recommender.go +++ b/vertical-pod-autoscaler/pkg/recommender/routines/recommender.go @@ -113,7 +113,7 @@ func (r *recommender) UpdateVPAs() { 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.InfoS("VPA dump", "vpa", vpa, "hasMatchingPods", hasMatchingPods, "podCount", vpa.PodCount, "MatchingPods", pods) + klog.InfoS("VPA dump", "vpa", vpa, "hasMatchingPods", hasMatchingPods, "podCount", vpa.PodCount, "matchingPods", pods) } } cnt.Add(vpa) From d3951fecc15c707ab4a7313b79195f8074b15551 Mon Sep 17 00:00:00 2001 From: Omer Aplatony Date: Fri, 18 Oct 2024 15:33:00 +0300 Subject: [PATCH 03/18] fixed typo Co-authored-by: Marco Voelz --- vertical-pod-autoscaler/pkg/recommender/model/types.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/vertical-pod-autoscaler/pkg/recommender/model/types.go b/vertical-pod-autoscaler/pkg/recommender/model/types.go index dcbacfec82ef..64f5a85dee6e 100644 --- a/vertical-pod-autoscaler/pkg/recommender/model/types.go +++ b/vertical-pod-autoscaler/pkg/recommender/model/types.go @@ -110,7 +110,7 @@ func ResourceNamesApiToModel(resources []apiv1.ResourceName) *[]ResourceName { case apiv1.ResourceMemory: result = append(result, ResourceMemory) default: - klog.ErrorS(nil, "Cannot traslate resource name", "resourceName", resource) + klog.ErrorS(nil, "Cannot translate resource name", "resourceName", resource) continue } } From 6530a78ccb00fd9aa2171bfb488192d77ee4801e Mon Sep 17 00:00:00 2001 From: Omer Aplatony Date: Fri, 18 Oct 2024 15:33:22 +0300 Subject: [PATCH 04/18] using klog.KRef Co-authored-by: Marco Voelz --- .../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 68163712b498..94489386b7e4 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", "namespace", key.Namespace, "name", key.Name) + klog.ErrorS(err, "Could not find groupKind", klog.KRef(key.Namespace, key.Name)) return false } From a7d38df343797030d1623a2bb9e713ee734a79b9 Mon Sep 17 00:00:00 2001 From: Omer Aplatony Date: Fri, 18 Oct 2024 15:33:35 +0300 Subject: [PATCH 05/18] using klog.KRef Co-authored-by: Marco Voelz --- .../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 94489386b7e4..855c4db3efec 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", "namespace", key.Namespace, "name", key.Name) + klog.ErrorS(err, "Could not find mappings", klog.KRef(key.Namespace, key.Name)) return false } From 38a44c5ccbf9274e53391840a198aefc4179e312 Mon Sep 17 00:00:00 2001 From: Omer Aplatony Date: Fri, 18 Oct 2024 15:34:24 +0300 Subject: [PATCH 06/18] using klog.KRef Co-authored-by: Marco Voelz --- .../pkg/updater/priority/update_priority_calculator.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/vertical-pod-autoscaler/pkg/updater/priority/update_priority_calculator.go b/vertical-pod-autoscaler/pkg/updater/priority/update_priority_calculator.go index 3eae0adcc0c3..f2a30fdbd05a 100644 --- a/vertical-pod-autoscaler/pkg/updater/priority/update_priority_calculator.go +++ b/vertical-pod-autoscaler/pkg/updater/priority/update_priority_calculator.go @@ -158,7 +158,7 @@ func (calc *UpdatePriorityCalculator) GetSortedPods(admission PodEvictionAdmissi if admission.Admit(podPrio.pod, podPrio.recommendation) { result = append(result, podPrio.pod) } else { - klog.V(2).InfoS("Pod removed from update queue by PodEvictionAdmission", "podName", podPrio.pod.Name) + klog.V(2).InfoS("Pod removed from update queue by PodEvictionAdmission", "pod", klog.KObj(podPrio.pod)) } } From a6780e382e12a26c419ff73cc602ad7c74c9f6a8 Mon Sep 17 00:00:00 2001 From: Omer Aplatony Date: Fri, 18 Oct 2024 15:35:05 +0300 Subject: [PATCH 07/18] using klog.KRef Co-authored-by: Marco Voelz --- .../pkg/updater/priority/update_priority_calculator.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/vertical-pod-autoscaler/pkg/updater/priority/update_priority_calculator.go b/vertical-pod-autoscaler/pkg/updater/priority/update_priority_calculator.go index f2a30fdbd05a..ab725d0f5bc6 100644 --- a/vertical-pod-autoscaler/pkg/updater/priority/update_priority_calculator.go +++ b/vertical-pod-autoscaler/pkg/updater/priority/update_priority_calculator.go @@ -199,7 +199,7 @@ func parseVpaObservedContainers(pod *apiv1.Pod) (bool, sets.String) { vpaContainerSet := sets.NewString() if hasObservedContainers { if containers, err := annotations.ParseVpaObservedContainersValue(observedContainers); err != nil { - klog.ErrorS(err, "VPA annotation failed to parse", "annotation", observedContainers) + klog.ErrorS(err, "VPA annotation failed to parse", "pod", klog.KObj(pod), "annotation", observedContainers) hasObservedContainers = false } else { vpaContainerSet.Insert(containers...) From 1dc6b5d400a65a0eb0cfee5303fc8e21f84831e4 Mon Sep 17 00:00:00 2001 From: Omer Aplatony Date: Fri, 18 Oct 2024 15:35:44 +0300 Subject: [PATCH 08/18] using klog.KRef Co-authored-by: Marco Voelz --- .../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 0d75bc55f653..3ba2504778d9 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, eventRecorder } err := e.client.CoreV1().Pods(podToEvict.Namespace).EvictV1(context.TODO(), eviction) if err != nil { - klog.ErrorS(err, "Failed to evict pod", "namespace", podToEvict.Namespace, "name", podToEvict.Name) + klog.ErrorS(err, "Failed to evict pod", klog.KObj(podToEvict)) return err } eventRecorder.Event(podToEvict, apiv1.EventTypeNormal, "EvictedByVPA", From e100ac1d653227bed54332a9206296759c75b424 Mon Sep 17 00:00:00 2001 From: Omer Aplatony Date: Fri, 18 Oct 2024 15:42:30 +0300 Subject: [PATCH 09/18] using klog.KRef Co-authored-by: Marco Voelz --- .../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 3ba2504778d9..bd4e64ec2982 100644 --- a/vertical-pod-autoscaler/pkg/updater/eviction/pods_eviction_restriction.go +++ b/vertical-pod-autoscaler/pkg/updater/eviction/pods_eviction_restriction.go @@ -222,7 +222,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, "namespace", creator.Namespace, "name", creator.Name, "livePods", actual, "requiredPods", required, "globalMinReplicas", f.minReplicas) + klog.V(2).InfoS("Too few replicas", "kind", creator.Kind, klog.KRef(creator.Namespace, creator.Name), "livePods", actual, "requiredPods", required, "globalMinReplicas", f.minReplicas) continue } From 82972552d3f322353d55d7fa0f96fe8cf4251974 Mon Sep 17 00:00:00 2001 From: Omer Aplatony Date: Fri, 18 Oct 2024 15:45:10 +0300 Subject: [PATCH 10/18] using klog.KRef Co-authored-by: Marco Voelz --- .../pkg/updater/priority/update_priority_calculator.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/vertical-pod-autoscaler/pkg/updater/priority/update_priority_calculator.go b/vertical-pod-autoscaler/pkg/updater/priority/update_priority_calculator.go index ab725d0f5bc6..5fbc868fd083 100644 --- a/vertical-pod-autoscaler/pkg/updater/priority/update_priority_calculator.go +++ b/vertical-pod-autoscaler/pkg/updater/priority/update_priority_calculator.go @@ -98,7 +98,7 @@ func (calc *UpdatePriorityCalculator) AddPod(pod *apiv1.Pod, now time.Time) { if hasObservedContainers && !vpaContainerSet.Has(cs.Name) { // Containers not observed by Admission Controller are not supported // by the quick OOM logic. - klog.V(4).InfoS("Not listed in VPA observed containers label. Skipping container quick OOM calculations", "label", annotations.VpaObservedContainersLabel, "observedContainers", pod.GetAnnotations()[annotations.VpaObservedContainersLabel], "containerName", cs.Name) + klog.V(4).InfoS("Not listed in VPA observed containers label. Skipping container quick OOM calculations", "label", annotations.VpaObservedContainersLabel, "observedContainers", pod.GetAnnotations()[annotations.VpaObservedContainersLabel], "containerName", cs.Name, "vpa", klog.KObj(calc.vpa)) continue } crp := vpa_api_util.GetContainerResourcePolicy(cs.Name, calc.vpa.Spec.ResourcePolicy) From f37f0dcad50afd7c5ce320d4f3162af607d698e5 Mon Sep 17 00:00:00 2001 From: Omer Aplatony Date: Fri, 18 Oct 2024 15:48:37 +0300 Subject: [PATCH 11/18] Use ErrorS Signed-off-by: Omer Aplatony --- .../pkg/updater/priority/update_priority_calculator.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/vertical-pod-autoscaler/pkg/updater/priority/update_priority_calculator.go b/vertical-pod-autoscaler/pkg/updater/priority/update_priority_calculator.go index 5fbc868fd083..8e047bc78062 100644 --- a/vertical-pod-autoscaler/pkg/updater/priority/update_priority_calculator.go +++ b/vertical-pod-autoscaler/pkg/updater/priority/update_priority_calculator.go @@ -84,7 +84,7 @@ func NewUpdatePriorityCalculator(vpa *vpa_types.VerticalPodAutoscaler, func (calc *UpdatePriorityCalculator) AddPod(pod *apiv1.Pod, now time.Time) { processedRecommendation, _, err := calc.recommendationProcessor.Apply(calc.vpa.Status.Recommendation, calc.vpa.Spec.ResourcePolicy, calc.vpa.Status.Conditions, pod) if err != nil { - klog.V(2).InfoS("Cannot process recommendation for pod", "pod", klog.KObj(pod), "error", err) + klog.V(2).ErrorS(err, "Cannot process recommendation for pod", "pod", klog.KObj(pod)) return } From 0e9193f1e99dc3de30131b0eaf4b1f544e20cbc4 Mon Sep 17 00:00:00 2001 From: Omer Aplatony Date: Fri, 18 Oct 2024 15:50:27 +0300 Subject: [PATCH 12/18] using klog.KRef Co-authored-by: Marco Voelz --- .../pkg/updater/priority/priority_processor.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/vertical-pod-autoscaler/pkg/updater/priority/priority_processor.go b/vertical-pod-autoscaler/pkg/updater/priority/priority_processor.go index 14c933af903d..37694a476869 100644 --- a/vertical-pod-autoscaler/pkg/updater/priority/priority_processor.go +++ b/vertical-pod-autoscaler/pkg/updater/priority/priority_processor.go @@ -53,7 +53,7 @@ func (*defaultPriorityProcessor) GetUpdatePriority(pod *apiv1.Pod, _ *vpa_types. for _, podContainer := range pod.Spec.Containers { if hasObservedContainers && !vpaContainerSet.Has(podContainer.Name) { - klog.V(4).InfoS("Not listed in VPA observed containers label. Skipping container priority calculations", "label", annotations.VpaObservedContainersLabel, "observedContainers", pod.GetAnnotations()[annotations.VpaObservedContainersLabel], "containerName", podContainer.Name) + klog.V(4).InfoS("Not listed in VPA observed containers label. Skipping container priority calculations", "label", annotations.VpaObservedContainersLabel, "observedContainers", pod.GetAnnotations()[annotations.VpaObservedContainersLabel], "containerName", podContainer.Name, "vpa", klog.KObj(vpa)) continue } recommendedRequest := vpa_api_util.GetRecommendationForContainer(podContainer.Name, recommendation) From 6b8beaf11dc0854d83ca4c5b7d0aa4cc155b733e Mon Sep 17 00:00:00 2001 From: Omer Aplatony Date: Fri, 18 Oct 2024 15:54:36 +0300 Subject: [PATCH 13/18] Change signature Signed-off-by: Omer Aplatony --- .../pkg/updater/priority/priority_processor.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/vertical-pod-autoscaler/pkg/updater/priority/priority_processor.go b/vertical-pod-autoscaler/pkg/updater/priority/priority_processor.go index 37694a476869..53c07fa7163c 100644 --- a/vertical-pod-autoscaler/pkg/updater/priority/priority_processor.go +++ b/vertical-pod-autoscaler/pkg/updater/priority/priority_processor.go @@ -40,7 +40,7 @@ func NewProcessor() PriorityProcessor { type defaultPriorityProcessor struct { } -func (*defaultPriorityProcessor) GetUpdatePriority(pod *apiv1.Pod, _ *vpa_types.VerticalPodAutoscaler, +func (*defaultPriorityProcessor) GetUpdatePriority(pod *apiv1.Pod, vpa *vpa_types.VerticalPodAutoscaler, recommendation *vpa_types.RecommendedPodResources) PodPriority { outsideRecommendedRange := false scaleUp := false From c72781df3796a26b8ba61292499670f1e5be9954 Mon Sep 17 00:00:00 2001 From: Omer Aplatony Date: Fri, 18 Oct 2024 16:04:54 +0300 Subject: [PATCH 14/18] add vpa to func signature Signed-off-by: Omer Aplatony --- .../pkg/updater/priority/priority_processor.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/vertical-pod-autoscaler/pkg/updater/priority/priority_processor.go b/vertical-pod-autoscaler/pkg/updater/priority/priority_processor.go index 53c07fa7163c..85fe669f05dc 100644 --- a/vertical-pod-autoscaler/pkg/updater/priority/priority_processor.go +++ b/vertical-pod-autoscaler/pkg/updater/priority/priority_processor.go @@ -40,7 +40,7 @@ func NewProcessor() PriorityProcessor { type defaultPriorityProcessor struct { } -func (*defaultPriorityProcessor) GetUpdatePriority(pod *apiv1.Pod, vpa *vpa_types.VerticalPodAutoscaler, +func (*defaultPriorityProcessor) GetUpdatePriority(pod *apiv1.Pod, vpa *vpa_types.VerticalPodAutoscaler, recommendation *vpa_types.RecommendedPodResources) PodPriority { outsideRecommendedRange := false scaleUp := false From 78d06299de8623053f143aa7a5fb5bc5fed175cf Mon Sep 17 00:00:00 2001 From: Omer Aplatony Date: Fri, 18 Oct 2024 16:12:56 +0300 Subject: [PATCH 15/18] Fix spacing issue in GetUpdatePriority function Signed-off-by: Omer Aplatony --- .../pkg/updater/priority/priority_processor.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/vertical-pod-autoscaler/pkg/updater/priority/priority_processor.go b/vertical-pod-autoscaler/pkg/updater/priority/priority_processor.go index 85fe669f05dc..53c07fa7163c 100644 --- a/vertical-pod-autoscaler/pkg/updater/priority/priority_processor.go +++ b/vertical-pod-autoscaler/pkg/updater/priority/priority_processor.go @@ -40,7 +40,7 @@ func NewProcessor() PriorityProcessor { type defaultPriorityProcessor struct { } -func (*defaultPriorityProcessor) GetUpdatePriority(pod *apiv1.Pod, vpa *vpa_types.VerticalPodAutoscaler, +func (*defaultPriorityProcessor) GetUpdatePriority(pod *apiv1.Pod, vpa *vpa_types.VerticalPodAutoscaler, recommendation *vpa_types.RecommendedPodResources) PodPriority { outsideRecommendedRange := false scaleUp := false From e694e09887466f08cf3924be7b0bf01b5d99a9a5 Mon Sep 17 00:00:00 2001 From: Omer Aplatony Date: Fri, 18 Oct 2024 16:24:33 +0300 Subject: [PATCH 16/18] use klog.KRef Signed-off-by: Omer Aplatony --- .../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 bd4e64ec2982..b900d379d85a 100644 --- a/vertical-pod-autoscaler/pkg/updater/eviction/pods_eviction_restriction.go +++ b/vertical-pod-autoscaler/pkg/updater/eviction/pods_eviction_restriction.go @@ -234,7 +234,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, "namespace", creator.Namespace, "name", creator.Name) + klog.ErrorS(err, "Failed to obtain replication info", "kind", creator.Kind, klog.KRef(creator.Namespace, creator.Name)) continue } } From 16272cb69038c2fb56d73d42dbf40e9386be46ce Mon Sep 17 00:00:00 2001 From: Omer Aplatony Date: Sun, 20 Oct 2024 21:24:27 +0300 Subject: [PATCH 17/18] Refactor logging statements to use klog.V(0).InfoS() instead of klog.Warningf(). Signed-off-by: Omer Aplatony --- .../pkg/recommender/input/cluster_feeder.go | 10 +++++----- .../pkg/recommender/model/container.go | 2 +- .../pkg/recommender/routines/recommender.go | 4 ++-- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/vertical-pod-autoscaler/pkg/recommender/input/cluster_feeder.go b/vertical-pod-autoscaler/pkg/recommender/input/cluster_feeder.go index 9d6f92a61d40..41b4e6292d1b 100644 --- a/vertical-pod-autoscaler/pkg/recommender/input/cluster_feeder.go +++ b/vertical-pod-autoscaler/pkg/recommender/input/cluster_feeder.go @@ -213,7 +213,7 @@ func (feeder *clusterStateFeeder) InitFromHistoryProvider(historyProvider histor ContainerName: containerName, } if err = feeder.clusterState.AddOrUpdateContainer(containerID, nil); err != nil { - klog.Warningf("Failed to add container %+v. Reason: %+v", containerID, err) + klog.V(0).InfoS("Failed to add container", "containerID", containerID, "err", err) } klog.V(4).Infof("Adding %d samples for container %v", len(sampleList), containerID) for _, sample := range sampleList { @@ -222,7 +222,7 @@ func (feeder *clusterStateFeeder) InitFromHistoryProvider(historyProvider histor ContainerUsageSample: sample, Container: containerID, }); err != nil { - klog.Warningf("Error adding metric sample for container %v: %v", containerID, err) + klog.V(0).InfoS("Failed to add sample", "sample", sample, "containerID", containerID, "err", err) } } } @@ -419,7 +419,7 @@ func (feeder *clusterStateFeeder) LoadPods() { feeder.clusterState.AddOrUpdatePod(pod.ID, pod.PodLabels, pod.Phase) for _, container := range pod.Containers { if err = feeder.clusterState.AddOrUpdateContainer(container.ID, container.Request); err != nil { - klog.Warningf("Failed to add container %+v. Reason: %+v", container.ID, err) + klog.V(0).InfoS("Failed to add container", "containerID", container.ID, "err", err) } } } @@ -440,7 +440,7 @@ func (feeder *clusterStateFeeder) LoadRealTimeMetrics() { if _, isKeyError := err.(model.KeyError); isKeyError && feeder.memorySaveMode { continue } - klog.Warningf("Error adding metric sample for container %v: %v", sample.Container, err) + klog.V(0).InfoS("Failed to add sample for container", "containerID", sample.Container, "err", err) droppedSampleCount++ } else { sampleCount++ @@ -454,7 +454,7 @@ Loop: case oomInfo := <-feeder.oomChan: klog.V(3).Infof("OOM detected %+v", oomInfo) if err = feeder.clusterState.RecordOOM(oomInfo.ContainerID, oomInfo.Timestamp, oomInfo.Memory); err != nil { - klog.Warningf("Failed to record OOM %+v. Reason: %+v", oomInfo, err) + klog.V(0).InfoS("Failed to record OOM", "oomInfo", oomInfo, "err", err) } default: break Loop diff --git a/vertical-pod-autoscaler/pkg/recommender/model/container.go b/vertical-pod-autoscaler/pkg/recommender/model/container.go index 405d44023048..0fde110e27aa 100644 --- a/vertical-pod-autoscaler/pkg/recommender/model/container.go +++ b/vertical-pod-autoscaler/pkg/recommender/model/container.go @@ -115,7 +115,7 @@ func (container *ContainerState) observeQualityMetrics(usage ResourceAmount, isO case corev1.ResourceMemory: recommendationValue = float64(recommendation.Value()) default: - klog.Warningf("Unknown resource: %v", resource) + klog.V(0).InfoS("Unknown resource", "resource", resource) return } metrics_quality.ObserveQualityMetrics(usageValue, recommendationValue, isOOM, resource, updateMode) diff --git a/vertical-pod-autoscaler/pkg/recommender/routines/recommender.go b/vertical-pod-autoscaler/pkg/recommender/routines/recommender.go index 87f1943dbbf6..2ded58817525 100644 --- a/vertical-pod-autoscaler/pkg/recommender/routines/recommender.go +++ b/vertical-pod-autoscaler/pkg/recommender/routines/recommender.go @@ -107,7 +107,7 @@ func (r *recommender) UpdateVPAs() { hasMatchingPods := vpa.PodCount > 0 vpa.UpdateConditions(hasMatchingPods) if err := r.clusterState.RecordRecommendation(vpa, time.Now()); err != nil { - klog.Warningf("%v", err) + klog.V(0).InfoS("", "err", err) if klog.V(4).Enabled() { pods := r.clusterState.GetMatchingPods(vpa) if len(pods) != vpa.PodCount { @@ -130,7 +130,7 @@ func (r *recommender) MaintainCheckpoints(ctx context.Context, minCheckpointsPer now := time.Now() if r.useCheckpoints { if err := r.checkpointWriter.StoreCheckpoints(ctx, now, minCheckpointsPerRun); err != nil { - klog.Warningf("Failed to store checkpoints. Reason: %+v", err) + klog.V(0).InfoS("Failed to store checkpoints", "err", err) } if time.Since(r.lastCheckpointGC) > r.checkpointsGCInterval { r.lastCheckpointGC = now From 65b7c00b77b4133964a24cedb477a066302f2105 Mon Sep 17 00:00:00 2001 From: Omer Aplatony Date: Sun, 20 Oct 2024 21:28:23 +0300 Subject: [PATCH 18/18] Refactor logging statements to use klog.V(0).InfoS() instead of klog.Warningf(). Signed-off-by: Omer Aplatony --- .../target/controller_fetcher/controller_fetcher.go | 2 +- .../pkg/updater/eviction/pods_eviction_restriction.go | 2 +- vertical-pod-autoscaler/pkg/updater/logic/updater.go | 9 ++++----- .../pkg/utils/metrics/quality/quality.go | 10 +++++----- vertical-pod-autoscaler/pkg/utils/vpa/capping.go | 2 +- 5 files changed, 12 insertions(+), 13 deletions(-) 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 855c4db3efec..933f2680c50f 100644 --- a/vertical-pod-autoscaler/pkg/target/controller_fetcher/controller_fetcher.go +++ b/vertical-pod-autoscaler/pkg/target/controller_fetcher/controller_fetcher.go @@ -136,7 +136,7 @@ func NewControllerFetcher(config *rest.Config, kubeClient kube_client.Interface, go informer.Run(stopCh) synced := cache.WaitForCacheSync(stopCh, informer.HasSynced) if !synced { - klog.Warningf("Could not sync cache for %s: %v", kind, err) + klog.V(0).InfoS("Initial sync failed", "kind", kind) } else { klog.InfoS("Initial sync completed", "kind", kind) } 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 b900d379d85a..74ddbe2ccf04 100644 --- a/vertical-pod-autoscaler/pkg/updater/eviction/pods_eviction_restriction.go +++ b/vertical-pod-autoscaler/pkg/updater/eviction/pods_eviction_restriction.go @@ -203,7 +203,7 @@ func (f *podsEvictionRestrictionFactoryImpl) NewPodsEvictionRestriction(pods []* continue } if creator == nil { - klog.Warningf("pod %s not replicated", pod.Name) + klog.V(0).InfoS("Pod is not managed by any controller", "pod", klog.KObj(pod)) continue } livePods[*creator] = append(livePods[*creator], pod) diff --git a/vertical-pod-autoscaler/pkg/updater/logic/updater.go b/vertical-pod-autoscaler/pkg/updater/logic/updater.go index 89c587c7b656..5352b663e525 100644 --- a/vertical-pod-autoscaler/pkg/updater/logic/updater.go +++ b/vertical-pod-autoscaler/pkg/updater/logic/updater.go @@ -126,8 +126,7 @@ func (u *updater) RunOnce(ctx context.Context) { return } if !isValid { - klog.Warningf("Admission Controller status has been refreshed more than %v ago. Skipping eviction loop", - status.AdmissionControllerStatusTimeout) + klog.V(0).InfoS("Admission Controller status is not valid. Skipping eviction loop", "timeout", status.AdmissionControllerStatusTimeout) return } } @@ -163,7 +162,7 @@ func (u *updater) RunOnce(ctx context.Context) { } if len(vpas) == 0 { - klog.Warningf("no VPA objects to process") + klog.V(0).InfoS("No VPA objects to process") if u.evictionAdmission != nil { u.evictionAdmission.CleanUp() } @@ -222,13 +221,13 @@ func (u *updater) RunOnce(ctx context.Context) { } err := u.evictionRateLimiter.Wait(ctx) if err != nil { - klog.Warningf("evicting pod %s failed: %v", klog.KObj(pod), err) + klog.V(0).InfoS("Eviction rate limiter wait failed", "error", err) return } klog.V(2).InfoS("Evicting pod", "pod", klog.KObj(pod)) evictErr := evictionLimiter.Evict(pod, u.eventRecorder) if evictErr != nil { - klog.Warningf("evicting pod %s failed: %v", klog.KObj(pod), evictErr) + klog.V(0).InfoS("Eviction failed", "error", evictErr, "pod", klog.KObj(pod)) } else { withEvicted = true metrics_updater.AddEvictedPod(vpaSize) diff --git a/vertical-pod-autoscaler/pkg/utils/metrics/quality/quality.go b/vertical-pod-autoscaler/pkg/utils/metrics/quality/quality.go index 55695eab11ea..3566911f6227 100644 --- a/vertical-pod-autoscaler/pkg/utils/metrics/quality/quality.go +++ b/vertical-pod-autoscaler/pkg/utils/metrics/quality/quality.go @@ -167,7 +167,7 @@ func observeUsageRecommendationDiff(usage, recommendation float64, isRecommendat strconv.FormatBool(isRecommendationMissing), strconv.FormatBool(isOOM)).Observe(diff) } default: - klog.Warningf("Unknown resource: %v", resource) + klog.V(0).InfoS("Unknown resource", "resource", resource) } } @@ -179,7 +179,7 @@ func observeRecommendation(recommendation float64, isOOM bool, resource corev1.R case corev1.ResourceMemory: memoryRecommendations.WithLabelValues(updateModeToString(updateMode), strconv.FormatBool(isOOM)).Observe(recommendation) default: - klog.Warningf("Unknown resource: %v", resource) + klog.V(0).InfoS("Unknown resource", "resource", resource) } } @@ -204,7 +204,7 @@ func ObserveRecommendationChange(previous, current corev1.ResourceList, updateMo } // This is not really expected thus a warning. if current == nil { - klog.Warningf("Cannot compare with current recommendation being nil. VPA mode: %v, size: %v", updateMode, vpaSize) + klog.V(0).InfoS("Current recommendation is nil", "update_mode", 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.Warningf("Cannot compare as old recommendation for %v is 0. VPA mode: %v, size: %v", resource, updateMode, vpaSize) + klog.V(0).InfoS("Cannot compare as old recommendation is 0", "resource", resource, "vpa_mode", updateMode, "size", vpaSize) } } } @@ -230,7 +230,7 @@ func quantityAsFloat(resource corev1.ResourceName, quantity resource.Quantity) f case corev1.ResourceMemory: return float64(quantity.Value()) default: - klog.Warningf("Unknown resource: %v", resource) + klog.V(0).InfoS("Unknown resource", "resource", resource) return 0.0 } } diff --git a/vertical-pod-autoscaler/pkg/utils/vpa/capping.go b/vertical-pod-autoscaler/pkg/utils/vpa/capping.go index 0999c2cc4c58..9998a8296501 100644 --- a/vertical-pod-autoscaler/pkg/utils/vpa/capping.go +++ b/vertical-pod-autoscaler/pkg/utils/vpa/capping.go @@ -82,7 +82,7 @@ func (c *cappingRecommendationProcessor) Apply( containerLimitRange, err := c.limitsRangeCalculator.GetContainerLimitRangeItem(pod.Namespace) if err != nil { - klog.Warningf("failed to fetch LimitRange for %v namespace", pod.Namespace) + klog.V(0).InfoS("Failed to fetch LimitRange for namespace", "namespace", pod.Namespace) } updatedContainerResources, containerAnnotations, err := getCappedRecommendationForContainer( *container, &containerRecommendation, policy, containerLimitRange)