diff --git a/pkg/api/pod/util.go b/pkg/api/pod/util.go index 1a61f970175..be52beb85ec 100644 --- a/pkg/api/pod/util.go +++ b/pkg/api/pod/util.go @@ -544,29 +544,10 @@ func dropDisabledFields( } } - if !utilfeature.DefaultFeatureGate.Enabled(features.VolumeSubpath) && !subpathInUse(oldPodSpec) { - // drop subpath from the pod if the feature is disabled and the old spec did not specify subpaths - VisitContainers(podSpec, AllContainers, func(c *api.Container, containerType ContainerType) bool { - for i := range c.VolumeMounts { - c.VolumeMounts[i].SubPath = "" - } - return true - }) - } if !utilfeature.DefaultFeatureGate.Enabled(features.EphemeralContainers) && !ephemeralContainersInUse(oldPodSpec) { podSpec.EphemeralContainers = nil } - if !utilfeature.DefaultFeatureGate.Enabled(features.VolumeSubpath) && !subpathExprInUse(oldPodSpec) { - // drop subpath env expansion from the pod if subpath feature is disabled and the old spec did not specify subpath env expansion - VisitContainers(podSpec, AllContainers, func(c *api.Container, containerType ContainerType) bool { - for i := range c.VolumeMounts { - c.VolumeMounts[i].SubPathExpr = "" - } - return true - }) - } - if !utilfeature.DefaultFeatureGate.Enabled(features.ProbeTerminationGracePeriod) && !probeGracePeriodInUse(oldPodSpec) { // Set pod-level terminationGracePeriodSeconds to nil if the feature is disabled and it is not used VisitContainers(podSpec, AllContainers, func(c *api.Container, containerType ContainerType) bool { @@ -728,26 +709,6 @@ func fsGroupPolicyInUse(podSpec *api.PodSpec) bool { return false } -// subpathInUse returns true if the pod spec is non-nil and has a volume mount that makes use of the subPath feature -func subpathInUse(podSpec *api.PodSpec) bool { - if podSpec == nil { - return false - } - - var inUse bool - VisitContainers(podSpec, AllContainers, func(c *api.Container, containerType ContainerType) bool { - for i := range c.VolumeMounts { - if len(c.VolumeMounts[i].SubPath) > 0 { - inUse = true - return false - } - } - return true - }) - - return inUse -} - // overheadInUse returns true if the pod spec is non-nil and has Overhead set func overheadInUse(podSpec *api.PodSpec) bool { if podSpec == nil { @@ -816,26 +777,6 @@ func emptyDirSizeLimitInUse(podSpec *api.PodSpec) bool { return false } -// subpathExprInUse returns true if the pod spec is non-nil and has a volume mount that makes use of the subPathExpr feature -func subpathExprInUse(podSpec *api.PodSpec) bool { - if podSpec == nil { - return false - } - - var inUse bool - VisitContainers(podSpec, AllContainers, func(c *api.Container, containerType ContainerType) bool { - for i := range c.VolumeMounts { - if len(c.VolumeMounts[i].SubPathExpr) > 0 { - inUse = true - return false - } - } - return true - }) - - return inUse -} - // probeGracePeriodInUse returns true if the pod spec is non-nil and has a probe that makes use // of the probe-level terminationGracePeriodSeconds feature func probeGracePeriodInUse(podSpec *api.PodSpec) bool { diff --git a/pkg/api/pod/util_test.go b/pkg/api/pod/util_test.go index c13458fdc77..c66b738b98c 100644 --- a/pkg/api/pod/util_test.go +++ b/pkg/api/pod/util_test.go @@ -652,100 +652,6 @@ func TestDropFSGroupFields(t *testing.T) { } -func TestDropSubPath(t *testing.T) { - podWithSubpaths := func() *api.Pod { - return &api.Pod{ - Spec: api.PodSpec{ - RestartPolicy: api.RestartPolicyNever, - Containers: []api.Container{{Name: "container1", Image: "testimage", VolumeMounts: []api.VolumeMount{{Name: "a", SubPath: "foo"}, {Name: "a", SubPath: "foo2"}, {Name: "a", SubPath: "foo3"}}}}, - InitContainers: []api.Container{{Name: "container1", Image: "testimage", VolumeMounts: []api.VolumeMount{{Name: "a", SubPath: "foo"}, {Name: "a", SubPath: "foo2"}}}}, - Volumes: []api.Volume{{Name: "a", VolumeSource: api.VolumeSource{HostPath: &api.HostPathVolumeSource{Path: "/dev/xvdc"}}}}, - }, - } - } - podWithoutSubpaths := func() *api.Pod { - return &api.Pod{ - Spec: api.PodSpec{ - RestartPolicy: api.RestartPolicyNever, - Containers: []api.Container{{Name: "container1", Image: "testimage", VolumeMounts: []api.VolumeMount{{Name: "a", SubPath: ""}, {Name: "a", SubPath: ""}, {Name: "a", SubPath: ""}}}}, - InitContainers: []api.Container{{Name: "container1", Image: "testimage", VolumeMounts: []api.VolumeMount{{Name: "a", SubPath: ""}, {Name: "a", SubPath: ""}}}}, - Volumes: []api.Volume{{Name: "a", VolumeSource: api.VolumeSource{HostPath: &api.HostPathVolumeSource{Path: "/dev/xvdc"}}}}, - }, - } - } - - podInfo := []struct { - description string - hasSubpaths bool - pod func() *api.Pod - }{ - { - description: "has subpaths", - hasSubpaths: true, - pod: podWithSubpaths, - }, - { - description: "does not have subpaths", - hasSubpaths: false, - pod: podWithoutSubpaths, - }, - { - description: "is nil", - hasSubpaths: false, - pod: func() *api.Pod { return nil }, - }, - } - - for _, enabled := range []bool{true, false} { - for _, oldPodInfo := range podInfo { - for _, newPodInfo := range podInfo { - oldPodHasSubpaths, oldPod := oldPodInfo.hasSubpaths, oldPodInfo.pod() - newPodHasSubpaths, newPod := newPodInfo.hasSubpaths, 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 featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.VolumeSubpath, enabled)() - - var oldPodSpec *api.PodSpec - if oldPod != nil { - oldPodSpec = &oldPod.Spec - } - dropDisabledFields(&newPod.Spec, nil, oldPodSpec, nil) - - // old pod should never be changed - if !reflect.DeepEqual(oldPod, oldPodInfo.pod()) { - t.Errorf("old pod changed: %v", cmp.Diff(oldPod, oldPodInfo.pod())) - } - - switch { - case enabled || oldPodHasSubpaths: - // new pod should not be changed if the feature is enabled, or if the old pod had subpaths - if !reflect.DeepEqual(newPod, newPodInfo.pod()) { - t.Errorf("new pod changed: %v", cmp.Diff(newPod, newPodInfo.pod())) - } - case newPodHasSubpaths: - // new pod should be changed - if reflect.DeepEqual(newPod, newPodInfo.pod()) { - t.Errorf("new pod was not changed") - } - // new pod should not have subpaths - if !reflect.DeepEqual(newPod, podWithoutSubpaths()) { - t.Errorf("new pod had subpaths: %v", cmp.Diff(newPod, podWithoutSubpaths())) - } - default: - // new pod should not need to be changed - if !reflect.DeepEqual(newPod, newPodInfo.pod()) { - t.Errorf("new pod changed: %v", cmp.Diff(newPod, newPodInfo.pod())) - } - } - }) - } - } - } -} - func TestDropProcMount(t *testing.T) { procMount := api.UnmaskedProcMount defaultProcMount := api.DefaultProcMount @@ -1046,100 +952,6 @@ func TestDropAppArmor(t *testing.T) { } } -func TestDropSubPathExpr(t *testing.T) { - podWithSubpaths := func() *api.Pod { - return &api.Pod{ - Spec: api.PodSpec{ - RestartPolicy: api.RestartPolicyNever, - Containers: []api.Container{{Name: "container1", Image: "testimage", VolumeMounts: []api.VolumeMount{{Name: "a", SubPathExpr: "foo"}, {Name: "a", SubPathExpr: "foo2"}, {Name: "a", SubPathExpr: "foo3"}}}}, - InitContainers: []api.Container{{Name: "container1", Image: "testimage", VolumeMounts: []api.VolumeMount{{Name: "a", SubPathExpr: "foo"}, {Name: "a", SubPathExpr: "foo2"}}}}, - Volumes: []api.Volume{{Name: "a", VolumeSource: api.VolumeSource{HostPath: &api.HostPathVolumeSource{Path: "/dev/xvdc"}}}}, - }, - } - } - podWithoutSubpaths := func() *api.Pod { - return &api.Pod{ - Spec: api.PodSpec{ - RestartPolicy: api.RestartPolicyNever, - Containers: []api.Container{{Name: "container1", Image: "testimage", VolumeMounts: []api.VolumeMount{{Name: "a", SubPathExpr: ""}, {Name: "a", SubPathExpr: ""}, {Name: "a", SubPathExpr: ""}}}}, - InitContainers: []api.Container{{Name: "container1", Image: "testimage", VolumeMounts: []api.VolumeMount{{Name: "a", SubPathExpr: ""}, {Name: "a", SubPathExpr: ""}}}}, - Volumes: []api.Volume{{Name: "a", VolumeSource: api.VolumeSource{HostPath: &api.HostPathVolumeSource{Path: "/dev/xvdc"}}}}, - }, - } - } - - podInfo := []struct { - description string - hasSubpaths bool - pod func() *api.Pod - }{ - { - description: "has subpaths", - hasSubpaths: true, - pod: podWithSubpaths, - }, - { - description: "does not have subpaths", - hasSubpaths: false, - pod: podWithoutSubpaths, - }, - { - description: "is nil", - hasSubpaths: false, - pod: func() *api.Pod { return nil }, - }, - } - - for _, enabled := range []bool{true, false} { - for _, oldPodInfo := range podInfo { - for _, newPodInfo := range podInfo { - oldPodHasSubpaths, oldPod := oldPodInfo.hasSubpaths, oldPodInfo.pod() - newPodHasSubpaths, newPod := newPodInfo.hasSubpaths, 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 featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.VolumeSubpath, enabled)() - - var oldPodSpec *api.PodSpec - if oldPod != nil { - oldPodSpec = &oldPod.Spec - } - dropDisabledFields(&newPod.Spec, nil, oldPodSpec, nil) - - // old pod should never be changed - if !reflect.DeepEqual(oldPod, oldPodInfo.pod()) { - t.Errorf("old pod changed: %v", cmp.Diff(oldPod, oldPodInfo.pod())) - } - - switch { - case enabled || oldPodHasSubpaths: - // new pod should not be changed if the feature is enabled, or if the old pod had subpaths - if !reflect.DeepEqual(newPod, newPodInfo.pod()) { - t.Errorf("new pod changed: %v", cmp.Diff(newPod, newPodInfo.pod())) - } - case newPodHasSubpaths: - // new pod should be changed - if reflect.DeepEqual(newPod, newPodInfo.pod()) { - t.Errorf("new pod was not changed") - } - // new pod should not have subpaths - if !reflect.DeepEqual(newPod, podWithoutSubpaths()) { - t.Errorf("new pod had subpaths: %v", cmp.Diff(newPod, podWithoutSubpaths())) - } - default: - // new pod should not need to be changed - if !reflect.DeepEqual(newPod, newPodInfo.pod()) { - t.Errorf("new pod changed: %v", cmp.Diff(newPod, newPodInfo.pod())) - } - } - }) - } - } - } -} - func TestDropProbeGracePeriod(t *testing.T) { podWithProbeGracePeriod := func() *api.Pod { livenessGracePeriod := int64(10) diff --git a/pkg/apis/core/validation/validation_test.go b/pkg/apis/core/validation/validation_test.go index 64cdf96bd7a..cc2c52845c8 100644 --- a/pkg/apis/core/validation/validation_test.go +++ b/pkg/apis/core/validation/validation_test.go @@ -5586,71 +5586,7 @@ func TestValidateVolumeMounts(t *testing.T) { } } -func TestValidateDisabledSubpath(t *testing.T) { - defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.VolumeSubpath, false)() - - volumes := []core.Volume{ - {Name: "abc", VolumeSource: core.VolumeSource{PersistentVolumeClaim: &core.PersistentVolumeClaimVolumeSource{ClaimName: "testclaim1"}}}, - {Name: "abc-123", VolumeSource: core.VolumeSource{PersistentVolumeClaim: &core.PersistentVolumeClaimVolumeSource{ClaimName: "testclaim2"}}}, - {Name: "123", VolumeSource: core.VolumeSource{HostPath: &core.HostPathVolumeSource{Path: "/foo/baz", Type: newHostPathType(string(core.HostPathUnset))}}}, - } - vols, v1err := ValidateVolumes(volumes, nil, field.NewPath("field"), PodValidationOptions{}) - if len(v1err) > 0 { - t.Errorf("Invalid test volume - expected success %v", v1err) - return - } - - container := core.Container{ - SecurityContext: nil, - } - - goodVolumeDevices := []core.VolumeDevice{ - {Name: "xyz", DevicePath: "/foofoo"}, - {Name: "uvw", DevicePath: "/foofoo/share/test"}, - } - - cases := map[string]struct { - mounts []core.VolumeMount - expectError bool - }{ - "subpath not specified": { - []core.VolumeMount{ - { - Name: "abc-123", - MountPath: "/bab", - }, - }, - false, - }, - "subpath specified": { - []core.VolumeMount{ - { - Name: "abc-123", - MountPath: "/bab", - SubPath: "baz", - }, - }, - false, // validation should not fail, dropping the field is handled in PrepareForCreate/PrepareForUpdate - }, - } - - for name, test := range cases { - errs := ValidateVolumeMounts(test.mounts, GetVolumeDeviceMap(goodVolumeDevices), vols, &container, field.NewPath("field")) - - if len(errs) != 0 && !test.expectError { - t.Errorf("test %v failed: %+v", name, errs) - } - - if len(errs) == 0 && test.expectError { - t.Errorf("test %v failed, expected error", name) - } - } -} - func TestValidateSubpathMutuallyExclusive(t *testing.T) { - // Enable feature VolumeSubpath - defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.VolumeSubpath, true)() - volumes := []core.Volume{ {Name: "abc", VolumeSource: core.VolumeSource{PersistentVolumeClaim: &core.PersistentVolumeClaimVolumeSource{ClaimName: "testclaim1"}}}, {Name: "abc-123", VolumeSource: core.VolumeSource{PersistentVolumeClaim: &core.PersistentVolumeClaimVolumeSource{ClaimName: "testclaim2"}}}, @@ -5788,45 +5724,6 @@ func TestValidateDisabledSubpathExpr(t *testing.T) { t.Errorf("test %v failed, expected error", name) } } - - // Repeat with subpath feature gate off - defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.VolumeSubpath, false)() - cases = map[string]struct { - mounts []core.VolumeMount - expectError bool - }{ - "subpath expr not specified": { - []core.VolumeMount{ - { - Name: "abc-123", - MountPath: "/bab", - }, - }, - false, - }, - "subpath expr specified": { - []core.VolumeMount{ - { - Name: "abc-123", - MountPath: "/bab", - SubPathExpr: "$(POD_NAME)", - }, - }, - false, // validation should not fail, dropping the field is handled in PrepareForCreate/PrepareForUpdate - }, - } - - for name, test := range cases { - errs := ValidateVolumeMounts(test.mounts, GetVolumeDeviceMap(goodVolumeDevices), vols, &container, field.NewPath("field")) - - if len(errs) != 0 && !test.expectError { - t.Errorf("test %v failed: %+v", name, errs) - } - - if len(errs) == 0 && test.expectError { - t.Errorf("test %v failed, expected error", name) - } - } } func TestValidateMountPropagation(t *testing.T) { diff --git a/pkg/features/kube_features.go b/pkg/features/kube_features.go index a36ef8c5cbe..16268891653 100644 --- a/pkg/features/kube_features.go +++ b/pkg/features/kube_features.go @@ -156,13 +156,6 @@ const ( // to the API server. BoundServiceAccountTokenVolume featuregate.Feature = "BoundServiceAccountTokenVolume" - // owner: @saad-ali - // ga: v1.10 - // - // Allow mounting a subpath of a volume in a container - // Do not remove this feature gate even though it's GA - VolumeSubpath featuregate.Feature = "VolumeSubpath" - // owner: @pohly // alpha: v1.14 // beta: v1.16 @@ -814,7 +807,6 @@ var defaultKubernetesFeatureGates = map[featuregate.Feature]featuregate.FeatureS InTreePluginvSphereUnregister: {Default: false, PreRelease: featuregate.Alpha}, CSIMigrationOpenStack: {Default: true, PreRelease: featuregate.Beta}, InTreePluginOpenStackUnregister: {Default: false, PreRelease: featuregate.Alpha}, - VolumeSubpath: {Default: true, PreRelease: featuregate.GA}, ConfigurableFSGroupPolicy: {Default: true, PreRelease: featuregate.Beta}, CSIInlineVolume: {Default: true, PreRelease: featuregate.Beta}, CSIStorageCapacity: {Default: true, PreRelease: featuregate.Beta}, diff --git a/pkg/kubelet/kubelet_pods.go b/pkg/kubelet/kubelet_pods.go index a6eb12e8f28..d68fa45f9be 100644 --- a/pkg/kubelet/kubelet_pods.go +++ b/pkg/kubelet/kubelet_pods.go @@ -186,10 +186,6 @@ func makeMounts(pod *v1.Pod, podDir string, container *v1.Container, hostName, h subPath := mount.SubPath if mount.SubPathExpr != "" { - if !utilfeature.DefaultFeatureGate.Enabled(features.VolumeSubpath) { - return nil, cleanupAction, fmt.Errorf("volume subpaths are disabled") - } - subPath, err = kubecontainer.ExpandContainerVolumeMounts(mount, expandEnvs) if err != nil { @@ -198,10 +194,6 @@ func makeMounts(pod *v1.Pod, podDir string, container *v1.Container, hostName, h } if subPath != "" { - if !utilfeature.DefaultFeatureGate.Enabled(features.VolumeSubpath) { - return nil, cleanupAction, fmt.Errorf("volume subpaths are disabled") - } - if filepath.IsAbs(subPath) { return nil, cleanupAction, fmt.Errorf("error SubPath `%s` must not be an absolute path", subPath) } diff --git a/pkg/kubelet/kubelet_pods_test.go b/pkg/kubelet/kubelet_pods_test.go index 1dd1da19744..561eead7fae 100644 --- a/pkg/kubelet/kubelet_pods_test.go +++ b/pkg/kubelet/kubelet_pods_test.go @@ -56,65 +56,8 @@ import ( "k8s.io/kubernetes/pkg/kubelet/cri/streaming/remotecommand" "k8s.io/kubernetes/pkg/kubelet/prober/results" kubetypes "k8s.io/kubernetes/pkg/kubelet/types" - "k8s.io/kubernetes/pkg/volume/util/hostutil" - "k8s.io/kubernetes/pkg/volume/util/subpath" ) -func TestDisabledSubpath(t *testing.T) { - fhu := hostutil.NewFakeHostUtil(nil) - fsp := &subpath.FakeSubpath{} - pod := v1.Pod{ - Spec: v1.PodSpec{ - HostNetwork: true, - }, - } - podVolumes := kubecontainer.VolumeMap{ - "disk": kubecontainer.VolumeInfo{Mounter: &stubVolume{path: "/mnt/disk"}}, - } - - cases := map[string]struct { - container v1.Container - expectError bool - }{ - "subpath not specified": { - v1.Container{ - VolumeMounts: []v1.VolumeMount{ - { - MountPath: "/mnt/path3", - Name: "disk", - ReadOnly: true, - }, - }, - }, - false, - }, - "subpath specified": { - v1.Container{ - VolumeMounts: []v1.VolumeMount{ - { - MountPath: "/mnt/path3", - SubPath: "/must/not/be/absolute", - Name: "disk", - ReadOnly: true, - }, - }, - }, - true, - }, - } - - defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.VolumeSubpath, false)() - for name, test := range cases { - _, _, err := makeMounts(&pod, "/pod", &test.container, "fakepodname", "", []string{}, podVolumes, fhu, fsp, nil, false) - if err != nil && !test.expectError { - t.Errorf("test %v failed: %v", name, err) - } - if err == nil && test.expectError { - t.Errorf("test %v failed: expected error", name) - } - } -} - func TestNodeHostsFileContent(t *testing.T) { testCases := []struct { hostsFileName string