generic ephemeral volumes: fix and test apiserver feature gate

The implementation should have preserved an existing ephemeral volume
source during an update even when the feature gate is currently
disabled, but due to a cut-and-paste error it was checking for CSI
volumes instead.

The new test detected that. It's based on
https://github.com/kubernetes/kubernetes/pull/97058/files#diff-7826f7adbc1996a05ab52e3f5f02429e94b68ce6bce0dc534d1be636154fded3

pkg/api/pod/util.go

@@ -565,7 +565,7 @@ func dropDisabledCSIVolumeSourceAlphaFields(podSpec, oldPodSpec *api.PodSpec) {
 // dropDisabledEphemeralVolumeSourceAlphaFields removes disabled alpha fields from []EphemeralVolumeSource.
 // This should be called from PrepareForCreate/PrepareForUpdate for all pod specs resources containing a EphemeralVolumeSource
 func dropDisabledEphemeralVolumeSourceAlphaFields(podSpec, oldPodSpec *api.PodSpec) {
-	if !utilfeature.DefaultFeatureGate.Enabled(features.GenericEphemeralVolume) && !csiInUse(oldPodSpec) {
+	if !utilfeature.DefaultFeatureGate.Enabled(features.GenericEphemeralVolume) && !ephemeralInUse(oldPodSpec) {
 		for i := range podSpec.Volumes {
 			podSpec.Volumes[i].Ephemeral = nil
 		}
@@ -711,6 +711,19 @@ func csiInUse(podSpec *api.PodSpec) bool {
 	return false
 }
 
+// ephemeralInUse returns true if any pod's spec include inline CSI volumes.
+func ephemeralInUse(podSpec *api.PodSpec) bool {
+	if podSpec == nil {
+		return false
+	}
+	for i := range podSpec.Volumes {
+		if podSpec.Volumes[i].Ephemeral != nil {
+			return true
+		}
+	}
+	return false
+}
+
 // podPriorityInUse returns true if status is not nil and number of PodIPs is greater than one
 func multiplePodIPsInUse(podStatus *api.PodStatus) bool {
 	if podStatus == nil {

pkg/registry/core/pod/strategy_test.go

@@ -25,7 +25,7 @@ import (
 	"testing"
 
 	"github.com/stretchr/testify/require"
-	"k8s.io/api/core/v1"
+	v1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/api/resource"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -1103,3 +1103,97 @@ func TestApplySeccompVersionSkew(t *testing.T) {
 		test.validation(t, test.pod)
 	}
 }
+
+// TestEphemeralVolumeEnablement checks the behavior of the API server
+// when the GenericEphemeralVolume feature is turned on and then off:
+// the Ephemeral struct must be preserved even during updates.
+func TestEphemeralVolumeEnablement(t *testing.T) {
+	// Enable the Feature Gate during the first pod creation
+	defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.GenericEphemeralVolume, true)()
+
+	pod := createPodWithGenericEphemeralVolume()
+	expectedPod := pod.DeepCopy()
+
+	Strategy.PrepareForCreate(context.Background(), pod)
+	require.Equal(t, expectedPod.Spec, pod.Spec, "pod spec")
+
+	errs := Strategy.Validate(context.Background(), pod)
+	require.Empty(t, errs, "errors from validation")
+
+	// Now let's disable the Feature Gate, update some other field from the Pod and expect the volume to remain present
+	defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.GenericEphemeralVolume, false)()
+	updatePod := testUpdatePod(t, pod, "aaa")
+
+	// And let's enable the FG again, add another from and check if the volume is still present
+	defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.GenericEphemeralVolume, true)()
+	testUpdatePod(t, updatePod, "bbb")
+}
+
+// TestEphemeralVolumeDisabled checks the behavior of the API server
+// when the GenericEphemeralVolume is off: the Ephemeral struct gets dropped,
+// validation fails.
+func TestEphemeralVolumeDisabled(t *testing.T) {
+	// Disable the Feature Gate during the first pod creation
+	defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.GenericEphemeralVolume, false)()
+
+	pod := createPodWithGenericEphemeralVolume()
+	expectedPod := pod.DeepCopy()
+	expectedPod.Spec.Volumes[0].VolumeSource.Ephemeral = nil
+
+	Strategy.PrepareForCreate(context.Background(), pod)
+	require.Equal(t, expectedPod.Spec, pod.Spec, "pod spec")
+
+	errs := Strategy.Validate(context.Background(), pod)
+	require.NotEmpty(t, errs, "no errors from validation")
+}
+
+func testUpdatePod(t *testing.T, oldPod *api.Pod, labelValue string) *api.Pod {
+	updatedPod := oldPod.DeepCopy()
+	updatedPod.Labels = map[string]string{"XYZ": labelValue}
+	expectedPod := updatedPod.DeepCopy()
+	Strategy.PrepareForUpdate(context.Background(), updatedPod, oldPod)
+	require.Equal(t, expectedPod.Spec, updatedPod.Spec, "updated pod spec")
+	errs := Strategy.Validate(context.Background(), updatedPod)
+	require.Empty(t, errs, "errors from validation")
+	return updatedPod
+}
+
+func createPodWithGenericEphemeralVolume() *api.Pod {
+	return &api.Pod{
+		ObjectMeta: metav1.ObjectMeta{
+			Namespace: "ns",
+			Name:      "pod",
+		},
+		Spec: api.PodSpec{
+			RestartPolicy: api.RestartPolicyAlways,
+			DNSPolicy:     api.DNSClusterFirst,
+			Containers: []api.Container{{
+				Name:                     "foo",
+				Image:                    "example",
+				TerminationMessagePolicy: api.TerminationMessageReadFile,
+				ImagePullPolicy:          api.PullAlways,
+			}},
+			Volumes: []api.Volume{
+				{
+					Name: "ephemeral",
+					VolumeSource: api.VolumeSource{
+						Ephemeral: &api.EphemeralVolumeSource{
+							VolumeClaimTemplate: &api.PersistentVolumeClaimTemplate{
+								Spec: api.PersistentVolumeClaimSpec{
+									AccessModes: []api.PersistentVolumeAccessMode{
+										api.ReadWriteOnce,
+									},
+									Resources: api.ResourceRequirements{
+										Requests: api.ResourceList{
+											api.ResourceStorage: resource.MustParse("1Gi"),
+										},
+									},
+								},
+							},
+						},
+					},
+				},
+			},
+		},
+	}
+}