diff --git a/pkg/controller/volume/persistentvolume/binder_test.go b/pkg/controller/volume/persistentvolume/binder_test.go index 0dcf6b7eaf4..bd191a9642c 100644 --- a/pkg/controller/volume/persistentvolume/binder_test.go +++ b/pkg/controller/volume/persistentvolume/binder_test.go @@ -248,6 +248,17 @@ func TestSync(t *testing.T) { }, noevents, noerrors, testSyncClaim, }, + { + // syncClaim that scheduled to a selected node + "1-18 - successful pre-bound PV to PVC provisioning", + newVolumeArray("volume1-18", "1Gi", "uid1-18", "claim1-18", v1.VolumeAvailable, v1.PersistentVolumeReclaimRetain, classWait), + newVolumeArray("volume1-18", "1Gi", "uid1-18", "claim1-18", v1.VolumeBound, v1.PersistentVolumeReclaimRetain, classWait), + claimWithAnnotation(pvutil.AnnSelectedNode, "node1", + newClaimArray("claim1-18", "uid1-18", "1Gi", "", v1.ClaimPending, &classWait)), + claimWithAnnotation(pvutil.AnnSelectedNode, "node1", + newClaimArray("claim1-18", "uid1-18", "1Gi", "volume1-18", v1.ClaimBound, &classWait, pvutil.AnnBoundByController, pvutil.AnnBindCompleted)), + noevents, noerrors, testSyncClaim, + }, // [Unit test set 2] User asked for a specific PV. // Test the binding when pv.ClaimRef is already set by controller or diff --git a/pkg/controller/volume/persistentvolume/framework_test.go b/pkg/controller/volume/persistentvolume/framework_test.go index cc550647e17..d578da7fbaf 100644 --- a/pkg/controller/volume/persistentvolume/framework_test.go +++ b/pkg/controller/volume/persistentvolume/framework_test.go @@ -471,9 +471,11 @@ const operationRecycle = "Recycle" var ( classGold string = "gold" classSilver string = "silver" + classCopper string = "copper" classEmpty string = "" classNonExisting string = "non-existing" classExternal string = "external" + classExternalWait string = "external-wait" classUnknownInternal string = "unknown-internal" classUnsupportedMountOptions string = "unsupported-mountoptions" classLarge string = "large" diff --git a/pkg/controller/volume/persistentvolume/provision_test.go b/pkg/controller/volume/persistentvolume/provision_test.go index 865a87fd7fb..919615805e1 100644 --- a/pkg/controller/volume/persistentvolume/provision_test.go +++ b/pkg/controller/volume/persistentvolume/provision_test.go @@ -24,6 +24,8 @@ import ( storage "k8s.io/api/storage/v1" apierrs "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + corelisters "k8s.io/client-go/listers/core/v1" + "k8s.io/client-go/tools/cache" api "k8s.io/kubernetes/pkg/apis/core" pvtesting "k8s.io/kubernetes/pkg/controller/volume/persistentvolume/testing" pvutil "k8s.io/kubernetes/pkg/controller/volume/persistentvolume/util" @@ -64,6 +66,18 @@ var storageClasses = []*storage.StorageClass{ ReclaimPolicy: &deleteReclaimPolicy, VolumeBindingMode: &modeImmediate, }, + { + TypeMeta: metav1.TypeMeta{ + Kind: "StorageClass", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "copper", + }, + Provisioner: mockPluginName, + Parameters: class1Parameters, + ReclaimPolicy: &deleteReclaimPolicy, + VolumeBindingMode: &modeWait, + }, { TypeMeta: metav1.TypeMeta{ Kind: "StorageClass", @@ -76,6 +90,18 @@ var storageClasses = []*storage.StorageClass{ ReclaimPolicy: &deleteReclaimPolicy, VolumeBindingMode: &modeImmediate, }, + { + TypeMeta: metav1.TypeMeta{ + Kind: "StorageClass", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "external-wait", + }, + Provisioner: "vendor.com/my-volume-wait", + Parameters: class1Parameters, + ReclaimPolicy: &deleteReclaimPolicy, + VolumeBindingMode: &modeWait, + }, { TypeMeta: metav1.TypeMeta{ Kind: "StorageClass", @@ -443,6 +469,41 @@ func TestProvisionSync(t *testing.T) { noevents, noerrors, wrapTestWithProvisionCalls([]provisionCall{}, testSyncClaim), }, + { + // volume provision for PVC scheduled + "11-23 - skip finding PV and provision for PVC annotated with AnnSelectedNode", + newVolumeArray("volume11-23", "1Gi", "", "", v1.VolumeAvailable, v1.PersistentVolumeReclaimDelete, classCopper), + []*v1.PersistentVolume{ + newVolume("volume11-23", "1Gi", "", "", v1.VolumeAvailable, v1.PersistentVolumeReclaimDelete, classCopper), + newVolume("pvc-uid11-23", "1Gi", "uid11-23", "claim11-23", v1.VolumeBound, v1.PersistentVolumeReclaimDelete, classCopper, pvutil.AnnDynamicallyProvisioned, pvutil.AnnBoundByController), + }, + claimWithAnnotation(pvutil.AnnSelectedNode, "node1", + newClaimArray("claim11-23", "uid11-23", "1Gi", "", v1.ClaimPending, &classCopper)), + claimWithAnnotation(pvutil.AnnSelectedNode, "node1", + newClaimArray("claim11-23", "uid11-23", "1Gi", "", v1.ClaimPending, &classCopper, pvutil.AnnStorageProvisioner)), + []string{"Normal ProvisioningSucceeded"}, + noerrors, + wrapTestWithInjectedOperation(wrapTestWithProvisionCalls([]provisionCall{provision1Success}, testSyncClaim), + func(ctrl *PersistentVolumeController, reactor *pvtesting.VolumeReactor) { + nodesIndexer := cache.NewIndexer(cache.MetaNamespaceKeyFunc, cache.Indexers{}) + node := &v1.Node{ObjectMeta: metav1.ObjectMeta{Name: "node1"}} + nodesIndexer.Add(node) + ctrl.NodeLister = corelisters.NewNodeLister(nodesIndexer) + }), + }, + { + // volume provision for PVC that scheduled + "11-24 - skip finding PV and wait external provisioner for PVC annotated with AnnSelectedNode", + newVolumeArray("volume11-24", "1Gi", "", "", v1.VolumeAvailable, v1.PersistentVolumeReclaimDelete, classExternalWait), + newVolumeArray("volume11-24", "1Gi", "", "", v1.VolumeAvailable, v1.PersistentVolumeReclaimDelete, classExternalWait), + claimWithAnnotation(pvutil.AnnSelectedNode, "node1", + newClaimArray("claim11-24", "uid11-24", "1Gi", "", v1.ClaimPending, &classExternalWait)), + claimWithAnnotation(pvutil.AnnStorageProvisioner, "vendor.com/my-volume-wait", + claimWithAnnotation(pvutil.AnnSelectedNode, "node1", + newClaimArray("claim11-24", "uid11-24", "1Gi", "", v1.ClaimPending, &classExternalWait))), + []string{"Normal ExternalProvisioning"}, + noerrors, testSyncClaim, + }, } runSyncTests(t, tests, storageClasses, []*v1.Pod{}) } diff --git a/pkg/controller/volume/persistentvolume/pv_controller.go b/pkg/controller/volume/persistentvolume/pv_controller.go index ff0cf4227ee..56f2cba5a6a 100644 --- a/pkg/controller/volume/persistentvolume/pv_controller.go +++ b/pkg/controller/volume/persistentvolume/pv_controller.go @@ -281,27 +281,6 @@ func checkVolumeSatisfyClaim(volume *v1.PersistentVolume, claim *v1.PersistentVo return nil } -func (ctrl *PersistentVolumeController) isDelayBindingProvisioning(claim *v1.PersistentVolumeClaim) bool { - // When feature VolumeScheduling enabled, - // Scheduler signal to the PV controller to start dynamic - // provisioning by setting the "AnnSelectedNode" annotation - // in the PVC - _, ok := claim.Annotations[pvutil.AnnSelectedNode] - return ok -} - -// shouldDelayBinding returns true if binding of claim should be delayed, false otherwise. -// If binding of claim should be delayed, only claims pbound by scheduler -func (ctrl *PersistentVolumeController) shouldDelayBinding(claim *v1.PersistentVolumeClaim) (bool, error) { - // If claim has already been assigned a node by scheduler for dynamic provisioning. - if ctrl.isDelayBindingProvisioning(claim) { - return false, nil - } - - // If claim is in delay binding mode. - return pvutil.IsDelayBindingMode(claim, ctrl.classLister) -} - // syncUnboundClaim is the main controller method to decide what to do with an // unbound claim. func (ctrl *PersistentVolumeController) syncUnboundClaim(claim *v1.PersistentVolumeClaim) error { @@ -309,7 +288,7 @@ func (ctrl *PersistentVolumeController) syncUnboundClaim(claim *v1.PersistentVol // OBSERVATION: pvc is "Pending" if claim.Spec.VolumeName == "" { // User did not care which PV they get. - delayBinding, err := ctrl.shouldDelayBinding(claim) + delayBinding, err := pvutil.IsDelayBindingMode(claim, ctrl.classLister) if err != nil { return err } @@ -325,7 +304,7 @@ func (ctrl *PersistentVolumeController) syncUnboundClaim(claim *v1.PersistentVol // No PV could be found // OBSERVATION: pvc is "Pending", will retry switch { - case delayBinding: + case delayBinding && !pvutil.IsDelayBindingProvisioning(claim): ctrl.eventRecorder.Event(claim, v1.EventTypeNormal, events.WaitForFirstConsumer, "waiting for first consumer to be created before binding") case v1helper.GetPersistentVolumeClaimClass(claim) != "": if err = ctrl.provisionClaim(claim); err != nil { diff --git a/pkg/controller/volume/persistentvolume/pv_controller_test.go b/pkg/controller/volume/persistentvolume/pv_controller_test.go index 7f61f409860..876af6a8166 100644 --- a/pkg/controller/volume/persistentvolume/pv_controller_test.go +++ b/pkg/controller/volume/persistentvolume/pv_controller_test.go @@ -374,7 +374,7 @@ func TestControllerCacheParsingError(t *testing.T) { } } -func makePVCClass(scName *string, hasSelectNodeAnno bool) *v1.PersistentVolumeClaim { +func makePVCClass(scName *string) *v1.PersistentVolumeClaim { claim := &v1.PersistentVolumeClaim{ ObjectMeta: metav1.ObjectMeta{ Annotations: map[string]string{}, @@ -384,10 +384,6 @@ func makePVCClass(scName *string, hasSelectNodeAnno bool) *v1.PersistentVolumeCl }, } - if hasSelectNodeAnno { - claim.Annotations[pvutil.AnnSelectedNode] = "node-name" - } - return claim } @@ -400,37 +396,33 @@ func makeStorageClass(scName string, mode *storagev1.VolumeBindingMode) *storage } } -func TestDelayBinding(t *testing.T) { +func TestDelayBindingMode(t *testing.T) { tests := map[string]struct { pvc *v1.PersistentVolumeClaim shouldDelay bool shouldFail bool }{ "nil-class": { - pvc: makePVCClass(nil, false), + pvc: makePVCClass(nil), shouldDelay: false, }, "class-not-found": { - pvc: makePVCClass(&classNotHere, false), + pvc: makePVCClass(&classNotHere), shouldDelay: false, }, "no-mode-class": { - pvc: makePVCClass(&classNoMode, false), + pvc: makePVCClass(&classNoMode), shouldDelay: false, shouldFail: true, }, "immediate-mode-class": { - pvc: makePVCClass(&classImmediateMode, false), + pvc: makePVCClass(&classImmediateMode), shouldDelay: false, }, "wait-mode-class": { - pvc: makePVCClass(&classWaitMode, false), + pvc: makePVCClass(&classWaitMode), shouldDelay: true, }, - "wait-mode-class-with-selectedNode": { - pvc: makePVCClass(&classWaitMode, true), - shouldDelay: false, - }, } classes := []*storagev1.StorageClass{ @@ -453,7 +445,7 @@ func TestDelayBinding(t *testing.T) { } for name, test := range tests { - shouldDelay, err := ctrl.shouldDelayBinding(test.pvc) + shouldDelay, err := pvutil.IsDelayBindingMode(test.pvc, ctrl.classLister) if err != nil && !test.shouldFail { t.Errorf("Test %q returned error: %v", name, err) } diff --git a/pkg/controller/volume/persistentvolume/util/util.go b/pkg/controller/volume/persistentvolume/util/util.go index e2652b802bd..93e377b506e 100644 --- a/pkg/controller/volume/persistentvolume/util/util.go +++ b/pkg/controller/volume/persistentvolume/util/util.go @@ -69,6 +69,16 @@ const ( AnnStorageProvisioner = "volume.beta.kubernetes.io/storage-provisioner" ) +// IsDelayBindingProvisioning checks if claim provisioning with selected-node annotation +func IsDelayBindingProvisioning(claim *v1.PersistentVolumeClaim) bool { + // When feature VolumeScheduling enabled, + // Scheduler signal to the PV controller to start dynamic + // provisioning by setting the "AnnSelectedNode" annotation + // in the PVC + _, ok := claim.Annotations[AnnSelectedNode] + return ok +} + // IsDelayBindingMode checks if claim is in delay binding mode. func IsDelayBindingMode(claim *v1.PersistentVolumeClaim, classLister storagelisters.StorageClassLister) (bool, error) { className := v1helper.GetPersistentVolumeClaimClass(claim)