kubelet: fix setup of emptyDir with sizeLimit (fsquota)

When size limit is specified subsequent invocations will fail because
ibytes is changed to -1 and stored internally in quotaSizeMap during the
first call.  Later invocation will see that the requested size doesn't
match the actual stored value and it will fail.

Signed-off-by: Alexandru Matei <alexandru.matei@uipath.com>
This commit is contained in:
Alexandru Matei 2023-04-05 13:05:27 +03:00
parent 330b5a2b8d
commit c77ad3116f
2 changed files with 41 additions and 24 deletions

View File

@ -353,10 +353,11 @@ func AssignQuota(m mount.Interface, path string, poduid types.UID, bytes *resour
} }
// When enforcing quotas are enabled, we'll condition this // When enforcing quotas are enabled, we'll condition this
// on their being disabled also. // on their being disabled also.
if ibytes > 0 { fsbytes := ibytes
ibytes = -1 if fsbytes > 0 {
fsbytes = -1
} }
if err = setQuotaOnDir(path, id, ibytes); err == nil { if err = setQuotaOnDir(path, id, fsbytes); err == nil {
quotaPodMap[id] = internalPodUid quotaPodMap[id] = internalPodUid
quotaSizeMap[id] = ibytes quotaSizeMap[id] = ibytes
podQuotaMap[internalPodUid] = id podQuotaMap[internalPodUid] = id
@ -364,7 +365,7 @@ func AssignQuota(m mount.Interface, path string, poduid types.UID, bytes *resour
dirPodMap[path] = internalPodUid dirPodMap[path] = internalPodUid
podUidMap[internalPodUid] = externalPodUid podUidMap[internalPodUid] = externalPodUid
podDirCountMap[internalPodUid]++ podDirCountMap[internalPodUid]++
klog.V(4).Infof("Assigning quota ID %d (%d) to %s", id, ibytes, path) klog.V(4).Infof("Assigning quota ID %d (request limit %d, actual limit %d) to %s", id, ibytes, fsbytes, path)
return nil return nil
} }
removeProjectID(path, id) removeProjectID(path, id)

View File

@ -383,6 +383,7 @@ func fakeClearQuota(path string) error {
} }
type quotaTestCase struct { type quotaTestCase struct {
name string
path string path string
poduid types.UID poduid types.UID
bytes int64 bytes int64
@ -444,42 +445,57 @@ volume1048581:1048581
var quotaTestCases = []quotaTestCase{ var quotaTestCases = []quotaTestCase{
{ {
"SupportsQuotaOnQuotaVolume",
"/quota1/a", "", 1024, "Supports", "", "", "/quota1/a", "", 1024, "Supports", "", "",
true, true, 0, 0, 0, 0, 1, 1, 0, 0, 1, 1, 1, true, true, 0, 0, 0, 0, 1, 1, 0, 0, 1, 1, 1,
}, },
{ {
"AssignQuotaFirstTime",
"/quota1/a", "", 1024, "Set", projects1, projid1, "/quota1/a", "", 1024, "Set", projects1, projid1,
true, true, 1, 1, 1, 1, 0, 0, 1, 1, 0, 0, 0, true, true, 1, 1, 1, 1, 0, 0, 1, 1, 0, 0, 0,
}, },
{ {
"AssignQuotaFirstTime",
"/quota1/b", "x", 1024, "Set", projects2, projid2, "/quota1/b", "x", 1024, "Set", projects2, projid2,
true, true, 1, 1, 1, 1, 0, 1, 1, 1, 1, 1, 1, true, true, 1, 1, 1, 1, 0, 1, 1, 1, 1, 1, 1,
}, },
{ {
"AssignQuotaFirstTime",
"/quota2/b", "x", 1024, "Set", projects3, projid3, "/quota2/b", "x", 1024, "Set", projects3, projid3,
true, true, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, true, true, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1,
}, },
{ {
"AssignQuotaSecondTimeWithSameSize",
"/quota1/b", "x", 1024, "Set", projects3, projid3, "/quota1/b", "x", 1024, "Set", projects3, projid3,
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, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
}, },
{ {
"ClearQuotaFirstTime",
"/quota1/b", "", 1024, "Clear", projects4, projid4, "/quota1/b", "", 1024, "Clear", projects4, projid4,
true, true, -1, -1, -1, -1, 0, -1, -1, -1, -1, -1, -1, true, true, -1, -1, -1, -1, 0, -1, -1, -1, -1, -1, -1,
}, },
{ {
"SupportsQuotaOnNonQuotaVolume",
"/noquota/a", "", 1024, "Supports", projects4, projid4, "/noquota/a", "", 1024, "Supports", projects4, projid4,
false, false, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, false, false, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
}, },
{ {
"ClearQuotaFirstTime",
"/quota1/a", "", 1024, "Clear", projects5, projid5, "/quota1/a", "", 1024, "Clear", projects5, projid5,
true, true, -1, -1, -1, -1, 0, -1, -1, -1, -1, -1, -1, true, true, -1, -1, -1, -1, 0, -1, -1, -1, -1, -1, -1,
}, },
{ {
"ClearQuotaSecondTime",
"/quota1/a", "", 1024, "Clear", projects5, projid5, "/quota1/a", "", 1024, "Clear", projects5, projid5,
true, false, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, true, false, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
}, },
{ {
"ClearQuotaFirstTime",
"/quota2/b", "", 1024, "Clear", "", "", "/quota2/b", "", 1024, "Clear", "", "",
true, true, -1, -1, -1, -1, 0, -1, -1, -1, -1, -1, -1, true, true, -1, -1, -1, -1, 0, -1, -1, -1, -1, -1, -1,
}, },
@ -522,11 +538,11 @@ func runCaseEnabled(t *testing.T, testcase quotaTestCase, seq int) bool {
supports, err := fakeSupportsQuotas(testcase.path) supports, err := fakeSupportsQuotas(testcase.path)
if err != nil { if err != nil {
fail = true fail = true
t.Errorf("Case %v (%s, %v) Got error in fakeSupportsQuotas: %v", seq, testcase.path, true, err) t.Errorf("Case %v (%s, %s, %v) Got error in fakeSupportsQuotas: %v", seq, testcase.name, testcase.path, true, err)
} }
if supports != testcase.supportsQuota { if supports != testcase.supportsQuota {
fail = true fail = true
t.Errorf("Case %v (%s, %v) fakeSupportsQuotas got %v, expect %v", seq, testcase.path, true, supports, testcase.supportsQuota) t.Errorf("Case %v (%s, %s, %v) fakeSupportsQuotas got %v, expect %v", seq, testcase.name, testcase.path, true, supports, testcase.supportsQuota)
} }
return fail return fail
case "Set": case "Set":
@ -538,15 +554,15 @@ func runCaseEnabled(t *testing.T, testcase quotaTestCase, seq int) bool {
case "GetInodes": case "GetInodes":
_, err = GetInodes(testcase.path) _, err = GetInodes(testcase.path)
default: default:
t.Errorf("Case %v (%s, %v) unknown operation %s", seq, testcase.path, true, testcase.op) t.Errorf("Case %v (%s, %s, %v) unknown operation %s", seq, testcase.name, testcase.path, true, testcase.op)
return true return true
} }
if err != nil && testcase.expectsSetQuota { if err != nil && testcase.expectsSetQuota {
fail = true fail = true
t.Errorf("Case %v (%s, %v) %s expected to clear quota but failed %v", seq, testcase.path, true, testcase.op, err) t.Errorf("Case %v (%s, %s, %v) %s expected to clear quota but failed %v", seq, testcase.name, testcase.path, true, testcase.op, err)
} else if err == nil && !testcase.expectsSetQuota { } else if err == nil && !testcase.expectsSetQuota {
fail = true fail = true
t.Errorf("Case %v (%s, %v) %s expected not to clear quota but succeeded", seq, testcase.path, true, testcase.op) t.Errorf("Case %v (%s, %s, %v) %s expected not to clear quota but succeeded", seq, testcase.name, testcase.path, true, testcase.op)
} }
return fail return fail
} }
@ -557,7 +573,7 @@ func runCaseDisabled(t *testing.T, testcase quotaTestCase, seq int) bool {
switch testcase.op { switch testcase.op {
case "Supports": case "Supports":
if supports, _ = fakeSupportsQuotas(testcase.path); supports { if supports, _ = fakeSupportsQuotas(testcase.path); supports {
t.Errorf("Case %v (%s, %v) supports quotas but shouldn't", seq, testcase.path, false) t.Errorf("Case %v (%s, %s, %v) supports quotas but shouldn't", seq, testcase.name, testcase.path, false)
return true return true
} }
return false return false
@ -570,11 +586,11 @@ func runCaseDisabled(t *testing.T, testcase quotaTestCase, seq int) bool {
case "GetInodes": case "GetInodes":
_, err = GetInodes(testcase.path) _, err = GetInodes(testcase.path)
default: default:
t.Errorf("Case %v (%s, %v) unknown operation %s", seq, testcase.path, false, testcase.op) t.Errorf("Case %v (%s, %s, %v) unknown operation %s", seq, testcase.name, testcase.path, false, testcase.op)
return true return true
} }
if err == nil { if err == nil {
t.Errorf("Case %v (%s, %v) %s: supports quotas but shouldn't", seq, testcase.path, false, testcase.op) t.Errorf("Case %v (%s, %s, %v) %s: supports quotas but shouldn't", seq, testcase.name, testcase.path, false, testcase.op)
return true return true
} }
return false return false
@ -678,50 +694,50 @@ func testAddRemoveQuotas(t *testing.T, enabled bool) {
compareProjectsFiles(t, testcase, projectsFile, projidFile, enabled) compareProjectsFiles(t, testcase, projectsFile, projidFile, enabled)
if len(podQuotaMap) != expectedPodQuotaCount { if len(podQuotaMap) != expectedPodQuotaCount {
fail = true fail = true
t.Errorf("Case %v (%s, %v) podQuotaCount mismatch: got %v, expect %v", seq, testcase.path, enabled, len(podQuotaMap), expectedPodQuotaCount) t.Errorf("Case %v (%s, %s, %v) podQuotaCount mismatch: got %v, expect %v", seq, testcase.name, testcase.path, enabled, len(podQuotaMap), expectedPodQuotaCount)
} }
if len(dirQuotaMap) != expectedDirQuotaCount { if len(dirQuotaMap) != expectedDirQuotaCount {
fail = true fail = true
t.Errorf("Case %v (%s, %v) dirQuotaCount mismatch: got %v, expect %v", seq, testcase.path, enabled, len(dirQuotaMap), expectedDirQuotaCount) t.Errorf("Case %v (%s, %s, %v) dirQuotaCount mismatch: got %v, expect %v", seq, testcase.name, testcase.path, enabled, len(dirQuotaMap), expectedDirQuotaCount)
} }
if len(quotaPodMap) != expectedQuotaPodCount { if len(quotaPodMap) != expectedQuotaPodCount {
fail = true fail = true
t.Errorf("Case %v (%s, %v) quotaPodCount mismatch: got %v, expect %v", seq, testcase.path, enabled, len(quotaPodMap), expectedQuotaPodCount) t.Errorf("Case %v (%s, %s, %v) quotaPodCount mismatch: got %v, expect %v", seq, testcase.name, testcase.path, enabled, len(quotaPodMap), expectedQuotaPodCount)
} }
if len(dirPodMap) != expectedDirPodCount { if len(dirPodMap) != expectedDirPodCount {
fail = true fail = true
t.Errorf("Case %v (%s, %v) dirPodCount mismatch: got %v, expect %v", seq, testcase.path, enabled, len(dirPodMap), expectedDirPodCount) t.Errorf("Case %v (%s, %s, %v) dirPodCount mismatch: got %v, expect %v", seq, testcase.name, testcase.path, enabled, len(dirPodMap), expectedDirPodCount)
} }
if len(devApplierMap) != expectedDevApplierCount { if len(devApplierMap) != expectedDevApplierCount {
fail = true fail = true
t.Errorf("Case %v (%s, %v) devApplierCount mismatch: got %v, expect %v", seq, testcase.path, enabled, len(devApplierMap), expectedDevApplierCount) t.Errorf("Case %v (%s, %s, %v) devApplierCount mismatch: got %v, expect %v", seq, testcase.name, testcase.path, enabled, len(devApplierMap), expectedDevApplierCount)
} }
if len(dirApplierMap) != expectedDirApplierCount { if len(dirApplierMap) != expectedDirApplierCount {
fail = true fail = true
t.Errorf("Case %v (%s, %v) dirApplierCount mismatch: got %v, expect %v", seq, testcase.path, enabled, len(dirApplierMap), expectedDirApplierCount) t.Errorf("Case %v (%s, %s, %v) dirApplierCount mismatch: got %v, expect %v", seq, testcase.name, testcase.path, enabled, len(dirApplierMap), expectedDirApplierCount)
} }
if len(podDirCountMap) != expectedPodDirCountCount { if len(podDirCountMap) != expectedPodDirCountCount {
fail = true fail = true
t.Errorf("Case %v (%s, %v) podDirCountCount mismatch: got %v, expect %v", seq, testcase.path, enabled, len(podDirCountMap), expectedPodDirCountCount) t.Errorf("Case %v (%s, %s, %v) podDirCountCount mismatch: got %v, expect %v", seq, testcase.name, testcase.path, enabled, len(podDirCountMap), expectedPodDirCountCount)
} }
if len(quotaSizeMap) != expectedQuotaSizeCount { if len(quotaSizeMap) != expectedQuotaSizeCount {
fail = true fail = true
t.Errorf("Case %v (%s, %v) quotaSizeCount mismatch: got %v, expect %v", seq, testcase.path, enabled, len(quotaSizeMap), expectedQuotaSizeCount) t.Errorf("Case %v (%s, %s, %v) quotaSizeCount mismatch: got %v, expect %v", seq, testcase.name, testcase.path, enabled, len(quotaSizeMap), expectedQuotaSizeCount)
} }
if len(supportsQuotasMap) != expectedSupportsQuotasCount { if len(supportsQuotasMap) != expectedSupportsQuotasCount {
fail = true fail = true
t.Errorf("Case %v (%s, %v) supportsQuotasCount mismatch: got %v, expect %v", seq, testcase.path, enabled, len(supportsQuotasMap), expectedSupportsQuotasCount) t.Errorf("Case %v (%s, %s, %v) supportsQuotasCount mismatch: got %v, expect %v", seq, testcase.name, testcase.path, enabled, len(supportsQuotasMap), expectedSupportsQuotasCount)
} }
if len(backingDevMap) != expectedBackingDevCount { if len(backingDevMap) != expectedBackingDevCount {
fail = true fail = true
t.Errorf("Case %v (%s, %v) BackingDevCount mismatch: got %v, expect %v", seq, testcase.path, enabled, len(backingDevMap), expectedBackingDevCount) t.Errorf("Case %v (%s, %s, %v) BackingDevCount mismatch: got %v, expect %v", seq, testcase.name, testcase.path, enabled, len(backingDevMap), expectedBackingDevCount)
} }
if len(mountpointMap) != expectedMountpointCount { if len(mountpointMap) != expectedMountpointCount {
fail = true fail = true
t.Errorf("Case %v (%s, %v) MountpointCount mismatch: got %v, expect %v", seq, testcase.path, enabled, len(mountpointMap), expectedMountpointCount) t.Errorf("Case %v (%s, %s, %v) MountpointCount mismatch: got %v, expect %v", seq, testcase.name, testcase.path, enabled, len(mountpointMap), expectedMountpointCount)
} }
if fail { if fail {
logAllMaps(fmt.Sprintf("%v %s", seq, testcase.path)) logAllMaps(fmt.Sprintf("%v %s %s", seq, testcase.name, testcase.path))
} }
} }
os.Remove(projectsFile) os.Remove(projectsFile)