From f9ff9c5ceaba1c5a74adfdb958920ab0da5125bc Mon Sep 17 00:00:00 2001 From: Dave Poole Date: Mon, 6 May 2024 09:51:49 -0700 Subject: [PATCH 1/2] Change the commandline used for systemd-run depeding on the installed version We found when testing on some ditros that they had older versions of systemd installed. Versions before 246 use `MemoryLimit` and after that use `MemoryMax` so we need to know which version we have when constructing the commandline. Also older versions didn't support the `-E` flag for environment variables and instead use the longer form `--setenv`. This same flag is supported in both old and new versions --- main/vmWatch.go | 54 +++++++++++++++++++++++++++++++++++++++++--- main/vmWatch_test.go | 16 +++++++++++++ 2 files changed, 67 insertions(+), 3 deletions(-) diff --git a/main/vmWatch.go b/main/vmWatch.go index caf2af09..cfc3c1d2 100644 --- a/main/vmWatch.go +++ b/main/vmWatch.go @@ -329,16 +329,28 @@ func setupVMWatchCommand(s *vmWatchSettings, hEnv *handlerenv.HandlerEnvironment var cmd *exec.Cmd // if we have systemd available, we will use that to launch the process, otherwise we will launch directly and manipulate our own cgroups if isSystemdAvailable() { + systemdVersion := getSystemdVersion() + // since systemd-run is in different paths on different distros, we will check for systemd but not use the full path // to systemd-run. This is how guest agent handles it also so seems appropriate. - systemdArgs := []string{"--scope", "-p", fmt.Sprintf("CPUQuota=%v%%", s.MaxCpuPercentage), "-p", fmt.Sprintf("MemoryMax=%v", s.MemoryLimitInBytes)} - // now append the env variables + systemdArgs := []string{"--scope", "-p", fmt.Sprintf("CPUQuota=%v%%", s.MaxCpuPercentage)} + + // systemd versions prior to 246 do not support MemoryMax, instead MemoryLimit should be used + if (systemdVersion < 246) { + systemdArgs = append(systemdArgs, "-p", fmt.Sprintf("MemoryLimit=%v", s.MemoryLimitInBytes)) + } else { + systemdArgs = append(systemdArgs, "-p", fmt.Sprintf("MemoryMax=%v", s.MemoryLimitInBytes)) + } + + // now append the env variables (--setenv is supported in all versions, -E only in newer versions) for _, v := range GetVMWatchEnvironmentVariables(s.ParameterOverrides, hEnv) { - systemdArgs = append(systemdArgs, "-E", v) + systemdArgs = append(systemdArgs, "--setenv", v) } systemdArgs = append(systemdArgs, GetVMWatchBinaryFullPath(processDirectory)) systemdArgs = append(systemdArgs, args...) + // since systemd-run is in different paths on different distros, we will check for systemd but not use the full path + // to systemd-run. This is how guest agent handles it also so seems appropriate. cmd = exec.Command("systemd-run", systemdArgs...) } else { cmd = exec.Command(GetVMWatchBinaryFullPath(processDirectory), args...) @@ -354,6 +366,42 @@ func isSystemdAvailable() bool { return err == nil && info.IsDir() } +func getSystemdVersion() (int) { + cmd := exec.Command("systemd-run", "--version") + + // Execute the command and capture the output + output, err := cmd.CombinedOutput() + if err != nil { + return 0 + } + + // Convert output bytes to string + outputStr := string(output) + + // Find the version information in the output + return extractVersion(outputStr) +} + +// Function to extract the version information from the output +// returns the version or 0 if not found +func extractVersion(output string) int { + lines := strings.Split(output, "\n") + for _, line := range lines { + if strings.HasPrefix(line, "systemd") { + parts := strings.Fields(line) + if len(parts) >= 2 { + ret, err := strconv.Atoi(parts[1]) + if err == nil { + return ret + } + return 0 + } + } + } + return 0 +} + + func createAndAssignCgroups(lg log.Logger, vmwatchSettings *vmWatchSettings, vmWatchPid int) error { // get our process and use this to determine the appropriate mount points for the cgroups myPid := os.Getpid() diff --git a/main/vmWatch_test.go b/main/vmWatch_test.go index 954ea857..7113c7b2 100644 --- a/main/vmWatch_test.go +++ b/main/vmWatch_test.go @@ -30,3 +30,19 @@ func TestGetMessageCorrectValue(t *testing.T) { res = VMWatchResult{Status: Running} require.Equal(t, "VMWatch is running", res.GetMessage()) } + +func TestExtractVersion(t *testing.T) { + v := extractVersion("systemd 123") + require.Equal(t, 123, v) + v = extractVersion(`someline +systemd 123 +some other line`) + require.Equal(t, 123, v) + v = extractVersion(`someline +systemd abc +some other line`) + require.Equal(t, 0, v) + v = extractVersion("junk") + require.Equal(t, 0, v) +} + From 5b29a32e8f3d204eba70183a677f741ff65e8f84 Mon Sep 17 00:00:00 2001 From: Dave Poole Date: Mon, 6 May 2024 10:44:02 -0700 Subject: [PATCH 2/2] feedback --- main/vmWatch.go | 46 +++++++++++++++++++++++----------------------- 1 file changed, 23 insertions(+), 23 deletions(-) diff --git a/main/vmWatch.go b/main/vmWatch.go index cfc3c1d2..7592a18a 100644 --- a/main/vmWatch.go +++ b/main/vmWatch.go @@ -369,36 +369,36 @@ func isSystemdAvailable() bool { func getSystemdVersion() (int) { cmd := exec.Command("systemd-run", "--version") - // Execute the command and capture the output - output, err := cmd.CombinedOutput() - if err != nil { - return 0 - } + // Execute the command and capture the output + output, err := cmd.CombinedOutput() + if err != nil { + return 0 + } - // Convert output bytes to string - outputStr := string(output) + // Convert output bytes to string + outputStr := string(output) - // Find the version information in the output - return extractVersion(outputStr) + // Find the version information in the output + return extractVersion(outputStr) } // Function to extract the version information from the output // returns the version or 0 if not found func extractVersion(output string) int { - lines := strings.Split(output, "\n") - for _, line := range lines { - if strings.HasPrefix(line, "systemd") { - parts := strings.Fields(line) - if len(parts) >= 2 { - ret, err := strconv.Atoi(parts[1]) - if err == nil { - return ret - } - return 0 - } - } - } - return 0 + lines := strings.Split(output, "\n") + for _, line := range lines { + if strings.HasPrefix(line, "systemd") { + parts := strings.Fields(line) + if len(parts) >= 2 { + ret, err := strconv.Atoi(parts[1]) + if err == nil { + return ret + } + return 0 + } + } + } + return 0 }