Skip to content

Commit db8b713

Browse files
TomerNewmanTomerNewman
committed
Adding support for Modules with no OOT kmod
When KMM is used by 3rd party vendor operators, they need an option to configure KMM Module not to load OOT kernel driver, but use the in-tree one, and just run the device-plugin. This commit makes moduleLoader in Module cr a pointer, hence making it an optional field.
1 parent 6131e7d commit db8b713

14 files changed

+168
-64
lines changed

‎api/v1beta1/module_types.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -319,7 +319,8 @@ type ModuleSpec struct {
319319

320320
// ModuleLoader allows overriding some properties of the container that loads the kernel module on the node.
321321
// Name and image are ignored and are set automatically by the KMM Operator.
322-
ModuleLoader ModuleLoaderSpec `json:"moduleLoader"`
322+
// +optional
323+
ModuleLoader *ModuleLoaderSpec `json:"moduleLoader,omitempty"`
323324

324325
// ImageRepoSecret is an optional secret that is used to pull both the module loader and the device plugin, and
325326
// to push the resulting image from the module loader build, if enabled.
@@ -351,7 +352,7 @@ type ModuleStatus struct {
351352
// if it was deployed during reconciliation
352353
DevicePlugin DaemonSetStatus `json:"devicePlugin,omitempty"`
353354
// ModuleLoader contains the status of the ModuleLoader daemonset
354-
ModuleLoader DaemonSetStatus `json:"moduleLoader"`
355+
ModuleLoader DaemonSetStatus `json:"moduleLoader,omitempty"`
355356
}
356357

357358
//+kubebuilder:object:root=true

‎api/v1beta1/zz_generated.deepcopy.go

Lines changed: 5 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎config/crd-hub/bases/hub.kmm.sigs.x-k8s.io_managedclustermodules.yaml

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2728,7 +2728,6 @@ spec:
27282728
type: object
27292729
type: array
27302730
required:
2731-
- moduleLoader
27322731
- selector
27332732
type: object
27342733
selector:

‎config/crd/bases/kmm.sigs.x-k8s.io_modules.yaml

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2704,7 +2704,6 @@ spec:
27042704
type: object
27052705
type: array
27062706
required:
2707-
- moduleLoader
27082707
- selector
27092708
type: object
27102709
status:
@@ -2745,8 +2744,6 @@ spec:
27452744
format: int32
27462745
type: integer
27472746
type: object
2748-
required:
2749-
- moduleLoader
27502747
type: object
27512748
type: object
27522749
served: true

‎internal/controllers/device_plugin_reconciler.go

Lines changed: 49 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -182,9 +182,10 @@ func (dprh *devicePluginReconcilerHelper) handleDevicePlugin(ctx context.Context
182182
}
183183

184184
logger := log.FromContext(ctx)
185-
ds := getExistingDS(existingDevicePluginDS, mod.Namespace, mod.Name, mod.Spec.ModuleLoader.Container.Version)
185+
186+
ds, version := getExistingDSFromVersion(existingDevicePluginDS, mod.Namespace, mod.Name, mod.Spec.ModuleLoader)
186187
if ds == nil {
187-
logger.Info("creating new device plugin DS", "version", mod.Spec.ModuleLoader.Container.Version)
188+
logger.Info("creating new device plugin DS", "version", version)
188189
ds = &appsv1.DaemonSet{
189190
ObjectMeta: metav1.ObjectMeta{Namespace: mod.Namespace, GenerateName: mod.Name + "-device-plugin-"},
190191
}
@@ -203,6 +204,11 @@ func (dprh *devicePluginReconcilerHelper) handleDevicePlugin(ctx context.Context
203204
func (dprh *devicePluginReconcilerHelper) garbageCollect(ctx context.Context,
204205
mod *kmmv1beta1.Module,
205206
existingDS []appsv1.DaemonSet) error {
207+
if mod.Spec.ModuleLoader == nil {
208+
// If ModuleLoader is not exist, Ordered Upgrade is not relevant
209+
// so there is no need to delete older-versioned Deamonsets
210+
return nil
211+
}
206212

207213
logger := log.FromContext(ctx)
208214
deleted := make([]string, 0)
@@ -248,21 +254,22 @@ func (dprh *devicePluginReconcilerHelper) setKMMOMetrics(ctx context.Context) {
248254
if mod.Spec.DevicePlugin != nil {
249255
numModulesWithDevicePlugin += 1
250256
}
251-
buildCapable, signCapable := isModuleBuildAndSignCapable(&mod)
252-
if buildCapable {
253-
numModulesWithBuild += 1
254-
}
255-
if signCapable {
256-
numModulesWithSign += 1
257-
}
258-
259-
if mod.Spec.ModuleLoader.Container.Modprobe.Args != nil {
260-
modprobeArgs := strings.Join(mod.Spec.ModuleLoader.Container.Modprobe.Args.Load, ",")
261-
dprh.metricsAPI.SetKMMModprobeArgs(mod.Name, mod.Namespace, modprobeArgs)
262-
}
263-
if mod.Spec.ModuleLoader.Container.Modprobe.RawArgs != nil {
264-
modprobeRawArgs := strings.Join(mod.Spec.ModuleLoader.Container.Modprobe.RawArgs.Load, ",")
265-
dprh.metricsAPI.SetKMMModprobeRawArgs(mod.Name, mod.Namespace, modprobeRawArgs)
257+
if mod.Spec.ModuleLoader != nil {
258+
buildCapable, signCapable := isModuleBuildAndSignCapable(&mod)
259+
if buildCapable {
260+
numModulesWithBuild += 1
261+
}
262+
if signCapable {
263+
numModulesWithSign += 1
264+
}
265+
if mod.Spec.ModuleLoader.Container.Modprobe.Args != nil {
266+
modprobeArgs := strings.Join(mod.Spec.ModuleLoader.Container.Modprobe.Args.Load, ",")
267+
dprh.metricsAPI.SetKMMModprobeArgs(mod.Name, mod.Namespace, modprobeArgs)
268+
}
269+
if mod.Spec.ModuleLoader.Container.Modprobe.RawArgs != nil {
270+
modprobeRawArgs := strings.Join(mod.Spec.ModuleLoader.Container.Modprobe.RawArgs.Load, ",")
271+
dprh.metricsAPI.SetKMMModprobeRawArgs(mod.Name, mod.Namespace, modprobeRawArgs)
272+
}
266273
}
267274
}
268275
dprh.metricsAPI.SetKMMModulesNum(numModules)
@@ -348,14 +355,8 @@ func (dsci *daemonSetCreatorImpl) setDevicePluginAsDesired(
348355
},
349356
},
350357
}
351-
standardLabels := map[string]string{constants.ModuleNameLabel: mod.Name}
352-
nodeSelector := map[string]string{utils.GetKernelModuleReadyNodeLabel(mod.Namespace, mod.Name): ""}
353358

354-
if mod.Spec.ModuleLoader.Container.Version != "" {
355-
versionLabel := utils.GetDevicePluginVersionLabelName(mod.Namespace, mod.Name)
356-
standardLabels[versionLabel] = mod.Spec.ModuleLoader.Container.Version
357-
nodeSelector[versionLabel] = mod.Spec.ModuleLoader.Container.Version
358-
}
359+
standardLabels, nodeSelector := generateDevicePluginLabelsAndSelector(mod)
359360

360361
ds.SetLabels(
361362
overrideLabels(ds.GetLabels(), standardLabels),
@@ -395,6 +396,21 @@ func (dsci *daemonSetCreatorImpl) setDevicePluginAsDesired(
395396
return controllerutil.SetControllerReference(mod, ds, dsci.scheme)
396397
}
397398

399+
func generateDevicePluginLabelsAndSelector(mod *kmmv1beta1.Module) (map[string]string, map[string]string) {
400+
labels := map[string]string{constants.ModuleNameLabel: mod.Name}
401+
nodeSelector := map[string]string{utils.GetKernelModuleReadyNodeLabel(mod.Namespace, mod.Name): ""}
402+
403+
if mod.Spec.ModuleLoader != nil && mod.Spec.ModuleLoader.Container.Version != "" {
404+
versionLabel := utils.GetDevicePluginVersionLabelName(mod.Namespace, mod.Name)
405+
labels[versionLabel] = mod.Spec.ModuleLoader.Container.Version
406+
nodeSelector[versionLabel] = mod.Spec.ModuleLoader.Container.Version
407+
} else if mod.Spec.ModuleLoader == nil {
408+
nodeSelector = mod.Spec.Selector
409+
}
410+
411+
return labels, nodeSelector
412+
}
413+
398414
func isModuleBuildAndSignCapable(mod *kmmv1beta1.Module) (bool, bool) {
399415
buildCapable := mod.Spec.ModuleLoader.Container.Build != nil
400416
signCapable := mod.Spec.ModuleLoader.Container.Sign != nil
@@ -412,19 +428,23 @@ func isModuleBuildAndSignCapable(mod *kmmv1beta1.Module) (bool, bool) {
412428
return buildCapable, signCapable
413429
}
414430

415-
func getExistingDS(existingDS []appsv1.DaemonSet,
431+
func getExistingDSFromVersion(existingDS []appsv1.DaemonSet,
416432
moduleNamespace string,
417433
moduleName string,
418-
moduleVersion string) *appsv1.DaemonSet {
434+
moduleLoader *kmmv1beta1.ModuleLoaderSpec) (*appsv1.DaemonSet, string) {
435+
version := ""
436+
if moduleLoader != nil {
437+
version = moduleLoader.Container.Version
438+
}
419439

420440
versionLabel := utils.GetDevicePluginVersionLabelName(moduleNamespace, moduleName)
421441
for _, ds := range existingDS {
422442
dsModuleVersion := ds.GetLabels()[versionLabel]
423-
if dsModuleVersion == moduleVersion {
424-
return &ds
443+
if dsModuleVersion == version {
444+
return &ds, version
425445
}
426446
}
427-
return nil
447+
return nil, version
428448
}
429449

430450
func isOlderVersionUnusedDevicePluginDaemonset(ds *appsv1.DaemonSet, moduleVersion string) bool {

‎internal/controllers/device_plugin_reconciler_test.go

Lines changed: 57 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -240,7 +240,7 @@ var _ = Describe("DevicePluginReconciler_garbageCollect", func() {
240240
Namespace: "namespace",
241241
},
242242
Spec: kmmv1beta1.ModuleSpec{
243-
ModuleLoader: kmmv1beta1.ModuleLoaderSpec{
243+
ModuleLoader: &kmmv1beta1.ModuleLoaderSpec{
244244
Container: kmmv1beta1.ModuleLoaderContainerSpec{
245245
Version: currentModuleVersion,
246246
},
@@ -298,6 +298,23 @@ var _ = Describe("DevicePluginReconciler_garbageCollect", func() {
298298
err := dprh.garbageCollect(context.Background(), mod, existingDS)
299299
Expect(err).To(HaveOccurred())
300300
})
301+
302+
It("should pass if moduleLoader is not defined", func() {
303+
modWithoutModuleLoader := mod
304+
modWithoutModuleLoader.Spec.ModuleLoader = nil
305+
deleteDS := appsv1.DaemonSet{
306+
ObjectMeta: metav1.ObjectMeta{
307+
Name: "devicePlugin",
308+
Namespace: "namespace",
309+
Labels: map[string]string{constants.ModuleNameLabel: mod.Name, devicePluginVersionLabel: "formerVersion"},
310+
},
311+
}
312+
313+
existingDS := []appsv1.DaemonSet{deleteDS}
314+
315+
err := dprh.garbageCollect(context.Background(), modWithoutModuleLoader, existingDS)
316+
Expect(err).ToNot(HaveOccurred())
317+
})
301318
})
302319

303320
var _ = Describe("DevicePluginReconciler_handleModuleDeletion", func() {
@@ -365,9 +382,20 @@ var _ = Describe("DevicePluginReconciler_setKMMOMetrics", func() {
365382
Name: "moduleName",
366383
Namespace: "namespace",
367384
},
385+
Spec: kmmv1beta1.ModuleSpec{
386+
ModuleLoader: &kmmv1beta1.ModuleLoaderSpec{},
387+
},
388+
}
389+
mod2 := kmmv1beta1.Module{
390+
Spec: kmmv1beta1.ModuleSpec{
391+
ModuleLoader: &kmmv1beta1.ModuleLoaderSpec{},
392+
},
393+
}
394+
mod3 := kmmv1beta1.Module{
395+
Spec: kmmv1beta1.ModuleSpec{
396+
ModuleLoader: &kmmv1beta1.ModuleLoaderSpec{},
397+
},
368398
}
369-
mod2 := kmmv1beta1.Module{}
370-
mod3 := kmmv1beta1.Module{}
371399
numBuild := 0
372400
numSign := 0
373401
numDevicePlugin := 0
@@ -453,7 +481,11 @@ var _ = Describe("DevicePluginReconciler_moduleUpdateDevicePluginStatus", func()
453481
ctx := context.Background()
454482

455483
It("device plugin not defined in the module", func() {
456-
mod := kmmv1beta1.Module{}
484+
mod := kmmv1beta1.Module{
485+
Spec: kmmv1beta1.ModuleSpec{
486+
ModuleLoader: &kmmv1beta1.ModuleLoaderSpec{},
487+
},
488+
}
457489
err := dprh.moduleUpdateDevicePluginStatus(ctx, &mod, nil)
458490
Expect(err).NotTo(HaveOccurred())
459491
})
@@ -527,8 +559,13 @@ var _ = Describe("DevicePluginReconciler_setDevicePluginAsDesired", func() {
527559

528560
It("should return an error if DevicePlugin not set in the Spec", func() {
529561
ds := appsv1.DaemonSet{}
562+
mod := kmmv1beta1.Module{
563+
Spec: kmmv1beta1.ModuleSpec{
564+
ModuleLoader: &kmmv1beta1.ModuleLoaderSpec{},
565+
},
566+
}
530567
Expect(
531-
dsc.setDevicePluginAsDesired(context.Background(), &ds, &kmmv1beta1.Module{}),
568+
dsc.setDevicePluginAsDesired(context.Background(), &ds, &mod),
532569
).To(
533570
HaveOccurred(),
534571
)
@@ -563,7 +600,7 @@ var _ = Describe("DevicePluginReconciler_setDevicePluginAsDesired", func() {
563600
Namespace: namespace,
564601
},
565602
Spec: kmmv1beta1.ModuleSpec{
566-
ModuleLoader: kmmv1beta1.ModuleLoaderSpec{
603+
ModuleLoader: &kmmv1beta1.ModuleLoaderSpec{
567604
Container: kmmv1beta1.ModuleLoaderContainerSpec{
568605
Version: "some version",
569606
},
@@ -589,7 +626,7 @@ var _ = Describe("DevicePluginReconciler_setDevicePluginAsDesired", func() {
589626
Expect(ds.GetLabels()).Should(HaveKeyWithValue(versionLabel, "some version"))
590627
})
591628

592-
It("should work as expected", func() {
629+
DescribeTable("should work as expected", func(moduleLoader *kmmv1beta1.ModuleLoaderSpec, expectedNodeSelector map[string]string) {
593630
const (
594631
dsName = "ds-name"
595632
serviceAccountName = "some-service-account"
@@ -646,6 +683,7 @@ var _ = Describe("DevicePluginReconciler_setDevicePluginAsDesired", func() {
646683
Namespace: namespace,
647684
},
648685
Spec: kmmv1beta1.ModuleSpec{
686+
ModuleLoader: moduleLoader,
649687
DevicePlugin: &kmmv1beta1.DevicePluginSpec{
650688
Container: kmmv1beta1.DevicePluginContainerSpec{
651689
Args: args,
@@ -722,10 +760,8 @@ var _ = Describe("DevicePluginReconciler_setDevicePluginAsDesired", func() {
722760
},
723761
},
724762
},
725-
ImagePullSecrets: []v1.LocalObjectReference{repoSecret},
726-
NodeSelector: map[string]string{
727-
utils.GetKernelModuleReadyNodeLabel(mod.Namespace, mod.Name): "",
728-
},
763+
ImagePullSecrets: []v1.LocalObjectReference{repoSecret},
764+
NodeSelector: expectedNodeSelector,
729765
PriorityClassName: "system-node-critical",
730766
ServiceAccountName: serviceAccountName,
731767
Volumes: []v1.Volume{
@@ -750,7 +786,16 @@ var _ = Describe("DevicePluginReconciler_setDevicePluginAsDesired", func() {
750786
).To(
751787
BeTrue(), cmp.Diff(expected, ds),
752788
)
753-
})
789+
},
790+
Entry("moduleLoader is nil",
791+
nil,
792+
map[string]string{"has-feature-x": "true"},
793+
),
794+
Entry("moduleLoader is defined",
795+
&kmmv1beta1.ModuleLoaderSpec{},
796+
map[string]string{utils.GetKernelModuleReadyNodeLabel(namespace, moduleName): ""},
797+
),
798+
)
754799
})
755800

756801
var _ = Describe("DevicePluginReconciler_getExistingDS", func() {

‎internal/controllers/hub/managedclustermodule_reconciler_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -272,7 +272,7 @@ var _ = Describe("ManagedClusterModuleReconciler_Reconcile", func() {
272272
},
273273
Spec: v1beta1.ManagedClusterModuleSpec{
274274
ModuleSpec: kmmv1beta1.ModuleSpec{
275-
ModuleLoader: kmmv1beta1.ModuleLoaderSpec{
275+
ModuleLoader: &kmmv1beta1.ModuleLoaderSpec{
276276
Container: kmmv1beta1.ModuleLoaderContainerSpec{
277277
Build: &kmmv1beta1.Build{},
278278
},

‎internal/controllers/module_reconciler.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,11 @@ func (mr *ModuleReconciler) Reconcile(ctx context.Context, mod *kmmv1beta1.Modul
100100
return ctrl.Result{}, fmt.Errorf("failed to set finalizer on Module %s/%s: %v", mod.Namespace, mod.Name, err)
101101
}
102102

103+
if mod.Spec.ModuleLoader == nil {
104+
logger.Info("ModuleLoader is not defined, skipping loading kernel module")
105+
return ctrl.Result{}, nil
106+
}
107+
103108
// get nodes targeted by selector
104109
targetedNodes, err := mr.nodeAPI.GetNodesListBySelector(ctx, mod.Spec.Selector, mod.Spec.Tolerations)
105110
if err != nil {

‎internal/controllers/module_reconciler_test.go

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,9 @@ var _ = Describe("ModuleReconciler_Reconcile", func() {
5151

5252
mod = &kmmv1beta1.Module{
5353
ObjectMeta: metav1.ObjectMeta{Name: moduleName, Namespace: namespace},
54+
Spec: kmmv1beta1.ModuleSpec{
55+
ModuleLoader: &kmmv1beta1.ModuleLoaderSpec{},
56+
},
5457
}
5558

5659
mr = &ModuleReconciler{
@@ -212,6 +215,20 @@ var _ = Describe("ModuleReconciler_Reconcile", func() {
212215
Expect(res).To(Equal(reconcile.Result{}))
213216
Expect(err).NotTo(HaveOccurred())
214217
})
218+
219+
It("Good flow, should not load kernel module when moduleLoader is missing", func() {
220+
modWithoutModuleLoader := mod
221+
modWithoutModuleLoader.Spec.ModuleLoader = nil
222+
gomock.InOrder(
223+
mockNamespaceHelper.EXPECT().setLabel(ctx, mod.Namespace),
224+
mockReconHelper.EXPECT().setFinalizerAndStatus(ctx, mod).Return(nil),
225+
)
226+
227+
res, err := mr.Reconcile(ctx, modWithoutModuleLoader)
228+
229+
Expect(res).To(Equal(reconcile.Result{}))
230+
Expect(err).NotTo(HaveOccurred())
231+
})
215232
})
216233

217234
var _ = Describe("setFinalizerAndStatus", func() {

‎internal/manifestwork/manifestwork_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -184,7 +184,7 @@ var _ = Describe("SetManifestWorkAsDesired", func() {
184184
Spec: hubv1beta1.ManagedClusterModuleSpec{
185185
SpokeNamespace: spokeNamespace,
186186
ModuleSpec: kmmv1beta1.ModuleSpec{
187-
ModuleLoader: kmmv1beta1.ModuleLoaderSpec{
187+
ModuleLoader: &kmmv1beta1.ModuleLoaderSpec{
188188
Container: kmmv1beta1.ModuleLoaderContainerSpec{
189189
Build: &kmmv1beta1.Build{},
190190
Sign: &kmmv1beta1.Sign{},
@@ -247,7 +247,7 @@ var _ = Describe("SetManifestWorkAsDesired", func() {
247247
}
248248

249249
expectedModuleSpec := kmmv1beta1.ModuleSpec{
250-
ModuleLoader: kmmv1beta1.ModuleLoaderSpec{
250+
ModuleLoader: &kmmv1beta1.ModuleLoaderSpec{
251251
Container: kmmv1beta1.ModuleLoaderContainerSpec{
252252
Build: nil,
253253
Sign: nil,

0 commit comments

Comments
 (0)