github/kubernetes
1
0
Fork 0
mirror of https://github.com/k3s-io/kubernetes.git synced 2025-07-24 12:15:52 +00:00
Code Issues Projects Releases Wiki Activity

Remove VolumeSubpath feature gate

Browse Source 
Remove the VolumeSubpath feature gate.

Feature gate convention has been updated since this was introduced to
indicate that they "are intended to be deprecated and removed after a
feature becomes GA or is dropped.".
This commit is contained in:
saad-ali 2021-09-17 00:34:39 -07:00
parent 9918aa1e03
commit beb17fe10b
6 changed files with 0 additions and 423 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

59
pkg/api/pod/util.go
View File

@ -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 {

188
pkg/api/pod/util_test.go
View File

@ -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)

103
pkg/apis/core/validation/validation_test.go
View File

@ -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) {

8
pkg/features/kube_features.go
View File

@ -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},

8
pkg/kubelet/kubelet_pods.go
View File

@ -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)
}

57
pkg/kubelet/kubelet_pods_test.go
View File

@ -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