Skip to content

Commit

Permalink
support inTree removal scenario for nodes without .ko files (#1210)
Browse files Browse the repository at this point in the history
Modprobe will fail with error in case it cannot find an appropriate
.ko file when asked to remove a kernel module. With in-tree removal
scenario, modporbe search path is /lib/modules on the host, and not
the worker image.
This PR contains the following:
1) Adding FileExists function that check the presence of a file under
   a search path based on a regexp
2) in worker flow, in case in-tree removal scenario is requisted, worker
   will verify if the requested .ko file(s) are present on the host,
   remove from the request missing files and will proceed with the
   removal scenario for the rest
3) unit-test
  • Loading branch information
yevgeny-shnaidman authored Sep 12, 2024
1 parent 6a77549 commit ed8e38a
Show file tree
Hide file tree
Showing 5 changed files with 113 additions and 9 deletions.
27 changes: 27 additions & 0 deletions internal/utils/filesystem_helper.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,10 @@ package utils

import (
"fmt"
"io/fs"
"os"
"path/filepath"
"regexp"

"github.com/go-logr/logr"
)
Expand All @@ -12,6 +14,7 @@ import (

type FSHelper interface {
RemoveSrcFilesFromDst(srcDir, dstDir string) error
FileExists(root, fileRegex string) (bool, error)
}

type fsHelper struct {
Expand Down Expand Up @@ -49,3 +52,27 @@ func (fh *fsHelper) RemoveSrcFilesFromDst(srcDir, dstDir string) error {
}
return nil
}

func (fh *fsHelper) FileExists(root, fileRegex string) (bool, error) {
regex, err := regexp.Compile(fileRegex)
if err != nil {
return false, fmt.Errorf("failed to compile regex %s: %v", fileRegex, err)
}

found := false
// Walk through the directory
err = filepath.WalkDir(root, func(path string, d fs.DirEntry, err error) error {
if err != nil {
return err
}

// Match file names against the regex
if !d.IsDir() && regex.MatchString(d.Name()) {
found = true
return fs.SkipAll
}
return nil
})

return found, err
}
42 changes: 40 additions & 2 deletions internal/utils/filesystem_helper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ var _ = Describe("RemoveSrcFilesFromDst", func() {
// source
err := os.MkdirAll("./srcDir/level1", 0750)
Expect(err).NotTo(HaveOccurred())
defer os.RemoveAll("./srcDir")
err = os.MkdirAll("./srcDir/level2", 0750)
Expect(err).NotTo(HaveOccurred())
err = os.MkdirAll("./srcDir/level4", 0750)
Expand All @@ -25,6 +26,7 @@ var _ = Describe("RemoveSrcFilesFromDst", func() {
// destination
err = os.MkdirAll("./dstDir/level1", 0750)
Expect(err).NotTo(HaveOccurred())
defer os.RemoveAll("./dstDir")
err = os.MkdirAll("./dstDir/level2", 0750)
Expect(err).NotTo(HaveOccurred())
err = os.MkdirAll("./dstDir/level3", 0750)
Expand All @@ -42,9 +44,45 @@ var _ = Describe("RemoveSrcFilesFromDst", func() {
verifyFileNotExists("./dstDir/level2/testfile2")

verifyFileExists("./dstDir/level3/testfile3")
})
})

defer os.RemoveAll("./dstDir")
defer os.RemoveAll("./srcDir")
var _ = Describe("FileExists", func() {
It("test files", func() {
err := os.MkdirAll("./testDir/level_1_0", 0750)
Expect(err).NotTo(HaveOccurred())
defer os.RemoveAll("./testDir")
err = os.MkdirAll("./testDir/level_1_1", 0750)
Expect(err).NotTo(HaveOccurred())
err = os.MkdirAll("./testDir/level_2_0", 0750)
Expect(err).NotTo(HaveOccurred())
err = os.MkdirAll("./testDir/level_2_1", 0750)
Expect(err).NotTo(HaveOccurred())
createEmptyFile("./testDir/level_1_1/not_ko_file")
createEmptyFile("./testDir/level_2_1/module1.ko.kz")
createEmptyFile("./testDir/level_1_0/module2.ko")

helper := NewFSHelper(logr.Discard())

By("find existing file by full name")
exists, err := helper.FileExists("testDir", "module2.ko")
Expect(err).NotTo(HaveOccurred())
Expect(exists).To(BeTrue())

By("find existing file by regexp name")
exists, err = helper.FileExists("testDir", `.*\.ko*$`)
Expect(err).NotTo(HaveOccurred())
Expect(exists).To(BeTrue())

By("find existing file by regexp name, comparing prefix")
exists, err = helper.FileExists("testDir", `^module1.ko`)
Expect(err).NotTo(HaveOccurred())
Expect(exists).To(BeTrue())

By("find non-existing file by regexp name")
exists, err = helper.FileExists("testDir", `.*\.koz.*$`)
Expect(err).NotTo(HaveOccurred())
Expect(exists).To(BeFalse())
})
})

Expand Down
15 changes: 15 additions & 0 deletions internal/utils/mock_filesystem_helper.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

22 changes: 19 additions & 3 deletions internal/worker/worker.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,10 +47,26 @@ func (w *worker) LoadKmod(ctx context.Context, cfg *kmmv1beta1.ModuleConfig, fir

if inTreeModulesToRemove != nil {
w.logger.Info("Unloading in-tree modules", "names", inTreeModulesToRemove)
modulesToUnload := make([]string, 0, len(inTreeModulesToRemove))
for _, module := range inTreeModulesToRemove {
exists, err := w.fh.FileExists("/lib/modules", fmt.Sprintf("^%s.ko", module))
if err != nil {
w.logger.Info(utils.WarnString(fmt.Sprintf("failed to check if module file %s present on the host:", module)), "error", err)
continue
}
if !exists {
w.logger.Info("not trying to unload in-tree module, since its file is not present on the host", "module", module)
continue
}
w.logger.Info("adding module to the list of intree modules to be unloadded", "module", module)
modulesToUnload = append(modulesToUnload, module)
}

runArgs := append([]string{"-rv"}, inTreeModulesToRemove...)
if err := w.mr.Run(ctx, runArgs...); err != nil {
return fmt.Errorf("could not remove in-tree modules %s: %v", strings.Join(inTreeModulesToRemove, ""), err)
if len(modulesToUnload) > 0 {
runArgs := append([]string{"-rv"}, modulesToUnload...)
if err := w.mr.Run(ctx, runArgs...); err != nil {
return fmt.Errorf("could not remove in-tree modules %s: %v", strings.Join(modulesToUnload, ""), err)
}
}
}

Expand Down
16 changes: 12 additions & 4 deletions internal/worker/worker_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package worker
import (
"context"
"errors"
"fmt"
"os"
"path/filepath"

Expand All @@ -15,6 +16,7 @@ import (

var _ = Describe("worker_LoadKmod", func() {
var (
fh *utils.MockFSHelper
mr *MockModprobeRunner
w Worker
imageDir string
Expand All @@ -23,8 +25,9 @@ var _ = Describe("worker_LoadKmod", func() {

BeforeEach(func() {
ctrl := gomock.NewController(GinkgoT())
fh = utils.NewMockFSHelper(ctrl)
mr = NewMockModprobeRunner(ctrl)
w = NewWorker(mr, nil, GinkgoLogr)
w = NewWorker(mr, fh, GinkgoLogr)

var err error
imageDir, err = os.MkdirTemp("", "imageDir")
Expand Down Expand Up @@ -66,8 +69,8 @@ var _ = Describe("worker_LoadKmod", func() {
)
})

It("should remove the in-tree module if configured", func() {
inTreeModulesToRemove := []string{"intree1", "intree2"}
It("should remove present-on-host in-tree module if configured", func() {
inTreeModulesToRemove := []string{"intree1", "intree2", "intree3", "intree4"}

cfg := v1beta1.ModuleConfig{
ContainerImage: imageName,
Expand All @@ -79,7 +82,11 @@ var _ = Describe("worker_LoadKmod", func() {
}

gomock.InOrder(
mr.EXPECT().Run(ctx, "-rv", "intree1", "intree2"),
fh.EXPECT().FileExists("/lib/modules", "^intree1.ko").Return(true, nil),
fh.EXPECT().FileExists("/lib/modules", "^intree2.ko").Return(false, nil),
fh.EXPECT().FileExists("/lib/modules", "^intree3.ko").Return(true, nil),
fh.EXPECT().FileExists("/lib/modules", "^intree4.ko").Return(false, fmt.Errorf("some error")),
mr.EXPECT().Run(ctx, "-rv", "intree1", "intree3"),
mr.EXPECT().Run(ctx, "-vd", filepath.Join(sharedFilesDir, dirName), moduleName),
)

Expand All @@ -102,6 +109,7 @@ var _ = Describe("worker_LoadKmod", func() {
}

gomock.InOrder(
fh.EXPECT().FileExists("/lib/modules", "^intreeToRemove.ko").Return(true, nil),
mr.EXPECT().Run(ctx, "-rv", "intreeToRemove"),
mr.EXPECT().Run(ctx, "-vd", filepath.Join(sharedFilesDir, dirName), moduleName),
)
Expand Down

0 comments on commit ed8e38a

Please sign in to comment.