diff --git a/pkg/controller/volume/persistentvolume/BUILD b/pkg/controller/volume/persistentvolume/BUILD index a9899d788b6..6fef2bf3839 100644 --- a/pkg/controller/volume/persistentvolume/BUILD +++ b/pkg/controller/volume/persistentvolume/BUILD @@ -20,6 +20,7 @@ go_library( "//pkg/cloudprovider:go_default_library", "//pkg/controller:go_default_library", "//pkg/controller/volume/events:go_default_library", + "//pkg/features:go_default_library", "//pkg/util/goroutinemap:go_default_library", "//pkg/util/goroutinemap/exponentialbackoff:go_default_library", "//pkg/util/io:go_default_library", @@ -37,6 +38,7 @@ go_library( "//vendor/k8s.io/apimachinery/pkg/types:go_default_library", "//vendor/k8s.io/apimachinery/pkg/util/runtime:go_default_library", "//vendor/k8s.io/apimachinery/pkg/util/wait:go_default_library", + "//vendor/k8s.io/apiserver/pkg/util/feature:go_default_library", "//vendor/k8s.io/client-go/informers/core/v1:go_default_library", "//vendor/k8s.io/client-go/informers/storage/v1:go_default_library", "//vendor/k8s.io/client-go/kubernetes:go_default_library", @@ -80,6 +82,7 @@ go_test( "//vendor/k8s.io/apimachinery/pkg/util/diff:go_default_library", "//vendor/k8s.io/apimachinery/pkg/util/wait:go_default_library", "//vendor/k8s.io/apimachinery/pkg/watch:go_default_library", + "//vendor/k8s.io/apiserver/pkg/util/feature:go_default_library", "//vendor/k8s.io/client-go/informers:go_default_library", "//vendor/k8s.io/client-go/kubernetes:go_default_library", "//vendor/k8s.io/client-go/kubernetes/fake:go_default_library", diff --git a/pkg/controller/volume/persistentvolume/binder_test.go b/pkg/controller/volume/persistentvolume/binder_test.go index 6f000e9edde..c500277d9a4 100644 --- a/pkg/controller/volume/persistentvolume/binder_test.go +++ b/pkg/controller/volume/persistentvolume/binder_test.go @@ -21,6 +21,7 @@ import ( "k8s.io/api/core/v1" storage "k8s.io/api/storage/v1" + utilfeature "k8s.io/apiserver/pkg/util/feature" ) // Test single call to syncClaim and syncVolume methods. @@ -33,6 +34,8 @@ func TestSync(t *testing.T) { "foo": "true", "bar": "false", } + modeBlock := v1.PersistentVolumeBlock + modeFile := v1.PersistentVolumeFilesystem tests := []controllerTest{ // [Unit test set 1] User did not care which PV they get. @@ -517,10 +520,216 @@ func TestSync(t *testing.T) { newClaimArray("claim13-5", "uid13-5", "1Gi", "volume13-5", v1.ClaimBound, nil, annBoundByController, annBindCompleted), noevents, noerrors, testSyncClaim, }, + + // All of these should bind as feature set is not enabled for BlockVolume + // meaning volumeMode will be ignored and dropped + { + // syncVolume binds a requested block claim to a block volume + "14-1 - binding to volumeMode block", + withVolumeVolumeMode(&modeBlock, newVolumeArray("volume14-1", "10Gi", "", "", v1.VolumePending, v1.PersistentVolumeReclaimRetain, classEmpty)), + withVolumeVolumeMode(&modeBlock, newVolumeArray("volume14-1", "10Gi", "uid14-1", "claim14-1", v1.VolumeBound, v1.PersistentVolumeReclaimRetain, classEmpty, annBoundByController)), + withClaimVolumeMode(&modeBlock, newClaimArray("claim14-1", "uid14-1", "10Gi", "", v1.ClaimPending, nil)), + withClaimVolumeMode(&modeBlock, newClaimArray("claim14-1", "uid14-1", "10Gi", "volume14-1", v1.ClaimBound, nil, annBoundByController, annBindCompleted)), + noevents, noerrors, testSyncClaim, + }, + { + // syncVolume binds a requested filesystem claim to a filesystem volume + "14-2 - binding to volumeMode filesystem", + withVolumeVolumeMode(&modeFile, newVolumeArray("volume14-2", "10Gi", "", "", v1.VolumePending, v1.PersistentVolumeReclaimRetain, classEmpty)), + withVolumeVolumeMode(&modeFile, newVolumeArray("volume14-2", "10Gi", "uid14-2", "claim14-2", v1.VolumeBound, v1.PersistentVolumeReclaimRetain, classEmpty, annBoundByController)), + withClaimVolumeMode(&modeFile, newClaimArray("claim14-2", "uid14-2", "10Gi", "", v1.ClaimPending, nil)), + withClaimVolumeMode(&modeFile, newClaimArray("claim14-2", "uid14-2", "10Gi", "volume14-2", v1.ClaimBound, nil, annBoundByController, annBindCompleted)), + noevents, noerrors, testSyncClaim, + }, + { + // syncVolume binds an unspecified volumemode for claim to a specified filesystem volume + "14-3 - binding to volumeMode filesystem using default for claim", + withVolumeVolumeMode(&modeFile, newVolumeArray("volume14-3", "10Gi", "", "", v1.VolumePending, v1.PersistentVolumeReclaimRetain, classEmpty)), + withVolumeVolumeMode(&modeFile, newVolumeArray("volume14-3", "10Gi", "uid14-3", "claim14-3", v1.VolumeBound, v1.PersistentVolumeReclaimRetain, classEmpty, annBoundByController)), + withClaimVolumeMode(nil, newClaimArray("claim14-3", "uid14-3", "10Gi", "", v1.ClaimPending, nil)), + withClaimVolumeMode(nil, newClaimArray("claim14-3", "uid14-3", "10Gi", "volume14-3", v1.ClaimBound, nil, annBoundByController, annBindCompleted)), + noevents, noerrors, testSyncClaim, + }, + { + // syncVolume binds a requested filesystem claim to an unspecified volumeMode for volume + "14-4 - binding to unspecified volumeMode using requested filesystem for claim", + withVolumeVolumeMode(nil, newVolumeArray("volume14-4", "10Gi", "", "", v1.VolumePending, v1.PersistentVolumeReclaimRetain, classEmpty)), + withVolumeVolumeMode(nil, newVolumeArray("volume14-4", "10Gi", "uid14-4", "claim14-4", v1.VolumeBound, v1.PersistentVolumeReclaimRetain, classEmpty, annBoundByController)), + withClaimVolumeMode(&modeFile, newClaimArray("claim14-4", "uid14-4", "10Gi", "", v1.ClaimPending, nil)), + withClaimVolumeMode(&modeFile, newClaimArray("claim14-4", "uid14-4", "10Gi", "volume14-4", v1.ClaimBound, nil, annBoundByController, annBindCompleted)), + noevents, noerrors, testSyncClaim, + }, + { + // syncVolume binds a requested filesystem claim to an unspecified volumeMode for volume + "14-5 - binding different volumeModes should be ignored", + withVolumeVolumeMode(&modeBlock, newVolumeArray("volume14-5", "10Gi", "", "", v1.VolumePending, v1.PersistentVolumeReclaimRetain, classEmpty)), + withVolumeVolumeMode(&modeBlock, newVolumeArray("volume14-5", "10Gi", "uid14-5", "claim14-5", v1.VolumeBound, v1.PersistentVolumeReclaimRetain, classEmpty, annBoundByController)), + withClaimVolumeMode(&modeFile, newClaimArray("claim14-5", "uid14-5", "10Gi", "", v1.ClaimPending, nil)), + withClaimVolumeMode(&modeFile, newClaimArray("claim14-5", "uid14-5", "10Gi", "volume14-5", v1.ClaimBound, nil, annBoundByController, annBindCompleted)), + noevents, noerrors, testSyncClaim, + }, } + runSyncTests(t, tests, []*storage.StorageClass{}) } +func TestSyncAlphaBlockVolume(t *testing.T) { + modeBlock := v1.PersistentVolumeBlock + modeFile := v1.PersistentVolumeFilesystem + + // Tests assume defaulting, so feature enabled will never have nil volumeMode + tests := []controllerTest{ + // PVC with VolumeMode + { + // syncVolume binds a requested block claim to a block volume + "14-1 - binding to volumeMode block", + withVolumeVolumeMode(&modeBlock, newVolumeArray("volume14-1", "10Gi", "", "", v1.VolumePending, v1.PersistentVolumeReclaimRetain, classEmpty)), + withVolumeVolumeMode(&modeBlock, newVolumeArray("volume14-1", "10Gi", "uid14-1", "claim14-1", v1.VolumeBound, v1.PersistentVolumeReclaimRetain, classEmpty, annBoundByController)), + withClaimVolumeMode(&modeBlock, newClaimArray("claim14-1", "uid14-1", "10Gi", "", v1.ClaimPending, nil)), + withClaimVolumeMode(&modeBlock, newClaimArray("claim14-1", "uid14-1", "10Gi", "volume14-1", v1.ClaimBound, nil, annBoundByController, annBindCompleted)), + noevents, noerrors, testSyncClaim, + }, + { + // syncVolume binds a requested filesystem claim to a filesystem volume + "14-2 - binding to volumeMode filesystem", + withVolumeVolumeMode(&modeFile, newVolumeArray("volume14-2", "10Gi", "", "", v1.VolumePending, v1.PersistentVolumeReclaimRetain, classEmpty)), + withVolumeVolumeMode(&modeFile, newVolumeArray("volume14-2", "10Gi", "uid14-2", "claim14-2", v1.VolumeBound, v1.PersistentVolumeReclaimRetain, classEmpty, annBoundByController)), + withClaimVolumeMode(&modeFile, newClaimArray("claim14-2", "uid14-2", "10Gi", "", v1.ClaimPending, nil)), + withClaimVolumeMode(&modeFile, newClaimArray("claim14-2", "uid14-2", "10Gi", "volume14-2", v1.ClaimBound, nil, annBoundByController, annBindCompleted)), + noevents, noerrors, testSyncClaim, + }, + { + // failed syncVolume do not bind to an unspecified volumemode for claim to a specified filesystem volume + "14-3 - do not bind pv volumeMode filesystem and pvc volumeMode block", + withVolumeVolumeMode(&modeFile, newVolumeArray("volume14-3", "10Gi", "", "", v1.VolumePending, v1.PersistentVolumeReclaimRetain, classEmpty)), + withVolumeVolumeMode(&modeFile, newVolumeArray("volume14-3", "10Gi", "", "", v1.VolumePending, v1.PersistentVolumeReclaimRetain, classEmpty)), + withClaimVolumeMode(&modeBlock, newClaimArray("claim14-3", "uid14-3", "10Gi", "", v1.ClaimPending, nil)), + withClaimVolumeMode(&modeBlock, newClaimArray("claim14-3", "uid14-3", "10Gi", "", v1.ClaimPending, nil)), + []string{"Normal FailedBinding"}, + noerrors, testSyncClaim, + }, + { + // failed syncVolume do not bind a requested filesystem claim to an unspecified volumeMode for volume + "14-4 - do not bind pv volumeMode block and pvc volumeMode filesystem", + withVolumeVolumeMode(&modeBlock, newVolumeArray("volume14-4", "10Gi", "", "", v1.VolumePending, v1.PersistentVolumeReclaimRetain, classEmpty)), + withVolumeVolumeMode(&modeBlock, newVolumeArray("volume14-4", "10Gi", "", "", v1.VolumePending, v1.PersistentVolumeReclaimRetain, classEmpty)), + withClaimVolumeMode(&modeFile, newClaimArray("claim14-4", "uid14-4", "10Gi", "", v1.ClaimPending, nil)), + withClaimVolumeMode(&modeFile, newClaimArray("claim14-4", "uid14-4", "10Gi", "", v1.ClaimPending, nil)), + []string{"Normal FailedBinding"}, + noerrors, testSyncClaim, + }, + { + // failed syncVolume do not bind when matching class but not matching volumeModes + "14-5 - do not bind when matching class but not volumeMode", + withVolumeVolumeMode(&modeFile, newVolumeArray("volume14-5", "10Gi", "", "", v1.VolumePending, v1.PersistentVolumeReclaimRetain, classGold)), + withVolumeVolumeMode(&modeFile, newVolumeArray("volume14-5", "10Gi", "", "", v1.VolumePending, v1.PersistentVolumeReclaimRetain, classGold)), + withClaimVolumeMode(&modeBlock, newClaimArray("claim14-5", "uid14-5", "10Gi", "", v1.ClaimPending, &classGold)), + withClaimVolumeMode(&modeBlock, newClaimArray("claim14-5", "uid14-5", "10Gi", "", v1.ClaimPending, &classGold)), + []string{"Warning ProvisioningFailed"}, + noerrors, testSyncClaim, + }, + { + // failed syncVolume do not bind when matching volumeModes but class does not match + "14-5-1 - do not bind when matching volumeModes but class does not match", + withVolumeVolumeMode(&modeFile, newVolumeArray("volume14-5-1", "10Gi", "", "", v1.VolumePending, v1.PersistentVolumeReclaimRetain, classGold)), + withVolumeVolumeMode(&modeFile, newVolumeArray("volume14-5-1", "10Gi", "", "", v1.VolumePending, v1.PersistentVolumeReclaimRetain, classGold)), + withClaimVolumeMode(&modeFile, newClaimArray("claim14-5-1", "uid14-5-1", "10Gi", "", v1.ClaimPending, &classSilver)), + withClaimVolumeMode(&modeFile, newClaimArray("claim14-5-1", "uid14-5-1", "10Gi", "", v1.ClaimPending, &classSilver)), + []string{"Warning ProvisioningFailed"}, + noerrors, testSyncClaim, + }, + { + // failed syncVolume do not bind when pvc is prebound to pv with matching volumeModes but class does not match + "14-5-2 - do not bind when pvc is prebound to pv with matching volumeModes but class does not match", + withVolumeVolumeMode(&modeFile, newVolumeArray("volume14-5-2", "10Gi", "", "", v1.VolumePending, v1.PersistentVolumeReclaimRetain, classGold)), + withVolumeVolumeMode(&modeFile, newVolumeArray("volume14-5-2", "10Gi", "", "", v1.VolumePending, v1.PersistentVolumeReclaimRetain, classGold)), + withClaimVolumeMode(&modeFile, newClaimArray("claim14-5-2", "uid14-5-2", "10Gi", "volume14-5-2", v1.ClaimPending, &classSilver)), + withClaimVolumeMode(&modeFile, newClaimArray("claim14-5-2", "uid14-5-2", "10Gi", "volume14-5-2", v1.ClaimPending, &classSilver)), + []string{"Warning VolumeMismatch"}, + noerrors, testSyncClaim, + }, + { + // syncVolume bind when pv is prebound and volumeModes match + "14-7 - bind when pv volume is prebound and volumeModes match", + withVolumeVolumeMode(&modeBlock, newVolumeArray("volume14-7", "10Gi", "", "claim14-7", v1.VolumePending, v1.PersistentVolumeReclaimRetain, classEmpty)), + withVolumeVolumeMode(&modeBlock, newVolumeArray("volume14-7", "10Gi", "uid14-7", "claim14-7", v1.VolumeBound, v1.PersistentVolumeReclaimRetain, classEmpty)), + withClaimVolumeMode(&modeBlock, newClaimArray("claim14-7", "uid14-7", "10Gi", "", v1.ClaimPending, nil)), + withClaimVolumeMode(&modeBlock, newClaimArray("claim14-7", "uid14-7", "10Gi", "volume14-7", v1.ClaimBound, nil, annBoundByController, annBindCompleted)), + noevents, noerrors, testSyncClaim, + }, + { + // failed syncVolume do not bind when pvc is prebound to pv with mismatching volumeModes + "14-8 - do not bind when pvc is prebound to pv with mismatching volumeModes", + withVolumeVolumeMode(&modeBlock, newVolumeArray("volume14-8", "10Gi", "", "", v1.VolumePending, v1.PersistentVolumeReclaimRetain, classEmpty)), + withVolumeVolumeMode(&modeBlock, newVolumeArray("volume14-8", "10Gi", "", "", v1.VolumePending, v1.PersistentVolumeReclaimRetain, classEmpty)), + withClaimVolumeMode(&modeFile, newClaimArray("claim14-8", "uid14-8", "10Gi", "volume14-8", v1.ClaimPending, nil)), + withClaimVolumeMode(&modeFile, newClaimArray("claim14-8", "uid14-8", "10Gi", "volume14-8", v1.ClaimPending, nil)), + []string{"Warning VolumeMismatch"}, + noerrors, testSyncClaim, + }, + { + // failed syncVolume do not bind when pvc is prebound to pv with mismatching volumeModes + "14-8-1 - do not bind when pv is prebound to pvc with mismatching volumeModes", + withVolumeVolumeMode(&modeBlock, newVolumeArray("volume14-8-1", "10Gi", "", "claim14-8-1", v1.VolumePending, v1.PersistentVolumeReclaimRetain, classEmpty)), + withVolumeVolumeMode(&modeBlock, newVolumeArray("volume14-8-1", "10Gi", "", "claim14-8-1", v1.VolumePending, v1.PersistentVolumeReclaimRetain, classEmpty)), + withClaimVolumeMode(&modeFile, newClaimArray("claim14-8-1", "uid14-8-1", "10Gi", "", v1.ClaimPending, nil)), + withClaimVolumeMode(&modeFile, newClaimArray("claim14-8-1", "uid14-8-1", "10Gi", "", v1.ClaimPending, nil)), + []string{"Normal FailedBinding"}, + noerrors, testSyncClaim, + }, + { + // syncVolume binds when pvc is prebound to pv with matching volumeModes block + "14-9 - bind when pvc is prebound to pv with matching volumeModes block", + withVolumeVolumeMode(&modeBlock, newVolumeArray("volume14-9", "10Gi", "", "", v1.VolumePending, v1.PersistentVolumeReclaimRetain, classEmpty)), + withVolumeVolumeMode(&modeBlock, newVolumeArray("volume14-9", "10Gi", "uid14-9", "claim14-9", v1.VolumeBound, v1.PersistentVolumeReclaimRetain, classEmpty, annBoundByController)), + withClaimVolumeMode(&modeBlock, newClaimArray("claim14-9", "uid14-9", "10Gi", "volume14-9", v1.ClaimPending, nil)), + withClaimVolumeMode(&modeBlock, newClaimArray("claim14-9", "uid14-9", "10Gi", "volume14-9", v1.ClaimBound, nil, annBindCompleted)), + noevents, noerrors, testSyncClaim, + }, + { + // syncVolume binds when pv is prebound to pvc with matching volumeModes block + "14-10 - bind when pv is prebound to pvc with matching volumeModes block", + withVolumeVolumeMode(&modeBlock, newVolumeArray("volume14-10", "10Gi", "", "claim14-10", v1.VolumePending, v1.PersistentVolumeReclaimRetain, classEmpty)), + withVolumeVolumeMode(&modeBlock, newVolumeArray("volume14-10", "10Gi", "uid14-10", "claim14-10", v1.VolumeBound, v1.PersistentVolumeReclaimRetain, classEmpty)), + withClaimVolumeMode(&modeBlock, newClaimArray("claim14-10", "uid14-10", "10Gi", "", v1.ClaimPending, nil)), + withClaimVolumeMode(&modeBlock, newClaimArray("claim14-10", "uid14-10", "10Gi", "volume14-10", v1.ClaimBound, nil, annBoundByController, annBindCompleted)), + noevents, noerrors, testSyncClaim, + }, + { + // syncVolume binds when pvc is prebound to pv with matching volumeModes filesystem + "14-11 - bind when pvc is prebound to pv with matching volumeModes filesystem", + withVolumeVolumeMode(&modeFile, newVolumeArray("volume14-11", "10Gi", "", "", v1.VolumePending, v1.PersistentVolumeReclaimRetain, classEmpty)), + withVolumeVolumeMode(&modeFile, newVolumeArray("volume14-11", "10Gi", "uid14-11", "claim14-11", v1.VolumeBound, v1.PersistentVolumeReclaimRetain, classEmpty, annBoundByController)), + withClaimVolumeMode(&modeFile, newClaimArray("claim14-11", "uid14-11", "10Gi", "volume14-11", v1.ClaimPending, nil)), + withClaimVolumeMode(&modeFile, newClaimArray("claim14-11", "uid14-11", "10Gi", "volume14-11", v1.ClaimBound, nil, annBindCompleted)), + noevents, noerrors, testSyncClaim, + }, + { + // syncVolume binds when pv is prebound to pvc with matching volumeModes filesystem + "14-12 - bind when pv is prebound to pvc with matching volumeModes filesystem", + withVolumeVolumeMode(&modeFile, newVolumeArray("volume14-12", "10Gi", "", "claim14-12", v1.VolumePending, v1.PersistentVolumeReclaimRetain, classEmpty)), + withVolumeVolumeMode(&modeFile, newVolumeArray("volume14-12", "10Gi", "uid14-12", "claim14-12", v1.VolumeBound, v1.PersistentVolumeReclaimRetain, classEmpty)), + withClaimVolumeMode(&modeFile, newClaimArray("claim14-12", "uid14-12", "10Gi", "", v1.ClaimPending, nil)), + withClaimVolumeMode(&modeFile, newClaimArray("claim14-12", "uid14-12", "10Gi", "volume14-12", v1.ClaimBound, nil, annBoundByController, annBindCompleted)), + noevents, noerrors, testSyncClaim, + }, + } + + err := utilfeature.DefaultFeatureGate.Set("BlockVolume=true") + if err != nil { + t.Errorf("Failed to enable feature gate for BlockVolume: %v", err) + return + } + + runSyncTests(t, tests, []*storage.StorageClass{}) + + err1 := utilfeature.DefaultFeatureGate.Set("BlockVolume=false") + if err1 != nil { + t.Errorf("Failed to disable feature gate for BlockVolume: %v", err) + return + } +} + // Test multiple calls to syncClaim/syncVolume and periodic sync of all // volume/claims. The test follows this pattern: // 0. Load the controller with initial data. diff --git a/pkg/controller/volume/persistentvolume/framework_test.go b/pkg/controller/volume/persistentvolume/framework_test.go index e81a60ccb36..d826947191d 100644 --- a/pkg/controller/volume/persistentvolume/framework_test.go +++ b/pkg/controller/volume/persistentvolume/framework_test.go @@ -680,6 +680,22 @@ func withLabelSelector(labels map[string]string, claims []*v1.PersistentVolumeCl return claims } +// withVolumeVolumeMode applies the given VolumeMode to the first volume in the array and +// returns the array. Meant to be used to compose volumes specified inline in +// a test. +func withVolumeVolumeMode(mode *v1.PersistentVolumeMode, volumes []*v1.PersistentVolume) []*v1.PersistentVolume { + volumes[0].Spec.VolumeMode = mode + return volumes +} + +// withClaimVolumeMode applies the given VolumeMode to the first claim in the array and +// returns the array. Meant to be used to compose volumes specified inline in +// a test. +func withClaimVolumeMode(mode *v1.PersistentVolumeMode, claims []*v1.PersistentVolumeClaim) []*v1.PersistentVolumeClaim { + claims[0].Spec.VolumeMode = mode + return claims +} + // withExpectedCapacity sets the claim.Spec.Capacity of the first claim in the // array to given value and returns the array. Meant to be used to compose // claims specified inline in a test. diff --git a/pkg/controller/volume/persistentvolume/index.go b/pkg/controller/volume/persistentvolume/index.go index d39fca6d21c..bf356f41f7c 100644 --- a/pkg/controller/volume/persistentvolume/index.go +++ b/pkg/controller/volume/persistentvolume/index.go @@ -24,8 +24,10 @@ import ( "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/labels" + utilfeature "k8s.io/apiserver/pkg/util/feature" "k8s.io/client-go/tools/cache" v1helper "k8s.io/kubernetes/pkg/apis/core/v1/helper" + "k8s.io/kubernetes/pkg/features" "k8s.io/kubernetes/pkg/volume" ) @@ -116,6 +118,16 @@ func (pvIndex *persistentVolumeOrderedIndex) findByClaim(claim *v1.PersistentVol // - find the smallest matching one if there is no volume pre-bound to // the claim. for _, volume := range volumes { + // check if volumeModes do not match (Alpha and feature gate protected) + isMisMatch, err := checkVolumeModeMisMatches(&claim.Spec, &volume.Spec) + if err != nil { + return nil, fmt.Errorf("error checking if volumeMode was a mismatch: %v", err) + } + // filter out mismatching volumeModes + if isMisMatch { + continue + } + if isVolumeBoundToClaim(volume, claim) { // this claim and volume are pre-bound; return // the volume if the size request is satisfied, @@ -157,6 +169,27 @@ func (pvIndex *persistentVolumeOrderedIndex) findByClaim(claim *v1.PersistentVol return nil, nil } +// checkVolumeModeMatches is a convenience method that checks volumeMode for PersistentVolume +// and PersistentVolumeClaims along with making sure that the Alpha feature gate BlockVolume is +// enabled. +// This is Alpha and could change in the future. +func checkVolumeModeMisMatches(pvcSpec *v1.PersistentVolumeClaimSpec, pvSpec *v1.PersistentVolumeSpec) (bool, error) { + if utilfeature.DefaultFeatureGate.Enabled(features.BlockVolume) { + if pvSpec.VolumeMode != nil && pvcSpec.VolumeMode != nil { + requestedVolumeMode := *pvcSpec.VolumeMode + pvVolumeMode := *pvSpec.VolumeMode + return requestedVolumeMode != pvVolumeMode, nil + } else { + // This also should retrun an error, this means that + // the defaulting has failed. + return true, fmt.Errorf("api defaulting for volumeMode failed") + } + } else { + // feature gate is disabled + return false, nil + } +} + // findBestMatchForClaim is a convenience method that finds a volume by the claim's AccessModes and requests for Storage func (pvIndex *persistentVolumeOrderedIndex) findBestMatchForClaim(claim *v1.PersistentVolumeClaim) (*v1.PersistentVolume, error) { return pvIndex.findByClaim(claim) diff --git a/pkg/controller/volume/persistentvolume/index_test.go b/pkg/controller/volume/persistentvolume/index_test.go index 7bab162d937..0f6551fa382 100644 --- a/pkg/controller/volume/persistentvolume/index_test.go +++ b/pkg/controller/volume/persistentvolume/index_test.go @@ -23,6 +23,7 @@ import ( "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + utilfeature "k8s.io/apiserver/pkg/util/feature" "k8s.io/client-go/kubernetes/scheme" ref "k8s.io/client-go/tools/reference" "k8s.io/kubernetes/pkg/api/testapi" @@ -50,6 +51,28 @@ func makePVC(size string, modfn func(*v1.PersistentVolumeClaim)) *v1.PersistentV return &pvc } +func makeVolumeModePVC(size string, mode *v1.PersistentVolumeMode, modfn func(*v1.PersistentVolumeClaim)) *v1.PersistentVolumeClaim { + pvc := v1.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{ + Name: "claim01", + Namespace: "myns", + }, + Spec: v1.PersistentVolumeClaimSpec{ + VolumeMode: mode, + AccessModes: []v1.PersistentVolumeAccessMode{v1.ReadWriteOnce}, + Resources: v1.ResourceRequirements{ + Requests: v1.ResourceList{ + v1.ResourceName(v1.ResourceStorage): resource.MustParse(size), + }, + }, + }, + } + if modfn != nil { + modfn(&pvc) + } + return &pvc +} + func TestMatchVolume(t *testing.T) { volList := newPersistentVolumeOrderedIndex() for _, pv := range createTestVolumes() { @@ -669,6 +692,249 @@ func testVolume(name, size string) *v1.PersistentVolume { } } +func createVolumeModeBlockTestVolume() *v1.PersistentVolume { + blockMode := v1.PersistentVolumeBlock + + return &v1.PersistentVolume{ + ObjectMeta: metav1.ObjectMeta{ + UID: "local-1", + Name: "block", + }, + Spec: v1.PersistentVolumeSpec{ + Capacity: v1.ResourceList{ + v1.ResourceName(v1.ResourceStorage): resource.MustParse("10G"), + }, + PersistentVolumeSource: v1.PersistentVolumeSource{ + Local: &v1.LocalVolumeSource{}, + }, + AccessModes: []v1.PersistentVolumeAccessMode{ + v1.ReadWriteOnce, + }, + VolumeMode: &blockMode, + }, + } +} + +func createVolumeModeFilesystemTestVolume() *v1.PersistentVolume { + filesystemMode := v1.PersistentVolumeFilesystem + + return &v1.PersistentVolume{ + ObjectMeta: metav1.ObjectMeta{ + UID: "local-1", + Name: "block", + }, + Spec: v1.PersistentVolumeSpec{ + Capacity: v1.ResourceList{ + v1.ResourceName(v1.ResourceStorage): resource.MustParse("10G"), + }, + PersistentVolumeSource: v1.PersistentVolumeSource{ + Local: &v1.LocalVolumeSource{}, + }, + AccessModes: []v1.PersistentVolumeAccessMode{ + v1.ReadWriteOnce, + }, + VolumeMode: &filesystemMode, + }, + } +} + +func createTestVolOrderedIndex(pv *v1.PersistentVolume) persistentVolumeOrderedIndex { + volFile := newPersistentVolumeOrderedIndex() + volFile.store.Add(pv) + return volFile +} + +func toggleBlockVolumeFeature(toggleFlag bool, t *testing.T) { + if toggleFlag { + // Enable alpha feature BlockVolume + err := utilfeature.DefaultFeatureGate.Set("BlockVolume=true") + if err != nil { + t.Errorf("Failed to enable feature gate for BlockVolume: %v", err) + return + } + } else { + err := utilfeature.DefaultFeatureGate.Set("BlockVolume=false") + if err != nil { + t.Errorf("Failed to disable feature gate for BlockVolume: %v", err) + return + } + } +} + +func TestAlphaVolumeModeCheck(t *testing.T) { + + blockMode := v1.PersistentVolumeBlock + filesystemMode := v1.PersistentVolumeFilesystem + + // If feature gate is enabled, VolumeMode will always be defaulted + // If feature gate is disabled, VolumeMode is dropped by API and ignored + scenarios := map[string]struct { + isExpectedMisMatch bool + vol *v1.PersistentVolume + pvc *v1.PersistentVolumeClaim + enableBlock bool + }{ + "feature enabled - pvc block and pv filesystem": { + isExpectedMisMatch: true, + vol: createVolumeModeFilesystemTestVolume(), + pvc: makeVolumeModePVC("8G", &blockMode, nil), + enableBlock: true, + }, + "feature enabled - pvc filesystem and pv block": { + isExpectedMisMatch: true, + vol: createVolumeModeBlockTestVolume(), + pvc: makeVolumeModePVC("8G", &filesystemMode, nil), + enableBlock: true, + }, + "feature enabled - pvc block and pv block": { + isExpectedMisMatch: false, + vol: createVolumeModeBlockTestVolume(), + pvc: makeVolumeModePVC("8G", &blockMode, nil), + enableBlock: true, + }, + "feature enabled - pvc filesystem and pv filesystem": { + isExpectedMisMatch: false, + vol: createVolumeModeFilesystemTestVolume(), + pvc: makeVolumeModePVC("8G", &filesystemMode, nil), + enableBlock: true, + }, + "feature disabled - pvc block and pv filesystem": { + isExpectedMisMatch: false, + vol: createVolumeModeFilesystemTestVolume(), + pvc: makeVolumeModePVC("8G", &blockMode, nil), + enableBlock: false, + }, + "feature disabled - pvc filesystem and pv block": { + isExpectedMisMatch: false, + vol: createVolumeModeBlockTestVolume(), + pvc: makeVolumeModePVC("8G", &filesystemMode, nil), + enableBlock: false, + }, + "feature disabled - pvc block and pv block": { + isExpectedMisMatch: false, + vol: createVolumeModeBlockTestVolume(), + pvc: makeVolumeModePVC("8G", &blockMode, nil), + enableBlock: false, + }, + "feature disabled - pvc filesystem and pv filesystem": { + isExpectedMisMatch: false, + vol: createVolumeModeFilesystemTestVolume(), + pvc: makeVolumeModePVC("8G", &filesystemMode, nil), + enableBlock: false, + }, + } + + for name, scenario := range scenarios { + toggleBlockVolumeFeature(scenario.enableBlock, t) + expectedMisMatch, err := checkVolumeModeMisMatches(&scenario.pvc.Spec, &scenario.vol.Spec) + if err != nil { + t.Errorf("Unexpected failure for checkVolumeModeMisMatches: %v", err) + } + // expected to match but either got an error or no returned pvmatch + if expectedMisMatch && !scenario.isExpectedMisMatch { + t.Errorf("Unexpected failure for scenario, expected not to mismatch on modes but did: %s", name) + } + if !expectedMisMatch && scenario.isExpectedMisMatch { + t.Errorf("Unexpected failure for scenario, did not mismatch on mode when expected to mismatch: %s", name) + } + } + +} + +func TestAlphaFilteringVolumeModes(t *testing.T) { + blockMode := v1.PersistentVolumeBlock + filesystemMode := v1.PersistentVolumeFilesystem + + // If feature gate is enabled, VolumeMode will always be defaulted + // If feature gate is disabled, VolumeMode is dropped by API and ignored + scenarios := map[string]struct { + isExpectedMatch bool + vol persistentVolumeOrderedIndex + pvc *v1.PersistentVolumeClaim + enableBlock bool + }{ + "1-1 feature enabled - pvc block and pv filesystem": { + isExpectedMatch: false, + vol: createTestVolOrderedIndex(createVolumeModeFilesystemTestVolume()), + pvc: makeVolumeModePVC("8G", &blockMode, nil), + enableBlock: true, + }, + "1-2 feature enabled - pvc filesystem and pv block": { + isExpectedMatch: false, + vol: createTestVolOrderedIndex(createVolumeModeBlockTestVolume()), + pvc: makeVolumeModePVC("8G", &filesystemMode, nil), + enableBlock: true, + }, + "1-3 feature enabled - pvc block and pv no mode with default filesystem": { + isExpectedMatch: false, + vol: createTestVolOrderedIndex(createVolumeModeFilesystemTestVolume()), + pvc: makeVolumeModePVC("8G", &blockMode, nil), + enableBlock: true, + }, + "1-4 feature enabled - pvc no mode defaulted to filesystem and pv block": { + isExpectedMatch: false, + vol: createTestVolOrderedIndex(createVolumeModeBlockTestVolume()), + pvc: makeVolumeModePVC("8G", &filesystemMode, nil), + enableBlock: true, + }, + "1-5 feature enabled - pvc block and pv block": { + isExpectedMatch: true, + vol: createTestVolOrderedIndex(createVolumeModeBlockTestVolume()), + pvc: makeVolumeModePVC("8G", &blockMode, nil), + enableBlock: true, + }, + "1-6 feature enabled - pvc filesystem and pv filesystem": { + isExpectedMatch: true, + vol: createTestVolOrderedIndex(createVolumeModeFilesystemTestVolume()), + pvc: makeVolumeModePVC("8G", &filesystemMode, nil), + enableBlock: true, + }, + "1-7 feature enabled - pvc mode is nil and defaulted and pv mode is nil and defaulted": { + isExpectedMatch: true, + vol: createTestVolOrderedIndex(createVolumeModeFilesystemTestVolume()), + pvc: makeVolumeModePVC("8G", &filesystemMode, nil), + enableBlock: true, + }, + "2-1 feature disabled - pvc mode is nil and pv mode is nil": { + isExpectedMatch: true, + vol: createTestVolOrderedIndex(testVolume("nomode-1", "8G")), + pvc: makeVolumeModePVC("8G", nil, nil), + enableBlock: false, + }, + "2-2 feature disabled - pvc mode is block and pv mode is block - fields should be dropped by api and not analyzed with gate disabled": { + isExpectedMatch: true, + vol: createTestVolOrderedIndex(createVolumeModeBlockTestVolume()), + pvc: makeVolumeModePVC("8G", &blockMode, nil), + enableBlock: false, + }, + "2-3 feature disabled - pvc mode is filesystem and pv mode is filesystem - fields should be dropped by api and not analyzed with gate disabled": { + isExpectedMatch: true, + vol: createTestVolOrderedIndex(createVolumeModeFilesystemTestVolume()), + pvc: makeVolumeModePVC("8G", &filesystemMode, nil), + enableBlock: false, + }, + } + + for name, scenario := range scenarios { + toggleBlockVolumeFeature(scenario.enableBlock, t) + pvmatch, err := scenario.vol.findBestMatchForClaim(scenario.pvc) + // expected to match but either got an error or no returned pvmatch + if pvmatch == nil && scenario.isExpectedMatch { + t.Errorf("Unexpected failure for scenario, no matching volume: %s", name) + } + if err != nil && scenario.isExpectedMatch { + t.Errorf("Unexpected failure for scenario: %s - %+v", name, err) + } + // expected to not match but either got an error or a returned pvmatch + if pvmatch != nil && !scenario.isExpectedMatch { + t.Errorf("Unexpected failure for scenario, expected no matching volume: %s", name) + } + if err != nil && !scenario.isExpectedMatch { + t.Errorf("Unexpected failure for scenario: %s - %+v", name, err) + } + } +} + func TestFindingPreboundVolumes(t *testing.T) { claim := &v1.PersistentVolumeClaim{ ObjectMeta: metav1.ObjectMeta{ diff --git a/pkg/controller/volume/persistentvolume/pv_controller.go b/pkg/controller/volume/persistentvolume/pv_controller.go index ebcb38995a6..20e10a28b78 100644 --- a/pkg/controller/volume/persistentvolume/pv_controller.go +++ b/pkg/controller/volume/persistentvolume/pv_controller.go @@ -231,6 +231,10 @@ func (ctrl *PersistentVolumeController) syncClaim(claim *v1.PersistentVolumeClai func checkVolumeSatisfyClaim(volume *v1.PersistentVolume, claim *v1.PersistentVolumeClaim) error { requestedQty := claim.Spec.Resources.Requests[v1.ResourceName(v1.ResourceStorage)] requestedSize := requestedQty.Value() + isMisMatch, err := checkVolumeModeMisMatches(&claim.Spec, &volume.Spec) + if err != nil { + return fmt.Errorf("error checking if volumeMode was a mismatch: %v", err) + } volumeQty := volume.Spec.Capacity[v1.ResourceStorage] volumeSize := volumeQty.Value() @@ -243,6 +247,10 @@ func checkVolumeSatisfyClaim(volume *v1.PersistentVolume, claim *v1.PersistentVo return fmt.Errorf("Class of volume[%s] is not the same as claim[%v]", volume.Name, claimToClaimKey(claim)) } + if isMisMatch { + return fmt.Errorf("VolumeMode[%v] of volume[%s] is incompatible with VolumeMode[%v] of claim[%v]", volume.Spec.VolumeMode, volume.Name, claim.Spec.VolumeMode, claim.Name) + } + return nil }