Skip to content

Commit

Permalink
nrt: cache: add support for dedicated informer
Browse files Browse the repository at this point in the history
Add an option to create and use a separate informer
to get unfiltered pod listing from the apiserver.

Due to mismatched view with respect to the kubelet,
the plugin needs access to pod in terminal phase
which are not deleted to make sure the reconciliation
is done correctly.

xref: #598
xref: kubernetes/kubernetes#119423

Signed-off-by: Francesco Romani <fromani@redhat.com>
  • Loading branch information
ffromani committed Oct 10, 2023
1 parent 5eb736d commit ed541c5
Show file tree
Hide file tree
Showing 21 changed files with 162 additions and 12 deletions.
13 changes: 13 additions & 0 deletions apis/config/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,14 @@ const (
CacheResyncOnlyExclusiveResources CacheResyncMethod = "OnlyExclusiveResources"
)

// CacheInformerMode is a "string" type
type CacheInformerMode string

const (
CacheInformerShared CacheInformerMode = "Shared"
CacheInformerDedicated CacheInformerMode = "Dedicated"
)

// NodeResourceTopologyCache define configuration details for the NodeResourceTopology cache.
type NodeResourceTopologyCache struct {
// ForeignPodsDetect sets how foreign pods should be handled.
Expand All @@ -192,6 +200,11 @@ type NodeResourceTopologyCache struct {
// 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
// InformerMode controls the channel the cache uses to get updates about pods.
// "Shared" uses the default settings; "Dedicated" creates a specific subscription which is
// guaranteed to best suit the cache needs, at cost of one extra connection.
// If unspecified, default is "Shared"
InformerMode *CacheInformerMode
}

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

defaultResyncMethod = CacheResyncAutodetect

defaultInformerMode = CacheInformerShared

// Defaults for NetworkOverhead
// DefaultWeightsName contains the default costs to be used by networkAware plugins
DefaultWeightsName = "UserDefined"
Expand Down Expand Up @@ -200,6 +202,9 @@ func SetDefaults_NodeResourceTopologyMatchArgs(obj *NodeResourceTopologyMatchArg
if obj.Cache.ResyncMethod == nil {
obj.Cache.ResyncMethod = &defaultResyncMethod
}
if obj.Cache.InformerMode == nil {
obj.Cache.InformerMode = &defaultInformerMode
}
}

// SetDefaults_PreemptionTolerationArgs reuses SetDefaults_DefaultPreemptionArgs
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 @@ -205,6 +205,7 @@ func TestSchedulingDefaults(t *testing.T) {
Cache: &NodeResourceTopologyCache{
ForeignPodsDetect: &defaultForeignPodsDetect,
ResyncMethod: &defaultResyncMethod,
InformerMode: &defaultInformerMode,
},
},
},
Expand Down
13 changes: 13 additions & 0 deletions apis/config/v1/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,14 @@ const (
CacheResyncOnlyExclusiveResources CacheResyncMethod = "OnlyExclusiveResources"
)

// CacheInformerMode is a "string" type
type CacheInformerMode string

const (
CacheInformerShared CacheInformerMode = "Shared"
CacheInformerDedicated CacheInformerMode = "Dedicated"
)

// NodeResourceTopologyCache define configuration details for the NodeResourceTopology cache.
type NodeResourceTopologyCache struct {
// ForeignPodsDetect sets how foreign pods should be handled.
Expand All @@ -190,6 +198,11 @@ type NodeResourceTopologyCache struct {
// 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"`
// InformerMode controls the channel the cache uses to get updates about pods.
// "Shared" uses the default settings; "Dedicated" creates a specific subscription which is
// guaranteed to best suit the cache needs, at cost of one extra connection.
// If unspecified, default is "Shared"
InformerMode *CacheInformerMode `json:"informerMode,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.

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

defaultResyncMethod = CacheResyncAutodetect

defaultInformerMode = CacheInformerShared
)

// SetDefaults_CoschedulingArgs sets the default parameters for Coscheduling plugin.
Expand Down Expand Up @@ -163,10 +165,14 @@ func SetDefaults_NodeResourceTopologyMatchArgs(obj *NodeResourceTopologyMatchArg
}
if obj.Cache.ForeignPodsDetect == nil {
obj.Cache.ForeignPodsDetect = &defaultForeignPodsDetect

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

// 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 @@ -150,6 +150,7 @@ func TestSchedulingDefaults(t *testing.T) {
Cache: &NodeResourceTopologyCache{
ForeignPodsDetect: &defaultForeignPodsDetect,
ResyncMethod: &defaultResyncMethod,
InformerMode: &defaultInformerMode,
},
},
},
Expand Down
13 changes: 13 additions & 0 deletions apis/config/v1beta2/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,14 @@ const (
CacheResyncOnlyExclusiveResources CacheResyncMethod = "OnlyExclusiveResources"
)

// CacheInformerMode is a "string" type
type CacheInformerMode string

const (
CacheInformerShared CacheInformerMode = "Shared"
CacheInformerDedicated CacheInformerMode = "Dedicated"
)

// NodeResourceTopologyCache define configuration details for the NodeResourceTopology cache.
type NodeResourceTopologyCache struct {
// ForeignPodsDetect sets how foreign pods should be handled.
Expand All @@ -171,6 +179,11 @@ type NodeResourceTopologyCache struct {
// 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"`
// InformerMode controls the channel the cache uses to get updates about pods.
// "Shared" uses the default settings; "Dedicated" creates a specific subscription which is
// guaranteed to best suit the cache needs, at cost of one extra connection.
// If unspecified, default is "Shared"
InformerMode *CacheInformerMode `json:"informerMode,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.

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

defaultResyncMethod = CacheResyncAutodetect

defaultInformerMode = CacheInformerShared

// Defaults for NetworkOverhead
// DefaultWeightsName contains the default costs to be used by networkAware plugins
DefaultWeightsName = "UserDefined"
Expand Down Expand Up @@ -200,6 +202,9 @@ func SetDefaults_NodeResourceTopologyMatchArgs(obj *NodeResourceTopologyMatchArg
if obj.Cache.ResyncMethod == nil {
obj.Cache.ResyncMethod = &defaultResyncMethod
}
if obj.Cache.InformerMode == nil {
obj.Cache.InformerMode = &defaultInformerMode
}
}

// SetDefaults_PreemptionTolerationArgs reuses SetDefaults_DefaultPreemptionArgs
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 @@ -205,6 +205,7 @@ func TestSchedulingDefaults(t *testing.T) {
Cache: &NodeResourceTopologyCache{
ForeignPodsDetect: &defaultForeignPodsDetect,
ResyncMethod: &defaultResyncMethod,
InformerMode: &defaultInformerMode,
},
},
},
Expand Down
13 changes: 13 additions & 0 deletions apis/config/v1beta3/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,14 @@ const (
CacheResyncOnlyExclusiveResources CacheResyncMethod = "OnlyExclusiveResources"
)

// CacheInformerMode is a "string" type
type CacheInformerMode string

const (
CacheInformerShared CacheInformerMode = "Shared"
CacheInformerDedicated CacheInformerMode = "Dedicated"
)

// NodeResourceTopologyCache define configuration details for the NodeResourceTopology cache.
type NodeResourceTopologyCache struct {
// ForeignPodsDetect sets how foreign pods should be handled.
Expand All @@ -190,6 +198,11 @@ type NodeResourceTopologyCache struct {
// 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"`
// InformerMode controls the channel the cache uses to get updates about pods.
// "Shared" uses the default settings; "Dedicated" creates a specific subscription which is
// guaranteed to best suit the cache needs, at cost of one extra connection.
// If unspecified, default is "Shared"
InformerMode *CacheInformerMode `json:"informerMode,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.

7 changes: 0 additions & 7 deletions pkg/noderesourcetopology/cache/overreserve.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,7 @@ import (
corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/labels"
podlisterv1 "k8s.io/client-go/listers/core/v1"
k8scache "k8s.io/client-go/tools/cache"
"k8s.io/klog/v2"
"k8s.io/kubernetes/pkg/scheduler/framework"

topologyv1alpha2 "github.com/k8stopologyawareschedwg/noderesourcetopology-api/pkg/apis/topology/v1alpha2"
listerv1alpha2 "github.com/k8stopologyawareschedwg/noderesourcetopology-api/pkg/generated/listers/topology/v1alpha2"
Expand Down Expand Up @@ -265,11 +263,6 @@ func (ov *OverReserve) FlushNodes(logID string, nrts ...*topologyv1alpha2.NodeRe
}
}

func InformerFromHandle(handle framework.Handle) (k8scache.SharedIndexInformer, podlisterv1.PodLister) {
podHandle := handle.SharedInformerFactory().Core().V1().Pods() // shortcut
return podHandle.Informer(), podHandle.Lister()
}

// to be used only in tests
func (ov *OverReserve) Store() *nrtStore {
return ov.nrts
Expand Down
4 changes: 0 additions & 4 deletions pkg/noderesourcetopology/cache/store.go
Original file line number Diff line number Diff line change
Expand Up @@ -254,10 +254,6 @@ func makeNodeToPodDataMap(podLister podlisterv1.PodLister, logID string) (map[st
return nodeToObjsMap, err
}
for _, pod := range pods {
if pod.Status.Phase != corev1.PodRunning {
// we are interested only about nodes which consume resources
continue
}
nodeObjs := nodeToObjsMap[pod.Spec.NodeName]
nodeObjs = append(nodeObjs, podData{
Namespace: pod.Namespace,
Expand Down
3 changes: 2 additions & 1 deletion pkg/noderesourcetopology/pluginhelpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ import (

apiconfig "sigs.k8s.io/scheduler-plugins/apis/config"
nrtcache "sigs.k8s.io/scheduler-plugins/pkg/noderesourcetopology/cache"
"sigs.k8s.io/scheduler-plugins/pkg/noderesourcetopology/podprovider"
"sigs.k8s.io/scheduler-plugins/pkg/noderesourcetopology/stringify"
)

Expand Down Expand Up @@ -66,7 +67,7 @@ func initNodeTopologyInformer(tcfg *apiconfig.NodeResourceTopologyMatchArgs, han
return nrtcache.NewPassthrough(nodeTopologyLister), nil
}

podSharedInformer, podLister := nrtcache.InformerFromHandle(handle)
podSharedInformer, podLister := podprovider.NewFromHandle(handle, tcfg.Cache)

nrtCache, err := nrtcache.NewOverReserve(tcfg.Cache, nodeTopologyLister, podLister)
if err != nil {
Expand Down
Loading

0 comments on commit ed541c5

Please sign in to comment.