From 3fbfd7eb1b355f377beef2fd2deb31682b801caa Mon Sep 17 00:00:00 2001 From: Paco Xu Date: Mon, 7 Mar 2022 15:40:02 +0800 Subject: [PATCH 1/3] add deprecated warning for node beta labels in pv/sc/rc/csi storage capacity - (pv) deprecated label using warning for node affinity - (storageclass) deprecated node labels: allowedTopologies.matchLabelExpressions.key - (CSIStorageCapacity) deprecated node labels - (RuntimeClass) deprecated node labels --- hack/.import-aliases | 1 + pkg/api/node/util.go | 48 ++++++++++++ pkg/api/persistentvolume/util.go | 48 +++++++----- pkg/api/pod/warnings.go | 17 ++--- pkg/api/storage/util.go | 73 +++++++++++++++++++ .../core/persistentvolume/strategy.go | 4 +- pkg/registry/node/runtimeclass/strategy.go | 7 +- .../storage/csistoragecapacity/strategy.go | 5 +- pkg/registry/storage/storageclass/strategy.go | 5 +- 9 files changed, 171 insertions(+), 37 deletions(-) create mode 100644 pkg/api/node/util.go create mode 100644 pkg/api/storage/util.go diff --git a/hack/.import-aliases b/hack/.import-aliases index ea5bbde09b0ea..680b4dbb94e69 100644 --- a/hack/.import-aliases +++ b/hack/.import-aliases @@ -41,6 +41,7 @@ "k8s.io/api/storage/v1beta1": "storagev1beta1", "k8s.io/apimachinery/pkg/api/errors": "apierrors", "k8s.io/component-helpers/node/util": "nodeutil", + "k8s.io/kubernetes/pkg/api/node": "nodeapi", "k8s.io/kubernetes/pkg/controller/util/node": "controllerutil", "k8s.io/kubelet/apis/stats/v1alpha1": "kubeletstatsv1alpha1", "k8s.io/kubernetes/pkg/controller/apis/config/v1alpha1": "controllerconfigv1alpha1", diff --git a/pkg/api/node/util.go b/pkg/api/node/util.go new file mode 100644 index 0000000000000..0d593b9357a27 --- /dev/null +++ b/pkg/api/node/util.go @@ -0,0 +1,48 @@ +/* +Copyright 2022 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package persistentvolume + +import ( + "context" + "fmt" + + "k8s.io/apimachinery/pkg/util/validation/field" + "k8s.io/kubernetes/pkg/apis/node" +) + +var DeprecatedNodeLabels = map[string]string{ + `beta.kubernetes.io/arch`: `deprecated since v1.14; use "kubernetes.io/arch" instead`, + `beta.kubernetes.io/os`: `deprecated since v1.14; use "kubernetes.io/os" instead`, + `failure-domain.beta.kubernetes.io/region`: `deprecated since v1.17; use "topology.kubernetes.io/region" instead`, + `failure-domain.beta.kubernetes.io/zone`: `deprecated since v1.17; use "topology.kubernetes.io/zone" instead`, + `beta.kubernetes.io/instance-type`: `deprecated since v1.17; use "node.kubernetes.io/instance-type" instead`, +} + +func GetWarningsForRuntimeClass(ctx context.Context, rc *node.RuntimeClass) []string { + var warnings []string + + if rc != nil { + // use of deprecated node labels in scheduling's node affinity + for key, _ := range rc.Scheduling.NodeSelector { + if msg, deprecated := DeprecatedNodeLabels[key]; deprecated { + warnings = append(warnings, fmt.Sprintf("%s: %s", field.NewPath("scheduling", "nodeSelector"), msg)) + } + } + } + + return warnings +} diff --git a/pkg/api/persistentvolume/util.go b/pkg/api/persistentvolume/util.go index 844e709417ecc..d01081808646d 100644 --- a/pkg/api/persistentvolume/util.go +++ b/pkg/api/persistentvolume/util.go @@ -1,5 +1,5 @@ /* -Copyright 2017 The Kubernetes Authors. +Copyright 2022 The Kubernetes Authors. Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. @@ -17,28 +17,42 @@ limitations under the License. package persistentvolume import ( - utilfeature "k8s.io/apiserver/pkg/util/feature" + "context" + "fmt" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/util/validation/field" + nodeapi "k8s.io/kubernetes/pkg/api/node" api "k8s.io/kubernetes/pkg/apis/core" - "k8s.io/kubernetes/pkg/features" ) -// DropDisabledFields removes disabled fields from the pv spec. -// This should be called from PrepareForCreate/PrepareForUpdate for all resources containing a pv spec. -func DropDisabledFields(pvSpec *api.PersistentVolumeSpec, oldPVSpec *api.PersistentVolumeSpec) { - if !utilfeature.DefaultFeatureGate.Enabled(features.CSINodeExpandSecret) && !hasNodeExpansionSecrets(oldPVSpec) { - if pvSpec.CSI != nil { - pvSpec.CSI.NodeExpandSecretRef = nil - } +func GetWarningsForPersistentVolume(ctx context.Context, pv *api.PersistentVolume) []string { + if pv == nil { + return nil } + return warningsForPersistentVolumeSpecAndMeta(nil, &pv.Spec, &pv.ObjectMeta) } -func hasNodeExpansionSecrets(oldPVSpec *api.PersistentVolumeSpec) bool { - if oldPVSpec == nil || oldPVSpec.CSI == nil { - return false +func warningsForPersistentVolumeSpecAndMeta(fieldPath *field.Path, pvSpec *api.PersistentVolumeSpec, meta *metav1.ObjectMeta) []string { + var warnings []string + + // use of deprecated node labels in node affinity + for i, k := range pvSpec.NodeAffinity.Required.NodeSelectorTerms { + expressions := k.MatchExpressions + for j, e := range expressions { + if msg, deprecated := nodeapi.DeprecatedNodeLabels[e.Key]; deprecated { + warnings = append( + warnings, + fmt.Sprintf( + "%s: %s is %s", + fieldPath.Child("spec", "NodeAffinity").Child("Required").Child("NodeSelectorTerms").Index(i).Child("MatchExpressions").Index(j).Child("key"), + e.Key, + msg, + ), + ) + } + } } - if oldPVSpec.CSI.NodeExpandSecretRef != nil { - return true - } - return false + return warnings } diff --git a/pkg/api/pod/warnings.go b/pkg/api/pod/warnings.go index 3a9328f919369..f7429798e27bc 100644 --- a/pkg/api/pod/warnings.go +++ b/pkg/api/pod/warnings.go @@ -24,6 +24,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/sets" "k8s.io/apimachinery/pkg/util/validation/field" + nodeapi "k8s.io/kubernetes/pkg/api/node" api "k8s.io/kubernetes/pkg/apis/core" "k8s.io/kubernetes/pkg/apis/core/pods" ) @@ -60,14 +61,6 @@ func GetWarningsForPodTemplate(ctx context.Context, fieldPath *field.Path, podTe return warningsForPodSpecAndMeta(fieldPath, &podTemplate.Spec, &podTemplate.ObjectMeta, oldSpec, oldMeta) } -var deprecatedNodeLabels = map[string]string{ - `beta.kubernetes.io/arch`: `deprecated since v1.14; use "kubernetes.io/arch" instead`, - `beta.kubernetes.io/os`: `deprecated since v1.14; use "kubernetes.io/os" instead`, - `failure-domain.beta.kubernetes.io/region`: `deprecated since v1.17; use "topology.kubernetes.io/region" instead`, - `failure-domain.beta.kubernetes.io/zone`: `deprecated since v1.17; use "topology.kubernetes.io/zone" instead`, - `beta.kubernetes.io/instance-type`: `deprecated since v1.17; use "node.kubernetes.io/instance-type" instead`, -} - var deprecatedAnnotations = []struct { key string prefix string @@ -92,7 +85,7 @@ func warningsForPodSpecAndMeta(fieldPath *field.Path, podSpec *api.PodSpec, meta // use of deprecated node labels in selectors/affinity/topology for k := range podSpec.NodeSelector { - if msg, deprecated := deprecatedNodeLabels[k]; deprecated { + if msg, deprecated := nodeapi.DeprecatedNodeLabels[k]; deprecated { warnings = append(warnings, fmt.Sprintf("%s: %s", fieldPath.Child("spec", "nodeSelector").Key(k), msg)) } } @@ -101,7 +94,7 @@ func warningsForPodSpecAndMeta(fieldPath *field.Path, podSpec *api.PodSpec, meta if n.RequiredDuringSchedulingIgnoredDuringExecution != nil { for i, t := range n.RequiredDuringSchedulingIgnoredDuringExecution.NodeSelectorTerms { for j, e := range t.MatchExpressions { - if msg, deprecated := deprecatedNodeLabels[e.Key]; deprecated { + if msg, deprecated := nodeapi.DeprecatedNodeLabels[e.Key]; deprecated { warnings = append( warnings, fmt.Sprintf( @@ -119,7 +112,7 @@ func warningsForPodSpecAndMeta(fieldPath *field.Path, podSpec *api.PodSpec, meta } for i, t := range n.PreferredDuringSchedulingIgnoredDuringExecution { for j, e := range t.Preference.MatchExpressions { - if msg, deprecated := deprecatedNodeLabels[e.Key]; deprecated { + if msg, deprecated := nodeapi.DeprecatedNodeLabels[e.Key]; deprecated { warnings = append( warnings, fmt.Sprintf( @@ -137,7 +130,7 @@ func warningsForPodSpecAndMeta(fieldPath *field.Path, podSpec *api.PodSpec, meta } } for i, t := range podSpec.TopologySpreadConstraints { - if msg, deprecated := deprecatedNodeLabels[t.TopologyKey]; deprecated { + if msg, deprecated := nodeapi.DeprecatedNodeLabels[t.TopologyKey]; deprecated { warnings = append(warnings, fmt.Sprintf( "%s: %s is %s", fieldPath.Child("spec", "topologySpreadConstraints").Index(i).Child("topologyKey"), diff --git a/pkg/api/storage/util.go b/pkg/api/storage/util.go new file mode 100644 index 0000000000000..84ee5d185b887 --- /dev/null +++ b/pkg/api/storage/util.go @@ -0,0 +1,73 @@ +/* +Copyright 2022 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package storage + +import ( + "context" + "fmt" + + "k8s.io/apimachinery/pkg/util/validation/field" + nodeapi "k8s.io/kubernetes/pkg/api/node" + "k8s.io/kubernetes/pkg/apis/storage" +) + +func GetWarningsForStorageClass(ctx context.Context, sc *storage.StorageClass) []string { + var warnings []string + + if sc != nil { + // use of deprecated node labels in allowedTopologies's matchLabelExpressions + for i, topo := range sc.AllowedTopologies { + for j, expression := range topo.MatchLabelExpressions { + if msg, deprecated := nodeapi.DeprecatedNodeLabels[expression.Key]; deprecated { + warnings = append(warnings, fmt.Sprintf("%s: %s", field.NewPath("allowedTopologies").Index(i).Child("matchLabelExpressions").Index(j), msg)) + } + } + } + } + + return warnings +} + +func GetWarningsForCSIStorageCapacity(ctx context.Context, csc *storage.CSIStorageCapacity) []string { + var warnings []string + + if csc != nil { + // use of deprecated node labels in allowedTopologies's matchLabelExpressions + for i, expression := range csc.NodeTopology.MatchExpressions { + if msg, deprecated := nodeapi.DeprecatedNodeLabels[expression.Key]; deprecated { + warnings = append( + warnings, + fmt.Sprintf( + "%s: %s is %s", + field.NewPath("nodeTopology").Child("matchExpressions").Index(i), + expression.Key, + msg, + ), + ) + } + } + + // use of deprecated node labels in allowedTopologies's matchLabels + for label, _ := range csc.NodeTopology.MatchLabels { + if msg, deprecated := nodeapi.DeprecatedNodeLabels[label]; deprecated { + warnings = append(warnings, fmt.Sprintf("%s: %s", field.NewPath("nodeTopology").Child("matchLabels").Child(label), msg)) + } + } + } + + return warnings +} diff --git a/pkg/registry/core/persistentvolume/strategy.go b/pkg/registry/core/persistentvolume/strategy.go index 89b5a0332fd4c..a9e190f7964c6 100644 --- a/pkg/registry/core/persistentvolume/strategy.go +++ b/pkg/registry/core/persistentvolume/strategy.go @@ -78,7 +78,7 @@ func (persistentvolumeStrategy) Validate(ctx context.Context, obj runtime.Object // WarningsOnCreate returns warnings for the creation of the given object. func (persistentvolumeStrategy) WarningsOnCreate(ctx context.Context, obj runtime.Object) []string { - return nil + return pvutil.GetWarningsForPersistentVolume(ctx, obj.(*api.PersistentVolume)) } // Canonicalize normalizes the object after validation. @@ -109,7 +109,7 @@ func (persistentvolumeStrategy) ValidateUpdate(ctx context.Context, obj, old run // WarningsOnUpdate returns warnings for the given update. func (persistentvolumeStrategy) WarningsOnUpdate(ctx context.Context, obj, old runtime.Object) []string { - return nil + return pvutil.GetWarningsForPersistentVolume(ctx, obj.(*api.PersistentVolume)) } func (persistentvolumeStrategy) AllowUnconditionalUpdate() bool { diff --git a/pkg/registry/node/runtimeclass/strategy.go b/pkg/registry/node/runtimeclass/strategy.go index 4d0a80514462f..995d7b5e27184 100644 --- a/pkg/registry/node/runtimeclass/strategy.go +++ b/pkg/registry/node/runtimeclass/strategy.go @@ -24,6 +24,7 @@ import ( "k8s.io/apiserver/pkg/registry/rest" "k8s.io/apiserver/pkg/storage/names" "k8s.io/kubernetes/pkg/api/legacyscheme" + nodeapi "k8s.io/kubernetes/pkg/api/node" "k8s.io/kubernetes/pkg/apis/node" "k8s.io/kubernetes/pkg/apis/node/validation" ) @@ -73,7 +74,9 @@ func (strategy) Validate(ctx context.Context, obj runtime.Object) field.ErrorLis } // WarningsOnCreate returns warnings for the creation of the given object. -func (strategy) WarningsOnCreate(ctx context.Context, obj runtime.Object) []string { return nil } +func (strategy) WarningsOnCreate(ctx context.Context, obj runtime.Object) []string { + return nodeapi.GetWarningsForRuntimeClass(ctx, obj.(*node.RuntimeClass)) +} // Canonicalize normalizes the object after validation. func (strategy) Canonicalize(obj runtime.Object) { @@ -89,7 +92,7 @@ func (strategy) ValidateUpdate(ctx context.Context, obj, old runtime.Object) fie // WarningsOnUpdate returns warnings for the given update. func (strategy) WarningsOnUpdate(ctx context.Context, obj, old runtime.Object) []string { - return nil + return nodeapi.GetWarningsForRuntimeClass(ctx, obj.(*node.RuntimeClass)) } // If AllowUnconditionalUpdate() is true and the object specified by diff --git a/pkg/registry/storage/csistoragecapacity/strategy.go b/pkg/registry/storage/csistoragecapacity/strategy.go index 5e294fbe2542b..95df1945b4d3e 100644 --- a/pkg/registry/storage/csistoragecapacity/strategy.go +++ b/pkg/registry/storage/csistoragecapacity/strategy.go @@ -23,6 +23,7 @@ import ( "k8s.io/apimachinery/pkg/util/validation/field" "k8s.io/apiserver/pkg/storage/names" "k8s.io/kubernetes/pkg/api/legacyscheme" + storageutil "k8s.io/kubernetes/pkg/api/storage" "k8s.io/kubernetes/pkg/apis/storage" "k8s.io/kubernetes/pkg/apis/storage/validation" ) @@ -56,7 +57,7 @@ func (csiStorageCapacityStrategy) Validate(ctx context.Context, obj runtime.Obje // WarningsOnCreate returns warnings for the creation of the given object. func (csiStorageCapacityStrategy) WarningsOnCreate(ctx context.Context, obj runtime.Object) []string { - return nil + return storageutil.GetWarningsForCSIStorageCapacity(ctx, obj.(*storage.CSIStorageCapacity)) } // Canonicalize normalizes the object after validation. @@ -80,7 +81,7 @@ func (csiStorageCapacityStrategy) ValidateUpdate(ctx context.Context, obj, old r // WarningsOnUpdate returns warnings for the given update. func (csiStorageCapacityStrategy) WarningsOnUpdate(ctx context.Context, obj, old runtime.Object) []string { - return nil + return storageutil.GetWarningsForCSIStorageCapacity(ctx, obj.(*storage.CSIStorageCapacity)) } func (csiStorageCapacityStrategy) AllowUnconditionalUpdate() bool { diff --git a/pkg/registry/storage/storageclass/strategy.go b/pkg/registry/storage/storageclass/strategy.go index a73f906792553..5c540e5fb84b7 100644 --- a/pkg/registry/storage/storageclass/strategy.go +++ b/pkg/registry/storage/storageclass/strategy.go @@ -23,6 +23,7 @@ import ( "k8s.io/apimachinery/pkg/util/validation/field" "k8s.io/apiserver/pkg/storage/names" "k8s.io/kubernetes/pkg/api/legacyscheme" + storageutil2 "k8s.io/kubernetes/pkg/api/storage" "k8s.io/kubernetes/pkg/apis/storage" "k8s.io/kubernetes/pkg/apis/storage/validation" ) @@ -52,7 +53,7 @@ func (storageClassStrategy) Validate(ctx context.Context, obj runtime.Object) fi // WarningsOnCreate returns warnings for the creation of the given object. func (storageClassStrategy) WarningsOnCreate(ctx context.Context, obj runtime.Object) []string { - return nil + return storageutil2.GetWarningsForStorageClass(ctx, obj.(*storage.StorageClass)) } // Canonicalize normalizes the object after validation. @@ -74,7 +75,7 @@ func (storageClassStrategy) ValidateUpdate(ctx context.Context, obj, old runtime // WarningsOnUpdate returns warnings for the given update. func (storageClassStrategy) WarningsOnUpdate(ctx context.Context, obj, old runtime.Object) []string { - return nil + return storageutil2.GetWarningsForStorageClass(ctx, obj.(*storage.StorageClass)) } func (storageClassStrategy) AllowUnconditionalUpdate() bool { From db147b7d674e4c1f0c29a8005755143b3cf29b89 Mon Sep 17 00:00:00 2001 From: Paco Xu Date: Mon, 7 Mar 2022 18:17:28 +0800 Subject: [PATCH 2/3] (ut) add tests for bad filed values checking; refactor some funcs --- hack/.import-aliases | 1 - pkg/api/node/util.go | 74 +++++++- pkg/api/node/util_test.go | 81 +++++++++ pkg/api/persistentvolume/util.go | 49 +++-- pkg/api/persistentvolume/util_test.go | 76 +++++++- pkg/api/pod/warnings.go | 43 +---- pkg/api/storage/util.go | 33 +--- pkg/api/storage/util_test.go | 172 ++++++++++++++++++ pkg/registry/node/runtimeclass/strategy.go | 4 +- pkg/registry/storage/storageclass/strategy.go | 6 +- test/e2e/framework/.import-restrictions | 2 + 11 files changed, 445 insertions(+), 96 deletions(-) create mode 100644 pkg/api/node/util_test.go create mode 100644 pkg/api/storage/util_test.go diff --git a/hack/.import-aliases b/hack/.import-aliases index 680b4dbb94e69..ea5bbde09b0ea 100644 --- a/hack/.import-aliases +++ b/hack/.import-aliases @@ -41,7 +41,6 @@ "k8s.io/api/storage/v1beta1": "storagev1beta1", "k8s.io/apimachinery/pkg/api/errors": "apierrors", "k8s.io/component-helpers/node/util": "nodeutil", - "k8s.io/kubernetes/pkg/api/node": "nodeapi", "k8s.io/kubernetes/pkg/controller/util/node": "controllerutil", "k8s.io/kubelet/apis/stats/v1alpha1": "kubeletstatsv1alpha1", "k8s.io/kubernetes/pkg/controller/apis/config/v1alpha1": "controllerconfigv1alpha1", diff --git a/pkg/api/node/util.go b/pkg/api/node/util.go index 0d593b9357a27..166366dfb0de6 100644 --- a/pkg/api/node/util.go +++ b/pkg/api/node/util.go @@ -14,17 +14,18 @@ See the License for the specific language governing permissions and limitations under the License. */ -package persistentvolume +package node import ( - "context" "fmt" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/validation/field" + api "k8s.io/kubernetes/pkg/apis/core" "k8s.io/kubernetes/pkg/apis/node" ) -var DeprecatedNodeLabels = map[string]string{ +var deprecatedNodeLabels = map[string]string{ `beta.kubernetes.io/arch`: `deprecated since v1.14; use "kubernetes.io/arch" instead`, `beta.kubernetes.io/os`: `deprecated since v1.14; use "kubernetes.io/os" instead`, `failure-domain.beta.kubernetes.io/region`: `deprecated since v1.17; use "topology.kubernetes.io/region" instead`, @@ -32,13 +33,20 @@ var DeprecatedNodeLabels = map[string]string{ `beta.kubernetes.io/instance-type`: `deprecated since v1.17; use "node.kubernetes.io/instance-type" instead`, } -func GetWarningsForRuntimeClass(ctx context.Context, rc *node.RuntimeClass) []string { +// GetLabelDeprecatedMessage returns the message for the deprecated label +// and a bool indicating if the label is deprecated. +func GetLabelDeprecatedMessage(key string) (string, bool) { + msg, ok := deprecatedNodeLabels[key] + return msg, ok +} + +func GetWarningsForRuntimeClass(rc *node.RuntimeClass) []string { var warnings []string - if rc != nil { + if rc != nil && rc.Scheduling != nil && rc.Scheduling.NodeSelector != nil { // use of deprecated node labels in scheduling's node affinity - for key, _ := range rc.Scheduling.NodeSelector { - if msg, deprecated := DeprecatedNodeLabels[key]; deprecated { + for key := range rc.Scheduling.NodeSelector { + if msg, deprecated := GetLabelDeprecatedMessage(key); deprecated { warnings = append(warnings, fmt.Sprintf("%s: %s", field.NewPath("scheduling", "nodeSelector"), msg)) } } @@ -46,3 +54,55 @@ func GetWarningsForRuntimeClass(ctx context.Context, rc *node.RuntimeClass) []st return warnings } + +// WarningsForNodeSelector tests if any of the node selector requirements in the template is deprecated. +// If there are deprecated node selector requirements in either match expressions or match labels, a warning is returned. +func WarningsForNodeSelector(nodeSelector *metav1.LabelSelector, fieldPath *field.Path) []string { + if nodeSelector == nil { + return nil + } + + var warnings []string + // use of deprecated node labels in matchLabelExpressions + for i, expression := range nodeSelector.MatchExpressions { + if msg, deprecated := GetLabelDeprecatedMessage(expression.Key); deprecated { + warnings = append( + warnings, + fmt.Sprintf( + "%s: %s is %s", + fieldPath.Child("matchExpressions").Index(i).Child("key"), + expression.Key, + msg, + ), + ) + } + } + + // use of deprecated node labels in matchLabels + for label := range nodeSelector.MatchLabels { + if msg, deprecated := GetLabelDeprecatedMessage(label); deprecated { + warnings = append(warnings, fmt.Sprintf("%s: %s", fieldPath.Child("matchLabels").Child(label), msg)) + } + } + return warnings +} + +// WarningsForNodeSelectorTerm checks match expressions of node selector term +func WarningsForNodeSelectorTerm(nodeSelectorTerm api.NodeSelectorTerm, fieldPath *field.Path) []string { + var warnings []string + // use of deprecated node labels in matchLabelExpressions + for i, expression := range nodeSelectorTerm.MatchExpressions { + if msg, deprecated := GetLabelDeprecatedMessage(expression.Key); deprecated { + warnings = append( + warnings, + fmt.Sprintf( + "%s: %s is %s", + fieldPath.Child("matchExpressions").Index(i).Child("key"), + expression.Key, + msg, + ), + ) + } + } + return warnings +} diff --git a/pkg/api/node/util_test.go b/pkg/api/node/util_test.go new file mode 100644 index 0000000000000..02b1fd152f67b --- /dev/null +++ b/pkg/api/node/util_test.go @@ -0,0 +1,81 @@ +/* +Copyright 2022 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package node + +import ( + "testing" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/util/sets" + + "k8s.io/kubernetes/pkg/apis/node" +) + +func TestWarnings(t *testing.T) { + testcases := []struct { + name string + template *node.RuntimeClass + expected []string + }{ + { + name: "null", + template: nil, + expected: nil, + }, + { + name: "no warning", + template: &node.RuntimeClass{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo", + }, + }, + expected: nil, + }, + { + name: "warning", + template: &node.RuntimeClass{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo", + }, + Scheduling: &node.Scheduling{ + NodeSelector: map[string]string{ + "beta.kubernetes.io/arch": "amd64", + "beta.kubernetes.io/os": "linux", + }, + }, + }, + expected: []string{ + `scheduling.nodeSelector: deprecated since v1.14; use "kubernetes.io/arch" instead`, + `scheduling.nodeSelector: deprecated since v1.14; use "kubernetes.io/os" instead`, + }, + }, + } + + for _, tc := range testcases { + t.Run("podspec_"+tc.name, func(t *testing.T) { + actual := sets.NewString(GetWarningsForRuntimeClass(tc.template)...) + expected := sets.NewString(tc.expected...) + for _, missing := range expected.Difference(actual).List() { + t.Errorf("missing: %s", missing) + } + for _, extra := range actual.Difference(expected).List() { + t.Errorf("extra: %s", extra) + } + }) + + } +} diff --git a/pkg/api/persistentvolume/util.go b/pkg/api/persistentvolume/util.go index d01081808646d..a86df0ba4a52b 100644 --- a/pkg/api/persistentvolume/util.go +++ b/pkg/api/persistentvolume/util.go @@ -18,39 +18,50 @@ package persistentvolume import ( "context" - "fmt" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/validation/field" + utilfeature "k8s.io/apiserver/pkg/util/feature" nodeapi "k8s.io/kubernetes/pkg/api/node" api "k8s.io/kubernetes/pkg/apis/core" + "k8s.io/kubernetes/pkg/features" ) +// DropDisabledFields removes disabled fields from the pv spec. +// This should be called from PrepareForCreate/PrepareForUpdate for all resources containing a pv spec. +func DropDisabledFields(pvSpec *api.PersistentVolumeSpec, oldPVSpec *api.PersistentVolumeSpec) { + if !utilfeature.DefaultFeatureGate.Enabled(features.CSINodeExpandSecret) && !hasNodeExpansionSecrets(oldPVSpec) { + if pvSpec.CSI != nil { + pvSpec.CSI.NodeExpandSecretRef = nil + } + } +} + +func hasNodeExpansionSecrets(oldPVSpec *api.PersistentVolumeSpec) bool { + if oldPVSpec == nil || oldPVSpec.CSI == nil { + return false + } + + if oldPVSpec.CSI.NodeExpandSecretRef != nil { + return true + } + return false +} + func GetWarningsForPersistentVolume(ctx context.Context, pv *api.PersistentVolume) []string { if pv == nil { return nil } - return warningsForPersistentVolumeSpecAndMeta(nil, &pv.Spec, &pv.ObjectMeta) + return warningsForPersistentVolumeSpecAndMeta(nil, &pv.Spec) } -func warningsForPersistentVolumeSpecAndMeta(fieldPath *field.Path, pvSpec *api.PersistentVolumeSpec, meta *metav1.ObjectMeta) []string { +func warningsForPersistentVolumeSpecAndMeta(fieldPath *field.Path, pvSpec *api.PersistentVolumeSpec) []string { var warnings []string - // use of deprecated node labels in node affinity - for i, k := range pvSpec.NodeAffinity.Required.NodeSelectorTerms { - expressions := k.MatchExpressions - for j, e := range expressions { - if msg, deprecated := nodeapi.DeprecatedNodeLabels[e.Key]; deprecated { - warnings = append( - warnings, - fmt.Sprintf( - "%s: %s is %s", - fieldPath.Child("spec", "NodeAffinity").Child("Required").Child("NodeSelectorTerms").Index(i).Child("MatchExpressions").Index(j).Child("key"), - e.Key, - msg, - ), - ) - } + if pvSpec.NodeAffinity != nil && pvSpec.NodeAffinity.Required != nil { + termFldPath := fieldPath.Child("spec", "nodeAffinity", "required", "nodeSelectorTerms") + // use of deprecated node labels in node affinity + for i, term := range pvSpec.NodeAffinity.Required.NodeSelectorTerms { + warnings = append(warnings, nodeapi.WarningsForNodeSelectorTerm(term, termFldPath.Index(i))...) } } diff --git a/pkg/api/persistentvolume/util_test.go b/pkg/api/persistentvolume/util_test.go index 3874adb18c7df..b143c1fef2e89 100644 --- a/pkg/api/persistentvolume/util_test.go +++ b/pkg/api/persistentvolume/util_test.go @@ -1,5 +1,5 @@ /* -Copyright 2018 The Kubernetes Authors. +Copyright 2022 The Kubernetes Authors. Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. @@ -17,11 +17,14 @@ limitations under the License. package persistentvolume import ( + "context" "reflect" "testing" "github.com/google/go-cmp/cmp" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/util/sets" utilfeature "k8s.io/apiserver/pkg/util/feature" featuregatetesting "k8s.io/component-base/featuregate/testing" api "k8s.io/kubernetes/pkg/apis/core" @@ -108,3 +111,74 @@ func specWithCSISecrets(secret *api.SecretReference) *api.PersistentVolumeSpec { } return pvSpec } + +func TestWarnings(t *testing.T) { + testcases := []struct { + name string + template *api.PersistentVolume + expected []string + }{ + { + name: "null", + template: nil, + expected: nil, + }, + { + name: "no warning", + template: &api.PersistentVolume{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo", + }, + Status: api.PersistentVolumeStatus{ + Phase: api.VolumeBound, + }, + }, + expected: nil, + }, + { + name: "warning", + template: &api.PersistentVolume{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo", + }, + Spec: api.PersistentVolumeSpec{ + NodeAffinity: &api.VolumeNodeAffinity{ + Required: &api.NodeSelector{ + NodeSelectorTerms: []api.NodeSelectorTerm{ + { + MatchExpressions: []api.NodeSelectorRequirement{ + { + Key: "beta.kubernetes.io/os", + Operator: "Equal", + Values: []string{"windows"}, + }, + }, + }, + }, + }, + }, + }, + Status: api.PersistentVolumeStatus{ + Phase: api.VolumeBound, + }, + }, + expected: []string{ + `spec.nodeAffinity.required.nodeSelectorTerms[0].matchExpressions[0].key: beta.kubernetes.io/os is deprecated since v1.14; use "kubernetes.io/os" instead`, + }, + }, + } + + for _, tc := range testcases { + t.Run("podspec_"+tc.name, func(t *testing.T) { + actual := sets.NewString(GetWarningsForPersistentVolume(context.TODO(), tc.template)...) + expected := sets.NewString(tc.expected...) + for _, missing := range expected.Difference(actual).List() { + t.Errorf("missing: %s", missing) + } + for _, extra := range actual.Difference(expected).List() { + t.Errorf("extra: %s", extra) + } + }) + + } +} diff --git a/pkg/api/pod/warnings.go b/pkg/api/pod/warnings.go index f7429798e27bc..53c905f1d1534 100644 --- a/pkg/api/pod/warnings.go +++ b/pkg/api/pod/warnings.go @@ -85,52 +85,25 @@ func warningsForPodSpecAndMeta(fieldPath *field.Path, podSpec *api.PodSpec, meta // use of deprecated node labels in selectors/affinity/topology for k := range podSpec.NodeSelector { - if msg, deprecated := nodeapi.DeprecatedNodeLabels[k]; deprecated { + if msg, deprecated := nodeapi.GetLabelDeprecatedMessage(k); deprecated { warnings = append(warnings, fmt.Sprintf("%s: %s", fieldPath.Child("spec", "nodeSelector").Key(k), msg)) } } if podSpec.Affinity != nil && podSpec.Affinity.NodeAffinity != nil { n := podSpec.Affinity.NodeAffinity if n.RequiredDuringSchedulingIgnoredDuringExecution != nil { - for i, t := range n.RequiredDuringSchedulingIgnoredDuringExecution.NodeSelectorTerms { - for j, e := range t.MatchExpressions { - if msg, deprecated := nodeapi.DeprecatedNodeLabels[e.Key]; deprecated { - warnings = append( - warnings, - fmt.Sprintf( - "%s: %s is %s", - fieldPath.Child("spec", "affinity", "nodeAffinity", "requiredDuringSchedulingIgnoredDuringExecution", "nodeSelectorTerms").Index(i). - Child("matchExpressions").Index(j). - Child("key"), - e.Key, - msg, - ), - ) - } - } + termFldPath := fieldPath.Child("spec", "affinity", "nodeAffinity", "requiredDuringSchedulingIgnoredDuringExecution", "nodeSelectorTerms") + for i, term := range n.RequiredDuringSchedulingIgnoredDuringExecution.NodeSelectorTerms { + warnings = append(warnings, nodeapi.WarningsForNodeSelectorTerm(term, termFldPath.Index(i))...) } } - for i, t := range n.PreferredDuringSchedulingIgnoredDuringExecution { - for j, e := range t.Preference.MatchExpressions { - if msg, deprecated := nodeapi.DeprecatedNodeLabels[e.Key]; deprecated { - warnings = append( - warnings, - fmt.Sprintf( - "%s: %s is %s", - fieldPath.Child("spec", "affinity", "nodeAffinity", "preferredDuringSchedulingIgnoredDuringExecution").Index(i). - Child("preference"). - Child("matchExpressions").Index(j). - Child("key"), - e.Key, - msg, - ), - ) - } - } + preferredFldPath := fieldPath.Child("spec", "affinity", "nodeAffinity", "preferredDuringSchedulingIgnoredDuringExecution") + for i, term := range n.PreferredDuringSchedulingIgnoredDuringExecution { + warnings = append(warnings, nodeapi.WarningsForNodeSelectorTerm(term.Preference, preferredFldPath.Index(i).Child("preference"))...) } } for i, t := range podSpec.TopologySpreadConstraints { - if msg, deprecated := nodeapi.DeprecatedNodeLabels[t.TopologyKey]; deprecated { + if msg, deprecated := nodeapi.GetLabelDeprecatedMessage(t.TopologyKey); deprecated { warnings = append(warnings, fmt.Sprintf( "%s: %s is %s", fieldPath.Child("spec", "topologySpreadConstraints").Index(i).Child("topologyKey"), diff --git a/pkg/api/storage/util.go b/pkg/api/storage/util.go index 84ee5d185b887..7c130feaf4149 100644 --- a/pkg/api/storage/util.go +++ b/pkg/api/storage/util.go @@ -28,12 +28,12 @@ import ( func GetWarningsForStorageClass(ctx context.Context, sc *storage.StorageClass) []string { var warnings []string - if sc != nil { + if sc != nil && sc.AllowedTopologies != nil { // use of deprecated node labels in allowedTopologies's matchLabelExpressions for i, topo := range sc.AllowedTopologies { for j, expression := range topo.MatchLabelExpressions { - if msg, deprecated := nodeapi.DeprecatedNodeLabels[expression.Key]; deprecated { - warnings = append(warnings, fmt.Sprintf("%s: %s", field.NewPath("allowedTopologies").Index(i).Child("matchLabelExpressions").Index(j), msg)) + if msg, deprecated := nodeapi.GetLabelDeprecatedMessage(expression.Key); deprecated { + warnings = append(warnings, fmt.Sprintf("%s: %s", field.NewPath("allowedTopologies").Index(i).Child("matchLabelExpressions").Index(j).Child("key"), msg)) } } } @@ -43,31 +43,8 @@ func GetWarningsForStorageClass(ctx context.Context, sc *storage.StorageClass) [ } func GetWarningsForCSIStorageCapacity(ctx context.Context, csc *storage.CSIStorageCapacity) []string { - var warnings []string - if csc != nil { - // use of deprecated node labels in allowedTopologies's matchLabelExpressions - for i, expression := range csc.NodeTopology.MatchExpressions { - if msg, deprecated := nodeapi.DeprecatedNodeLabels[expression.Key]; deprecated { - warnings = append( - warnings, - fmt.Sprintf( - "%s: %s is %s", - field.NewPath("nodeTopology").Child("matchExpressions").Index(i), - expression.Key, - msg, - ), - ) - } - } - - // use of deprecated node labels in allowedTopologies's matchLabels - for label, _ := range csc.NodeTopology.MatchLabels { - if msg, deprecated := nodeapi.DeprecatedNodeLabels[label]; deprecated { - warnings = append(warnings, fmt.Sprintf("%s: %s", field.NewPath("nodeTopology").Child("matchLabels").Child(label), msg)) - } - } + return nodeapi.WarningsForNodeSelector(csc.NodeTopology, field.NewPath("nodeTopology")) } - - return warnings + return nil } diff --git a/pkg/api/storage/util_test.go b/pkg/api/storage/util_test.go new file mode 100644 index 0000000000000..17c0fbb164577 --- /dev/null +++ b/pkg/api/storage/util_test.go @@ -0,0 +1,172 @@ +/* +Copyright 2022 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package storage + +import ( + "context" + "testing" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/util/sets" + + "k8s.io/kubernetes/pkg/apis/core" + "k8s.io/kubernetes/pkg/apis/storage" +) + +func TestStorageClassWarnings(t *testing.T) { + testcases := []struct { + name string + template *storage.StorageClass + expected []string + }{ + { + name: "null", + template: nil, + expected: nil, + }, + { + name: "no warning", + template: &storage.StorageClass{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo", + }, + }, + expected: nil, + }, + { + name: "warning", + template: &storage.StorageClass{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo", + }, + AllowedTopologies: []core.TopologySelectorTerm{ + { + MatchLabelExpressions: []core.TopologySelectorLabelRequirement{ + { + Key: "beta.kubernetes.io/arch", + Values: []string{"amd64"}, + }, + { + Key: "beta.kubernetes.io/os", + Values: []string{"linux"}, + }, + }, + }, + }, + }, + expected: []string{ + `allowedTopologies[0].matchLabelExpressions[0].key: deprecated since v1.14; use "kubernetes.io/arch" instead`, + `allowedTopologies[0].matchLabelExpressions[1].key: deprecated since v1.14; use "kubernetes.io/os" instead`, + }, + }, + } + + for _, tc := range testcases { + t.Run("podspec_"+tc.name, func(t *testing.T) { + actual := sets.NewString(GetWarningsForStorageClass(context.TODO(), tc.template)...) + expected := sets.NewString(tc.expected...) + for _, missing := range expected.Difference(actual).List() { + t.Errorf("missing: %s", missing) + } + for _, extra := range actual.Difference(expected).List() { + t.Errorf("extra: %s", extra) + } + }) + + } +} + +func TestCSIStorageCapacityWarnings(t *testing.T) { + testcases := []struct { + name string + template *storage.CSIStorageCapacity + expected []string + }{ + { + name: "null", + template: nil, + expected: nil, + }, + { + name: "no warning", + template: &storage.CSIStorageCapacity{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo", + }, + }, + expected: nil, + }, + { + name: "MatchLabels warning", + template: &storage.CSIStorageCapacity{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo", + }, + NodeTopology: &metav1.LabelSelector{ + MatchLabels: map[string]string{ + "beta.kubernetes.io/arch": "amd64", + "beta.kubernetes.io/os": "linux", + }, + }, + }, + expected: []string{ + `nodeTopology.matchLabels.beta.kubernetes.io/arch: deprecated since v1.14; use "kubernetes.io/arch" instead`, + `nodeTopology.matchLabels.beta.kubernetes.io/os: deprecated since v1.14; use "kubernetes.io/os" instead`, + }, + }, + { + name: "MatchExpressions warning", + template: &storage.CSIStorageCapacity{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo", + }, + NodeTopology: &metav1.LabelSelector{ + MatchExpressions: []metav1.LabelSelectorRequirement{ + { + Key: "beta.kubernetes.io/arch", + Operator: metav1.LabelSelectorOpIn, + Values: []string{"amd64"}, + }, + { + Key: "beta.kubernetes.io/os", + Operator: metav1.LabelSelectorOpIn, + Values: []string{"linux"}, + }, + }, + }, + }, + expected: []string{ + `nodeTopology.matchExpressions[0].key: beta.kubernetes.io/arch is deprecated since v1.14; use "kubernetes.io/arch" instead`, + `nodeTopology.matchExpressions[1].key: beta.kubernetes.io/os is deprecated since v1.14; use "kubernetes.io/os" instead`, + }, + }, + } + + for _, tc := range testcases { + t.Run("podspec_"+tc.name, func(t *testing.T) { + actual := sets.NewString(GetWarningsForCSIStorageCapacity(context.TODO(), tc.template)...) + expected := sets.NewString(tc.expected...) + for _, missing := range expected.Difference(actual).List() { + t.Errorf("missing: %s", missing) + } + for _, extra := range actual.Difference(expected).List() { + t.Errorf("extra: %s", extra) + } + }) + + } +} diff --git a/pkg/registry/node/runtimeclass/strategy.go b/pkg/registry/node/runtimeclass/strategy.go index 995d7b5e27184..6acc70d9b30df 100644 --- a/pkg/registry/node/runtimeclass/strategy.go +++ b/pkg/registry/node/runtimeclass/strategy.go @@ -75,7 +75,7 @@ func (strategy) Validate(ctx context.Context, obj runtime.Object) field.ErrorLis // WarningsOnCreate returns warnings for the creation of the given object. func (strategy) WarningsOnCreate(ctx context.Context, obj runtime.Object) []string { - return nodeapi.GetWarningsForRuntimeClass(ctx, obj.(*node.RuntimeClass)) + return nodeapi.GetWarningsForRuntimeClass(obj.(*node.RuntimeClass)) } // Canonicalize normalizes the object after validation. @@ -92,7 +92,7 @@ func (strategy) ValidateUpdate(ctx context.Context, obj, old runtime.Object) fie // WarningsOnUpdate returns warnings for the given update. func (strategy) WarningsOnUpdate(ctx context.Context, obj, old runtime.Object) []string { - return nodeapi.GetWarningsForRuntimeClass(ctx, obj.(*node.RuntimeClass)) + return nodeapi.GetWarningsForRuntimeClass(obj.(*node.RuntimeClass)) } // If AllowUnconditionalUpdate() is true and the object specified by diff --git a/pkg/registry/storage/storageclass/strategy.go b/pkg/registry/storage/storageclass/strategy.go index 5c540e5fb84b7..c3b86b7ab11e8 100644 --- a/pkg/registry/storage/storageclass/strategy.go +++ b/pkg/registry/storage/storageclass/strategy.go @@ -23,7 +23,7 @@ import ( "k8s.io/apimachinery/pkg/util/validation/field" "k8s.io/apiserver/pkg/storage/names" "k8s.io/kubernetes/pkg/api/legacyscheme" - storageutil2 "k8s.io/kubernetes/pkg/api/storage" + storageutil "k8s.io/kubernetes/pkg/api/storage" "k8s.io/kubernetes/pkg/apis/storage" "k8s.io/kubernetes/pkg/apis/storage/validation" ) @@ -53,7 +53,7 @@ func (storageClassStrategy) Validate(ctx context.Context, obj runtime.Object) fi // WarningsOnCreate returns warnings for the creation of the given object. func (storageClassStrategy) WarningsOnCreate(ctx context.Context, obj runtime.Object) []string { - return storageutil2.GetWarningsForStorageClass(ctx, obj.(*storage.StorageClass)) + return storageutil.GetWarningsForStorageClass(ctx, obj.(*storage.StorageClass)) } // Canonicalize normalizes the object after validation. @@ -75,7 +75,7 @@ func (storageClassStrategy) ValidateUpdate(ctx context.Context, obj, old runtime // WarningsOnUpdate returns warnings for the given update. func (storageClassStrategy) WarningsOnUpdate(ctx context.Context, obj, old runtime.Object) []string { - return storageutil2.GetWarningsForStorageClass(ctx, obj.(*storage.StorageClass)) + return storageutil.GetWarningsForStorageClass(ctx, obj.(*storage.StorageClass)) } func (storageClassStrategy) AllowUnconditionalUpdate() bool { diff --git a/test/e2e/framework/.import-restrictions b/test/e2e/framework/.import-restrictions index 13c7be44b0f21..d515c37f202ed 100644 --- a/test/e2e/framework/.import-restrictions +++ b/test/e2e/framework/.import-restrictions @@ -7,6 +7,7 @@ rules: - k8s.io/kubernetes/pkg/api/v1/resource - k8s.io/kubernetes/pkg/api/v1/service - k8s.io/kubernetes/pkg/api/pod + - k8s.io/kubernetes/pkg/api/node - k8s.io/kubernetes/pkg/apis/apps - k8s.io/kubernetes/pkg/apis/apps/validation - k8s.io/kubernetes/pkg/apis/autoscaling @@ -23,6 +24,7 @@ rules: - k8s.io/kubernetes/pkg/apis/core/validation - k8s.io/kubernetes/pkg/apis/extensions - k8s.io/kubernetes/pkg/apis/networking + - k8s.io/kubernetes/pkg/apis/node - k8s.io/kubernetes/pkg/apis/policy - k8s.io/kubernetes/pkg/apis/policy/validation - k8s.io/kubernetes/pkg/apis/scheduling From 234c33e8b83335cacbf39fc374e2c4dca737b13f Mon Sep 17 00:00:00 2001 From: Paco Xu Date: Wed, 25 May 2022 11:41:51 +0800 Subject: [PATCH 3/3] deprecated node labels: make naming consistant and remove some unused args in funcs --- pkg/api/node/util.go | 20 +++++++++---------- pkg/api/persistentvolume/util.go | 8 +++----- pkg/api/persistentvolume/util_test.go | 5 ++--- pkg/api/pod/warnings.go | 8 ++++---- pkg/api/storage/util.go | 9 ++++----- pkg/api/storage/util_test.go | 5 ++--- .../core/persistentvolume/strategy.go | 4 ++-- .../storage/csistoragecapacity/strategy.go | 4 ++-- pkg/registry/storage/storageclass/strategy.go | 4 ++-- 9 files changed, 31 insertions(+), 36 deletions(-) diff --git a/pkg/api/node/util.go b/pkg/api/node/util.go index 166366dfb0de6..fcc6dba41454a 100644 --- a/pkg/api/node/util.go +++ b/pkg/api/node/util.go @@ -33,9 +33,9 @@ var deprecatedNodeLabels = map[string]string{ `beta.kubernetes.io/instance-type`: `deprecated since v1.17; use "node.kubernetes.io/instance-type" instead`, } -// GetLabelDeprecatedMessage returns the message for the deprecated label +// GetNodeLabelDeprecatedMessage returns the message for the deprecated node label // and a bool indicating if the label is deprecated. -func GetLabelDeprecatedMessage(key string) (string, bool) { +func GetNodeLabelDeprecatedMessage(key string) (string, bool) { msg, ok := deprecatedNodeLabels[key] return msg, ok } @@ -46,7 +46,7 @@ func GetWarningsForRuntimeClass(rc *node.RuntimeClass) []string { if rc != nil && rc.Scheduling != nil && rc.Scheduling.NodeSelector != nil { // use of deprecated node labels in scheduling's node affinity for key := range rc.Scheduling.NodeSelector { - if msg, deprecated := GetLabelDeprecatedMessage(key); deprecated { + if msg, deprecated := GetNodeLabelDeprecatedMessage(key); deprecated { warnings = append(warnings, fmt.Sprintf("%s: %s", field.NewPath("scheduling", "nodeSelector"), msg)) } } @@ -55,9 +55,9 @@ func GetWarningsForRuntimeClass(rc *node.RuntimeClass) []string { return warnings } -// WarningsForNodeSelector tests if any of the node selector requirements in the template is deprecated. +// GetWarningsForNodeSelector tests if any of the node selector requirements in the template is deprecated. // If there are deprecated node selector requirements in either match expressions or match labels, a warning is returned. -func WarningsForNodeSelector(nodeSelector *metav1.LabelSelector, fieldPath *field.Path) []string { +func GetWarningsForNodeSelector(nodeSelector *metav1.LabelSelector, fieldPath *field.Path) []string { if nodeSelector == nil { return nil } @@ -65,7 +65,7 @@ func WarningsForNodeSelector(nodeSelector *metav1.LabelSelector, fieldPath *fiel var warnings []string // use of deprecated node labels in matchLabelExpressions for i, expression := range nodeSelector.MatchExpressions { - if msg, deprecated := GetLabelDeprecatedMessage(expression.Key); deprecated { + if msg, deprecated := GetNodeLabelDeprecatedMessage(expression.Key); deprecated { warnings = append( warnings, fmt.Sprintf( @@ -80,19 +80,19 @@ func WarningsForNodeSelector(nodeSelector *metav1.LabelSelector, fieldPath *fiel // use of deprecated node labels in matchLabels for label := range nodeSelector.MatchLabels { - if msg, deprecated := GetLabelDeprecatedMessage(label); deprecated { + if msg, deprecated := GetNodeLabelDeprecatedMessage(label); deprecated { warnings = append(warnings, fmt.Sprintf("%s: %s", fieldPath.Child("matchLabels").Child(label), msg)) } } return warnings } -// WarningsForNodeSelectorTerm checks match expressions of node selector term -func WarningsForNodeSelectorTerm(nodeSelectorTerm api.NodeSelectorTerm, fieldPath *field.Path) []string { +// GetWarningsForNodeSelectorTerm checks match expressions of node selector term +func GetWarningsForNodeSelectorTerm(nodeSelectorTerm api.NodeSelectorTerm, fieldPath *field.Path) []string { var warnings []string // use of deprecated node labels in matchLabelExpressions for i, expression := range nodeSelectorTerm.MatchExpressions { - if msg, deprecated := GetLabelDeprecatedMessage(expression.Key); deprecated { + if msg, deprecated := GetNodeLabelDeprecatedMessage(expression.Key); deprecated { warnings = append( warnings, fmt.Sprintf( diff --git a/pkg/api/persistentvolume/util.go b/pkg/api/persistentvolume/util.go index a86df0ba4a52b..29862d73d5b6a 100644 --- a/pkg/api/persistentvolume/util.go +++ b/pkg/api/persistentvolume/util.go @@ -1,5 +1,5 @@ /* -Copyright 2022 The Kubernetes Authors. +Copyright 2017 The Kubernetes Authors. Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. @@ -17,8 +17,6 @@ limitations under the License. package persistentvolume import ( - "context" - "k8s.io/apimachinery/pkg/util/validation/field" utilfeature "k8s.io/apiserver/pkg/util/feature" nodeapi "k8s.io/kubernetes/pkg/api/node" @@ -47,7 +45,7 @@ func hasNodeExpansionSecrets(oldPVSpec *api.PersistentVolumeSpec) bool { return false } -func GetWarningsForPersistentVolume(ctx context.Context, pv *api.PersistentVolume) []string { +func GetWarningsForPersistentVolume(pv *api.PersistentVolume) []string { if pv == nil { return nil } @@ -61,7 +59,7 @@ func warningsForPersistentVolumeSpecAndMeta(fieldPath *field.Path, pvSpec *api.P termFldPath := fieldPath.Child("spec", "nodeAffinity", "required", "nodeSelectorTerms") // use of deprecated node labels in node affinity for i, term := range pvSpec.NodeAffinity.Required.NodeSelectorTerms { - warnings = append(warnings, nodeapi.WarningsForNodeSelectorTerm(term, termFldPath.Index(i))...) + warnings = append(warnings, nodeapi.GetWarningsForNodeSelectorTerm(term, termFldPath.Index(i))...) } } diff --git a/pkg/api/persistentvolume/util_test.go b/pkg/api/persistentvolume/util_test.go index b143c1fef2e89..1838c054c38e9 100644 --- a/pkg/api/persistentvolume/util_test.go +++ b/pkg/api/persistentvolume/util_test.go @@ -1,5 +1,5 @@ /* -Copyright 2022 The Kubernetes Authors. +Copyright 2018 The Kubernetes Authors. Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. @@ -17,7 +17,6 @@ limitations under the License. package persistentvolume import ( - "context" "reflect" "testing" @@ -170,7 +169,7 @@ func TestWarnings(t *testing.T) { for _, tc := range testcases { t.Run("podspec_"+tc.name, func(t *testing.T) { - actual := sets.NewString(GetWarningsForPersistentVolume(context.TODO(), tc.template)...) + actual := sets.NewString(GetWarningsForPersistentVolume(tc.template)...) expected := sets.NewString(tc.expected...) for _, missing := range expected.Difference(actual).List() { t.Errorf("missing: %s", missing) diff --git a/pkg/api/pod/warnings.go b/pkg/api/pod/warnings.go index 53c905f1d1534..c0b000a68fa27 100644 --- a/pkg/api/pod/warnings.go +++ b/pkg/api/pod/warnings.go @@ -85,7 +85,7 @@ func warningsForPodSpecAndMeta(fieldPath *field.Path, podSpec *api.PodSpec, meta // use of deprecated node labels in selectors/affinity/topology for k := range podSpec.NodeSelector { - if msg, deprecated := nodeapi.GetLabelDeprecatedMessage(k); deprecated { + if msg, deprecated := nodeapi.GetNodeLabelDeprecatedMessage(k); deprecated { warnings = append(warnings, fmt.Sprintf("%s: %s", fieldPath.Child("spec", "nodeSelector").Key(k), msg)) } } @@ -94,16 +94,16 @@ func warningsForPodSpecAndMeta(fieldPath *field.Path, podSpec *api.PodSpec, meta if n.RequiredDuringSchedulingIgnoredDuringExecution != nil { termFldPath := fieldPath.Child("spec", "affinity", "nodeAffinity", "requiredDuringSchedulingIgnoredDuringExecution", "nodeSelectorTerms") for i, term := range n.RequiredDuringSchedulingIgnoredDuringExecution.NodeSelectorTerms { - warnings = append(warnings, nodeapi.WarningsForNodeSelectorTerm(term, termFldPath.Index(i))...) + warnings = append(warnings, nodeapi.GetWarningsForNodeSelectorTerm(term, termFldPath.Index(i))...) } } preferredFldPath := fieldPath.Child("spec", "affinity", "nodeAffinity", "preferredDuringSchedulingIgnoredDuringExecution") for i, term := range n.PreferredDuringSchedulingIgnoredDuringExecution { - warnings = append(warnings, nodeapi.WarningsForNodeSelectorTerm(term.Preference, preferredFldPath.Index(i).Child("preference"))...) + warnings = append(warnings, nodeapi.GetWarningsForNodeSelectorTerm(term.Preference, preferredFldPath.Index(i).Child("preference"))...) } } for i, t := range podSpec.TopologySpreadConstraints { - if msg, deprecated := nodeapi.GetLabelDeprecatedMessage(t.TopologyKey); deprecated { + if msg, deprecated := nodeapi.GetNodeLabelDeprecatedMessage(t.TopologyKey); deprecated { warnings = append(warnings, fmt.Sprintf( "%s: %s is %s", fieldPath.Child("spec", "topologySpreadConstraints").Index(i).Child("topologyKey"), diff --git a/pkg/api/storage/util.go b/pkg/api/storage/util.go index 7c130feaf4149..133d8b2b64651 100644 --- a/pkg/api/storage/util.go +++ b/pkg/api/storage/util.go @@ -17,7 +17,6 @@ limitations under the License. package storage import ( - "context" "fmt" "k8s.io/apimachinery/pkg/util/validation/field" @@ -25,14 +24,14 @@ import ( "k8s.io/kubernetes/pkg/apis/storage" ) -func GetWarningsForStorageClass(ctx context.Context, sc *storage.StorageClass) []string { +func GetWarningsForStorageClass(sc *storage.StorageClass) []string { var warnings []string if sc != nil && sc.AllowedTopologies != nil { // use of deprecated node labels in allowedTopologies's matchLabelExpressions for i, topo := range sc.AllowedTopologies { for j, expression := range topo.MatchLabelExpressions { - if msg, deprecated := nodeapi.GetLabelDeprecatedMessage(expression.Key); deprecated { + if msg, deprecated := nodeapi.GetNodeLabelDeprecatedMessage(expression.Key); deprecated { warnings = append(warnings, fmt.Sprintf("%s: %s", field.NewPath("allowedTopologies").Index(i).Child("matchLabelExpressions").Index(j).Child("key"), msg)) } } @@ -42,9 +41,9 @@ func GetWarningsForStorageClass(ctx context.Context, sc *storage.StorageClass) [ return warnings } -func GetWarningsForCSIStorageCapacity(ctx context.Context, csc *storage.CSIStorageCapacity) []string { +func GetWarningsForCSIStorageCapacity(csc *storage.CSIStorageCapacity) []string { if csc != nil { - return nodeapi.WarningsForNodeSelector(csc.NodeTopology, field.NewPath("nodeTopology")) + return nodeapi.GetWarningsForNodeSelector(csc.NodeTopology, field.NewPath("nodeTopology")) } return nil } diff --git a/pkg/api/storage/util_test.go b/pkg/api/storage/util_test.go index 17c0fbb164577..5f34570487ce2 100644 --- a/pkg/api/storage/util_test.go +++ b/pkg/api/storage/util_test.go @@ -17,7 +17,6 @@ limitations under the License. package storage import ( - "context" "testing" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -77,7 +76,7 @@ func TestStorageClassWarnings(t *testing.T) { for _, tc := range testcases { t.Run("podspec_"+tc.name, func(t *testing.T) { - actual := sets.NewString(GetWarningsForStorageClass(context.TODO(), tc.template)...) + actual := sets.NewString(GetWarningsForStorageClass(tc.template)...) expected := sets.NewString(tc.expected...) for _, missing := range expected.Difference(actual).List() { t.Errorf("missing: %s", missing) @@ -158,7 +157,7 @@ func TestCSIStorageCapacityWarnings(t *testing.T) { for _, tc := range testcases { t.Run("podspec_"+tc.name, func(t *testing.T) { - actual := sets.NewString(GetWarningsForCSIStorageCapacity(context.TODO(), tc.template)...) + actual := sets.NewString(GetWarningsForCSIStorageCapacity(tc.template)...) expected := sets.NewString(tc.expected...) for _, missing := range expected.Difference(actual).List() { t.Errorf("missing: %s", missing) diff --git a/pkg/registry/core/persistentvolume/strategy.go b/pkg/registry/core/persistentvolume/strategy.go index a9e190f7964c6..e175a4097adbb 100644 --- a/pkg/registry/core/persistentvolume/strategy.go +++ b/pkg/registry/core/persistentvolume/strategy.go @@ -78,7 +78,7 @@ func (persistentvolumeStrategy) Validate(ctx context.Context, obj runtime.Object // WarningsOnCreate returns warnings for the creation of the given object. func (persistentvolumeStrategy) WarningsOnCreate(ctx context.Context, obj runtime.Object) []string { - return pvutil.GetWarningsForPersistentVolume(ctx, obj.(*api.PersistentVolume)) + return pvutil.GetWarningsForPersistentVolume(obj.(*api.PersistentVolume)) } // Canonicalize normalizes the object after validation. @@ -109,7 +109,7 @@ func (persistentvolumeStrategy) ValidateUpdate(ctx context.Context, obj, old run // WarningsOnUpdate returns warnings for the given update. func (persistentvolumeStrategy) WarningsOnUpdate(ctx context.Context, obj, old runtime.Object) []string { - return pvutil.GetWarningsForPersistentVolume(ctx, obj.(*api.PersistentVolume)) + return pvutil.GetWarningsForPersistentVolume(obj.(*api.PersistentVolume)) } func (persistentvolumeStrategy) AllowUnconditionalUpdate() bool { diff --git a/pkg/registry/storage/csistoragecapacity/strategy.go b/pkg/registry/storage/csistoragecapacity/strategy.go index 95df1945b4d3e..559b6bba04e7b 100644 --- a/pkg/registry/storage/csistoragecapacity/strategy.go +++ b/pkg/registry/storage/csistoragecapacity/strategy.go @@ -57,7 +57,7 @@ func (csiStorageCapacityStrategy) Validate(ctx context.Context, obj runtime.Obje // WarningsOnCreate returns warnings for the creation of the given object. func (csiStorageCapacityStrategy) WarningsOnCreate(ctx context.Context, obj runtime.Object) []string { - return storageutil.GetWarningsForCSIStorageCapacity(ctx, obj.(*storage.CSIStorageCapacity)) + return storageutil.GetWarningsForCSIStorageCapacity(obj.(*storage.CSIStorageCapacity)) } // Canonicalize normalizes the object after validation. @@ -81,7 +81,7 @@ func (csiStorageCapacityStrategy) ValidateUpdate(ctx context.Context, obj, old r // WarningsOnUpdate returns warnings for the given update. func (csiStorageCapacityStrategy) WarningsOnUpdate(ctx context.Context, obj, old runtime.Object) []string { - return storageutil.GetWarningsForCSIStorageCapacity(ctx, obj.(*storage.CSIStorageCapacity)) + return storageutil.GetWarningsForCSIStorageCapacity(obj.(*storage.CSIStorageCapacity)) } func (csiStorageCapacityStrategy) AllowUnconditionalUpdate() bool { diff --git a/pkg/registry/storage/storageclass/strategy.go b/pkg/registry/storage/storageclass/strategy.go index c3b86b7ab11e8..7e833a06f4f78 100644 --- a/pkg/registry/storage/storageclass/strategy.go +++ b/pkg/registry/storage/storageclass/strategy.go @@ -53,7 +53,7 @@ func (storageClassStrategy) Validate(ctx context.Context, obj runtime.Object) fi // WarningsOnCreate returns warnings for the creation of the given object. func (storageClassStrategy) WarningsOnCreate(ctx context.Context, obj runtime.Object) []string { - return storageutil.GetWarningsForStorageClass(ctx, obj.(*storage.StorageClass)) + return storageutil.GetWarningsForStorageClass(obj.(*storage.StorageClass)) } // Canonicalize normalizes the object after validation. @@ -75,7 +75,7 @@ func (storageClassStrategy) ValidateUpdate(ctx context.Context, obj, old runtime // WarningsOnUpdate returns warnings for the given update. func (storageClassStrategy) WarningsOnUpdate(ctx context.Context, obj, old runtime.Object) []string { - return storageutil.GetWarningsForStorageClass(ctx, obj.(*storage.StorageClass)) + return storageutil.GetWarningsForStorageClass(obj.(*storage.StorageClass)) } func (storageClassStrategy) AllowUnconditionalUpdate() bool {