Merge pull request #655 from zwpaper/refactor-ctrl-runtime-capacity

migrate nrt to ctrl runtime client

go.mod

@@ -13,7 +13,7 @@ require (
 	github.com/patrickmn/go-cache v2.1.0+incompatible
 	github.com/paypal/load-watcher v0.2.3
 	github.com/spf13/pflag v1.0.5
-	github.com/stretchr/testify v1.8.1
+	github.com/stretchr/testify v1.8.4
 	gonum.org/v1/gonum v0.12.0
 	k8s.io/api v0.27.7
 	k8s.io/apimachinery v0.27.7

go.sum

@@ -472,8 +472,9 @@ github.com/stretchr/testify v1.5.1/go.mod h1:5W2xD1RspED5o8YsWQXVCued0rvSQ+mT+I5
 github.com/stretchr/testify v1.7.0/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg=
 github.com/stretchr/testify v1.7.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg=
 github.com/stretchr/testify v1.8.0/go.mod h1:yNjHg4UonilssWZ8iaSj1OCr/vHnekPRkoO+kdMU+MU=
-github.com/stretchr/testify v1.8.1 h1:w7B6lhMri9wdJUVmEZPGGhZzrYTPvgJArz7wNPgYKsk=
 github.com/stretchr/testify v1.8.1/go.mod h1:w2LPCIKwWwSfY2zedu0+kehJoqGctiVI29o6fzry7u4=
+github.com/stretchr/testify v1.8.4 h1:CcVxjf3Q8PM0mHUKJCdn+eZZtm5yQwehR5yeSVQQcUk=
+github.com/stretchr/testify v1.8.4/go.mod h1:sz/lmYIOXD/1dqDmKjjqLyZ2RngseejIcXlSw2iwfAo=
 github.com/tarm/serial v0.0.0-20180830185346-98f6abe2eb07/go.mod h1:kDXzergiv9cbyO7IOYJZWg1U88JhDg3PB6klq9Hg2pA=
 github.com/tmc/grpc-websocket-proxy v0.0.0-20220101234140-673ab2c3ae75 h1:6fotK7otjonDflCTK0BCfls4SPy3NcCVb5dqqmbRknE=
 github.com/viant/assertly v0.4.8/go.mod h1:aGifi++jvCrUaklKEKT0BU95igDNaqkvz+49uaYMPRU=

manifests/noderesourcetopology/crd.yaml

@@ -18,124 +18,6 @@ spec:
     singular: noderesourcetopology
   scope: Cluster
   versions:
-  - name: v1alpha1
-    schema:
-      openAPIV3Schema:
-        description: NodeResourceTopology describes node resources and their topology.
-        properties:
-          apiVersion:
-            description: 'APIVersion defines the versioned schema of this representation
-              of an object. Servers should convert recognized schemas to the latest
-              internal value, and may reject unrecognized values. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources'
-            type: string
-          kind:
-            description: 'Kind is a string value representing the REST resource this
-              object represents. Servers may infer this from the endpoint the client
-              submits requests to. Cannot be updated. In CamelCase. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds'
-            type: string
-          metadata:
-            type: object
-          topologyPolicies:
-            items:
-              type: string
-            type: array
-          zones:
-            description: ZoneList contains an array of Zone objects.
-            items:
-              description: Zone represents a resource topology zone, e.g. socket,
-                node, die or core.
-              properties:
-                attributes:
-                  description: AttributeList contains an array of AttributeInfo objects.
-                  items:
-                    description: AttributeInfo contains one attribute of a Zone.
-                    properties:
-                      name:
-                        type: string
-                      value:
-                        type: string
-                    required:
-                    - name
-                    - value
-                    type: object
-                  type: array
-                costs:
-                  description: CostList contains an array of CostInfo objects.
-                  items:
-                    description: CostInfo describes the cost (or distance) between
-                      two Zones.
-                    properties:
-                      name:
-                        type: string
-                      value:
-                        format: int64
-                        type: integer
-                    required:
-                    - name
-                    - value
-                    type: object
-                  type: array
-                name:
-                  type: string
-                parent:
-                  type: string
-                resources:
-                  description: ResourceInfoList contains an array of ResourceInfo
-                    objects.
-                  items:
-                    description: ResourceInfo contains information about one resource
-                      type.
-                    properties:
-                      allocatable:
-                        anyOf:
-                        - type: integer
-                        - type: string
-                        description: Allocatable quantity of the resource, corresponding
-                          to allocatable in node status, i.e. total amount of this
-                          resource available to be used by pods.
-                        pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$
-                        x-kubernetes-int-or-string: true
-                      available:
-                        anyOf:
-                        - type: integer
-                        - type: string
-                        description: Available is the amount of this resource currently
-                          available for new (to be scheduled) pods, i.e. Allocatable
-                          minus the resources reserved by currently running pods.
-                        pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$
-                        x-kubernetes-int-or-string: true
-                      capacity:
-                        anyOf:
-                        - type: integer
-                        - type: string
-                        description: Capacity of the resource, corresponding to capacity
-                          in node status, i.e. total amount of this resource that
-                          the node has.
-                        pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$
-                        x-kubernetes-int-or-string: true
-                      name:
-                        description: Name of the resource.
-                        type: string
-                    required:
-                    - allocatable
-                    - available
-                    - capacity
-                    - name
-                    type: object
-                  type: array
-                type:
-                  type: string
-              required:
-              - name
-              - type
-              type: object
-            type: array
-        required:
-        - topologyPolicies
-        - zones
-        type: object
-    served: true
-    storage: false
   - name: v1alpha2
     schema:
       openAPIV3Schema:

pkg/networkaware/networkoverhead/networkoverhead.go

@@ -61,6 +61,15 @@ const (
 	preFilterStateKey = "PreFilter" + Name
 )
 
+var scheme = runtime.NewScheme()
+
+func init() {
+	utilruntime.Must(clientgoscheme.AddToScheme(scheme))
+
+	utilruntime.Must(agv1alpha1.AddToScheme(scheme))
+	utilruntime.Must(ntv1alpha1.AddToScheme(scheme))
+}
+
 // NetworkOverhead : Filter and Score nodes based on Pod's AppGroup requirements: MaxNetworkCosts requirements among Pods with dependencies
 type NetworkOverhead struct {
 	client.Client
@@ -137,14 +146,6 @@ func New(obj runtime.Object, handle framework.Handle) (framework.Plugin, error)
 	if err != nil {
 		return nil, err
 	}
-
-	scheme := runtime.NewScheme()
-
-	utilruntime.Must(clientgoscheme.AddToScheme(scheme))
-
-	utilruntime.Must(agv1alpha1.AddToScheme(scheme))
-	utilruntime.Must(ntv1alpha1.AddToScheme(scheme))
-
 	client, err := client.New(handle.KubeConfig(), client.Options{
 		Scheme: scheme,
 	})

pkg/networkaware/networkoverhead/networkoverhead_test.go

@@ -32,7 +32,7 @@ import (
 	"k8s.io/apimachinery/pkg/util/wait"
 	"k8s.io/client-go/informers"
 	testClientSet "k8s.io/client-go/kubernetes/fake"
-	"k8s.io/client-go/kubernetes/scheme"
+	clientgoscheme "k8s.io/client-go/kubernetes/scheme"
 	"k8s.io/client-go/util/workqueue"
 	"k8s.io/kubernetes/pkg/scheduler/framework"
 	"k8s.io/kubernetes/pkg/scheduler/framework/plugins/defaultbinder"
@@ -488,7 +488,7 @@ func BenchmarkNetworkOverheadPreFilter(b *testing.B) {
 
 	for _, tt := range tests {
 		b.Run(tt.name, func(b *testing.B) {
-			s := scheme.Scheme
+			s := clientgoscheme.Scheme
 			utilruntime.Must(agv1alpha1.AddToScheme(s))
 			utilruntime.Must(ntv1alpha1.AddToScheme(s))
 
@@ -716,7 +716,7 @@ func TestNetworkOverheadScore(t *testing.T) {
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			s := scheme.Scheme
+			s := clientgoscheme.Scheme
 			utilruntime.Must(agv1alpha1.AddToScheme(s))
 			utilruntime.Must(ntv1alpha1.AddToScheme(s))
 
@@ -956,7 +956,7 @@ func BenchmarkNetworkOverheadScore(b *testing.B) {
 
 	for _, tt := range tests {
 		b.Run(tt.name, func(b *testing.B) {
-			s := scheme.Scheme
+			s := clientgoscheme.Scheme
 			utilruntime.Must(agv1alpha1.AddToScheme(s))
 			utilruntime.Must(ntv1alpha1.AddToScheme(s))
 
@@ -1189,7 +1189,7 @@ func TestNetworkOverheadFilter(t *testing.T) {
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			s := scheme.Scheme
+			s := clientgoscheme.Scheme
 			utilruntime.Must(agv1alpha1.AddToScheme(s))
 			utilruntime.Must(ntv1alpha1.AddToScheme(s))
 
@@ -1413,7 +1413,7 @@ func BenchmarkNetworkOverheadFilter(b *testing.B) {
 
 	for _, tt := range tests {
 		b.Run(tt.name, func(b *testing.B) {
-			s := scheme.Scheme
+			s := clientgoscheme.Scheme
 			utilruntime.Must(agv1alpha1.AddToScheme(s))
 			utilruntime.Must(ntv1alpha1.AddToScheme(s))
 

pkg/networkaware/topologicalsort/topologicalsort.go

@@ -40,6 +40,13 @@ const (
 	Name = "TopologicalSort"
 )
 
+var scheme = runtime.NewScheme()
+
+func init() {
+	utilruntime.Must(clientgoscheme.AddToScheme(scheme))
+	utilruntime.Must(agv1alpha.AddToScheme(scheme))
+}
+
 // TopologicalSort : Sort pods based on their AppGroup and corresponding microservice dependencies
 type TopologicalSort struct {
 	client.Client
@@ -73,11 +80,6 @@ func New(obj runtime.Object, handle framework.Handle) (framework.Plugin, error)
 		return nil, err
 	}
 
-	scheme := runtime.NewScheme()
-
-	utilruntime.Must(clientgoscheme.AddToScheme(scheme))
-	utilruntime.Must(agv1alpha.AddToScheme(scheme))
-
 	client, err := client.New(handle.KubeConfig(), client.Options{
 		Scheme: scheme,
 	})

pkg/networkaware/topologicalsort/topologicalsort_test.go

@@ -28,7 +28,7 @@ import (
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/util/rand"
 	utilruntime "k8s.io/apimachinery/pkg/util/runtime"
-	"k8s.io/client-go/kubernetes/scheme"
+	clientgoscheme "k8s.io/client-go/kubernetes/scheme"
 	"k8s.io/client-go/util/workqueue"
 	"k8s.io/kubernetes/pkg/scheduler/framework"
 	st "k8s.io/kubernetes/pkg/scheduler/testing"
@@ -249,7 +249,7 @@ func TestTopologicalSortLess(t *testing.T) {
 		t.Run(tt.name, func(t *testing.T) {
 			pods := makePodsAppGroup(tt.deploymentNames, tt.agName, tt.podPhase)
 
-			s := scheme.Scheme
+			s := clientgoscheme.Scheme
 			utilruntime.Must(agv1alpha1.AddToScheme(s))
 			client := fake.NewClientBuilder().
 				WithScheme(s).
@@ -429,7 +429,7 @@ func BenchmarkTopologicalSortPlugin(b *testing.B) {
 
 			pods := makePodsAppGroup(tt.deploymentNames, tt.agName, tt.podPhase)
 
-			s := scheme.Scheme
+			s := clientgoscheme.Scheme
 			utilruntime.Must(agv1alpha1.AddToScheme(s))
 			client := fake.NewClientBuilder().
 				WithScheme(s).

pkg/noderesourcetopology/cache/cache.go

@@ -17,6 +17,8 @@ limitations under the License.
 package cache
 
 import (
+	"context"
+
 	corev1 "k8s.io/api/core/v1"
 
 	topologyv1alpha2 "github.com/k8stopologyawareschedwg/noderesourcetopology-api/pkg/apis/topology/v1alpha2"
@@ -30,7 +32,7 @@ type Interface interface {
 	// The pod argument is used only for logging purposes.
 	// Returns a boolean to signal the caller if the NRT data is clean. If false, then the node has foreign
 	// Pods detected - so it should be ignored or handled differently by the caller.
-	GetCachedNRTCopy(nodeName string, pod *corev1.Pod) (*topologyv1alpha2.NodeResourceTopology, bool)
+	GetCachedNRTCopy(ctx context.Context, nodeName string, pod *corev1.Pod) (*topologyv1alpha2.NodeResourceTopology, bool)
 
 	// NodeMaybeOverReserved declares a node was filtered out for not enough resources available.
 	// This means this node is eligible for a resync. When a node is marked discarded (dirty), it matters not

pkg/noderesourcetopology/cache/discardreserved.go

@@ -17,14 +17,16 @@ limitations under the License.
 package cache
 
 import (
+	"context"
 	"sync"
 
+	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"
 
-	topologyv1alpha2 "github.com/k8stopologyawareschedwg/noderesourcetopology-api/pkg/apis/topology/v1alpha2"
-	listerv1alpha2 "github.com/k8stopologyawareschedwg/noderesourcetopology-api/pkg/generated/listers/topology/v1alpha2"
+	ctrlclient "sigs.k8s.io/controller-runtime/pkg/client"
 )
 
 // DiscardReserved is intended to solve similiar problem as Overreserve Cache,
@@ -42,17 +44,17 @@ import (
 type DiscardReserved struct {
 	rMutex         sync.RWMutex
 	reservationMap map[string]map[types.UID]bool // Key is NodeName, value is Pod UID : reserved status
-	lister         listerv1alpha2.NodeResourceTopologyLister
+	client         ctrlclient.Client
 }
 
-func NewDiscardReserved(lister listerv1alpha2.NodeResourceTopologyLister) Interface {
+func NewDiscardReserved(client ctrlclient.Client) Interface {
 	return &DiscardReserved{
-		lister:         lister,
+		client:         client,
 		reservationMap: make(map[string]map[types.UID]bool),
 	}
 }
 
-func (pt *DiscardReserved) GetCachedNRTCopy(nodeName string, _ *corev1.Pod) (*topologyv1alpha2.NodeResourceTopology, bool) {
+func (pt *DiscardReserved) GetCachedNRTCopy(ctx context.Context, nodeName string, _ *corev1.Pod) (*topologyv1alpha2.NodeResourceTopology, bool) {
 	pt.rMutex.RLock()
 	defer pt.rMutex.RUnlock()
 	if t, ok := pt.reservationMap[nodeName]; ok {
@@ -61,8 +63,8 @@ func (pt *DiscardReserved) GetCachedNRTCopy(nodeName string, _ *corev1.Pod) (*to
 		}
 	}
 
-	nrt, err := pt.lister.Get(nodeName)
-	if err != nil {
+	nrt := &topologyv1alpha2.NodeResourceTopology{}
+	if err := pt.client.Get(ctx, types.NamespacedName{Name: nodeName}, nrt); err != nil {
 		return nil, false
 	}
 	return nrt, true