diff --git a/pkg/controller/volume/persistentvolume/controller.go b/pkg/controller/volume/persistentvolume/controller.go index ce85b498498..f08e79fb0a4 100644 --- a/pkg/controller/volume/persistentvolume/controller.go +++ b/pkg/controller/volume/persistentvolume/controller.go @@ -1072,7 +1072,8 @@ func (ctrl *PersistentVolumeController) deleteVolumeOperation(arg interface{}) { return } - if err = ctrl.doDeleteVolume(volume); err != nil { + deleted, err := ctrl.doDeleteVolume(volume) + if err != nil { // Delete failed, update the volume and emit an event. glog.V(3).Infof("deletion of volume %q failed: %v", volume.Name, err) if _, err = ctrl.updateVolumePhaseWithEvent(volume, api.VolumeFailed, api.EventTypeWarning, "VolumeFailedDelete", err.Error()); err != nil { @@ -1084,6 +1085,10 @@ func (ctrl *PersistentVolumeController) deleteVolumeOperation(arg interface{}) { // the volume in every syncVolume() call. return } + if !deleted { + // The volume waits for deletion by an external plugin. Do nothing. + return + } glog.V(4).Infof("deleteVolumeOperation [%s]: success", volume.Name) // Delete the volume @@ -1140,51 +1145,40 @@ func (ctrl *PersistentVolumeController) isVolumeReleased(volume *api.PersistentV return true, nil } -// doDeleteVolume finds appropriate delete plugin and deletes given volume -// (it will be re-used in future provisioner error cases). -func (ctrl *PersistentVolumeController) doDeleteVolume(volume *api.PersistentVolume) error { +// doDeleteVolume finds appropriate delete plugin and deletes given volume. It +// returns 'true', when the volume was deleted and 'false' when the volume +// cannot be deleted because of the deleter is external. No error should be +// reported in this case. +func (ctrl *PersistentVolumeController) doDeleteVolume(volume *api.PersistentVolume) (bool, error) { glog.V(4).Infof("doDeleteVolume [%s]", volume.Name) var err error - // Find a plugin. Try to find the same plugin that provisioned the volume - var plugin vol.DeletableVolumePlugin - if hasAnnotation(volume.ObjectMeta, annDynamicallyProvisioned) { - provisionPluginName := volume.Annotations[annDynamicallyProvisioned] - if provisionPluginName != "" { - plugin, err = ctrl.volumePluginMgr.FindDeletablePluginByName(provisionPluginName) - if err != nil { - glog.V(3).Infof("did not find a deleter plugin %q for volume %q: %v, will try to find a generic one", - provisionPluginName, volume.Name, err) - } - } + plugin, err := ctrl.findDeletablePlugin(volume) + if err != nil { + return false, err } - - spec := vol.NewSpecFromPersistentVolume(volume, false) if plugin == nil { - // The plugin that provisioned the volume was not found or the volume - // was not dynamically provisioned. Try to find a plugin by spec. - plugin, err = ctrl.volumePluginMgr.FindDeletablePluginBySpec(spec) - if err != nil { - // No deleter found. Emit an event and mark the volume Failed. - return fmt.Errorf("Error getting deleter volume plugin for volume %q: %v", volume.Name, err) - } + // External deleter is requested, do nothing + glog.V(3).Infof("external deleter for volume %q requested, ignoring", volume.Name) + return false, nil } - glog.V(5).Infof("found a deleter plugin %q for volume %q", plugin.GetPluginName(), volume.Name) // Plugin found + glog.V(5).Infof("found a deleter plugin %q for volume %q", plugin.GetPluginName(), volume.Name) + spec := vol.NewSpecFromPersistentVolume(volume, false) deleter, err := plugin.NewDeleter(spec) if err != nil { // Cannot create deleter - return fmt.Errorf("Failed to create deleter for volume %q: %v", volume.Name, err) + return false, fmt.Errorf("Failed to create deleter for volume %q: %v", volume.Name, err) } if err = deleter.Delete(); err != nil { // Deleter failed - return fmt.Errorf("Delete of volume %q failed: %v", volume.Name, err) + return false, fmt.Errorf("Delete of volume %q failed: %v", volume.Name, err) } glog.V(2).Infof("volume %q deleted", volume.Name) - return nil + return true, nil } // provisionClaim starts new asynchronous operation to provision a claim if @@ -1339,11 +1333,19 @@ func (ctrl *PersistentVolumeController) provisionClaimOperation(claimObj interfa ctrl.eventRecorder.Event(claim, api.EventTypeWarning, "ProvisioningFailed", strerr) for i := 0; i < ctrl.createProvisionedPVRetryCount; i++ { - if err = ctrl.doDeleteVolume(volume); err == nil { + deleted, err := ctrl.doDeleteVolume(volume) + if err == nil && deleted { // Delete succeeded glog.V(4).Infof("provisionClaimOperation [%s]: cleaning volume %s succeeded", claimToClaimKey(claim), volume.Name) break } + if !deleted { + // This is unreachable code, the volume was provisioned by an + // internal plugin and therefore there MUST be an internal + // plugin that deletes it. + glog.Errorf("Error finding internal deleter for volume plugin %q", plugin.GetPluginName()) + break + } // Delete failed, try again after a while. glog.V(3).Infof("failed to delete volume %q: %v", volume.Name, err) time.Sleep(ctrl.createProvisionedPVInterval) @@ -1453,3 +1455,34 @@ func (ctrl *PersistentVolumeController) findAlphaProvisionablePlugin() (vol.Prov glog.V(4).Infof("using alpha provisioner %s", ctrl.alphaProvisioner.GetPluginName()) return ctrl.alphaProvisioner, storageClass, nil } + +// findDeletablePlugin finds a deleter plugin for a given volume. It returns +// either the deleter plugin or nil when an external deleter is requested. +func (ctrl *PersistentVolumeController) findDeletablePlugin(volume *api.PersistentVolume) (vol.DeletableVolumePlugin, error) { + // Find a plugin. Try to find the same plugin that provisioned the volume + var plugin vol.DeletableVolumePlugin + if hasAnnotation(volume.ObjectMeta, annDynamicallyProvisioned) { + provisionPluginName := volume.Annotations[annDynamicallyProvisioned] + if provisionPluginName != "" { + plugin, err := ctrl.volumePluginMgr.FindDeletablePluginByName(provisionPluginName) + if err != nil { + if !strings.HasPrefix(provisionPluginName, "kubernetes.io/") { + // External provisioner is requested, do not report error + return nil, nil + } + return nil, err + } + return plugin, nil + } + } + + // The plugin that provisioned the volume was not found or the volume + // was not dynamically provisioned. Try to find a plugin by spec. + spec := vol.NewSpecFromPersistentVolume(volume, false) + plugin, err := ctrl.volumePluginMgr.FindDeletablePluginBySpec(spec) + if err != nil { + // No deleter found. Emit an event and mark the volume Failed. + return nil, fmt.Errorf("Error getting deleter volume plugin for volume %q: %v", volume.Name, err) + } + return plugin, nil +} diff --git a/pkg/controller/volume/persistentvolume/delete_test.go b/pkg/controller/volume/persistentvolume/delete_test.go index 0accc96522f..567ae739e92 100644 --- a/pkg/controller/volume/persistentvolume/delete_test.go +++ b/pkg/controller/volume/persistentvolume/delete_test.go @@ -133,6 +133,21 @@ func TestDeleteSync(t *testing.T) { // deleter simulates one delete() call that succeeds. wrapTestWithReclaimCalls(operationDelete, []error{nil}, testSyncVolume), }, + { + // PV requires external deleter + "8-10 - external deleter", + newVolumeArray("volume8-10", "1Gi", "uid10-1", "claim10-1", api.VolumeBound, api.PersistentVolumeReclaimDelete, annBoundByController), + newVolumeArray("volume8-10", "1Gi", "uid10-1", "claim10-1", api.VolumeReleased, api.PersistentVolumeReclaimDelete, annBoundByController), + noclaims, + noclaims, + noevents, noerrors, + func(ctrl *PersistentVolumeController, reactor *volumeReactor, test controllerTest) error { + // Inject external deleter annotation + test.initialVolumes[0].Annotations[annDynamicallyProvisioned] = "external.io/test" + test.expectedVolumes[0].Annotations[annDynamicallyProvisioned] = "external.io/test" + return testSyncVolume(ctrl, reactor, test) + }, + }, } runSyncTests(t, tests, []*storage.StorageClass{}) }