Adding support for Modules with no OOT kmod #1084

TomerNewman · 2025-04-23T14:38:31Z

When KMM is used by 3rd party vendor operators, they need an option to configure KMM Module to not 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, and so making it an optional field.

/cc @ybettan @yevgeny-shnaidman

k8s-ci-robot · 2025-04-23T14:38:41Z

Hi @TomerNewman. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

netlify · 2025-04-23T14:38:50Z

✅ Deploy Preview for kubernetes-sigs-kmm ready!

Name	Link
🔨 Latest commit	`5025177`
🔍 Latest deploy log	https://app.netlify.com/sites/kubernetes-sigs-kmm/deploys/680f651edcb1e40008f96853
😎 Deploy Preview	https://deploy-preview-1084--kubernetes-sigs-kmm.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

ybettan · 2025-04-28T06:29:47Z

I would update the PR&commit titles as it is not accurate. Maybe something like "Adding support for Modules with no OOT kmod".

ybettan · 2025-04-28T06:31:53Z

/ok-to-test

codecov-commenter · 2025-04-28T06:33:23Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 75.03%. Comparing base (fa23a9b) to head (5025177).
Report is 272 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1084      +/-   ##
==========================================
- Coverage   79.09%   75.03%   -4.06%     
==========================================
  Files          51       76      +25     
  Lines        5109     6927    +1818     
==========================================
+ Hits         4041     5198    +1157     
- Misses        882     1515     +633     
- Partials      186      214      +28

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

ybettan · 2025-04-28T06:44:27Z

/approve

We can merge once @yevgeny-shnaidman is happy with the change as well.

k8s-ci-robot · 2025-04-28T06:44:33Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: TomerNewman, ybettan

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [ybettan]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

yevgeny-shnaidman · 2025-04-28T06:48:54Z

internal/controllers/device_plugin_reconciler.go

@@ -203,6 +208,9 @@ func (dprh *devicePluginReconcilerHelper) handleDevicePlugin(ctx context.Context
 func (dprh *devicePluginReconcilerHelper) garbageCollect(ctx context.Context,
 	mod *kmmv1beta1.Module,
 	existingDS []appsv1.DaemonSet) error {
+	if mod.Spec.ModuleLoader == nil {


lets add a comment here that explains why we don't need to go through garbage collect flow in case module loader does not exists

yevgeny-shnaidman · 2025-04-28T06:49:45Z

internal/controllers/device_plugin_reconciler.go

@@ -182,9 +182,14 @@ func (dprh *devicePluginReconcilerHelper) handleDevicePlugin(ctx context.Context
 	}

 	logger := log.FromContext(ctx)
-	ds := getExistingDS(existingDevicePluginDS, mod.Namespace, mod.Name, mod.Spec.ModuleLoader.Container.Version)
+	version := ""


WDYT about moving this logic into the getExistingDS function? It will receive the Moduleloader pointer instead of the version itself

@yevgeny-shnaidman I can do but either way I have to do something like

version := "" if mod.Spec.ModuleLoader != nil { version = mod.Spec.ModuleLoader.Container.Version }

for the log line down below in order to not get a nil pointer reference error
logger.Info("creating new device plugin DS", "version", version)

yevgeny-shnaidman · 2025-04-28T06:54:03Z

internal/controllers/device_plugin_reconciler.go

-		if mod.Spec.ModuleLoader.Container.Modprobe.RawArgs != nil {
-			modprobeRawArgs := strings.Join(mod.Spec.ModuleLoader.Container.Modprobe.RawArgs.Load, ",")
-			dprh.metricsAPI.SetKMMModprobeRawArgs(mod.Name, mod.Namespace, modprobeRawArgs)
+		if mod.Spec.ModuleLoader != nil {


WDYT about moving the "if mod.Spec.ModuleLoader != nil {" above line 259? numModulesWithBuild and numModulesWithSign need to be calculated only in case ModuleLoader is not nil. And, in addition, we want have to change the isModuleBuildAndSignCapable function, since the pointer was verified prior to calling that function

@yevgeny-shnaidman isModuleBuildAndSignCapable returns false, false whenever ModuleLoader is nil automatically though

yevgeny-shnaidman · 2025-04-28T07:00:36Z

internal/controllers/device_plugin_reconciler.go

@@ -351,10 +361,12 @@ func (dsci *daemonSetCreatorImpl) setDevicePluginAsDesired(
 	standardLabels := map[string]string{constants.ModuleNameLabel: mod.Name}
 	nodeSelector := map[string]string{utils.GetKernelModuleReadyNodeLabel(mod.Namespace, mod.Name): ""}

-	if mod.Spec.ModuleLoader.Container.Version != "" {
+	if mod.Spec.ModuleLoader != nil && mod.Spec.ModuleLoader.Container.Version != "" {


determining the labels has not become a little bit more complicated. WDYT about moving it to a dedicated function?

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.

yevgeny-shnaidman · 2025-05-05T06:32:17Z

/lgtm

k8s-ci-robot requested review from ybettan and yevgeny-shnaidman April 23, 2025 14:38

k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Apr 23, 2025

k8s-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Apr 23, 2025

k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Apr 28, 2025

TomerNewman changed the title ~~Allowing kmm to accept Modules with loading an actual kmod~~ Adding support for Modules with no OOT kmod Apr 28, 2025

TomerNewman force-pushed the feature/kmm-skip-load-kmod branch from bfb5208 to a586e51 Compare April 28, 2025 06:39

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Apr 28, 2025

yevgeny-shnaidman reviewed Apr 28, 2025

View reviewed changes

TomerNewman force-pushed the feature/kmm-skip-load-kmod branch 2 times, most recently from 4a2fa3e to db8b713 Compare April 28, 2025 10:51

TomerNewman force-pushed the feature/kmm-skip-load-kmod branch from db8b713 to 5025177 Compare April 28, 2025 11:23

k8s-ci-robot assigned yevgeny-shnaidman May 5, 2025

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 5, 2025

k8s-ci-robot merged commit 3613be4 into kubernetes-sigs:main May 5, 2025
22 checks passed

TomerNewman mentioned this pull request May 12, 2025

documentation for kmm supporting also intree kmod #1100

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adding support for Modules with no OOT kmod #1084

Adding support for Modules with no OOT kmod #1084

TomerNewman commented Apr 23, 2025

k8s-ci-robot commented Apr 23, 2025

netlify bot commented Apr 23, 2025 •

edited

Loading

ybettan commented Apr 28, 2025

ybettan commented Apr 28, 2025

codecov-commenter commented Apr 28, 2025 •

edited

Loading

ybettan commented Apr 28, 2025

k8s-ci-robot commented Apr 28, 2025

yevgeny-shnaidman Apr 28, 2025

yevgeny-shnaidman Apr 28, 2025

TomerNewman Apr 28, 2025

TomerNewman Apr 28, 2025

yevgeny-shnaidman Apr 28, 2025

TomerNewman Apr 28, 2025

TomerNewman Apr 28, 2025

yevgeny-shnaidman Apr 28, 2025

yevgeny-shnaidman commented May 5, 2025

Adding support for Modules with no OOT kmod #1084

Adding support for Modules with no OOT kmod #1084

Conversation

TomerNewman commented Apr 23, 2025

k8s-ci-robot commented Apr 23, 2025

netlify bot commented Apr 23, 2025 • edited Loading

✅ Deploy Preview for kubernetes-sigs-kmm ready!

ybettan commented Apr 28, 2025

ybettan commented Apr 28, 2025

codecov-commenter commented Apr 28, 2025 • edited Loading

Codecov Report

ybettan commented Apr 28, 2025

k8s-ci-robot commented Apr 28, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yevgeny-shnaidman commented May 5, 2025

netlify bot commented Apr 23, 2025 •

edited

Loading

codecov-commenter commented Apr 28, 2025 •

edited

Loading