Skip to content

Commit

Permalink
nrt: overreserve: use pfp compute method from NRT
Browse files Browse the repository at this point in the history
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 node 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 behavior) or force one of the existing methods.

Signed-off-by: Francesco Romani <fromani@redhat.com>
  • Loading branch information
ffromani committed May 31, 2023
1 parent 30100f9 commit 97728ec
Show file tree
Hide file tree
Showing 22 changed files with 298 additions and 25 deletions.
16 changes: 16 additions & 0 deletions apis/config/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,15 @@ const (
ForeignPodsDetectOnlyExclusiveResources 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.
Expand All @@ -160,6 +169,13 @@ 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...).
// Has no effect if caching is disabled (CacheResyncPeriod is zero) or if DiscardReservedNodes
// is enabled. "Autodetect" is the default, reads hint from NRT objects. Fallback is "All".
ResyncMethod *CacheResyncMethod
}

// +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object
Expand Down
6 changes: 6 additions & 0 deletions apis/config/v1/defaults.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,8 @@ var (

defaultForeignPodsDetect = ForeignPodsDetectAll

defaultResyncMethod = CacheResyncAutodetect

// Defaults for NetworkOverhead
// DefaultWeightsName contains the default costs to be used by networkAware plugins
DefaultWeightsName = "UserDefined"
Expand Down Expand Up @@ -159,6 +161,10 @@ func SetDefaults_NodeResourceTopologyMatchArgs(obj *NodeResourceTopologyMatchArg
}
if obj.Cache.ForeignPodsDetect == nil {
obj.Cache.ForeignPodsDetect = &defaultForeignPodsDetect

}
if obj.Cache.ResyncMethod == nil {
obj.Cache.ResyncMethod = &defaultResyncMethod
}
}

Expand Down
1 change: 1 addition & 0 deletions apis/config/v1/defaults_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,7 @@ func TestSchedulingDefaults(t *testing.T) {
},
Cache: &NodeResourceTopologyCache{
ForeignPodsDetect: &defaultForeignPodsDetect,
ResyncMethod: &defaultResyncMethod,
},
},
},
Expand Down
16 changes: 16 additions & 0 deletions apis/config/v1/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,15 @@ const (
ForeignPodsDetectOnlyExclusiveResources 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.
Expand All @@ -157,6 +166,13 @@ type NodeResourceTopologyCache struct {
// Has no effect if caching is disabled (CacheResyncPeriod is zero) or if DiscardReservedNodes
// is enabled. If unspecified, default is "All". Use "None" to disable.
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...).
// Has no effect if caching is disabled (CacheResyncPeriod is zero) or if DiscardReservedNodes
// is enabled. "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
Expand Down
2 changes: 2 additions & 0 deletions apis/config/v1/zz_generated.conversion.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions apis/config/v1/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions apis/config/v1beta2/defaults.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,8 @@ var (
}

defaultForeignPodsDetect = ForeignPodsDetectAll

defaultResyncMethod = CacheResyncAutodetect
)

// SetDefaults_CoschedulingArgs sets the default parameters for Coscheduling plugin.
Expand Down Expand Up @@ -158,6 +160,9 @@ func SetDefaults_NodeResourceTopologyMatchArgs(obj *NodeResourceTopologyMatchArg
if obj.Cache.ForeignPodsDetect == nil {
obj.Cache.ForeignPodsDetect = &defaultForeignPodsDetect
}
if obj.Cache.ResyncMethod == nil {
obj.Cache.ResyncMethod = &defaultResyncMethod
}
}

// SetDefaults_PreemptionTolerationArgs reuses SetDefaults_DefaultPreemptionArgs
Expand Down
1 change: 1 addition & 0 deletions apis/config/v1beta2/defaults_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,7 @@ func TestSchedulingDefaults(t *testing.T) {
},
Cache: &NodeResourceTopologyCache{
ForeignPodsDetect: &defaultForeignPodsDetect,
ResyncMethod: &defaultResyncMethod,
},
},
},
Expand Down
16 changes: 16 additions & 0 deletions apis/config/v1beta2/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,15 @@ const (
ForeignPodsDetectOnlyExclusiveResources 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.
Expand All @@ -153,6 +162,13 @@ type NodeResourceTopologyCache struct {
// Has no effect if caching is disabled (CacheResyncPeriod is zero) or if DiscardReservedNodes
// is enabled. If unspecified, default is "All". Use "None" to disable.
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...).
// Has no effect if caching is disabled (CacheResyncPeriod is zero) or if DiscardReservedNodes
// is enabled. "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
Expand Down
2 changes: 2 additions & 0 deletions apis/config/v1beta2/zz_generated.conversion.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions apis/config/v1beta2/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 6 additions & 0 deletions apis/config/v1beta3/defaults.go
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,8 @@ var (

defaultForeignPodsDetect = ForeignPodsDetectAll

defaultResyncMethod = CacheResyncAutodetect

// Defaults for NetworkOverhead
// DefaultWeightsName contains the default costs to be used by networkAware plugins
DefaultWeightsName = "UserDefined"
Expand Down Expand Up @@ -160,6 +162,10 @@ func SetDefaults_NodeResourceTopologyMatchArgs(obj *NodeResourceTopologyMatchArg
}
if obj.Cache.ForeignPodsDetect == nil {
obj.Cache.ForeignPodsDetect = &defaultForeignPodsDetect

}
if obj.Cache.ResyncMethod == nil {
obj.Cache.ResyncMethod = &defaultResyncMethod
}
}

Expand Down
1 change: 1 addition & 0 deletions apis/config/v1beta3/defaults_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,7 @@ func TestSchedulingDefaults(t *testing.T) {
},
Cache: &NodeResourceTopologyCache{
ForeignPodsDetect: &defaultForeignPodsDetect,
ResyncMethod: &defaultResyncMethod,
},
},
},
Expand Down
16 changes: 16 additions & 0 deletions apis/config/v1beta3/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,15 @@ const (
ForeignPodsDetectOnlyExclusiveResources 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.
Expand All @@ -157,6 +166,13 @@ type NodeResourceTopologyCache struct {
// Has no effect if caching is disabled (CacheResyncPeriod is zero) or if DiscardReservedNodes
// is enabled. If unspecified, default is "All". Use "None" to disable.
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...).
// Has no effect if caching is disabled (CacheResyncPeriod is zero) or if DiscardReservedNodes
// is enabled. "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
Expand Down
2 changes: 2 additions & 0 deletions apis/config/v1beta3/zz_generated.conversion.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions apis/config/v1beta3/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions apis/config/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

26 changes: 21 additions & 5 deletions pkg/noderesourcetopology/cache/overreserve.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -47,26 +48,30 @@ 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")
}

resyncMethod := getCacheResyncMethod(cfg)

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
}
Expand Down Expand Up @@ -220,15 +225,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)
Expand Down Expand Up @@ -274,4 +279,15 @@ func logIDFromTime() string {
return fmt.Sprintf("resync%v", time.Now().UnixMilli())
}

func getCacheResyncMethod(cfg *apiconfig.NodeResourceTopologyCache) apiconfig.CacheResyncMethod {
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)
}
return resyncMethod
}

func (ov *OverReserve) PostBind(nodeName string, pod *corev1.Pod) {}
Loading

0 comments on commit 97728ec

Please sign in to comment.