From 62d7f8f570e629a0bb0fc33c77dc926450e14095 Mon Sep 17 00:00:00 2001 From: Dave Chen Date: Mon, 29 Mar 2021 16:05:56 +0800 Subject: [PATCH] Fix couple of nits in `nodevolumelimits` plugin Signed-off-by: Dave Chen --- .../framework/plugins/nodevolumelimits/csi.go | 28 +++++++++---------- .../plugins/nodevolumelimits/non_csi.go | 18 ++++++------ 2 files changed, 23 insertions(+), 23 deletions(-) diff --git a/pkg/scheduler/framework/plugins/nodevolumelimits/csi.go b/pkg/scheduler/framework/plugins/nodevolumelimits/csi.go index 15895a0acc9..204f022f63a 100644 --- a/pkg/scheduler/framework/plugins/nodevolumelimits/csi.go +++ b/pkg/scheduler/framework/plugins/nodevolumelimits/csi.go @@ -75,14 +75,14 @@ func (pl *CSILimits) Filter(ctx context.Context, _ *framework.CycleState, pod *v node := nodeInfo.Node() if node == nil { - return framework.NewStatus(framework.Error, "node not found") + return framework.NewStatus(framework.Error, fmt.Sprintf("node not found: %s", node.Name)) } // If CSINode doesn't exist, the predicate may read the limits from Node object csiNode, err := pl.csiNodeLister.Get(node.Name) if err != nil { // TODO: return the error once CSINode is created by default (2 releases) - klog.V(5).Infof("Could not get a CSINode object for the node: %v", err) + klog.V(5).InfoS("Could not get a CSINode object for the node", "node", node.Name, "err", err) } newVolumes := make(map[string]string) @@ -149,13 +149,13 @@ func (pl *CSILimits) filterAttachableVolumes( pvc, err := pl.pvcLister.PersistentVolumeClaims(namespace).Get(pvcName) if err != nil { - klog.V(5).Infof("Unable to look up PVC info for %s/%s", namespace, pvcName) + klog.V(5).InfoS("Unable to look up PVC info", "PVC", fmt.Sprintf("%s/%s", namespace, pvcName)) continue } driverName, volumeHandle := pl.getCSIDriverInfo(csiNode, pvc) if driverName == "" || volumeHandle == "" { - klog.V(5).Infof("Could not find a CSI driver name or volume handle, not counting volume") + klog.V(5).Info("Could not find a CSI driver name or volume handle, not counting volume") continue } @@ -175,13 +175,13 @@ func (pl *CSILimits) getCSIDriverInfo(csiNode *storagev1.CSINode, pvc *v1.Persis pvcName := pvc.Name if pvName == "" { - klog.V(5).Infof("Persistent volume had no name for claim %s/%s", namespace, pvcName) + klog.V(5).InfoS("Persistent volume had no name for claim", "PVC", fmt.Sprintf("%s/%s", namespace, pvcName)) return pl.getCSIDriverInfoFromSC(csiNode, pvc) } pv, err := pl.pvLister.Get(pvName) if err != nil { - klog.V(5).Infof("Unable to look up PV info for PVC %s/%s and PV %s", namespace, pvcName, pvName) + klog.V(5).InfoS("Unable to look up PV info for PVC and PV", "PVC", fmt.Sprintf("%s/%s", namespace, pvcName), "PV", pvName) // If we can't fetch PV associated with PVC, may be it got deleted // or PVC was prebound to a PVC that hasn't been created yet. // fallback to using StorageClass for volume counting @@ -197,23 +197,23 @@ func (pl *CSILimits) getCSIDriverInfo(csiNode *storagev1.CSINode, pvc *v1.Persis pluginName, err := pl.translator.GetInTreePluginNameFromSpec(pv, nil) if err != nil { - klog.V(5).Infof("Unable to look up plugin name from PV spec: %v", err) + klog.V(5).InfoS("Unable to look up plugin name from PV spec", "err", err) return "", "" } if !isCSIMigrationOn(csiNode, pluginName) { - klog.V(5).Infof("CSI Migration of plugin %s is not enabled", pluginName) + klog.V(5).InfoS("CSI Migration of plugin is not enabled", "plugin", pluginName) return "", "" } csiPV, err := pl.translator.TranslateInTreePVToCSI(pv) if err != nil { - klog.V(5).Infof("Unable to translate in-tree volume to CSI: %v", err) + klog.V(5).InfoS("Unable to translate in-tree volume to CSI", "err", err) return "", "" } if csiPV.Spec.PersistentVolumeSource.CSI == nil { - klog.V(5).Infof("Unable to get a valid volume source for translated PV %s", pvName) + klog.V(5).InfoS("Unable to get a valid volume source for translated PV", "PV", pvName) return "", "" } @@ -232,13 +232,13 @@ func (pl *CSILimits) getCSIDriverInfoFromSC(csiNode *storagev1.CSINode, pvc *v1. // If StorageClass is not set or not found, then PVC must be using immediate binding mode // and hence it must be bound before scheduling. So it is safe to not count it. if scName == "" { - klog.V(5).Infof("PVC %s/%s has no StorageClass", namespace, pvcName) + klog.V(5).InfoS("PVC has no StorageClass", "PVC", fmt.Sprintf("%s/%s", namespace, pvcName)) return "", "" } storageClass, err := pl.scLister.Get(scName) if err != nil { - klog.V(5).Infof("Could not get StorageClass for PVC %s/%s: %v", namespace, pvcName, err) + klog.V(5).InfoS("Could not get StorageClass for PVC", "PVC", fmt.Sprintf("%s/%s", namespace, pvcName), "err", err) return "", "" } @@ -250,13 +250,13 @@ func (pl *CSILimits) getCSIDriverInfoFromSC(csiNode *storagev1.CSINode, pvc *v1. provisioner := storageClass.Provisioner if pl.translator.IsMigratableIntreePluginByName(provisioner) { if !isCSIMigrationOn(csiNode, provisioner) { - klog.V(5).Infof("CSI Migration of plugin %s is not enabled", provisioner) + klog.V(5).InfoS("CSI Migration of provisioner is not enabled", "provisioner", provisioner) return "", "" } driverName, err := pl.translator.GetCSINameFromInTreeName(provisioner) if err != nil { - klog.V(5).Infof("Unable to look up driver name from plugin name: %v", err) + klog.V(5).InfoS("Unable to look up driver name from provisioner name", "provisioner", provisioner, "err", err) return "", "" } return driverName, volumeHandle diff --git a/pkg/scheduler/framework/plugins/nodevolumelimits/non_csi.go b/pkg/scheduler/framework/plugins/nodevolumelimits/non_csi.go index f0ea1bce0bc..5bb60cc4396 100644 --- a/pkg/scheduler/framework/plugins/nodevolumelimits/non_csi.go +++ b/pkg/scheduler/framework/plugins/nodevolumelimits/non_csi.go @@ -18,6 +18,7 @@ package nodevolumelimits import ( "context" + "errors" "fmt" "os" "regexp" @@ -171,8 +172,7 @@ func newNonCSILimits( filter = cinderVolumeFilter volumeLimitKey = v1.ResourceName(volumeutil.CinderVolumeLimitKey) default: - klog.Fatalf("Wrong filterName, Only Support %v %v %v %v", ebsVolumeFilterType, - gcePDVolumeFilterType, azureDiskVolumeFilterType, cinderVolumeFilterType) + klog.ErrorS(errors.New("Wrong filterName"), "Cannot create nonCSILimits plugin", "candidates", fmt.Sprintf("%v %v %v %v", ebsVolumeFilterType, gcePDVolumeFilterType, azureDiskVolumeFilterType, cinderVolumeFilterType)) return nil } pl := &nonCSILimits{ @@ -215,7 +215,7 @@ func (pl *nonCSILimits) Filter(ctx context.Context, _ *framework.CycleState, pod node := nodeInfo.Node() if node == nil { - return framework.NewStatus(framework.Error, "node not found") + return framework.NewStatus(framework.Error, fmt.Sprintf("node not found: %s", node.Name)) } var csiNode *storage.CSINode @@ -225,7 +225,7 @@ func (pl *nonCSILimits) Filter(ctx context.Context, _ *framework.CycleState, pod if err != nil { // we don't fail here because the CSINode object is only necessary // for determining whether the migration is enabled or not - klog.V(5).Infof("Could not get a CSINode object for the node: %v", err) + klog.V(5).InfoS("Could not get a CSINode object for the node", "node", node.Name, "err", err) } } @@ -287,7 +287,7 @@ func (pl *nonCSILimits) filterVolumes(volumes []v1.Volume, namespace string, fil if err != nil || pvc == nil { // If the PVC is invalid, we don't count the volume because // there's no guarantee that it belongs to the running predicate. - klog.V(4).Infof("Unable to look up PVC info for %s/%s, assuming PVC doesn't match predicate when counting limits: %v", namespace, pvcName, err) + klog.V(4).InfoS("Unable to look up PVC info, assuming PVC doesn't match predicate when counting limits", "PVC", fmt.Sprintf("%s/%s", namespace, pvcName), "err", err) continue } @@ -298,7 +298,7 @@ func (pl *nonCSILimits) filterVolumes(volumes []v1.Volume, namespace string, fil // original PV where it was bound to, so we count the volume if // it belongs to the running predicate. if pl.matchProvisioner(pvc) { - klog.V(4).Infof("PVC %s/%s is not bound, assuming PVC matches predicate when counting limits", namespace, pvcName) + klog.V(4).InfoS("PVC is not bound, assuming PVC matches predicate when counting limits", "PVC", fmt.Sprintf("%s/%s", namespace, pvcName)) filteredVolumes.Insert(pvID) } continue @@ -309,7 +309,7 @@ func (pl *nonCSILimits) filterVolumes(volumes []v1.Volume, namespace string, fil // If the PV is invalid and PVC belongs to the running predicate, // log the error and count the PV towards the PV limit. if pl.matchProvisioner(pvc) { - klog.V(4).Infof("Unable to look up PV info for %s/%s/%s, assuming PV matches predicate when counting limits: %v", namespace, pvcName, pvName, err) + klog.V(4).InfoS("Unable to look up PV, assuming PV matches predicate when counting limits", "PV", fmt.Sprintf("%s/%s/%s", namespace, pvcName, pvName), "err", err) filteredVolumes.Insert(pvID) } continue @@ -342,9 +342,9 @@ func (pl *nonCSILimits) matchProvisioner(pvc *v1.PersistentVolumeClaim) bool { func getMaxVolLimitFromEnv() int { if rawMaxVols := os.Getenv(KubeMaxPDVols); rawMaxVols != "" { if parsedMaxVols, err := strconv.Atoi(rawMaxVols); err != nil { - klog.Errorf("Unable to parse maximum PD volumes value, using default: %v", err) + klog.ErrorS(err, "Unable to parse maximum PD volumes value, using default.") } else if parsedMaxVols <= 0 { - klog.Errorf("Maximum PD volumes must be a positive value, using default") + klog.Error("Maximum PD volumes must be a positive value, using default") } else { return parsedMaxVols }