pkg/volume: Use QuotaMonitoring in UserNamespace

Enable LocalStorageCapacityIsolationFSQuotaMonitoring
only when hostUsers in PodSpec is set to false.
Modify unit tests and e2e tests to verify

Signed-off-by: PannagaRamamanohara <pbhojara@redhat.com>
This commit is contained in:
PannagaRamamanohara 2024-07-11 13:41:28 -04:00
parent 03fe89c233
commit d16fd6a915
6 changed files with 83 additions and 48 deletions

View File

@ -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)
}

View File

@ -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)
if err := fsquota.AssignQuota(ed.mounter, dir, ed.pod.UID, mounterSize); err != nil {
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, 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)
}

View File

@ -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 {

View File

@ -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":

View File

@ -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
}

View File

@ -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, false)
runOneQuotaTest(f, true, true)
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 {
supportsQuota, err := fsquota.SupportsQuotas(mount.New(""), dir)
func supportsQuotas(dir string, userNamespacesEnabled bool) bool {
supportsQuota, err := fsquota.SupportsQuotas(mount.New(""), dir, userNamespacesEnabled)
return supportsQuota && err == nil
}