From 5ed705faf8b74ba2da0848727199162ff95ea5e2 Mon Sep 17 00:00:00 2001 From: wackxu Date: Wed, 28 Mar 2018 14:45:06 +0800 Subject: [PATCH] Remove alpha annotation for volume node affinity --- pkg/apis/core/helper/helpers.go | 32 +---- pkg/apis/core/helper/helpers_test.go | 87 ------------- pkg/apis/core/v1/helper/helpers.go | 29 ----- pkg/apis/core/v1/helper/helpers_test.go | 87 ------------- pkg/apis/core/validation/validation.go | 40 +----- pkg/apis/core/validation/validation_test.go | 134 -------------------- pkg/volume/util/util.go | 28 ---- pkg/volume/util/util_test.go | 113 ----------------- 8 files changed, 2 insertions(+), 548 deletions(-) diff --git a/pkg/apis/core/helper/helpers.go b/pkg/apis/core/helper/helpers.go index 96899f18419..b9312b2b08f 100644 --- a/pkg/apis/core/helper/helpers.go +++ b/pkg/apis/core/helper/helpers.go @@ -564,34 +564,4 @@ func PersistentVolumeClaimHasClass(claim *core.PersistentVolumeClaim) bool { } return false -} - -// GetStorageNodeAffinityFromAnnotation gets the json serialized data from PersistentVolume.Annotations -// and converts it to the NodeAffinity type in core. -// TODO: update when storage node affinity graduates to beta -func GetStorageNodeAffinityFromAnnotation(annotations map[string]string) (*core.NodeAffinity, error) { - if len(annotations) > 0 && annotations[core.AlphaStorageNodeAffinityAnnotation] != "" { - var affinity core.NodeAffinity - err := json.Unmarshal([]byte(annotations[core.AlphaStorageNodeAffinityAnnotation]), &affinity) - if err != nil { - return nil, err - } - return &affinity, nil - } - return nil, nil -} - -// Converts NodeAffinity type to Alpha annotation for use in PersistentVolumes -// TODO: update when storage node affinity graduates to beta -func StorageNodeAffinityToAlphaAnnotation(annotations map[string]string, affinity *core.NodeAffinity) error { - if affinity == nil { - return nil - } - - json, err := json.Marshal(*affinity) - if err != nil { - return err - } - annotations[core.AlphaStorageNodeAffinityAnnotation] = string(json) - return nil -} +} \ No newline at end of file diff --git a/pkg/apis/core/helper/helpers_test.go b/pkg/apis/core/helper/helpers_test.go index 03e01f8083e..81bb96b786f 100644 --- a/pkg/apis/core/helper/helpers_test.go +++ b/pkg/apis/core/helper/helpers_test.go @@ -286,93 +286,6 @@ func TestSysctlsFromPodAnnotation(t *testing.T) { } } -// TODO: remove when alpha support for topology constraints is removed -func TestGetNodeAffinityFromAnnotations(t *testing.T) { - testCases := []struct { - annotations map[string]string - expectErr bool - }{ - { - annotations: nil, - expectErr: false, - }, - { - annotations: map[string]string{}, - expectErr: false, - }, - { - annotations: map[string]string{ - core.AlphaStorageNodeAffinityAnnotation: `{ - "requiredDuringSchedulingIgnoredDuringExecution": { - "nodeSelectorTerms": [ - { "matchExpressions": [ - { "key": "test-key1", - "operator": "In", - "values": ["test-value1", "test-value2"] - }, - { "key": "test-key2", - "operator": "In", - "values": ["test-value1", "test-value2"] - } - ]} - ]} - }`, - }, - expectErr: false, - }, - { - annotations: map[string]string{ - core.AlphaStorageNodeAffinityAnnotation: `[{ - "requiredDuringSchedulingIgnoredDuringExecution": { - "nodeSelectorTerms": [ - { "matchExpressions": [ - { "key": "test-key1", - "operator": "In", - "values": ["test-value1", "test-value2"] - }, - { "key": "test-key2", - "operator": "In", - "values": ["test-value1", "test-value2"] - } - ]} - ]} - }]`, - }, - expectErr: true, - }, - { - annotations: map[string]string{ - core.AlphaStorageNodeAffinityAnnotation: `{ - "requiredDuringSchedulingIgnoredDuringExecution": { - "nodeSelectorTerms": - "matchExpressions": [ - { "key": "test-key1", - "operator": "In", - "values": ["test-value1", "test-value2"] - }, - { "key": "test-key2", - "operator": "In", - "values": ["test-value1", "test-value2"] - } - ]} - } - }`, - }, - expectErr: true, - }, - } - - for i, tc := range testCases { - _, err := GetStorageNodeAffinityFromAnnotation(tc.annotations) - if err == nil && tc.expectErr { - t.Errorf("[%v]expected error but got none.", i) - } - if err != nil && !tc.expectErr { - t.Errorf("[%v]did not expect error but got: %v", i, err) - } - } -} - func TestIsHugePageResourceName(t *testing.T) { testCases := []struct { name core.ResourceName diff --git a/pkg/apis/core/v1/helper/helpers.go b/pkg/apis/core/v1/helper/helpers.go index bb6d9385f94..62d3287ea83 100644 --- a/pkg/apis/core/v1/helper/helpers.go +++ b/pkg/apis/core/v1/helper/helpers.go @@ -419,32 +419,3 @@ func GetPersistentVolumeClaimClass(claim *v1.PersistentVolumeClaim) string { return "" } -// GetStorageNodeAffinityFromAnnotation gets the json serialized data from PersistentVolume.Annotations -// and converts it to the NodeAffinity type in api. -// TODO: update when storage node affinity graduates to beta -func GetStorageNodeAffinityFromAnnotation(annotations map[string]string) (*v1.NodeAffinity, error) { - if len(annotations) > 0 && annotations[v1.AlphaStorageNodeAffinityAnnotation] != "" { - var affinity v1.NodeAffinity - err := json.Unmarshal([]byte(annotations[v1.AlphaStorageNodeAffinityAnnotation]), &affinity) - if err != nil { - return nil, err - } - return &affinity, nil - } - return nil, nil -} - -// Converts NodeAffinity type to Alpha annotation for use in PersistentVolumes -// TODO: update when storage node affinity graduates to beta -func StorageNodeAffinityToAlphaAnnotation(annotations map[string]string, affinity *v1.NodeAffinity) error { - if affinity == nil { - return nil - } - - json, err := json.Marshal(*affinity) - if err != nil { - return err - } - annotations[v1.AlphaStorageNodeAffinityAnnotation] = string(json) - return nil -} diff --git a/pkg/apis/core/v1/helper/helpers_test.go b/pkg/apis/core/v1/helper/helpers_test.go index be1ecfaf93d..8462abc955f 100644 --- a/pkg/apis/core/v1/helper/helpers_test.go +++ b/pkg/apis/core/v1/helper/helpers_test.go @@ -566,90 +566,3 @@ func TestSysctlsFromPodAnnotation(t *testing.T) { } } } - -// TODO: remove when alpha support for topology constraints is removed -func TestGetNodeAffinityFromAnnotations(t *testing.T) { - testCases := []struct { - annotations map[string]string - expectErr bool - }{ - { - annotations: nil, - expectErr: false, - }, - { - annotations: map[string]string{}, - expectErr: false, - }, - { - annotations: map[string]string{ - v1.AlphaStorageNodeAffinityAnnotation: `{ - "requiredDuringSchedulingIgnoredDuringExecution": { - "nodeSelectorTerms": [ - { "matchExpressions": [ - { "key": "test-key1", - "operator": "In", - "values": ["test-value1", "test-value2"] - }, - { "key": "test-key2", - "operator": "In", - "values": ["test-value1", "test-value2"] - } - ]} - ]} - }`, - }, - expectErr: false, - }, - { - annotations: map[string]string{ - v1.AlphaStorageNodeAffinityAnnotation: `[{ - "requiredDuringSchedulingIgnoredDuringExecution": { - "nodeSelectorTerms": [ - { "matchExpressions": [ - { "key": "test-key1", - "operator": "In", - "values": ["test-value1", "test-value2"] - }, - { "key": "test-key2", - "operator": "In", - "values": ["test-value1", "test-value2"] - } - ]} - ]} - }]`, - }, - expectErr: true, - }, - { - annotations: map[string]string{ - v1.AlphaStorageNodeAffinityAnnotation: `{ - "requiredDuringSchedulingIgnoredDuringExecution": { - "nodeSelectorTerms": - "matchExpressions": [ - { "key": "test-key1", - "operator": "In", - "values": ["test-value1", "test-value2"] - }, - { "key": "test-key2", - "operator": "In", - "values": ["test-value1", "test-value2"] - } - ]} - } - }`, - }, - expectErr: true, - }, - } - - for i, tc := range testCases { - _, err := GetStorageNodeAffinityFromAnnotation(tc.annotations) - if err == nil && tc.expectErr { - t.Errorf("[%v]expected error but got none.", i) - } - if err != nil && !tc.expectErr { - t.Errorf("[%v]did not expect error but got: %v", i, err) - } - } -} diff --git a/pkg/apis/core/validation/validation.go b/pkg/apis/core/validation/validation.go index 5b54d6fe642..e61b6fd98f3 100644 --- a/pkg/apis/core/validation/validation.go +++ b/pkg/apis/core/validation/validation.go @@ -1536,18 +1536,9 @@ func ValidatePersistentVolume(pv *core.PersistentVolume) field.ErrorList { } } - nodeAffinitySpecified, errs := validateStorageNodeAffinityAnnotation(pv.ObjectMeta.Annotations, metaPath.Child("annotations")) + nodeAffinitySpecified, errs := validateVolumeNodeAffinity(pv.Spec.NodeAffinity, specPath.Child("nodeAffinity")) allErrs = append(allErrs, errs...) - volumeNodeAffinitySpecified, errs := validateVolumeNodeAffinity(pv.Spec.NodeAffinity, specPath.Child("nodeAffinity")) - allErrs = append(allErrs, errs...) - - if nodeAffinitySpecified && volumeNodeAffinitySpecified { - allErrs = append(allErrs, field.Forbidden(specPath.Child("nodeAffinity"), "may not specify both alpha nodeAffinity annotation and nodeAffinity field")) - } - - nodeAffinitySpecified = nodeAffinitySpecified || volumeNodeAffinitySpecified - numVolumes := 0 if pv.Spec.HostPath != nil { if numVolumes > 0 { @@ -5009,35 +5000,6 @@ func sysctlIntersection(a []core.Sysctl, b []core.Sysctl) []string { return result } -// validateStorageNodeAffinityAnnotation tests that the serialized TopologyConstraints in PersistentVolume.Annotations has valid data -func validateStorageNodeAffinityAnnotation(annotations map[string]string, fldPath *field.Path) (bool, field.ErrorList) { - allErrs := field.ErrorList{} - - na, err := helper.GetStorageNodeAffinityFromAnnotation(annotations) - if err != nil { - allErrs = append(allErrs, field.Invalid(fldPath, core.AlphaStorageNodeAffinityAnnotation, err.Error())) - return false, allErrs - } - if na == nil { - return false, allErrs - } - - if !utilfeature.DefaultFeatureGate.Enabled(features.VolumeScheduling) { - allErrs = append(allErrs, field.Forbidden(fldPath, "Storage node affinity is disabled by feature-gate")) - } - - policySpecified := false - if na.RequiredDuringSchedulingIgnoredDuringExecution != nil { - allErrs = append(allErrs, ValidateNodeSelector(na.RequiredDuringSchedulingIgnoredDuringExecution, fldPath.Child("requiredDuringSchedulingIgnoredDuringExecution"))...) - policySpecified = true - } - - if len(na.PreferredDuringSchedulingIgnoredDuringExecution) > 0 { - allErrs = append(allErrs, field.Forbidden(fldPath.Child("preferredDuringSchedulingIgnoredDuringExection"), "Storage node affinity does not support preferredDuringSchedulingIgnoredDuringExecution")) - } - return policySpecified, allErrs -} - // validateVolumeNodeAffinity tests that the PersistentVolume.NodeAffinity has valid data // returns: // - true if volumeNodeAffinity is set diff --git a/pkg/apis/core/validation/validation_test.go b/pkg/apis/core/validation/validation_test.go index a48c04f7c52..7d13fb09871 100644 --- a/pkg/apis/core/validation/validation_test.go +++ b/pkg/apis/core/validation/validation_test.go @@ -35,7 +35,6 @@ import ( "k8s.io/kubernetes/pkg/api/legacyscheme" _ "k8s.io/kubernetes/pkg/api/testapi" "k8s.io/kubernetes/pkg/apis/core" - "k8s.io/kubernetes/pkg/apis/core/helper" "k8s.io/kubernetes/pkg/capabilities" "k8s.io/kubernetes/pkg/features" "k8s.io/kubernetes/pkg/security/apparmor" @@ -65,24 +64,6 @@ func testVolume(name string, namespace string, spec core.PersistentVolumeSpec) * } } -func testVolumeWithAlphaNodeAffinity(t *testing.T, name string, namespace string, affinity *core.NodeAffinity, spec core.PersistentVolumeSpec) *core.PersistentVolume { - objMeta := metav1.ObjectMeta{Name: name} - if namespace != "" { - objMeta.Namespace = namespace - } - - objMeta.Annotations = map[string]string{} - err := helper.StorageNodeAffinityToAlphaAnnotation(objMeta.Annotations, affinity) - if err != nil { - t.Fatalf("Failed to get node affinity annotation: %v", err) - } - - return &core.PersistentVolume{ - ObjectMeta: objMeta, - Spec: spec, - } -} - func TestValidatePersistentVolumes(t *testing.T) { validMode := core.PersistentVolumeFilesystem scenarios := map[string]struct { @@ -512,29 +493,6 @@ func TestValidateLocalVolumes(t *testing.T) { isExpectedFailure bool volume *core.PersistentVolume }{ - "alpha valid local volume": { - isExpectedFailure: false, - volume: testVolumeWithAlphaNodeAffinity( - t, - "valid-local-volume", - "", - &core.NodeAffinity{ - RequiredDuringSchedulingIgnoredDuringExecution: &core.NodeSelector{ - NodeSelectorTerms: []core.NodeSelectorTerm{ - { - MatchExpressions: []core.NodeSelectorRequirement{ - { - Key: "test-label-key", - Operator: core.NodeSelectorOpIn, - Values: []string{"test-label-value"}, - }, - }, - }, - }, - }, - }, - testLocalVolume("/foo", nil)), - }, "alpha invalid local volume nil annotations": { isExpectedFailure: true, volume: testVolume( @@ -542,75 +500,6 @@ func TestValidateLocalVolumes(t *testing.T) { "", testLocalVolume("/foo", nil)), }, - "alpha invalid local volume empty affinity": { - isExpectedFailure: true, - volume: testVolumeWithAlphaNodeAffinity( - t, - "invalid-local-volume-empty-affinity", - "", - &core.NodeAffinity{}, - testLocalVolume("/foo", nil)), - }, - "alpha invalid local volume preferred affinity": { - isExpectedFailure: true, - volume: testVolumeWithAlphaNodeAffinity( - t, - "invalid-local-volume-preferred-affinity", - "", - &core.NodeAffinity{ - RequiredDuringSchedulingIgnoredDuringExecution: &core.NodeSelector{ - NodeSelectorTerms: []core.NodeSelectorTerm{ - { - MatchExpressions: []core.NodeSelectorRequirement{ - { - Key: "test-label-key", - Operator: core.NodeSelectorOpIn, - Values: []string{"test-label-value"}, - }, - }, - }, - }, - }, - PreferredDuringSchedulingIgnoredDuringExecution: []core.PreferredSchedulingTerm{ - { - Weight: 10, - Preference: core.NodeSelectorTerm{ - MatchExpressions: []core.NodeSelectorRequirement{ - { - Key: "test-label-key", - Operator: core.NodeSelectorOpIn, - Values: []string{"test-label-value"}, - }, - }, - }, - }, - }, - }, - testLocalVolume("/foo", nil)), - }, - "alpha and beta local volume": { - isExpectedFailure: true, - volume: testVolumeWithAlphaNodeAffinity( - t, - "invalid-alpha-beta-local-volume", - "", - &core.NodeAffinity{ - RequiredDuringSchedulingIgnoredDuringExecution: &core.NodeSelector{ - NodeSelectorTerms: []core.NodeSelectorTerm{ - { - MatchExpressions: []core.NodeSelectorRequirement{ - { - Key: "test-label-key", - Operator: core.NodeSelectorOpIn, - Values: []string{"test-label-value"}, - }, - }, - }, - }, - }, - }, - testLocalVolume("/foo", simpleVolumeNodeAffinity("foo", "bar"))), - }, "valid local volume": { isExpectedFailure: false, volume: testVolume("valid-local-volume", "", @@ -654,29 +543,6 @@ func TestValidateLocalVolumesDisabled(t *testing.T) { isExpectedFailure bool volume *core.PersistentVolume }{ - "alpha disabled valid local volume": { - isExpectedFailure: true, - volume: testVolumeWithAlphaNodeAffinity( - t, - "valid-local-volume", - "", - &core.NodeAffinity{ - RequiredDuringSchedulingIgnoredDuringExecution: &core.NodeSelector{ - NodeSelectorTerms: []core.NodeSelectorTerm{ - { - MatchExpressions: []core.NodeSelectorRequirement{ - { - Key: "test-label-key", - Operator: core.NodeSelectorOpIn, - Values: []string{"test-label-value"}, - }, - }, - }, - }, - }, - }, - testLocalVolume("/foo", nil)), - }, "feature disabled valid local volume": { isExpectedFailure: true, volume: testVolume("valid-local-volume", "", diff --git a/pkg/volume/util/util.go b/pkg/volume/util/util.go index 518103414ca..05959589b15 100644 --- a/pkg/volume/util/util.go +++ b/pkg/volume/util/util.go @@ -266,37 +266,9 @@ func GetClassForVolume(kubeClient clientset.Interface, pv *v1.PersistentVolume) // CheckNodeAffinity looks at the PV node affinity, and checks if the node has the same corresponding labels // This ensures that we don't mount a volume that doesn't belong to this node func CheckNodeAffinity(pv *v1.PersistentVolume, nodeLabels map[string]string) error { - if err := checkAlphaNodeAffinity(pv, nodeLabels); err != nil { - return err - } return checkVolumeNodeAffinity(pv, nodeLabels) } -func checkAlphaNodeAffinity(pv *v1.PersistentVolume, nodeLabels map[string]string) error { - affinity, err := v1helper.GetStorageNodeAffinityFromAnnotation(pv.Annotations) - if err != nil { - return fmt.Errorf("Error getting storage node affinity: %v", err) - } - if affinity == nil { - return nil - } - - if affinity.RequiredDuringSchedulingIgnoredDuringExecution != nil { - terms := affinity.RequiredDuringSchedulingIgnoredDuringExecution.NodeSelectorTerms - glog.V(10).Infof("Match for RequiredDuringSchedulingIgnoredDuringExecution node selector terms %+v", terms) - for _, term := range terms { - selector, err := v1helper.NodeSelectorRequirementsAsSelector(term.MatchExpressions) - if err != nil { - return fmt.Errorf("Failed to parse MatchExpressions: %v", err) - } - if !selector.Matches(labels.Set(nodeLabels)) { - return fmt.Errorf("NodeSelectorTerm %+v does not match node labels", term.MatchExpressions) - } - } - } - return nil -} - func checkVolumeNodeAffinity(pv *v1.PersistentVolume, nodeLabels map[string]string) error { if pv.Spec.NodeAffinity == nil { return nil diff --git a/pkg/volume/util/util_test.go b/pkg/volume/util/util_test.go index ae4e6587d4b..5809211d5c0 100644 --- a/pkg/volume/util/util_test.go +++ b/pkg/volume/util/util_test.go @@ -29,7 +29,6 @@ import ( "hash/fnv" _ "k8s.io/kubernetes/pkg/apis/core/install" - "k8s.io/kubernetes/pkg/apis/core/v1/helper" "k8s.io/kubernetes/pkg/util/mount" "reflect" @@ -47,105 +46,6 @@ var nodeLabels map[string]string = map[string]string{ "test-key2": "test-value2", } -func TestCheckAlphaNodeAffinity(t *testing.T) { - type affinityTest struct { - name string - expectSuccess bool - pv *v1.PersistentVolume - } - - cases := []affinityTest{ - { - name: "valid-no-constraints", - expectSuccess: true, - pv: testVolumeWithAlphaNodeAffinity(t, &v1.NodeAffinity{}), - }, - { - name: "valid-constraints", - expectSuccess: true, - pv: testVolumeWithAlphaNodeAffinity(t, &v1.NodeAffinity{ - RequiredDuringSchedulingIgnoredDuringExecution: &v1.NodeSelector{ - NodeSelectorTerms: []v1.NodeSelectorTerm{ - { - MatchExpressions: []v1.NodeSelectorRequirement{ - { - Key: "test-key1", - Operator: v1.NodeSelectorOpIn, - Values: []string{"test-value1", "test-value3"}, - }, - { - Key: "test-key2", - Operator: v1.NodeSelectorOpIn, - Values: []string{"test-value0", "test-value2"}, - }, - }, - }, - }, - }, - }), - }, - { - name: "invalid-key", - expectSuccess: false, - pv: testVolumeWithAlphaNodeAffinity(t, &v1.NodeAffinity{ - RequiredDuringSchedulingIgnoredDuringExecution: &v1.NodeSelector{ - NodeSelectorTerms: []v1.NodeSelectorTerm{ - { - MatchExpressions: []v1.NodeSelectorRequirement{ - { - Key: "test-key1", - Operator: v1.NodeSelectorOpIn, - Values: []string{"test-value1", "test-value3"}, - }, - { - Key: "test-key3", - Operator: v1.NodeSelectorOpIn, - Values: []string{"test-value0", "test-value2"}, - }, - }, - }, - }, - }, - }), - }, - { - name: "invalid-values", - expectSuccess: false, - pv: testVolumeWithAlphaNodeAffinity(t, &v1.NodeAffinity{ - RequiredDuringSchedulingIgnoredDuringExecution: &v1.NodeSelector{ - NodeSelectorTerms: []v1.NodeSelectorTerm{ - { - MatchExpressions: []v1.NodeSelectorRequirement{ - { - Key: "test-key1", - Operator: v1.NodeSelectorOpIn, - Values: []string{"test-value3", "test-value4"}, - }, - { - Key: "test-key2", - Operator: v1.NodeSelectorOpIn, - Values: []string{"test-value0", "test-value2"}, - }, - }, - }, - }, - }, - }), - }, - } - - for _, c := range cases { - err := CheckNodeAffinity(c.pv, nodeLabels) - - if err != nil && c.expectSuccess { - t.Errorf("CheckTopology %v returned error: %v", c.name, err) - } - if err == nil && !c.expectSuccess { - t.Errorf("CheckTopology %v returned success, expected error", c.name) - } - } -} - func TestCheckVolumeNodeAffinity(t *testing.T) { type affinityTest struct { name string @@ -250,19 +150,6 @@ func TestCheckVolumeNodeAffinity(t *testing.T) { } } -func testVolumeWithAlphaNodeAffinity(t *testing.T, affinity *v1.NodeAffinity) *v1.PersistentVolume { - objMeta := metav1.ObjectMeta{Name: "test-constraints"} - objMeta.Annotations = map[string]string{} - err := helper.StorageNodeAffinityToAlphaAnnotation(objMeta.Annotations, affinity) - if err != nil { - t.Fatalf("Failed to get node affinity annotation: %v", err) - } - - return &v1.PersistentVolume{ - ObjectMeta: objMeta, - } -} - func testVolumeWithNodeAffinity(t *testing.T, affinity *v1.VolumeNodeAffinity) *v1.PersistentVolume { objMeta := metav1.ObjectMeta{Name: "test-constraints"} return &v1.PersistentVolume{