Skip to content

Commit 2f2c1fb

Browse files
k8s-ci-robotk8s-ci-robot
authored
Merge pull request kubernetes#124210 from thockin/remove_gate_SkipReadOnlyValidationGCE
Remove the gate "SkipReadOnlyValidationGCE"
2 parents 64f76b6 + ae01c21 commit 2f2c1fb

File tree

5 files changed

+17
-260
lines changed

5 files changed

+17
-260
lines changed

‎pkg/apis/apps/validation/validation.go

Lines changed: 6 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -280,15 +280,15 @@ func ValidateControllerRevisionUpdate(newHistory, oldHistory *apps.ControllerRev
280280
// ValidateDaemonSet tests if required fields in the DaemonSet are set.
281281
func ValidateDaemonSet(ds *apps.DaemonSet, opts apivalidation.PodValidationOptions) field.ErrorList {
282282
allErrs := apivalidation.ValidateObjectMeta(&ds.ObjectMeta, true, ValidateDaemonSetName, field.NewPath("metadata"))
283-
allErrs = append(allErrs, ValidateDaemonSetSpec(&ds.Spec, nil, field.NewPath("spec"), opts)...)
283+
allErrs = append(allErrs, ValidateDaemonSetSpec(&ds.Spec, field.NewPath("spec"), opts)...)
284284
return allErrs
285285
}
286286

287287
// ValidateDaemonSetUpdate tests if required fields in the DaemonSet are set.
288288
func ValidateDaemonSetUpdate(ds, oldDS *apps.DaemonSet, opts apivalidation.PodValidationOptions) field.ErrorList {
289289
allErrs := apivalidation.ValidateObjectMetaUpdate(&ds.ObjectMeta, &oldDS.ObjectMeta, field.NewPath("metadata"))
290290
allErrs = append(allErrs, ValidateDaemonSetSpecUpdate(&ds.Spec, &oldDS.Spec, field.NewPath("spec"))...)
291-
allErrs = append(allErrs, ValidateDaemonSetSpec(&ds.Spec, &oldDS.Spec, field.NewPath("spec"), opts)...)
291+
allErrs = append(allErrs, ValidateDaemonSetSpec(&ds.Spec, field.NewPath("spec"), opts)...)
292292
return allErrs
293293
}
294294

@@ -344,7 +344,7 @@ func ValidateDaemonSetStatusUpdate(ds, oldDS *apps.DaemonSet) field.ErrorList {
344344
}
345345

346346
// ValidateDaemonSetSpec tests if required fields in the DaemonSetSpec are set.
347-
func ValidateDaemonSetSpec(spec, oldSpec *apps.DaemonSetSpec, fldPath *field.Path, opts apivalidation.PodValidationOptions) field.ErrorList {
347+
func ValidateDaemonSetSpec(spec *apps.DaemonSetSpec, fldPath *field.Path, opts apivalidation.PodValidationOptions) field.ErrorList {
348348
allErrs := field.ErrorList{}
349349
labelSelectorValidationOpts := unversionedvalidation.LabelSelectorValidationOptions{AllowInvalidLabelValueInSelector: opts.AllowInvalidLabelValueInSelector}
350350

@@ -359,12 +359,6 @@ func ValidateDaemonSetSpec(spec, oldSpec *apps.DaemonSetSpec, fldPath *field.Pat
359359
}
360360

361361
allErrs = append(allErrs, apivalidation.ValidatePodTemplateSpec(&spec.Template, fldPath.Child("template"), opts)...)
362-
// get rid of apivalidation.ValidateReadOnlyPersistentDisks,stop passing oldSpec to this function
363-
var oldVols []api.Volume
364-
if oldSpec != nil {
365-
oldVols = oldSpec.Template.Spec.Volumes // +k8s:verify-mutation:reason=clone
366-
}
367-
allErrs = append(allErrs, apivalidation.ValidateReadOnlyPersistentDisks(spec.Template.Spec.Volumes, oldVols, fldPath.Child("template", "spec", "volumes"))...)
368362
// RestartPolicy has already been first-order validated as per ValidatePodTemplateSpec().
369363
if spec.Template.Spec.RestartPolicy != api.RestartPolicyAlways {
370364
allErrs = append(allErrs, field.NotSupported(fldPath.Child("template", "spec", "restartPolicy"), spec.Template.Spec.RestartPolicy, []string{string(api.RestartPolicyAlways)}))
@@ -566,12 +560,7 @@ func ValidateDeploymentSpec(spec, oldSpec *apps.DeploymentSpec, fldPath *field.P
566560
if err != nil {
567561
allErrs = append(allErrs, field.Invalid(fldPath.Child("selector"), spec.Selector, "invalid label selector"))
568562
} else {
569-
// oldSpec is not empty, pass oldSpec.template
570-
var oldTemplate *api.PodTemplateSpec
571-
if oldSpec != nil {
572-
oldTemplate = &oldSpec.Template // +k8s:verify-mutation:reason=clone
573-
}
574-
allErrs = append(allErrs, ValidatePodTemplateSpecForReplicaSet(&spec.Template, oldTemplate, selector, spec.Replicas, fldPath.Child("template"), opts)...)
563+
allErrs = append(allErrs, ValidatePodTemplateSpecForReplicaSet(&spec.Template, selector, spec.Replicas, fldPath.Child("template"), opts)...)
575564
}
576565

577566
allErrs = append(allErrs, ValidateDeploymentStrategy(&spec.Strategy, fldPath.Child("strategy"))...)
@@ -734,18 +723,13 @@ func ValidateReplicaSetSpec(spec, oldSpec *apps.ReplicaSetSpec, fldPath *field.P
734723
if err != nil {
735724
allErrs = append(allErrs, field.Invalid(fldPath.Child("selector"), spec.Selector, "invalid label selector"))
736725
} else {
737-
// oldSpec is not empty, pass oldSpec.template.
738-
var oldTemplate *api.PodTemplateSpec
739-
if oldSpec != nil {
740-
oldTemplate = &oldSpec.Template // +k8s:verify-mutation:reason=clone
741-
}
742-
allErrs = append(allErrs, ValidatePodTemplateSpecForReplicaSet(&spec.Template, oldTemplate, selector, spec.Replicas, fldPath.Child("template"), opts)...)
726+
allErrs = append(allErrs, ValidatePodTemplateSpecForReplicaSet(&spec.Template, selector, spec.Replicas, fldPath.Child("template"), opts)...)
743727
}
744728
return allErrs
745729
}
746730

747731
// ValidatePodTemplateSpecForReplicaSet validates the given template and ensures that it is in accordance with the desired selector and replicas.
748-
func ValidatePodTemplateSpecForReplicaSet(template, oldTemplate *api.PodTemplateSpec, selector labels.Selector, replicas int32, fldPath *field.Path, opts apivalidation.PodValidationOptions) field.ErrorList {
732+
func ValidatePodTemplateSpecForReplicaSet(template *api.PodTemplateSpec, selector labels.Selector, replicas int32, fldPath *field.Path, opts apivalidation.PodValidationOptions) field.ErrorList {
749733
allErrs := field.ErrorList{}
750734
if template == nil {
751735
allErrs = append(allErrs, field.Required(fldPath, ""))
@@ -758,15 +742,6 @@ func ValidatePodTemplateSpecForReplicaSet(template, oldTemplate *api.PodTemplate
758742
}
759743
}
760744
allErrs = append(allErrs, apivalidation.ValidatePodTemplateSpec(template, fldPath, opts)...)
761-
// Daemons run on more than one node, Cancel verification of read and write volumes.
762-
// get rid of apivalidation.ValidateReadOnlyPersistentDisks,stop passing oldTemplate to this function
763-
var oldVols []api.Volume
764-
if oldTemplate != nil {
765-
oldVols = oldTemplate.Spec.Volumes // +k8s:verify-mutation:reason=clone
766-
}
767-
if replicas > 1 {
768-
allErrs = append(allErrs, apivalidation.ValidateReadOnlyPersistentDisks(template.Spec.Volumes, oldVols, fldPath.Child("spec", "volumes"))...)
769-
}
770745
// RestartPolicy has already been first-order validated as per ValidatePodTemplateSpec().
771746
if template.Spec.RestartPolicy != api.RestartPolicyAlways {
772747
allErrs = append(allErrs, field.NotSupported(fldPath.Child("spec", "restartPolicy"), template.Spec.RestartPolicy, []string{string(api.RestartPolicyAlways)}))

‎pkg/apis/apps/validation/validation_test.go

Lines changed: 9 additions & 85 deletions
Original file line numberDiff line numberDiff line change
@@ -1572,10 +1572,9 @@ func TestValidateDaemonSetUpdate(t *testing.T) {
15721572
},
15731573
}
15741574
type dsUpdateTest struct {
1575-
old apps.DaemonSet
1576-
update apps.DaemonSet
1577-
expectedErrNum int
1578-
enableSkipReadOnlyValidationGCE bool
1575+
old apps.DaemonSet
1576+
update apps.DaemonSet
1577+
expectedErrNum int
15791578
}
15801579
successCases := map[string]dsUpdateTest{
15811580
"no change": {
@@ -1729,7 +1728,6 @@ func TestValidateDaemonSetUpdate(t *testing.T) {
17291728
},
17301729
},
17311730
"Read-write volume verification": {
1732-
enableSkipReadOnlyValidationGCE: true,
17331731
old: apps.DaemonSet{
17341732
ObjectMeta: metav1.ObjectMeta{Name: "abc", Namespace: metav1.NamespaceDefault},
17351733
Spec: apps.DaemonSetSpec{
@@ -1756,7 +1754,6 @@ func TestValidateDaemonSetUpdate(t *testing.T) {
17561754
}
17571755
for testName, successCase := range successCases {
17581756
t.Run(testName, func(t *testing.T) {
1759-
defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.SkipReadOnlyValidationGCE, successCase.enableSkipReadOnlyValidationGCE)()
17601757
// ResourceVersion is required for updates.
17611758
successCase.old.ObjectMeta.ResourceVersion = "1"
17621759
successCase.update.ObjectMeta.ResourceVersion = "2"
@@ -1848,32 +1845,6 @@ func TestValidateDaemonSetUpdate(t *testing.T) {
18481845
},
18491846
expectedErrNum: 1,
18501847
},
1851-
"invalid read-write volume": {
1852-
enableSkipReadOnlyValidationGCE: false,
1853-
old: apps.DaemonSet{
1854-
ObjectMeta: metav1.ObjectMeta{Name: "abc", Namespace: metav1.NamespaceDefault},
1855-
Spec: apps.DaemonSetSpec{
1856-
Selector: &metav1.LabelSelector{MatchLabels: validSelector},
1857-
TemplateGeneration: 1,
1858-
Template: validPodTemplateAbc.Template,
1859-
UpdateStrategy: apps.DaemonSetUpdateStrategy{
1860-
Type: apps.OnDeleteDaemonSetStrategyType,
1861-
},
1862-
},
1863-
},
1864-
update: apps.DaemonSet{
1865-
ObjectMeta: metav1.ObjectMeta{Name: "abc", Namespace: metav1.NamespaceDefault},
1866-
Spec: apps.DaemonSetSpec{
1867-
Selector: &metav1.LabelSelector{MatchLabels: validSelector},
1868-
TemplateGeneration: 2,
1869-
Template: readWriteVolumePodTemplate.Template,
1870-
UpdateStrategy: apps.DaemonSetUpdateStrategy{
1871-
Type: apps.OnDeleteDaemonSetStrategyType,
1872-
},
1873-
},
1874-
},
1875-
expectedErrNum: 1,
1876-
},
18771848
"invalid update strategy": {
18781849
old: apps.DaemonSet{
18791850
ObjectMeta: metav1.ObjectMeta{Name: "abc", Namespace: metav1.NamespaceDefault},
@@ -1977,7 +1948,6 @@ func TestValidateDaemonSetUpdate(t *testing.T) {
19771948
}
19781949
for testName, errorCase := range errorCases {
19791950
t.Run(testName, func(t *testing.T) {
1980-
defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.SkipReadOnlyValidationGCE, errorCase.enableSkipReadOnlyValidationGCE)()
19811951
// ResourceVersion is required for updates.
19821952
errorCase.old.ObjectMeta.ResourceVersion = "1"
19831953
errorCase.update.ObjectMeta.ResourceVersion = "2"
@@ -2645,10 +2615,9 @@ func TestValidateDeploymentUpdate(t *testing.T) {
26452615
},
26462616
}
26472617
type depUpdateTest struct {
2648-
old apps.Deployment
2649-
update apps.Deployment
2650-
expectedErrNum int
2651-
enableSkipReadOnlyValidationGCE bool
2618+
old apps.Deployment
2619+
update apps.Deployment
2620+
expectedErrNum int
26522621
}
26532622
successCases := map[string]depUpdateTest{
26542623
"positive replicas": {
@@ -2671,7 +2640,6 @@ func TestValidateDeploymentUpdate(t *testing.T) {
26712640
},
26722641
},
26732642
"Read-write volume verification": {
2674-
enableSkipReadOnlyValidationGCE: true,
26752643
old: apps.Deployment{
26762644
ObjectMeta: metav1.ObjectMeta{Name: "abc", Namespace: metav1.NamespaceDefault},
26772645
Spec: apps.DeploymentSpec{
@@ -2693,7 +2661,6 @@ func TestValidateDeploymentUpdate(t *testing.T) {
26932661
}
26942662
for testName, successCase := range successCases {
26952663
t.Run(testName, func(t *testing.T) {
2696-
defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.SkipReadOnlyValidationGCE, successCase.enableSkipReadOnlyValidationGCE)()
26972664
// ResourceVersion is required for updates.
26982665
successCase.old.ObjectMeta.ResourceVersion = "1"
26992666
successCase.update.ObjectMeta.ResourceVersion = "2"
@@ -2710,26 +2677,6 @@ func TestValidateDeploymentUpdate(t *testing.T) {
27102677
}
27112678
})
27122679
errorCases := map[string]depUpdateTest{
2713-
"more than one read/write": {
2714-
old: apps.Deployment{
2715-
ObjectMeta: metav1.ObjectMeta{Name: "", Namespace: metav1.NamespaceDefault},
2716-
Spec: apps.DeploymentSpec{
2717-
Selector: &metav1.LabelSelector{MatchLabels: validLabels},
2718-
Template: validPodTemplate.Template,
2719-
Strategy: apps.DeploymentStrategy{Type: apps.RecreateDeploymentStrategyType},
2720-
},
2721-
},
2722-
update: apps.Deployment{
2723-
ObjectMeta: metav1.ObjectMeta{Name: "abc", Namespace: metav1.NamespaceDefault},
2724-
Spec: apps.DeploymentSpec{
2725-
Replicas: 2,
2726-
Selector: &metav1.LabelSelector{MatchLabels: validLabels},
2727-
Template: readWriteVolumePodTemplate.Template,
2728-
Strategy: apps.DeploymentStrategy{Type: apps.RecreateDeploymentStrategyType},
2729-
},
2730-
},
2731-
expectedErrNum: 2,
2732-
},
27332680
"invalid selector": {
27342681
old: apps.Deployment{
27352682
ObjectMeta: metav1.ObjectMeta{Name: "", Namespace: metav1.NamespaceDefault},
@@ -2793,7 +2740,6 @@ func TestValidateDeploymentUpdate(t *testing.T) {
27932740
}
27942741
for testName, errorCase := range errorCases {
27952742
t.Run(testName, func(t *testing.T) {
2796-
defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.SkipReadOnlyValidationGCE, errorCase.enableSkipReadOnlyValidationGCE)()
27972743
// ResourceVersion is required for updates.
27982744
errorCase.old.ObjectMeta.ResourceVersion = "1"
27992745
errorCase.update.ObjectMeta.ResourceVersion = "2"
@@ -3074,10 +3020,9 @@ func TestValidateReplicaSetUpdate(t *testing.T) {
30743020
},
30753021
}
30763022
type rcUpdateTest struct {
3077-
old apps.ReplicaSet
3078-
update apps.ReplicaSet
3079-
expectedErrNum int
3080-
enableSkipReadOnlyValidationGCE bool
3023+
old apps.ReplicaSet
3024+
update apps.ReplicaSet
3025+
expectedErrNum int
30813026
}
30823027
successCases := map[string]rcUpdateTest{
30833028
"positive replicas": {
@@ -3098,7 +3043,6 @@ func TestValidateReplicaSetUpdate(t *testing.T) {
30983043
},
30993044
},
31003045
"Read-write volume verification": {
3101-
enableSkipReadOnlyValidationGCE: true,
31023046
old: apps.ReplicaSet{
31033047
ObjectMeta: metav1.ObjectMeta{Name: "abc", Namespace: metav1.NamespaceDefault},
31043048
Spec: apps.ReplicaSetSpec{
@@ -3118,7 +3062,6 @@ func TestValidateReplicaSetUpdate(t *testing.T) {
31183062
}
31193063
for testName, successCase := range successCases {
31203064
t.Run(testName, func(t *testing.T) {
3121-
defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.SkipReadOnlyValidationGCE, successCase.enableSkipReadOnlyValidationGCE)()
31223065
// ResourceVersion is required for updates.
31233066
successCase.old.ObjectMeta.ResourceVersion = "1"
31243067
successCase.update.ObjectMeta.ResourceVersion = "2"
@@ -3136,24 +3079,6 @@ func TestValidateReplicaSetUpdate(t *testing.T) {
31363079
})
31373080
}
31383081
errorCases := map[string]rcUpdateTest{
3139-
"more than one read/write": {
3140-
old: apps.ReplicaSet{
3141-
ObjectMeta: metav1.ObjectMeta{Name: "", Namespace: metav1.NamespaceDefault},
3142-
Spec: apps.ReplicaSetSpec{
3143-
Selector: &metav1.LabelSelector{MatchLabels: validLabels},
3144-
Template: validPodTemplate.Template,
3145-
},
3146-
},
3147-
update: apps.ReplicaSet{
3148-
ObjectMeta: metav1.ObjectMeta{Name: "abc", Namespace: metav1.NamespaceDefault},
3149-
Spec: apps.ReplicaSetSpec{
3150-
Replicas: 2,
3151-
Selector: &metav1.LabelSelector{MatchLabels: validLabels},
3152-
Template: readWriteVolumePodTemplate.Template,
3153-
},
3154-
},
3155-
expectedErrNum: 2,
3156-
},
31573082
"invalid selector": {
31583083
old: apps.ReplicaSet{
31593084
ObjectMeta: metav1.ObjectMeta{Name: "", Namespace: metav1.NamespaceDefault},
@@ -3211,7 +3136,6 @@ func TestValidateReplicaSetUpdate(t *testing.T) {
32113136
}
32123137
for testName, errorCase := range errorCases {
32133138
t.Run(testName, func(t *testing.T) {
3214-
defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.SkipReadOnlyValidationGCE, errorCase.enableSkipReadOnlyValidationGCE)()
32153139
// ResourceVersion is required for updates.
32163140
errorCase.old.ObjectMeta.ResourceVersion = "1"
32173141
errorCase.update.ObjectMeta.ResourceVersion = "2"

‎pkg/apis/core/validation/validation.go

Lines changed: 2 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -5910,7 +5910,7 @@ func ValidateNonEmptySelector(selectorMap map[string]string, fldPath *field.Path
59105910
}
59115911

59125912
// Validates the given template and ensures that it is in accordance with the desired selector and replicas.
5913-
func ValidatePodTemplateSpecForRC(template, oldTemplate *core.PodTemplateSpec, selectorMap map[string]string, replicas int32, fldPath *field.Path, opts PodValidationOptions) field.ErrorList {
5913+
func ValidatePodTemplateSpecForRC(template *core.PodTemplateSpec, selectorMap map[string]string, replicas int32, fldPath *field.Path, opts PodValidationOptions) field.ErrorList {
59145914
allErrs := field.ErrorList{}
59155915
if template == nil {
59165916
allErrs = append(allErrs, field.Required(fldPath, ""))
@@ -5924,14 +5924,6 @@ func ValidatePodTemplateSpecForRC(template, oldTemplate *core.PodTemplateSpec, s
59245924
}
59255925
}
59265926
allErrs = append(allErrs, ValidatePodTemplateSpec(template, fldPath, opts)...)
5927-
// get rid of apivalidation.ValidateReadOnlyPersistentDisks,stop passing oldTemplate to this function
5928-
var oldVols []core.Volume
5929-
if oldTemplate != nil {
5930-
oldVols = oldTemplate.Spec.Volumes // +k8s:verify-mutation:reason=clone
5931-
}
5932-
if replicas > 1 {
5933-
allErrs = append(allErrs, ValidateReadOnlyPersistentDisks(template.Spec.Volumes, oldVols, fldPath.Child("spec", "volumes"))...)
5934-
}
59355927
// RestartPolicy has already been first-order validated as per ValidatePodTemplateSpec().
59365928
if template.Spec.RestartPolicy != core.RestartPolicyAlways {
59375929
allErrs = append(allErrs, field.NotSupported(fldPath.Child("spec", "restartPolicy"), template.Spec.RestartPolicy, []core.RestartPolicy{core.RestartPolicyAlways}))
@@ -5949,12 +5941,7 @@ func ValidateReplicationControllerSpec(spec, oldSpec *core.ReplicationController
59495941
allErrs = append(allErrs, ValidateNonnegativeField(int64(spec.MinReadySeconds), fldPath.Child("minReadySeconds"))...)
59505942
allErrs = append(allErrs, ValidateNonEmptySelector(spec.Selector, fldPath.Child("selector"))...)
59515943
allErrs = append(allErrs, ValidateNonnegativeField(int64(spec.Replicas), fldPath.Child("replicas"))...)
5952-
// oldSpec is not empty, pass oldSpec.template.
5953-
var oldTemplate *core.PodTemplateSpec
5954-
if oldSpec != nil {
5955-
oldTemplate = oldSpec.Template // +k8s:verify-mutation:reason=clone
5956-
}
5957-
allErrs = append(allErrs, ValidatePodTemplateSpecForRC(spec.Template, oldTemplate, spec.Selector, spec.Replicas, fldPath.Child("template"), opts)...)
5944+
allErrs = append(allErrs, ValidatePodTemplateSpecForRC(spec.Template, spec.Selector, spec.Replicas, fldPath.Child("template"), opts)...)
59585945
return allErrs
59595946
}
59605947

@@ -5975,33 +5962,6 @@ func ValidatePodTemplateSpec(spec *core.PodTemplateSpec, fldPath *field.Path, op
59755962
return allErrs
59765963
}
59775964

5978-
// ValidateReadOnlyPersistentDisks stick this AFTER the short-circuit checks
5979-
func ValidateReadOnlyPersistentDisks(volumes, oldVolumes []core.Volume, fldPath *field.Path) field.ErrorList {
5980-
allErrs := field.ErrorList{}
5981-
5982-
if utilfeature.DefaultFeatureGate.Enabled(features.SkipReadOnlyValidationGCE) {
5983-
return field.ErrorList{}
5984-
}
5985-
5986-
isWriteablePD := func(vol *core.Volume) bool {
5987-
return vol.GCEPersistentDisk != nil && !vol.GCEPersistentDisk.ReadOnly
5988-
}
5989-
5990-
for i := range oldVolumes {
5991-
if isWriteablePD(&oldVolumes[i]) {
5992-
return field.ErrorList{}
5993-
}
5994-
}
5995-
5996-
for i := range volumes {
5997-
idxPath := fldPath.Index(i)
5998-
if isWriteablePD(&volumes[i]) {
5999-
allErrs = append(allErrs, field.Invalid(idxPath.Child("gcePersistentDisk", "readOnly"), false, "must be true for replicated pods > 1; GCE PD can only be mounted on multiple machines if it is read-only"))
6000-
}
6001-
}
6002-
return allErrs
6003-
}
6004-
60055965
// ValidateTaintsInNodeAnnotations tests that the serialized taints in Node.Annotations has valid data
60065966
func ValidateTaintsInNodeAnnotations(annotations map[string]string, fldPath *field.Path) field.ErrorList {
60075967
allErrs := field.ErrorList{}

0 commit comments

Comments
 (0)