Skip to content

Commit 92f8407

Browse files
marquizmarquiz
committed
topology-updater: ditch apihelper
Stop using pkg/apihelper for accessing the Kubernetes API. Modify unit tests to use the fake kubernetes client instead of mocked apihelper interface.
1 parent 5d30be1 commit 92f8407

File tree

3 files changed

+50
-56
lines changed

3 files changed

+50
-56
lines changed

‎pkg/nfd-topology-updater/nfd-topology-updater.go

+7-4
Original file line numberDiff line numberDiff line change
@@ -26,12 +26,12 @@ import (
2626

2727
"k8s.io/apimachinery/pkg/api/errors"
2828
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
29+
k8sclient "k8s.io/client-go/kubernetes"
2930
"k8s.io/klog/v2"
3031
kubeletconfigv1beta1 "k8s.io/kubelet/config/v1beta1"
3132

3233
"github.com/k8stopologyawareschedwg/noderesourcetopology-api/pkg/apis/topology/v1alpha2"
3334
topologyclientset "github.com/k8stopologyawareschedwg/noderesourcetopology-api/pkg/generated/clientset/versioned"
34-
"sigs.k8s.io/node-feature-discovery/pkg/apihelper"
3535
"sigs.k8s.io/node-feature-discovery/pkg/nfd-topology-updater/kubeletnotifier"
3636
"sigs.k8s.io/node-feature-discovery/pkg/podres"
3737
"sigs.k8s.io/node-feature-discovery/pkg/resourcemonitor"
@@ -74,7 +74,6 @@ type NfdTopologyUpdater interface {
7474
type nfdTopologyUpdater struct {
7575
nodeName string
7676
args Args
77-
apihelper apihelper.APIHelpers
7877
topoClient topologyclientset.Interface
7978
resourcemonitorArgs resourcemonitor.Args
8079
stop chan struct{} // channel for signaling stop
@@ -137,13 +136,17 @@ func (w *nfdTopologyUpdater) Run() error {
137136
if err != nil {
138137
return err
139138
}
140-
w.apihelper = apihelper.K8sHelpers{Kubeconfig: kubeconfig}
141139
topoClient, err := topologyclientset.NewForConfig(kubeconfig)
142140
if err != nil {
143141
return nil
144142
}
145143
w.topoClient = topoClient
146144

145+
k8sClient, err := k8sclient.NewForConfig(kubeconfig)
146+
if err != nil {
147+
return nil
148+
}
149+
147150
if err := w.configure(); err != nil {
148151
return fmt.Errorf("faild to configure Node Feature Discovery Topology Updater: %w", err)
149152
}
@@ -160,7 +163,7 @@ func (w *nfdTopologyUpdater) Run() error {
160163

161164
var resScan resourcemonitor.ResourcesScanner
162165

163-
resScan, err = resourcemonitor.NewPodResourcesScanner(w.resourcemonitorArgs.Namespace, podResClient, w.apihelper, w.resourcemonitorArgs.PodSetFingerprint)
166+
resScan, err = resourcemonitor.NewPodResourcesScanner(w.resourcemonitorArgs.Namespace, podResClient, k8sClient, w.resourcemonitorArgs.PodSetFingerprint)
164167
if err != nil {
165168
return fmt.Errorf("failed to initialize ResourceMonitor instance: %w", err)
166169
}

‎pkg/resourcemonitor/podresourcesscanner.go

+6-10
Original file line numberDiff line numberDiff line change
@@ -23,28 +23,28 @@ import (
2323

2424
corev1 "k8s.io/api/core/v1"
2525
"k8s.io/apimachinery/pkg/api/resource"
26+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
27+
client "k8s.io/client-go/kubernetes"
2628
"k8s.io/klog/v2"
2729
podresourcesapi "k8s.io/kubelet/pkg/apis/podresources/v1"
2830

29-
"sigs.k8s.io/node-feature-discovery/pkg/apihelper"
30-
3131
"github.com/k8stopologyawareschedwg/noderesourcetopology-api/pkg/apis/topology/v1alpha2"
3232
"github.com/k8stopologyawareschedwg/podfingerprint"
3333
)
3434

3535
type PodResourcesScanner struct {
3636
namespace string
3737
podResourceClient podresourcesapi.PodResourcesListerClient
38-
apihelper apihelper.APIHelpers
38+
k8sClient client.Interface
3939
podFingerprint bool
4040
}
4141

4242
// NewPodResourcesScanner creates a new ResourcesScanner instance
43-
func NewPodResourcesScanner(namespace string, podResourceClient podresourcesapi.PodResourcesListerClient, kubeApihelper apihelper.APIHelpers, podFingerprint bool) (ResourcesScanner, error) {
43+
func NewPodResourcesScanner(namespace string, podResourceClient podresourcesapi.PodResourcesListerClient, k8sClient client.Interface, podFingerprint bool) (ResourcesScanner, error) {
4444
resourcemonitorInstance := &PodResourcesScanner{
4545
namespace: namespace,
4646
podResourceClient: podResourceClient,
47-
apihelper: kubeApihelper,
47+
k8sClient: k8sClient,
4848
podFingerprint: podFingerprint,
4949
}
5050
if resourcemonitorInstance.namespace != "*" {
@@ -58,11 +58,7 @@ func NewPodResourcesScanner(namespace string, podResourceClient podresourcesapi.
5858

5959
// isWatchable tells if the the given namespace should be watched.
6060
func (resMon *PodResourcesScanner) isWatchable(podNamespace string, podName string, hasDevice bool) (bool, bool, error) {
61-
cli, err := resMon.apihelper.GetClient()
62-
if err != nil {
63-
return false, false, err
64-
}
65-
pod, err := resMon.apihelper.GetPod(cli, podNamespace, podName)
61+
pod, err := resMon.k8sClient.CoreV1().Pods(podNamespace).Get(context.TODO(), podName, metav1.GetOptions{})
6662
if err != nil {
6763
return false, false, err
6864
}

‎pkg/resourcemonitor/podresourcesscanner_test.go

+37-42
Original file line numberDiff line numberDiff line change
@@ -25,22 +25,16 @@ import (
2525
"github.com/k8stopologyawareschedwg/podfingerprint"
2626
. "github.com/smartystreets/goconvey/convey"
2727
"github.com/stretchr/testify/mock"
28-
2928
corev1 "k8s.io/api/core/v1"
3029
"k8s.io/apimachinery/pkg/api/resource"
3130
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
32-
k8sclient "k8s.io/client-go/kubernetes"
31+
fakeclient "k8s.io/client-go/kubernetes/fake"
3332
v1 "k8s.io/kubelet/pkg/apis/podresources/v1"
3433

35-
"sigs.k8s.io/node-feature-discovery/pkg/apihelper"
36-
mockv1 "sigs.k8s.io/node-feature-discovery/pkg/podres/mocks"
34+
mockpodres "sigs.k8s.io/node-feature-discovery/pkg/podres/mocks"
3735
)
3836

3937
func TestPodScanner(t *testing.T) {
40-
41-
var resScan ResourcesScanner
42-
var err error
43-
4438
// PodFingerprint only depends on Name/Namespace of the pods running on a Node
4539
// so we can precalculate the expected value
4640
expectedFingerprintCompute := func(pods []*corev1.Pod) (string, error) {
@@ -54,11 +48,11 @@ func TestPodScanner(t *testing.T) {
5448
}
5549

5650
Convey("When I scan for pod resources using fake client and no namespace", t, func() {
57-
mockPodResClient := new(mockv1.PodResourcesListerClient)
58-
mockAPIHelper := new(apihelper.MockAPIHelpers)
59-
mockClient := &k8sclient.Clientset{}
51+
mockPodResClient := new(mockpodres.PodResourcesListerClient)
52+
53+
fakeCli := fakeclient.NewSimpleClientset()
6054
computePodFingerprint := true
61-
resScan, err = NewPodResourcesScanner("*", mockPodResClient, mockAPIHelper, computePodFingerprint)
55+
resScan, err := NewPodResourcesScanner("*", mockPodResClient, fakeCli, computePodFingerprint)
6256

6357
Convey("Creating a Resources Scanner using a mock client", func() {
6458
So(err, ShouldBeNil)
@@ -172,8 +166,9 @@ func TestPodScanner(t *testing.T) {
172166
},
173167
},
174168
}
175-
mockAPIHelper.On("GetClient").Return(mockClient, nil)
176-
mockAPIHelper.On("GetPod", mock.AnythingOfType("*kubernetes.Clientset"), "default", "test-pod-0").Return(pod, nil)
169+
170+
fakeCli := fakeclient.NewSimpleClientset(pod)
171+
resScan.(*PodResourcesScanner).k8sClient = fakeCli
177172
res, err := resScan.Scan()
178173

179174
Convey("Error is nil", func() {
@@ -286,8 +281,9 @@ func TestPodScanner(t *testing.T) {
286281
},
287282
},
288283
}
289-
mockAPIHelper.On("GetClient").Return(mockClient, nil)
290-
mockAPIHelper.On("GetPod", mock.AnythingOfType("*kubernetes.Clientset"), "default", "test-pod-0").Return(pod, nil)
284+
285+
fakeCli = fakeclient.NewSimpleClientset(pod)
286+
resScan.(*PodResourcesScanner).k8sClient = fakeCli
291287
res, err := resScan.Scan()
292288

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

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

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

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

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

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

685680
Convey("Creating a Resources Scanner using a mock client", func() {
686681
So(err, ShouldBeNil)
@@ -752,12 +747,12 @@ func TestPodScanner(t *testing.T) {
752747
Name: "test-cnt-0",
753748
Resources: corev1.ResourceRequirements{
754749
Requests: corev1.ResourceList{
755-
corev1.ResourceCPU: *resource.NewQuantity(2, resource.DecimalSI),
750+
corev1.ResourceCPU: *resource.NewQuantity(1, resource.DecimalSI),
756751
corev1.ResourceName("fake.io/resource"): *resource.NewQuantity(1, resource.DecimalSI),
757752
corev1.ResourceMemory: *resource.NewQuantity(100, resource.DecimalSI),
758753
},
759754
Limits: corev1.ResourceList{
760-
corev1.ResourceCPU: *resource.NewQuantity(2, resource.DecimalSI),
755+
corev1.ResourceCPU: *resource.NewQuantity(1, resource.DecimalSI),
761756
corev1.ResourceName("fake.io/resource"): *resource.NewQuantity(1, resource.DecimalSI),
762757
corev1.ResourceMemory: *resource.NewQuantity(100, resource.DecimalSI),
763758
},
@@ -766,8 +761,8 @@ func TestPodScanner(t *testing.T) {
766761
},
767762
},
768763
}
769-
mockAPIHelper.On("GetClient").Return(mockClient, nil)
770-
mockAPIHelper.On("GetPod", mock.AnythingOfType("*kubernetes.Clientset"), "default", "test-pod-0").Return(pod, nil)
764+
fakeCli = fakeclient.NewSimpleClientset(pod)
765+
resScan.(*PodResourcesScanner).k8sClient = fakeCli
771766
res, err := resScan.Scan()
772767

773768
Convey("Error is nil", func() {
@@ -831,8 +826,8 @@ func TestPodScanner(t *testing.T) {
831826
},
832827
},
833828
}
834-
mockAPIHelper.On("GetClient").Return(mockClient, nil)
835-
mockAPIHelper.On("GetPod", mock.AnythingOfType("*kubernetes.Clientset"), "pod-res-test", "test-pod-0").Return(pod, nil)
829+
fakeCli = fakeclient.NewSimpleClientset(pod)
830+
resScan.(*PodResourcesScanner).k8sClient = fakeCli
836831
res, err := resScan.Scan()
837832

838833
Convey("Error is nil", func() {
@@ -911,8 +906,8 @@ func TestPodScanner(t *testing.T) {
911906
},
912907
},
913908
}
914-
mockAPIHelper.On("GetClient").Return(mockClient, nil)
915-
mockAPIHelper.On("GetPod", mock.AnythingOfType("*kubernetes.Clientset"), "default", "test-pod-0").Return(pod, nil)
909+
fakeCli = fakeclient.NewSimpleClientset(pod)
910+
resScan.(*PodResourcesScanner).k8sClient = fakeCli
916911
res, err := resScan.Scan()
917912

918913
Convey("Error is nil", func() {
@@ -977,8 +972,8 @@ func TestPodScanner(t *testing.T) {
977972
},
978973
},
979974
}
980-
mockAPIHelper.On("GetClient").Return(mockClient, nil)
981-
mockAPIHelper.On("GetPod", mock.AnythingOfType("*kubernetes.Clientset"), "default", "test-pod-0").Return(pod, nil)
975+
fakeCli = fakeclient.NewSimpleClientset(pod)
976+
resScan.(*PodResourcesScanner).k8sClient = fakeCli
982977
res, err := resScan.Scan()
983978

984979
Convey("Error is nil", func() {
@@ -1035,8 +1030,8 @@ func TestPodScanner(t *testing.T) {
10351030
},
10361031
},
10371032
}
1038-
mockAPIHelper.On("GetClient").Return(mockClient, nil)
1039-
mockAPIHelper.On("GetPod", mock.AnythingOfType("*kubernetes.Clientset"), "pod-res-test", "test-pod-0").Return(pod, nil)
1033+
fakeCli = fakeclient.NewSimpleClientset(pod)
1034+
resScan.(*PodResourcesScanner).k8sClient = fakeCli
10401035
res, err := resScan.Scan()
10411036

10421037
Convey("Error is nil", func() {
@@ -1119,8 +1114,8 @@ func TestPodScanner(t *testing.T) {
11191114
},
11201115
},
11211116
}
1122-
mockAPIHelper.On("GetClient").Return(mockClient, nil)
1123-
mockAPIHelper.On("GetPod", mock.AnythingOfType("*kubernetes.Clientset"), "pod-res-test", "test-pod-0").Return(pod, nil)
1117+
fakeCli = fakeclient.NewSimpleClientset(pod)
1118+
resScan.(*PodResourcesScanner).k8sClient = fakeCli
11241119
res, err := resScan.Scan()
11251120

11261121
Convey("Error is nil", func() {

0 commit comments

Comments
 (0)