Skip to content

Commit

Permalink
Merge pull request #1567 from marquiz/devel/apihelper-refactor-3
Browse files Browse the repository at this point in the history 
topology-updater: ditch apihelper
  • Loading branch information
@k8s-ci-robot
k8s-ci-robot authored Jan 26, 2024
2 parents 8e53bb3 + c581a25 commit 33858b7
Show file tree
Hide file tree
Showing 3 changed files with 50 additions and 56 deletions.
11 changes: 7 additions & 4 deletions pkg/nfd-topology-updater/nfd-topology-updater.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,12 +26,12 @@ import (

"k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
k8sclient "k8s.io/client-go/kubernetes"
"k8s.io/klog/v2"
kubeletconfigv1beta1 "k8s.io/kubelet/config/v1beta1"

"github.com/k8stopologyawareschedwg/noderesourcetopology-api/pkg/apis/topology/v1alpha2"
topologyclientset "github.com/k8stopologyawareschedwg/noderesourcetopology-api/pkg/generated/clientset/versioned"
"sigs.k8s.io/node-feature-discovery/pkg/apihelper"
"sigs.k8s.io/node-feature-discovery/pkg/nfd-topology-updater/kubeletnotifier"
"sigs.k8s.io/node-feature-discovery/pkg/podres"
"sigs.k8s.io/node-feature-discovery/pkg/resourcemonitor"
Expand Down Expand Up @@ -74,7 +74,6 @@ type NfdTopologyUpdater interface {
type nfdTopologyUpdater struct {
nodeName string
args Args
apihelper apihelper.APIHelpers
topoClient topologyclientset.Interface
resourcemonitorArgs resourcemonitor.Args
stop chan struct{} // channel for signaling stop
Expand Down Expand Up @@ -137,13 +136,17 @@ func (w *nfdTopologyUpdater) Run() error {
if err != nil {
return err
}
w.apihelper = apihelper.K8sHelpers{Kubeconfig: kubeconfig}
topoClient, err := topologyclientset.NewForConfig(kubeconfig)
if err != nil {
return nil
}
w.topoClient = topoClient

k8sClient, err := k8sclient.NewForConfig(kubeconfig)
if err != nil {
return err
}

if err := w.configure(); err != nil {
return fmt.Errorf("faild to configure Node Feature Discovery Topology Updater: %w", err)
}
Expand All @@ -160,7 +163,7 @@ func (w *nfdTopologyUpdater) Run() error {

var resScan resourcemonitor.ResourcesScanner

resScan, err = resourcemonitor.NewPodResourcesScanner(w.resourcemonitorArgs.Namespace, podResClient, w.apihelper, w.resourcemonitorArgs.PodSetFingerprint)
resScan, err = resourcemonitor.NewPodResourcesScanner(w.resourcemonitorArgs.Namespace, podResClient, k8sClient, w.resourcemonitorArgs.PodSetFingerprint)
if err != nil {
return fmt.Errorf("failed to initialize ResourceMonitor instance: %w", err)
}
Expand Down
16 changes: 6 additions & 10 deletions pkg/resourcemonitor/podresourcesscanner.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,28 +23,28 @@ import (

corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/resource"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
client "k8s.io/client-go/kubernetes"
"k8s.io/klog/v2"
podresourcesapi "k8s.io/kubelet/pkg/apis/podresources/v1"

"sigs.k8s.io/node-feature-discovery/pkg/apihelper"

"github.com/k8stopologyawareschedwg/noderesourcetopology-api/pkg/apis/topology/v1alpha2"
"github.com/k8stopologyawareschedwg/podfingerprint"
)

type PodResourcesScanner struct {
namespace string
podResourceClient podresourcesapi.PodResourcesListerClient
apihelper apihelper.APIHelpers
k8sClient client.Interface
podFingerprint bool
}

// NewPodResourcesScanner creates a new ResourcesScanner instance
func NewPodResourcesScanner(namespace string, podResourceClient podresourcesapi.PodResourcesListerClient, kubeApihelper apihelper.APIHelpers, podFingerprint bool) (ResourcesScanner, error) {
func NewPodResourcesScanner(namespace string, podResourceClient podresourcesapi.PodResourcesListerClient, k8sClient client.Interface, podFingerprint bool) (ResourcesScanner, error) {
resourcemonitorInstance := &PodResourcesScanner{
namespace: namespace,
podResourceClient: podResourceClient,
apihelper: kubeApihelper,
k8sClient: k8sClient,
podFingerprint: podFingerprint,
}
if resourcemonitorInstance.namespace != "*" {
Expand All @@ -58,11 +58,7 @@ func NewPodResourcesScanner(namespace string, podResourceClient podresourcesapi.

// isWatchable tells if the the given namespace should be watched.
func (resMon *PodResourcesScanner) isWatchable(podNamespace string, podName string, hasDevice bool) (bool, bool, error) {
cli, err := resMon.apihelper.GetClient()
if err != nil {
return false, false, err
}
pod, err := resMon.apihelper.GetPod(cli, podNamespace, podName)
pod, err := resMon.k8sClient.CoreV1().Pods(podNamespace).Get(context.TODO(), podName, metav1.GetOptions{})
if err != nil {
return false, false, err
}
Expand Down
79 changes: 37 additions & 42 deletions pkg/resourcemonitor/podresourcesscanner_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,22 +25,16 @@ import (
"github.com/k8stopologyawareschedwg/podfingerprint"
. "github.com/smartystreets/goconvey/convey"
"github.com/stretchr/testify/mock"

corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/resource"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
k8sclient "k8s.io/client-go/kubernetes"
fakeclient "k8s.io/client-go/kubernetes/fake"
v1 "k8s.io/kubelet/pkg/apis/podresources/v1"

"sigs.k8s.io/node-feature-discovery/pkg/apihelper"
mockv1 "sigs.k8s.io/node-feature-discovery/pkg/podres/mocks"
mockpodres "sigs.k8s.io/node-feature-discovery/pkg/podres/mocks"
)

func TestPodScanner(t *testing.T) {

var resScan ResourcesScanner
var err error

// PodFingerprint only depends on Name/Namespace of the pods running on a Node
// so we can precalculate the expected value
expectedFingerprintCompute := func(pods []*corev1.Pod) (string, error) {
Expand All @@ -54,11 +48,11 @@ func TestPodScanner(t *testing.T) {
}

Convey("When I scan for pod resources using fake client and no namespace", t, func() {
mockPodResClient := new(mockv1.PodResourcesListerClient)
mockAPIHelper := new(apihelper.MockAPIHelpers)
mockClient := &k8sclient.Clientset{}
mockPodResClient := new(mockpodres.PodResourcesListerClient)

fakeCli := fakeclient.NewSimpleClientset()
computePodFingerprint := true
resScan, err = NewPodResourcesScanner("*", mockPodResClient, mockAPIHelper, computePodFingerprint)
resScan, err := NewPodResourcesScanner("*", mockPodResClient, fakeCli, computePodFingerprint)

Convey("Creating a Resources Scanner using a mock client", func() {
So(err, ShouldBeNil)
Expand Down Expand Up @@ -172,8 +166,9 @@ func TestPodScanner(t *testing.T) {
},
},
}
mockAPIHelper.On("GetClient").Return(mockClient, nil)
mockAPIHelper.On("GetPod", mock.AnythingOfType("*kubernetes.Clientset"), "default", "test-pod-0").Return(pod, nil)

fakeCli := fakeclient.NewSimpleClientset(pod)
resScan.(*PodResourcesScanner).k8sClient = fakeCli
res, err := resScan.Scan()

Convey("Error is nil", func() {
Expand Down Expand Up @@ -286,8 +281,9 @@ func TestPodScanner(t *testing.T) {
},
},
}
mockAPIHelper.On("GetClient").Return(mockClient, nil)
mockAPIHelper.On("GetPod", mock.AnythingOfType("*kubernetes.Clientset"), "default", "test-pod-0").Return(pod, nil)

fakeCli = fakeclient.NewSimpleClientset(pod)
resScan.(*PodResourcesScanner).k8sClient = fakeCli
res, err := resScan.Scan()

Convey("Error is nil", func() {
Expand Down Expand Up @@ -373,8 +369,8 @@ func TestPodScanner(t *testing.T) {
},
},
}
mockAPIHelper.On("GetClient").Return(mockClient, nil)
mockAPIHelper.On("GetPod", mock.AnythingOfType("*kubernetes.Clientset"), "default", "test-pod-0").Return(pod, nil)
fakeCli = fakeclient.NewSimpleClientset(pod)
resScan.(*PodResourcesScanner).k8sClient = fakeCli
res, err := resScan.Scan()

Convey("Error is nil", func() {
Expand Down Expand Up @@ -463,8 +459,8 @@ func TestPodScanner(t *testing.T) {
},
},
}
mockAPIHelper.On("GetClient").Return(mockClient, nil)
mockAPIHelper.On("GetPod", mock.AnythingOfType("*kubernetes.Clientset"), "default", "test-pod-0").Return(pod, nil)
fakeCli = fakeclient.NewSimpleClientset(pod)
resScan.(*PodResourcesScanner).k8sClient = fakeCli
res, err := resScan.Scan()

Convey("Error is nil", func() {
Expand Down Expand Up @@ -541,8 +537,8 @@ func TestPodScanner(t *testing.T) {
},
},
}
mockAPIHelper.On("GetClient").Return(mockClient, nil)
mockAPIHelper.On("GetPod", mock.AnythingOfType("*kubernetes.Clientset"), "default", "test-pod-0").Return(pod, nil)
fakeCli = fakeclient.NewSimpleClientset(pod)
resScan.(*PodResourcesScanner).k8sClient = fakeCli
res, err := resScan.Scan()

Convey("Error is nil", func() {
Expand Down Expand Up @@ -633,8 +629,8 @@ func TestPodScanner(t *testing.T) {
},
},
}
mockAPIHelper.On("GetClient").Return(mockClient, nil)
mockAPIHelper.On("GetPod", mock.AnythingOfType("*kubernetes.Clientset"), "default", "test-pod-0").Return(pod, nil)
fakeCli = fakeclient.NewSimpleClientset(pod)
resScan.(*PodResourcesScanner).k8sClient = fakeCli
res, err := resScan.Scan()

Convey("Error is nil", func() {
Expand Down Expand Up @@ -676,11 +672,10 @@ func TestPodScanner(t *testing.T) {
})

Convey("When I scan for pod resources using fake client and given namespace", t, func() {
mockPodResClient := new(mockv1.PodResourcesListerClient)
mockAPIHelper := new(apihelper.MockAPIHelpers)
mockClient := &k8sclient.Clientset{}
mockPodResClient := new(mockpodres.PodResourcesListerClient)
fakeCli := fakeclient.NewSimpleClientset()
computePodFingerprint := false
resScan, err = NewPodResourcesScanner("pod-res-test", mockPodResClient, mockAPIHelper, computePodFingerprint)
resScan, err := NewPodResourcesScanner("pod-res-test", mockPodResClient, fakeCli, computePodFingerprint)

Convey("Creating a Resources Scanner using a mock client", func() {
So(err, ShouldBeNil)
Expand Down Expand Up @@ -752,12 +747,12 @@ func TestPodScanner(t *testing.T) {
Name: "test-cnt-0",
Resources: corev1.ResourceRequirements{
Requests: corev1.ResourceList{
corev1.ResourceCPU: *resource.NewQuantity(2, resource.DecimalSI),
corev1.ResourceCPU: *resource.NewQuantity(1, resource.DecimalSI),
corev1.ResourceName("fake.io/resource"): *resource.NewQuantity(1, resource.DecimalSI),
corev1.ResourceMemory: *resource.NewQuantity(100, resource.DecimalSI),
},
Limits: corev1.ResourceList{
corev1.ResourceCPU: *resource.NewQuantity(2, resource.DecimalSI),
corev1.ResourceCPU: *resource.NewQuantity(1, resource.DecimalSI),
corev1.ResourceName("fake.io/resource"): *resource.NewQuantity(1, resource.DecimalSI),
corev1.ResourceMemory: *resource.NewQuantity(100, resource.DecimalSI),
},
Expand All @@ -766,8 +761,8 @@ func TestPodScanner(t *testing.T) {
},
},
}
mockAPIHelper.On("GetClient").Return(mockClient, nil)
mockAPIHelper.On("GetPod", mock.AnythingOfType("*kubernetes.Clientset"), "default", "test-pod-0").Return(pod, nil)
fakeCli = fakeclient.NewSimpleClientset(pod)
resScan.(*PodResourcesScanner).k8sClient = fakeCli
res, err := resScan.Scan()

Convey("Error is nil", func() {
Expand Down Expand Up @@ -831,8 +826,8 @@ func TestPodScanner(t *testing.T) {
},
},
}
mockAPIHelper.On("GetClient").Return(mockClient, nil)
mockAPIHelper.On("GetPod", mock.AnythingOfType("*kubernetes.Clientset"), "pod-res-test", "test-pod-0").Return(pod, nil)
fakeCli = fakeclient.NewSimpleClientset(pod)
resScan.(*PodResourcesScanner).k8sClient = fakeCli
res, err := resScan.Scan()

Convey("Error is nil", func() {
Expand Down Expand Up @@ -911,8 +906,8 @@ func TestPodScanner(t *testing.T) {
},
},
}
mockAPIHelper.On("GetClient").Return(mockClient, nil)
mockAPIHelper.On("GetPod", mock.AnythingOfType("*kubernetes.Clientset"), "default", "test-pod-0").Return(pod, nil)
fakeCli = fakeclient.NewSimpleClientset(pod)
resScan.(*PodResourcesScanner).k8sClient = fakeCli
res, err := resScan.Scan()

Convey("Error is nil", func() {
Expand Down Expand Up @@ -977,8 +972,8 @@ func TestPodScanner(t *testing.T) {
},
},
}
mockAPIHelper.On("GetClient").Return(mockClient, nil)
mockAPIHelper.On("GetPod", mock.AnythingOfType("*kubernetes.Clientset"), "default", "test-pod-0").Return(pod, nil)
fakeCli = fakeclient.NewSimpleClientset(pod)
resScan.(*PodResourcesScanner).k8sClient = fakeCli
res, err := resScan.Scan()

Convey("Error is nil", func() {
Expand Down Expand Up @@ -1035,8 +1030,8 @@ func TestPodScanner(t *testing.T) {
},
},
}
mockAPIHelper.On("GetClient").Return(mockClient, nil)
mockAPIHelper.On("GetPod", mock.AnythingOfType("*kubernetes.Clientset"), "pod-res-test", "test-pod-0").Return(pod, nil)
fakeCli = fakeclient.NewSimpleClientset(pod)
resScan.(*PodResourcesScanner).k8sClient = fakeCli
res, err := resScan.Scan()

Convey("Error is nil", func() {
Expand Down Expand Up @@ -1119,8 +1114,8 @@ func TestPodScanner(t *testing.T) {
},
},
}
mockAPIHelper.On("GetClient").Return(mockClient, nil)
mockAPIHelper.On("GetPod", mock.AnythingOfType("*kubernetes.Clientset"), "pod-res-test", "test-pod-0").Return(pod, nil)
fakeCli = fakeclient.NewSimpleClientset(pod)
resScan.(*PodResourcesScanner).k8sClient = fakeCli
res, err := resScan.Scan()

Convey("Error is nil", func() {
Expand Down

0 comments on commit 33858b7

Please sign in to comment.