Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Check first if ipvs module is builtin #82223

Merged
merged 1 commit into from Jan 17, 2020
Merged

Check first if ipvs module is builtin #82223

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
19 changes: 16 additions & 3 deletions pkg/proxy/ipvs/proxier.go
View file
Open in desktop
Expand Up @@ -582,11 +582,24 @@ func (handle *LinuxKernelHandler) GetModules() ([]string, error) {

var bmods []string

// Find out loaded kernel modules. If this is a full static kernel it will thrown an error
// Find out loaded kernel modules. If this is a full static kernel it will try to verify if the module is compiled using /boot/config-KERNELVERSION
modulesFile, err := os.Open("/proc/modules")
if err == os.ErrNotExist {
klog.Warningf("Failed to read file /proc/modules with error %v. Assuming this is a kernel without loadable modules support enabled", err)
kernelConfigFile := fmt.Sprintf("/boot/config-%s", kernelVersionStr)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are we confident this check here works across all Linux distributions/kernels? I'm okay with the latter half of this PR where we don't re-run modprobe if there's already a match, but unsure about this part.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

About the /proc/modules or about the /boot/config?

About the /boot/config no, I'm far away of being confident but. As I wrote here when I was recompiling the Kernel, I simply didn't copied the file to /boot so in my case this was missing.

On the other hand, this 'if' only exists on statically compiled Kernel (that lacks /proc/modules path) so this case might be pretty particular:

  • A very specific distro with a full static kernel (something of IoT maybe?)
  • A sysadmin who recompiles the kernel (as our original case/issue) that knows he needs to copy the file (and maybe we should put this into a better logging).

kConfig, err := ioutil.ReadFile(kernelConfigFile)
if err != nil {
return nil, fmt.Errorf("Failed to read Kernel Config file %s with error %v", kernelConfigFile, err)
}
for _, module := range ipvsModules {
if match, _ := regexp.Match("CONFIG_"+strings.ToUpper(module)+"=y", kConfig); match {
bmods = append(bmods, module)
}
}
return bmods, nil
}
if err != nil {
klog.Warningf("Failed to read file /proc/modules with error %v. Kube-proxy requires loadable modules support enabled in the kernel", err)
return nil, err
return nil, fmt.Errorf("Failed to read file /proc/modules with error %v", err)
}

mods, err := getFirstColumn(modulesFile)
Expand Down