Merge pull request #710 from ffromani/nrt-klog-to-logr

[noderesourcetopology] move logging to the logr interface

go.mod

@@ -9,7 +9,7 @@ require (
 	github.com/dustin/go-humanize v1.0.1
 	github.com/go-logr/logr v1.3.0
 	github.com/google/go-cmp v0.6.0
-	github.com/k8stopologyawareschedwg/noderesourcetopology-api v0.1.1
+	github.com/k8stopologyawareschedwg/noderesourcetopology-api v0.1.2
 	github.com/k8stopologyawareschedwg/podfingerprint v0.2.2
 	github.com/patrickmn/go-cache v2.1.0+incompatible
 	github.com/paypal/load-watcher v0.2.3

go.sum

@@ -1497,8 +1497,8 @@ github.com/julienschmidt/httprouter v1.2.0/go.mod h1:SYymIcj16QtmaHHD7aYtjjsJG7V
 github.com/julienschmidt/httprouter v1.3.0/go.mod h1:JR6WtHb+2LUe8TCKY3cZOxFyyO8IZAc4RVcycCCAKdM=
 github.com/jung-kurt/gofpdf v1.0.0/go.mod h1:7Id9E/uU8ce6rXgefFLlgrJj/GYY22cpxn+r32jIOes=
 github.com/jung-kurt/gofpdf v1.0.3-0.20190309125859-24315acbbda5/go.mod h1:7Id9E/uU8ce6rXgefFLlgrJj/GYY22cpxn+r32jIOes=
-github.com/k8stopologyawareschedwg/noderesourcetopology-api v0.1.1 h1:BI3L7hNqRvXtB42FO4NI/0ZjDDVRPOMBDFLShhFtf28=
-github.com/k8stopologyawareschedwg/noderesourcetopology-api v0.1.1/go.mod h1:AkACMQGiTgCt0lQw3m7TTU8PLH9lYKNK5e9DqFf5VuM=
+github.com/k8stopologyawareschedwg/noderesourcetopology-api v0.1.2 h1:uAwqOtyrFYggq3pVf3hs1XKkBxrQ8dkgjWz3LCLJsiY=
+github.com/k8stopologyawareschedwg/noderesourcetopology-api v0.1.2/go.mod h1:LBzS4n6GX1C69tzSd5EibZ9cGOXFuHP7GxEMDYVe1sM=
 github.com/k8stopologyawareschedwg/podfingerprint v0.2.2 h1:iFHPfZInM9pz2neye5RdmORMp1hPmte1EGJYpOOzZVg=
 github.com/k8stopologyawareschedwg/podfingerprint v0.2.2/go.mod h1:C23pM15t06dXg/OihGlqBvnYzLr+MXDXJ7zMfbNAyXI=
 github.com/karrick/godirwalk v1.17.0/go.mod h1:j4mkqPuvaLI8mp1DroR3P6ad7cyYd4c1qeJ3RV7ULlk=

pkg/noderesourcetopology/cache/discardreserved.go

@@ -20,13 +20,15 @@ import (
 	"context"
 	"sync"
 
+	"github.com/go-logr/logr"
 	topologyv1alpha2 "github.com/k8stopologyawareschedwg/noderesourcetopology-api/pkg/apis/topology/v1alpha2"
 
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/klog/v2"
 
 	ctrlclient "sigs.k8s.io/controller-runtime/pkg/client"
+	"sigs.k8s.io/scheduler-plugins/pkg/noderesourcetopology/logging"
 )
 
 // DiscardReserved is intended to solve similiar problem as Overreserve Cache,
@@ -45,12 +47,14 @@ type DiscardReserved struct {
 	rMutex         sync.RWMutex
 	reservationMap map[string]map[types.UID]bool // Key is NodeName, value is Pod UID : reserved status
 	client         ctrlclient.Client
+	lh             logr.Logger
 }
 
-func NewDiscardReserved(client ctrlclient.Client) Interface {
+func NewDiscardReserved(lh logr.Logger, client ctrlclient.Client) Interface {
 	return &DiscardReserved{
 		client:         client,
 		reservationMap: make(map[string]map[types.UID]bool),
+		lh:             lh,
 	}
 }
 
@@ -74,7 +78,7 @@ func (pt *DiscardReserved) NodeMaybeOverReserved(nodeName string, pod *corev1.Po
 func (pt *DiscardReserved) NodeHasForeignPods(nodeName string, pod *corev1.Pod)    {}
 
 func (pt *DiscardReserved) ReserveNodeResources(nodeName string, pod *corev1.Pod) {
-	klog.V(5).InfoS("nrtcache NRT Reserve", "logID", klog.KObj(pod), "UID", pod.GetUID(), "node", nodeName)
+	pt.lh.V(5).Info("NRT Reserve", "logID", logging.PodLogID(pod), "podUID", pod.GetUID(), "node", nodeName)
 	pt.rMutex.Lock()
 	defer pt.rMutex.Unlock()
 
@@ -85,14 +89,14 @@ func (pt *DiscardReserved) ReserveNodeResources(nodeName string, pod *corev1.Pod
 }
 
 func (pt *DiscardReserved) UnreserveNodeResources(nodeName string, pod *corev1.Pod) {
-	klog.V(5).InfoS("nrtcache NRT Unreserve", "logID", klog.KObj(pod), "UID", pod.GetUID(), "node", nodeName)
+	pt.lh.V(5).Info("NRT Unreserve", "logID", klog.KObj(pod), "podUID", pod.GetUID(), "node", nodeName)
 
 	pt.removeReservationForNode(nodeName, pod)
 }
 
 // PostBind is invoked to cleanup reservationMap
 func (pt *DiscardReserved) PostBind(nodeName string, pod *corev1.Pod) {
-	klog.V(5).InfoS("nrtcache NRT PostBind", "logID", klog.KObj(pod), "UID", pod.GetUID(), "node", nodeName)
+	pt.lh.V(5).Info("NRT PostBind", "logID", klog.KObj(pod), "podUID", pod.GetUID(), "node", nodeName)
 
 	pt.removeReservationForNode(nodeName, pod)
 }

pkg/noderesourcetopology/cache/discardreserved_test.go

@@ -26,6 +26,7 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/types"
 	podlisterv1 "k8s.io/client-go/listers/core/v1"
+	"k8s.io/klog/v2"
 
 	ctrlclient "sigs.k8s.io/controller-runtime/pkg/client"
 )
@@ -50,7 +51,7 @@ func TestDiscardReservedNodesGetCachedNRTCopy(t *testing.T) {
 	checkGetCachedNRTCopy(
 		t,
 		func(client ctrlclient.Client, _ podlisterv1.PodLister) (Interface, error) {
-			return NewDiscardReserved(client), nil
+			return NewDiscardReserved(klog.Background(), client), nil
 		},
 		testCases...,
 	)

pkg/noderesourcetopology/cache/foreign_pods.go

@@ -17,11 +17,14 @@ limitations under the License.
 package cache
 
 import (
+	"fmt"
+
+	"github.com/go-logr/logr"
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/util/sets"
 	k8scache "k8s.io/client-go/tools/cache"
-	"k8s.io/klog/v2"
 
+	"sigs.k8s.io/scheduler-plugins/pkg/noderesourcetopology/logging"
 	"sigs.k8s.io/scheduler-plugins/pkg/noderesourcetopology/resourcerequests"
 )
 
@@ -35,19 +38,19 @@ var (
 	onlyExclusiveResources = false
 )
 
-func SetupForeignPodsDetector(schedProfileName string, podInformer k8scache.SharedInformer, cc Interface) {
+func SetupForeignPodsDetector(lh logr.Logger, schedProfileName string, podInformer k8scache.SharedInformer, cc Interface) {
 	foreignCache := func(obj interface{}) {
 		pod, ok := obj.(*corev1.Pod)
 		if !ok {
-			klog.V(3).InfoS("nrtcache: foreign: unsupported object %T", obj)
+			lh.V(3).Info("unsupported object", "kind", fmt.Sprintf("%T", obj))
 			return
 		}
 		if !IsForeignPod(pod) {
 			return
 		}
 
 		cc.NodeHasForeignPods(pod.Spec.NodeName, pod)
-		klog.V(6).InfoS("nrtcache: has foreign pods", "logID", klog.KObj(pod), "node", pod.Spec.NodeName, "podUID", pod.UID)
+		lh.V(6).Info("detected foreign pods", "logID", logging.PodLogID(pod), "podUID", pod.GetUID(), "node", pod.Spec.NodeName)
 	}
 
 	podInformer.AddEventHandler(k8scache.ResourceEventHandlerFuncs{
@@ -67,11 +70,11 @@ func TrackAllForeignPods() {
 	onlyExclusiveResources = false
 }
 
-func RegisterSchedulerProfileName(schedProfileName string) {
-	klog.InfoS("nrtcache: setting up foreign pod detection", "profile", schedProfileName)
+func RegisterSchedulerProfileName(lh logr.Logger, schedProfileName string) {
+	lh.Info("setting up detection", "profile", schedProfileName)
 	schedProfileNames.Insert(schedProfileName)
 
-	klog.V(5).InfoS("nrtcache: registered scheduler profiles", "names", schedProfileNames.List())
+	lh.V(5).Info("registered scheduler profiles", "names", schedProfileNames.List())
 }
 
 func IsForeignPod(pod *corev1.Pod) bool {

pkg/noderesourcetopology/cache/foreign_pods_test.go

@@ -22,6 +22,7 @@ import (
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/resource"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/klog/v2"
 )
 
 func TestIsForeignPod(t *testing.T) {
@@ -195,7 +196,7 @@ func TestIsForeignPod(t *testing.T) {
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			for _, profileName := range tt.profileNames {
-				RegisterSchedulerProfileName(profileName)
+				RegisterSchedulerProfileName(klog.Background(), profileName)
 			}
 			defer CleanRegisteredSchedulerProfileNames()