Merge pull request #126014 from PannagaRao/kep-ephemeral-storage-quota

pkg/volume/*: Enable quotas in user namespace

pkg/features/kube_features.go

@@ -1087,7 +1087,7 @@ var defaultKubernetesFeatureGates = map[featuregate.Feature]featuregate.FeatureS
 
 	LegacyServiceAccountTokenCleanUp: {Default: true, PreRelease: featuregate.GA, LockToDefault: true}, // GA in 1.30; remove in 1.32
 
-	LocalStorageCapacityIsolationFSQuotaMonitoring: {Default: false, PreRelease: featuregate.Alpha},
+	LocalStorageCapacityIsolationFSQuotaMonitoring: {Default: true, PreRelease: featuregate.Beta},
 
 	LogarithmicScaleDown: {Default: true, PreRelease: featuregate.GA, LockToDefault: true},
 

pkg/kubelet/userns/userns_manager.go

@@ -386,6 +386,8 @@ func (m *UsernsManager) createUserNs(pod *v1.Pod) (userNs userNamespace, err err
 func (m *UsernsManager) GetOrCreateUserNamespaceMappings(pod *v1.Pod, runtimeHandler string) (*runtimeapi.UserNamespace, error) {
 	featureEnabled := utilfeature.DefaultFeatureGate.Enabled(features.UserNamespacesSupport)
 
+	// TODO: If the default value for hostUsers ever changes, change the default value of
+	// userNamespacesEnabled as well
 	if pod == nil || pod.Spec.HostUsers == nil {
 		// if the feature is enabled, specify to use the node mode...
 		if featureEnabled {
@@ -512,3 +514,7 @@ func (m *UsernsManager) CleanupOrphanedPodUsernsAllocations(pods []*v1.Pod, runn
 
 	return nil
 }
+
+func EnabledUserNamespacesSupport() bool {
+	return utilfeature.DefaultFeatureGate.Enabled(features.UserNamespacesSupport)
+}

pkg/volume/emptydir/empty_dir.go

@@ -34,6 +34,7 @@ import (
 	v1helper "k8s.io/kubernetes/pkg/apis/core/v1/helper"
 	"k8s.io/kubernetes/pkg/features"
 	"k8s.io/kubernetes/pkg/kubelet/cm"
+	usernamespacefeature "k8s.io/kubernetes/pkg/kubelet/userns"
 	"k8s.io/kubernetes/pkg/volume"
 	volumeutil "k8s.io/kubernetes/pkg/volume/util"
 	"k8s.io/kubernetes/pkg/volume/util/fsquota"
@@ -294,12 +295,19 @@ func (ed *emptyDir) assignQuota(dir string, mounterSize *resource.Quantity) erro
 	if mounterSize != nil {
 		// Deliberately shadow the outer use of err as noted
 		// above.
-		hasQuotas, err := fsquota.SupportsQuotas(ed.mounter, dir)
+		// hostUsers field is interpreted as true by default, so a pod is by default
+		// not confined by a user namespace.
+		userNamespacesEnabled := false
+		if usernamespacefeature.EnabledUserNamespacesSupport() {
+			userNamespacesEnabled = ed.pod.Spec.HostUsers != nil && !*ed.pod.Spec.HostUsers
+		}
+		hasQuotas, err := fsquota.SupportsQuotas(ed.mounter, dir, userNamespacesEnabled)
+		klog.V(3).Infof("assignQuota called, hasQuotas = %t userNamespacesEnabled = %t", hasQuotas, userNamespacesEnabled)
 		if err != nil {
 			klog.V(3).Infof("Unable to check for quota support on %s: %s", dir, err.Error())
 		} else if hasQuotas {
-			klog.V(4).Infof("emptydir trying to assign quota %v on %s", mounterSize, dir)
+			klog.V(3).Infof("emptydir trying to assign quota %v on %s", mounterSize, dir)
-			if err := fsquota.AssignQuota(ed.mounter, dir, ed.pod.UID, mounterSize); err != nil {
+			if err := fsquota.AssignQuota(ed.mounter, dir, ed.pod.UID, mounterSize, userNamespacesEnabled); err != nil {
 				klog.V(3).Infof("Set quota on %s failed %s", dir, err.Error())
 				return err
 			}
@@ -514,7 +522,11 @@ func (ed *emptyDir) TearDownAt(dir string) error {
 func (ed *emptyDir) teardownDefault(dir string) error {
 	if utilfeature.DefaultFeatureGate.Enabled(features.LocalStorageCapacityIsolationFSQuotaMonitoring) {
 		// Remove any quota
-		err := fsquota.ClearQuota(ed.mounter, dir)
+		userNamespacesEnabled := false
+		if usernamespacefeature.EnabledUserNamespacesSupport() {
+			userNamespacesEnabled = ed.pod.Spec.HostUsers != nil && !*ed.pod.Spec.HostUsers
+		}
+		err := fsquota.ClearQuota(ed.mounter, dir, userNamespacesEnabled)
 		if err != nil {
 			klog.Warningf("Warning: Failed to clear quota on %s: %v", dir, err)
 		}

pkg/volume/util/fsquota/quota_linux.go

@@ -225,11 +225,11 @@ func GetQuotaOnDir(m mount.Interface, path string) (common.QuotaID, error) {
 	return getApplier(path).GetQuotaOnDir(path)
 }
 
-func clearQuotaOnDir(m mount.Interface, path string) error {
+func clearQuotaOnDir(m mount.Interface, path string, userNamespacesEnabled bool) error {
 	// Since we may be called without path being in the map,
 	// we explicitly have to check in this case.
 	klog.V(4).Infof("clearQuotaOnDir %s", path)
-	supportsQuotas, err := SupportsQuotas(m, path)
+	supportsQuotas, err := SupportsQuotas(m, path, userNamespacesEnabled)
 	if err != nil {
 		// Log-and-continue instead of returning an error for now
 		// due to unspecified backwards compatibility concerns (a subject to revise)
@@ -269,11 +269,17 @@ func clearQuotaOnDir(m mount.Interface, path string) error {
 // don't cache the result because nothing will clean it up.
 // However, do cache the device->applier map; the number of devices
 // is bounded.
-func SupportsQuotas(m mount.Interface, path string) (bool, error) {
+// User namespaces prevent changes to project IDs on the filesystem,
+// ensuring xfs-quota metrics' reliability; hence, userNamespacesEnabled is checked.
+func SupportsQuotas(m mount.Interface, path string, userNamespacesEnabled bool) (bool, error) {
 	if !enabledQuotasForMonitoring() {
 		klog.V(3).Info("SupportsQuotas called, but quotas disabled")
 		return false, nil
 	}
+	if !userNamespacesEnabled {
+		klog.V(3).Info("SupportQuotas called and LocalStorageCapacityIsolationFSQuotaMonitoring enabled, but pod is not in a user namespace")
+		return false, nil
+	}
 	supportsQuotasLock.Lock()
 	defer supportsQuotasLock.Unlock()
 	if supportsQuotas, ok := supportsQuotasMap[path]; ok {
@@ -307,12 +313,12 @@ func SupportsQuotas(m mount.Interface, path string) (bool, error) {
 // AssignQuota chooses the quota ID based on the pod UID and path.
 // If the pod UID is identical to another one known, it may (but presently
 // doesn't) choose the same quota ID as other volumes in the pod.
-func AssignQuota(m mount.Interface, path string, poduid types.UID, bytes *resource.Quantity) error { //nolint:staticcheck
+func AssignQuota(m mount.Interface, path string, poduid types.UID, bytes *resource.Quantity, userNamespacesEnabled bool) error { //nolint:staticcheck
 	if bytes == nil {
 		return fmt.Errorf("attempting to assign null quota to %s", path)
 	}
 	ibytes := bytes.Value()
-	if ok, err := SupportsQuotas(m, path); !ok {
+	if ok, err := SupportsQuotas(m, path, userNamespacesEnabled); !ok {
 		return fmt.Errorf("quotas not supported on %s: %v", path, err)
 	}
 	quotaLock.Lock()
@@ -410,7 +416,7 @@ func GetInodes(path string) (*resource.Quantity, error) {
 }
 
 // ClearQuota -- remove the quota assigned to a directory
-func ClearQuota(m mount.Interface, path string) error {
+func ClearQuota(m mount.Interface, path string, userNamespacesEnabled bool) error {
 	klog.V(3).Infof("ClearQuota %s", path)
 	if !enabledQuotasForMonitoring() {
 		return fmt.Errorf("clearQuota called, but quotas disabled")
@@ -426,7 +432,7 @@ func ClearQuota(m mount.Interface, path string) error {
 		// be found, which needs to be cleaned up.
 		defer delete(supportsQuotasMap, path)
 		defer clearApplier(path)
-		return clearQuotaOnDir(m, path)
+		return clearQuotaOnDir(m, path, userNamespacesEnabled)
 	}
 	_, ok = podQuotaMap[poduid]
 	if !ok {
@@ -443,7 +449,7 @@ func ClearQuota(m mount.Interface, path string) error {
 	}
 	count, ok := podDirCountMap[poduid]
 	if count <= 1 || !ok {
-		err = clearQuotaOnDir(m, path)
+		err = clearQuotaOnDir(m, path, userNamespacesEnabled)
 		// This error should be noted; we still need to clean up
 		// and otherwise handle in the same way.
 		if err != nil {

pkg/volume/util/fsquota/quota_linux_test.go

@@ -366,19 +366,19 @@ func (v testVolumeQuota) GetInodes(_ string, _ common.QuotaID) (int64, error) {
 	return 1, nil
 }
 
-func fakeSupportsQuotas(path string) (bool, error) {
+func fakeSupportsQuotas(path string, userNamespacesEnabled bool) (bool, error) {
 	dummySetFSInfo(path)
-	return SupportsQuotas(dummyQuotaTest(), path)
+	return SupportsQuotas(dummyQuotaTest(), path, userNamespacesEnabled)
 }
 
-func fakeAssignQuota(path string, poduid types.UID, bytes int64) error {
+func fakeAssignQuota(path string, poduid types.UID, bytes int64, userNamespacesEnabled bool) error {
 	dummySetFSInfo(path)
-	return AssignQuota(dummyQuotaTest(), path, poduid, resource.NewQuantity(bytes, resource.DecimalSI))
+	return AssignQuota(dummyQuotaTest(), path, poduid, resource.NewQuantity(bytes, resource.DecimalSI), userNamespacesEnabled)
 }
 
-func fakeClearQuota(path string) error {
+func fakeClearQuota(path string, userNamespacesEnabled bool) error {
 	dummySetFSInfo(path)
-	return ClearQuota(dummyQuotaTest(), path)
+	return ClearQuota(dummyQuotaTest(), path, userNamespacesEnabled)
 }
 
 type quotaTestCase struct {
@@ -391,6 +391,7 @@ type quotaTestCase struct {
 	expectedProjid                   string
 	supportsQuota                    bool
 	expectsSetQuota                  bool
+	userNamespacesEnabled            bool
 	deltaExpectedPodQuotaCount       int
 	deltaExpectedDirQuotaCount       int
 	deltaExpectedQuotaPodCount       int
@@ -444,59 +445,64 @@ volume1048581:1048581
 
 var quotaTestCases = []quotaTestCase{
 	{
-		"SupportsQuotaOnQuotaVolume",
+		"SupportsQuotaOnQuotaVolumeWithUserNamespace",
 		"/quota1/a", "", 1024, "Supports", "", "",
-		true, true, 0, 0, 0, 0, 1, 1, 0, 0, 1, 1, 1,
+		true, true, true, 0, 0, 0, 0, 1, 1, 0, 0, 1, 1, 1,
+	},
+	{
+		"SupportsQuotaOnQuotaVolumeWithoutUserNamespace",
+		"/quota1/a", "", 1024, "Supports", "", "",
+		true, true, false, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
 	},
 	{
 		"AssignQuotaFirstTime",
 		"/quota1/a", "", 1024, "Set", projects1, projid1,
-		true, true, 1, 1, 1, 1, 0, 0, 1, 1, 0, 0, 0,
+		true, true, true, 1, 1, 1, 1, 0, 0, 1, 1, 0, 0, 0,
 	},
 	{
 		"AssignQuotaFirstTime",
 		"/quota1/b", "x", 1024, "Set", projects2, projid2,
-		true, true, 1, 1, 1, 1, 0, 1, 1, 1, 1, 1, 1,
+		true, true, true, 1, 1, 1, 1, 0, 1, 1, 1, 1, 1, 1,
 	},
 	{
 		"AssignQuotaFirstTime",
 		"/quota2/b", "x", 1024, "Set", projects3, projid3,
-		true, true, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1,
+		true, true, true, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1,
 	},
 	{
 		"AssignQuotaSecondTimeWithSameSize",
 		"/quota1/b", "x", 1024, "Set", projects3, projid3,
-		true, true, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+		true, true, true, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
 	},
 	{
 		"AssignQuotaSecondTimeWithDifferentSize",
 		"/quota2/b", "x", 2048, "Set", projects3, projid3,
-		true, false, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+		true, false, true, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
 	},
 	{
 		"ClearQuotaFirstTime",
 		"/quota1/b", "", 1024, "Clear", projects4, projid4,
-		true, true, -1, -1, -1, -1, 0, -1, -1, -1, -1, -1, -1,
+		true, true, true, -1, -1, -1, -1, 0, -1, -1, -1, -1, -1, -1,
 	},
 	{
 		"SupportsQuotaOnNonQuotaVolume",
 		"/noquota/a", "", 1024, "Supports", projects4, projid4,
-		false, false, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+		false, false, true, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
 	},
 	{
 		"ClearQuotaFirstTime",
 		"/quota1/a", "", 1024, "Clear", projects5, projid5,
-		true, true, -1, -1, -1, -1, 0, -1, -1, -1, -1, -1, -1,
+		true, true, true, -1, -1, -1, -1, 0, -1, -1, -1, -1, -1, -1,
 	},
 	{
 		"ClearQuotaSecondTime",
 		"/quota1/a", "", 1024, "Clear", projects5, projid5,
-		true, false, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+		true, false, true, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
 	},
 	{
 		"ClearQuotaFirstTime",
 		"/quota2/b", "", 1024, "Clear", "", "",
-		true, true, -1, -1, -1, -1, 0, -1, -1, -1, -1, -1, -1,
+		true, true, true, -1, -1, -1, -1, 0, -1, -1, -1, -1, -1, -1,
 	},
 }
 
@@ -534,20 +540,21 @@ func runCaseEnabled(t *testing.T, testcase quotaTestCase, seq int) bool {
 	var err error
 	switch testcase.op {
 	case "Supports":
-		supports, err := fakeSupportsQuotas(testcase.path)
+		supports, err := fakeSupportsQuotas(testcase.path, testcase.userNamespacesEnabled)
 		if err != nil {
 			fail = true
 			t.Errorf("Case %v (%s, %s, %v) Got error in fakeSupportsQuotas: %v", seq, testcase.name, testcase.path, true, err)
 		}
-		if supports != testcase.supportsQuota {
+		expectedSupport := testcase.supportsQuota && testcase.userNamespacesEnabled
+		if supports != expectedSupport {
 			fail = true
-			t.Errorf("Case %v (%s, %s, %v) fakeSupportsQuotas got %v, expect %v", seq, testcase.name, testcase.path, true, supports, testcase.supportsQuota)
+			t.Errorf("Case %v (%s, %s, userNamespacesEnabled: %v) fakeSupportsQuotas got %v, expect %v", seq, testcase.name, testcase.path, testcase.userNamespacesEnabled, supports, expectedSupport)
 		}
 		return fail
 	case "Set":
-		err = fakeAssignQuota(testcase.path, testcase.poduid, testcase.bytes)
+		err = fakeAssignQuota(testcase.path, testcase.poduid, testcase.bytes, testcase.userNamespacesEnabled)
 	case "Clear":
-		err = fakeClearQuota(testcase.path)
+		err = fakeClearQuota(testcase.path, testcase.userNamespacesEnabled)
 	case "GetConsumption":
 		_, err = GetConsumption(testcase.path)
 	case "GetInodes":
@@ -571,15 +578,15 @@ func runCaseDisabled(t *testing.T, testcase quotaTestCase, seq int) bool {
 	var supports bool
 	switch testcase.op {
 	case "Supports":
-		if supports, _ = fakeSupportsQuotas(testcase.path); supports {
+		if supports, _ = fakeSupportsQuotas(testcase.path, testcase.userNamespacesEnabled); supports {
 			t.Errorf("Case %v (%s, %s, %v) supports quotas but shouldn't", seq, testcase.name, testcase.path, false)
 			return true
 		}
 		return false
 	case "Set":
-		err = fakeAssignQuota(testcase.path, testcase.poduid, testcase.bytes)
+		err = fakeAssignQuota(testcase.path, testcase.poduid, testcase.bytes, testcase.userNamespacesEnabled)
 	case "Clear":
-		err = fakeClearQuota(testcase.path)
+		err = fakeClearQuota(testcase.path, testcase.userNamespacesEnabled)
 	case "GetConsumption":
 		_, err = GetConsumption(testcase.path)
 	case "GetInodes":

pkg/volume/util/fsquota/quota_unsupported.go

@@ -39,12 +39,12 @@ func GetQuotaOnDir(_ mount.Interface, _ string) (common.QuotaID, error) {
 }
 
 // SupportsQuotas -- dummy implementation
-func SupportsQuotas(_ mount.Interface, _ string) (bool, error) {
+func SupportsQuotas(_ mount.Interface, _ string, _ bool) (bool, error) {
 	return false, errNotImplemented
 }
 
 // AssignQuota -- dummy implementation
-func AssignQuota(_ mount.Interface, _ string, _ types.UID, _ *resource.Quantity) error {
+func AssignQuota(_ mount.Interface, _ string, _ types.UID, _ *resource.Quantity, _ bool) error {
 	return errNotImplemented
 }
 
@@ -59,6 +59,6 @@ func GetInodes(_ string) (*resource.Quantity, error) {
 }
 
 // ClearQuota -- dummy implementation
-func ClearQuota(_ mount.Interface, _ string) error {
+func ClearQuota(_ mount.Interface, _ string, _ bool) error {
 	return errNotImplemented
 }

test/e2e_node/quota_lsci_test.go

@@ -43,7 +43,7 @@ const (
 	LSCIQuotaFeature = features.LocalStorageCapacityIsolationFSQuotaMonitoring
 )
 
-func runOneQuotaTest(f *framework.Framework, quotasRequested bool) {
+func runOneQuotaTest(f *framework.Framework, quotasRequested bool, userNamespacesEnabled bool) {
 	evictionTestTimeout := 10 * time.Minute
 	sizeLimit := resource.MustParse("100Mi")
 	useOverLimit := 101 /* Mb */
@@ -63,7 +63,10 @@ func runOneQuotaTest(f *framework.Framework, quotasRequested bool) {
 			defer withFeatureGate(LSCIQuotaFeature, quotasRequested)()
 			// TODO: remove hardcoded kubelet volume directory path
 			// framework.TestContext.KubeVolumeDir is currently not populated for node e2e
-			if quotasRequested && !supportsQuotas("/var/lib/kubelet") {
+			if !supportsUserNS(ctx, f) {
+				e2eskipper.Skipf("runtime does not support user namespaces")
+			}
+			if quotasRequested && !supportsQuotas("/var/lib/kubelet", userNamespacesEnabled) {
 				// No point in running this as a positive test if quotas are not
 				// enabled on the underlying filesystem.
 				e2eskipper.Skipf("Cannot run LocalStorageCapacityIsolationFSQuotaMonitoring on filesystem without project quota enabled")
@@ -98,11 +101,12 @@ func runOneQuotaTest(f *framework.Framework, quotasRequested bool) {
 // pod that creates a file, deletes it, and writes data to it.  If
 // quotas are used to monitor, it will detect this deleted-but-in-use
 // file; if du is used to monitor, it will not detect this.
-var _ = SIGDescribe("LocalStorageCapacityIsolationFSQuotaMonitoring", framework.WithSlow(), framework.WithSerial(), framework.WithDisruptive(), feature.LocalStorageCapacityIsolationQuota, nodefeature.LSCIQuotaMonitoring, func() {
+var _ = SIGDescribe("LocalStorageCapacityIsolationFSQuotaMonitoring", framework.WithSlow(), framework.WithSerial(), framework.WithDisruptive(), feature.LocalStorageCapacityIsolationQuota, nodefeature.LSCIQuotaMonitoring, nodefeature.UserNamespacesSupport, feature.UserNamespacesSupport, func() {
 	f := framework.NewDefaultFramework("localstorage-quota-monitoring-test")
 	f.NamespacePodSecurityLevel = admissionapi.LevelPrivileged
-	runOneQuotaTest(f, true)
+	runOneQuotaTest(f, true, true)
-	runOneQuotaTest(f, false)
+	runOneQuotaTest(f, true, false)
+	runOneQuotaTest(f, false, true)
 	addAfterEachForCleaningUpPods(f)
 })
 
@@ -152,7 +156,7 @@ func diskConcealingPod(name string, diskConsumedMB int, volumeSource *v1.VolumeS
 
 // Don't bother returning an error; if something goes wrong,
 // simply treat it as "no".
-func supportsQuotas(dir string) bool {
+func supportsQuotas(dir string, userNamespacesEnabled bool) bool {
-	supportsQuota, err := fsquota.SupportsQuotas(mount.New(""), dir)
+	supportsQuota, err := fsquota.SupportsQuotas(mount.New(""), dir, userNamespacesEnabled)
 	return supportsQuota && err == nil
 }