EmptyVolume SizeLimit validation and tests

Signed-off-by: Serguei Bezverkhi <sbezverk@cisco.com>

pkg/api/pod/BUILD

@@ -38,6 +38,7 @@ go_test(
     deps = [
         "//pkg/apis/core:go_default_library",
         "//pkg/features:go_default_library",
+        "//staging/src/k8s.io/apimachinery/pkg/api/resource:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/util/diff:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/util/sets:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/util/validation/field:go_default_library",

pkg/api/pod/util.go

@@ -242,19 +242,12 @@ func DropDisabledFields(podSpec, oldPodSpec *api.PodSpec) {
 		podSpec.PriorityClassName = ""
 	}
 
-	if !utilfeature.DefaultFeatureGate.Enabled(features.LocalStorageCapacityIsolation) {
+	if !utilfeature.DefaultFeatureGate.Enabled(features.LocalStorageCapacityIsolation) && !emptyDirSizeLimitInUse(oldPodSpec) {
 		for i := range podSpec.Volumes {
 			if podSpec.Volumes[i].EmptyDir != nil {
 				podSpec.Volumes[i].EmptyDir.SizeLimit = nil
 			}
 		}
-		if oldPodSpec != nil {
-			for i := range oldPodSpec.Volumes {
-				if oldPodSpec.Volumes[i].EmptyDir != nil {
-					oldPodSpec.Volumes[i].EmptyDir.SizeLimit = nil
-				}
-			}
-		}
 	}
 
 	if !utilfeature.DefaultFeatureGate.Enabled(features.VolumeSubpath) && !subpathInUse(oldPodSpec) {
@@ -428,3 +421,18 @@ func podPriorityInUse(podSpec *api.PodSpec) bool {
 	}
 	return false
 }
+
+// emptyDirSizeLimitInUse returns true if any pod's EptyDir volumes use SizeLimit.
+func emptyDirSizeLimitInUse(podSpec *api.PodSpec) bool {
+	if podSpec == nil {
+		return false
+	}
+	for i := range podSpec.Volumes {
+		if podSpec.Volumes[i].EmptyDir != nil {
+			if podSpec.Volumes[i].EmptyDir.SizeLimit != nil {
+				return true
+			}
+		}
+	}
+	return false
+}

pkg/api/pod/util_test.go

@@ -22,6 +22,7 @@ import (
 	"strings"
 	"testing"
 
+	"k8s.io/apimachinery/pkg/api/resource"
 	"k8s.io/apimachinery/pkg/util/diff"
 	"k8s.io/apimachinery/pkg/util/sets"
 	"k8s.io/apimachinery/pkg/util/validation/field"
@@ -728,3 +729,113 @@ func TestDropPodPriority(t *testing.T) {
 		}
 	}
 }
+
+func TestDropEmptyDirSizeLimit(t *testing.T) {
+	sizeLimit := resource.MustParse("1Gi")
+	podWithEmptyDirSizeLimit := func() *api.Pod {
+		return &api.Pod{
+			Spec: api.PodSpec{
+				RestartPolicy: api.RestartPolicyNever,
+				Volumes: []api.Volume{
+					{
+						Name: "a",
+						VolumeSource: api.VolumeSource{
+							EmptyDir: &api.EmptyDirVolumeSource{
+								Medium:    "memory",
+								SizeLimit: &sizeLimit,
+							},
+						},
+					},
+				},
+			},
+		}
+	}
+	podWithoutEmptyDirSizeLimit := func() *api.Pod {
+		return &api.Pod{
+			Spec: api.PodSpec{
+				RestartPolicy: api.RestartPolicyNever,
+				Volumes: []api.Volume{
+					{
+						Name: "a",
+						VolumeSource: api.VolumeSource{
+							EmptyDir: &api.EmptyDirVolumeSource{
+								Medium: "memory",
+							},
+						},
+					},
+				},
+			},
+		}
+	}
+
+	podInfo := []struct {
+		description          string
+		hasEmptyDirSizeLimit bool
+		pod                  func() *api.Pod
+	}{
+		{
+			description:          "has EmptyDir Size Limit",
+			hasEmptyDirSizeLimit: true,
+			pod:                  podWithEmptyDirSizeLimit,
+		},
+		{
+			description:          "does not have EmptyDir Size Limit",
+			hasEmptyDirSizeLimit: false,
+			pod:                  podWithoutEmptyDirSizeLimit,
+		},
+		{
+			description:          "is nil",
+			hasEmptyDirSizeLimit: false,
+			pod:                  func() *api.Pod { return nil },
+		},
+	}
+
+	for _, enabled := range []bool{true, false} {
+		for _, oldPodInfo := range podInfo {
+			for _, newPodInfo := range podInfo {
+				oldPodHasEmptyDirSizeLimit, oldPod := oldPodInfo.hasEmptyDirSizeLimit, oldPodInfo.pod()
+				newPodHasEmptyDirSizeLimit, newPod := newPodInfo.hasEmptyDirSizeLimit, newPodInfo.pod()
+				if newPod == nil {
+					continue
+				}
+
+				t.Run(fmt.Sprintf("feature enabled=%v, old pod %v, new pod %v", enabled, oldPodInfo.description, newPodInfo.description), func(t *testing.T) {
+					defer utilfeaturetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.LocalStorageCapacityIsolation, enabled)()
+
+					var oldPodSpec *api.PodSpec
+					if oldPod != nil {
+						oldPodSpec = &oldPod.Spec
+					}
+					DropDisabledFields(&newPod.Spec, oldPodSpec)
+
+					// old pod should never be changed
+					if !reflect.DeepEqual(oldPod, oldPodInfo.pod()) {
+						t.Errorf("old pod changed: %v", diff.ObjectReflectDiff(oldPod, oldPodInfo.pod()))
+					}
+
+					switch {
+					case enabled || oldPodHasEmptyDirSizeLimit:
+						// new pod should not be changed if the feature is enabled, or if the old pod had EmptyDir SizeLimit
+						if !reflect.DeepEqual(newPod, newPodInfo.pod()) {
+							t.Errorf("new pod changed: %v", diff.ObjectReflectDiff(newPod, newPodInfo.pod()))
+						}
+					case newPodHasEmptyDirSizeLimit:
+						// new pod should be changed
+						if reflect.DeepEqual(newPod, newPodInfo.pod()) {
+							t.Errorf("new pod was not changed")
+						}
+						// new pod should not have EmptyDir SizeLimit
+						if !reflect.DeepEqual(newPod, podWithoutEmptyDirSizeLimit()) {
+							t.Errorf("new pod had EmptyDir SizeLimit: %v", diff.ObjectReflectDiff(newPod, podWithoutEmptyDirSizeLimit()))
+						}
+					default:
+						// new pod should not need to be changed
+						if !reflect.DeepEqual(newPod, newPodInfo.pod()) {
+							t.Errorf("new pod changed: %v", diff.ObjectReflectDiff(newPod, newPodInfo.pod()))
+						}
+					}
+				})
+			}
+		}
+	}
+}