diff --git a/api/openapi-spec/swagger.json b/api/openapi-spec/swagger.json index f22e418f637..cb3665ce795 100644 --- a/api/openapi-spec/swagger.json +++ b/api/openapi-spec/swagger.json @@ -11917,7 +11917,7 @@ "type": "string" }, "subPathExpr": { - "description": "Expanded path within the volume from which the container's volume should be mounted. Behaves similarly to SubPath but environment variable references $(VAR_NAME) are expanded using the container's environment. Defaults to \"\" (volume's root). SubPathExpr and SubPath are mutually exclusive. This field is beta in 1.15.", + "description": "Expanded path within the volume from which the container's volume should be mounted. Behaves similarly to SubPath but environment variable references $(VAR_NAME) are expanded using the container's environment. Defaults to \"\" (volume's root). SubPathExpr and SubPath are mutually exclusive.", "type": "string" } }, diff --git a/pkg/api/pod/util_test.go b/pkg/api/pod/util_test.go index daaa1a14a77..93952681008 100644 --- a/pkg/api/pod/util_test.go +++ b/pkg/api/pod/util_test.go @@ -1935,52 +1935,51 @@ func TestDropSubPathExpr(t *testing.T) { }, } - 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 + enabled := true + 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.VolumeSubpathEnvExpansion, 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", diff.ObjectReflectDiff(oldPod, oldPodInfo.pod())) } - 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.VolumeSubpathEnvExpansion, enabled)() - - var oldPodSpec *api.PodSpec - if oldPod != nil { - oldPodSpec = &oldPod.Spec + 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", diff.ObjectReflectDiff(newPod, newPodInfo.pod())) } - dropDisabledFields(&newPod.Spec, nil, oldPodSpec, nil) - - // old pod should never be changed - if !reflect.DeepEqual(oldPod, oldPodInfo.pod()) { - t.Errorf("old pod changed: %v", diff.ObjectReflectDiff(oldPod, oldPodInfo.pod())) + case newPodHasSubpaths: + // new pod should be changed + if reflect.DeepEqual(newPod, newPodInfo.pod()) { + t.Errorf("new pod was not changed") } - - 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", diff.ObjectReflectDiff(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", diff.ObjectReflectDiff(newPod, podWithoutSubpaths())) - } - 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())) - } + // new pod should not have subpaths + if !reflect.DeepEqual(newPod, podWithoutSubpaths()) { + t.Errorf("new pod had subpaths: %v", diff.ObjectReflectDiff(newPod, podWithoutSubpaths())) } - }) - } + 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())) + } + } + }) } } } diff --git a/pkg/apis/core/types.go b/pkg/apis/core/types.go index e62b076e112..64aaf46ce84 100644 --- a/pkg/apis/core/types.go +++ b/pkg/apis/core/types.go @@ -1715,7 +1715,6 @@ type VolumeMount struct { // Behaves similarly to SubPath but environment variable references $(VAR_NAME) are expanded using the container's environment. // Defaults to "" (volume's root). // SubPathExpr and SubPath are mutually exclusive. - // This field is beta in 1.15. // +optional SubPathExpr string } diff --git a/pkg/apis/core/validation/validation_test.go b/pkg/apis/core/validation/validation_test.go index 49ff99ae4d5..dd935f33800 100644 --- a/pkg/apis/core/validation/validation_test.go +++ b/pkg/apis/core/validation/validation_test.go @@ -5160,45 +5160,6 @@ func TestValidateDisabledSubpathExpr(t *testing.T) { } } - // Repeat with feature gate off - defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.VolumeSubpathEnvExpansion, 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) - } - } - // Repeat with subpath feature gate off defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.VolumeSubpath, false)() cases = map[string]struct { diff --git a/pkg/features/kube_features.go b/pkg/features/kube_features.go index 2a15da2263b..8bb0193ca72 100644 --- a/pkg/features/kube_features.go +++ b/pkg/features/kube_features.go @@ -299,7 +299,9 @@ const ( BalanceAttachedNodeVolumes featuregate.Feature = "BalanceAttachedNodeVolumes" // owner: @kevtaylor + // alpha: v1.14 // beta: v1.15 + // ga: v1.17 // // Allow subpath environment variable substitution // Only applicable if the VolumeSubpath feature is also enabled @@ -539,7 +541,7 @@ var defaultKubernetesFeatureGates = map[featuregate.Feature]featuregate.FeatureS CSIMigrationOpenStack: {Default: false, PreRelease: featuregate.Alpha}, VolumeSubpath: {Default: true, PreRelease: featuregate.GA}, BalanceAttachedNodeVolumes: {Default: false, PreRelease: featuregate.Alpha}, - VolumeSubpathEnvExpansion: {Default: true, PreRelease: featuregate.Beta}, + VolumeSubpathEnvExpansion: {Default: true, PreRelease: featuregate.GA, LockToDefault: true}, // remove in 1.19, ResourceQuotaScopeSelectors: {Default: true, PreRelease: featuregate.Beta}, CSIBlockVolume: {Default: true, PreRelease: featuregate.Beta}, CSIInlineVolume: {Default: true, PreRelease: featuregate.Beta}, diff --git a/staging/src/k8s.io/api/core/v1/generated.proto b/staging/src/k8s.io/api/core/v1/generated.proto index b99d1044277..7628ce22c02 100644 --- a/staging/src/k8s.io/api/core/v1/generated.proto +++ b/staging/src/k8s.io/api/core/v1/generated.proto @@ -5032,7 +5032,6 @@ message VolumeMount { // Behaves similarly to SubPath but environment variable references $(VAR_NAME) are expanded using the container's environment. // Defaults to "" (volume's root). // SubPathExpr and SubPath are mutually exclusive. - // This field is beta in 1.15. // +optional optional string subPathExpr = 6; } diff --git a/staging/src/k8s.io/api/core/v1/types.go b/staging/src/k8s.io/api/core/v1/types.go index 50c8ad70f6d..21aab328ba7 100644 --- a/staging/src/k8s.io/api/core/v1/types.go +++ b/staging/src/k8s.io/api/core/v1/types.go @@ -1784,7 +1784,6 @@ type VolumeMount struct { // Behaves similarly to SubPath but environment variable references $(VAR_NAME) are expanded using the container's environment. // Defaults to "" (volume's root). // SubPathExpr and SubPath are mutually exclusive. - // This field is beta in 1.15. // +optional SubPathExpr string `json:"subPathExpr,omitempty" protobuf:"bytes,6,opt,name=subPathExpr"` } diff --git a/staging/src/k8s.io/api/core/v1/types_swagger_doc_generated.go b/staging/src/k8s.io/api/core/v1/types_swagger_doc_generated.go index 35b8389a750..422020bad42 100644 --- a/staging/src/k8s.io/api/core/v1/types_swagger_doc_generated.go +++ b/staging/src/k8s.io/api/core/v1/types_swagger_doc_generated.go @@ -2366,7 +2366,7 @@ var map_VolumeMount = map[string]string{ "mountPath": "Path within the container at which the volume should be mounted. Must not contain ':'.", "subPath": "Path within the volume from which the container's volume should be mounted. Defaults to \"\" (volume's root).", "mountPropagation": "mountPropagation determines how mounts are propagated from the host to container and the other way around. When not set, MountPropagationNone is used. This field is beta in 1.10.", - "subPathExpr": "Expanded path within the volume from which the container's volume should be mounted. Behaves similarly to SubPath but environment variable references $(VAR_NAME) are expanded using the container's environment. Defaults to \"\" (volume's root). SubPathExpr and SubPath are mutually exclusive. This field is beta in 1.15.", + "subPathExpr": "Expanded path within the volume from which the container's volume should be mounted. Behaves similarly to SubPath but environment variable references $(VAR_NAME) are expanded using the container's environment. Defaults to \"\" (volume's root). SubPathExpr and SubPath are mutually exclusive.", } func (VolumeMount) SwaggerDoc() map[string]string { diff --git a/test/e2e/common/expansion.go b/test/e2e/common/expansion.go index 1b938877219..5116a6b97cd 100644 --- a/test/e2e/common/expansion.go +++ b/test/e2e/common/expansion.go @@ -158,7 +158,7 @@ var _ = framework.KubeDescribe("Variable Expansion", func() { Description: Make sure a container's subpath can be set using an expansion of environment variables. */ - ginkgo.It("should allow substituting values in a volume subpath [sig-storage][NodeFeature:VolumeSubpathEnvExpansion]", func() { + ginkgo.It("should allow substituting values in a volume subpath [sig-storage]", func() { podName := "var-expansion-" + string(uuid.NewUUID()) pod := &v1.Pod{ ObjectMeta: metav1.ObjectMeta{ @@ -218,7 +218,7 @@ var _ = framework.KubeDescribe("Variable Expansion", func() { Description: Make sure a container's subpath can not be set using an expansion of environment variables when backticks are supplied. */ - ginkgo.It("should fail substituting values in a volume subpath with backticks [sig-storage][NodeFeature:VolumeSubpathEnvExpansion][Slow]", func() { + ginkgo.It("should fail substituting values in a volume subpath with backticks [sig-storage][Slow]", func() { podName := "var-expansion-" + string(uuid.NewUUID()) pod := &v1.Pod{ @@ -267,7 +267,7 @@ var _ = framework.KubeDescribe("Variable Expansion", func() { Description: Make sure a container's subpath can not be set using an expansion of environment variables when absolute path is supplied. */ - ginkgo.It("should fail substituting values in a volume subpath with absolute path [sig-storage][NodeFeature:VolumeSubpathEnvExpansion][Slow]", func() { + ginkgo.It("should fail substituting values in a volume subpath with absolute path [sig-storage][Slow]", func() { podName := "var-expansion-" + string(uuid.NewUUID()) pod := &v1.Pod{ @@ -315,7 +315,7 @@ var _ = framework.KubeDescribe("Variable Expansion", func() { Testname: var-expansion-subpath-ready-from-failed-state Description: Verify that a failing subpath expansion can be modified during the lifecycle of a container. */ - ginkgo.It("should verify that a failing subpath expansion can be modified during the lifecycle of a container [sig-storage][NodeFeature:VolumeSubpathEnvExpansion][Slow]", func() { + ginkgo.It("should verify that a failing subpath expansion can be modified during the lifecycle of a container [sig-storage][Slow]", func() { podName := "var-expansion-" + string(uuid.NewUUID()) containerName := "dapi-container" @@ -406,7 +406,7 @@ var _ = framework.KubeDescribe("Variable Expansion", func() { 3. successful expansion of the subpathexpr isn't required for volume cleanup */ - ginkgo.It("should succeed in writing subpaths in container [sig-storage][NodeFeature:VolumeSubpathEnvExpansion][Slow]", func() { + ginkgo.It("should succeed in writing subpaths in container [sig-storage][Slow]", func() { podName := "var-expansion-" + string(uuid.NewUUID()) containerName := "dapi-container" @@ -515,7 +515,7 @@ var _ = framework.KubeDescribe("Variable Expansion", func() { */ - ginkgo.It("should not change the subpath mount on a container restart if the environment variable changes [sig-storage][NodeFeature:VolumeSubpathEnvExpansion][Slow]", func() { + ginkgo.It("should not change the subpath mount on a container restart if the environment variable changes [sig-storage][Slow]", func() { suffix := string(uuid.NewUUID()) podName := fmt.Sprintf("var-expansion-%s", suffix)