delete: Implement Deleter

pkg/controller/persistentvolume/persistentvolume_controller.go

@@ -1120,25 +1120,55 @@ func (ctrl *PersistentVolumeController) recycleVolumeOperation(arg interface{})
 // deleteVolumeOperation deletes a volume. This method is running in standalone
 // goroutine and already has all necessary locks.
 func (ctrl *PersistentVolumeController) deleteVolumeOperation(arg interface{}) {
-	volume := arg.(*api.PersistentVolume)
+	volume, ok := arg.(*api.PersistentVolume)
-	glog.V(4).Infof("deleteVolumeOperation [%s] started", volume.Name)
+	if !ok {
-	/*else if pv.Spec.ReclaimPolicy == "Delete" {
+		glog.Errorf("Cannot convert deleteVolumeOperation argument to volume, got %+v", arg)
-	plugin := findDeleterPluginForPV(pv)
+		return
-	if plugin != nil {
-		// maintain a map with the current deleter goroutines that are running
-		// if the key is already present in the map, return
-		//
-		// launch the goroutine that:
-		// 1. deletes the storage asset
-		// 2. deletes the PV API object
-		// 3. deletes itself from the map when it's done
-	} else {
-		// make an event calling out that no deleter was configured
-		// mark the PV as failed
-		// NB: external provisioners/deleters are currently not
-		// considered.
 	}
-	*/
+	glog.V(4).Infof("deleteVolumeOperation [%s] started", volume.Name)
+
+	// This method may have been waiting for a volume lock for some time.
+	// Previous deleteVolumeOperation might just have saved an updated version, so
+	// read current volume state now.
+	newVolume, err := ctrl.kubeClient.Core().PersistentVolumes().Get(volume.Name)
+	if err != nil {
+		glog.V(3).Infof("error reading peristent volume %q: %v", volume.Name, err)
+		return
+	}
+	needsReclaim, err := ctrl.isVolumeReleased(newVolume)
+	if err != nil {
+		glog.V(3).Infof("error reading claim for volume %q: %v", volume.Name, err)
+		return
+	}
+	if !needsReclaim {
+		glog.V(3).Infof("volume %q no longer needs deletion, skipping", volume.Name)
+		return
+	}
+
+	if err = ctrl.doDeleteVolume(volume); 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 {
+			glog.V(4).Infof("deleteVolumeOperation [%s]: failed to mark volume as failed: %v", volume.Name, err)
+			// Save failed, retry on the next deletion attempt
+			return
+		}
+		// Despite the volume being Failed, the controller will retry deleting
+		// the volume in every syncVolume() call.
+		return
+	}
+
+	glog.V(4).Infof("deleteVolumeOperation [%s]: success", volume.Name)
+	// Delete the volume
+	if err = ctrl.kubeClient.Core().PersistentVolumes().Delete(volume.Name, nil); err != nil {
+		// Oops, could not delete the volume and therefore the controller will
+		// try to delete the volume again on next update. We _could_ maintain a
+		// cache of "recently deleted volumes" and avoid unnecessary deletion,
+		// this is left out as future optimization.
+		glog.V(3).Infof("failed to delete volume %q from database: %v", volume.Name, err)
+		return
+	}
+	return
 }
 
 // isVolumeReleased returns true if given volume is released and can be recycled
@@ -1183,6 +1213,34 @@ 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 {
+	glog.V(4).Infof("doDeleteVolume [%s]", volume.Name)
+	// Find a plugin.
+	spec := vol.NewSpecFromPersistentVolume(volume, false)
+	plugin, err := ctrl.recyclePluginMgr.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)
+	}
+
+	// Plugin found
+	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)
+	}
+
+	if err = deleter.Delete(); err != nil {
+		// Deleter failed
+		return fmt.Errorf("Delete of volume %q failed: %v", volume.Name, err)
+	}
+
+	glog.V(2).Infof("volume %q deleted", volume.Name)
+	return nil
+}
+
 // scheduleOperation starts given asynchronous operation on given volume. It
 // makes sure the operation is already not running.
 func (ctrl *PersistentVolumeController) scheduleOperation(operationName string, operation func(arg interface{}), arg interface{}) {

pkg/controller/persistentvolume/persistentvolume_delete_test.go

@@ -0,0 +1,169 @@
+/*
+Copyright 2016 The Kubernetes Authors All rights reserved.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package persistentvolume
+
+import (
+	"errors"
+	"testing"
+
+	"k8s.io/kubernetes/pkg/api"
+)
+
+// Test single call to syncVolume, expecting recycling to happen.
+// 1. Fill in the controller with initial data
+// 2. Call the syncVolume *once*.
+// 3. Compare resulting volumes with expected volumes.
+func TestDeleteSync(t *testing.T) {
+	tests := []controllerTest{
+		{
+			// delete volume bound by controller
+			"8-1 - successful delete",
+			newVolumeArray("volume8-1", "1Gi", "uid8-1", "claim8-1", api.VolumeBound, api.PersistentVolumeReclaimDelete, annBoundByController),
+			novolumes,
+			noclaims,
+			noclaims,
+			noevents,
+			// Inject deleter into the controller and call syncVolume. The
+			// deleter simulates one delete() call that succeeds.
+			wrapTestWithControllerConfig(operationDelete, []error{nil}, testSyncVolume),
+		},
+		{
+			// delete volume bound by user
+			"8-2 - successful delete with prebound volume",
+			newVolumeArray("volume8-2", "1Gi", "uid8-2", "claim8-2", api.VolumeBound, api.PersistentVolumeReclaimDelete),
+			novolumes,
+			noclaims,
+			noclaims,
+			noevents,
+			// Inject deleter into the controller and call syncVolume. The
+			// deleter simulates one delete() call that succeeds.
+			wrapTestWithControllerConfig(operationDelete, []error{nil}, testSyncVolume),
+		},
+		{
+			// delete failure - plugin not found
+			"8-3 - plugin not found",
+			newVolumeArray("volume8-3", "1Gi", "uid8-3", "claim8-3", api.VolumeBound, api.PersistentVolumeReclaimDelete),
+			newVolumeArray("volume8-3", "1Gi", "uid8-3", "claim8-3", api.VolumeFailed, api.PersistentVolumeReclaimDelete),
+			noclaims,
+			noclaims,
+			[]string{"Warning VolumeFailedDelete"}, testSyncVolume,
+		},
+		{
+			// delete failure - newDeleter returns error
+			"8-4 - newDeleter returns error",
+			newVolumeArray("volume8-4", "1Gi", "uid8-4", "claim8-4", api.VolumeBound, api.PersistentVolumeReclaimDelete),
+			newVolumeArray("volume8-4", "1Gi", "uid8-4", "claim8-4", api.VolumeFailed, api.PersistentVolumeReclaimDelete),
+			noclaims,
+			noclaims,
+			[]string{"Warning VolumeFailedDelete"},
+			wrapTestWithControllerConfig(operationDelete, []error{}, testSyncVolume),
+		},
+		{
+			// delete failure - delete() returns error
+			"8-5 - delete returns error",
+			newVolumeArray("volume8-5", "1Gi", "uid8-5", "claim8-5", api.VolumeBound, api.PersistentVolumeReclaimDelete),
+			newVolumeArray("volume8-5", "1Gi", "uid8-5", "claim8-5", api.VolumeFailed, api.PersistentVolumeReclaimDelete),
+			noclaims,
+			noclaims,
+			[]string{"Warning VolumeFailedDelete"},
+			wrapTestWithControllerConfig(operationDelete, []error{errors.New("Mock delete error")}, testSyncVolume),
+		},
+		{
+			// delete success(?) - volume is deleted before doDelete() starts
+			"8-6 - volume is deleted before deleting",
+			newVolumeArray("volume8-6", "1Gi", "uid8-6", "claim8-6", api.VolumeBound, api.PersistentVolumeReclaimDelete),
+			novolumes,
+			noclaims,
+			noclaims,
+			noevents,
+			wrapTestWithInjectedOperation(wrapTestWithControllerConfig(operationDelete, []error{}, testSyncVolume), func(ctrl *PersistentVolumeController, reactor *volumeReactor) {
+				// Delete the volume before delete operation starts
+				reactor.lock.Lock()
+				delete(reactor.volumes, "volume8-6")
+				reactor.lock.Unlock()
+			}),
+		},
+		{
+			// delete success(?) - volume is bound just at the time doDelete()
+			// starts. This simulates "volume no longer needs recycling,
+			// skipping".
+			"8-7 - volume is bound before deleting",
+			newVolumeArray("volume8-7", "1Gi", "uid8-7", "claim8-7", api.VolumeBound, api.PersistentVolumeReclaimDelete, annBoundByController),
+			newVolumeArray("volume8-7", "1Gi", "uid8-7", "claim8-7", api.VolumeBound, api.PersistentVolumeReclaimDelete, annBoundByController),
+			noclaims,
+			newClaimArray("claim8-7", "uid8-7", "10Gi", "volume8-7", api.ClaimBound),
+			noevents,
+			wrapTestWithInjectedOperation(wrapTestWithControllerConfig(operationDelete, []error{}, testSyncVolume), func(ctrl *PersistentVolumeController, reactor *volumeReactor) {
+				reactor.lock.Lock()
+				defer reactor.lock.Unlock()
+				// Bind the volume to ressurected claim (this should never
+				// happen)
+				claim := newClaim("claim8-7", "uid8-7", "10Gi", "volume8-7", api.ClaimBound)
+				reactor.claims[claim.Name] = claim
+				ctrl.claims.Add(claim)
+				volume := reactor.volumes["volume8-7"]
+				volume.Status.Phase = api.VolumeBound
+			}),
+		},
+		{
+			// delete success - volume bound by user is deleted, while a new
+			// claim is created with another UID.
+			"8-9 - prebound volume is deleted while the claim exists",
+			newVolumeArray("volume8-9", "1Gi", "uid8-9", "claim8-9", api.VolumeBound, api.PersistentVolumeReclaimDelete),
+			novolumes,
+			newClaimArray("claim8-9", "uid8-9-x", "10Gi", "", api.ClaimPending),
+			newClaimArray("claim8-9", "uid8-9-x", "10Gi", "", api.ClaimPending),
+			noevents,
+			// Inject deleter into the controller and call syncVolume. The
+			// deleter simulates one delete() call that succeeds.
+			wrapTestWithControllerConfig(operationDelete, []error{nil}, testSyncVolume),
+		},
+	}
+	runSyncTests(t, tests)
+}
+
+// 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.
+// 1. Call controllerTest.testCall() once as in TestSync()
+// 2. For all volumes/claims changed by previous syncVolume/syncClaim calls,
+//    call appropriate syncVolume/syncClaim (simulating "volume/claim changed"
+//    events). Go to 2. if these calls change anything.
+// 3. When all changes are processed and no new changes were made, call
+//    syncVolume/syncClaim on all volumes/claims (simulating "periodic sync").
+// 4. If some changes were done by step 3., go to 2. (simulation of
+//    "volume/claim updated" events, eventually performing step 3. again)
+// 5. When 3. does not do any changes, finish the tests and compare final set
+//    of volumes/claims with expected claims/volumes and report differences.
+// Some limit of calls in enforced to prevent endless loops.
+func TestDeleteMultiSync(t *testing.T) {
+	tests := []controllerTest{
+		{
+			// delete failure - delete returns error. The controller should
+			// try again.
+			"9-1 - delete returns error",
+			newVolumeArray("volume9-1", "1Gi", "uid9-1", "claim9-1", api.VolumeBound, api.PersistentVolumeReclaimDelete),
+			novolumes,
+			noclaims,
+			noclaims,
+			[]string{"Warning VolumeFailedDelete"},
+			wrapTestWithControllerConfig(operationDelete, []error{errors.New("Mock delete error"), nil}, testSyncVolume),
+		},
+	}
+
+	runMultisyncTests(t, tests)
+}

pkg/controller/persistentvolume/persistentvolume_framework_test.go

@@ -198,7 +198,30 @@ func (r *volumeReactor) React(action core.Action) (handled bool, ret runtime.Obj
 			glog.V(4).Infof("GetVolume: volume %s not found", name)
 			return true, nil, fmt.Errorf("Cannot find volume %s", name)
 		}
+
+	case action.Matches("delete", "persistentvolumes"):
+		name := action.(core.DeleteAction).GetName()
+		glog.V(4).Infof("deleted volume %s", name)
+		_, found := r.volumes[name]
+		if found {
+			delete(r.volumes, name)
+			return true, nil, nil
+		} else {
+			return true, nil, fmt.Errorf("Cannot delete volume %s: not found", name)
 		}
+
+	case action.Matches("delete", "persistentvolumeclaims"):
+		name := action.(core.DeleteAction).GetName()
+		glog.V(4).Infof("deleted claim %s", name)
+		_, found := r.volumes[name]
+		if found {
+			delete(r.claims, name)
+			return true, nil, nil
+		} else {
+			return true, nil, fmt.Errorf("Cannot delete claim %s: not found", name)
+		}
+	}
+
 	return false, nil, nil
 }