Skip to content

Commit 96a33d1

Browse files
TomerNewmank8s-ci-robot
TomerNewman
authored and
k8s-ci-robot
committed
Creating a new node package
Today, we have some node's functionality spread across the code base. This commit is about creating a new interface gathering generic node's functionality to be used in other packages and mocked in unit-testing.
1 parent 39ac0dd commit 96a33d1

18 files changed

+349
-274
lines changed

‎cmd/manager/main.go

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ package main
1919
import (
2020
"flag"
2121
"fmt"
22+
"github.com/kubernetes-sigs/kernel-module-management/internal/node"
2223
"os"
2324
"strconv"
2425

@@ -118,13 +119,14 @@ func main() {
118119

119120
registryAPI := registry.NewRegistry()
120121
buildHelperAPI := build.NewHelper()
121-
122+
nodeAPI := node.NewNode(client)
122123
kernelAPI := module.NewKernelMapper(buildHelperAPI, sign.NewSignerHelper())
123124

124125
dpc := controllers.NewDevicePluginReconciler(
125126
client,
126127
metricsAPI,
127128
filterAPI,
129+
nodeAPI,
128130
scheme)
129131
if err = dpc.SetupWithManager(mgr); err != nil {
130132
cmd.FatalError(setupLogger, err, "unable to create controller", "name", controllers.DevicePluginReconcilerName)
@@ -136,6 +138,7 @@ func main() {
136138
registryAPI,
137139
nmcHelper,
138140
filterAPI,
141+
nodeAPI,
139142
scheme,
140143
)
141144
if err = mnc.SetupWithManager(mgr); err != nil {
@@ -146,7 +149,7 @@ func main() {
146149

147150
eventRecorder := mgr.GetEventRecorderFor("kmm")
148151

149-
if err = controllers.NewNMCReconciler(client, scheme, workerImage, &cfg.Worker, eventRecorder).SetupWithManager(ctx, mgr); err != nil {
152+
if err = controllers.NewNMCReconciler(client, scheme, workerImage, &cfg.Worker, eventRecorder, nodeAPI).SetupWithManager(ctx, mgr); err != nil {
150153
cmd.FatalError(setupLogger, err, "unable to create controller", "name", controllers.NodeModulesConfigReconcilerName)
151154
}
152155

@@ -184,13 +187,13 @@ func main() {
184187
podHelperAPI,
185188
registryAPI,
186189
)
187-
188190
bsc := controllers.NewBuildSignReconciler(
189191
client,
190192
buildAPI,
191193
signAPI,
192194
kernelAPI,
193-
filterAPI)
195+
filterAPI,
196+
nodeAPI)
194197
if err = bsc.SetupWithManager(mgr, constants.KernelLabel); err != nil {
195198
cmd.FatalError(setupLogger, err, "unable to create controller", "name", controllers.BuildSignReconcilerName)
196199
}

‎internal/controllers/build_sign_reconciler.go

Lines changed: 6 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -19,13 +19,12 @@ package controllers
1919
import (
2020
"context"
2121
"fmt"
22-
"strings"
23-
2422
kmmv1beta1 "github.com/kubernetes-sigs/kernel-module-management/api/v1beta1"
2523
"github.com/kubernetes-sigs/kernel-module-management/internal/api"
2624
"github.com/kubernetes-sigs/kernel-module-management/internal/build"
2725
"github.com/kubernetes-sigs/kernel-module-management/internal/filter"
2826
"github.com/kubernetes-sigs/kernel-module-management/internal/module"
27+
"github.com/kubernetes-sigs/kernel-module-management/internal/node"
2928
"github.com/kubernetes-sigs/kernel-module-management/internal/sign"
3029
"github.com/kubernetes-sigs/kernel-module-management/internal/utils"
3130
v1 "k8s.io/api/core/v1"
@@ -35,6 +34,7 @@ import (
3534
"sigs.k8s.io/controller-runtime/pkg/handler"
3635
"sigs.k8s.io/controller-runtime/pkg/log"
3736
"sigs.k8s.io/controller-runtime/pkg/reconcile"
37+
"strings"
3838
)
3939

4040
const BuildSignReconcilerName = "BuildSignReconciler"
@@ -43,6 +43,7 @@ const BuildSignReconcilerName = "BuildSignReconciler"
4343
type BuildSignReconciler struct {
4444
filter *filter.Filter
4545
reconHelperAPI buildSignReconcilerHelperAPI
46+
nodeAPI node.Node
4647
}
4748

4849
func NewBuildSignReconciler(
@@ -51,11 +52,13 @@ func NewBuildSignReconciler(
5152
signAPI sign.SignManager,
5253
kernelAPI module.KernelMapper,
5354
filter *filter.Filter,
55+
nodeAPI node.Node,
5456
) *BuildSignReconciler {
5557
reconHelperAPI := newBuildSignReconcilerHelper(client, buildAPI, signAPI, kernelAPI)
5658
return &BuildSignReconciler{
5759
reconHelperAPI: reconHelperAPI,
5860
filter: filter,
61+
nodeAPI: nodeAPI,
5962
}
6063
}
6164

@@ -73,8 +76,7 @@ func (r *BuildSignReconciler) Reconcile(ctx context.Context, mod *kmmv1beta1.Mod
7376
res := ctrl.Result{}
7477

7578
logger := log.FromContext(ctx)
76-
77-
targetedNodes, err := r.reconHelperAPI.getNodesListBySelector(ctx, mod)
79+
targetedNodes, err := r.nodeAPI.GetNodesListBySelector(ctx, mod.Spec.Selector)
7880
if err != nil {
7981
return res, fmt.Errorf("could get targeted nodes for module %s: %w", mod.Name, err)
8082
}
@@ -121,7 +123,6 @@ func (r *BuildSignReconciler) Reconcile(ctx context.Context, mod *kmmv1beta1.Mod
121123
//go:generate mockgen -source=build_sign_reconciler.go -package=controllers -destination=mock_build_sign_reconciler.go buildSignReconcilerHelperAPI
122124

123125
type buildSignReconcilerHelperAPI interface {
124-
getNodesListBySelector(ctx context.Context, mod *kmmv1beta1.Module) ([]v1.Node, error)
125126
getRelevantKernelMappings(ctx context.Context, mod *kmmv1beta1.Module, targetedNodes []v1.Node) (map[string]*api.ModuleLoaderData, error)
126127
handleBuild(ctx context.Context, mld *api.ModuleLoaderData) (bool, error)
127128
handleSigning(ctx context.Context, mld *api.ModuleLoaderData) (bool, error)
@@ -146,7 +147,6 @@ func newBuildSignReconcilerHelper(client client.Client,
146147
kernelAPI: kernelAPI,
147148
}
148149
}
149-
150150
func (bsrh *buildSignReconcilerHelper) getRelevantKernelMappings(ctx context.Context,
151151
mod *kmmv1beta1.Module,
152152
targetedNodes []v1.Node) (map[string]*api.ModuleLoaderData, error) {
@@ -183,26 +183,6 @@ func (bsrh *buildSignReconcilerHelper) getRelevantKernelMappings(ctx context.Con
183183
return mldMappings, nil
184184
}
185185

186-
func (bsrh *buildSignReconcilerHelper) getNodesListBySelector(ctx context.Context, mod *kmmv1beta1.Module) ([]v1.Node, error) {
187-
logger := log.FromContext(ctx)
188-
logger.V(1).Info("Listing nodes", "selector", mod.Spec.Selector)
189-
190-
selectedNodes := v1.NodeList{}
191-
opt := client.MatchingLabels(mod.Spec.Selector)
192-
if err := bsrh.client.List(ctx, &selectedNodes, opt); err != nil {
193-
logger.Error(err, "Could not list nodes")
194-
return nil, fmt.Errorf("could not list nodes: %v", err)
195-
}
196-
nodes := make([]v1.Node, 0, len(selectedNodes.Items))
197-
198-
for _, node := range selectedNodes.Items {
199-
if utils.IsNodeSchedulable(&node) {
200-
nodes = append(nodes, node)
201-
}
202-
}
203-
return nodes, nil
204-
}
205-
206186
// handleBuild returns true if build is not needed or finished successfully
207187
func (bsrh *buildSignReconcilerHelper) handleBuild(ctx context.Context, mld *api.ModuleLoaderData) (bool, error) {
208188

‎internal/controllers/build_sign_reconciler_test.go

Lines changed: 10 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,8 @@ import (
77
kmmv1beta1 "github.com/kubernetes-sigs/kernel-module-management/api/v1beta1"
88
"github.com/kubernetes-sigs/kernel-module-management/internal/api"
99
"github.com/kubernetes-sigs/kernel-module-management/internal/build"
10-
"github.com/kubernetes-sigs/kernel-module-management/internal/client"
1110
"github.com/kubernetes-sigs/kernel-module-management/internal/module"
11+
"github.com/kubernetes-sigs/kernel-module-management/internal/node"
1212
"github.com/kubernetes-sigs/kernel-module-management/internal/sign"
1313
"github.com/kubernetes-sigs/kernel-module-management/internal/utils"
1414
. "github.com/onsi/ginkgo/v2"
@@ -27,18 +27,21 @@ var _ = Describe("BuildSignReconciler_Reconcile", func() {
2727
mockReconHelper *MockbuildSignReconcilerHelperAPI
2828
mod *kmmv1beta1.Module
2929
bsr *BuildSignReconciler
30+
mn *node.MockNode
3031
)
3132

3233
BeforeEach(func() {
3334
ctrl = gomock.NewController(GinkgoT())
3435
mockReconHelper = NewMockbuildSignReconcilerHelperAPI(ctrl)
36+
mn = node.NewMockNode(ctrl)
3537

3638
mod = &kmmv1beta1.Module{
3739
ObjectMeta: metav1.ObjectMeta{Namespace: namespace, Name: moduleName},
3840
}
3941

4042
bsr = &BuildSignReconciler{
4143
reconHelperAPI: mockReconHelper,
44+
nodeAPI: mn,
4245
}
4346
})
4447

@@ -52,10 +55,10 @@ var _ = Describe("BuildSignReconciler_Reconcile", func() {
5255
mappings := map[string]*api.ModuleLoaderData{"kernelVersion": &api.ModuleLoaderData{}}
5356
returnedError := fmt.Errorf("some error")
5457
if getNodesError {
55-
mockReconHelper.EXPECT().getNodesListBySelector(ctx, &mod).Return(nil, returnedError)
58+
mn.EXPECT().GetNodesListBySelector(ctx, mod.Spec.Selector).Return(nil, returnedError)
5659
goto executeTestFunction
5760
}
58-
mockReconHelper.EXPECT().getNodesListBySelector(ctx, &mod).Return(selectNodesList, nil)
61+
mn.EXPECT().GetNodesListBySelector(ctx, mod.Spec.Selector).Return(selectNodesList, nil)
5962
if getMappingsError {
6063
mockReconHelper.EXPECT().getRelevantKernelMappings(ctx, &mod, selectNodesList).Return(nil, returnedError)
6164
goto executeTestFunction
@@ -90,7 +93,7 @@ var _ = Describe("BuildSignReconciler_Reconcile", func() {
9093
selectNodesList := []v1.Node{v1.Node{}}
9194
mappings := map[string]*api.ModuleLoaderData{"kernelVersion": &api.ModuleLoaderData{}}
9295
gomock.InOrder(
93-
mockReconHelper.EXPECT().getNodesListBySelector(ctx, mod).Return(selectNodesList, nil),
96+
mn.EXPECT().GetNodesListBySelector(ctx, mod.Spec.Selector).Return(selectNodesList, nil),
9497
mockReconHelper.EXPECT().getRelevantKernelMappings(ctx, mod, selectNodesList).Return(mappings, nil),
9598
mockReconHelper.EXPECT().handleBuild(ctx, mappings["kernelVersion"]).Return(false, nil),
9699
mockReconHelper.EXPECT().garbageCollect(ctx, mod, mappings).Return(nil),
@@ -106,7 +109,7 @@ var _ = Describe("BuildSignReconciler_Reconcile", func() {
106109
selectNodesList := []v1.Node{v1.Node{}}
107110
mappings := map[string]*api.ModuleLoaderData{"kernelVersion": &api.ModuleLoaderData{}}
108111
gomock.InOrder(
109-
mockReconHelper.EXPECT().getNodesListBySelector(ctx, mod).Return(selectNodesList, nil),
112+
mn.EXPECT().GetNodesListBySelector(ctx, mod.Spec.Selector).Return(selectNodesList, nil),
110113
mockReconHelper.EXPECT().getRelevantKernelMappings(ctx, mod, selectNodesList).Return(mappings, nil),
111114
mockReconHelper.EXPECT().handleBuild(ctx, mappings["kernelVersion"]).Return(true, nil),
112115
mockReconHelper.EXPECT().handleSigning(ctx, mappings["kernelVersion"]).Return(false, nil),
@@ -120,10 +123,11 @@ var _ = Describe("BuildSignReconciler_Reconcile", func() {
120123
})
121124

122125
It("Good flow", func() {
126+
123127
selectNodesList := []v1.Node{v1.Node{}}
124128
mappings := map[string]*api.ModuleLoaderData{"kernelVersion": &api.ModuleLoaderData{}}
125129
gomock.InOrder(
126-
mockReconHelper.EXPECT().getNodesListBySelector(ctx, mod).Return(selectNodesList, nil),
130+
mn.EXPECT().GetNodesListBySelector(ctx, mod.Spec.Selector).Return(selectNodesList, nil),
127131
mockReconHelper.EXPECT().getRelevantKernelMappings(ctx, mod, selectNodesList).Return(mappings, nil),
128132
mockReconHelper.EXPECT().handleBuild(ctx, mappings["kernelVersion"]).Return(true, nil),
129133
mockReconHelper.EXPECT().handleSigning(ctx, mappings["kernelVersion"]).Return(true, nil),
@@ -137,62 +141,6 @@ var _ = Describe("BuildSignReconciler_Reconcile", func() {
137141
})
138142
})
139143

140-
var _ = Describe("BuildSignReconciler_getNodesListBySelector", func() {
141-
var (
142-
ctrl *gomock.Controller
143-
clnt *client.MockClient
144-
bsrh buildSignReconcilerHelperAPI
145-
)
146-
147-
BeforeEach(func() {
148-
ctrl = gomock.NewController(GinkgoT())
149-
clnt = client.NewMockClient(ctrl)
150-
bsrh = newBuildSignReconcilerHelper(clnt, nil, nil, nil)
151-
})
152-
153-
It("list failed", func() {
154-
clnt.EXPECT().List(context.Background(), gomock.Any(), gomock.Any()).Return(fmt.Errorf("some error"))
155-
156-
nodes, err := bsrh.getNodesListBySelector(context.Background(), &kmmv1beta1.Module{})
157-
158-
Expect(err).To(HaveOccurred())
159-
Expect(nodes).To(BeNil())
160-
})
161-
162-
It("Return only schedulable nodes", func() {
163-
node1 := v1.Node{
164-
Spec: v1.NodeSpec{
165-
Taints: []v1.Taint{
166-
v1.Taint{
167-
Effect: v1.TaintEffectNoSchedule,
168-
},
169-
},
170-
},
171-
}
172-
node2 := v1.Node{}
173-
node3 := v1.Node{
174-
Spec: v1.NodeSpec{
175-
Taints: []v1.Taint{
176-
v1.Taint{
177-
Effect: v1.TaintEffectPreferNoSchedule,
178-
},
179-
},
180-
},
181-
}
182-
clnt.EXPECT().List(context.Background(), gomock.Any(), gomock.Any()).DoAndReturn(
183-
func(_ interface{}, list *v1.NodeList, _ ...interface{}) error {
184-
list.Items = []v1.Node{node1, node2, node3}
185-
return nil
186-
},
187-
)
188-
nodes, err := bsrh.getNodesListBySelector(context.Background(), &kmmv1beta1.Module{})
189-
190-
Expect(err).NotTo(HaveOccurred())
191-
Expect(nodes).To(Equal([]v1.Node{node2, node3}))
192-
193-
})
194-
})
195-
196144
var _ = Describe("BuildSignReconciler_getRelevantKernelMappings", func() {
197145
var (
198146
ctrl *gomock.Controller

‎internal/controllers/device_plugin_reconciler.go

Lines changed: 9 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import (
2020
"context"
2121
"errors"
2222
"fmt"
23+
"github.com/kubernetes-sigs/kernel-module-management/internal/node"
2324
"strings"
2425

2526
kmmv1beta1 "github.com/kubernetes-sigs/kernel-module-management/api/v1beta1"
@@ -56,9 +57,10 @@ func NewDevicePluginReconciler(
5657
client client.Client,
5758
metricsAPI metrics.Metrics,
5859
filter *filter.Filter,
60+
nodeAPI node.Node,
5961
scheme *runtime.Scheme,
6062
) *DevicePluginReconciler {
61-
reconHelperAPI := newDevicePluginReconcilerHelper(client, metricsAPI, scheme)
63+
reconHelperAPI := newDevicePluginReconcilerHelper(client, metricsAPI, nodeAPI, scheme)
6264
return &DevicePluginReconciler{
6365
client: client,
6466
reconHelperAPI: reconHelperAPI,
@@ -136,16 +138,20 @@ type devicePluginReconcilerHelper struct {
136138
client client.Client
137139
metricsAPI metrics.Metrics
138140
daemonSetHelper daemonSetCreator
141+
nodeAPI node.Node
139142
}
140143

141144
func newDevicePluginReconcilerHelper(client client.Client,
142145
metricsAPI metrics.Metrics,
143-
scheme *runtime.Scheme) devicePluginReconcilerHelperAPI {
146+
nodeAPI node.Node,
147+
scheme *runtime.Scheme,
148+
) devicePluginReconcilerHelperAPI {
144149
daemonSetHelper := newDaemonSetCreator(scheme)
145150
return &devicePluginReconcilerHelper{
146151
client: client,
147152
metricsAPI: metricsAPI,
148153
daemonSetHelper: daemonSetHelper,
154+
nodeAPI: nodeAPI,
149155
}
150156
}
151157

@@ -274,7 +280,7 @@ func (dprh *devicePluginReconcilerHelper) moduleUpdateDevicePluginStatus(ctx con
274280
}
275281

276282
// get the number of nodes targeted by selector (which also relevant for device plugin)
277-
numTargetedNodes, err := dprh.getNumTargetedNodes(ctx, mod.Spec.Selector)
283+
numTargetedNodes, err := dprh.nodeAPI.GetNumTargetedNodes(ctx, mod.Spec.Selector)
278284
if err != nil {
279285
return fmt.Errorf("failed to determine the number of nodes that should be targeted by Module's %s/%s selector: %v", mod.Namespace, mod.Name, err)
280286
}
@@ -295,26 +301,6 @@ func (dprh *devicePluginReconcilerHelper) moduleUpdateDevicePluginStatus(ctx con
295301
return dprh.client.Status().Patch(ctx, mod, client.MergeFrom(unmodifiedMod))
296302
}
297303

298-
func (dprh *devicePluginReconcilerHelper) getNumTargetedNodes(ctx context.Context, selectorLabels map[string]string) (int, error) {
299-
logger := log.FromContext(ctx)
300-
logger.V(1).Info("Listing nodes", "selector", selectorLabels)
301-
302-
selectedNodes := v1.NodeList{}
303-
opt := client.MatchingLabels(selectorLabels)
304-
if err := dprh.client.List(ctx, &selectedNodes, opt); err != nil {
305-
return 0, fmt.Errorf("could not list nodes: %v", err)
306-
}
307-
308-
numNodes := 0
309-
310-
for _, node := range selectedNodes.Items {
311-
if utils.IsNodeSchedulable(&node) {
312-
numNodes += 1
313-
}
314-
}
315-
return numNodes, nil
316-
}
317-
318304
//go:generate mockgen -source=device_plugin_reconciler.go -package=controllers -destination=mock_device_plugin_reconciler.go daemonSetCreator
319305

320306
type daemonSetCreator interface {

0 commit comments

Comments
 (0)