Merge pull request #108568 from stevekuznetsov/skuznets/verbose-error

kubelet: cgroups: be verbose about validation

pkg/kubelet/cm/cgroup_manager_linux.go

@@ -253,21 +253,20 @@ func updateSystemdCgroupInfo(cgroupConfig *libcontainerconfigs.Cgroup, cgroupNam
 	cgroupConfig.Name = base
 }
 
-// Exists checks if all subsystem cgroups already exist
+// Validate checks if all subsystem cgroups already exist
-func (m *cgroupManagerImpl) Exists(name CgroupName) bool {
+func (m *cgroupManagerImpl) Validate(name CgroupName) error {
 	if libcontainercgroups.IsCgroup2UnifiedMode() {
 		cgroupPath := m.buildCgroupUnifiedPath(name)
 		neededControllers := getSupportedUnifiedControllers()
 		enabledControllers, err := readUnifiedControllers(cgroupPath)
 		if err != nil {
-			return false
+			return fmt.Errorf("could not read controllers for cgroup %q: %w", name, err)
 		}
 		difference := neededControllers.Difference(enabledControllers)
 		if difference.Len() > 0 {
-			klog.V(4).InfoS("The cgroup has some missing controllers", "cgroupName", name, "controllers", difference)
+			return fmt.Errorf("cgroup %q has some missing controllers: %v", name, strings.Join(difference.List(), ", "))
-			return false
 		}
-		return true
+		return nil // valid V2 cgroup
 	}
 
 	// Get map of all cgroup paths on the system for the particular cgroup
@@ -297,11 +296,15 @@ func (m *cgroupManagerImpl) Exists(name CgroupName) bool {
 	}
 
 	if len(missingPaths) > 0 {
-		klog.V(4).InfoS("The cgroup has some missing paths", "cgroupName", name, "paths", missingPaths)
+		return fmt.Errorf("cgroup %q has some missing paths: %v", name, strings.Join(missingPaths, ", "))
-		return false
 	}
 
-	return true
+	return nil // valid V1 cgroup
+}
+
+// Exists checks if all subsystem cgroups already exist
+func (m *cgroupManagerImpl) Exists(name CgroupName) bool {
+	return m.Validate(name) == nil
 }
 
 // Destroy destroys the specified cgroup

pkg/kubelet/cm/cgroup_manager_unsupported.go

@@ -41,6 +41,10 @@ func (m *unsupportedCgroupManager) Name(_ CgroupName) string {
 	return ""
 }
 
+func (m *unsupportedCgroupManager) Validate(_ CgroupName) error {
+	return errNotSupported
+}
+
 func (m *unsupportedCgroupManager) Exists(_ CgroupName) bool {
 	return false
 }

pkg/kubelet/cm/container_manager_linux.go

@@ -257,8 +257,8 @@ func NewContainerManager(mountUtil mount.Interface, cadvisorInterface cadvisor.I
 		// of note, we always use the cgroupfs driver when performing this check since
 		// the input is provided in that format.
 		// this is important because we do not want any name conversion to occur.
-		if !cgroupManager.Exists(cgroupRoot) {
+		if err := cgroupManager.Validate(cgroupRoot); err != nil {
-			return nil, fmt.Errorf("invalid configuration: cgroup-root %q doesn't exist", cgroupRoot)
+			return nil, fmt.Errorf("invalid configuration: %w", err)
 		}
 		klog.InfoS("Container manager verified user specified cgroup-root exists", "cgroupRoot", cgroupRoot)
 		// Include the top level cgroup for enforcing node allocatable into cgroup-root.

pkg/kubelet/cm/node_container_manager_linux.go

@@ -156,8 +156,8 @@ func enforceExistingCgroup(cgroupManager CgroupManager, cName CgroupName, rl v1.
 		return fmt.Errorf("%q cgroup is not config properly", cgroupConfig.Name)
 	}
 	klog.V(4).InfoS("Enforcing limits on cgroup", "cgroupName", cName, "cpuShares", cgroupConfig.ResourceParameters.CpuShares, "memory", cgroupConfig.ResourceParameters.Memory, "pidsLimit", cgroupConfig.ResourceParameters.PidsLimit)
-	if !cgroupManager.Exists(cgroupConfig.Name) {
+	if err := cgroupManager.Validate(cgroupConfig.Name); err != nil {
-		return fmt.Errorf("%q cgroup does not exist", cgroupConfig.Name)
+		return err
 	}
 	if err := cgroupManager.Update(cgroupConfig); err != nil {
 		return err

pkg/kubelet/cm/types.go

@@ -66,6 +66,8 @@ type CgroupManager interface {
 	Destroy(*CgroupConfig) error
 	// Update cgroup configuration.
 	Update(*CgroupConfig) error
+	// Validate checks if the cgroup is valid
+	Validate(name CgroupName) error
 	// Exists checks if the cgroup already exists
 	Exists(name CgroupName) bool
 	// Name returns the literal cgroupfs name on the host after any driver specific conversions.