Automatically load ipvs required kernel modules in kubeadm #59566

stewart-yu · 2018-02-08T12:09:10Z

What this PR does / why we need it:
This PR is part of #59402, aiming to load kernel modules in kubeadm

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes ##59402

Special notes for your reviewer:

Release note:

NONE

stewart-yu · 2018-02-08T12:09:45Z

/assign @m1093782566 @Lion-Wei

m1093782566 · 2018-02-08T12:11:27Z

/ok-to-test

rramkumar1 · 2018-02-08T18:35:57Z

In this PR, I would not delete the code in the ipvs proxier that loads the modules. I suggest you leave that as the final step once we know the module loading logic is propagated everywhere

rramkumar1 · 2018-02-08T18:37:21Z

cmd/kubeadm/app/util/loadkernelmodule.go

@@ -0,0 +1,51 @@
+package util


Why are we doing this in code? I thought the whole point was to move the kernel module loading logic into startup scripts. Doesn't kubeadm have startup scripts as well?

just WIP. Will fix for moment, thanks

Actuall, kubeadm itself don't have startup scripts. You can do this work in kubernetes-anywhere, but that's just one use case of kubeadm.

Lion-Wei · 2018-02-09T02:40:32Z

pkg/proxy/ipvs/proxier.go

-	loadModules := sets.NewString()
-	wantModules.Insert(ipvsModules...)
-	loadModules.Insert(mods...)
-	modules := wantModules.Difference(loadModules).UnsortedList()


I think we still need check whether kernel modules are loaded in kube-proxy, right?

i keep it now, will deleting it once all function well in kubeadm.

stewart-yu · 2018-02-09T09:34:43Z

/test pull-kubernetes-bazel-test

dixudx · 2018-02-13T03:33:11Z

/area kubeadm

fabriziopandini · 2018-03-02T16:11:48Z

/uncc @fabriziopandini
No experiences on IPVS ☹️

Might be a silly question, but what about nodes? Afaik this piece of code will be executed only on masters...

stewart-yu · 2018-03-27T07:57:24Z

Kindly ping @luxas @xiangpengzhao for review,PTAL

Lion-Wei · 2018-03-27T08:00:45Z

@fabriziopandini That's true, I think this pr still WIP. Need figure out a good way to identify ipvs mode in nodes. @stewart-yu

m1093782566 · 2018-03-27T08:02:51Z

/assign @luxas

luxas · 2018-05-21T13:23:57Z

cmd/kubeadm/app/preflight/checks.go

@@ -926,6 +934,11 @@ func RunJoinNodeChecks(execer utilsexec.Interface, cfg *kubeadmapi.NodeConfigura
 	checks = addCommonChecks(execer, cfg, checks)

 	addIPv6Checks := false
+
+	checks = append(checks,
+		ipvsutil.RequiredIPVSKernelModulesAvailableCheck{},


please move this to the []Checker slice above

luxas · 2018-05-21T13:25:19Z

pkg/util/ipvs/kernelcheck_linux.go

+}
+
+// Name returns label for RequiredIPVSKernelModulesAvailableCheck
+func (r RequiredIPVSKernelModulesAvailableCheck) Name() string {


Shouldn't you pass this as a reference here in both funcs? I think that's standard practice at least

In general, that's right. But if add reference, will generate incompatible types errors. Also follow others checkers's format.

luxas · 2018-05-21T13:25:59Z

pkg/util/ipvs/kernelcheck_linux_test.go

+	loadModules.Insert(mods...)
+	modules := wantModules.Difference(loadModules).UnsortedList()
+
+	//TODO: add more testcase about builtin kernel ipvs support


Can you add more now or do you strictly want to do that later?

luxas · 2018-05-21T13:27:10Z

pkg/util/ipvs/kernelcheck_unsupported.go

+
+// Check try to validates IPVS required kernel modules exists or not.
+func (r RequiredIPVSKernelModulesAvailableCheck) Check() (warnings, errors []error) {
+	errors = append(errors, fmt.Errorf("IPVS not supported for this platform"))


I don't think you should return an error here, as ipvs isn't required for kubeadm to run. This will work just fine on windows for example, even though those kernel modules aren't there

stewart-yu · 2018-05-22T02:53:10Z

/cc @m1093782566 @luxas all comments updated, PTAL
Test coverage in local Env is 78.8%

m1093782566 · 2018-05-22T03:07:32Z

pkg/util/ipvs/kernelcheck_unsupported.go

+
+// Check try to validates IPVS required kernel modules exists or not.
+func (r RequiredIPVSKernelModulesAvailableCheck) Check() (warnings, errors []error) {
+	warnings = []error{}


nit: remove this line and return nil, nil directly.

m1093782566 · 2018-05-22T03:23:32Z

Looks good to me for IPVS part. @luxas for kubeadm part opinions and final approval.

stewart-yu · 2018-05-22T05:33:54Z

/test pull-kubernetes-e2e-kops-aws

luxas

/approve
/lgtm
Thanks @stewart-yu 🎉!

luxas · 2018-05-22T05:36:41Z

@m1093782566 please approve the ipvs parts

m1093782566 · 2018-05-22T05:57:53Z

Okay, let's merge this PR.

/approve
/lgtm

k8s-ci-robot · 2018-05-22T05:58:17Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: luxas, m1093782566, stewart-yu, timothysc

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:

~~cmd/kubeadm/OWNERS~~ [luxas,timothysc]
~~pkg/util/ipvs/OWNERS~~ [m1093782566]

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

k8s-github-robot · 2018-05-22T08:19:33Z

/test all [submit-queue is verifying that this PR is safe to merge]

m1093782566 · 2018-05-22T09:14:49Z

/retest

k8s-github-robot · 2018-05-22T09:52:16Z

Automatic merge from submit-queue. If you want to cherry-pick this change to another branch, please follow the instructions here.

k8s-ci-robot added release-note-none size/M cncf-cla: yes needs-ok-to-test labels Feb 8, 2018

k8s-ci-robot requested review from fabriziopandini and thockin February 8, 2018 12:09

k8s-ci-robot removed the needs-ok-to-test label Feb 8, 2018

k8s-ci-robot assigned m1093782566 Feb 8, 2018

rramkumar1 reviewed Feb 8, 2018

View reviewed changes

Lion-Wei reviewed Feb 9, 2018

View reviewed changes

stewart-yu force-pushed the ipvsGAkubeadm branch 3 times, most recently from 0c6c28c to 032f620 Compare February 9, 2018 07:34

stewart-yu force-pushed the ipvsGAkubeadm branch 2 times, most recently from 7a2233c to cb4785c Compare February 9, 2018 10:15

k8s-ci-robot added the area/kubeadm label Feb 13, 2018

k8s-ci-robot added the needs-rebase label Feb 26, 2018

stewart-yu force-pushed the ipvsGAkubeadm branch from cb4785c to be3860b Compare February 27, 2018 02:52

k8s-ci-robot removed the needs-rebase label Feb 27, 2018

k8s-ci-robot removed the request for review from fabriziopandini March 2, 2018 16:11

k8s-ci-robot assigned luxas Mar 27, 2018

stewart-yu force-pushed the ipvsGAkubeadm branch 6 times, most recently from 9bed6ae to adef916 Compare May 21, 2018 12:13

luxas reviewed May 21, 2018

View reviewed changes

timothysc assigned luxas May 21, 2018

stewart-yu force-pushed the ipvsGAkubeadm branch from adef916 to 07874c8 Compare May 22, 2018 02:13

m1093782566 reviewed May 22, 2018

View reviewed changes

stewart-yu added 2 commits May 22, 2018 11:20

load kernel modules required by IPVS in kubeadm

2f7941d

auto generated file

5f4025c

stewart-yu force-pushed the ipvsGAkubeadm branch from 07874c8 to 5f4025c Compare May 22, 2018 03:47

k8s-ci-robot added the lgtm label May 22, 2018

luxas approved these changes May 22, 2018

View reviewed changes

k8s-ci-robot added the approved label May 22, 2018

k8s-github-robot merged commit e85b81b into kubernetes:master May 22, 2018

stewart-yu deleted the ipvsGAkubeadm branch May 22, 2018 09:54

neolit123 mentioned this pull request Jul 6, 2018

RequiredIPVSKernelModulesAvailable warning seems confusing kubernetes/kubeadm#975

Closed

stewart-yu mentioned this pull request Nov 13, 2018

kubeadm: fix RequiredIPVSKernelModulesAvailableCheck #70051

Closed

cmluciano mentioned this pull request Sep 10, 2020

ipvs: ensure selected scheduler kernel modules are loaded #93040

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Automatically load ipvs required kernel modules in kubeadm #59566

Automatically load ipvs required kernel modules in kubeadm #59566

stewart-yu commented Feb 8, 2018

stewart-yu commented Feb 8, 2018

m1093782566 commented Feb 8, 2018

rramkumar1 commented Feb 8, 2018

rramkumar1 Feb 8, 2018

stewart-yu Feb 9, 2018

Lion-Wei Feb 9, 2018

Lion-Wei Feb 9, 2018

stewart-yu Feb 9, 2018

stewart-yu commented Feb 9, 2018

dixudx commented Feb 13, 2018

fabriziopandini commented Mar 2, 2018

stewart-yu commented Mar 27, 2018

Lion-Wei commented Mar 27, 2018

m1093782566 commented Mar 27, 2018

luxas May 21, 2018

luxas May 21, 2018

stewart-yu May 22, 2018 •

edited

Loading

luxas May 22, 2018

luxas May 21, 2018

luxas May 21, 2018

stewart-yu commented May 22, 2018

m1093782566 May 22, 2018

stewart-yu May 22, 2018

m1093782566 commented May 22, 2018 •

edited

Loading

stewart-yu commented May 22, 2018

luxas left a comment

luxas commented May 22, 2018

m1093782566 commented May 22, 2018 •

edited

Loading

k8s-ci-robot commented May 22, 2018

k8s-github-robot commented May 22, 2018

m1093782566 commented May 22, 2018

k8s-github-robot commented May 22, 2018

Automatically load ipvs required kernel modules in kubeadm #59566

Automatically load ipvs required kernel modules in kubeadm #59566

Conversation

stewart-yu commented Feb 8, 2018

stewart-yu commented Feb 8, 2018

m1093782566 commented Feb 8, 2018

rramkumar1 commented Feb 8, 2018

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

stewart-yu commented Feb 9, 2018

dixudx commented Feb 13, 2018

fabriziopandini commented Mar 2, 2018

stewart-yu commented Mar 27, 2018

Lion-Wei commented Mar 27, 2018

m1093782566 commented Mar 27, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

stewart-yu May 22, 2018 • edited Loading

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

stewart-yu commented May 22, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

m1093782566 commented May 22, 2018 • edited Loading

stewart-yu commented May 22, 2018

luxas left a comment

Choose a reason for hiding this comment

luxas commented May 22, 2018

m1093782566 commented May 22, 2018 • edited Loading

k8s-ci-robot commented May 22, 2018

k8s-github-robot commented May 22, 2018

m1093782566 commented May 22, 2018

k8s-github-robot commented May 22, 2018

stewart-yu May 22, 2018 •

edited

Loading

m1093782566 commented May 22, 2018 •

edited

Loading

m1093782566 commented May 22, 2018 •

edited

Loading