diff --git a/pkg/apis/core/types.go b/pkg/apis/core/types.go index 5de3b9e24d5..574bd240c2a 100644 --- a/pkg/apis/core/types.go +++ b/pkg/apis/core/types.go @@ -467,6 +467,16 @@ type PersistentVolumeSpec struct { // This is an alpha feature and may change in the future. // +optional VolumeMode *PersistentVolumeMode + // NodeAffinity defines constraints that limit what nodes this volume can be accessed from. + // This field influences the scheduling of pods that use this volume. + // +optional + NodeAffinity *VolumeNodeAffinity +} + +// VolumeNodeAffinity defines constraints that limit what nodes this volume can be accessed from. +type VolumeNodeAffinity struct { + // Required specifies hard node constraints that must be met. + Required *NodeSelector } // PersistentVolumeReclaimPolicy describes a policy for end-of-life maintenance of persistent volumes diff --git a/pkg/apis/core/validation/validation.go b/pkg/apis/core/validation/validation.go index c94d08d93d1..0a4bd397538 100644 --- a/pkg/apis/core/validation/validation.go +++ b/pkg/apis/core/validation/validation.go @@ -1383,6 +1383,9 @@ func validateLocalVolumeSource(ls *core.LocalVolumeSource, fldPath *field.Path) return allErrs } + if !path.IsAbs(ls.Path) { + allErrs = append(allErrs, field.Invalid(fldPath, ls.Path, "must be an absolute path")) + } allErrs = append(allErrs, validatePathNoBacksteps(ls.Path, fldPath.Child("path"))...) return allErrs } @@ -1497,6 +1500,10 @@ func ValidatePersistentVolume(pv *core.PersistentVolume) field.ErrorList { nodeAffinitySpecified, errs := validateStorageNodeAffinityAnnotation(pv.ObjectMeta.Annotations, metaPath.Child("annotations")) allErrs = append(allErrs, errs...) + volumeNodeAffinitySpecified, errs := validateVolumeNodeAffinity(pv.Spec.NodeAffinity, specPath.Child("nodeAffinity")) + nodeAffinitySpecified = nodeAffinitySpecified || volumeNodeAffinitySpecified + allErrs = append(allErrs, errs...) + numVolumes := 0 if pv.Spec.HostPath != nil { if numVolumes > 0 { @@ -1725,6 +1732,10 @@ func ValidatePersistentVolumeUpdate(newPv, oldPv *core.PersistentVolume) field.E allErrs = append(allErrs, ValidateImmutableField(newPv.Spec.VolumeMode, oldPv.Spec.VolumeMode, field.NewPath("volumeMode"))...) } + if utilfeature.DefaultFeatureGate.Enabled(features.VolumeScheduling) { + allErrs = append(allErrs, ValidateImmutableField(newPv.Spec.NodeAffinity, oldPv.Spec.NodeAffinity, field.NewPath("nodeAffinity"))...) + } + return allErrs } @@ -4936,7 +4947,7 @@ func validateStorageNodeAffinityAnnotation(annotations map[string]string, fldPat return false, allErrs } - if !utilfeature.DefaultFeatureGate.Enabled(features.PersistentLocalVolumes) { + if !utilfeature.DefaultFeatureGate.Enabled(features.VolumeScheduling) { allErrs = append(allErrs, field.Forbidden(fldPath, "Storage node affinity is disabled by feature-gate")) } @@ -4952,6 +4963,27 @@ func validateStorageNodeAffinityAnnotation(annotations map[string]string, fldPat return policySpecified, allErrs } +// validateVolumeNodeAffinity tests that the PersistentVolume.NodeAffinity has valid data +func validateVolumeNodeAffinity(nodeAffinity *core.VolumeNodeAffinity, fldPath *field.Path) (bool, field.ErrorList) { + allErrs := field.ErrorList{} + + if nodeAffinity == nil { + return false, allErrs + } + + if !utilfeature.DefaultFeatureGate.Enabled(features.VolumeScheduling) { + allErrs = append(allErrs, field.Forbidden(fldPath, "Volume node affinity is disabled by feature-gate")) + } + + if nodeAffinity.Required != nil { + allErrs = append(allErrs, ValidateNodeSelector(nodeAffinity.Required, fldPath.Child("required"))...) + } else { + allErrs = append(allErrs, field.Required(fldPath.Child("required"), "must specify required node constraints")) + } + + return true, allErrs +} + // ValidateCIDR validates whether a CIDR matches the conventions expected by net.ParseCIDR func ValidateCIDR(cidr string) (*net.IPNet, error) { _, net, err := net.ParseCIDR(cidr) diff --git a/pkg/apis/core/validation/validation_test.go b/pkg/apis/core/validation/validation_test.go index 81a47404463..15f9d8a40d9 100644 --- a/pkg/apis/core/validation/validation_test.go +++ b/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 { diff --git a/pkg/apis/storage/fuzzer/fuzzer.go b/pkg/apis/storage/fuzzer/fuzzer.go index ea35b74a5ac..f559821f6a8 100644 --- a/pkg/apis/storage/fuzzer/fuzzer.go +++ b/pkg/apis/storage/fuzzer/fuzzer.go @@ -22,6 +22,7 @@ import ( runtimeserializer "k8s.io/apimachinery/pkg/runtime/serializer" api "k8s.io/kubernetes/pkg/apis/core" "k8s.io/kubernetes/pkg/apis/storage" + storageapi "k8s.io/kubernetes/pkg/apis/storage" ) // Funcs returns the fuzzer functions for the storage api group. @@ -31,6 +32,8 @@ var Funcs = func(codecs runtimeserializer.CodecFactory) []interface{} { c.FuzzNoCustom(obj) // fuzz self without calling this function again reclamationPolicies := []api.PersistentVolumeReclaimPolicy{api.PersistentVolumeReclaimDelete, api.PersistentVolumeReclaimRetain} obj.ReclaimPolicy = &reclamationPolicies[c.Rand.Intn(len(reclamationPolicies))] + bindingModes := []storageapi.VolumeBindingMode{storageapi.VolumeBindingImmediate, storageapi.VolumeBindingWaitForFirstConsumer} + obj.VolumeBindingMode = &bindingModes[c.Rand.Intn(len(bindingModes))] }, } } diff --git a/pkg/apis/storage/util/util_test.go b/pkg/apis/storage/util/util_test.go index d75b2455865..1a0ef70b0b5 100644 --- a/pkg/apis/storage/util/util_test.go +++ b/pkg/apis/storage/util/util_test.go @@ -27,6 +27,9 @@ func TestDropAlphaFields(t *testing.T) { bindingMode := storage.VolumeBindingWaitForFirstConsumer // Test that field gets dropped when feature gate is not set + if err := utilfeature.DefaultFeatureGate.Set("VolumeScheduling=false"); err != nil { + t.Fatalf("Failed to set feature gate for VolumeScheduling: %v", err) + } class := &storage.StorageClass{ VolumeBindingMode: &bindingMode, } diff --git a/pkg/apis/storage/v1/defaults_test.go b/pkg/apis/storage/v1/defaults_test.go index 4fb6304f773..4143ad75e2a 100644 --- a/pkg/apis/storage/v1/defaults_test.go +++ b/pkg/apis/storage/v1/defaults_test.go @@ -52,6 +52,10 @@ func TestSetDefaultVolumeBindingMode(t *testing.T) { class := &storagev1.StorageClass{} // When feature gate is disabled, field should not be defaulted + err := utilfeature.DefaultFeatureGate.Set("VolumeScheduling=false") + if err != nil { + t.Fatalf("Failed to enable feature gate for VolumeScheduling: %v", err) + } output := roundTrip(t, runtime.Object(class)).(*storagev1.StorageClass) if output.VolumeBindingMode != nil { t.Errorf("Expected VolumeBindingMode to not be defaulted, got: %+v", output.VolumeBindingMode) @@ -59,12 +63,11 @@ func TestSetDefaultVolumeBindingMode(t *testing.T) { class = &storagev1.StorageClass{} - err := utilfeature.DefaultFeatureGate.Set("VolumeScheduling=true") + // When feature gate is enabled, field should be defaulted + err = utilfeature.DefaultFeatureGate.Set("VolumeScheduling=true") if err != nil { t.Fatalf("Failed to enable feature gate for VolumeScheduling: %v", err) } - - // When feature gate is enabled, field should be defaulted defaultMode := storagev1.VolumeBindingImmediate output = roundTrip(t, runtime.Object(class)).(*storagev1.StorageClass) outMode := output.VolumeBindingMode diff --git a/pkg/apis/storage/v1beta1/defaults_test.go b/pkg/apis/storage/v1beta1/defaults_test.go index 34aff74b8e0..e846c7d5733 100644 --- a/pkg/apis/storage/v1beta1/defaults_test.go +++ b/pkg/apis/storage/v1beta1/defaults_test.go @@ -52,6 +52,10 @@ func TestSetDefaultVolumeBindingMode(t *testing.T) { class := &storagev1beta1.StorageClass{} // When feature gate is disabled, field should not be defaulted + err := utilfeature.DefaultFeatureGate.Set("VolumeScheduling=false") + if err != nil { + t.Fatalf("Failed to enable feature gate for VolumeScheduling: %v", err) + } output := roundTrip(t, runtime.Object(class)).(*storagev1beta1.StorageClass) if output.VolumeBindingMode != nil { t.Errorf("Expected VolumeBindingMode to not be defaulted, got: %+v", output.VolumeBindingMode) @@ -59,12 +63,11 @@ func TestSetDefaultVolumeBindingMode(t *testing.T) { class = &storagev1beta1.StorageClass{} - err := utilfeature.DefaultFeatureGate.Set("VolumeScheduling=true") + // When feature gate is enabled, field should be defaulted + err = utilfeature.DefaultFeatureGate.Set("VolumeScheduling=true") if err != nil { t.Fatalf("Failed to enable feature gate for VolumeScheduling: %v", err) } - - // When feature gate is enabled, field should be defaulted defaultMode := storagev1beta1.VolumeBindingImmediate output = roundTrip(t, runtime.Object(class)).(*storagev1beta1.StorageClass) outMode := output.VolumeBindingMode diff --git a/pkg/apis/storage/validation/validation_test.go b/pkg/apis/storage/validation/validation_test.go index 2cb0a61a414..7e6f79687ac 100644 --- a/pkg/apis/storage/validation/validation_test.go +++ b/pkg/apis/storage/validation/validation_test.go @@ -42,16 +42,18 @@ func TestValidateStorageClass(t *testing.T) { successCases := []storage.StorageClass{ { // empty parameters - ObjectMeta: metav1.ObjectMeta{Name: "foo"}, - Provisioner: "kubernetes.io/foo-provisioner", - Parameters: map[string]string{}, - ReclaimPolicy: &deleteReclaimPolicy, + ObjectMeta: metav1.ObjectMeta{Name: "foo"}, + Provisioner: "kubernetes.io/foo-provisioner", + Parameters: map[string]string{}, + ReclaimPolicy: &deleteReclaimPolicy, + VolumeBindingMode: &immediateMode1, }, { // nil parameters - ObjectMeta: metav1.ObjectMeta{Name: "foo"}, - Provisioner: "kubernetes.io/foo-provisioner", - ReclaimPolicy: &deleteReclaimPolicy, + ObjectMeta: metav1.ObjectMeta{Name: "foo"}, + Provisioner: "kubernetes.io/foo-provisioner", + ReclaimPolicy: &deleteReclaimPolicy, + VolumeBindingMode: &immediateMode1, }, { // some parameters @@ -62,13 +64,15 @@ func TestValidateStorageClass(t *testing.T) { "foo-parameter": "free-form-string", "foo-parameter2": "{\"embedded\": \"json\", \"with\": {\"structures\":\"inside\"}}", }, - ReclaimPolicy: &deleteReclaimPolicy, + ReclaimPolicy: &deleteReclaimPolicy, + VolumeBindingMode: &immediateMode1, }, { // retain reclaimPolicy - ObjectMeta: metav1.ObjectMeta{Name: "foo"}, - Provisioner: "kubernetes.io/foo-provisioner", - ReclaimPolicy: &retainReclaimPolicy, + ObjectMeta: metav1.ObjectMeta{Name: "foo"}, + Provisioner: "kubernetes.io/foo-provisioner", + ReclaimPolicy: &retainReclaimPolicy, + VolumeBindingMode: &immediateMode1, }, } @@ -144,6 +148,7 @@ func TestAlphaExpandPersistentVolumesFeatureValidation(t *testing.T) { Parameters: map[string]string{}, ReclaimPolicy: &deleteReclaimPolicy, AllowVolumeExpansion: &falseVar, + VolumeBindingMode: &immediateMode1, } // Enable alpha feature ExpandPersistentVolumes @@ -462,6 +467,10 @@ func TestValidateVolumeBindingModeAlphaDisabled(t *testing.T) { "invalid mode": makeClassWithBinding(&invalidMode), } + err := utilfeature.DefaultFeatureGate.Set("VolumeScheduling=false") + if err != nil { + t.Fatalf("Failed to enable feature gate for VolumeScheduling: %v", err) + } for testName, storageClass := range errorCases { if errs := ValidateStorageClass(storageClass); len(errs) == 0 { t.Errorf("Expected failure for test: %v", testName) diff --git a/pkg/controller/volume/persistentvolume/BUILD b/pkg/controller/volume/persistentvolume/BUILD index 733472f951f..92a76c8ac52 100644 --- a/pkg/controller/volume/persistentvolume/BUILD +++ b/pkg/controller/volume/persistentvolume/BUILD @@ -79,7 +79,6 @@ go_test( deps = [ "//pkg/api/testapi:go_default_library", "//pkg/apis/core:go_default_library", - "//pkg/apis/core/v1/helper:go_default_library", "//pkg/controller:go_default_library", "//pkg/volume:go_default_library", "//vendor/github.com/golang/glog:go_default_library", diff --git a/pkg/controller/volume/persistentvolume/index_test.go b/pkg/controller/volume/persistentvolume/index_test.go index 9c6ba25a2c6..ba9360ef843 100644 --- a/pkg/controller/volume/persistentvolume/index_test.go +++ b/pkg/controller/volume/persistentvolume/index_test.go @@ -20,8 +20,6 @@ import ( "sort" "testing" - "github.com/golang/glog" - "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -29,7 +27,6 @@ import ( "k8s.io/client-go/kubernetes/scheme" ref "k8s.io/client-go/tools/reference" "k8s.io/kubernetes/pkg/api/testapi" - "k8s.io/kubernetes/pkg/apis/core/v1/helper" "k8s.io/kubernetes/pkg/volume" ) @@ -680,9 +677,8 @@ func createTestVolumes() []*v1.PersistentVolume { }, { ObjectMeta: metav1.ObjectMeta{ - UID: "affinity-pv", - Name: "affinity001", - Annotations: getAnnotationWithNodeAffinity("key1", "value1"), + UID: "affinity-pv", + Name: "affinity001", }, Spec: v1.PersistentVolumeSpec{ Capacity: v1.ResourceList{ @@ -696,13 +692,13 @@ func createTestVolumes() []*v1.PersistentVolume { v1.ReadOnlyMany, }, StorageClassName: classWait, + NodeAffinity: getVolumeNodeAffinity("key1", "value1"), }, }, { ObjectMeta: metav1.ObjectMeta{ - UID: "affinity-pv2", - Name: "affinity002", - Annotations: getAnnotationWithNodeAffinity("key1", "value1"), + UID: "affinity-pv2", + Name: "affinity002", }, Spec: v1.PersistentVolumeSpec{ Capacity: v1.ResourceList{ @@ -716,13 +712,13 @@ func createTestVolumes() []*v1.PersistentVolume { v1.ReadOnlyMany, }, StorageClassName: classWait, + NodeAffinity: getVolumeNodeAffinity("key1", "value1"), }, }, { ObjectMeta: metav1.ObjectMeta{ - UID: "affinity-prebound", - Name: "affinity003", - Annotations: getAnnotationWithNodeAffinity("key1", "value1"), + UID: "affinity-prebound", + Name: "affinity003", }, Spec: v1.PersistentVolumeSpec{ Capacity: v1.ResourceList{ @@ -737,13 +733,13 @@ func createTestVolumes() []*v1.PersistentVolume { }, StorageClassName: classWait, ClaimRef: &v1.ObjectReference{Name: "claim02", Namespace: "myns"}, + NodeAffinity: getVolumeNodeAffinity("key1", "value1"), }, }, { ObjectMeta: metav1.ObjectMeta{ - UID: "affinity-pv3", - Name: "affinity003", - Annotations: getAnnotationWithNodeAffinity("key1", "value3"), + UID: "affinity-pv3", + Name: "affinity003", }, Spec: v1.PersistentVolumeSpec{ Capacity: v1.ResourceList{ @@ -757,6 +753,7 @@ func createTestVolumes() []*v1.PersistentVolume { v1.ReadOnlyMany, }, StorageClassName: classWait, + NodeAffinity: getVolumeNodeAffinity("key1", "value3"), }, }, } @@ -776,9 +773,9 @@ func testVolume(name, size string) *v1.PersistentVolume { } } -func getAnnotationWithNodeAffinity(key string, value string) map[string]string { - affinity := &v1.NodeAffinity{ - RequiredDuringSchedulingIgnoredDuringExecution: &v1.NodeSelector{ +func getVolumeNodeAffinity(key string, value string) *v1.VolumeNodeAffinity { + return &v1.VolumeNodeAffinity{ + Required: &v1.NodeSelector{ NodeSelectorTerms: []v1.NodeSelectorTerm{ { MatchExpressions: []v1.NodeSelectorRequirement{ @@ -792,14 +789,6 @@ func getAnnotationWithNodeAffinity(key string, value string) map[string]string { }, }, } - - annotations := map[string]string{} - err := helper.StorageNodeAffinityToAlphaAnnotation(annotations, affinity) - if err != nil { - glog.Fatalf("Failed to get node affinity annotation: %v", err) - } - - return annotations } func createVolumeModeBlockTestVolume() *v1.PersistentVolume { diff --git a/pkg/controller/volume/persistentvolume/scheduler_binder_test.go b/pkg/controller/volume/persistentvolume/scheduler_binder_test.go index c5f33f0409d..138abe2f25e 100644 --- a/pkg/controller/volume/persistentvolume/scheduler_binder_test.go +++ b/pkg/controller/volume/persistentvolume/scheduler_binder_test.go @@ -331,7 +331,7 @@ func makeTestPV(name, node, capacity, version string, boundToPVC *v1.PersistentV }, } if node != "" { - pv.Annotations = getAnnotationWithNodeAffinity("key1", node) + pv.Spec.NodeAffinity = getVolumeNodeAffinity("key1", node) } if boundToPVC != nil { diff --git a/pkg/features/kube_features.go b/pkg/features/kube_features.go index 82d65f1b99e..1556e2f06a9 100644 --- a/pkg/features/kube_features.go +++ b/pkg/features/kube_features.go @@ -263,7 +263,7 @@ var defaultKubernetesFeatureGates = map[utilfeature.Feature]utilfeature.FeatureS TaintBasedEvictions: {Default: false, PreRelease: utilfeature.Alpha}, RotateKubeletServerCertificate: {Default: false, PreRelease: utilfeature.Alpha}, RotateKubeletClientCertificate: {Default: true, PreRelease: utilfeature.Beta}, - PersistentLocalVolumes: {Default: false, PreRelease: utilfeature.Alpha}, + PersistentLocalVolumes: {Default: true, PreRelease: utilfeature.Beta}, LocalStorageCapacityIsolation: {Default: false, PreRelease: utilfeature.Alpha}, HugePages: {Default: true, PreRelease: utilfeature.Beta}, DebugContainers: {Default: false, PreRelease: utilfeature.Alpha}, @@ -276,7 +276,7 @@ var defaultKubernetesFeatureGates = map[utilfeature.Feature]utilfeature.FeatureS CPUManager: {Default: true, PreRelease: utilfeature.Beta}, ServiceNodeExclusion: {Default: false, PreRelease: utilfeature.Alpha}, MountContainers: {Default: false, PreRelease: utilfeature.Alpha}, - VolumeScheduling: {Default: false, PreRelease: utilfeature.Alpha}, + VolumeScheduling: {Default: true, PreRelease: utilfeature.Beta}, CSIPersistentVolume: {Default: true, PreRelease: utilfeature.Beta}, CustomPodDNS: {Default: false, PreRelease: utilfeature.Alpha}, BlockVolume: {Default: false, PreRelease: utilfeature.Alpha}, diff --git a/pkg/registry/storage/storageclass/storage/storage_test.go b/pkg/registry/storage/storageclass/storage/storage_test.go index aaca94134f1..3de3249f70a 100644 --- a/pkg/registry/storage/storageclass/storage/storage_test.go +++ b/pkg/registry/storage/storageclass/storage/storage_test.go @@ -45,6 +45,7 @@ func newStorage(t *testing.T) (*REST, *etcdtesting.EtcdTestServer) { func validNewStorageClass(name string) *storageapi.StorageClass { deleteReclaimPolicy := api.PersistentVolumeReclaimDelete + bindingMode := storageapi.VolumeBindingImmediate return &storageapi.StorageClass{ ObjectMeta: metav1.ObjectMeta{ Name: name, @@ -53,7 +54,8 @@ func validNewStorageClass(name string) *storageapi.StorageClass { Parameters: map[string]string{ "foo": "bar", }, - ReclaimPolicy: &deleteReclaimPolicy, + ReclaimPolicy: &deleteReclaimPolicy, + VolumeBindingMode: &bindingMode, } } diff --git a/pkg/scheduler/scheduler_test.go b/pkg/scheduler/scheduler_test.go index 7b679aa04db..440c9f76ae7 100644 --- a/pkg/scheduler/scheduler_test.go +++ b/pkg/scheduler/scheduler_test.go @@ -207,7 +207,8 @@ func TestScheduler(t *testing.T) { NextPod: func() *v1.Pod { return item.sendPod }, - Recorder: eventBroadcaster.NewRecorder(legacyscheme.Scheme, v1.EventSource{Component: "scheduler"}), + Recorder: eventBroadcaster.NewRecorder(legacyscheme.Scheme, v1.EventSource{Component: "scheduler"}), + VolumeBinder: volumebinder.NewFakeVolumeBinder(&persistentvolume.FakeVolumeBinderConfig{AllBound: true}), }, } @@ -555,6 +556,7 @@ func setupTestScheduler(queuedPodStore *clientcache.FIFO, scache schedulercache. Recorder: &record.FakeRecorder{}, PodConditionUpdater: fakePodConditionUpdater{}, PodPreemptor: fakePodPreemptor{}, + VolumeBinder: volumebinder.NewFakeVolumeBinder(&persistentvolume.FakeVolumeBinderConfig{AllBound: true}), }, } @@ -604,6 +606,7 @@ func setupTestSchedulerLongBindingWithRetry(queuedPodStore *clientcache.FIFO, sc PodConditionUpdater: fakePodConditionUpdater{}, PodPreemptor: fakePodPreemptor{}, StopEverything: stop, + VolumeBinder: volumebinder.NewFakeVolumeBinder(&persistentvolume.FakeVolumeBinderConfig{AllBound: true}), }, } diff --git a/staging/src/k8s.io/api/core/v1/types.go b/staging/src/k8s.io/api/core/v1/types.go index 7e4ce23eba5..314816f1712 100644 --- a/staging/src/k8s.io/api/core/v1/types.go +++ b/staging/src/k8s.io/api/core/v1/types.go @@ -530,6 +530,16 @@ type PersistentVolumeSpec struct { // This is an alpha feature and may change in the future. // +optional VolumeMode *PersistentVolumeMode `json:"volumeMode,omitempty" protobuf:"bytes,8,opt,name=volumeMode,casttype=PersistentVolumeMode"` + // NodeAffinity defines constraints that limit what nodes this volume can be accessed from. + // This field influences the scheduling of pods that use this volume. + // +optional + NodeAffinity *VolumeNodeAffinity `json:"nodeAffinity,omitempty" protobuf:"bytes,9,opt,name=nodeAffinity"` +} + +// VolumeNodeAffinity defines constraints that limit what nodes this volume can be accessed from. +type VolumeNodeAffinity struct { + // Required specifies hard node constraints that must be met. + Required *NodeSelector `json:"required,omitempty" protobuf:"bytes,1,opt,name=required"` } // PersistentVolumeReclaimPolicy describes a policy for end-of-life maintenance of persistent volumes. diff --git a/test/e2e/framework/BUILD b/test/e2e/framework/BUILD index 9a88fe9a52c..3b69a9ccb4b 100644 --- a/test/e2e/framework/BUILD +++ b/test/e2e/framework/BUILD @@ -50,7 +50,6 @@ go_library( "//pkg/apis/apps:go_default_library", "//pkg/apis/batch:go_default_library", "//pkg/apis/core:go_default_library", - "//pkg/apis/core/v1/helper:go_default_library", "//pkg/apis/extensions:go_default_library", "//pkg/client/clientset_generated/internalclientset:go_default_library", "//pkg/client/conditions:go_default_library", diff --git a/test/e2e/framework/pv_util.go b/test/e2e/framework/pv_util.go index fb5a194ccc9..9c4979a7d4e 100644 --- a/test/e2e/framework/pv_util.go +++ b/test/e2e/framework/pv_util.go @@ -36,7 +36,6 @@ import ( "k8s.io/apimachinery/pkg/util/uuid" clientset "k8s.io/client-go/kubernetes" "k8s.io/kubernetes/pkg/api/testapi" - "k8s.io/kubernetes/pkg/apis/core/v1/helper" awscloud "k8s.io/kubernetes/pkg/cloudprovider/providers/aws" gcecloud "k8s.io/kubernetes/pkg/cloudprovider/providers/gce" "k8s.io/kubernetes/pkg/volume/util/volumehelper" @@ -81,7 +80,7 @@ type PersistentVolumeConfig struct { NamePrefix string Labels labels.Set StorageClassName string - NodeAffinity *v1.NodeAffinity + NodeAffinity *v1.VolumeNodeAffinity } // PersistentVolumeClaimConfig is consumed by MakePersistentVolumeClaim() to generate a PVC object. @@ -603,13 +602,9 @@ func MakePersistentVolume(pvConfig PersistentVolumeConfig) *v1.PersistentVolume }, ClaimRef: claimRef, StorageClassName: pvConfig.StorageClassName, + NodeAffinity: pvConfig.NodeAffinity, }, } - err := helper.StorageNodeAffinityToAlphaAnnotation(pv.Annotations, pvConfig.NodeAffinity) - if err != nil { - Logf("Setting storage node affinity failed: %v", err) - return nil - } return pv } diff --git a/test/e2e/storage/persistent_volumes-local.go b/test/e2e/storage/persistent_volumes-local.go index ed5107c4f05..da75a0552f1 100644 --- a/test/e2e/storage/persistent_volumes-local.go +++ b/test/e2e/storage/persistent_volumes-local.go @@ -141,7 +141,7 @@ var ( Level: "s0:c0,c1"} ) -var _ = utils.SIGDescribe("PersistentVolumes-local [Feature:LocalPersistentVolumes]", func() { +var _ = utils.SIGDescribe("PersistentVolumes-local ", func() { f := framework.NewDefaultFramework("persistent-local-volumes-test") var ( @@ -680,8 +680,8 @@ func makeLocalPVConfig(config *localTestConfig, volume *localTestVolume) framewo }, NamePrefix: "local-pv", StorageClassName: config.scName, - NodeAffinity: &v1.NodeAffinity{ - RequiredDuringSchedulingIgnoredDuringExecution: &v1.NodeSelector{ + NodeAffinity: &v1.VolumeNodeAffinity{ + Required: &v1.NodeSelector{ NodeSelectorTerms: []v1.NodeSelectorTerm{ { MatchExpressions: []v1.NodeSelectorRequirement{ diff --git a/test/integration/scheduler/BUILD b/test/integration/scheduler/BUILD index 377a66d33fa..0ef5421c3da 100644 --- a/test/integration/scheduler/BUILD +++ b/test/integration/scheduler/BUILD @@ -28,7 +28,6 @@ go_test( "//pkg/api/legacyscheme:go_default_library", "//pkg/api/testapi:go_default_library", "//pkg/apis/componentconfig:go_default_library", - "//pkg/apis/core/v1/helper:go_default_library", "//pkg/client/clientset_generated/internalclientset:go_default_library", "//pkg/client/informers/informers_generated/internalversion:go_default_library", "//pkg/controller/nodelifecycle:go_default_library", diff --git a/test/integration/scheduler/local-pv-neg-affinity_test.go b/test/integration/scheduler/local-pv-neg-affinity_test.go index fbe645eb577..bfbe2797882 100644 --- a/test/integration/scheduler/local-pv-neg-affinity_test.go +++ b/test/integration/scheduler/local-pv-neg-affinity_test.go @@ -39,7 +39,6 @@ import ( "k8s.io/client-go/tools/record" "k8s.io/kubernetes/pkg/api/legacyscheme" "k8s.io/kubernetes/pkg/api/testapi" - "k8s.io/kubernetes/pkg/apis/core/v1/helper" "k8s.io/kubernetes/pkg/controller/volume/persistentvolume" "k8s.io/kubernetes/pkg/scheduler" "k8s.io/kubernetes/pkg/scheduler/factory" @@ -253,31 +252,26 @@ func makeHostBoundPV(t *testing.T, name, scName, pvcName, ns string, node string Path: "/tmp/" + node + "/test-path", }, }, - }, - } - - if pvcName != "" { - pv.Spec.ClaimRef = &v1.ObjectReference{Name: pvcName, Namespace: ns} - } - - testNodeAffinity := &v1.NodeAffinity{ - RequiredDuringSchedulingIgnoredDuringExecution: &v1.NodeSelector{ - NodeSelectorTerms: []v1.NodeSelectorTerm{ - { - MatchExpressions: []v1.NodeSelectorRequirement{ + NodeAffinity: &v1.VolumeNodeAffinity{ + Required: &v1.NodeSelector{ + NodeSelectorTerms: []v1.NodeSelectorTerm{ { - Key: affinityLabelKey, - Operator: v1.NodeSelectorOpIn, - Values: []string{node}, + MatchExpressions: []v1.NodeSelectorRequirement{ + { + Key: affinityLabelKey, + Operator: v1.NodeSelectorOpIn, + Values: []string{node}, + }, + }, }, }, }, }, }, } - err := helper.StorageNodeAffinityToAlphaAnnotation(pv.Annotations, testNodeAffinity) - if err != nil { - t.Fatalf("Setting storage node affinity failed: %v", err) + + if pvcName != "" { + pv.Spec.ClaimRef = &v1.ObjectReference{Name: pvcName, Namespace: ns} } return pv diff --git a/test/integration/scheduler/volume_binding_test.go b/test/integration/scheduler/volume_binding_test.go index de733fd2486..ce76645a32a 100644 --- a/test/integration/scheduler/volume_binding_test.go +++ b/test/integration/scheduler/volume_binding_test.go @@ -29,7 +29,6 @@ import ( "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" clientset "k8s.io/client-go/kubernetes" - "k8s.io/kubernetes/pkg/apis/core/v1/helper" ) type testConfig struct { @@ -252,6 +251,21 @@ func makePV(t *testing.T, name, scName, pvcName, ns string) *v1.PersistentVolume Path: "/test-path", }, }, + NodeAffinity: &v1.VolumeNodeAffinity{ + Required: &v1.NodeSelector{ + NodeSelectorTerms: []v1.NodeSelectorTerm{ + { + MatchExpressions: []v1.NodeSelectorRequirement{ + { + Key: labelKey, + Operator: v1.NodeSelectorOpIn, + Values: []string{labelValue}, + }, + }, + }, + }, + }, + }, }, } @@ -259,25 +273,6 @@ func makePV(t *testing.T, name, scName, pvcName, ns string) *v1.PersistentVolume pv.Spec.ClaimRef = &v1.ObjectReference{Name: pvcName, Namespace: ns} } - testNodeAffinity := &v1.NodeAffinity{ - RequiredDuringSchedulingIgnoredDuringExecution: &v1.NodeSelector{ - NodeSelectorTerms: []v1.NodeSelectorTerm{ - { - MatchExpressions: []v1.NodeSelectorRequirement{ - { - Key: labelKey, - Operator: v1.NodeSelectorOpIn, - Values: []string{labelValue}, - }, - }, - }, - }, - }, - } - err := helper.StorageNodeAffinityToAlphaAnnotation(pv.Annotations, testNodeAffinity) - if err != nil { - t.Fatalf("Setting storage node affinity failed: %v", err) - } return pv }