nrt: overreserve: use pfp compute method from NRT

When computing the expected node pod fingerprint (PFP), we
now look for the attribute describing the computation method
provided by the topology-updater agent. If not provided, we
fall back to the previous backward-compatible method, which
is consider all pods.

If the method is supplied, the scheduler plugin now computes
the PFP in the same way advertised as the agent.

Finally, in order to keep the user fully in control, add
config tunables to force the global behavior of the resync
loop. The cluster admin can let the plugin autodetect (default)
or force one of the existing methods.

Signed-off-by: Francesco Romani <fromani@redhat.com>

apis/config/types.go

@@ -151,6 +151,15 @@ const (
 	DetectOnlyExclusiveResources ForeignPodsDetectMode = "OnlyExclusiveResources"
 )
 
+// CacheResyncMethod is a "string" type.
+type CacheResyncMethod string
+
+const (
+	CacheResyncAutodetect             CacheResyncMethod = "Autodetect"
+	CacheResyncAll                    CacheResyncMethod = "All"
+	CacheResyncOnlyExclusiveResources CacheResyncMethod = "OnlyExclusiveResources"
+)
+
 // NodeResourceTopologyCache define configuration details for the NodeResourceTopology cache.
 type NodeResourceTopologyCache struct {
 	// ForeignPodsDetect sets how foreign pods should be handled.
@@ -160,6 +169,12 @@ type NodeResourceTopologyCache struct {
 	// Has no effect if caching is disabled (CacheResyncPeriod is zero) or
 	// if DiscardReservedNodes is enabled. If unspecified, default is "All".
 	ForeignPodsDetect *ForeignPodsDetectMode
+	// ResyncMethod sets how the resync behaves to compute the expected node state.
+	// "All" consider all pods to compute the node state. "OnlyExclusiveResources" consider
+	// only pods regardless of their QoS which have exclusive resources assigned to their
+	// containers (CPUs, devices...).
+	// "Autodetect" is the default, reads hint from NRT objects. Fallback is "All".
+	ResyncMethod *CacheResyncMethod
 }
 
 // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object

apis/config/v1/defaults.go

@@ -73,6 +73,8 @@ var (
 
 	defaultForeignPodsDetect = DetectAll
 
+	defaultResyncMethod = CacheResyncAutodetect
+
 	// Defaults for NetworkOverhead
 	// DefaultWeightsName contains the default costs to be used by networkAware plugins
 	DefaultWeightsName = "UserDefined"
@@ -157,6 +159,7 @@ func SetDefaults_NodeResourceTopologyMatchArgs(obj *NodeResourceTopologyMatchArg
 	if obj.Cache == nil {
 		obj.Cache = &NodeResourceTopologyCache{
 			ForeignPodsDetect: &defaultForeignPodsDetect,
+			ResyncMethod:      &defaultResyncMethod,
 		}
 	}
 }

apis/config/v1/defaults_test.go

@@ -144,6 +144,7 @@ func TestSchedulingDefaults(t *testing.T) {
 				},
 				Cache: &NodeResourceTopologyCache{
 					ForeignPodsDetect: &defaultForeignPodsDetect,
+					ResyncMethod:      &defaultResyncMethod,
 				},
 			},
 		},

apis/config/v1/types.go

@@ -148,6 +148,15 @@ const (
 	DetectOnlyExclusiveResources ForeignPodsDetectMode = "OnlyExclusiveResources"
 )
 
+// CacheResyncMethod is a "string" type.
+type CacheResyncMethod string
+
+const (
+	CacheResyncAutodetect             CacheResyncMethod = "Autodetect"
+	CacheResyncAll                    CacheResyncMethod = "All"
+	CacheResyncOnlyExclusiveResources CacheResyncMethod = "OnlyExclusiveResources"
+)
+
 // NodeResourceTopologyCache define configuration details for the NodeResourceTopology cache.
 type NodeResourceTopologyCache struct {
 	// ForeignPodsDetect sets how foreign pods should be handled.
@@ -157,6 +166,12 @@ type NodeResourceTopologyCache struct {
 	// Has no effect if caching is disabled (CacheResyncPeriod is zero) or
 	// if DiscardReservedNodes is enabled. If unspecified, default is "All".
 	ForeignPodsDetect *ForeignPodsDetectMode `json:"foreignPodsDetect,omitempty"`
+	// ResyncMethod sets how the resync behaves to compute the expected node state.
+	// "All" consider all pods to compute the node state. "OnlyExclusiveResources" consider
+	// only pods regardless of their QoS which have exclusive resources assigned to their
+	// containers (CPUs, devices...).
+	// "Autodetect" is the default, reads hint from NRT objects. Fallback is "All".
+	ResyncMethod *CacheResyncMethod `json:"resyncMethod,omitempty"`
 }
 
 // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object

apis/config/v1beta2/defaults.go

@@ -73,6 +73,8 @@ var (
 	}
 
 	defaultForeignPodsDetect = DetectAll
+
+	defaultResyncMethod = CacheResyncAutodetect
 )
 
 // SetDefaults_CoschedulingArgs sets the default parameters for Coscheduling plugin.
@@ -155,6 +157,7 @@ func SetDefaults_NodeResourceTopologyMatchArgs(obj *NodeResourceTopologyMatchArg
 	if obj.Cache == nil {
 		obj.Cache = &NodeResourceTopologyCache{
 			ForeignPodsDetect: &defaultForeignPodsDetect,
+			ResyncMethod:      &defaultResyncMethod,
 		}
 	}
 }

apis/config/v1beta2/defaults_test.go

@@ -146,6 +146,7 @@ func TestSchedulingDefaults(t *testing.T) {
 				},
 				Cache: &NodeResourceTopologyCache{
 					ForeignPodsDetect: &defaultForeignPodsDetect,
+					ResyncMethod:      &defaultResyncMethod,
 				},
 			},
 		},

apis/config/v1beta2/types.go

@@ -144,6 +144,15 @@ const (
 	DetectOnlyExclusiveResources ForeignPodsDetectMode = "OnlyExclusiveResources"
 )
 
+// CacheResyncMethod is a "string" type.
+type CacheResyncMethod string
+
+const (
+	CacheResyncAutodetect             CacheResyncMethod = "Autodetect"
+	CacheResyncAll                    CacheResyncMethod = "All"
+	CacheResyncOnlyExclusiveResources CacheResyncMethod = "OnlyExclusiveResources"
+)
+
 // NodeResourceTopologyCache define configuration details for the NodeResourceTopology cache.
 type NodeResourceTopologyCache struct {
 	// ForeignPodsDetect sets how foreign pods should be handled.
@@ -153,6 +162,12 @@ type NodeResourceTopologyCache struct {
 	// Has no effect if caching is disabled (CacheResyncPeriod is zero) or
 	// if DiscardReservedNodes is enabled. If unspecified, default is "All".
 	ForeignPodsDetect *ForeignPodsDetectMode `json:"foreignPodsDetect,omitempty"`
+	// ResyncMethod sets how the resync behaves to compute the expected node state.
+	// "All" consider all pods to compute the node state. "OnlyExclusiveResources" consider
+	// only pods regardless of their QoS which have exclusive resources assigned to their
+	// containers (CPUs, devices...).
+	// "Autodetect" is the default, reads hint from NRT objects. Fallback is "All".
+	ResyncMethod *CacheResyncMethod `json:"resyncMethod,omitempty"`
 }
 
 // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object

apis/config/v1beta3/defaults.go

@@ -74,6 +74,8 @@ var (
 
 	defaultForeignPodsDetect = DetectAll
 
+	defaultResyncMethod = CacheResyncAutodetect
+
 	// Defaults for NetworkOverhead
 	// DefaultWeightsName contains the default costs to be used by networkAware plugins
 	DefaultWeightsName = "UserDefined"
@@ -158,6 +160,7 @@ func SetDefaults_NodeResourceTopologyMatchArgs(obj *NodeResourceTopologyMatchArg
 	if obj.Cache == nil {
 		obj.Cache = &NodeResourceTopologyCache{
 			ForeignPodsDetect: &defaultForeignPodsDetect,
+			ResyncMethod:      &defaultResyncMethod,
 		}
 	}
 }

apis/config/v1beta3/defaults_test.go

@@ -144,6 +144,7 @@ func TestSchedulingDefaults(t *testing.T) {
 				},
 				Cache: &NodeResourceTopologyCache{
 					ForeignPodsDetect: &defaultForeignPodsDetect,
+					ResyncMethod:      &defaultResyncMethod,
 				},
 			},
 		},

apis/config/v1beta3/types.go

@@ -148,6 +148,15 @@ const (
 	DetectOnlyExclusiveResources ForeignPodsDetectMode = "OnlyExclusiveResources"
 )
 
+// CacheResyncMethod is a "string" type.
+type CacheResyncMethod string
+
+const (
+	CacheResyncAutodetect             CacheResyncMethod = "Autodetect"
+	CacheResyncAll                    CacheResyncMethod = "All"
+	CacheResyncOnlyExclusiveResources CacheResyncMethod = "OnlyExclusiveResources"
+)
+
 // NodeResourceTopologyCache define configuration details for the NodeResourceTopology cache.
 type NodeResourceTopologyCache struct {
 	// ForeignPodsDetect sets how foreign pods should be handled.
@@ -157,6 +166,12 @@ type NodeResourceTopologyCache struct {
 	// Has no effect if caching is disabled (CacheResyncPeriod is zero) or
 	// if DiscardReservedNodes is enabled. If unspecified, default is "All".
 	ForeignPodsDetect *ForeignPodsDetectMode `json:"foreignPodsDetect,omitempty"`
+	// ResyncMethod sets how the resync behaves to compute the expected node state.
+	// "All" consider all pods to compute the node state. "OnlyExclusiveResources" consider
+	// only pods regardless of their QoS which have exclusive resources assigned to their
+	// containers (CPUs, devices...).
+	// "Autodetect" is the default, reads hint from NRT objects. Fallback is "All".
+	ResyncMethod *CacheResyncMethod `json:"resyncMethod,omitempty"`
 }
 
 // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object

pkg/noderesourcetopology/cache/overreserve.go

@@ -32,6 +32,7 @@ import (
 	topologyv1alpha2 "github.com/k8stopologyawareschedwg/noderesourcetopology-api/pkg/apis/topology/v1alpha2"
 	listerv1alpha2 "github.com/k8stopologyawareschedwg/noderesourcetopology-api/pkg/generated/listers/topology/v1alpha2"
 
+	apiconfig "sigs.k8s.io/scheduler-plugins/apis/config"
 	"sigs.k8s.io/scheduler-plugins/pkg/noderesourcetopology/stringify"
 
 	"github.com/k8stopologyawareschedwg/podfingerprint"
@@ -47,26 +48,36 @@ type OverReserve struct {
 	nodesWithForeignPods   counter
 	nrtLister              listerv1alpha2.NodeResourceTopologyLister
 	podLister              podlisterv1.PodLister
+	resyncMethod           apiconfig.CacheResyncMethod
 }
 
-func NewOverReserve(nrtLister listerv1alpha2.NodeResourceTopologyLister, podLister podlisterv1.PodLister) (*OverReserve, error) {
+func NewOverReserve(cfg *apiconfig.NodeResourceTopologyCache, nrtLister listerv1alpha2.NodeResourceTopologyLister, podLister podlisterv1.PodLister) (*OverReserve, error) {
 	if nrtLister == nil || podLister == nil {
 		return nil, fmt.Errorf("nrtcache: received nil references")
 	}
 
+	var resyncMethod apiconfig.CacheResyncMethod
+	if cfg != nil && cfg.ResyncMethod != nil {
+		resyncMethod = *cfg.ResyncMethod
+	} else { // explicitly set to nil?
+		resyncMethod = apiconfig.CacheResyncAutodetect
+		klog.InfoS("cache resync method missing", "fallback", resyncMethod)
+	}
+
 	nrtObjs, err := nrtLister.List(labels.Everything())
 	if err != nil {
 		return nil, err
 	}
 
-	klog.V(3).InfoS("nrtcache: initializing", "objects", len(nrtObjs))
+	klog.V(3).InfoS("nrtcache: initializing", "objects", len(nrtObjs), "method", resyncMethod)
 	obj := &OverReserve{
 		nrts:                   newNrtStore(nrtObjs),
 		assumedResources:       make(map[string]*resourceStore),
 		nodesMaybeOverreserved: newCounter(),
 		nodesWithForeignPods:   newCounter(),
 		nrtLister:              nrtLister,
 		podLister:              podLister,
+		resyncMethod:           resyncMethod,
 	}
 	return obj, nil
 }
@@ -220,15 +231,15 @@ func (ov *OverReserve) Resync() {
 			continue
 		}
 
-		pfpExpected := podFingerprintForNodeTopology(nrtCandidate)
+		pfpExpected, pfpMethod := podFingerprintForNodeTopology(nrtCandidate, ov.resyncMethod)
 		if pfpExpected == "" {
 			klog.V(3).InfoS("nrtcache: missing NodeTopology podset fingerprint data", "logID", logID, "node", nodeName)
 			continue
 		}
 
-		klog.V(6).InfoS("nrtcache: trying to resync NodeTopology", "logID", logID, "node", nodeName, "fingerprint", pfpExpected)
+		klog.V(6).InfoS("nrtcache: trying to resync NodeTopology", "logID", logID, "node", nodeName, "fingerprint", pfpExpected, "method", pfpMethod)
 
-		err = checkPodFingerprintForNode(logID, objs, nodeName, pfpExpected)
+		err = checkPodFingerprintForNode(logID, objs, nodeName, pfpExpected, pfpMethod)
 		if errors.Is(err, podfingerprint.ErrSignatureMismatch) {
 			// can happen, not critical
 			klog.V(5).InfoS("nrtcache: NodeTopology podset fingerprint mismatch", "logID", logID, "node", nodeName)

pkg/noderesourcetopology/cache/overreserve_test.go

@@ -45,12 +45,12 @@ func TestInitEmptyLister(t *testing.T) {
 	fakePodLister := &fakePodLister{}
 
 	var err error
-	_, err = NewOverReserve(nil, fakePodLister)
+	_, err = NewOverReserve(nil, nil, fakePodLister)
 	if err == nil {
 		t.Fatalf("accepted nil lister")
 	}
 
-	_, err = NewOverReserve(fakeInformer.Lister(), nil)
+	_, err = NewOverReserve(nil, fakeInformer.Lister(), nil)
 	if err == nil {
 		t.Fatalf("accepted nil indexer")
 	}
@@ -693,7 +693,7 @@ func makeDefaultTestTopology() []*topologyv1alpha2.NodeResourceTopology {
 }
 
 func mustOverReserve(t *testing.T, nrtLister listerv1alpha2.NodeResourceTopologyLister, podLister podlisterv1.PodLister) *OverReserve {
-	obj, err := NewOverReserve(nrtLister, podLister)
+	obj, err := NewOverReserve(nil, nrtLister, podLister)
 	if err != nil {
 		t.Fatalf("unexpected error creating cache: %v", err)
 	}