From 151509fb74da7dd57dfe8c6a63ea3057eac2118c Mon Sep 17 00:00:00 2001 From: Tim Hockin Date: Wed, 7 Jun 2023 16:04:34 -0700 Subject: [PATCH] Remove unreachable warning on volume name dup Volume names are validated to be unique and always have been. The cited issues are all about apply getting messed up, not the aspiserver allowing dups. ``` $ k create -f /tmp/bad.yaml The Deployment "bad-volumes-test" is invalid: spec.template.spec.volumes[1].name: Duplicate value: "config" $ k apply --server-side -f /tmp/bad.yaml Error from server: failed to create typed patch object (default/bad-volumes-test; apps/v1, Kind=Deployment): .spec.template.spec.volumes: duplicate entries for key [name="config"] $ k apply -f /tmp/bad.yaml -o json | jq '.spec.template.spec.volumes' The Deployment "bad-volumes-test" is invalid: spec.template.spec.volumes[1].name: Duplicate value: "config" ``` --- pkg/api/pod/warnings.go | 12 ------------ pkg/api/pod/warnings_test.go | 14 -------------- pkg/registry/batch/cronjob/strategy_test.go | 8 ++++---- pkg/registry/batch/job/strategy_test.go | 2 +- 4 files changed, 5 insertions(+), 31 deletions(-) diff --git a/pkg/api/pod/warnings.go b/pkg/api/pod/warnings.go index 31a66b116be..b03e744e8ce 100644 --- a/pkg/api/pod/warnings.go +++ b/pkg/api/pod/warnings.go @@ -192,18 +192,6 @@ func warningsForPodSpecAndMeta(fieldPath *field.Path, podSpec *api.PodSpec, meta } } - // duplicate volume names (#78266, #58477) - if len(podSpec.Volumes) > 1 { - items := sets.NewString() - for i, item := range podSpec.Volumes { - if items.Has(item.Name) { - warnings = append(warnings, fmt.Sprintf("%s: duplicate name %q", fieldPath.Child("spec", "volumes").Index(i).Child("name"), item.Name)) - } else { - items.Insert(item.Name) - } - } - } - // fractional memory/ephemeral-storage requests/limits (#79950, #49442, #18538) if value, ok := podSpec.Overhead[api.ResourceMemory]; ok && value.MilliValue()%int64(1000) != int64(0) { warnings = append(warnings, fmt.Sprintf("%s: fractional byte value %q is invalid, must be an integer", fieldPath.Child("spec", "overhead").Key(string(api.ResourceMemory)), value.String())) diff --git a/pkg/api/pod/warnings_test.go b/pkg/api/pod/warnings_test.go index e849fb37bef..5eb1e9f8529 100644 --- a/pkg/api/pod/warnings_test.go +++ b/pkg/api/pod/warnings_test.go @@ -254,20 +254,6 @@ func TestWarnings(t *testing.T) { `spec.imagePullSecrets[0].name: invalid empty name ""`, }, }, - { - name: "duplicate volume", - template: &api.PodTemplateSpec{Spec: api.PodSpec{ - Volumes: []api.Volume{ - {Name: "a"}, - {Name: "a"}, - {Name: "a"}, - }}, - }, - expected: []string{ - `spec.volumes[1].name: duplicate name "a"`, - `spec.volumes[2].name: duplicate name "a"`, - }, - }, { name: "duplicate env", template: &api.PodTemplateSpec{Spec: api.PodSpec{ diff --git a/pkg/registry/batch/cronjob/strategy_test.go b/pkg/registry/batch/cronjob/strategy_test.go index c533f4ef522..1369480f0de 100644 --- a/pkg/registry/batch/cronjob/strategy_test.go +++ b/pkg/registry/batch/cronjob/strategy_test.go @@ -222,14 +222,14 @@ func TestStrategy_ResetFields(t *testing.T) { } } -func TestJobStatusStrategy_ResetFields(t *testing.T) { +func TestCronJobStatusStrategy_ResetFields(t *testing.T) { resetFields := StatusStrategy.GetResetFields() if len(resetFields) != 2 { t.Errorf("ResetFields should have 2 elements, but have %d", len(resetFields)) } } -func TestJobStrategy_WarningsOnCreate(t *testing.T) { +func TestCronJobStrategy_WarningsOnCreate(t *testing.T) { ctx := genericapirequest.NewDefaultContext() now := metav1.Now() @@ -291,7 +291,7 @@ func TestJobStrategy_WarningsOnCreate(t *testing.T) { } } -func TestJobStrategy_WarningsOnUpdate(t *testing.T) { +func TestCronJobStrategy_WarningsOnUpdate(t *testing.T) { ctx := genericapirequest.NewDefaultContext() now := metav1.Now() @@ -380,7 +380,7 @@ func TestJobStrategy_WarningsOnUpdate(t *testing.T) { JobTemplate: batch.JobTemplateSpec{ Spec: batch.JobSpec{ Template: api.PodTemplateSpec{ - Spec: api.PodSpec{Volumes: []api.Volume{{Name: "volume-name"}, {Name: "volume-name"}}}, + Spec: api.PodSpec{ImagePullSecrets: []api.LocalObjectReference{{Name: ""}}}, }, }, }, diff --git a/pkg/registry/batch/job/strategy_test.go b/pkg/registry/batch/job/strategy_test.go index bb794122086..5dbcfe89314 100644 --- a/pkg/registry/batch/job/strategy_test.go +++ b/pkg/registry/batch/job/strategy_test.go @@ -813,7 +813,7 @@ func TestJobStrategy_WarningsOnUpdate(t *testing.T) { Spec: batch.JobSpec{ Selector: validSelector, Template: api.PodTemplateSpec{ - Spec: api.PodSpec{Volumes: []api.Volume{{Name: "volume-name"}, {Name: "volume-name"}}}, + Spec: api.PodSpec{ImagePullSecrets: []api.LocalObjectReference{{Name: ""}}}, }, ManualSelector: pointer.BoolPtr(true), Parallelism: pointer.Int32Ptr(1),