From 50d70b626e29af9ad7baab0c6f452f40972e431e Mon Sep 17 00:00:00 2001 From: Tamir David Date: Tue, 13 Aug 2024 16:05:55 +0300 Subject: [PATCH] fix: remove only protected c files and their directories --- odiglet/pkg/instrumentation/fs/agents.go | 17 +++++-- odiglet/pkg/instrumentation/fs/copy.go | 17 +++---- odiglet/pkg/instrumentation/fs/copy_test.go | 9 ++-- odiglet/pkg/instrumentation/fs/remove.go | 50 ++++++++++++++------- 4 files changed, 60 insertions(+), 33 deletions(-) diff --git a/odiglet/pkg/instrumentation/fs/agents.go b/odiglet/pkg/instrumentation/fs/agents.go index 3cfc52510..ed668e1ff 100644 --- a/odiglet/pkg/instrumentation/fs/agents.go +++ b/odiglet/pkg/instrumentation/fs/agents.go @@ -22,14 +22,25 @@ func CopyAgentsDirectoryToHost() error { // as we want a fresh copy of instrumentation agents with no files leftover from previous odigos versions. // we cannot remove /var/odigos itself: "unlinkat /var/odigos: device or resource busy" // so we will just remove it's content - // We kept the .so/.node files to avoid removing the instrumentations that are already loaded in the process memory - err := removeFilesInDir(hostDir) + + // We kept the following list of files to avoid removing instrumentations that are already loaded in the process memory + filesToKeepMap := map[string]struct{}{ + "/var/odigos/nodejs-ebpf/build/Release/dtrace-injector-native.node": {}, + "/var/odigos/nodejs-ebpf/build/Release/obj.target/dtrace-injector-native.node": {}, + "/var/odigos/nodejs-ebpf/build/Release/.deps/Release/dtrace-injector-native.node.d": {}, + "/var/odigos/nodejs-ebpf/build/Release/.deps/Release/obj.target/dtrace-injector-native.node.d": {}, + "/var/odigos/java-ebpf/tracing_probes.so": {}, + "/var/odigos/java-ext-ebpf/end_span_usdt.so": {}, + "/var/odigos/python-ebpf/pythonUSDT.abi3.so": {}, + } + + err := removeFilesInDir(hostDir, filesToKeepMap) if err != nil { log.Logger.Error(err, "Error removing instrumentation directory from host") return err } - err = copyDirectories(containerDir, hostDir) + err = copyDirectories(containerDir, hostDir, filesToKeepMap) if err != nil { log.Logger.Error(err, "Error copying instrumentation directory to host") return err diff --git a/odiglet/pkg/instrumentation/fs/copy.go b/odiglet/pkg/instrumentation/fs/copy.go index fbeccddc1..bfdd153d5 100644 --- a/odiglet/pkg/instrumentation/fs/copy.go +++ b/odiglet/pkg/instrumentation/fs/copy.go @@ -27,7 +27,7 @@ func getNumberOfWorkers() int { return min(maxWorkers, max(1, runtime.NumCPU()/4)) } -func copyDirectories(srcDir string, destDir string) error { +func copyDirectories(srcDir string, destDir string, filesToKeepMap map[string]struct{}) error { start := time.Now() hostContainEbpfDir := HostContainsEbpfDir(destDir) @@ -37,7 +37,7 @@ func copyDirectories(srcDir string, destDir string) error { CopyCFiles := !hostContainEbpfDir || shouldRecreateCFiles log.Logger.V(0).Info("Copying instrumentation files to host", "srcDir", srcDir, "destDir", destDir, "CopyCFiles", CopyCFiles) - files, err := getFiles(srcDir, CopyCFiles) + files, err := getFiles(srcDir, CopyCFiles, filesToKeepMap) if err != nil { return err } @@ -85,21 +85,17 @@ func worker(fileChan <-chan string, sourceDir, destDir string, wg *sync.WaitGrou } } -func getFiles(dir string, CopyCFiles bool) ([]string, error) { +func getFiles(dir string, CopyCFiles bool, filesToKeepMap map[string]struct{}) ([]string, error) { var files []string err := filepath.WalkDir(dir, func(path string, d fs.DirEntry, err error) error { if err != nil { return err } if !d.IsDir() { - if !CopyCFiles { - // filter out C files in ebpf directories - if strings.Contains(filepath.Dir(path), "ebpf") { - switch ext := filepath.Ext(path); ext { - case ".so", ".node", ".node.d", ".a": - return nil - } + if _, found := filesToKeepMap[strings.Replace(path, "/instrumentations/", "/var/odigos/", 1)]; found { + log.Logger.V(0).Info("Skipping copying file", "file", path) + return nil } } @@ -107,6 +103,7 @@ func getFiles(dir string, CopyCFiles bool) ([]string, error) { } return nil }) + return files, err } diff --git a/odiglet/pkg/instrumentation/fs/copy_test.go b/odiglet/pkg/instrumentation/fs/copy_test.go index 6dfd33623..1b172a25a 100644 --- a/odiglet/pkg/instrumentation/fs/copy_test.go +++ b/odiglet/pkg/instrumentation/fs/copy_test.go @@ -15,7 +15,8 @@ func TestGetFiles(t *testing.T) { t.Fatalf("createTestFiles failed: %v", err) } - gotFiles, err := getFiles(tempDir+"/dir1", false) + filesToKeep := make(map[string]struct{}) + gotFiles, err := getFiles(tempDir+"/dir1", false, filesToKeep) if err != nil { t.Fatalf("getFiles failed: %v", err) } @@ -66,6 +67,7 @@ func createTestFiles(tempDir string, num int) ([]string, error) { } func TestCopyDirectories(t *testing.T) { + filesToKeep := make(map[string]struct{}) src := t.TempDir() dest := t.TempDir() files, err := createTestFiles(src, 10) @@ -73,7 +75,7 @@ func TestCopyDirectories(t *testing.T) { t.Fatalf("createTestFiles failed: %v", err) } - err = copyDirectories(src, dest) + err = copyDirectories(src, dest, filesToKeep) if err != nil { t.Fatalf("copyDirectories failed: %v", err) } @@ -98,6 +100,7 @@ func TestCopyDirectories(t *testing.T) { } func BenchmarkCopyDirectories(b *testing.B) { + filesToKeep := make(map[string]struct{}) src := b.TempDir() dest := b.TempDir() _, err := createTestFiles(src, b.N) @@ -107,7 +110,7 @@ func BenchmarkCopyDirectories(b *testing.B) { b.ResetTimer() for i := 0; i < b.N; i++ { - err = copyDirectories(src, dest) + err = copyDirectories(src, dest, filesToKeep) if err != nil { b.Fatalf("copyDirectories failed: %v", err) } diff --git a/odiglet/pkg/instrumentation/fs/remove.go b/odiglet/pkg/instrumentation/fs/remove.go index d50d2f8b5..f5e6cd577 100644 --- a/odiglet/pkg/instrumentation/fs/remove.go +++ b/odiglet/pkg/instrumentation/fs/remove.go @@ -4,43 +4,59 @@ import ( "fmt" "os" "path/filepath" - "strings" "github.com/odigos-io/odigos/odiglet/pkg/log" ) -func removeFilesInDir(hostDir string) error { +func removeFilesInDir(hostDir string, filesToKeep map[string]struct{}) error { shouldRecreateCFiles := ShouldRecreateAllCFiles() - log.Logger.V(0).Info(fmt.Sprintf("Removing files in directory: %s, shouldRecreateCFiles: %s", hostDir, fmt.Sprintf("%t", shouldRecreateCFiles))) + log.Logger.V(0).Info(fmt.Sprintf("Removing files in directory: %s, shouldRecreateCFiles: %t", hostDir, shouldRecreateCFiles)) + + // Mark directories as protected if they contain a file that needs to be preserved. + // If C files should be recreated, skip marking any directories as protected. + protectedDirs := make(map[string]bool) + if !shouldRecreateCFiles { + for file := range filesToKeep { + dir := filepath.Dir(file) + for dir != hostDir { + protectedDirs[dir] = true + dir = filepath.Dir(dir) + } + protectedDirs[hostDir] = true // Protect the main directory + } + } return filepath.Walk(hostDir, func(path string, info os.FileInfo, err error) error { if err != nil { return err } - // Skip the root directory itself] if path == hostDir { return nil } - if info.IsDir() { + // Skip removing any files listed in filesToKeepMap + if !info.IsDir() { + if _, found := filesToKeep[path]; found { + log.Logger.V(0).Info(fmt.Sprintf("Skipping protected file: %s", path)) + return nil + } + } + + // Skip removing protected directories + if info.IsDir() && protectedDirs[path] { + log.Logger.V(0).Info(fmt.Sprintf("Skipping protected directory: %s", path)) return nil } - if !shouldRecreateCFiles { - // filter out C files in ebpf directories - if strings.Contains(filepath.Dir(path), "ebpf") { - switch ext := filepath.Ext(info.Name()); ext { - case ".so", ".node", "node.d", ".a": - return nil - } - } + // Remove removing unprotected files and directories + if err := os.RemoveAll(path); err != nil { + return fmt.Errorf("error removing %s: %w", path, err) } - // Remove the file - err = os.Remove(path) - if err != nil { - return err + // Skip further processing in this directory since it has been removed + if info.IsDir() { + return filepath.SkipDir } return nil