From 628fc39443098c7e6683da739c62accf39100c51 Mon Sep 17 00:00:00 2001 From: Ismail Alidzhikov Date: Mon, 10 Jun 2024 16:18:48 +0300 Subject: [PATCH] vpa-recommender: Log object's namespace --- .../checkpoint/checkpoint_writer.go | 2 +- .../pkg/recommender/input/cluster_feeder.go | 22 +++++++++---------- .../input/metrics/metrics_source.go | 7 +++--- .../pkg/recommender/model/cluster.go | 6 ++--- .../routines/capping_post_processor.go | 2 +- .../pkg/recommender/routines/recommender.go | 6 ++--- 6 files changed, 23 insertions(+), 22 deletions(-) diff --git a/vertical-pod-autoscaler/pkg/recommender/checkpoint/checkpoint_writer.go b/vertical-pod-autoscaler/pkg/recommender/checkpoint/checkpoint_writer.go index c738c95e4df1..f9bc6dbf646a 100644 --- a/vertical-pod-autoscaler/pkg/recommender/checkpoint/checkpoint_writer.go +++ b/vertical-pod-autoscaler/pkg/recommender/checkpoint/checkpoint_writer.go @@ -94,7 +94,7 @@ func (writer *checkpointWriter) StoreCheckpoints(ctx context.Context, now time.T for container, aggregatedContainerState := range aggregateContainerStateMap { containerCheckpoint, err := aggregatedContainerState.SaveToCheckpoint() if err != nil { - klog.Errorf("Cannot serialize checkpoint for vpa %v container %v. Reason: %+v", vpa.ID.VpaName, container, err) + klog.Errorf("Cannot serialize checkpoint for vpa %s/%s container %v. Reason: %+v", vpa.ID.Namespace, vpa.ID.VpaName, container, err) continue } checkpointName := fmt.Sprintf("%s-%s", vpa.ID.VpaName, container) diff --git a/vertical-pod-autoscaler/pkg/recommender/input/cluster_feeder.go b/vertical-pod-autoscaler/pkg/recommender/input/cluster_feeder.go index 6da25fea5eb4..67f60397e81f 100644 --- a/vertical-pod-autoscaler/pkg/recommender/input/cluster_feeder.go +++ b/vertical-pod-autoscaler/pkg/recommender/input/cluster_feeder.go @@ -42,7 +42,7 @@ import ( "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/recommender/input/spec" "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/recommender/model" "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/target" - "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/target/controller_fetcher" + controllerfetcher "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/target/controller_fetcher" metrics_recommender "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/utils/metrics/recommender" ) @@ -229,13 +229,13 @@ func (feeder *clusterStateFeeder) setVpaCheckpoint(checkpoint *vpa_types.Vertica vpaID := model.VpaID{Namespace: checkpoint.Namespace, VpaName: checkpoint.Spec.VPAObjectName} vpa, exists := feeder.clusterState.Vpas[vpaID] if !exists { - return fmt.Errorf("cannot load checkpoint to missing VPA object %+v", vpaID) + return fmt.Errorf("cannot load checkpoint to missing VPA object %s/%s", vpa.ID.Namespace, vpa.ID.VpaName) } cs := model.NewAggregateContainerState() err := cs.LoadFromCheckpoint(&checkpoint.Status) if err != nil { - return fmt.Errorf("cannot load checkpoint for VPA %+v. Reason: %v", vpa.ID, err) + return fmt.Errorf("cannot load checkpoint for VPA %s/%s. Reason: %v", vpa.ID.Namespace, vpa.ID.VpaName, err) } vpa.ContainersInitialAggregateState[checkpoint.Spec.ContainerName] = cs return nil @@ -254,7 +254,7 @@ func (feeder *clusterStateFeeder) InitFromCheckpoints() { klog.V(3).Infof("Fetching checkpoints from namespace %s", namespace) checkpointList, err := feeder.vpaCheckpointClient.VerticalPodAutoscalerCheckpoints(namespace).List(context.TODO(), metav1.ListOptions{}) if err != nil { - klog.Errorf("Cannot list VPA checkpoints from namespace %v. Reason: %+v", namespace, err) + klog.Errorf("Cannot list VPA checkpoints from namespace %s. Reason: %+v", namespace, err) } for _, checkpoint := range checkpointList.Items { @@ -319,16 +319,16 @@ func filterVPAs(feeder *clusterStateFeeder, allVpaCRDs []*vpa_types.VerticalPodA for _, vpaCRD := range allVpaCRDs { if feeder.recommenderName == DefaultRecommenderName { if !implicitDefaultRecommender(vpaCRD.Spec.Recommenders) && !selectsRecommender(vpaCRD.Spec.Recommenders, &feeder.recommenderName) { - klog.V(6).Infof("Ignoring vpaCRD %s in namespace %s as current recommender's name %v doesn't appear among its recommenders", vpaCRD.Name, vpaCRD.Namespace, feeder.recommenderName) + klog.V(6).Infof("Ignoring vpaCRD %s as current recommender's name %v doesn't appear among its recommenders", klog.KObj(vpaCRD), feeder.recommenderName) continue } } else { if implicitDefaultRecommender(vpaCRD.Spec.Recommenders) { - klog.V(6).Infof("Ignoring vpaCRD %s in namespace %s as %v recommender doesn't process CRDs implicitly destined to %v recommender", vpaCRD.Name, vpaCRD.Namespace, feeder.recommenderName, DefaultRecommenderName) + klog.V(6).Infof("Ignoring vpaCRD %s as %v recommender doesn't process CRDs implicitly destined to %v recommender", klog.KObj(vpaCRD), feeder.recommenderName, DefaultRecommenderName) continue } if !selectsRecommender(vpaCRD.Spec.Recommenders, &feeder.recommenderName) { - klog.V(6).Infof("Ignoring vpaCRD %s in namespace %s as current recommender's name %v doesn't appear among its recommenders", vpaCRD.Name, vpaCRD.Namespace, feeder.recommenderName) + klog.V(6).Infof("Ignoring vpaCRD %s as current recommender's name %v doesn't appear among its recommenders", klog.KObj(vpaCRD), feeder.recommenderName) continue } } @@ -359,7 +359,7 @@ func (feeder *clusterStateFeeder) LoadVPAs() { } selector, conditions := feeder.getSelector(vpaCRD) - klog.V(4).Infof("Using selector %s for VPA %s/%s", selector.String(), vpaCRD.Namespace, vpaCRD.Name) + klog.V(4).Infof("Using selector %s for VPA %s", selector.String(), klog.KObj(vpaCRD)) if feeder.clusterState.AddOrUpdateVpa(vpaCRD, selector) == nil { // Successfully added VPA to the model. @@ -377,9 +377,9 @@ func (feeder *clusterStateFeeder) LoadVPAs() { // Delete non-existent VPAs from the model. for vpaID := range feeder.clusterState.Vpas { if _, exists := vpaKeys[vpaID]; !exists { - klog.V(3).Infof("Deleting VPA %v", vpaID) + klog.V(3).Infof("Deleting VPA %s/%s", vpaID.Namespace, vpaID.VpaName) if err := feeder.clusterState.DeleteVpa(vpaID); err != nil { - klog.Errorf("Deleting VPA %v failed: %v", vpaID, err) + klog.Errorf("Deleting VPA %s/%s failed: %v", vpaID.Namespace, vpaID.VpaName, err) } } } @@ -398,7 +398,7 @@ func (feeder *clusterStateFeeder) LoadPods() { } for key := range feeder.clusterState.Pods { if _, exists := pods[key]; !exists { - klog.V(3).Infof("Deleting Pod %v", key) + klog.V(3).Infof("Deleting Pod %s/%s", key.Namespace, key.PodName) feeder.clusterState.DeletePod(key) } } diff --git a/vertical-pod-autoscaler/pkg/recommender/input/metrics/metrics_source.go b/vertical-pod-autoscaler/pkg/recommender/input/metrics/metrics_source.go index 04978268baf5..807c4f017a22 100644 --- a/vertical-pod-autoscaler/pkg/recommender/input/metrics/metrics_source.go +++ b/vertical-pod-autoscaler/pkg/recommender/input/metrics/metrics_source.go @@ -18,6 +18,8 @@ package metrics import ( "context" + "time" + k8sapiv1 "k8s.io/api/core/v1" v1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/labels" @@ -28,7 +30,6 @@ import ( "k8s.io/metrics/pkg/apis/metrics/v1beta1" resourceclient "k8s.io/metrics/pkg/client/clientset/versioned/typed/metrics/v1beta1" "k8s.io/metrics/pkg/client/external_metrics" - "time" ) // PodMetricsLister wraps both metrics-client and External Metrics @@ -113,10 +114,10 @@ func (s *externalMetricsClient) List(ctx context.Context, namespace string, opts return nil, err } if m == nil || len(m.Items) == 0 { - klog.V(4).Infof("External Metrics Query for VPA %+v: resource %+v, metric %+v, No items,", vpa.ID, resourceName, metricName) + klog.V(4).Infof("External Metrics Query for VPA %s/%s: resource %+v, metric %+v, No items,", vpa.ID.Namespace, vpa.ID.VpaName, resourceName, metricName) continue } - klog.V(4).Infof("External Metrics Query for VPA %+v: resource %+v, metric %+v, %d items, item[0]: %+v", vpa.ID, resourceName, metricName, len(m.Items), m.Items[0]) + klog.V(4).Infof("External Metrics Query for VPA %s/%s: resource %+v, metric %+v, %d items, item[0]: %+v", vpa.ID.Namespace, vpa.ID.VpaName, resourceName, metricName, len(m.Items), m.Items[0]) podMets.Timestamp = m.Items[0].Timestamp if m.Items[0].WindowSeconds != nil { podMets.Window = v1.Duration{Duration: time.Duration(*m.Items[0].WindowSeconds) * time.Second} diff --git a/vertical-pod-autoscaler/pkg/recommender/model/cluster.go b/vertical-pod-autoscaler/pkg/recommender/model/cluster.go index be3c61d39451..b9912b946ed5 100644 --- a/vertical-pod-autoscaler/pkg/recommender/model/cluster.go +++ b/vertical-pod-autoscaler/pkg/recommender/model/cluster.go @@ -25,7 +25,7 @@ import ( "k8s.io/klog/v2" vpa_types "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/apis/autoscaling.k8s.io/v1" - "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/target/controller_fetcher" + controllerfetcher "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/target/controller_fetcher" vpa_utils "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/utils/vpa" ) @@ -329,7 +329,7 @@ func (cluster *ClusterState) MakeAggregateStateKey(pod *PodState, containerName func (cluster *ClusterState) aggregateStateKeyForContainerID(containerID ContainerID) AggregateStateKey { pod, podExists := cluster.Pods[containerID.PodID] if !podExists { - panic(fmt.Sprintf("Pod not present in the ClusterState: %v", containerID.PodID)) + panic(fmt.Sprintf("Pod not present in the ClusterState: %s/%s", containerID.PodID.Namespace, containerID.PodID.PodName)) } return cluster.MakeAggregateStateKey(pod, containerID.ContainerName) } @@ -433,7 +433,7 @@ func (cluster *ClusterState) RecordRecommendation(vpa *Vpa, now time.Time) error } else { if lastLogged.Add(RecommendationMissingMaxDuration).Before(now) { cluster.EmptyVPAs[vpa.ID] = now - return fmt.Errorf("VPA %v/%v is missing recommendation for more than %v", vpa.ID.Namespace, vpa.ID.VpaName, RecommendationMissingMaxDuration) + return fmt.Errorf("VPA %s/%s is missing recommendation for more than %v", vpa.ID.Namespace, vpa.ID.VpaName, RecommendationMissingMaxDuration) } } return nil 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 e82322802f98..684028d9c187 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 %v/%v: %v", vpa.GetNamespace(), vpa.GetName(), err) + klog.Errorf("Failed to apply policy for VPA %s: %v", klog.KObj(vpa), err) 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 5dc5d1df0d90..139e20425bf1 100644 --- a/vertical-pod-autoscaler/pkg/recommender/routines/recommender.go +++ b/vertical-pod-autoscaler/pkg/recommender/routines/recommender.go @@ -28,7 +28,7 @@ import ( "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/recommender/input" "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/recommender/logic" "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/recommender/model" - "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/target/controller_fetcher" + controllerfetcher "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/target/controller_fetcher" metrics_recommender "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/utils/metrics/recommender" vpa_utils "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/utils/vpa" ) @@ -116,7 +116,7 @@ func (r *recommender) UpdateVPAs() { 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 %v/%v", vpa.ID.Namespace, vpa.ID.VpaName) + klog.Errorf("ClusterState pod count and matching pods disagree for VPA %s/%s", vpa.ID.Namespace, vpa.ID.VpaName) } } } @@ -126,7 +126,7 @@ func (r *recommender) UpdateVPAs() { r.vpaClient.VerticalPodAutoscalers(vpa.ID.Namespace), vpa.ID.VpaName, vpa.AsStatus(), &observedVpa.Status) if err != nil { klog.Errorf( - "Cannot update VPA %v/%v object. Reason: %+v", vpa.ID.Namespace, vpa.ID.VpaName, err) + "Cannot update VPA %s/%s object. Reason: %+v", vpa.ID.Namespace, vpa.ID.VpaName, err) } } }