From 34c063cfe16792808890c482c0e79ed6625d5a81 Mon Sep 17 00:00:00 2001 From: markturansky Date: Wed, 2 Dec 2015 11:31:26 -0500 Subject: [PATCH] attempt recycling once, fail pv permanently --- .../persistentvolume_recycler_controller.go | 71 +++++++++------- ...rsistentvolume_recycler_controller_test.go | 82 +++++++++++++++++++ 2 files changed, 122 insertions(+), 31 deletions(-) create mode 100644 pkg/controller/persistentvolume/persistentvolume_recycler_controller_test.go diff --git a/pkg/controller/persistentvolume/persistentvolume_recycler_controller.go b/pkg/controller/persistentvolume/persistentvolume_recycler_controller.go index 0cd6de4085..8c28bd8f0b 100644 --- a/pkg/controller/persistentvolume/persistentvolume_recycler_controller.go +++ b/pkg/controller/persistentvolume/persistentvolume_recycler_controller.go @@ -130,23 +130,26 @@ func (recycler *PersistentVolumeRecycler) handleRecycle(pv *api.PersistentVolume spec := volume.NewSpecFromPersistentVolume(pv, false) plugin, err := recycler.pluginMgr.FindRecyclablePluginBySpec(spec) if err != nil { - return fmt.Errorf("Could not find recyclable volume plugin for spec: %+v", err) - } - volRecycler, err := plugin.NewRecycler(spec) - if err != nil { - return fmt.Errorf("Could not obtain Recycler for spec: %+v", err) - } - // blocks until completion - err = volRecycler.Recycle() - if err != nil { - glog.Errorf("PersistentVolume[%s] failed recycling: %+v", pv.Name, err) - pv.Status.Message = fmt.Sprintf("Recycling error: %s", err) nextPhase = api.VolumeFailed - } else { - glog.V(5).Infof("PersistentVolume[%s] successfully recycled\n", pv.Name) - nextPhase = api.VolumePending + pv.Status.Message = fmt.Sprintf("%v", err) + } + + // an error above means a suitable plugin for this volume was not found. + // we don't need to attempt recycling when plugin is nil, but we do need to persist the next/failed phase + // of the volume so that subsequent syncs won't attempt recycling through this handler func. + if plugin != nil { + volRecycler, err := plugin.NewRecycler(spec) if err != nil { - glog.Errorf("Error updating pv.Status: %+v", err) + return fmt.Errorf("Could not obtain Recycler for spec: %#v error: %v", spec, err) + } + // blocks until completion + if err := volRecycler.Recycle(); err != nil { + glog.Errorf("PersistentVolume[%s] failed recycling: %+v", pv.Name, err) + pv.Status.Message = fmt.Sprintf("Recycling error: %s", err) + nextPhase = api.VolumeFailed + } else { + glog.V(5).Infof("PersistentVolume[%s] successfully recycled\n", pv.Name) + nextPhase = api.VolumePending } } @@ -172,24 +175,30 @@ func (recycler *PersistentVolumeRecycler) handleDelete(pv *api.PersistentVolume) spec := volume.NewSpecFromPersistentVolume(pv, false) plugin, err := recycler.pluginMgr.FindDeletablePluginBySpec(spec) if err != nil { - return fmt.Errorf("Could not find deletable volume plugin for spec: %+v", err) - } - deleter, err := plugin.NewDeleter(spec) - if err != nil { - return fmt.Errorf("could not obtain Deleter for spec: %+v", err) - } - // blocks until completion - err = deleter.Delete() - if err != nil { - glog.Errorf("PersistentVolume[%s] failed deletion: %+v", pv.Name, err) - pv.Status.Message = fmt.Sprintf("Deletion error: %s", err) nextPhase = api.VolumeFailed - } else { - glog.V(5).Infof("PersistentVolume[%s] successfully deleted through plugin\n", pv.Name) - // after successful deletion through the plugin, we can also remove the PV from the cluster - err = recycler.client.DeletePersistentVolume(pv) + pv.Status.Message = fmt.Sprintf("%v", err) + } + + // an error above means a suitable plugin for this volume was not found. + // we don't need to attempt deleting when plugin is nil, but we do need to persist the next/failed phase + // of the volume so that subsequent syncs won't attempt deletion through this handler func. + if plugin != nil { + deleter, err := plugin.NewDeleter(spec) if err != nil { - return fmt.Errorf("error deleting persistent volume: %+v", err) + return fmt.Errorf("Could not obtain Deleter for spec: %#v error: %v", spec, err) + } + // blocks until completion + err = deleter.Delete() + if err != nil { + glog.Errorf("PersistentVolume[%s] failed deletion: %+v", pv.Name, err) + pv.Status.Message = fmt.Sprintf("Deletion error: %s", err) + nextPhase = api.VolumeFailed + } else { + glog.V(5).Infof("PersistentVolume[%s] successfully deleted through plugin\n", pv.Name) + // after successful deletion through the plugin, we can also remove the PV from the cluster + if err := recycler.client.DeletePersistentVolume(pv); err != nil { + return fmt.Errorf("error deleting persistent volume: %+v", err) + } } } diff --git a/pkg/controller/persistentvolume/persistentvolume_recycler_controller_test.go b/pkg/controller/persistentvolume/persistentvolume_recycler_controller_test.go new file mode 100644 index 0000000000..0aa22e6f1c --- /dev/null +++ b/pkg/controller/persistentvolume/persistentvolume_recycler_controller_test.go @@ -0,0 +1,82 @@ +/* +Copyright 2015 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 ( + "testing" + + "k8s.io/kubernetes/pkg/api" + "k8s.io/kubernetes/pkg/api/resource" + "k8s.io/kubernetes/pkg/client/unversioned/testclient" + "k8s.io/kubernetes/pkg/volume" +) + +func TestFailedRecycling(t *testing.T) { + pv := &api.PersistentVolume{ + Spec: api.PersistentVolumeSpec{ + AccessModes: []api.PersistentVolumeAccessMode{api.ReadWriteOnce}, + Capacity: api.ResourceList{ + api.ResourceName(api.ResourceStorage): resource.MustParse("8Gi"), + }, + PersistentVolumeSource: api.PersistentVolumeSource{ + HostPath: &api.HostPathVolumeSource{ + Path: "/tmp/data02", + }, + }, + PersistentVolumeReclaimPolicy: api.PersistentVolumeReclaimRecycle, + ClaimRef: &api.ObjectReference{ + Name: "foo", + Namespace: "bar", + }, + }, + Status: api.PersistentVolumeStatus{ + Phase: api.VolumeReleased, + }, + } + + mockClient := &mockBinderClient{ + volume: pv, + } + + // no Init called for pluginMgr and no plugins are available. Volume should fail recycling. + plugMgr := volume.VolumePluginMgr{} + + recycler := &PersistentVolumeRecycler{ + kubeClient: &testclient.Fake{}, + client: mockClient, + pluginMgr: plugMgr, + } + + err := recycler.reclaimVolume(pv) + if err != nil { + t.Errorf("Unexpected non-nil error: %v", err) + } + + if mockClient.volume.Status.Phase != api.VolumeFailed { + t.Errorf("Expected %s but got %s", api.VolumeFailed, mockClient.volume.Status.Phase) + } + + pv.Spec.PersistentVolumeReclaimPolicy = api.PersistentVolumeReclaimDelete + err = recycler.reclaimVolume(pv) + if err != nil { + t.Errorf("Unexpected non-nil error: %v", err) + } + + if mockClient.volume.Status.Phase != api.VolumeFailed { + t.Errorf("Expected %s but got %s", api.VolumeFailed, mockClient.volume.Status.Phase) + } +}