RuntimeClass validation and tests

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

pkg/api/pod/util.go

@@ -277,11 +277,9 @@ func DropDisabledFields(podSpec, oldPodSpec *api.PodSpec) {
 
 	dropDisabledRunAsGroupField(podSpec, oldPodSpec)
 
-	if !utilfeature.DefaultFeatureGate.Enabled(features.RuntimeClass) {
+	if !utilfeature.DefaultFeatureGate.Enabled(features.RuntimeClass) && !runtimeClassInUse(oldPodSpec) {
+		// Set RuntimeClassName to nil only if feature is disabled and it is not used
 		podSpec.RuntimeClassName = nil
-		if oldPodSpec != nil {
-			oldPodSpec.RuntimeClassName = nil
-		}
 	}
 
 	dropDisabledProcMountField(podSpec, oldPodSpec)
@@ -397,3 +395,14 @@ func subpathInUse(podSpec *api.PodSpec) bool {
 	}
 	return false
 }
+
+// runtimeClassInUse returns true if the pod spec is non-nil and has a RuntimeClassName set
+func runtimeClassInUse(podSpec *api.PodSpec) bool {
+	if podSpec == nil {
+		return false
+	}
+	if podSpec.RuntimeClassName != nil {
+		return true
+	}
+	return false
+}

pkg/api/pod/util_test.go

@@ -429,3 +429,92 @@ func TestDropSubPath(t *testing.T) {
 		}
 	}
 }
+
+func TestDropRuntimeClass(t *testing.T) {
+	runtimeClassName := "some_container_engine"
+	podWithoutRuntimeClass := func() *api.Pod {
+		return &api.Pod{
+			Spec: api.PodSpec{
+				RuntimeClassName: nil,
+			},
+		}
+	}
+	podWithRuntimeClass := func() *api.Pod {
+		return &api.Pod{
+			Spec: api.PodSpec{
+				RuntimeClassName: &runtimeClassName,
+			},
+		}
+	}
+
+	podInfo := []struct {
+		description            string
+		hasPodRuntimeClassName bool
+		pod                    func() *api.Pod
+	}{
+		{
+			description:            "pod Without RuntimeClassName",
+			hasPodRuntimeClassName: false,
+			pod:                    podWithoutRuntimeClass,
+		},
+		{
+			description:            "pod With RuntimeClassName",
+			hasPodRuntimeClassName: true,
+			pod:                    podWithRuntimeClass,
+		},
+		{
+			description:            "is nil",
+			hasPodRuntimeClassName: false,
+			pod:                    func() *api.Pod { return nil },
+		},
+	}
+
+	for _, enabled := range []bool{true, false} {
+		for _, oldPodInfo := range podInfo {
+			for _, newPodInfo := range podInfo {
+				oldPodHasRuntimeClassName, oldPod := oldPodInfo.hasPodRuntimeClassName, oldPodInfo.pod()
+				newPodHasRuntimeClassName, newPod := newPodInfo.hasPodRuntimeClassName, 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.RuntimeClass, 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 || oldPodHasRuntimeClassName:
+						// new pod should not be changed if the feature is enabled, or if the old pod had RuntimeClass
+						if !reflect.DeepEqual(newPod, newPodInfo.pod()) {
+							t.Errorf("new pod changed: %v", diff.ObjectReflectDiff(newPod, newPodInfo.pod()))
+						}
+					case newPodHasRuntimeClassName:
+						// new pod should be changed
+						if reflect.DeepEqual(newPod, newPodInfo.pod()) {
+							t.Errorf("new pod was not changed")
+						}
+						// new pod should not have RuntimeClass
+						if !reflect.DeepEqual(newPod, podWithoutRuntimeClass()) {
+							t.Errorf("new pod had PodRuntimeClassName: %v", diff.ObjectReflectDiff(newPod, podWithoutRuntimeClass()))
+						}
+					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()))
+						}
+					}
+				})
+			}
+		}
+	}
+}

pkg/apis/core/validation/validation.go

@@ -3053,7 +3053,7 @@ func ValidatePodSpec(spec *core.PodSpec, fldPath *field.Path) field.ErrorList {
 		}
 	}
 
-	if spec.RuntimeClassName != nil && utilfeature.DefaultFeatureGate.Enabled(features.RuntimeClass) {
+	if spec.RuntimeClassName != nil {
 		allErrs = append(allErrs, ValidateRuntimeClassName(*spec.RuntimeClassName, fldPath.Child("runtimeClassName"))...)
 	}