diff --git a/controllers/object_controls.go b/controllers/object_controls.go index 7fdb2613e..7ec4b9ea7 100644 --- a/controllers/object_controls.go +++ b/controllers/object_controls.go @@ -966,19 +966,22 @@ func TransformGPUDiscoveryPlugin(obj *appsv1.DaemonSet, config *gpuv1.ClusterPol return nil } -// Read and parse os-release file -func parseOSRelease() (map[string]string, error) { - release := map[string]string{} +// parseOSRelease can be overridden in tests for mocking filesystem access. +// In production, it reads and parses /host-etc/os-release. +var parseOSRelease = parseOSReleaseFromFile - // TODO: mock this call instead - if os.Getenv("UNIT_TEST") == "true" { - return release, nil - } +// osReleaseFilePath is the path to the os-release file, configurable for testing. +var osReleaseFilePath = "/host-etc/os-release" + +// parseOSReleaseFromFile reads and parses the os-release file from the host filesystem. +func parseOSReleaseFromFile() (map[string]string, error) { + release := map[string]string{} - f, err := os.Open("/host-etc/os-release") + f, err := os.Open(osReleaseFilePath) if err != nil { return nil, err } + defer f.Close() re := regexp.MustCompile(`^(?P\w+)=(?P.+)`) diff --git a/controllers/object_controls_test.go b/controllers/object_controls_test.go index 125257980..3e57983fc 100644 --- a/controllers/object_controls_test.go +++ b/controllers/object_controls_test.go @@ -146,6 +146,18 @@ func getModuleRoot(dir string) (string, error) { return dir, nil } +// mockOSRelease returns a mock parseOSRelease function for testing. +// It allows tests to simulate different operating systems without filesystem access. +func mockOSRelease(osID, version string) func() (map[string]string, error) { + return func() (map[string]string, error) { + return map[string]string{ + "ID": osID, + "VERSION_ID": version, + "NAME": osID, + }, nil + } +} + // setup creates a mock kubernetes cluster and client. Nodes are labeled with the minimum // required NFD labels to be detected as GPU nodes by the GPU Operator. A sample // ClusterPolicy resource is applied to the cluster. The ClusterPolicyController @@ -158,8 +170,8 @@ func setup() error { boolTrue = new(bool) *boolTrue = true - // add env for calls that we cannot mock - os.Setenv("UNIT_TEST", "true") + // Mock parseOSRelease to avoid filesystem dependency in tests + parseOSRelease = mockOSRelease("ubuntu", "20.04") s := scheme.Scheme if err := gpuv1.AddToScheme(s); err != nil { @@ -1393,3 +1405,59 @@ func TestService(t *testing.T) { }) } } + +func TestParseOSReleaseFromFile(t *testing.T) { + tests := []struct { + description string + content string + expected map[string]string + }{ + { + description: "quoted values", + content: `NAME="Ubuntu"` + "\n" + `VERSION_ID="20.04"`, + expected: map[string]string{"NAME": "Ubuntu", "VERSION_ID": "20.04"}, + }, + { + description: "unquoted values", + content: `NAME=Ubuntu` + "\n" + `ID=ubuntu`, + expected: map[string]string{"NAME": "Ubuntu", "ID": "ubuntu"}, + }, + { + description: "mixed quoted and unquoted", + content: `ID="rhel"` + "\n" + `VERSION_ID=8.5`, + expected: map[string]string{"ID": "rhel", "VERSION_ID": "8.5"}, + }, + { + description: "empty lines and comments", + content: `NAME="Ubuntu"` + "\n\n# comment\n" + `ID=ubuntu`, + expected: map[string]string{"NAME": "Ubuntu", "ID": "ubuntu"}, + }, + } + + tempDir := t.TempDir() + + // Save original value and restore after tests for future subsequent tests (if needed) + originalPath := osReleaseFilePath + defer func() { osReleaseFilePath = originalPath }() + + for i, test := range tests { + t.Run(test.description, func(t *testing.T) { + testFile := filepath.Join(tempDir, fmt.Sprintf("os-release-%d", i)) + err := os.WriteFile(testFile, []byte(test.content), 0600) + require.NoError(t, err) + + // Override the path for this test + osReleaseFilePath = testFile + result, err := parseOSReleaseFromFile() + require.NoError(t, err) + require.Equal(t, test.expected, result) + }) + } + + t.Run("file not found", func(t *testing.T) { + osReleaseFilePath = "/nonexistent/path" + _, err := parseOSReleaseFromFile() + require.Error(t, err) + require.True(t, os.IsNotExist(err)) + }) +}