cgroups: add GetFreezerState() helper to Manager

This is effectively a nicer implementation of the container.isPaused()
helper, but to be used within the cgroup code for handling some fun
issues we have to fix with the systemd cgroup driver.

Signed-off-by: Aleksa Sarai <[email protected]>

libcontainer/cgroups/cgroups.go

@@ -44,6 +44,9 @@ type Manager interface {
 
 	// GetCgroups returns the cgroup data as configured.
 	GetCgroups() (*configs.Cgroup, error)
+
+	// GetFreezerState retrieves the current FreezerState of the cgroup.
+	GetFreezerState() (configs.FreezerState, error)
 }
 
 type NotFoundError struct {

libcontainer/cgroups/fs/apply_raw.go

@@ -166,9 +166,6 @@ func (m *manager) Apply(pid int) (err error) {
 	}
 
 	for _, sys := range m.getSubsystems() {
-		// TODO: Apply should, ideally, be reentrant or be broken up into a separate
-		// create and join phase so that the cgroup hierarchy for a container can be
-		// created then join consists of writing the process pids to cgroup.procs
 		p, err := d.path(sys.Name())
 		if err != nil {
 			// The non-presence of the devices subsystem is
@@ -181,10 +178,10 @@ func (m *manager) Apply(pid int) (err error) {
 		m.paths[sys.Name()] = p
 
 		if err := sys.Apply(d); err != nil {
-			// In the case of rootless (including euid=0 in userns), where an explicit cgroup path hasn't
-			// been set, we don't bail on error in case of permission problems.
-			// Cases where limits have been set (and we couldn't create our own
-			// cgroup) are handled by Set.
+			// In the case of rootless (including euid=0 in userns), where an
+			// explicit cgroup path hasn't been set, we don't bail on error in
+			// case of permission problems. Cases where limits have been set
+			// (and we couldn't create our own cgroup) are handled by Set.
 			if isIgnorableError(m.rootless, err) && m.cgroups.Path == "" {
 				delete(m.paths, sys.Name())
 				continue
@@ -272,22 +269,25 @@ func (m *manager) Set(container *configs.Config) error {
 
 // Freeze toggles the container's freezer cgroup depending on the state
 // provided
-func (m *manager) Freeze(state configs.FreezerState) error {
-	if m.cgroups == nil {
+func (m *manager) Freeze(state configs.FreezerState) (Err error) {
+	path := m.GetPaths()["freezer"]
+	if m.cgroups == nil || path == "" {
 		return errors.New("cannot toggle freezer: cgroups not configured for container")
 	}
 
-	paths := m.GetPaths()
-	dir := paths["freezer"]
 	prevState := m.cgroups.Resources.Freezer
 	m.cgroups.Resources.Freezer = state
+	defer func() {
+		if Err != nil {
+			m.cgroups.Resources.Freezer = prevState
+		}
+	}()
+
 	freezer, err := m.getSubsystems().Get("freezer")
 	if err != nil {
 		return err
 	}
-	err = freezer.Set(dir, m.cgroups)
-	if err != nil {
-		m.cgroups.Resources.Freezer = prevState
+	if err := freezer.Set(path, m.cgroups); err != nil {
 		return err
 	}
 	return nil
@@ -413,3 +413,15 @@ func (m *manager) GetPaths() map[string]string {
 func (m *manager) GetCgroups() (*configs.Cgroup, error) {
 	return m.cgroups, nil
 }
+
+func (m *manager) GetFreezerState() (configs.FreezerState, error) {
+	paths := m.GetPaths()
+	dir := paths["freezer"]
+	freezer, err := m.getSubsystems().Get("freezer")
+
+	// If the container doesn't have the freezer cgroup, say it's undefined.
+	if err != nil || dir == "" {
+		return configs.Undefined, nil
+	}
+	return freezer.(*FreezerGroup).GetState(dir)
+}

libcontainer/cgroups/fs/freezer.go

@@ -3,13 +3,16 @@
 package fs
 
 import (
+	"errors"
 	"fmt"
+	"os"
 	"strings"
 	"time"
 
 	"github.com/opencontainers/runc/libcontainer/cgroups"
 	"github.com/opencontainers/runc/libcontainer/cgroups/fscommon"
 	"github.com/opencontainers/runc/libcontainer/configs"
+	"golang.org/x/sys/unix"
 )
 
 type FreezerGroup struct {
@@ -39,11 +42,11 @@ func (s *FreezerGroup) Set(path string, cgroup *configs.Cgroup) error {
 				return err
 			}
 
-			state, err := fscommon.ReadFile(path, "freezer.state")
+			state, err := s.GetState(path)
 			if err != nil {
 				return err
 			}
-			if strings.TrimSpace(state) == string(cgroup.Resources.Freezer) {
+			if state == cgroup.Resources.Freezer {
 				break
 			}
 
@@ -65,3 +68,30 @@ func (s *FreezerGroup) Remove(d *cgroupData) error {
 func (s *FreezerGroup) GetStats(path string, stats *cgroups.Stats) error {
 	return nil
 }
+
+func (s *FreezerGroup) GetState(path string) (configs.FreezerState, error) {
+	for {
+		state, err := fscommon.ReadFile(path, "freezer.state")
+		if err != nil {
+			// If the kernel is too old, then we just treat the freezer as
+			// being in an "undefined" state.
+			if os.IsNotExist(err) || errors.Is(err, unix.ENODEV) {
+				err = nil
+			}
+			return configs.Undefined, err
+		}
+		switch strings.TrimSpace(state) {
+		case "THAWED":
+			return configs.Thawed, nil
+		case "FROZEN":
+			return configs.Frozen, nil
+		case "FREEZING":
+			// Make sure we get a stable freezer state, so retry if the cgroup
+			// is still undergoing freezing. This should be a temporary delay.
+			time.Sleep(1 * time.Millisecond)
+			continue
+		default:
+			return configs.Undefined, fmt.Errorf("unknown freezer.state %q", state)
+		}
+	}
+}

libcontainer/cgroups/fs2/freezer.go

@@ -3,51 +3,72 @@
 package fs2
 
 import (
-	"strconv"
+	stdErrors "errors"
+	"os"
 	"strings"
 
 	"github.com/opencontainers/runc/libcontainer/cgroups/fscommon"
 	"github.com/opencontainers/runc/libcontainer/configs"
 	"github.com/pkg/errors"
+	"golang.org/x/sys/unix"
 )
 
 func setFreezer(dirPath string, state configs.FreezerState) error {
-	var desired int
+	if err := supportsFreezer(dirPath); err != nil {
+		// We can ignore this request as long as the user didn't ask us to
+		// freeze the container (since without the freezer cgroup, that's a
+		// no-op).
+		if state == configs.Undefined || state == configs.Thawed {
+			err = nil
+		}
+		return errors.Wrap(err, "freezer not supported")
+	}
+
+	var stateStr string
 	switch state {
 	case configs.Undefined:
 		return nil
 	case configs.Frozen:
-		desired = 1
+		stateStr = "1"
 	case configs.Thawed:
-		desired = 0
+		stateStr = "0"
 	default:
-		return errors.Errorf("unknown freezer state %+v", state)
+		return errors.Errorf("invalid freezer state %q requested", state)
+	}
+
+	if err := fscommon.WriteFile(dirPath, "cgroup.freeze", stateStr); err != nil {
+		return err
 	}
-	supportedErr := supportsFreezer(dirPath)
-	if supportedErr != nil && desired != 0 {
-		// can ignore error if desired == 1
-		return errors.Wrap(supportedErr, "freezer not supported")
+	// Confirm that the cgroup did actually change states.
+	if actualState, err := getFreezer(dirPath); err != nil {
+		return err
+	} else if actualState != state {
+		return errors.Errorf(`expected "cgroup.freeze" to be in state %q but was in %q`, state, actualState)
 	}
-	return freezeWithInt(dirPath, desired)
+	return nil
 }
 
 func supportsFreezer(dirPath string) error {
 	_, err := fscommon.ReadFile(dirPath, "cgroup.freeze")
 	return err
 }
 
-// freeze writes desired int to "cgroup.freeze".
-func freezeWithInt(dirPath string, desired int) error {
-	desiredS := strconv.Itoa(desired)
-	if err := fscommon.WriteFile(dirPath, "cgroup.freeze", desiredS); err != nil {
-		return err
-	}
-	got, err := fscommon.ReadFile(dirPath, "cgroup.freeze")
+func getFreezer(dirPath string) (configs.FreezerState, error) {
+	state, err := fscommon.ReadFile(dirPath, "cgroup.freeze")
 	if err != nil {
-		return err
+		// If the kernel is too old, then we just treat the freezer as being in
+		// an "undefined" state.
+		if os.IsNotExist(err) || stdErrors.Is(err, unix.ENODEV) {
+			err = nil
+		}
+		return configs.Undefined, err
 	}
-	if gotS := strings.TrimSpace(string(got)); gotS != desiredS {
-		return errors.Errorf("expected \"cgroup.freeze\" in %q to be %q, got %q", dirPath, desiredS, gotS)
+	switch strings.TrimSpace(state) {
+	case "0":
+		return configs.Thawed, nil
+	case "1":
+		return configs.Frozen, nil
+	default:
+		return configs.Undefined, errors.Errorf(`unknown "cgroup.freeze" state: %q`, state)
 	}
-	return nil
 }

libcontainer/cgroups/fs2/fs2.go

@@ -240,3 +240,7 @@ func (m *manager) GetPaths() map[string]string {
 func (m *manager) GetCgroups() (*configs.Cgroup, error) {
 	return m.config, nil
 }
+
+func (m *manager) GetFreezerState() (configs.FreezerState, error) {
+	return getFreezer(m.dirPath)
+}

libcontainer/cgroups/systemd/v1.go

@@ -439,3 +439,15 @@ func (m *legacyManager) GetPaths() map[string]string {
 func (m *legacyManager) GetCgroups() (*configs.Cgroup, error) {
 	return m.cgroups, nil
 }
+
+func (m *legacyManager) GetFreezerState() (configs.FreezerState, error) {
+	path, err := getSubsystemPath(m.cgroups, "freezer")
+	if err != nil && !cgroups.IsNotFound(err) {
+		return configs.Undefined, err
+	}
+	freezer, err := legacySubsystems.Get("freezer")
+	if err != nil {
+		return configs.Undefined, err
+	}
+	return freezer.(*fs.FreezerGroup).GetState(path)
+}

libcontainer/cgroups/systemd/v2.go

@@ -319,3 +319,11 @@ func (m *unifiedManager) GetPaths() map[string]string {
 func (m *unifiedManager) GetCgroups() (*configs.Cgroup, error) {
 	return m.cgroups, nil
 }
+
+func (m *unifiedManager) GetFreezerState() (configs.FreezerState, error) {
+	fsMgr, err := m.fsManager()
+	if err != nil {
+		return configs.Undefined, err
+	}
+	return fsMgr.GetFreezerState()
+}

libcontainer/container_linux.go

@@ -1847,30 +1847,11 @@ func (c *linuxContainer) runType() Status {
 }
 
 func (c *linuxContainer) isPaused() (bool, error) {
-	var filename, pausedState string
-
-	fcg := c.cgroupManager.Path("freezer")
-	if !cgroups.IsCgroup2UnifiedMode() {
-		if fcg == "" {
-			// container doesn't have a freezer cgroup
-			return false, nil
-		}
-		filename = "freezer.state"
-		pausedState = "FROZEN"
-	} else {
-		filename = "cgroup.freeze"
-		pausedState = "1"
-	}
-
-	data, err := ioutil.ReadFile(filepath.Join(fcg, filename))
+	state, err := c.cgroupManager.GetFreezerState()
 	if err != nil {
-		// If freezer cgroup is not mounted, the container would just be not paused.
-		if os.IsNotExist(err) || errors.Is(err, unix.ENODEV) {
-			return false, nil
-		}
-		return false, newSystemErrorWithCause(err, "checking if container is paused")
+		return false, err
 	}
-	return bytes.Equal(bytes.TrimSpace(data), []byte(pausedState)), nil
+	return state == configs.Frozen, nil
 }
 
 func (c *linuxContainer) currentState() (*State, error) {

libcontainer/container_linux_test.go

@@ -61,10 +61,15 @@ func (m *mockCgroupManager) Path(subsys string) string {
 func (m *mockCgroupManager) Freeze(state configs.FreezerState) error {
 	return nil
 }
+
 func (m *mockCgroupManager) GetCgroups() (*configs.Cgroup, error) {
 	return nil, nil
 }
 
+func (m *mockCgroupManager) GetFreezerState() (configs.FreezerState, error) {
+	return configs.Thawed, nil
+}
+
 func (m *mockIntelRdtManager) Apply(pid int) error {
 	return nil
 }