Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

migrate nrt to ctrl runtime client #655

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 2 additions & 1 deletion go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -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=
Expand Down
118 changes: 0 additions & 118 deletions manifests/noderesourcetopology/crd.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
17 changes: 9 additions & 8 deletions pkg/networkaware/networkoverhead/networkoverhead.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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,
})
Expand Down
12 changes: 6 additions & 6 deletions pkg/networkaware/networkoverhead/networkoverhead_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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))

Expand Down Expand Up @@ -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))

Expand Down Expand Up @@ -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))

Expand Down Expand Up @@ -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))

Expand Down Expand Up @@ -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))

Expand Down
12 changes: 7 additions & 5 deletions pkg/networkaware/topologicalsort/topologicalsort.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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,
})
Expand Down
6 changes: 3 additions & 3 deletions pkg/networkaware/topologicalsort/topologicalsort_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: revert this?

Copy link
Member Author

@zwpaper zwpaper Nov 15, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the scheme name is in conflict with the var scheme for init schemes, changed to keep consistency

"k8s.io/client-go/util/workqueue"
"k8s.io/kubernetes/pkg/scheduler/framework"
st "k8s.io/kubernetes/pkg/scheduler/testing"
Expand Down Expand Up @@ -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).
Expand Down Expand Up @@ -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).
Expand Down
4 changes: 3 additions & 1 deletion pkg/noderesourcetopology/cache/cache.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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
Expand Down
18 changes: 10 additions & 8 deletions pkg/noderesourcetopology/cache/discardreserved.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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 {
Expand All @@ -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
Expand Down
Loading