From 446f36559e933d3eeea666bd2165db21b7bb419e Mon Sep 17 00:00:00 2001 From: lichuqiang Date: Sat, 21 Apr 2018 11:26:26 +0800 Subject: [PATCH] pv_controller change for provisioning --- .../volume/persistentvolume/pv_controller.go | 43 ++++++++++++++++-- .../persistentvolume/pv_controller_test.go | 45 +++++++++++++++++-- 2 files changed, 82 insertions(+), 6 deletions(-) diff --git a/pkg/controller/volume/persistentvolume/pv_controller.go b/pkg/controller/volume/persistentvolume/pv_controller.go index ac796ce481a..a46c75510dd 100644 --- a/pkg/controller/volume/persistentvolume/pv_controller.go +++ b/pkg/controller/volume/persistentvolume/pv_controller.go @@ -285,6 +285,16 @@ func (ctrl *PersistentVolumeController) shouldDelayBinding(claim *v1.PersistentV return false, nil } + if utilfeature.DefaultFeatureGate.Enabled(features.DynamicProvisioningScheduling) { + // When feature DynamicProvisioningScheduling enabled, + // Scheduler signal to the PV controller to start dynamic + // provisioning by setting the "annSelectedNode" annotation + // in the PVC + if _, ok := claim.Annotations[annSelectedNode]; ok { + return false, nil + } + } + className := v1helper.GetPersistentVolumeClaimClass(claim) if className == "" { return false, nil @@ -299,8 +309,6 @@ func (ctrl *PersistentVolumeController) shouldDelayBinding(claim *v1.PersistentV return false, fmt.Errorf("VolumeBindingMode not set for StorageClass %q", className) } - // TODO: add check to handle dynamic provisioning later - return *class.VolumeBindingMode == storage.VolumeBindingWaitForFirstConsumer, nil } @@ -328,7 +336,6 @@ func (ctrl *PersistentVolumeController) syncUnboundClaim(claim *v1.PersistentVol // OBSERVATION: pvc is "Pending", will retry switch { case delayBinding: - // TODO: Skip dynamic provisioning for now 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 { @@ -1428,9 +1435,16 @@ func (ctrl *PersistentVolumeController) provisionClaimOperation(claim *v1.Persis } opComplete := util.OperationCompleteHook(plugin.GetPluginName(), "volume_provision") + // TODO: modify the Provision() interface to pass in the allowed topology information + // of the provisioned volume. volume, err = provisioner.Provision() opComplete(&err) if err != nil { + // Other places of failure has nothing to do with DynamicProvisioningScheduling, + // so just let controller retry in the next sync. We'll only call func + // rescheduleProvisioning here when the underlying provisioning actually failed. + ctrl.rescheduleProvisioning(claim) + strerr := fmt.Sprintf("Failed to provision volume with StorageClass %q: %v", storageClass.Name, err) glog.V(2).Infof("failed to provision volume for claim %q with StorageClass %q: %v", claimToClaimKey(claim), storageClass.Name, err) ctrl.eventRecorder.Event(claim, v1.EventTypeWarning, events.ProvisioningFailed, strerr) @@ -1521,6 +1535,29 @@ func (ctrl *PersistentVolumeController) provisionClaimOperation(claim *v1.Persis } } +// rescheduleProvisioning signal back to the scheduler to retry dynamic provisioning +// by removing the annSelectedNode annotation +func (ctrl *PersistentVolumeController) rescheduleProvisioning(claim *v1.PersistentVolumeClaim) { + if _, ok := claim.Annotations[annSelectedNode]; !ok { + // Provisioning not triggered by the scheduler, skip + return + } + + // The claim from method args can be pointing to watcher cache. We must not + // modify these, therefore create a copy. + newClaim := claim.DeepCopy() + delete(newClaim.Annotations, annSelectedNode) + // Try to update the PVC object + if _, err := ctrl.kubeClient.CoreV1().PersistentVolumeClaims(newClaim.Namespace).Update(newClaim); err != nil { + glog.V(4).Infof("Failed to delete annotation 'annSelectedNode' for PersistentVolumeClaim %q: %v", claimToClaimKey(newClaim), err) + return + } + if _, err := ctrl.storeClaimUpdate(newClaim); err != nil { + // We will get an "claim updated" event soon, this is not a big error + glog.V(4).Infof("Updating PersistentVolumeClaim %q: cannot update internal cache: %v", claimToClaimKey(newClaim), err) + } +} + // getProvisionedVolumeNameForClaim returns PV.Name for the provisioned volume. // The name must be unique. func (ctrl *PersistentVolumeController) getProvisionedVolumeNameForClaim(claim *v1.PersistentVolumeClaim) string { diff --git a/pkg/controller/volume/persistentvolume/pv_controller_test.go b/pkg/controller/volume/persistentvolume/pv_controller_test.go index 5454fe26ece..48b7c1d983b 100644 --- a/pkg/controller/volume/persistentvolume/pv_controller_test.go +++ b/pkg/controller/volume/persistentvolume/pv_controller_test.go @@ -312,8 +312,8 @@ func TestDelayBinding(t *testing.T) { } } - // When feature gate is disabled, should always be delayed - name := "feature-disabled" + // When volumeScheduling feature gate is disabled, should always be delayed + name := "volumeScheduling-feature-disabled" shouldDelay, err := ctrl.shouldDelayBinding(makePVCClass(&classWaitMode)) if err != nil { t.Errorf("Test %q returned error: %v", name, err) @@ -322,7 +322,7 @@ func TestDelayBinding(t *testing.T) { t.Errorf("Test %q returned true, expected false", name) } - // Enable feature gate + // Enable volumeScheduling feature gate utilfeature.DefaultFeatureGate.Set("VolumeScheduling=true") defer utilfeature.DefaultFeatureGate.Set("VolumeScheduling=false") @@ -338,4 +338,43 @@ func TestDelayBinding(t *testing.T) { t.Errorf("Test %q returned unexpected %v", name, test.shouldDelay) } } + + // When dynamicProvisioningScheduling feature gate is disabled, should be delayed, + // even if the pvc has selectedNode annotation. + provisionedClaim := makePVCClass(&classWaitMode) + provisionedClaim.Annotations = map[string]string{annSelectedNode: "node-name"} + name = "dynamicProvisioningScheduling-feature-disabled" + shouldDelay, err = ctrl.shouldDelayBinding(provisionedClaim) + if err != nil { + t.Errorf("Test %q returned error: %v", name, err) + } + if !shouldDelay { + t.Errorf("Test %q returned false, expected true", name) + } + + // Enable DynamicProvisioningScheduling feature gate + utilfeature.DefaultFeatureGate.Set("DynamicProvisioningScheduling=true") + defer utilfeature.DefaultFeatureGate.Set("DynamicProvisioningScheduling=false") + + // When the pvc does not have selectedNode annotation, should be delayed, + // even if dynamicProvisioningScheduling feature gate is enabled. + name = "dynamicProvisioningScheduling-feature-enabled, selectedNode-annotation-not-set" + shouldDelay, err = ctrl.shouldDelayBinding(makePVCClass(&classWaitMode)) + if err != nil { + t.Errorf("Test %q returned error: %v", name, err) + } + if !shouldDelay { + t.Errorf("Test %q returned false, expected true", name) + } + + // Should not be delayed when dynamicProvisioningScheduling feature gate is enabled, + // and the pvc has selectedNode annotation. + name = "dynamicProvisioningScheduling-feature-enabled, selectedNode-annotation-set" + shouldDelay, err = ctrl.shouldDelayBinding(provisionedClaim) + if err != nil { + t.Errorf("Test %q returned error: %v", name, err) + } + if shouldDelay { + t.Errorf("Test %q returned true, expected false", name) + } }