Merge pull request #5261 from kolyshkin/aa

Use sync.OnceFunc, sync.OnceValue[s]

libcontainer/apparmor/apparmor_linux.go

@@ -11,21 +11,14 @@ import (
 	"github.com/opencontainers/runc/internal/pathrs"
 )
 
-var (
-	appArmorEnabled bool
-	checkAppArmor   sync.Once
-)
-
 // isEnabled returns true if apparmor is enabled for the host.
-func isEnabled() bool {
-	checkAppArmor.Do(func() {
-		if _, err := os.Stat("/sys/kernel/security/apparmor"); err == nil {
-			buf, err := os.ReadFile("/sys/module/apparmor/parameters/enabled")
-			appArmorEnabled = err == nil && len(buf) > 1 && buf[0] == 'Y'
-		}
-	})
-	return appArmorEnabled
-}
+var isEnabled = sync.OnceValue(func() bool {
+	if _, err := os.Stat("/sys/kernel/security/apparmor"); err != nil {
+		return false
+	}
+	buf, err := os.ReadFile("/sys/module/apparmor/parameters/enabled")
+	return err == nil && len(buf) > 0 && buf[0] == 'Y'
+})
 
 func setProcAttr(attr, value string) error {
 	attr = pathrs.LexicallyCleanPath(attr)

libcontainer/configs/validate/intelrdt.go

@@ -1,41 +1,12 @@
 package validate
 
 import (
-	"sync"
-
 	"github.com/opencontainers/runc/libcontainer/intelrdt"
 )
 
-// Cache the result of intelrdt IsEnabled functions to avoid repeated sysfs
-// access and enable mocking for unit tests.
-type intelRdtStatus struct {
-	sync.Once
-	rdtEnabled bool
-	catEnabled bool
-	mbaEnabled bool
-}
-
-var intelRdt = &intelRdtStatus{}
-
-func (i *intelRdtStatus) init() {
-	i.Do(func() {
-		i.rdtEnabled = intelrdt.IsEnabled()
-		i.catEnabled = intelrdt.IsCATEnabled()
-		i.mbaEnabled = intelrdt.IsMBAEnabled()
-	})
-}
-
-func (i *intelRdtStatus) isEnabled() bool {
-	i.init()
-	return i.rdtEnabled
-}
-
-func (i *intelRdtStatus) isCATEnabled() bool {
-	i.init()
-	return i.catEnabled
-}
-
-func (i *intelRdtStatus) isMBAEnabled() bool {
-	i.init()
-	return i.mbaEnabled
-}
+// Allow mocking for TestValidateIntelRdt.
+var (
+	intelRdtIsEnabled    = intelrdt.IsEnabled
+	intelRdtIsCATEnabled = intelrdt.IsCATEnabled
+	intelRdtIsMBAEnabled = intelrdt.IsMBAEnabled
+)

libcontainer/configs/validate/intelrdt_test.go

@@ -4,12 +4,10 @@ import (
 	"testing"
 
 	"github.com/opencontainers/runc/libcontainer/configs"
+	"github.com/opencontainers/runc/libcontainer/intelrdt"
 )
 
 func TestValidateIntelRdt(t *testing.T) {
-	// Call init to trigger the sync.Once and enable overriding the rdt status
-	intelRdt.init()
-
 	testCases := []struct {
 		name       string
 		rdtEnabled bool
@@ -90,11 +88,17 @@ func TestValidateIntelRdt(t *testing.T) {
 			},
 		},
 	}
+
 	for _, tc := range testCases {
 		t.Run(tc.name, func(t *testing.T) {
-			intelRdt.rdtEnabled = tc.rdtEnabled
-			intelRdt.catEnabled = tc.catEnabled
-			intelRdt.mbaEnabled = tc.mbaEnabled
+			t.Cleanup(func() {
+				intelRdtIsEnabled = intelrdt.IsEnabled
+				intelRdtIsCATEnabled = intelrdt.IsCATEnabled
+				intelRdtIsMBAEnabled = intelrdt.IsMBAEnabled
+			})
+			intelRdtIsEnabled = func() bool { return tc.rdtEnabled }
+			intelRdtIsCATEnabled = func() bool { return tc.catEnabled }
+			intelRdtIsMBAEnabled = func() bool { return tc.mbaEnabled }
 
 			config := &configs.Config{
 				Rootfs:   "/var",

libcontainer/configs/validate/validator.go

@@ -306,22 +306,22 @@ func sysctl(config *configs.Config) error {
 }
 
 func intelrdtCheck(config *configs.Config) error {
-	if config.IntelRdt != nil {
-		if !intelRdt.isEnabled() {
-			return fmt.Errorf("intelRdt is specified in config, but Intel RDT is not enabled")
-		}
+	if config.IntelRdt == nil {
+		return nil
+	}
+	if !intelRdtIsEnabled() {
+		return fmt.Errorf("intelRdt is specified in config, but Intel RDT is not enabled")
+	}
 
-		switch clos := config.IntelRdt.ClosID; {
-		case clos == ".", clos == "..", len(clos) > 1 && strings.Contains(clos, "/"):
-			return fmt.Errorf("invalid intelRdt.ClosID %q", clos)
-		}
-
-		if !intelRdt.isCATEnabled() && config.IntelRdt.L3CacheSchema != "" {
-			return errors.New("intelRdt.l3CacheSchema is specified in config, but Intel RDT/CAT is not enabled")
-		}
-		if !intelRdt.isMBAEnabled() && config.IntelRdt.MemBwSchema != "" {
-			return errors.New("intelRdt.memBwSchema is specified in config, but Intel RDT/MBA is not enabled")
-		}
+	switch clos := config.IntelRdt.ClosID; {
+	case clos == ".", clos == "..", len(clos) > 1 && strings.Contains(clos, "/"):
+		return fmt.Errorf("invalid intelRdt.ClosID %q", clos)
+	}
+	if !intelRdtIsCATEnabled() && config.IntelRdt.L3CacheSchema != "" {
+		return errors.New("intelRdt.l3CacheSchema is specified in config, but Intel RDT/CAT is not enabled")
+	}
+	if !intelRdtIsMBAEnabled() && config.IntelRdt.MemBwSchema != "" {
+		return errors.New("intelRdt.memBwSchema is specified in config, but Intel RDT/MBA is not enabled")
 	}
 
 	return nil

libcontainer/intelrdt/intelrdt.go

@@ -162,7 +162,7 @@ func NewManager(config *configs.Config, id, path string) *Manager {
 		return nil
 	}
 
-	rootPath, err := Root()
+	rootPath, err := root()
 	if err != nil {
 		return nil
 	}
@@ -180,12 +180,6 @@ func NewManager(config *configs.Config, id, path string) *Manager {
 		}
 	}
 
-	return newManager(config, id, path)
-}
-
-// newManager is the same as NewManager, except it does not check if the feature
-// is actually available. Used by unit tests that mock intelrdt paths.
-func newManager(config *configs.Config, id, path string) *Manager {
 	return &Manager{
 		config: config,
 		id:     id,
@@ -203,47 +197,42 @@ var (
 	// The flag to indicate if Intel RDT/MBA is enabled
 	mbaEnabled bool
 
-	// For Intel RDT initialization
-	initOnce sync.Once
-
 	errNotFound = errors.New("Intel RDT not available")
 )
 
 // Check if Intel RDT sub-features are enabled in featuresInit()
-func featuresInit() {
-	initOnce.Do(func() {
-		// 1. Check if Intel RDT "resource control" filesystem is available.
-		// The user guarantees to mount the filesystem.
-		root, err := Root()
-		if err != nil {
-			return
-		}
+var featuresInit = sync.OnceFunc(func() {
+	// 1. Check if Intel RDT "resource control" filesystem is available.
+	// The user guarantees to mount the filesystem.
+	root, err := root()
+	if err != nil {
+		return
+	}
 
-		// 2. Check if Intel RDT sub-features are available in "resource
-		// control" filesystem. Intel RDT sub-features can be
-		// selectively disabled or enabled by kernel command line
-		// (e.g., rdt=!l3cat,mba) in 4.14 and newer kernel
-		if _, err := os.Stat(filepath.Join(root, "info", "L3")); err == nil {
-			catEnabled = true
-		}
-		if _, err := os.Stat(filepath.Join(root, "info", "MB")); err == nil {
-			mbaEnabled = true
-		}
-		if _, err := os.Stat(filepath.Join(root, "info", "L3_MON")); err != nil {
-			return
-		}
-		enabledMonFeatures, err = getMonFeatures(root)
-		if err != nil {
-			return
-		}
-		if enabledMonFeatures.mbmTotalBytes || enabledMonFeatures.mbmLocalBytes {
-			mbmEnabled = true
-		}
-		if enabledMonFeatures.llcOccupancy {
-			cmtEnabled = true
-		}
-	})
-}
+	// 2. Check if Intel RDT sub-features are available in "resource
+	// control" filesystem. Intel RDT sub-features can be
+	// selectively disabled or enabled by kernel command line
+	// (e.g., rdt=!l3cat,mba) in 4.14 and newer kernel
+	if _, err := os.Stat(filepath.Join(root, "info", "L3")); err == nil {
+		catEnabled = true
+	}
+	if _, err := os.Stat(filepath.Join(root, "info", "MB")); err == nil {
+		mbaEnabled = true
+	}
+	if _, err := os.Stat(filepath.Join(root, "info", "L3_MON")); err != nil {
+		return
+	}
+	enabledMonFeatures, err = getMonFeatures(root)
+	if err != nil {
+		return
+	}
+	if enabledMonFeatures.mbmTotalBytes || enabledMonFeatures.mbmLocalBytes {
+		mbmEnabled = true
+	}
+	if enabledMonFeatures.llcOccupancy {
+		cmtEnabled = true
+	}
+})
 
 // findIntelRdtMountpointDir returns the mount point of the Intel RDT "resource control" filesystem.
 func findIntelRdtMountpointDir() (string, error) {
@@ -264,13 +253,6 @@ func findIntelRdtMountpointDir() (string, error) {
 	return mi[0].Mountpoint, nil
 }
 
-// For Root() use only.
-var (
-	intelRdtRoot    string
-	intelRdtRootErr error
-	rootOnce        sync.Once
-)
-
 // The kernel creates this (empty) directory if resctrl is supported by the
 // hardware and kernel. The user is responsible for mounting the resctrl
 // filesystem, and they could mount it somewhere else if they wanted to.
@@ -278,30 +260,28 @@ const defaultResctrlMountpoint = "/sys/fs/resctrl"
 
 // Root returns the Intel RDT "resource control" filesystem mount point.
 func Root() (string, error) {
-	rootOnce.Do(func() {
-		// Does this system support resctrl?
-		var statfs unix.Statfs_t
-		if err := unix.Statfs(defaultResctrlMountpoint, &statfs); err != nil {
-			if errors.Is(err, unix.ENOENT) {
-				err = errNotFound
-			}
-			intelRdtRootErr = err
-			return
-		}
-
-		// Has the resctrl fs been mounted to the default mount point?
-		if statfs.Type == unix.RDTGROUP_SUPER_MAGIC {
-			intelRdtRoot = defaultResctrlMountpoint
-			return
-		}
-
-		// The resctrl fs could have been mounted somewhere nonstandard.
-		intelRdtRoot, intelRdtRootErr = findIntelRdtMountpointDir()
-	})
-
-	return intelRdtRoot, intelRdtRootErr
+	return root()
 }
 
+var root = sync.OnceValues(func() (string, error) {
+	// Does this system support resctrl?
+	var statfs unix.Statfs_t
+	if err := unix.Statfs(defaultResctrlMountpoint, &statfs); err != nil {
+		if errors.Is(err, unix.ENOENT) {
+			err = errNotFound
+		}
+		return "", err
+	}
+
+	// Has the resctrl fs been mounted to the default mount point?
+	if statfs.Type == unix.RDTGROUP_SUPER_MAGIC {
+		return defaultResctrlMountpoint, nil
+	}
+
+	// The resctrl fs could have been mounted somewhere nonstandard.
+	return findIntelRdtMountpointDir()
+})
+
 // Gets a single uint64 value from the specified file.
 func getIntelRdtParamUint(path, file string) (uint64, error) {
 	fileName := filepath.Join(path, file)
@@ -331,7 +311,7 @@ func getIntelRdtParamString(path, file string) (string, error) {
 func getL3CacheInfo() (*L3CacheInfo, error) {
 	l3CacheInfo := &L3CacheInfo{}
 
-	rootPath, err := Root()
+	rootPath, err := root()
 	if err != nil {
 		return l3CacheInfo, err
 	}
@@ -361,7 +341,7 @@ func getL3CacheInfo() (*L3CacheInfo, error) {
 func getMemBwInfo() (*MemBwInfo, error) {
 	memBwInfo := &MemBwInfo{}
 
-	rootPath, err := Root()
+	rootPath, err := root()
 	if err != nil {
 		return memBwInfo, err
 	}
@@ -394,7 +374,7 @@ func getMemBwInfo() (*MemBwInfo, error) {
 
 // Get diagnostics for last filesystem operation error from file info/last_cmd_status
 func getLastCmdStatus() (string, error) {
-	rootPath, err := Root()
+	rootPath, err := root()
 	if err != nil {
 		return "", err
 	}
@@ -425,7 +405,7 @@ func WriteIntelRdtTasks(dir string, pid int) error {
 
 // IsEnabled checks if Intel RDT is enabled.
 func IsEnabled() bool {
-	fsroot, err := Root()
+	fsroot, err := root()
 	return err == nil && fsroot != ""
 }
 
@@ -544,7 +524,7 @@ func (m *Manager) GetStats() (*Stats, error) {
 	defer m.mu.Unlock()
 	stats := newStats()
 
-	rootPath, err := Root()
+	rootPath, err := root()
 	if err != nil {
 		return nil, err
 	}

libcontainer/intelrdt/intelrdt_test.go

@@ -105,15 +105,20 @@ func TestIntelRdtSet(t *testing.T) {
 
 	for _, tc := range tcs {
 		t.Run(tc.name, func(t *testing.T) {
-			helper := NewIntelRdtTestUtil(t)
-			helper.config.IntelRdt = tc.config
+			intelRdtRoot := fakeRoot(t)
+			config := &configs.Config{
+				IntelRdt: tc.config,
+			}
 
-			intelrdt := newManager(helper.config, "", helper.IntelRdtPath)
-			if err := intelrdt.Set(helper.config); err != nil {
+			intelrdt := &Manager{
+				config: config,
+				path:   intelRdtRoot,
+			}
+			if err := intelrdt.Set(config); err != nil {
 				t.Fatal(err)
 			}
 
-			tmpStrings, err := getIntelRdtParamString(helper.IntelRdtPath, "schemata")
+			tmpStrings, err := getIntelRdtParamString(intelRdtRoot, "schemata")
 			if err != nil {
 				t.Fatalf("Failed to parse file 'schemata' - %s", err)
 			}
@@ -183,7 +188,7 @@ func TestApply(t *testing.T) {
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			NewIntelRdtTestUtil(t)
+			intelRdtRoot := fakeRoot(t)
 			id := "abcd-1234"
 			closPath := filepath.Join(intelRdtRoot, id)
 			if tt.config.ClosID != "" {
@@ -195,7 +200,11 @@ func TestApply(t *testing.T) {
 					t.Fatal(err)
 				}
 			}
-			m := newManager(&configs.Config{IntelRdt: &tt.config}, id, closPath)
+			m := &Manager{
+				config: &configs.Config{IntelRdt: &tt.config},
+				id:     id,
+				path:   closPath,
+			}
 			err := m.Apply(pid)
 			if tt.isError && err == nil {
 				t.Fatal("expected error, got nil")
@@ -281,8 +290,7 @@ func TestDestroy(t *testing.T) {
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			NewIntelRdtTestUtil(t)
-
+			intelRdtRoot := fakeRoot(t)
 			id := "abcd-1234"
 			closPath := filepath.Join(intelRdtRoot, id)
 			if tt.config.ClosID != "" {
@@ -292,7 +300,11 @@ func TestDestroy(t *testing.T) {
 					t.Fatal(err)
 				}
 			}
-			m := newManager(&configs.Config{IntelRdt: &tt.config}, id, closPath)
+			m := &Manager{
+				config: &configs.Config{IntelRdt: &tt.config},
+				id:     id,
+				path:   closPath,
+			}
 			if err := m.Apply(1234); err != nil {
 				t.Fatalf("Apply() failed: %v", err)
 			}

libcontainer/intelrdt/util_test.go

@@ -8,35 +8,25 @@ import (
 	"os"
 	"path/filepath"
 	"testing"
-
-	"github.com/opencontainers/runc/libcontainer/configs"
 )
 
-type intelRdtTestUtil struct {
-	config *configs.Config
-
-	// Path to the mock Intel RDT "resource control" filesystem directory
-	IntelRdtPath string
-
-	t *testing.T
-}
-
-// Creates a new test util
-func NewIntelRdtTestUtil(t *testing.T) *intelRdtTestUtil {
-	config := &configs.Config{
-		IntelRdt: &configs.IntelRdt{},
-	}
-
+// fakeRoot creates a new fake root for tests and returns its path.
+// Once this is called, Root() returns the same path.
+func fakeRoot(t *testing.T) string {
 	// Assign fake intelRtdRoot value, returned by Root().
-	intelRdtRoot = t.TempDir()
-	// Make sure Root() won't even try to parse mountinfo.
-	rootOnce.Do(func() {})
-
-	testIntelRdtPath := filepath.Join(intelRdtRoot, "resctrl")
-
-	// Ensure the full mock Intel RDT "resource control" filesystem path exists
-	if err := os.MkdirAll(testIntelRdtPath, 0o755); err != nil {
+	intelRdtRoot := filepath.Join(t.TempDir(), "resctrl")
+	if err := os.MkdirAll(intelRdtRoot, 0o755); err != nil {
 		t.Fatal(err)
 	}
-	return &intelRdtTestUtil{config: config, IntelRdtPath: testIntelRdtPath, t: t}
+
+	origRoot := root
+	t.Cleanup(func() {
+		root = origRoot
+	})
+
+	root = func() (string, error) {
+		return intelRdtRoot, nil
+	}
+
+	return intelRdtRoot
 }

libcontainer/utils/utils_unix.go

@@ -18,29 +18,21 @@ import (
 	"github.com/opencontainers/runc/internal/pathrs"
 )
 
-var (
-	haveCloseRangeCloexecBool bool
-	haveCloseRangeCloexecOnce sync.Once
-)
+var haveCloseRangeCloexec = sync.OnceValue(func() bool {
+	// Make sure we're not closing a random file descriptor.
+	tmpFd, err := unix.FcntlInt(0, unix.F_DUPFD_CLOEXEC, 0)
+	if err != nil {
+		return false
+	}
+	defer unix.Close(tmpFd)
 
-func haveCloseRangeCloexec() bool {
-	haveCloseRangeCloexecOnce.Do(func() {
-		// Make sure we're not closing a random file descriptor.
-		tmpFd, err := unix.FcntlInt(0, unix.F_DUPFD_CLOEXEC, 0)
-		if err != nil {
-			return
-		}
-		defer unix.Close(tmpFd)
-
-		err = unix.CloseRange(uint(tmpFd), uint(tmpFd), unix.CLOSE_RANGE_CLOEXEC)
-		// Any error means we cannot use close_range(CLOSE_RANGE_CLOEXEC).
-		// -ENOSYS and -EINVAL ultimately mean we don't have support, but any
-		// other potential error would imply that even the most basic close
-		// operation wouldn't work.
-		haveCloseRangeCloexecBool = err == nil
-	})
-	return haveCloseRangeCloexecBool
-}
+	err = unix.CloseRange(uint(tmpFd), uint(tmpFd), unix.CLOSE_RANGE_CLOEXEC)
+	// Any error means we cannot use close_range(CLOSE_RANGE_CLOEXEC).
+	// -ENOSYS and -EINVAL ultimately mean we don't have support, but any
+	// other potential error would imply that even the most basic close
+	// operation wouldn't work.
+	return err == nil
+})
 
 type fdFunc func(fd int)
 
@@ -162,10 +154,13 @@ func WithProcfdFile(file *os.File, fn func(procfd string) error) error {
 
 type ProcThreadSelfCloser func()
 
-var (
-	haveProcThreadSelf     bool
-	haveProcThreadSelfOnce sync.Once
-)
+var haveProcThreadSelf = sync.OnceValue(func() bool {
+	if _, err := os.Stat("/proc/thread-self/"); err != nil {
+		logrus.Debugf("cannot stat /proc/thread-self (%v), falling back to /proc/self/task/<tid>", err)
+		return false
+	}
+	return true
+})
 
 // ProcThreadSelf returns a string that is equivalent to
 // /proc/thread-self/<subpath>, with a graceful fallback on older kernels where
@@ -174,14 +169,6 @@ var (
 // the returned procThreadSelfCloser function (which is runtime.UnlockOSThread)
 // *only once* after it has finished using the returned path string.
 func ProcThreadSelf(subpath string) (string, ProcThreadSelfCloser) {
-	haveProcThreadSelfOnce.Do(func() {
-		if _, err := os.Stat("/proc/thread-self/"); err == nil {
-			haveProcThreadSelf = true
-		} else {
-			logrus.Debugf("cannot stat /proc/thread-self (%v), falling back to /proc/self/task/<tid>", err)
-		}
-	})
-
 	// We need to lock our thread until the caller is done with the path string
 	// because any non-atomic operation on the path (such as opening a file,
 	// then reading it) could be interrupted by the Go runtime where the
@@ -196,7 +183,7 @@ func ProcThreadSelf(subpath string) (string, ProcThreadSelfCloser) {
 	runtime.LockOSThread()
 
 	threadSelf := "/proc/thread-self/"
-	if !haveProcThreadSelf {
+	if !haveProcThreadSelf() {
 		// Pre-3.17 kernels did not have /proc/thread-self, so do it manually.
 		threadSelf = "/proc/self/task/" + strconv.Itoa(unix.Gettid()) + "/"
 		if _, err := os.Stat(threadSelf); err != nil {