Add VolumeNodeAffinity to PersistentVolumeSpec

pkg/apis/core/validation/validation_test.go

@@ -63,7 +63,7 @@ func testVolume(name string, namespace string, spec core.PersistentVolumeSpec) *
 	}
 }
 
-func testVolumeWithNodeAffinity(t *testing.T, name string, namespace string, affinity *core.NodeAffinity, spec core.PersistentVolumeSpec) *core.PersistentVolume {
+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
@@ -372,42 +372,6 @@ func TestValidatePersistentVolumes(t *testing.T) {
 				VolumeMode:       &validMode,
 			}),
 		},
-		// LocalVolume alpha feature disabled
-		// TODO: remove when no longer alpha
-		"alpha disabled valid local volume": {
-			isExpectedFailure: true,
-			volume: testVolumeWithNodeAffinity(
-				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"},
-									},
-								},
-							},
-						},
-					},
-				},
-				core.PersistentVolumeSpec{
-					Capacity: core.ResourceList{
-						core.ResourceName(core.ResourceStorage): resource.MustParse("10G"),
-					},
-					AccessModes: []core.PersistentVolumeAccessMode{core.ReadWriteOnce},
-					PersistentVolumeSource: core.PersistentVolumeSource{
-						Local: &core.LocalVolumeSource{
-							Path: "/foo",
-						},
-					},
-					StorageClassName: "test-storage-class",
-				}),
-		},
 		"bad-hostpath-volume-backsteps": {
 			isExpectedFailure: true,
 			volume: testVolume("foo", "", core.PersistentVolumeSpec{
@@ -424,20 +388,31 @@ func TestValidatePersistentVolumes(t *testing.T) {
 				StorageClassName: "backstep-hostpath",
 			}),
 		},
-		"bad-local-volume-backsteps": {
+		"volume-node-affinity": {
+			isExpectedFailure: false,
+			volume:            testVolumeWithNodeAffinity(simpleVolumeNodeAffinity("foo", "bar")),
+		},
+		"volume-empty-node-affinity": {
 			isExpectedFailure: true,
-			volume: testVolume("foo", "", core.PersistentVolumeSpec{
-				Capacity: core.ResourceList{
-					core.ResourceName(core.ResourceStorage): resource.MustParse("10G"),
-				},
-				AccessModes: []core.PersistentVolumeAccessMode{core.ReadWriteOnce},
-				PersistentVolumeSource: core.PersistentVolumeSource{
-					Local: &core.LocalVolumeSource{
-						Path: "/foo/..",
+			volume:            testVolumeWithNodeAffinity(&core.VolumeNodeAffinity{}),
+		},
+		"volume-bad-node-affinity": {
+			isExpectedFailure: true,
+			volume: testVolumeWithNodeAffinity(
+				&core.VolumeNodeAffinity{
+					Required: &core.NodeSelector{
+						NodeSelectorTerms: []core.NodeSelectorTerm{
+							{
+								MatchExpressions: []core.NodeSelectorRequirement{
+									{
+										Operator: core.NodeSelectorOpIn,
+										Values:   []string{"test-label-value"},
+									},
+								},
+							},
+						},
 					},
-				},
-				StorageClassName: "backstep-local",
-			}),
+				}),
 		},
 	}
 
@@ -514,14 +489,30 @@ func TestValidatePersistentVolumeSourceUpdate(t *testing.T) {
 	}
 }
 
+func testLocalVolume(path string, affinity *core.VolumeNodeAffinity) core.PersistentVolumeSpec {
+	return core.PersistentVolumeSpec{
+		Capacity: core.ResourceList{
+			core.ResourceName(core.ResourceStorage): resource.MustParse("10G"),
+		},
+		AccessModes: []core.PersistentVolumeAccessMode{core.ReadWriteOnce},
+		PersistentVolumeSource: core.PersistentVolumeSource{
+			Local: &core.LocalVolumeSource{
+				Path: path,
+			},
+		},
+		NodeAffinity:     affinity,
+		StorageClassName: "test-storage-class",
+	}
+}
+
 func TestValidateLocalVolumes(t *testing.T) {
 	scenarios := map[string]struct {
 		isExpectedFailure bool
 		volume            *core.PersistentVolume
 	}{
-		"valid local volume": {
+		"alpha valid local volume": {
 			isExpectedFailure: false,
-			volume: testVolumeWithNodeAffinity(
+			volume: testVolumeWithAlphaNodeAffinity(
 				t,
 				"valid-local-volume",
 				"",
@@ -540,60 +531,27 @@ func TestValidateLocalVolumes(t *testing.T) {
 						},
 					},
 				},
-				core.PersistentVolumeSpec{
-					Capacity: core.ResourceList{
-						core.ResourceName(core.ResourceStorage): resource.MustParse("10G"),
-					},
-					AccessModes: []core.PersistentVolumeAccessMode{core.ReadWriteOnce},
-					PersistentVolumeSource: core.PersistentVolumeSource{
-						Local: &core.LocalVolumeSource{
-							Path: "/foo",
-						},
-					},
-					StorageClassName: "test-storage-class",
-				}),
+				testLocalVolume("/foo", nil)),
 		},
-		"invalid local volume nil annotations": {
+		"alpha invalid local volume nil annotations": {
 			isExpectedFailure: true,
 			volume: testVolume(
 				"invalid-local-volume-nil-annotations",
 				"",
-				core.PersistentVolumeSpec{
-					Capacity: core.ResourceList{
-						core.ResourceName(core.ResourceStorage): resource.MustParse("10G"),
-					},
-					AccessModes: []core.PersistentVolumeAccessMode{core.ReadWriteOnce},
-					PersistentVolumeSource: core.PersistentVolumeSource{
-						Local: &core.LocalVolumeSource{
-							Path: "/foo",
-						},
-					},
-					StorageClassName: "test-storage-class",
-				}),
+				testLocalVolume("/foo", nil)),
 		},
-		"invalid local volume empty affinity": {
+		"alpha invalid local volume empty affinity": {
 			isExpectedFailure: true,
-			volume: testVolumeWithNodeAffinity(
+			volume: testVolumeWithAlphaNodeAffinity(
 				t,
 				"invalid-local-volume-empty-affinity",
 				"",
 				&core.NodeAffinity{},
-				core.PersistentVolumeSpec{
-					Capacity: core.ResourceList{
-						core.ResourceName(core.ResourceStorage): resource.MustParse("10G"),
-					},
-					AccessModes: []core.PersistentVolumeAccessMode{core.ReadWriteOnce},
-					PersistentVolumeSource: core.PersistentVolumeSource{
-						Local: &core.LocalVolumeSource{
-							Path: "/foo",
-						},
-					},
-					StorageClassName: "test-storage-class",
-				}),
+				testLocalVolume("/foo", nil)),
 		},
-		"invalid local volume preferred affinity": {
+		"alpha invalid local volume preferred affinity": {
 			isExpectedFailure: true,
-			volume: testVolumeWithNodeAffinity(
+			volume: testVolumeWithAlphaNodeAffinity(
 				t,
 				"invalid-local-volume-preferred-affinity",
 				"",
@@ -626,24 +584,56 @@ func TestValidateLocalVolumes(t *testing.T) {
 						},
 					},
 				},
-				core.PersistentVolumeSpec{
-					Capacity: core.ResourceList{
-						core.ResourceName(core.ResourceStorage): resource.MustParse("10G"),
-					},
-					AccessModes: []core.PersistentVolumeAccessMode{core.ReadWriteOnce},
-					PersistentVolumeSource: core.PersistentVolumeSource{
-						Local: &core.LocalVolumeSource{
-							Path: "/foo",
-						},
-					},
-					StorageClassName: "test-storage-class",
-				}),
+				testLocalVolume("/foo", nil)),
+		},
+		"valid local volume": {
+			isExpectedFailure: false,
+			volume: testVolume("valid-local-volume", "",
+				testLocalVolume("/foo", simpleVolumeNodeAffinity("foo", "bar"))),
+		},
+		"invalid local volume no node affinity": {
+			isExpectedFailure: true,
+			volume: testVolume("invalid-local-volume-no-node-affinity", "",
+				testLocalVolume("/foo", nil)),
 		},
 		"invalid local volume empty path": {
 			isExpectedFailure: true,
-			volume: testVolumeWithNodeAffinity(
+			volume: testVolume("invalid-local-volume-empty-path", "",
+				testLocalVolume("", simpleVolumeNodeAffinity("foo", "bar"))),
+		},
+		"invalid-local-volume-backsteps": {
+			isExpectedFailure: true,
+			volume: testVolume("foo", "",
+				testLocalVolume("/foo/..", simpleVolumeNodeAffinity("foo", "bar"))),
+		},
+		"invalid-local-volume-relative-path": {
+			isExpectedFailure: true,
+			volume: testVolume("foo", "",
+				testLocalVolume("foo", simpleVolumeNodeAffinity("foo", "bar"))),
+		},
+	}
+
+	for name, scenario := range scenarios {
+		errs := ValidatePersistentVolume(scenario.volume)
+		if len(errs) == 0 && scenario.isExpectedFailure {
+			t.Errorf("Unexpected success for scenario: %s", name)
+		}
+		if len(errs) > 0 && !scenario.isExpectedFailure {
+			t.Errorf("Unexpected failure for scenario: %s - %+v", name, errs)
+		}
+	}
+}
+
+func TestValidateLocalVolumesDisabled(t *testing.T) {
+	scenarios := map[string]struct {
+		isExpectedFailure bool
+		volume            *core.PersistentVolume
+	}{
+		"alpha disabled valid local volume": {
+			isExpectedFailure: true,
+			volume: testVolumeWithAlphaNodeAffinity(
 				t,
-				"invalid-local-volume-empty-path",
+				"valid-local-volume",
 				"",
 				&core.NodeAffinity{
 					RequiredDuringSchedulingIgnoredDuringExecution: &core.NodeSelector{
@@ -660,24 +650,16 @@ func TestValidateLocalVolumes(t *testing.T) {
 						},
 					},
 				},
-				core.PersistentVolumeSpec{
-					Capacity: core.ResourceList{
-						core.ResourceName(core.ResourceStorage): resource.MustParse("10G"),
-					},
-					AccessModes: []core.PersistentVolumeAccessMode{core.ReadWriteOnce},
-					PersistentVolumeSource: core.PersistentVolumeSource{
-						Local: &core.LocalVolumeSource{},
-					},
-					StorageClassName: "test-storage-class",
-				}),
+				testLocalVolume("/foo", nil)),
+		},
+		"feature disabled valid local volume": {
+			isExpectedFailure: true,
+			volume: testVolume("valid-local-volume", "",
+				testLocalVolume("/foo", simpleVolumeNodeAffinity("foo", "bar"))),
 		},
 	}
 
-	err := utilfeature.DefaultFeatureGate.Set("PersistentLocalVolumes=true")
-	if err != nil {
-		t.Errorf("Failed to enable feature gate for LocalPersistentVolumes: %v", err)
-		return
-	}
+	utilfeature.DefaultFeatureGate.Set("PersistentLocalVolumes=false")
 	for name, scenario := range scenarios {
 		errs := ValidatePersistentVolume(scenario.volume)
 		if len(errs) == 0 && scenario.isExpectedFailure {
@@ -687,6 +669,98 @@ func TestValidateLocalVolumes(t *testing.T) {
 			t.Errorf("Unexpected failure for scenario: %s - %+v", name, errs)
 		}
 	}
+	utilfeature.DefaultFeatureGate.Set("PersistentLocalVolumes=true")
+
+	utilfeature.DefaultFeatureGate.Set("VolumeScheduling=false")
+	for name, scenario := range scenarios {
+		errs := ValidatePersistentVolume(scenario.volume)
+		if len(errs) == 0 && scenario.isExpectedFailure {
+			t.Errorf("Unexpected success for scenario: %s", name)
+		}
+		if len(errs) > 0 && !scenario.isExpectedFailure {
+			t.Errorf("Unexpected failure for scenario: %s - %+v", name, errs)
+		}
+	}
+	utilfeature.DefaultFeatureGate.Set("VolumeScheduling=true")
+}
+
+func testVolumeWithNodeAffinity(affinity *core.VolumeNodeAffinity) *core.PersistentVolume {
+	return testVolume("test-affinity-volume", "",
+		core.PersistentVolumeSpec{
+			Capacity: core.ResourceList{
+				core.ResourceName(core.ResourceStorage): resource.MustParse("10G"),
+			},
+			AccessModes: []core.PersistentVolumeAccessMode{core.ReadWriteOnce},
+			PersistentVolumeSource: core.PersistentVolumeSource{
+				GCEPersistentDisk: &core.GCEPersistentDiskVolumeSource{
+					PDName: "foo",
+				},
+			},
+			StorageClassName: "test-storage-class",
+			NodeAffinity:     affinity,
+		})
+}
+
+func simpleVolumeNodeAffinity(key, value string) *core.VolumeNodeAffinity {
+	return &core.VolumeNodeAffinity{
+		Required: &core.NodeSelector{
+			NodeSelectorTerms: []core.NodeSelectorTerm{
+				{
+					MatchExpressions: []core.NodeSelectorRequirement{
+						{
+							Key:      key,
+							Operator: core.NodeSelectorOpIn,
+							Values:   []string{value},
+						},
+					},
+				},
+			},
+		},
+	}
+}
+
+func TestValidateVolumeNodeAffinityUpdate(t *testing.T) {
+	scenarios := map[string]struct {
+		isExpectedFailure bool
+		oldPV             *core.PersistentVolume
+		newPV             *core.PersistentVolume
+	}{
+		"nil-nothing-changed": {
+			isExpectedFailure: false,
+			oldPV:             testVolumeWithNodeAffinity(nil),
+			newPV:             testVolumeWithNodeAffinity(nil),
+		},
+		"affinity-nothing-changed": {
+			isExpectedFailure: false,
+			oldPV:             testVolumeWithNodeAffinity(simpleVolumeNodeAffinity("foo", "bar")),
+			newPV:             testVolumeWithNodeAffinity(simpleVolumeNodeAffinity("foo", "bar")),
+		},
+		"affinity-changed": {
+			isExpectedFailure: true,
+			oldPV:             testVolumeWithNodeAffinity(simpleVolumeNodeAffinity("foo", "bar")),
+			newPV:             testVolumeWithNodeAffinity(simpleVolumeNodeAffinity("foo", "bar2")),
+		},
+		"nil-to-obj": {
+			isExpectedFailure: true,
+			oldPV:             testVolumeWithNodeAffinity(nil),
+			newPV:             testVolumeWithNodeAffinity(simpleVolumeNodeAffinity("foo", "bar")),
+		},
+		"obj-to-nil": {
+			isExpectedFailure: true,
+			oldPV:             testVolumeWithNodeAffinity(simpleVolumeNodeAffinity("foo", "bar")),
+			newPV:             testVolumeWithNodeAffinity(nil),
+		},
+	}
+
+	for name, scenario := range scenarios {
+		errs := ValidatePersistentVolumeUpdate(scenario.newPV, scenario.oldPV)
+		if len(errs) == 0 && scenario.isExpectedFailure {
+			t.Errorf("Unexpected success for scenario: %s", name)
+		}
+		if len(errs) > 0 && !scenario.isExpectedFailure {
+			t.Errorf("Unexpected failure for scenario: %s - %+v", name, errs)
+		}
+	}
 }
 
 func testVolumeClaim(name string, namespace string, spec core.PersistentVolumeClaimSpec) *core.PersistentVolumeClaim {