Merge pull request #112624 from pacoxu/fix-fsquota-bug

fsquota: only generate pod uuid is nil

pkg/volume/emptydir/empty_dir.go

@@ -301,12 +301,18 @@ func (ed *emptyDir) assignQuota(dir string, mounterSize *resource.Quantity) erro
 		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)
-			err := fsquota.AssignQuota(ed.mounter, dir, ed.pod.UID, mounterSize)
+			_, err := fsquota.GetQuotaOnDir(ed.mounter, dir)
 			if err != nil {
-				klog.V(3).Infof("Set quota on %s failed %s", dir, err.Error())
+				klog.V(4).Infof("Attempt to check quota on dir %s failed: %v", dir, err)
+				// this is not a fatal error so return nil
+				return nil
 			}
-			return err
+			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("Set quota on %s failed %s", dir, err.Error())
+				return err
+			}
+			return nil
 		}
 	}
 	return nil

pkg/volume/util/fsquota/common/quota_common.go

@@ -0,0 +1,28 @@
+/*
+Copyright 2023 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package common
+
+// QuotaID is generic quota identifier.
+// Data type based on quotactl(2).
+type QuotaID int32
+
+const (
+	// UnknownQuotaID -- cannot determine whether a quota is in force
+	UnknownQuotaID QuotaID = -1
+	// BadQuotaID -- Invalid quota
+	BadQuotaID QuotaID = 0
+)

pkg/volume/util/fsquota/common/quota_common_linux.go

@@ -23,17 +23,6 @@ import (
 	"regexp"
 )
 
-// QuotaID is generic quota identifier.
-// Data type based on quotactl(2).
-type QuotaID int32
-
-const (
-	// UnknownQuotaID -- cannot determine whether a quota is in force
-	UnknownQuotaID QuotaID = -1
-	// BadQuotaID -- Invalid quota
-	BadQuotaID QuotaID = 0
-)
-
 // QuotaType -- type of quota to be applied
 type QuotaType int
 

pkg/volume/util/fsquota/project.go

@@ -164,6 +164,9 @@ func readProjectFiles(projects *os.File, projid *os.File) projectsList {
 	return projectsList{parseProjFile(projects, parseProject), parseProjFile(projid, parseProjid)}
 }
 
+// findAvailableQuota finds the next available quota from the FirstQuota
+// it returns error if QuotaIDIsInUse returns error when getting quota id in use;
+// it searches at most maxUnusedQuotasToSearch(128) time
 func findAvailableQuota(path string, idMap map[common.QuotaID]bool) (common.QuotaID, error) {
 	unusedQuotasSearched := 0
 	for id := common.FirstQuota; true; id++ {
@@ -318,6 +321,7 @@ func writeProjectFiles(fProjects *os.File, fProjid *os.File, writeProjid bool, l
 	return fmt.Errorf("unable to write project files: %v", err)
 }
 
+// if ID is common.BadQuotaID, generate new project id if the dir is not in a project
 func createProjectID(path string, ID common.QuotaID) (common.QuotaID, error) {
 	quotaIDLock.Lock()
 	defer quotaIDLock.Unlock()

pkg/volume/util/fsquota/quota.go

@@ -23,10 +23,15 @@ import (
 	"k8s.io/apimachinery/pkg/types"
 	utilfeature "k8s.io/apiserver/pkg/util/feature"
 	"k8s.io/kubernetes/pkg/features"
+	"k8s.io/kubernetes/pkg/volume/util/fsquota/common"
 )
 
 // Interface -- quota interface
 type Interface interface {
+	// GetQuotaOnDir gets the quota ID (if any) that applies to
+	// this directory
+	GetQuotaOnDir(m mount.Interface, path string) (common.QuotaID, error)
+
 	// Does the path provided support quotas, and if so, what types
 	SupportsQuotas(m mount.Interface, path string) (bool, error)
 	// Assign a quota (picked by the quota mechanism) to a path,

pkg/volume/util/fsquota/quota_linux.go

@@ -214,7 +214,7 @@ func setQuotaOnDir(path string, id common.QuotaID, bytes int64) error {
 	return getApplier(path).SetQuotaOnDir(path, id, bytes)
 }
 
-func getQuotaOnDir(m mount.Interface, path string) (common.QuotaID, error) {
+func GetQuotaOnDir(m mount.Interface, path string) (common.QuotaID, error) {
 	_, _, err := getFSInfo(m, path)
 	if err != nil {
 		return common.BadQuotaID, err
@@ -235,7 +235,7 @@ func clearQuotaOnDir(m mount.Interface, path string) error {
 	if !supportsQuotas {
 		return nil
 	}
-	projid, err := getQuotaOnDir(m, path)
+	projid, err := GetQuotaOnDir(m, path)
 	if err == nil && projid != common.BadQuotaID {
 		// This means that we have a quota on the directory but
 		// we can't clear it.  That's not good.
@@ -304,7 +304,7 @@ 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 // SA4009 poduid is overwritten by design, see comment below
+func AssignQuota(m mount.Interface, path string, poduid types.UID, bytes *resource.Quantity) error { //nolint:staticcheck
 	if bytes == nil {
 		return fmt.Errorf("attempting to assign null quota to %s", path)
 	}
@@ -319,11 +319,11 @@ func AssignQuota(m mount.Interface, path string, poduid types.UID, bytes *resour
 	// volumes in a pod, we can simply remove this line of code.
 	// If and when we decide permanently that we're going to adopt
 	// one quota per volume, we can rip all of the pod code out.
-	poduid = types.UID(uuid.NewUUID()) //nolint:staticcheck // SA4009 poduid is overwritten by design, see comment above
-	if pod, ok := dirPodMap[path]; ok && pod != poduid {
-		return fmt.Errorf("requesting quota on existing directory %s but different pod %s %s", path, pod, poduid)
+	volumeuid := types.UID(uuid.NewUUID())
+	if pod, ok := dirPodMap[path]; ok && pod != volumeuid {
+		return fmt.Errorf("requesting quota on existing directory %s but different pod %s %s", path, pod, volumeuid)
 	}
-	oid, ok := podQuotaMap[poduid]
+	oid, ok := podQuotaMap[volumeuid]
 	if ok {
 		if quotaSizeMap[oid] != ibytes {
 			return fmt.Errorf("requesting quota of different size: old %v new %v", quotaSizeMap[oid], bytes)
@@ -342,12 +342,12 @@ func AssignQuota(m mount.Interface, path string, poduid types.UID, bytes *resour
 			ibytes = -1
 		}
 		if err = setQuotaOnDir(path, id, ibytes); err == nil {
-			quotaPodMap[id] = poduid
+			quotaPodMap[id] = volumeuid
 			quotaSizeMap[id] = ibytes
-			podQuotaMap[poduid] = id
+			podQuotaMap[volumeuid] = id
 			dirQuotaMap[path] = id
-			dirPodMap[path] = poduid
-			podDirCountMap[poduid]++
+			dirPodMap[path] = volumeuid
+			podDirCountMap[volumeuid]++
 			klog.V(4).Infof("Assigning quota ID %d (%d) to %s", id, ibytes, path)
 			return nil
 		}
@@ -415,7 +415,7 @@ func ClearQuota(m mount.Interface, path string) error {
 	if !ok {
 		return fmt.Errorf("clearQuota: No quota available for %s", path)
 	}
-	projid, err := getQuotaOnDir(m, path)
+	projid, err := GetQuotaOnDir(m, path)
 	if err != nil {
 		// Log-and-continue instead of returning an error for now
 		// due to unspecified backwards compatibility concerns (a subject to revise)

pkg/volume/util/fsquota/quota_unsupported.go

@@ -22,6 +22,7 @@ package fsquota
 import (
 	"errors"
 
+	"k8s.io/kubernetes/pkg/volume/util/fsquota/common"
 	"k8s.io/mount-utils"
 
 	"k8s.io/apimachinery/pkg/api/resource"
@@ -33,6 +34,10 @@ import (
 
 var errNotImplemented = errors.New("not implemented")
 
+func GetQuotaOnDir(_ mount.Interface, _ string) (common.QuotaID, error) {
+	return common.BadQuotaID, errNotImplemented
+}
+
 // SupportsQuotas -- dummy implementation
 func SupportsQuotas(_ mount.Interface, _ string) (bool, error) {
 	return false, errNotImplemented