Merge pull request #62556 from msau42/fix-node-selector-term

Automatic merge from submit-queue. If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>. Fix volume node affinity to OR node selector terms **What this PR does / why we need it**: Fixes node selector terms to be ORed, to be consistent with documentation and Pod.NodeAffinity. Also handles the "node selector term nil or empty matches nothing" behavior. **Which issue(s) this PR fixes** *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close the issue(s) when PR gets merged)*: Fixes #62551 **Special notes for your reviewer**: **Release note**: ```release-note Fixes issue where PersistentVolume.NodeAffinity.NodeSelectorTerms were ANDed instead of ORed. ```

Merge pull request #62556 from msau42/fix-node-selector-term
e36fa85e · Kubernetes Submit Queue · GitHub · 0eb364a3 · d2ab5f29 · e36fa85e
Unverified Commit e36fa85e authored Apr 16, 2018 by Kubernetes Submit Queue Committed by GitHub Apr 16, 2018
Hide whitespace changes
Inline Side-by-side

Showing with 81 additions and 5 deletions

util.go pkg/volume/util/util.go +4 -2

util_test.go pkg/volume/util/util_test.go +77 -3

No files found.
--- a/pkg/volume/util/util.go
+++ b/pkg/volume/util/util.go
@@ -283,10 +283,12 @@ func checkVolumeNodeAffinity(pv *v1.PersistentVolume, nodeLabels map[string]stri
 			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)
+			if selector.Matches(labels.Set(nodeLabels)) {
+				// Terms are ORed, so only one needs to match
+				return nil
 			}
 		}
+		return fmt.Errorf("No matching NodeSelectorTerms")
 	}
 	return nil
 }

--- a/pkg/volume/util/util_test.go
+++ b/pkg/volume/util/util_test.go
@@ -66,7 +66,53 @@ func TestCheckVolumeNodeAffinity(t *testing.T) {
 			pv:            testVolumeWithNodeAffinity(t, &v1.VolumeNodeAffinity{}),
 		},
 		{
-			name:          "valid-constraints",
+			name:          "select-nothing",
+			expectSuccess: false,
+			pv:            testVolumeWithNodeAffinity(t, &v1.VolumeNodeAffinity{Required: &v1.NodeSelector{}}),
+		},
+		{
+			name:          "select-nothing-empty-terms",
+			expectSuccess: false,
+			pv: testVolumeWithNodeAffinity(t, &v1.VolumeNodeAffinity{
+				Required: &v1.NodeSelector{
+					NodeSelectorTerms: []v1.NodeSelectorTerm{
+						{
+							MatchExpressions: []v1.NodeSelectorRequirement{},
+						},
+					},
+				},
+			}),
+		},
+		{
+			name:          "valid-multiple-terms",
+			expectSuccess: true,
+			pv: testVolumeWithNodeAffinity(t, &v1.VolumeNodeAffinity{
+				Required: &v1.NodeSelector{
+					NodeSelectorTerms: []v1.NodeSelectorTerm{
+						{
+							MatchExpressions: []v1.NodeSelectorRequirement{
+								{
+									Key:      "test-key3",
+									Operator: v1.NodeSelectorOpIn,
+									Values:   []string{"test-value1", "test-value3"},
+								},
+							},
+						},
+						{
+							MatchExpressions: []v1.NodeSelectorRequirement{
+								{
+									Key:      "test-key2",
+									Operator: v1.NodeSelectorOpIn,
+									Values:   []string{"test-value0", "test-value2"},
+								},
+							},
+						},
+					},
+				},
+			}),
+		},
+		{
+			name:          "valid-multiple-match-expressions",
 			expectSuccess: true,
 			pv: testVolumeWithNodeAffinity(t, &v1.VolumeNodeAffinity{
 				Required: &v1.NodeSelector{
@@ -90,7 +136,7 @@ func TestCheckVolumeNodeAffinity(t *testing.T) {
 			}),
 		},
 		{
-			name:          "invalid-key",
+			name:          "invalid-multiple-match-expressions-key",
 			expectSuccess: false,
 			pv: testVolumeWithNodeAffinity(t, &v1.VolumeNodeAffinity{
 				Required: &v1.NodeSelector{
@@ -114,7 +160,7 @@ func TestCheckVolumeNodeAffinity(t *testing.T) {
 			}),
 		},
 		{
-			name:          "invalid-values",
+			name:          "invalid-multiple-match-expressions-values",
 			expectSuccess: false,
 			pv: testVolumeWithNodeAffinity(t, &v1.VolumeNodeAffinity{
 				Required: &v1.NodeSelector{
@@ -137,6 +183,34 @@ func TestCheckVolumeNodeAffinity(t *testing.T) {
 				},
 			}),
 		},
+		{
+			name:          "invalid-multiple-terms",
+			expectSuccess: false,
+			pv: testVolumeWithNodeAffinity(t, &v1.VolumeNodeAffinity{
+				Required: &v1.NodeSelector{
+					NodeSelectorTerms: []v1.NodeSelectorTerm{
+						{
+							MatchExpressions: []v1.NodeSelectorRequirement{
+								{
+									Key:      "test-key3",
+									Operator: v1.NodeSelectorOpIn,
+									Values:   []string{"test-value1", "test-value3"},
+								},
+							},
+						},
+						{
+							MatchExpressions: []v1.NodeSelectorRequirement{
+								{
+									Key:      "test-key2",
+									Operator: v1.NodeSelectorOpIn,
+									Values:   []string{"test-value0", "test-value1"},
+								},
+							},
+						},
+					},
+				},
+			}),
+		},
 	}

 	for _, c := range cases {