Do not report warning event when an unknown deleter is requested
When Kubernetes does not have a plugin to delete a PV it should wait for either external deleter or storage admin to delete the volume instead of throwing an error. Related to #32077
This commit is contained in:
		| @@ -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 | ||||
| } | ||||
|   | ||||
| @@ -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{}) | ||||
| } | ||||
|   | ||||
		Reference in New Issue
	
	Block a user
	 Jan Safranek
					Jan Safranek