Skip to content

fix(host-preflights): builtin kernel modules file from wrong path #1741

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

Merged
merged 5 commits into from
Feb 18, 2025
Merged

fix(host-preflights): builtin kernel modules file from wrong path #1741

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
20b34c0
fix(host-preflights): buildtin kernel modules file from wrong path
emosbaugh Feb 18, 2025
76bd1d9
f
emosbaugh Feb 18, 2025
c22ed19
f
emosbaugh Feb 18, 2025
3bd2e7f
f
emosbaugh Feb 18, 2025
8403b02
f
emosbaugh Feb 18, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion pkg/collect/host_collector.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,9 @@ func GetHostCollector(collector *troubleshootv1beta2.HostCollect, bundlePath str
hostCollector: collector.KernelModules,
BundlePath: bundlePath,
loadable: kernelModulesLoadable{},
loaded: kernelModulesLoaded{},
loaded: kernelModulesLoaded{
fs: os.DirFS("/"),
},
}, true
case collector.TCPConnect != nil:
return &CollectHostTCPConnect{collector.TCPConnect, bundlePath}, true
Expand Down
56 changes: 32 additions & 24 deletions pkg/collect/host_kernel_modules.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"encoding/json"
"fmt"
"io"
"io/fs"
"os"
"os/exec"
"path/filepath"
Expand Down Expand Up @@ -38,7 +39,7 @@ const HostKernelModulesPath = `host-collectors/system/kernel_modules.json`
// kernelModuleCollector defines the interface used to collect modules from the
// underlying host.
type kernelModuleCollector interface {
collect() (map[string]KernelModuleInfo, error)
collect(kernelRelease string) (map[string]KernelModuleInfo, error)
}

// CollectHostKernelModules is responsible for collecting kernel module status
Expand Down Expand Up @@ -79,14 +80,20 @@ func (c *CollectHostKernelModules) IsExcluded() (bool, error) {
// a module is loaded, it may have one or more instances. The size represents
// the amount of memory (in bytes) that the module is using.
func (c *CollectHostKernelModules) Collect(progressChan chan<- interface{}) (map[string][]byte, error) {
modules, err := c.loadable.collect()
out, err := exec.Command("uname", "-r").Output()
if err != nil {
return nil, errors.Wrap(err, "failed to determine kernel release")
}
kernelRelease := strings.TrimSpace(string(out))

modules, err := c.loadable.collect(kernelRelease)
if err != nil {
return nil, errors.Wrap(err, "failed to read loadable kernel modules")
}
if modules == nil {
modules = map[string]KernelModuleInfo{}
}
loaded, err := c.loaded.collect()
loaded, err := c.loaded.collect(kernelRelease)
if err != nil {
return nil, errors.Wrap(err, "failed to read loaded kernel modules")
}
Expand Down Expand Up @@ -114,20 +121,16 @@ func (c *CollectHostKernelModules) Collect(progressChan chan<- interface{}) (map
type kernelModulesLoadable struct{}

// collect the list of modules that can be loaded by the kernel.
func (l kernelModulesLoadable) collect() (map[string]KernelModuleInfo, error) {
func (l kernelModulesLoadable) collect(kernelRelease string) (map[string]KernelModuleInfo, error) {
modules := make(map[string]KernelModuleInfo)

out, err := exec.Command("uname", "-r").Output()
if err != nil {
return nil, errors.Wrap(err, "failed to determine kernel release")
}
kernel := strings.TrimSpace(string(out))

kernelPath := "/lib/modules/" + kernel

kernelPath := filepath.Join("/lib/modules", kernelRelease)
if _, err := os.Stat(kernelPath); os.IsNotExist(err) {
klog.V(2).Infof("modules are not loadable because kernel path %q does not exist, assuming we are in a container", kernelPath)
Copy link
Member

@squizzi squizzi Feb 18, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

nit: It might be worth logging in this instance that /lib/modules does not exist, checking /usr/lib/modules before the assuming we are in container log fires, because that makes it sound like we only ever checked /usr/lib/modules.

Or maybe update the kernelPath in Infof to be:

klog.V(2).Infof("modules are not loadable because kernel path %q or %q do not exist, assuming we are in a container", kernelPathLib, kernelPathUsr)

Copy link
Member Author

@emosbaugh emosbaugh Feb 18, 2025

Choose a reason for hiding this comment

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

updated to print the expected path

return modules, nil
kernelPath = filepath.Join("/usr/lib/modules", kernelRelease)
if _, err := os.Stat(kernelPath); os.IsNotExist(err) {
klog.V(2).Infof("kernel modules are not loadable because path %q does not exist, assuming we are in a container", kernelPath)
return modules, nil
}
}

cmd := exec.Command("/usr/bin/find", kernelPath, "-type", "f", "-name", "*.ko*")
Expand Down Expand Up @@ -155,16 +158,18 @@ func (l kernelModulesLoadable) collect() (map[string]KernelModuleInfo, error) {

// kernelModulesLoaded retrieves the list of modules that the kernel is aware of. The
// modules will either be in loaded, loading or unloading state.
type kernelModulesLoaded struct{}
type kernelModulesLoaded struct {
fs fs.FS
}

// collect the list of modules that the kernel is aware of.
func (l kernelModulesLoaded) collect() (map[string]KernelModuleInfo, error) {
func (l kernelModulesLoaded) collect(kernelRelease string) (map[string]KernelModuleInfo, error) {
modules, err := l.collectProc()
if err != nil {
return nil, fmt.Errorf("proc: %w", err)
}

builtin, err := l.collectBuiltin()
builtin, err := l.collectBuiltin(kernelRelease)
if err != nil {
return nil, fmt.Errorf("builtin: %w", err)
}
Expand All @@ -181,7 +186,7 @@ func (l kernelModulesLoaded) collect() (map[string]KernelModuleInfo, error) {
func (l kernelModulesLoaded) collectProc() (map[string]KernelModuleInfo, error) {
modules := make(map[string]KernelModuleInfo)

file, err := os.Open("/proc/modules")
file, err := l.fs.Open("proc/modules")
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -221,14 +226,17 @@ func (l kernelModulesLoaded) collectProc() (map[string]KernelModuleInfo, error)
return modules, nil
}

func (l kernelModulesLoaded) collectBuiltin() (map[string]KernelModuleInfo, error) {
out, err := exec.Command("uname", "-r").Output()
if err != nil {
return nil, errors.Wrap(err, "failed to determine kernel release")
func (l kernelModulesLoaded) collectBuiltin(kernelRelease string) (map[string]KernelModuleInfo, error) {
builtinPath := filepath.Join("lib/modules", kernelRelease, "modules.builtin")
if _, err := fs.Stat(l.fs, builtinPath); os.IsNotExist(err) {
builtinPath = filepath.Join("usr/lib/modules", kernelRelease, "modules.builtin")
if _, err := fs.Stat(l.fs, builtinPath); os.IsNotExist(err) {
klog.V(2).Infof("kernel builtin modules path %q does not exist, assuming we are in a container", builtinPath)
return nil, nil
}
}
kernel := strings.TrimSpace(string(out))

file, err := os.Open(fmt.Sprintf("/usr/lib/modules/%s/modules.builtin", kernel))
file, err := l.fs.Open(builtinPath)
if err != nil {
if os.IsNotExist(err) {
return nil, nil
Expand Down
128 changes: 127 additions & 1 deletion pkg/collect/host_kernel_modules_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
package collect

import (
"io/fs"
"reflect"
"strings"
"testing"
"testing/fstest"

"github.com/pkg/errors"
troubleshootv1beta2 "github.com/replicatedhq/troubleshoot/pkg/apis/troubleshoot/v1beta2"
Expand All @@ -14,7 +16,7 @@ type mockKernelModulesCollector struct {
err error
}

func (m mockKernelModulesCollector) collect() (map[string]KernelModuleInfo, error) {
func (m mockKernelModulesCollector) collect(kernelRelease string) (map[string]KernelModuleInfo, error) {
if m.err != nil {
return nil, m.err
}
Expand Down Expand Up @@ -213,3 +215,127 @@ kernel/arch/x86/events/intel/intel-cstate.ko`,
})
}
}

func Test_kernelModulesLoaded_collect(t *testing.T) {
tests := []struct {
name string
fs fs.FS
kernelRelease string
want map[string]KernelModuleInfo
wantErr bool
}{
{
name: "lib modules path",
fs: &fstest.MapFS{
"proc/modules": &fstest.MapFile{
Data: []byte(`module1 1000 2 - Live 0x0000000000000000
module2 2000 1 - Loading 0x0000000000000000
`),
Mode: 0444,
},
"lib/modules/5.4.0/modules.builtin": &fstest.MapFile{
Data: []byte(`kernel/builtin1.ko
kernel/builtin2.ko
`),
Mode: 0644,
},
},
kernelRelease: "5.4.0",
want: map[string]KernelModuleInfo{
"module1": {
Size: 1000,
Instances: 2,
Status: KernelModuleLoaded,
},
"module2": {
Size: 2000,
Instances: 1,
Status: KernelModuleLoading,
},
"builtin1": {
Status: KernelModuleLoaded,
},
"builtin2": {
Status: KernelModuleLoaded,
},
},
},
{
name: "usr lib modules path",
fs: &fstest.MapFS{
"proc/modules": &fstest.MapFile{
Data: []byte(`module1 1000 2 - Live 0x0000000000000000
`),
Mode: 0444,
},
"usr/lib/modules/5.4.0/modules.builtin": &fstest.MapFile{
Data: []byte(`kernel/builtin1.ko
kernel/builtin2.ko
`),
Mode: 0644,
},
},
kernelRelease: "5.4.0",
want: map[string]KernelModuleInfo{
"module1": {
Size: 1000,
Instances: 2,
Status: KernelModuleLoaded,
},
"builtin1": {
Status: KernelModuleLoaded,
},
"builtin2": {
Status: KernelModuleLoaded,
},
},
},
{
name: "no builtin modules file",
fs: &fstest.MapFS{
"proc/modules": &fstest.MapFile{
Data: []byte(`module1 1000 2 - Live 0x0000000000000000
`),
Mode: 0444,
},
},
kernelRelease: "5.4.0",
want: map[string]KernelModuleInfo{
"module1": {
Size: 1000,
Instances: 2,
Status: KernelModuleLoaded,
},
},
},
{
name: "no proc modules file should error",
fs: &fstest.MapFS{
"lib/modules/5.4.0/modules.builtin": &fstest.MapFile{
Data: []byte(`kernel/builtin1.ko
kernel/builtin2.ko
`),
Mode: 0644,
},
},
kernelRelease: "5.4.0",
wantErr: true,
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
l := kernelModulesLoaded{
fs: tt.fs,
}
got, err := l.collect(tt.kernelRelease)
if (err != nil) != tt.wantErr {
t.Errorf("kernelModulesLoaded.collect() error = %v, wantErr %v", err, tt.wantErr)
return
}
if !reflect.DeepEqual(got, tt.want) {
t.Errorf("kernelModulesLoaded.collect() = %v, want %v", got, tt.want)
}
})
}
}
Loading