Replace parseOSRelease env var hack with mockable function variable

Signed-off-by: Karthik Vetrivel <[email protected]>

Author: karthikvetrivel

controllers/object_controls.go

@@ -966,19 +966,19 @@ 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
-	}
+// 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")
 	if err != nil {
 		return nil, err
 	}
+	defer f.Close()
 
 	re := regexp.MustCompile(`^(?P<key>\w+)=(?P<value>.+)`)
 

controllers/object_controls_test.go

@@ -17,11 +17,13 @@
 package controllers
 
 import (
+	"bufio"
 	"context"
 	"fmt"
 	"log"
 	"os"
 	"path/filepath"
+	"regexp"
 	goruntime "runtime"
 	"strings"
 	"testing"
@@ -146,6 +148,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 +172,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 +1407,68 @@ func TestService(t *testing.T) {
 		})
 	}
 }
+
+func TestParseOSReleaseFromFile(t *testing.T) {
+	parseFile := func(path string) (map[string]string, error) {
+		release := map[string]string{}
+		f, err := os.Open(path)
+		if err != nil {
+			return nil, err
+		}
+		defer f.Close()
+
+		re := regexp.MustCompile(`^(?P<key>\w+)=(?P<value>.+)`)
+		scanner := bufio.NewScanner(f)
+		for scanner.Scan() {
+			line := scanner.Text()
+			if match := re.FindStringSubmatch(line); match != nil {
+				release[match[1]] = strings.Trim(match[2], `"`)
+			}
+		}
+		return release, nil
+	}
+
+	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()
+	for i, test := range tests {
+		testFile := filepath.Join(tempDir, fmt.Sprintf("os-release-%d", i))
+		err := os.WriteFile(testFile, []byte(test.content), 0644)
+		require.NoError(t, err)
+
+		result, err := parseFile(testFile)
+		require.NoError(t, err)
+		require.Equal(t, test.expected, result, test.description)
+	}
+
+	t.Run("file not found", func(t *testing.T) {
+		_, err := parseFile("/nonexistent/path")
+		require.Error(t, err)
+		require.True(t, os.IsNotExist(err))
+	})
+}