Skip to content

Commit

Permalink
Merge pull request #50924 from liggitt/alpha-fields
Browse files Browse the repository at this point in the history
Automatic merge from submit-queue (batch tested with PRs 51682, 51546, 51369, 50924, 51827)

Clear values for disabled alpha fields

Fixes #51831

Before persisting new or updated resources, alpha fields that are disabled by feature gate must be removed from the incoming objects.

This adds a helper for clearing these values for pod specs and calls it from the strategies of all in-tree resources containing pod specs.

Addresses kubernetes/community#869
  • Loading branch information
Kubernetes Submit Queue committed Sep 3, 2017
2 parents e528a6e + 0228189 commit f9a82dd
Show file tree
Hide file tree
Showing 22 changed files with 107 additions and 3 deletions.
2 changes: 2 additions & 0 deletions pkg/api/pod/BUILD
Expand Up @@ -11,7 +11,9 @@ go_library(
srcs = ["util.go"],
deps = [
"//pkg/api:go_default_library",
"//pkg/features:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
"//vendor/k8s.io/apiserver/pkg/util/feature:go_default_library",
],
)

Expand Down
19 changes: 19 additions & 0 deletions pkg/api/pod/util.go
Expand Up @@ -18,7 +18,9 @@ package pod

import (
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
utilfeature "k8s.io/apiserver/pkg/util/feature"
"k8s.io/kubernetes/pkg/api"
"k8s.io/kubernetes/pkg/features"
)

// Visitor is called with each object name, and returns true if visiting should continue
Expand Down Expand Up @@ -225,3 +227,20 @@ func UpdatePodCondition(status *api.PodStatus, condition *api.PodCondition) bool
// Return true if one of the fields have changed.
return !isEqual
}

// DropDisabledAlphaFields 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) {
if !utilfeature.DefaultFeatureGate.Enabled(features.PodPriority) {
podSpec.Priority = nil
podSpec.PriorityClassName = ""
}

if !utilfeature.DefaultFeatureGate.Enabled(features.LocalStorageCapacityIsolation) {
for i := range podSpec.Volumes {
if podSpec.Volumes[i].EmptyDir != nil {
podSpec.Volumes[i].EmptyDir.SizeLimit = nil
}
}
}
}
1 change: 1 addition & 0 deletions pkg/registry/apps/statefulset/BUILD
Expand Up @@ -15,6 +15,7 @@ go_library(
],
deps = [
"//pkg/api:go_default_library",
"//pkg/api/pod:go_default_library",
"//pkg/apis/apps:go_default_library",
"//pkg/apis/apps/validation:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/api/equality:go_default_library",
Expand Down
6 changes: 6 additions & 0 deletions pkg/registry/apps/statefulset/strategy.go
Expand Up @@ -24,6 +24,7 @@ import (
"k8s.io/apiserver/pkg/registry/rest"
"k8s.io/apiserver/pkg/storage/names"
"k8s.io/kubernetes/pkg/api"
"k8s.io/kubernetes/pkg/api/pod"
"k8s.io/kubernetes/pkg/apis/apps"
"k8s.io/kubernetes/pkg/apis/apps/validation"
)
Expand Down Expand Up @@ -55,6 +56,8 @@ func (statefulSetStrategy) PrepareForCreate(ctx genericapirequest.Context, obj r
statefulSet.Status = apps.StatefulSetStatus{}

statefulSet.Generation = 1

pod.DropDisabledAlphaFields(&statefulSet.Spec.Template.Spec)
}

// PrepareForUpdate clears fields that are not allowed to be set by end users on update.
Expand All @@ -64,6 +67,9 @@ func (statefulSetStrategy) PrepareForUpdate(ctx genericapirequest.Context, obj,
// Update is not allowed to set status
newStatefulSet.Status = oldStatefulSet.Status

pod.DropDisabledAlphaFields(&newStatefulSet.Spec.Template.Spec)
pod.DropDisabledAlphaFields(&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.
// See metav1.ObjectMeta description for more information on Generation.
Expand Down
1 change: 1 addition & 0 deletions pkg/registry/batch/cronjob/BUILD
Expand Up @@ -14,6 +14,7 @@ go_library(
],
deps = [
"//pkg/api:go_default_library",
"//pkg/api/pod:go_default_library",
"//pkg/apis/batch:go_default_library",
"//pkg/apis/batch/validation:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/runtime:go_default_library",
Expand Down
6 changes: 6 additions & 0 deletions pkg/registry/batch/cronjob/strategy.go
Expand Up @@ -23,6 +23,7 @@ import (
"k8s.io/apiserver/pkg/registry/rest"
"k8s.io/apiserver/pkg/storage/names"
"k8s.io/kubernetes/pkg/api"
"k8s.io/kubernetes/pkg/api/pod"
"k8s.io/kubernetes/pkg/apis/batch"
"k8s.io/kubernetes/pkg/apis/batch/validation"
)
Expand Down Expand Up @@ -51,13 +52,18 @@ func (cronJobStrategy) NamespaceScoped() bool {
func (cronJobStrategy) PrepareForCreate(ctx genericapirequest.Context, obj runtime.Object) {
cronJob := obj.(*batch.CronJob)
cronJob.Status = batch.CronJobStatus{}

pod.DropDisabledAlphaFields(&cronJob.Spec.JobTemplate.Spec.Template.Spec)
}

// PrepareForUpdate clears fields that are not allowed to be set by end users on update.
func (cronJobStrategy) PrepareForUpdate(ctx genericapirequest.Context, obj, old runtime.Object) {
newCronJob := obj.(*batch.CronJob)
oldCronJob := old.(*batch.CronJob)
newCronJob.Status = oldCronJob.Status

pod.DropDisabledAlphaFields(&newCronJob.Spec.JobTemplate.Spec.Template.Spec)
pod.DropDisabledAlphaFields(&oldCronJob.Spec.JobTemplate.Spec.Template.Spec)
}

// Validate validates a new scheduled job.
Expand Down
1 change: 1 addition & 0 deletions pkg/registry/batch/job/BUILD
Expand Up @@ -14,6 +14,7 @@ go_library(
],
deps = [
"//pkg/api:go_default_library",
"//pkg/api/pod:go_default_library",
"//pkg/apis/batch:go_default_library",
"//pkg/apis/batch/validation:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
Expand Down
6 changes: 6 additions & 0 deletions pkg/registry/batch/job/strategy.go
Expand Up @@ -31,6 +31,7 @@ import (
"k8s.io/apiserver/pkg/storage"
"k8s.io/apiserver/pkg/storage/names"
"k8s.io/kubernetes/pkg/api"
"k8s.io/kubernetes/pkg/api/pod"
"k8s.io/kubernetes/pkg/apis/batch"
"k8s.io/kubernetes/pkg/apis/batch/validation"
)
Expand Down Expand Up @@ -59,13 +60,18 @@ func (jobStrategy) NamespaceScoped() bool {
func (jobStrategy) PrepareForCreate(ctx genericapirequest.Context, obj runtime.Object) {
job := obj.(*batch.Job)
job.Status = batch.JobStatus{}

pod.DropDisabledAlphaFields(&job.Spec.Template.Spec)
}

// PrepareForUpdate clears fields that are not allowed to be set by end users on update.
func (jobStrategy) PrepareForUpdate(ctx genericapirequest.Context, obj, old runtime.Object) {
newJob := obj.(*batch.Job)
oldJob := old.(*batch.Job)
newJob.Status = oldJob.Status

pod.DropDisabledAlphaFields(&newJob.Spec.Template.Spec)
pod.DropDisabledAlphaFields(&oldJob.Spec.Template.Spec)
}

// Validate validates a new job.
Expand Down
2 changes: 2 additions & 0 deletions pkg/registry/core/node/BUILD
Expand Up @@ -16,6 +16,7 @@ go_library(
deps = [
"//pkg/api:go_default_library",
"//pkg/api/validation:go_default_library",
"//pkg/features:go_default_library",
"//pkg/kubelet/client:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/api/errors:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/apis/meta/internalversion:go_default_library",
Expand All @@ -32,6 +33,7 @@ go_library(
"//vendor/k8s.io/apiserver/pkg/registry/rest:go_default_library",
"//vendor/k8s.io/apiserver/pkg/storage:go_default_library",
"//vendor/k8s.io/apiserver/pkg/storage/names:go_default_library",
"//vendor/k8s.io/apiserver/pkg/util/feature:go_default_library",
],
)

Expand Down
13 changes: 12 additions & 1 deletion pkg/registry/core/node/strategy.go
Expand Up @@ -34,8 +34,10 @@ import (
"k8s.io/apiserver/pkg/registry/generic"
pkgstorage "k8s.io/apiserver/pkg/storage"
"k8s.io/apiserver/pkg/storage/names"
utilfeature "k8s.io/apiserver/pkg/util/feature"
"k8s.io/kubernetes/pkg/api"
"k8s.io/kubernetes/pkg/api/validation"
"k8s.io/kubernetes/pkg/features"
"k8s.io/kubernetes/pkg/kubelet/client"
)

Expand All @@ -61,15 +63,24 @@ func (nodeStrategy) AllowCreateOnUpdate() bool {

// PrepareForCreate clears fields that are not allowed to be set by end users on creation.
func (nodeStrategy) PrepareForCreate(ctx genericapirequest.Context, obj runtime.Object) {
_ = obj.(*api.Node)
node := obj.(*api.Node)
// Nodes allow *all* fields, including status, to be set on create.

if !utilfeature.DefaultFeatureGate.Enabled(features.DynamicKubeletConfig) {
node.Spec.ConfigSource = nil
}
}

// PrepareForUpdate clears fields that are not allowed to be set by end users on update.
func (nodeStrategy) PrepareForUpdate(ctx genericapirequest.Context, obj, old runtime.Object) {
newNode := obj.(*api.Node)
oldNode := old.(*api.Node)
newNode.Status = oldNode.Status

if !utilfeature.DefaultFeatureGate.Enabled(features.DynamicKubeletConfig) {
newNode.Spec.ConfigSource = nil
oldNode.Spec.ConfigSource = nil
}
}

// Validate validates a new node.
Expand Down
1 change: 1 addition & 0 deletions pkg/registry/core/pod/BUILD
Expand Up @@ -15,6 +15,7 @@ go_library(
deps = [
"//pkg/api:go_default_library",
"//pkg/api/helper/qos:go_default_library",
"//pkg/api/pod:go_default_library",
"//pkg/api/validation:go_default_library",
"//pkg/kubelet/client:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/api/errors:go_default_library",
Expand Down
6 changes: 6 additions & 0 deletions pkg/registry/core/pod/strategy.go
Expand Up @@ -39,6 +39,7 @@ import (
"k8s.io/apiserver/pkg/storage/names"
"k8s.io/kubernetes/pkg/api"
"k8s.io/kubernetes/pkg/api/helper/qos"
podutil "k8s.io/kubernetes/pkg/api/pod"
"k8s.io/kubernetes/pkg/api/validation"
"k8s.io/kubernetes/pkg/kubelet/client"
)
Expand All @@ -65,13 +66,18 @@ func (podStrategy) PrepareForCreate(ctx genericapirequest.Context, obj runtime.O
Phase: api.PodPending,
QOSClass: qos.GetPodQOS(pod),
}

podutil.DropDisabledAlphaFields(&pod.Spec)
}

// PrepareForUpdate clears fields that are not allowed to be set by end users on update.
func (podStrategy) PrepareForUpdate(ctx genericapirequest.Context, obj, old runtime.Object) {
newPod := obj.(*api.Pod)
oldPod := old.(*api.Pod)
newPod.Status = oldPod.Status

podutil.DropDisabledAlphaFields(&newPod.Spec)
podutil.DropDisabledAlphaFields(&oldPod.Spec)
}

// Validate validates a new pod.
Expand Down
1 change: 1 addition & 0 deletions pkg/registry/core/podtemplate/BUILD
Expand Up @@ -13,6 +13,7 @@ go_library(
],
deps = [
"//pkg/api:go_default_library",
"//pkg/api/pod:go_default_library",
"//pkg/api/validation:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/runtime:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/util/validation/field:go_default_library",
Expand Down
11 changes: 9 additions & 2 deletions pkg/registry/core/podtemplate/strategy.go
Expand Up @@ -22,6 +22,7 @@ import (
genericapirequest "k8s.io/apiserver/pkg/endpoints/request"
"k8s.io/apiserver/pkg/storage/names"
"k8s.io/kubernetes/pkg/api"
"k8s.io/kubernetes/pkg/api/pod"
"k8s.io/kubernetes/pkg/api/validation"
)

Expand All @@ -42,7 +43,9 @@ func (podTemplateStrategy) NamespaceScoped() bool {

// PrepareForCreate clears fields that are not allowed to be set by end users on creation.
func (podTemplateStrategy) PrepareForCreate(ctx genericapirequest.Context, obj runtime.Object) {
_ = obj.(*api.PodTemplate)
template := obj.(*api.PodTemplate)

pod.DropDisabledAlphaFields(&template.Template.Spec)
}

// Validate validates a new pod template.
Expand All @@ -62,7 +65,11 @@ func (podTemplateStrategy) AllowCreateOnUpdate() bool {

// PrepareForUpdate clears fields that are not allowed to be set by end users on update.
func (podTemplateStrategy) PrepareForUpdate(ctx genericapirequest.Context, obj, old runtime.Object) {
_ = obj.(*api.PodTemplate)
newTemplate := obj.(*api.PodTemplate)
oldTemplate := old.(*api.PodTemplate)

pod.DropDisabledAlphaFields(&newTemplate.Template.Spec)
pod.DropDisabledAlphaFields(&oldTemplate.Template.Spec)
}

// ValidateUpdate is the default update validation for an end user.
Expand Down
1 change: 1 addition & 0 deletions pkg/registry/core/replicationcontroller/BUILD
Expand Up @@ -16,6 +16,7 @@ go_library(
deps = [
"//pkg/api:go_default_library",
"//pkg/api/helper:go_default_library",
"//pkg/api/pod:go_default_library",
"//pkg/api/validation:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/api/equality:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/apis/meta/internalversion:go_default_library",
Expand Down
12 changes: 12 additions & 0 deletions pkg/registry/core/replicationcontroller/strategy.go
Expand Up @@ -35,6 +35,7 @@ import (
"k8s.io/apiserver/pkg/storage/names"
"k8s.io/kubernetes/pkg/api"
"k8s.io/kubernetes/pkg/api/helper"
"k8s.io/kubernetes/pkg/api/pod"
"k8s.io/kubernetes/pkg/api/validation"
)

Expand Down Expand Up @@ -64,6 +65,10 @@ func (rcStrategy) PrepareForCreate(ctx genericapirequest.Context, obj runtime.Ob
controller.Status = api.ReplicationControllerStatus{}

controller.Generation = 1

if controller.Spec.Template != nil {
pod.DropDisabledAlphaFields(&controller.Spec.Template.Spec)
}
}

// PrepareForUpdate clears fields that are not allowed to be set by end users on update.
Expand All @@ -73,6 +78,13 @@ func (rcStrategy) PrepareForUpdate(ctx genericapirequest.Context, obj, old runti
// update is not allowed to set status
newController.Status = oldController.Status

if oldController.Spec.Template != nil {
pod.DropDisabledAlphaFields(&oldController.Spec.Template.Spec)
}
if newController.Spec.Template != nil {
pod.DropDisabledAlphaFields(&newController.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
// the burden of managing the status onto the clients because we can't (in general)
Expand Down
1 change: 1 addition & 0 deletions pkg/registry/extensions/daemonset/BUILD
Expand Up @@ -14,6 +14,7 @@ go_library(
],
deps = [
"//pkg/api:go_default_library",
"//pkg/api/pod:go_default_library",
"//pkg/apis/extensions:go_default_library",
"//pkg/apis/extensions/validation:go_default_library",
"//vendor/k8s.io/api/apps/v1beta2:go_default_library",
Expand Down
6 changes: 6 additions & 0 deletions pkg/registry/extensions/daemonset/strategy.go
Expand Up @@ -30,6 +30,7 @@ import (
"k8s.io/apiserver/pkg/registry/rest"
"k8s.io/apiserver/pkg/storage/names"
"k8s.io/kubernetes/pkg/api"
"k8s.io/kubernetes/pkg/api/pod"
"k8s.io/kubernetes/pkg/apis/extensions"
"k8s.io/kubernetes/pkg/apis/extensions/validation"
)
Expand Down Expand Up @@ -63,13 +64,18 @@ func (daemonSetStrategy) PrepareForCreate(ctx genericapirequest.Context, obj run
if daemonSet.Spec.TemplateGeneration < 1 {
daemonSet.Spec.TemplateGeneration = 1
}

pod.DropDisabledAlphaFields(&daemonSet.Spec.Template.Spec)
}

// PrepareForUpdate clears fields that are not allowed to be set by end users on update.
func (daemonSetStrategy) PrepareForUpdate(ctx genericapirequest.Context, obj, old runtime.Object) {
newDaemonSet := obj.(*extensions.DaemonSet)
oldDaemonSet := old.(*extensions.DaemonSet)

pod.DropDisabledAlphaFields(&newDaemonSet.Spec.Template.Spec)
pod.DropDisabledAlphaFields(&oldDaemonSet.Spec.Template.Spec)

// update is not allowed to set status
newDaemonSet.Status = oldDaemonSet.Status

Expand Down
1 change: 1 addition & 0 deletions pkg/registry/extensions/deployment/BUILD
Expand Up @@ -15,6 +15,7 @@ go_library(
],
deps = [
"//pkg/api:go_default_library",
"//pkg/api/pod:go_default_library",
"//pkg/apis/extensions:go_default_library",
"//pkg/apis/extensions/validation:go_default_library",
"//vendor/k8s.io/api/apps/v1beta1:go_default_library",
Expand Down
6 changes: 6 additions & 0 deletions pkg/registry/extensions/deployment/strategy.go
Expand Up @@ -31,6 +31,7 @@ import (
"k8s.io/apiserver/pkg/registry/rest"
"k8s.io/apiserver/pkg/storage/names"
"k8s.io/kubernetes/pkg/api"
"k8s.io/kubernetes/pkg/api/pod"
"k8s.io/kubernetes/pkg/apis/extensions"
"k8s.io/kubernetes/pkg/apis/extensions/validation"
)
Expand Down Expand Up @@ -61,6 +62,8 @@ func (deploymentStrategy) PrepareForCreate(ctx genericapirequest.Context, obj ru
deployment := obj.(*extensions.Deployment)
deployment.Status = extensions.DeploymentStatus{}
deployment.Generation = 1

pod.DropDisabledAlphaFields(&deployment.Spec.Template.Spec)
}

// Validate validates a new deployment.
Expand All @@ -84,6 +87,9 @@ func (deploymentStrategy) PrepareForUpdate(ctx genericapirequest.Context, obj, o
oldDeployment := old.(*extensions.Deployment)
newDeployment.Status = oldDeployment.Status

pod.DropDisabledAlphaFields(&newDeployment.Spec.Template.Spec)
pod.DropDisabledAlphaFields(&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
// because annotations are copied from deployments to their replica sets.
Expand Down
1 change: 1 addition & 0 deletions pkg/registry/extensions/replicaset/BUILD
Expand Up @@ -15,6 +15,7 @@ go_library(
],
deps = [
"//pkg/api:go_default_library",
"//pkg/api/pod:go_default_library",
"//pkg/apis/extensions:go_default_library",
"//pkg/apis/extensions/validation:go_default_library",
"//vendor/k8s.io/api/apps/v1beta2:go_default_library",
Expand Down

0 comments on commit f9a82dd

Please sign in to comment.