From 12dd768bee90d21e8d8d3228e246c30b2a3d5a35 Mon Sep 17 00:00:00 2001 From: Jordan Liggitt Date: Mon, 7 Jan 2019 14:17:29 -0500 Subject: [PATCH] Pass pod annotations into DropDisabledFields() --- pkg/api/pod/util.go | 53 +++++++++++++++++-- pkg/api/pod/util_test.go | 14 ++--- pkg/registry/apps/daemonset/strategy.go | 4 +- pkg/registry/apps/deployment/strategy.go | 4 +- pkg/registry/apps/replicaset/strategy.go | 4 +- pkg/registry/apps/statefulset/strategy.go | 4 +- pkg/registry/batch/cronjob/strategy.go | 4 +- pkg/registry/batch/job/strategy.go | 4 +- pkg/registry/core/pod/strategy.go | 4 +- pkg/registry/core/podtemplate/strategy.go | 4 +- .../core/replicationcontroller/strategy.go | 15 +----- 11 files changed, 75 insertions(+), 39 deletions(-) diff --git a/pkg/api/pod/util.go b/pkg/api/pod/util.go index 1c2c6a9fe0b..8723e59c41e 100644 --- a/pkg/api/pod/util.go +++ b/pkg/api/pod/util.go @@ -232,9 +232,56 @@ func UpdatePodCondition(status *api.PodStatus, condition *api.PodCondition) bool return !isEqual } -// DropDisabledFields removes disabled fields from the pod spec. -// This should be called from PrepareForCreate/PrepareForUpdate for all resources containing a pod spec. -func DropDisabledFields(podSpec, oldPodSpec *api.PodSpec) { +// DropDisabledTemplateFields removes disabled fields from the pod template metadata and spec. +// This should be called from PrepareForCreate/PrepareForUpdate for all resources containing a PodTemplateSpec +func DropDisabledTemplateFields(podTemplate, oldPodTemplate *api.PodTemplateSpec) { + var ( + podSpec *api.PodSpec + podAnnotations map[string]string + oldPodSpec *api.PodSpec + oldPodAnnotations map[string]string + ) + if podTemplate != nil { + podSpec = &podTemplate.Spec + podAnnotations = podTemplate.Annotations + } + if oldPodTemplate != nil { + oldPodSpec = &oldPodTemplate.Spec + oldPodAnnotations = oldPodTemplate.Annotations + } + dropDisabledFields(podSpec, podAnnotations, oldPodSpec, oldPodAnnotations) +} + +// DropDisabledPodFields removes disabled fields from the pod metadata and spec. +// This should be called from PrepareForCreate/PrepareForUpdate for all resources containing a Pod +func DropDisabledPodFields(pod, oldPod *api.Pod) { + var ( + podSpec *api.PodSpec + podAnnotations map[string]string + oldPodSpec *api.PodSpec + oldPodAnnotations map[string]string + ) + if pod != nil { + podSpec = &pod.Spec + podAnnotations = pod.Annotations + } + if oldPod != nil { + oldPodSpec = &oldPod.Spec + oldPodAnnotations = oldPod.Annotations + } + dropDisabledFields(podSpec, podAnnotations, oldPodSpec, oldPodAnnotations) +} + +// dropDisabledFields removes disabled fields from the pod metadata and spec. +func dropDisabledFields( + podSpec *api.PodSpec, podAnnotations map[string]string, + oldPodSpec *api.PodSpec, oldPodAnnotations map[string]string, +) { + // the new spec must always be non-nil + if podSpec == nil { + podSpec = &api.PodSpec{} + } + if !utilfeature.DefaultFeatureGate.Enabled(features.PodPriority) && !podPriorityInUse(oldPodSpec) { // Set to nil pod's priority fields if the feature is disabled and the old pod // does not specify any values for these fields. diff --git a/pkg/api/pod/util_test.go b/pkg/api/pod/util_test.go index eca0309b86c..dc1027dc9f2 100644 --- a/pkg/api/pod/util_test.go +++ b/pkg/api/pod/util_test.go @@ -375,7 +375,7 @@ func TestDropAlphaVolumeDevices(t *testing.T) { if oldPod != nil { oldPodSpec = &oldPod.Spec } - DropDisabledFields(&newPod.Spec, oldPodSpec) + dropDisabledFields(&newPod.Spec, nil, oldPodSpec, nil) // old pod should never be changed if !reflect.DeepEqual(oldPod, oldPodInfo.pod()) { @@ -469,7 +469,7 @@ func TestDropSubPath(t *testing.T) { if oldPod != nil { oldPodSpec = &oldPod.Spec } - DropDisabledFields(&newPod.Spec, oldPodSpec) + dropDisabledFields(&newPod.Spec, nil, oldPodSpec, nil) // old pod should never be changed if !reflect.DeepEqual(oldPod, oldPodInfo.pod()) { @@ -558,7 +558,7 @@ func TestDropRuntimeClass(t *testing.T) { if oldPod != nil { oldPodSpec = &oldPod.Spec } - DropDisabledFields(&newPod.Spec, oldPodSpec) + dropDisabledFields(&newPod.Spec, nil, oldPodSpec, nil) // old pod should never be changed if !reflect.DeepEqual(oldPod, oldPodInfo.pod()) { @@ -652,7 +652,7 @@ func TestDropProcMount(t *testing.T) { if oldPod != nil { oldPodSpec = &oldPod.Spec } - DropDisabledFields(&newPod.Spec, oldPodSpec) + dropDisabledFields(&newPod.Spec, nil, oldPodSpec, nil) // old pod should never be changed if !reflect.DeepEqual(oldPod, oldPodInfo.pod()) { @@ -768,7 +768,7 @@ func TestDropPodPriority(t *testing.T) { if oldPod != nil { oldPodSpec = &oldPod.Spec } - DropDisabledFields(&newPod.Spec, oldPodSpec) + dropDisabledFields(&newPod.Spec, nil, oldPodSpec, nil) // old pod should never be changed if !reflect.DeepEqual(oldPod, oldPodInfo.pod()) { @@ -878,7 +878,7 @@ func TestDropEmptyDirSizeLimit(t *testing.T) { if oldPod != nil { oldPodSpec = &oldPod.Spec } - DropDisabledFields(&newPod.Spec, oldPodSpec) + dropDisabledFields(&newPod.Spec, nil, oldPodSpec, nil) // old pod should never be changed if !reflect.DeepEqual(oldPod, oldPodInfo.pod()) { @@ -1013,7 +1013,7 @@ func TestDropRunAsGroup(t *testing.T) { if oldPod != nil { oldPodSpec = &oldPod.Spec } - DropDisabledFields(&newPod.Spec, oldPodSpec) + dropDisabledFields(&newPod.Spec, nil, oldPodSpec, nil) // old pod should never be changed if !reflect.DeepEqual(oldPod, oldPodInfo.pod()) { diff --git a/pkg/registry/apps/daemonset/strategy.go b/pkg/registry/apps/daemonset/strategy.go index 08ffd63f730..4edba00eec9 100644 --- a/pkg/registry/apps/daemonset/strategy.go +++ b/pkg/registry/apps/daemonset/strategy.go @@ -75,7 +75,7 @@ func (daemonSetStrategy) PrepareForCreate(ctx context.Context, obj runtime.Objec daemonSet.Spec.TemplateGeneration = 1 } - pod.DropDisabledFields(&daemonSet.Spec.Template.Spec, nil) + pod.DropDisabledTemplateFields(&daemonSet.Spec.Template, nil) } // PrepareForUpdate clears fields that are not allowed to be set by end users on update. @@ -83,7 +83,7 @@ func (daemonSetStrategy) PrepareForUpdate(ctx context.Context, obj, old runtime. newDaemonSet := obj.(*apps.DaemonSet) oldDaemonSet := old.(*apps.DaemonSet) - pod.DropDisabledFields(&newDaemonSet.Spec.Template.Spec, &oldDaemonSet.Spec.Template.Spec) + pod.DropDisabledTemplateFields(&newDaemonSet.Spec.Template, &oldDaemonSet.Spec.Template) // update is not allowed to set status newDaemonSet.Status = oldDaemonSet.Status diff --git a/pkg/registry/apps/deployment/strategy.go b/pkg/registry/apps/deployment/strategy.go index cccd62116f3..f5e9cfe7115 100644 --- a/pkg/registry/apps/deployment/strategy.go +++ b/pkg/registry/apps/deployment/strategy.go @@ -73,7 +73,7 @@ func (deploymentStrategy) PrepareForCreate(ctx context.Context, obj runtime.Obje deployment.Status = apps.DeploymentStatus{} deployment.Generation = 1 - pod.DropDisabledFields(&deployment.Spec.Template.Spec, nil) + pod.DropDisabledTemplateFields(&deployment.Spec.Template, nil) } // Validate validates a new deployment. @@ -97,7 +97,7 @@ func (deploymentStrategy) PrepareForUpdate(ctx context.Context, obj, old runtime oldDeployment := old.(*apps.Deployment) newDeployment.Status = oldDeployment.Status - pod.DropDisabledFields(&newDeployment.Spec.Template.Spec, &oldDeployment.Spec.Template.Spec) + pod.DropDisabledTemplateFields(&newDeployment.Spec.Template, &oldDeployment.Spec.Template) // Spec updates bump the generation so that we can distinguish between // scaling events and template changes, annotation updates bump the generation diff --git a/pkg/registry/apps/replicaset/strategy.go b/pkg/registry/apps/replicaset/strategy.go index 37bf07eaa3d..72d70074988 100644 --- a/pkg/registry/apps/replicaset/strategy.go +++ b/pkg/registry/apps/replicaset/strategy.go @@ -80,7 +80,7 @@ func (rsStrategy) PrepareForCreate(ctx context.Context, obj runtime.Object) { rs.Generation = 1 - pod.DropDisabledFields(&rs.Spec.Template.Spec, nil) + pod.DropDisabledTemplateFields(&rs.Spec.Template, nil) } // PrepareForUpdate clears fields that are not allowed to be set by end users on update. @@ -90,7 +90,7 @@ func (rsStrategy) PrepareForUpdate(ctx context.Context, obj, old runtime.Object) // update is not allowed to set status newRS.Status = oldRS.Status - pod.DropDisabledFields(&newRS.Spec.Template.Spec, &oldRS.Spec.Template.Spec) + pod.DropDisabledTemplateFields(&newRS.Spec.Template, &oldRS.Spec.Template) // 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 diff --git a/pkg/registry/apps/statefulset/strategy.go b/pkg/registry/apps/statefulset/strategy.go index 1a8608900bc..c3dc71d1188 100644 --- a/pkg/registry/apps/statefulset/strategy.go +++ b/pkg/registry/apps/statefulset/strategy.go @@ -72,7 +72,7 @@ func (statefulSetStrategy) PrepareForCreate(ctx context.Context, obj runtime.Obj statefulSet.Generation = 1 - pod.DropDisabledFields(&statefulSet.Spec.Template.Spec, nil) + pod.DropDisabledTemplateFields(&statefulSet.Spec.Template, nil) } // PrepareForUpdate clears fields that are not allowed to be set by end users on update. @@ -82,7 +82,7 @@ func (statefulSetStrategy) PrepareForUpdate(ctx context.Context, obj, old runtim // Update is not allowed to set status newStatefulSet.Status = oldStatefulSet.Status - pod.DropDisabledFields(&newStatefulSet.Spec.Template.Spec, &oldStatefulSet.Spec.Template.Spec) + pod.DropDisabledTemplateFields(&newStatefulSet.Spec.Template, &oldStatefulSet.Spec.Template) // Any changes to the spec increment the generation number, any changes to the // status should reflect the generation number of the corresponding object. diff --git a/pkg/registry/batch/cronjob/strategy.go b/pkg/registry/batch/cronjob/strategy.go index 3ccbee4eb42..d23a5cdbdf2 100644 --- a/pkg/registry/batch/cronjob/strategy.go +++ b/pkg/registry/batch/cronjob/strategy.go @@ -68,7 +68,7 @@ func (cronJobStrategy) PrepareForCreate(ctx context.Context, obj runtime.Object) cronJob := obj.(*batch.CronJob) cronJob.Status = batch.CronJobStatus{} - pod.DropDisabledFields(&cronJob.Spec.JobTemplate.Spec.Template.Spec, nil) + pod.DropDisabledTemplateFields(&cronJob.Spec.JobTemplate.Spec.Template, nil) } // PrepareForUpdate clears fields that are not allowed to be set by end users on update. @@ -77,7 +77,7 @@ func (cronJobStrategy) PrepareForUpdate(ctx context.Context, obj, old runtime.Ob oldCronJob := old.(*batch.CronJob) newCronJob.Status = oldCronJob.Status - pod.DropDisabledFields(&newCronJob.Spec.JobTemplate.Spec.Template.Spec, &oldCronJob.Spec.JobTemplate.Spec.Template.Spec) + pod.DropDisabledTemplateFields(&newCronJob.Spec.JobTemplate.Spec.Template, &oldCronJob.Spec.JobTemplate.Spec.Template) } // Validate validates a new scheduled job. diff --git a/pkg/registry/batch/job/strategy.go b/pkg/registry/batch/job/strategy.go index 5ce5e51178b..b0a5e8a3472 100644 --- a/pkg/registry/batch/job/strategy.go +++ b/pkg/registry/batch/job/strategy.go @@ -80,7 +80,7 @@ func (jobStrategy) PrepareForCreate(ctx context.Context, obj runtime.Object) { job.Spec.TTLSecondsAfterFinished = nil } - pod.DropDisabledFields(&job.Spec.Template.Spec, nil) + pod.DropDisabledTemplateFields(&job.Spec.Template, nil) } // PrepareForUpdate clears fields that are not allowed to be set by end users on update. @@ -93,7 +93,7 @@ func (jobStrategy) PrepareForUpdate(ctx context.Context, obj, old runtime.Object newJob.Spec.TTLSecondsAfterFinished = nil } - pod.DropDisabledFields(&newJob.Spec.Template.Spec, &oldJob.Spec.Template.Spec) + pod.DropDisabledTemplateFields(&newJob.Spec.Template, &oldJob.Spec.Template) } // Validate validates a new job. diff --git a/pkg/registry/core/pod/strategy.go b/pkg/registry/core/pod/strategy.go index 52fb9ec45aa..307870e1fbc 100644 --- a/pkg/registry/core/pod/strategy.go +++ b/pkg/registry/core/pod/strategy.go @@ -73,7 +73,7 @@ func (podStrategy) PrepareForCreate(ctx context.Context, obj runtime.Object) { QOSClass: qos.GetPodQOS(pod), } - podutil.DropDisabledFields(&pod.Spec, nil) + podutil.DropDisabledPodFields(pod, nil) } // PrepareForUpdate clears fields that are not allowed to be set by end users on update. @@ -82,7 +82,7 @@ func (podStrategy) PrepareForUpdate(ctx context.Context, obj, old runtime.Object oldPod := old.(*api.Pod) newPod.Status = oldPod.Status - podutil.DropDisabledFields(&newPod.Spec, &oldPod.Spec) + podutil.DropDisabledPodFields(newPod, oldPod) } // Validate validates a new pod. diff --git a/pkg/registry/core/podtemplate/strategy.go b/pkg/registry/core/podtemplate/strategy.go index 13d841f7829..8ce614cbf3d 100644 --- a/pkg/registry/core/podtemplate/strategy.go +++ b/pkg/registry/core/podtemplate/strategy.go @@ -47,7 +47,7 @@ func (podTemplateStrategy) NamespaceScoped() bool { func (podTemplateStrategy) PrepareForCreate(ctx context.Context, obj runtime.Object) { template := obj.(*api.PodTemplate) - pod.DropDisabledFields(&template.Template.Spec, nil) + pod.DropDisabledTemplateFields(&template.Template, nil) } // Validate validates a new pod template. @@ -70,7 +70,7 @@ func (podTemplateStrategy) PrepareForUpdate(ctx context.Context, obj, old runtim newTemplate := obj.(*api.PodTemplate) oldTemplate := old.(*api.PodTemplate) - pod.DropDisabledFields(&newTemplate.Template.Spec, &oldTemplate.Template.Spec) + pod.DropDisabledTemplateFields(&newTemplate.Template, &oldTemplate.Template) } // ValidateUpdate is the default update validation for an end user. diff --git a/pkg/registry/core/replicationcontroller/strategy.go b/pkg/registry/core/replicationcontroller/strategy.go index 2c0cc34a402..7c05fee212e 100644 --- a/pkg/registry/core/replicationcontroller/strategy.go +++ b/pkg/registry/core/replicationcontroller/strategy.go @@ -80,9 +80,7 @@ func (rcStrategy) PrepareForCreate(ctx context.Context, obj runtime.Object) { controller.Generation = 1 - if controller.Spec.Template != nil { - pod.DropDisabledFields(&controller.Spec.Template.Spec, nil) - } + pod.DropDisabledTemplateFields(controller.Spec.Template, nil) } // PrepareForUpdate clears fields that are not allowed to be set by end users on update. @@ -92,16 +90,7 @@ 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 { - oldSpec = &oldController.Spec.Template.Spec - } - if newController.Spec.Template != nil { - newSpec = &newController.Spec.Template.Spec - } else { - newSpec = &api.PodSpec{} - } - pod.DropDisabledFields(newSpec, oldSpec) + pod.DropDisabledTemplateFields(newController.Spec.Template, oldController.Spec.Template) // 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