github
/
k3s
mirror of https://github.com/k3s-io/k3s
1
0
Fork 0
Code Issues Projects Releases Wiki Activity

Merge pull request #70490 from liggitt/tolerate-existing-subpath 

tolerate existing subpath on pod update when feature is disabled
pull/564/head
Kubernetes Prow Robot 2018-12-18 11:16:50 -08:00 committed by GitHub
parent b97092ca8f c4a0254201
commit f4487a06fe
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
14 changed files with 226 additions and 49 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
pkg/api/pod/BUILD
View File

@ -38,6 +38,7 @@ go_test(
deps = [
"//pkg/apis/core:go_default_library",
"//pkg/features:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/util/diff:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/util/sets:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/util/validation/field:go_default_library",
"//staging/src/k8s.io/apiserver/pkg/util/feature:go_default_library",

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

@ -232,12 +232,16 @@ func UpdatePodCondition(status *api.PodStatus, condition *api.PodCondition) bool
return !isEqual
}
// DropDisabledAlphaFields removes disabled fields from the pod spec.
// DropDisabledFields removes disabled fields from the pod spec.
// This should be called from PrepareForCreate/PrepareForUpdate for all resources containing a pod spec.
func DropDisabledAlphaFields(podSpec *api.PodSpec) {
func DropDisabledFields(podSpec, oldPodSpec *api.PodSpec) {
if !utilfeature.DefaultFeatureGate.Enabled(features.PodPriority) {
podSpec.Priority = nil
podSpec.PriorityClassName = ""
if oldPodSpec != nil {
oldPodSpec.Priority = nil
oldPodSpec.PriorityClassName = ""
}
}
if !utilfeature.DefaultFeatureGate.Enabled(features.LocalStorageCapacityIsolation) {
@ -246,22 +250,46 @@ func DropDisabledAlphaFields(podSpec *api.PodSpec) {
podSpec.Volumes[i].EmptyDir.SizeLimit = nil
}
}
if oldPodSpec != nil {
for i := range oldPodSpec.Volumes {
if oldPodSpec.Volumes[i].EmptyDir != nil {
oldPodSpec.Volumes[i].EmptyDir.SizeLimit = nil
}
}
}
}
DropDisabledVolumeDevicesAlphaFields(podSpec)
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
for i := range podSpec.Containers {
for j := range podSpec.Containers[i].VolumeMounts {
podSpec.Containers[i].VolumeMounts[j].SubPath = ""
}
}
for i := range podSpec.InitContainers {
for j := range podSpec.InitContainers[i].VolumeMounts {
podSpec.InitContainers[i].VolumeMounts[j].SubPath = ""
}
}
}
DropDisabledRunAsGroupField(podSpec)
dropDisabledVolumeDevicesAlphaFields(podSpec, oldPodSpec)
if !utilfeature.DefaultFeatureGate.Enabled(features.RuntimeClass) && podSpec.RuntimeClassName != nil {
dropDisabledRunAsGroupField(podSpec, oldPodSpec)
if !utilfeature.DefaultFeatureGate.Enabled(features.RuntimeClass) {
podSpec.RuntimeClassName = nil
if oldPodSpec != nil {
oldPodSpec.RuntimeClassName = nil
}
}
DropDisabledProcMountField(podSpec)
dropDisabledProcMountField(podSpec, oldPodSpec)
}
// DropDisabledRunAsGroupField removes disabled fields from PodSpec related
// dropDisabledRunAsGroupField removes disabled fields from PodSpec related
// to RunAsGroup
func DropDisabledRunAsGroupField(podSpec *api.PodSpec) {
func dropDisabledRunAsGroupField(podSpec, oldPodSpec *api.PodSpec) {
if !utilfeature.DefaultFeatureGate.Enabled(features.RunAsGroup) {
if podSpec.SecurityContext != nil {
podSpec.SecurityContext.RunAsGroup = nil
@ -276,12 +304,28 @@ func DropDisabledRunAsGroupField(podSpec *api.PodSpec) {
podSpec.InitContainers[i].SecurityContext.RunAsGroup = nil
}
}
if oldPodSpec != nil {
if oldPodSpec.SecurityContext != nil {
oldPodSpec.SecurityContext.RunAsGroup = nil
}
for i := range oldPodSpec.Containers {
if oldPodSpec.Containers[i].SecurityContext != nil {
oldPodSpec.Containers[i].SecurityContext.RunAsGroup = nil
}
}
for i := range oldPodSpec.InitContainers {
if oldPodSpec.InitContainers[i].SecurityContext != nil {
oldPodSpec.InitContainers[i].SecurityContext.RunAsGroup = nil
}
}
}
}
}
// DropDisabledProcMountField removes disabled fields from PodSpec related
// dropDisabledProcMountField removes disabled fields from PodSpec related
// to ProcMount
func DropDisabledProcMountField(podSpec *api.PodSpec) {
func dropDisabledProcMountField(podSpec, oldPodSpec *api.PodSpec) {
if !utilfeature.DefaultFeatureGate.Enabled(features.ProcMountType) {
defProcMount := api.DefaultProcMount
for i := range podSpec.Containers {
@ -294,12 +338,25 @@ func DropDisabledProcMountField(podSpec *api.PodSpec) {
podSpec.InitContainers[i].SecurityContext.ProcMount = &defProcMount
}
}
if oldPodSpec != nil {
for i := range oldPodSpec.Containers {
if oldPodSpec.Containers[i].SecurityContext != nil {
oldPodSpec.Containers[i].SecurityContext.ProcMount = &defProcMount
}
}
for i := range oldPodSpec.InitContainers {
if oldPodSpec.InitContainers[i].SecurityContext != nil {
oldPodSpec.InitContainers[i].SecurityContext.ProcMount = &defProcMount
}
}
}
}
}
// DropDisabledVolumeDevicesAlphaFields removes disabled fields from []VolumeDevice.
// dropDisabledVolumeDevicesAlphaFields removes disabled fields from []VolumeDevice.
// This should be called from PrepareForCreate/PrepareForUpdate for all resources containing a VolumeDevice
func DropDisabledVolumeDevicesAlphaFields(podSpec *api.PodSpec) {
func dropDisabledVolumeDevicesAlphaFields(podSpec, oldPodSpec *api.PodSpec) {
if !utilfeature.DefaultFeatureGate.Enabled(features.BlockVolume) {
for i := range podSpec.Containers {
podSpec.Containers[i].VolumeDevices = nil
@ -307,5 +364,36 @@ func DropDisabledVolumeDevicesAlphaFields(podSpec *api.PodSpec) {
for i := range podSpec.InitContainers {
podSpec.InitContainers[i].VolumeDevices = nil
}
if oldPodSpec != nil {
for i := range oldPodSpec.Containers {
oldPodSpec.Containers[i].VolumeDevices = nil
}
for i := range oldPodSpec.InitContainers {
oldPodSpec.InitContainers[i].VolumeDevices = nil
}
}
}
}
// 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
}
for i := range podSpec.Containers {
for j := range podSpec.Containers[i].VolumeMounts {
if len(podSpec.Containers[i].VolumeMounts[j].SubPath) > 0 {
return true
}
}
}
for i := range podSpec.InitContainers {
for j := range podSpec.InitContainers[i].VolumeMounts {
if len(podSpec.InitContainers[i].VolumeMounts[j].SubPath) > 0 {
return true
}
}
}
return false
}

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

@ -17,10 +17,12 @@ limitations under the License.
package pod
import (
"fmt"
"reflect"
"strings"
"testing"
"k8s.io/apimachinery/pkg/util/diff"
"k8s.io/apimachinery/pkg/util/sets"
"k8s.io/apimachinery/pkg/util/validation/field"
utilfeature "k8s.io/apiserver/pkg/util/feature"
@ -307,7 +309,7 @@ func TestDropAlphaVolumeDevices(t *testing.T) {
defer utilfeaturetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.BlockVolume, true)()
// now test dropping the fields - should not be dropped
DropDisabledAlphaFields(&testPod.Spec)
DropDisabledFields(&testPod.Spec, nil)
// check to make sure VolumeDevices is still present
// if featureset is set to true
@ -322,14 +324,108 @@ func TestDropAlphaVolumeDevices(t *testing.T) {
defer utilfeaturetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.BlockVolume, false)()
// now test dropping the fields
DropDisabledAlphaFields(&testPod.Spec)
DropDisabledFields(&testPod.Spec, nil)
// check to make sure VolumeDevices is nil
// if featureset is set to false
if testPod.Spec.Containers[0].VolumeDevices != nil {
t.Error("DropDisabledAlphaFields for Containers failed")
t.Error("DropDisabledFields for Containers failed")
}
if testPod.Spec.InitContainers[0].VolumeDevices != nil {
t.Error("DropDisabledAlphaFields for InitContainers failed")
t.Error("DropDisabledFields for InitContainers failed")
}
}
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 utilfeaturetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.VolumeSubpath, 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 || 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()))
}
}
})
}
}
}
}

6
pkg/apis/core/validation/validation.go
View File

@ -2299,11 +2299,7 @@ func ValidateVolumeMounts(mounts []core.VolumeMount, voldevices map[string]strin
}
if len(mnt.SubPath) > 0 {
if !utilfeature.DefaultFeatureGate.Enabled(features.VolumeSubpath) {
allErrs = append(allErrs, field.Forbidden(fldPath.Child("subPath"), "subPath is disabled by feature-gate"))
} else {
allErrs = append(allErrs, validateLocalDescendingPath(mnt.SubPath, fldPath.Child("subPath"))...)
}
allErrs = append(allErrs, validateLocalDescendingPath(mnt.SubPath, fldPath.Child("subPath"))...)
}
if mnt.MountPropagation != nil {

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

@ -4931,7 +4931,7 @@ func TestValidateDisabledSubpath(t *testing.T) {
SubPath: "baz",
},
},
true,
false, // validation should not fail, dropping the field is handled in PrepareForCreate/PrepareForUpdate
},
}

5
pkg/registry/apps/daemonset/strategy.go
View File

@ -75,7 +75,7 @@ func (daemonSetStrategy) PrepareForCreate(ctx context.Context, obj runtime.Objec
daemonSet.Spec.TemplateGeneration = 1
}
pod.DropDisabledAlphaFields(&daemonSet.Spec.Template.Spec)
pod.DropDisabledFields(&daemonSet.Spec.Template.Spec, nil)
}
// PrepareForUpdate clears fields that are not allowed to be set by end users on update.
@ -83,8 +83,7 @@ func (daemonSetStrategy) PrepareForUpdate(ctx context.Context, obj, old runtime.
newDaemonSet := obj.(*apps.DaemonSet)
oldDaemonSet := old.(*apps.DaemonSet)
pod.DropDisabledAlphaFields(&newDaemonSet.Spec.Template.Spec)
pod.DropDisabledAlphaFields(&oldDaemonSet.Spec.Template.Spec)
pod.DropDisabledFields(&newDaemonSet.Spec.Template.Spec, &oldDaemonSet.Spec.Template.Spec)
// update is not allowed to set status
newDaemonSet.Status = oldDaemonSet.Status

5
pkg/registry/apps/deployment/strategy.go
View File

@ -73,7 +73,7 @@ func (deploymentStrategy) PrepareForCreate(ctx context.Context, obj runtime.Obje
deployment.Status = apps.DeploymentStatus{}
deployment.Generation = 1
pod.DropDisabledAlphaFields(&deployment.Spec.Template.Spec)
pod.DropDisabledFields(&deployment.Spec.Template.Spec, nil)
}
// Validate validates a new deployment.
@ -97,8 +97,7 @@ func (deploymentStrategy) PrepareForUpdate(ctx context.Context, obj, old runtime
oldDeployment := old.(*apps.Deployment)
newDeployment.Status = oldDeployment.Status
pod.DropDisabledAlphaFields(&newDeployment.Spec.Template.Spec)
pod.DropDisabledAlphaFields(&oldDeployment.Spec.Template.Spec)
pod.DropDisabledFields(&newDeployment.Spec.Template.Spec, &oldDeployment.Spec.Template.Spec)
// Spec updates bump the generation so that we can distinguish between
// scaling events and template changes, annotation updates bump the generation

5
pkg/registry/apps/replicaset/strategy.go
View File

@ -80,7 +80,7 @@ func (rsStrategy) PrepareForCreate(ctx context.Context, obj runtime.Object) {
rs.Generation = 1
pod.DropDisabledAlphaFields(&rs.Spec.Template.Spec)
pod.DropDisabledFields(&rs.Spec.Template.Spec, nil)
}
// PrepareForUpdate clears fields that are not allowed to be set by end users on update.
@ -90,8 +90,7 @@ func (rsStrategy) PrepareForUpdate(ctx context.Context, obj, old runtime.Object)
// update is not allowed to set status
newRS.Status = oldRS.Status
pod.DropDisabledAlphaFields(&newRS.Spec.Template.Spec)
pod.DropDisabledAlphaFields(&oldRS.Spec.Template.Spec)
pod.DropDisabledFields(&newRS.Spec.Template.Spec, &oldRS.Spec.Template.Spec)
// Any changes to the spec increment the generation number, any changes to the
// status should reflect the generation number of the corresponding object. We push

5
pkg/registry/apps/statefulset/strategy.go
View File

@ -72,7 +72,7 @@ func (statefulSetStrategy) PrepareForCreate(ctx context.Context, obj runtime.Obj
statefulSet.Generation = 1
pod.DropDisabledAlphaFields(&statefulSet.Spec.Template.Spec)
pod.DropDisabledFields(&statefulSet.Spec.Template.Spec, nil)
}
// PrepareForUpdate clears fields that are not allowed to be set by end users on update.
@ -82,8 +82,7 @@ func (statefulSetStrategy) PrepareForUpdate(ctx context.Context, obj, old runtim
// Update is not allowed to set status
newStatefulSet.Status = oldStatefulSet.Status
pod.DropDisabledAlphaFields(&newStatefulSet.Spec.Template.Spec)
pod.DropDisabledAlphaFields(&oldStatefulSet.Spec.Template.Spec)
pod.DropDisabledFields(&newStatefulSet.Spec.Template.Spec, &oldStatefulSet.Spec.Template.Spec)
// Any changes to the spec increment the generation number, any changes to the
// status should reflect the generation number of the corresponding object.

5
pkg/registry/batch/cronjob/strategy.go
View File

@ -68,7 +68,7 @@ func (cronJobStrategy) PrepareForCreate(ctx context.Context, obj runtime.Object)
cronJob := obj.(*batch.CronJob)
cronJob.Status = batch.CronJobStatus{}
pod.DropDisabledAlphaFields(&cronJob.Spec.JobTemplate.Spec.Template.Spec)
pod.DropDisabledFields(&cronJob.Spec.JobTemplate.Spec.Template.Spec, nil)
}
// PrepareForUpdate clears fields that are not allowed to be set by end users on update.
@ -77,8 +77,7 @@ func (cronJobStrategy) PrepareForUpdate(ctx context.Context, obj, old runtime.Ob
oldCronJob := old.(*batch.CronJob)
newCronJob.Status = oldCronJob.Status
pod.DropDisabledAlphaFields(&newCronJob.Spec.JobTemplate.Spec.Template.Spec)
pod.DropDisabledAlphaFields(&oldCronJob.Spec.JobTemplate.Spec.Template.Spec)
pod.DropDisabledFields(&newCronJob.Spec.JobTemplate.Spec.Template.Spec, &oldCronJob.Spec.JobTemplate.Spec.Template.Spec)
}
// Validate validates a new scheduled job.

5
pkg/registry/batch/job/strategy.go
View File

@ -80,7 +80,7 @@ func (jobStrategy) PrepareForCreate(ctx context.Context, obj runtime.Object) {
job.Spec.TTLSecondsAfterFinished = nil
}
pod.DropDisabledAlphaFields(&job.Spec.Template.Spec)
pod.DropDisabledFields(&job.Spec.Template.Spec, nil)
}
// PrepareForUpdate clears fields that are not allowed to be set by end users on update.
@ -94,8 +94,7 @@ func (jobStrategy) PrepareForUpdate(ctx context.Context, obj, old runtime.Object
oldJob.Spec.TTLSecondsAfterFinished = nil
}
pod.DropDisabledAlphaFields(&newJob.Spec.Template.Spec)
pod.DropDisabledAlphaFields(&oldJob.Spec.Template.Spec)
pod.DropDisabledFields(&newJob.Spec.Template.Spec, &oldJob.Spec.Template.Spec)
}
// Validate validates a new job.

5
pkg/registry/core/pod/strategy.go
View File

@ -73,7 +73,7 @@ func (podStrategy) PrepareForCreate(ctx context.Context, obj runtime.Object) {
QOSClass: qos.GetPodQOS(pod),
}
podutil.DropDisabledAlphaFields(&pod.Spec)
podutil.DropDisabledFields(&pod.Spec, nil)
}
// PrepareForUpdate clears fields that are not allowed to be set by end users on update.
@ -82,8 +82,7 @@ func (podStrategy) PrepareForUpdate(ctx context.Context, obj, old runtime.Object
oldPod := old.(*api.Pod)
newPod.Status = oldPod.Status
podutil.DropDisabledAlphaFields(&newPod.Spec)
podutil.DropDisabledAlphaFields(&oldPod.Spec)
podutil.DropDisabledFields(&newPod.Spec, &oldPod.Spec)
}
// Validate validates a new pod.

5
pkg/registry/core/podtemplate/strategy.go
View File

@ -47,7 +47,7 @@ func (podTemplateStrategy) NamespaceScoped() bool {
func (podTemplateStrategy) PrepareForCreate(ctx context.Context, obj runtime.Object) {
template := obj.(*api.PodTemplate)
pod.DropDisabledAlphaFields(&template.Template.Spec)
pod.DropDisabledFields(&template.Template.Spec, nil)
}
// Validate validates a new pod template.
@ -70,8 +70,7 @@ func (podTemplateStrategy) PrepareForUpdate(ctx context.Context, obj, old runtim
newTemplate := obj.(*api.PodTemplate)
oldTemplate := old.(*api.PodTemplate)
pod.DropDisabledAlphaFields(&newTemplate.Template.Spec)
pod.DropDisabledAlphaFields(&oldTemplate.Template.Spec)
pod.DropDisabledFields(&newTemplate.Template.Spec, &oldTemplate.Template.Spec)
}
// ValidateUpdate is the default update validation for an end user.

10
pkg/registry/core/replicationcontroller/strategy.go
View File

@ -81,7 +81,7 @@ func (rcStrategy) PrepareForCreate(ctx context.Context, obj runtime.Object) {
controller.Generation = 1
if controller.Spec.Template != nil {
pod.DropDisabledAlphaFields(&controller.Spec.Template.Spec)
pod.DropDisabledFields(&controller.Spec.Template.Spec, nil)
}
}
@ -92,12 +92,16 @@ func (rcStrategy) PrepareForUpdate(ctx context.Context, obj, old runtime.Object)
// update is not allowed to set status
newController.Status = oldController.Status
var newSpec, oldSpec *api.PodSpec
if oldController.Spec.Template != nil {
pod.DropDisabledAlphaFields(&oldController.Spec.Template.Spec)
oldSpec = &oldController.Spec.Template.Spec
}
if newController.Spec.Template != nil {
pod.DropDisabledAlphaFields(&newController.Spec.Template.Spec)
newSpec = &newController.Spec.Template.Spec
} else {
newSpec = &api.PodSpec{}
}
pod.DropDisabledFields(newSpec, oldSpec)
// Any changes to the spec increment the generation number, any changes to the
// status should reflect the generation number of the corresponding object. We push