Merge pull request #24331 from jsafrane/devel/refactor-binder

cmd/kube-controller-manager/app/controllermanager.go

@@ -373,38 +373,23 @@ func StartControllers(s *options.CMServer, kubeClient *client.Client, kubeconfig
 		}
 	}
 
-	volumePlugins := ProbeRecyclableVolumePlugins(s.VolumeConfiguration)
 	provisioner, err := NewVolumeProvisioner(cloud, s.VolumeConfiguration)
 	if err != nil {
 		glog.Fatal("A Provisioner could not be created, but one was expected. Provisioning will not work. This functionality is considered an early Alpha version.")
 	}
 
-	pvclaimBinder := persistentvolumecontroller.NewPersistentVolumeClaimBinder(clientset.NewForConfigOrDie(restclient.AddUserAgent(kubeconfig, "persistent-volume-binder")), s.PVClaimBinderSyncPeriod.Duration)
-	pvclaimBinder.Run()
-	time.Sleep(wait.Jitter(s.ControllerStartInterval.Duration, ControllerStartJitter))
-
-	pvRecycler, err := persistentvolumecontroller.NewPersistentVolumeRecycler(
-		clientset.NewForConfigOrDie(restclient.AddUserAgent(kubeconfig, "persistent-volume-recycler")),
+	volumeController := persistentvolumecontroller.NewPersistentVolumeController(
+		clientset.NewForConfigOrDie(restclient.AddUserAgent(kubeconfig, "persistent-volume-binder")),
 		s.PVClaimBinderSyncPeriod.Duration,
-		int(s.VolumeConfiguration.PersistentVolumeRecyclerConfiguration.MaximumRetry),
+		provisioner,
 		ProbeRecyclableVolumePlugins(s.VolumeConfiguration),
 		cloud,
+		s.ClusterName,
+		nil, nil, nil,
 	)
-	if err != nil {
-		glog.Fatalf("Failed to start persistent volume recycler: %+v", err)
-	}
-	pvRecycler.Run()
+	volumeController.Run()
 	time.Sleep(wait.Jitter(s.ControllerStartInterval.Duration, ControllerStartJitter))
 
-	if provisioner != nil {
-		pvController, err := persistentvolumecontroller.NewPersistentVolumeProvisionerController(persistentvolumecontroller.NewControllerClient(clientset.NewForConfigOrDie(restclient.AddUserAgent(kubeconfig, "persistent-volume-provisioner"))), s.PVClaimBinderSyncPeriod.Duration, s.ClusterName, volumePlugins, provisioner, cloud)
-		if err != nil {
-			glog.Fatalf("Failed to start persistent volume provisioner controller: %+v", err)
-		}
-		pvController.Run()
-		time.Sleep(wait.Jitter(s.ControllerStartInterval.Duration, ControllerStartJitter))
-	}
-
 	go volume.NewAttachDetachController(clientset.NewForConfigOrDie(restclient.AddUserAgent(kubeconfig, "attachdetach-controller")), podInformer, nodeInformer, ResyncPeriod(s)()).
 		Run(wait.NeverStop)
 	time.Sleep(wait.Jitter(s.ControllerStartInterval.Duration, ControllerStartJitter))

contrib/mesos/pkg/controllermanager/controllermanager.go

@@ -271,37 +271,23 @@ func (s *CMServer) Run(_ []string) error {
 		}
 	}
 
-	volumePlugins := kubecontrollermanager.ProbeRecyclableVolumePlugins(s.VolumeConfiguration)
 	provisioner, err := kubecontrollermanager.NewVolumeProvisioner(cloud, s.VolumeConfiguration)
 	if err != nil {
 		glog.Fatal("A Provisioner could not be created, but one was expected. Provisioning will not work. This functionality is considered an early Alpha version.")
 	}
 
-	pvclaimBinder := persistentvolumecontroller.NewPersistentVolumeClaimBinder(
+	volumeController := persistentvolumecontroller.NewPersistentVolumeController(
 		clientset.NewForConfigOrDie(restclient.AddUserAgent(kubeconfig, "persistent-volume-binder")),
 		s.PVClaimBinderSyncPeriod.Duration,
-	)
-	pvclaimBinder.Run()
-
-	pvRecycler, err := persistentvolumecontroller.NewPersistentVolumeRecycler(
-		clientset.NewForConfigOrDie(restclient.AddUserAgent(kubeconfig, "persistent-volume-recycler")),
-		s.PVClaimBinderSyncPeriod.Duration,
-		int(s.VolumeConfiguration.PersistentVolumeRecyclerConfiguration.MaximumRetry),
+		provisioner,
 		kubecontrollermanager.ProbeRecyclableVolumePlugins(s.VolumeConfiguration),
 		cloud,
+		s.ClusterName,
+		nil,
+		nil,
+		nil,
 	)
-	if err != nil {
-		glog.Fatalf("Failed to start persistent volume recycler: %+v", err)
-	}
-	pvRecycler.Run()
-
-	if provisioner != nil {
-		pvController, err := persistentvolumecontroller.NewPersistentVolumeProvisionerController(persistentvolumecontroller.NewControllerClient(clientset.NewForConfigOrDie(restclient.AddUserAgent(kubeconfig, "persistent-volume-controller"))), s.PVClaimBinderSyncPeriod.Duration, s.ClusterName, volumePlugins, provisioner, cloud)
-		if err != nil {
-			glog.Fatalf("Failed to start persistent volume provisioner controller: %+v", err)
-		}
-		pvController.Run()
-	}
+	volumeController.Run()
 
 	var rootCA []byte
 

pkg/api/testing/fuzzer.go

@@ -340,7 +340,7 @@ func FuzzerFor(t *testing.T, version unversioned.GroupVersion, src rand.Source)
 		},
 		func(pvc *api.PersistentVolumeClaim, c fuzz.Continue) {
 			c.FuzzNoCustom(pvc) // fuzz self without calling this function again
-			types := []api.PersistentVolumeClaimPhase{api.ClaimBound, api.ClaimPending}
+			types := []api.PersistentVolumeClaimPhase{api.ClaimBound, api.ClaimPending, api.ClaimLost}
 			pvc.Status.Phase = types[c.Rand.Intn(len(types))]
 		},
 		func(s *api.NamespaceSpec, c fuzz.Continue) {

pkg/api/types.go

@@ -411,6 +411,10 @@ const (
 	ClaimPending PersistentVolumeClaimPhase = "Pending"
 	// used for PersistentVolumeClaims that are bound
 	ClaimBound PersistentVolumeClaimPhase = "Bound"
+	// used for PersistentVolumeClaims that lost their underlying
+	// PersistentVolume. The claim was bound to a PersistentVolume and this
+	// volume does not exist any longer and all data on it was lost.
+	ClaimLost PersistentVolumeClaimPhase = "Lost"
 )
 
 // Represents a host path mapped into a pod.

pkg/api/v1/types.go

@@ -506,6 +506,10 @@ const (
 	ClaimPending PersistentVolumeClaimPhase = "Pending"
 	// used for PersistentVolumeClaims that are bound
 	ClaimBound PersistentVolumeClaimPhase = "Bound"
+	// used for PersistentVolumeClaims that lost their underlying
+	// PersistentVolume. The claim was bound to a PersistentVolume and this
+	// volume does not exist any longer and all data on it was lost.
+	ClaimLost PersistentVolumeClaimPhase = "Lost"
 )
 
 // Represents a host path mapped into a pod.

pkg/controller/persistentvolume/controller_test.go

+/*
+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 (
+	"testing"
+	"time"
+
+	"github.com/golang/glog"
+	"k8s.io/kubernetes/pkg/api"
+	"k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset/fake"
+	"k8s.io/kubernetes/pkg/controller/framework"
+	"k8s.io/kubernetes/pkg/conversion"
+)
+
+// Test the real controller methods (add/update/delete claim/volume) with
+// a fake API server.
+// There is no controller API to 'initiate syncAll now', therefore these tests
+// can't reliably simulate periodic sync of volumes/claims - it would be
+// either very timing-sensitive or slow to wait for real periodic sync.
+func TestControllerSync(t *testing.T) {
+	tests := []controllerTest{
+		// [Unit test set 5] - controller tests.
+		// We test the controller as if
+		// it was connected to real API server, i.e. we call add/update/delete
+		// Claim/Volume methods. Also, all changes to volumes and claims are
+		// sent to add/update/delete Claim/Volume as real controller would do.
+		{
+			// addVolume gets a new volume. Check it's marked as Available and
+			// that it's not bound to any claim - we bind volumes on periodic
+			// syncClaim, not on addVolume.
+			"5-1 - addVolume",
+			novolumes, /* added in testCall below */
+			newVolumeArray("volume5-1", "10Gi", "", "", api.VolumeAvailable, api.PersistentVolumeReclaimRetain),
+			newClaimArray("claim5-1", "uid5-1", "1Gi", "", api.ClaimPending),
+			newClaimArray("claim5-1", "uid5-1", "1Gi", "", api.ClaimPending),
+			noevents, noerrors,
+			// Custom test function that generates an add event
+			func(ctrl *PersistentVolumeController, reactor *volumeReactor, test controllerTest) error {
+				volume := newVolume("volume5-1", "10Gi", "", "", api.VolumePending, api.PersistentVolumeReclaimRetain)
+				reactor.volumes[volume.Name] = volume
+				reactor.volumeSource.Add(volume)
+				return nil
+			},
+		},
+		{
+			// addClaim gets a new claim. Check it's bound to a volume.
+			"5-2 - complete bind",
+			newVolumeArray("volume5-2", "10Gi", "", "", api.VolumeAvailable, api.PersistentVolumeReclaimRetain),
+			newVolumeArray("volume5-2", "10Gi", "uid5-2", "claim5-2", api.VolumeBound, api.PersistentVolumeReclaimRetain, annBoundByController),
+			noclaims, /* added in testAddClaim5_2 */
+			newClaimArray("claim5-2", "uid5-2", "1Gi", "volume5-2", api.ClaimBound, annBoundByController, annBindCompleted),
+			noevents, noerrors,
+			// Custom test function that generates an add event
+			func(ctrl *PersistentVolumeController, reactor *volumeReactor, test controllerTest) error {
+				claim := newClaim("claim5-2", "uid5-2", "1Gi", "", api.ClaimPending)
+				reactor.claims[claim.Name] = claim
+				reactor.claimSource.Add(claim)
+				return nil
+			},
+		},
+		{
+			// deleteClaim with a bound claim makes bound volume released.
+			"5-3 - delete claim",
+			newVolumeArray("volume5-3", "10Gi", "uid5-3", "claim5-3", api.VolumeBound, api.PersistentVolumeReclaimRetain, annBoundByController),
+			newVolumeArray("volume5-3", "10Gi", "uid5-3", "claim5-3", api.VolumeReleased, api.PersistentVolumeReclaimRetain, annBoundByController),
+			newClaimArray("claim5-3", "uid5-3", "1Gi", "volume5-3", api.ClaimBound, annBoundByController, annBindCompleted),
+			noclaims,
+			noevents, noerrors,
+			// Custom test function that generates a delete event
+			func(ctrl *PersistentVolumeController, reactor *volumeReactor, test controllerTest) error {
+				obj := ctrl.claims.List()[0]
+				claim := obj.(*api.PersistentVolumeClaim)
+				// Remove the claim from list of resulting claims.
+				delete(reactor.claims, claim.Name)
+				// Poke the controller with deletion event. Cloned claim is
+				// needed to prevent races (and we would get a clone from etcd
+				// too).
+				clone, _ := conversion.NewCloner().DeepCopy(claim)
+				claimClone := clone.(*api.PersistentVolumeClaim)
+				reactor.claimSource.Delete(claimClone)
+				return nil
+			},
+		},
+		{
+			// deleteVolume with a bound volume. Check the claim is Lost.
+			"5-4 - delete volume",
+			newVolumeArray("volume5-4", "10Gi", "uid5-4", "claim5-4", api.VolumeBound, api.PersistentVolumeReclaimRetain),
+			novolumes,
+			newClaimArray("claim5-4", "uid5-4", "1Gi", "volume5-4", api.ClaimBound, annBoundByController, annBindCompleted),
+			newClaimArray("claim5-4", "uid5-4", "1Gi", "volume5-4", api.ClaimLost, annBoundByController, annBindCompleted),
+			[]string{"Warning ClaimLost"}, noerrors,
+			// Custom test function that generates a delete event
+			func(ctrl *PersistentVolumeController, reactor *volumeReactor, test controllerTest) error {
+				obj := ctrl.volumes.store.List()[0]
+				volume := obj.(*api.PersistentVolume)
+				// Remove the volume from list of resulting volumes.
+				delete(reactor.volumes, volume.Name)
+				// Poke the controller with deletion event. Cloned volume is
+				// needed to prevent races (and we would get a clone from etcd
+				// too).
+				clone, _ := conversion.NewCloner().DeepCopy(volume)
+				volumeClone := clone.(*api.PersistentVolume)
+				reactor.volumeSource.Delete(volumeClone)
+				return nil
+			},
+		},
+	}
+
+	for _, test := range tests {
+		glog.V(4).Infof("starting test %q", test.name)
+
+		// Initialize the controller
+		client := &fake.Clientset{}
+		volumeSource := framework.NewFakeControllerSource()
+		claimSource := framework.NewFakeControllerSource()
+		ctrl := newTestController(client, volumeSource, claimSource)
+		reactor := newVolumeReactor(client, ctrl, volumeSource, claimSource, test.errors)
+		for _, claim := range test.initialClaims {
+			claimSource.Add(claim)
+			reactor.claims[claim.Name] = claim
+		}
+		for _, volume := range test.initialVolumes {
+			volumeSource.Add(volume)
+			reactor.volumes[volume.Name] = volume
+		}
+
+		// Start the controller
+		defer ctrl.Stop()
+		go ctrl.Run()
+
+		// Wait for the controller to pass initial sync.
+		for !ctrl.isFullySynced() {
+			time.Sleep(10 * time.Millisecond)
+		}
+
+		// Call the tested function
+		err := test.test(ctrl, reactor, test)
+		if err != nil {
+			t.Errorf("Test %q initial test call failed: %v", test.name, err)
+		}
+
+		reactor.waitTest()
+
+		evaluateTestResults(ctrl, reactor, test, t)
+	}
+}

pkg/controller/persistentvolume/delete_test.go

+/*
+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, noerrors,
+			// 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, noerrors,
+			// 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"}, noerrors, 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"}, noerrors,
+			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"}, noerrors,
+			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, noerrors,
+			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, noerrors,
+			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, noerrors,
+			// 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"}, noerrors,
+			wrapTestWithControllerConfig(operationDelete, []error{errors.New("Mock delete error"), nil}, testSyncVolume),
+		},
+	}
+
+	runMultisyncTests(t, tests)
+}

pkg/controller/persistentvolume/types.go → pkg/controller/persistentvolume/index.go

@@ -24,34 +24,9 @@ import (
 	"k8s.io/kubernetes/pkg/client/cache"
 )
 
-const (
-	// A PVClaim can request a quality of service tier by adding this annotation.  The value of the annotation
-	// is arbitrary.  The values are pre-defined by a cluster admin and known to users when requesting a QoS.
-	// For example tiers might be gold, silver, and tin and the admin configures what that means for each volume plugin that can provision a volume.
-	// Values in the alpha version of this feature are not meaningful, but will be in the full version of this feature.
-	qosProvisioningKey = "volume.alpha.kubernetes.io/storage-class"
-	// Name of a tag attached to a real volume in cloud (e.g. AWS EBS or GCE PD)
-	// with namespace of a persistent volume claim used to create this volume.
-	cloudVolumeCreatedForClaimNamespaceTag = "kubernetes.io/created-for/pvc/namespace"
-	// Name of a tag attached to a real volume in cloud (e.g. AWS EBS or GCE PD)
-	// with name of a persistent volume claim used to create this volume.
-	cloudVolumeCreatedForClaimNameTag = "kubernetes.io/created-for/pvc/name"
-	// Name of a tag attached to a real volume in cloud (e.g. AWS EBS or GCE PD)
-	// with name of appropriate Kubernetes persistent volume .
-	cloudVolumeCreatedForVolumeNameTag = "kubernetes.io/created-for/pv/name"
-)
-
 // persistentVolumeOrderedIndex is a cache.Store that keeps persistent volumes indexed by AccessModes and ordered by storage capacity.
 type persistentVolumeOrderedIndex struct {
-	cache.Indexer
-}
-
-var _ cache.Store = &persistentVolumeOrderedIndex{} // persistentVolumeOrderedIndex is a Store
-
-func NewPersistentVolumeOrderedIndex() *persistentVolumeOrderedIndex {
-	return &persistentVolumeOrderedIndex{
-		cache.NewIndexer(cache.MetaNamespaceKeyFunc, cache.Indexers{"accessmodes": accessModesIndexFunc}),
-	}
+	store cache.Indexer
 }
 
 // accessModesIndexFunc is an indexing function that returns a persistent volume's AccessModes as a string
@@ -63,15 +38,15 @@ func accessModesIndexFunc(obj interface{}) ([]string, error) {
 	return []string{""}, fmt.Errorf("object is not a persistent volume: %v", obj)
 }
 
-// ListByAccessModes returns all volumes with the given set of AccessModeTypes *in order* of their storage capacity (low to high)
-func (pvIndex *persistentVolumeOrderedIndex) ListByAccessModes(modes []api.PersistentVolumeAccessMode) ([]*api.PersistentVolume, error) {
+// listByAccessModes returns all volumes with the given set of AccessModeTypes *in order* of their storage capacity (low to high)
+func (pvIndex *persistentVolumeOrderedIndex) listByAccessModes(modes []api.PersistentVolumeAccessMode) ([]*api.PersistentVolume, error) {
 	pv := &api.PersistentVolume{
 		Spec: api.PersistentVolumeSpec{
 			AccessModes: modes,
 		},
 	}
 
-	objs, err := pvIndex.Index("accessmodes", pv)
+	objs, err := pvIndex.store.Index("accessmodes", pv)
 	if err != nil {
 		return nil, err
 	}
@@ -101,7 +76,7 @@ func (pvIndex *persistentVolumeOrderedIndex) findByClaim(claim *api.PersistentVo
 	allPossibleModes := pvIndex.allPossibleMatchingAccessModes(claim.Spec.AccessModes)
 
 	for _, modes := range allPossibleModes {
-		volumes, err := pvIndex.ListByAccessModes(modes)
+		volumes, err := pvIndex.listByAccessModes(modes)
 		if err != nil {
 			return nil, err
 		}
@@ -117,16 +92,20 @@ func (pvIndex *persistentVolumeOrderedIndex) findByClaim(claim *api.PersistentVo
 				continue
 			}
 
-			if claim.Name == volume.Spec.ClaimRef.Name && claim.Namespace == volume.Spec.ClaimRef.Namespace && claim.UID == volume.Spec.ClaimRef.UID {
-				// exact match! No search required.
+			if isVolumeBoundToClaim(volume, claim) {
+				// Exact match! No search required.
 				return volume, nil
 			}
 		}
 
-		// a claim requesting provisioning will have an exact match pre-bound to the claim.
-		// no need to search through unbound volumes.  The matching volume will be created by the provisioner
-		// and will match above when the claim is re-processed by the binder.
-		if keyExists(qosProvisioningKey, claim.Annotations) {
+		// We want to provision volumes if the annotation is set even if there
+		// is matching PV. Therefore, do not look for available PV and let
+		// a new volume to be provisioned.
+		//
+		// When provisioner creates a new PV to this claim, an exact match
+		// pre-bound to the claim will be found by the checks above during
+		// subsequent claim sync.
+		if hasAnnotation(claim.ObjectMeta, annClass) {
 			return nil, nil
 		}
 
@@ -213,7 +192,7 @@ func matchStorageCapacity(pvA, pvB *api.PersistentVolume) bool {
 //
 func (pvIndex *persistentVolumeOrderedIndex) allPossibleMatchingAccessModes(requestedModes []api.PersistentVolumeAccessMode) [][]api.PersistentVolumeAccessMode {
 	matchedModes := [][]api.PersistentVolumeAccessMode{}
-	keys := pvIndex.Indexer.ListIndexFuncValues("accessmodes")
+	keys := pvIndex.store.ListIndexFuncValues("accessmodes")
 	for _, key := range keys {
 		indexedModes := api.GetAccessModesFromString(key)
 		if containedInAll(indexedModes, requestedModes) {
@@ -265,3 +244,7 @@ func (c byAccessModes) Len() int {
 func claimToClaimKey(claim *api.PersistentVolumeClaim) string {
 	return fmt.Sprintf("%s/%s", claim.Namespace, claim.Name)
 }
+
+func claimrefToClaimKey(claimref *api.ObjectReference) string {
+	return fmt.Sprintf("%s/%s", claimref.Namespace, claimref.Name)
+}

pkg/controller/persistentvolume/persistentvolume_index_test.go → pkg/controller/persistentvolume/index_test.go

@@ -22,12 +22,17 @@ import (
 	"k8s.io/kubernetes/pkg/api"
 	"k8s.io/kubernetes/pkg/api/resource"
 	"k8s.io/kubernetes/pkg/api/testapi"
+	"k8s.io/kubernetes/pkg/client/cache"
 )
 
+func newPersistentVolumeOrderedIndex() persistentVolumeOrderedIndex {
+	return persistentVolumeOrderedIndex{cache.NewIndexer(cache.MetaNamespaceKeyFunc, cache.Indexers{"accessmodes": accessModesIndexFunc})}
+}
+
 func TestMatchVolume(t *testing.T) {
-	volList := NewPersistentVolumeOrderedIndex()
+	volList := newPersistentVolumeOrderedIndex()
 	for _, pv := range createTestVolumes() {
-		volList.Add(pv)
+		volList.store.Add(pv)
 	}
 
 	scenarios := map[string]struct {
@@ -122,7 +127,7 @@ func TestMatchVolume(t *testing.T) {
 }
 
 func TestMatchingWithBoundVolumes(t *testing.T) {
-	volumeIndex := NewPersistentVolumeOrderedIndex()
+	volumeIndex := newPersistentVolumeOrderedIndex()
 	// two similar volumes, one is bound
 	pv1 := &api.PersistentVolume{
 		ObjectMeta: api.ObjectMeta{
@@ -158,8 +163,8 @@ func TestMatchingWithBoundVolumes(t *testing.T) {
 		},
 	}
 
-	volumeIndex.Add(pv1)
-	volumeIndex.Add(pv2)
+	volumeIndex.store.Add(pv1)
+	volumeIndex.store.Add(pv2)
 
 	claim := &api.PersistentVolumeClaim{
 		ObjectMeta: api.ObjectMeta{
@@ -189,12 +194,12 @@ func TestMatchingWithBoundVolumes(t *testing.T) {
 }
 
 func TestSort(t *testing.T) {
-	volList := NewPersistentVolumeOrderedIndex()
+	volList := newPersistentVolumeOrderedIndex()
 	for _, pv := range createTestVolumes() {
-		volList.Add(pv)
+		volList.store.Add(pv)
 	}
 
-	volumes, err := volList.ListByAccessModes([]api.PersistentVolumeAccessMode{api.ReadWriteOnce, api.ReadOnlyMany})
+	volumes, err := volList.listByAccessModes([]api.PersistentVolumeAccessMode{api.ReadWriteOnce, api.ReadOnlyMany})
 	if err != nil {
 		t.Error("Unexpected error retrieving volumes by access modes:", err)
 	}
@@ -205,7 +210,7 @@ func TestSort(t *testing.T) {
 		}
 	}
 
-	volumes, err = volList.ListByAccessModes([]api.PersistentVolumeAccessMode{api.ReadWriteOnce, api.ReadOnlyMany, api.ReadWriteMany})
+	volumes, err = volList.listByAccessModes([]api.PersistentVolumeAccessMode{api.ReadWriteOnce, api.ReadOnlyMany, api.ReadWriteMany})
 	if err != nil {
 		t.Error("Unexpected error retrieving volumes by access modes:", err)
 	}
@@ -218,9 +223,9 @@ func TestSort(t *testing.T) {
 }
 
 func TestAllPossibleAccessModes(t *testing.T) {
-	index := NewPersistentVolumeOrderedIndex()
+	index := newPersistentVolumeOrderedIndex()
 	for _, pv := range createTestVolumes() {
-		index.Add(pv)
+		index.store.Add(pv)
 	}
 
 	// the mock PVs creates contain 2 types of accessmodes:   RWO+ROX and RWO+ROW+RWX
@@ -292,10 +297,10 @@ func TestFindingVolumeWithDifferentAccessModes(t *testing.T) {
 		},
 	}
 
-	index := NewPersistentVolumeOrderedIndex()
-	index.Add(gce)
-	index.Add(ebs)
-	index.Add(nfs)
+	index := newPersistentVolumeOrderedIndex()
+	index.store.Add(gce)
+	index.store.Add(ebs)
+	index.store.Add(nfs)
 
 	volume, _ := index.findBestMatchForClaim(claim)
 	if volume.Name != ebs.Name {
@@ -521,10 +526,10 @@ func TestFindingPreboundVolumes(t *testing.T) {
 	pv5 := testVolume("pv5", "5Gi")
 	pv8 := testVolume("pv8", "8Gi")
 
-	index := NewPersistentVolumeOrderedIndex()
-	index.Add(pv1)
-	index.Add(pv5)
-	index.Add(pv8)
+	index := newPersistentVolumeOrderedIndex()
+	index.store.Add(pv1)
+	index.store.Add(pv5)
+	index.store.Add(pv8)
 
 	// expected exact match on size
 	volume, _ := index.findBestMatchForClaim(claim)

pkg/controller/persistentvolume/persistentvolume_provisioner_controller_test.go

-/*
-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 (
-	"fmt"
-	"testing"
-	"time"
-
-	"k8s.io/kubernetes/pkg/api"
-	"k8s.io/kubernetes/pkg/api/errors"
-	"k8s.io/kubernetes/pkg/api/resource"
-	"k8s.io/kubernetes/pkg/api/testapi"
-	clientset "k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset"
-	fake_cloud "k8s.io/kubernetes/pkg/cloudprovider/providers/fake"
-	"k8s.io/kubernetes/pkg/util"
-	volumetest "k8s.io/kubernetes/pkg/volume/testing"
-	"k8s.io/kubernetes/pkg/watch"
-)
-
-func TestProvisionerRunStop(t *testing.T) {
-	controller, _, _ := makeTestController()
-
-	if len(controller.stopChannels) != 0 {
-		t.Errorf("Non-running provisioner should not have any stopChannels.  Got %v", len(controller.stopChannels))
-	}
-
-	controller.Run()
-
-	if len(controller.stopChannels) != 2 {
-		t.Errorf("Running provisioner should have exactly 2 stopChannels.  Got %v", len(controller.stopChannels))
-	}
-
-	controller.Stop()
-
-	if len(controller.stopChannels) != 0 {
-		t.Errorf("Non-running provisioner should not have any stopChannels.  Got %v", len(controller.stopChannels))
-	}
-}
-
-func makeTestVolume() *api.PersistentVolume {
-	return &api.PersistentVolume{
-		ObjectMeta: api.ObjectMeta{
-			Annotations: map[string]string{},
-			Name:        "pv01",
-		},
-		Spec: api.PersistentVolumeSpec{
-			PersistentVolumeReclaimPolicy: api.PersistentVolumeReclaimDelete,
-			AccessModes:                   []api.PersistentVolumeAccessMode{api.ReadWriteOnce},
-			Capacity: api.ResourceList{
-				api.ResourceName(api.ResourceStorage): resource.MustParse("10Gi"),
-			},
-			PersistentVolumeSource: api.PersistentVolumeSource{
-				HostPath: &api.HostPathVolumeSource{
-					Path: "/somepath/data01",
-				},
-			},
-		},
-	}
-}
-
-func makeTestClaim() *api.PersistentVolumeClaim {
-	return &api.PersistentVolumeClaim{
-		ObjectMeta: api.ObjectMeta{
-			Annotations: map[string]string{},
-			Name:        "claim01",
-			Namespace:   "ns",
-			SelfLink:    testapi.Default.SelfLink("pvc", ""),
-		},
-		Spec: api.PersistentVolumeClaimSpec{
-			AccessModes: []api.PersistentVolumeAccessMode{api.ReadWriteOnce},
-			Resources: api.ResourceRequirements{
-				Requests: api.ResourceList{
-					api.ResourceName(api.ResourceStorage): resource.MustParse("8G"),
-				},
-			},
-		},
-	}
-}
-
-func makeTestController() (*PersistentVolumeProvisionerController, *mockControllerClient, *volumetest.FakeVolumePlugin) {
-	mockClient := &mockControllerClient{}
-	mockVolumePlugin := &volumetest.FakeVolumePlugin{}
-	controller, _ := NewPersistentVolumeProvisionerController(mockClient, 1*time.Second, "fake-kubernetes", nil, mockVolumePlugin, &fake_cloud.FakeCloud{})
-	return controller, mockClient, mockVolumePlugin
-}
-
-func TestReconcileClaim(t *testing.T) {
-	controller, mockClient, _ := makeTestController()
-	pvc := makeTestClaim()
-
-	// watch would have added the claim to the store
-	controller.claimStore.Add(pvc)
-	// store it in fake API server
-	mockClient.UpdatePersistentVolumeClaim(pvc)
-
-	err := controller.reconcileClaim(pvc)
-	if err != nil {
-		t.Errorf("Unexpected error: %v", err)
-	}
-
-	// non-provisionable PVC should not have created a volume on reconciliation
-	if mockClient.volume != nil {
-		t.Error("Unexpected volume found in mock client.  Expected nil")
-	}
-
-	pvc.Annotations[qosProvisioningKey] = "foo"
-	// store it in fake API server
-	mockClient.UpdatePersistentVolumeClaim(pvc)
-
-	err = controller.reconcileClaim(pvc)
-	if err != nil {
-		t.Errorf("Unexpected error: %v", err)
-	}
-
-	// PVC requesting provisioning should have a PV created for it
-	if mockClient.volume == nil {
-		t.Error("Expected to find bound volume but got nil")
-	}
-
-	if mockClient.volume.Spec.ClaimRef.Name != pvc.Name {
-		t.Errorf("Expected PV to be bound to %s but got %s", mockClient.volume.Spec.ClaimRef.Name, pvc.Name)
-	}
-
-	// the PVC should have correct annotation
-	if mockClient.claim.Annotations[pvProvisioningRequiredAnnotationKey] != pvProvisioningCompletedAnnotationValue {
-		t.Errorf("Annotation %q not set", pvProvisioningRequiredAnnotationKey)
-	}
-
-	// Run the syncClaim 2nd time to simulate periodic sweep running in parallel
-	// to the previous syncClaim. There is a lock in handleUpdateVolume(), so
-	// they will be called sequentially, but the second call will have old
-	// version of the claim.
-	oldPVName := mockClient.volume.Name
-
-	// Make the "old" claim
-	pvc2 := makeTestClaim()
-	pvc2.Annotations[qosProvisioningKey] = "foo"
-	// Add a dummy annotation so we recognize the claim was updated (i.e.
-	// stored in mockClient)
-	pvc2.Annotations["test"] = "test"
-
-	err = controller.reconcileClaim(pvc2)
-	if err != nil {
-		t.Errorf("Unexpected error: %v", err)
-	}
-
-	// The 2nd PVC should be ignored, no new PV was created
-	if val, found := pvc2.Annotations[pvProvisioningRequiredAnnotationKey]; found {
-		t.Errorf("2nd PVC got unexpected annotation %q: %q", pvProvisioningRequiredAnnotationKey, val)
-	}
-	if mockClient.volume.Name != oldPVName {
-		t.Errorf("2nd PVC unexpectedly provisioned a new volume")
-	}
-	if _, found := mockClient.claim.Annotations["test"]; found {
-		t.Errorf("2nd PVC was unexpectedly updated")
-	}
-}
-
-func checkTagValue(t *testing.T, tags map[string]string, tag string, expectedValue string) {
-	value, found := tags[tag]
-	if !found || value != expectedValue {
-		t.Errorf("Expected tag value %s = %s but value %s found", tag, expectedValue, value)
-	}
-}
-
-func TestReconcileVolume(t *testing.T) {
-
-	controller, mockClient, mockVolumePlugin := makeTestController()
-	pv := makeTestVolume()
-	pvc := makeTestClaim()
-	mockClient.volume = pv
-
-	err := controller.reconcileVolume(pv)
-	if err != nil {
-		t.Errorf("Unexpected error %v", err)
-	}
-
-	// watch adds claim to the store.
-	// we need to add it to our mock client to mimic normal Get call
-	controller.claimStore.Add(pvc)
-	mockClient.claim = pvc
-
-	// pretend the claim and volume are bound, no provisioning required
-	claimRef, _ := api.GetReference(pvc)
-	pv.Spec.ClaimRef = claimRef
-	mockClient.volume = pv
-	err = controller.reconcileVolume(pv)
-	if err != nil {
-		t.Errorf("Unexpected error %v", err)
-	}
-
-	pv.Annotations[pvProvisioningRequiredAnnotationKey] = "!pvProvisioningCompleted"
-	pv.Annotations[qosProvisioningKey] = "foo"
-	mockClient.volume = pv
-	err = controller.reconcileVolume(pv)
-
-	if !isAnnotationMatch(pvProvisioningRequiredAnnotationKey, pvProvisioningCompletedAnnotationValue, mockClient.volume.Annotations) {
-		t.Errorf("Expected %s but got %s", pvProvisioningRequiredAnnotationKey, mockClient.volume.Annotations[pvProvisioningRequiredAnnotationKey])
-	}
-
-	// Check that the volume plugin was called with correct tags
-	tags := *mockVolumePlugin.LastProvisionerOptions.CloudTags
-	checkTagValue(t, tags, cloudVolumeCreatedForClaimNamespaceTag, pvc.Namespace)
-	checkTagValue(t, tags, cloudVolumeCreatedForClaimNameTag, pvc.Name)
-	checkTagValue(t, tags, cloudVolumeCreatedForVolumeNameTag, pv.Name)
-
-}
-
-var _ controllerClient = &mockControllerClient{}
-
-type mockControllerClient struct {
-	volume *api.PersistentVolume
-	claim  *api.PersistentVolumeClaim
-}
-
-func (c *mockControllerClient) GetPersistentVolume(name string) (*api.PersistentVolume, error) {
-	return c.volume, nil
-}
-
-func (c *mockControllerClient) CreatePersistentVolume(pv *api.PersistentVolume) (*api.PersistentVolume, error) {
-	if pv.GenerateName != "" && pv.Name == "" {
-		pv.Name = fmt.Sprintf(pv.GenerateName, util.NewUUID())
-	}
-	c.volume = pv
-	return c.volume, nil
-}
-
-func (c *mockControllerClient) ListPersistentVolumes(options api.ListOptions) (*api.PersistentVolumeList, error) {
-	return &api.PersistentVolumeList{
-		Items: []api.PersistentVolume{*c.volume},
-	}, nil
-}
-
-func (c *mockControllerClient) WatchPersistentVolumes(options api.ListOptions) (watch.Interface, error) {
-	return watch.NewFake(), nil
-}
-
-func (c *mockControllerClient) UpdatePersistentVolume(pv *api.PersistentVolume) (*api.PersistentVolume, error) {
-	return c.CreatePersistentVolume(pv)
-}
-
-func (c *mockControllerClient) DeletePersistentVolume(volume *api.PersistentVolume) error {
-	c.volume = nil
-	return nil
-}
-
-func (c *mockControllerClient) UpdatePersistentVolumeStatus(volume *api.PersistentVolume) (*api.PersistentVolume, error) {
-	return volume, nil
-}
-
-func (c *mockControllerClient) GetPersistentVolumeClaim(namespace, name string) (*api.PersistentVolumeClaim, error) {
-	if c.claim != nil {
-		return c.claim, nil
-	} else {
-		return nil, errors.NewNotFound(api.Resource("persistentvolumes"), name)
-	}
-}
-
-func (c *mockControllerClient) ListPersistentVolumeClaims(namespace string, options api.ListOptions) (*api.PersistentVolumeClaimList, error) {
-	return &api.PersistentVolumeClaimList{
-		Items: []api.PersistentVolumeClaim{*c.claim},
-	}, nil
-}
-
-func (c *mockControllerClient) WatchPersistentVolumeClaims(namespace string, options api.ListOptions) (watch.Interface, error) {
-	return watch.NewFake(), nil
-}
-
-func (c *mockControllerClient) UpdatePersistentVolumeClaim(claim *api.PersistentVolumeClaim) (*api.PersistentVolumeClaim, error) {
-	c.claim = claim
-	return c.claim, nil
-}
-
-func (c *mockControllerClient) UpdatePersistentVolumeClaimStatus(claim *api.PersistentVolumeClaim) (*api.PersistentVolumeClaim, error) {
-	return claim, nil
-}
-
-func (c *mockControllerClient) GetKubeClient() clientset.Interface {
-	return nil
-}

pkg/controller/persistentvolume/persistentvolume_recycler_controller_test.go

-/*
-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 (
-	"fmt"
-	"testing"
-	"time"
-
-	"k8s.io/kubernetes/pkg/api"
-	"k8s.io/kubernetes/pkg/api/resource"
-	"k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset/fake"
-	"k8s.io/kubernetes/pkg/volume"
-	"k8s.io/kubernetes/pkg/volume/host_path"
-	volumetest "k8s.io/kubernetes/pkg/volume/testing"
-)
-
-const (
-	mySyncPeriod   = 2 * time.Second
-	myMaximumRetry = 3
-)
-
-func TestFailedRecycling(t *testing.T) {
-	pv := preparePV()
-
-	mockClient := &mockBinderClient{
-		volume: pv,
-	}
-
-	// no Init called for pluginMgr and no plugins are available.  Volume should fail recycling.
-	plugMgr := volume.VolumePluginMgr{}
-
-	recycler := &PersistentVolumeRecycler{
-		kubeClient:      fake.NewSimpleClientset(),
-		client:          mockClient,
-		pluginMgr:       plugMgr,
-		releasedVolumes: make(map[string]releasedVolumeStatus),
-	}
-
-	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)
-	}
-
-	// Use a new volume for the next test
-	pv = preparePV()
-	mockClient.volume = pv
-
-	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)
-	}
-}
-
-func TestRecyclingRetry(t *testing.T) {
-	// Test that recycler controller retries to recycle a volume several times, which succeeds eventually
-	pv := preparePV()
-
-	mockClient := &mockBinderClient{
-		volume: pv,
-	}
-
-	plugMgr := volume.VolumePluginMgr{}
-	// Use a fake NewRecycler function
-	plugMgr.InitPlugins(host_path.ProbeRecyclableVolumePlugins(newFailingMockRecycler, volume.VolumeConfig{}), volumetest.NewFakeVolumeHost("/tmp/fake", nil, nil))
-	// Reset a global call counter
-	failedCallCount = 0
-
-	recycler := &PersistentVolumeRecycler{
-		kubeClient:      fake.NewSimpleClientset(),
-		client:          mockClient,
-		pluginMgr:       plugMgr,
-		syncPeriod:      mySyncPeriod,
-		maximumRetry:    myMaximumRetry,
-		releasedVolumes: make(map[string]releasedVolumeStatus),
-	}
-
-	// All but the last attempt will fail
-	testRecycleFailures(t, recycler, mockClient, pv, myMaximumRetry-1)
-
-	// The last attempt should succeed
-	err := recycler.reclaimVolume(pv)
-	if err != nil {
-		t.Errorf("Last step: Recycler failed: %v", err)
-	}
-
-	if mockClient.volume.Status.Phase != api.VolumePending {
-		t.Errorf("Last step: The volume should be Pending, but is %s instead", mockClient.volume.Status.Phase)
-	}
-	// Check the cache, it should not have any entry
-	status, found := recycler.releasedVolumes[pv.Name]
-	if found {
-		t.Errorf("Last step: Expected PV to be removed from cache, got %v", status)
-	}
-}
-
-func TestRecyclingRetryAlwaysFail(t *testing.T) {
-	// Test that recycler controller retries to recycle a volume several times, which always fails.
-	pv := preparePV()
-
-	mockClient := &mockBinderClient{
-		volume: pv,
-	}
-
-	plugMgr := volume.VolumePluginMgr{}
-	// Use a fake NewRecycler function
-	plugMgr.InitPlugins(host_path.ProbeRecyclableVolumePlugins(newAlwaysFailingMockRecycler, volume.VolumeConfig{}), volumetest.NewFakeVolumeHost("/tmp/fake", nil, nil))
-	// Reset a global call counter
-	failedCallCount = 0
-
-	recycler := &PersistentVolumeRecycler{
-		kubeClient:      fake.NewSimpleClientset(),
-		client:          mockClient,
-		pluginMgr:       plugMgr,
-		syncPeriod:      mySyncPeriod,
-		maximumRetry:    myMaximumRetry,
-		releasedVolumes: make(map[string]releasedVolumeStatus),
-	}
-
-	// myMaximumRetry recycle attempts will fail
-	testRecycleFailures(t, recycler, mockClient, pv, myMaximumRetry)
-
-	// The volume should be failed after myMaximumRetry attempts
-	err := recycler.reclaimVolume(pv)
-	if err != nil {
-		t.Errorf("Last step: Recycler failed: %v", err)
-	}
-
-	if mockClient.volume.Status.Phase != api.VolumeFailed {
-		t.Errorf("Last step: The volume should be Failed, but is %s instead", mockClient.volume.Status.Phase)
-	}
-	// Check the cache, it should not have any entry
-	status, found := recycler.releasedVolumes[pv.Name]
-	if found {
-		t.Errorf("Last step: Expected PV to be removed from cache, got %v", status)
-	}
-}
-
-func preparePV() *api.PersistentVolume {
-	return &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,
-		},
-	}
-}
-
-// Test that `count` attempts to recycle a PV fails.
-func testRecycleFailures(t *testing.T, recycler *PersistentVolumeRecycler, mockClient *mockBinderClient, pv *api.PersistentVolume, count int) {
-	for i := 1; i <= count; i++ {
-		err := recycler.reclaimVolume(pv)
-		if err != nil {
-			t.Errorf("STEP %d: Recycler faled: %v", i, err)
-		}
-
-		// Check the status, it should be failed
-		if mockClient.volume.Status.Phase != api.VolumeReleased {
-			t.Errorf("STEP %d: The volume should be Released, but is %s instead", i, mockClient.volume.Status.Phase)
-		}
-
-		// Check the failed volume cache
-		status, found := recycler.releasedVolumes[pv.Name]
-		if !found {
-			t.Errorf("STEP %d: cannot find released volume status", i)
-		}
-		if status.retryCount != i {
-			t.Errorf("STEP %d: Expected nr. of attempts to be %d, got %d", i, i, status.retryCount)
-		}
-
-		// call reclaimVolume too early, it should not increment the retryCount
-		time.Sleep(mySyncPeriod / 2)
-		err = recycler.reclaimVolume(pv)
-		if err != nil {
-			t.Errorf("STEP %d: Recycler failed: %v", i, err)
-		}
-
-		status, found = recycler.releasedVolumes[pv.Name]
-		if !found {
-			t.Errorf("STEP %d: cannot find released volume status", i)
-		}
-		if status.retryCount != i {
-			t.Errorf("STEP %d: Expected nr. of attempts to be %d, got %d", i, i, status.retryCount)
-		}
-
-		// Call the next reclaimVolume() after full pvRecycleRetryPeriod
-		time.Sleep(mySyncPeriod / 2)
-	}
-}
-
-func newFailingMockRecycler(spec *volume.Spec, host volume.VolumeHost, config volume.VolumeConfig) (volume.Recycler, error) {
-	return &failingMockRecycler{
-		path:       spec.PersistentVolume.Spec.HostPath.Path,
-		errorCount: myMaximumRetry - 1, // fail two times and then successfully recycle the volume
-	}, nil
-}
-
-func newAlwaysFailingMockRecycler(spec *volume.Spec, host volume.VolumeHost, config volume.VolumeConfig) (volume.Recycler, error) {
-	return &failingMockRecycler{
-		path:       spec.PersistentVolume.Spec.HostPath.Path,
-		errorCount: 1000, // always fail
-	}, nil
-}
-
-type failingMockRecycler struct {
-	path string
-	// How many times should the recycler fail before returning success.
-	errorCount int
-	volume.MetricsNil
-}
-
-// Counter of failingMockRecycler.Recycle() calls. Global variable just for
-// testing. It's too much code to create a custom volume plugin, which would
-// hold this variable.
-var failedCallCount = 0
-
-func (r *failingMockRecycler) GetPath() string {
-	return r.path
-}
-
-func (r *failingMockRecycler) Recycle() error {
-	failedCallCount += 1
-	if failedCallCount <= r.errorCount {
-		return fmt.Errorf("Failing for %d. time", failedCallCount)
-	}
-	// return nil means recycle passed
-	return nil
-}

pkg/controller/persistentvolume/recycle_test.go

+/*
+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 TestRecycleSync(t *testing.T) {
+	tests := []controllerTest{
+		{
+			// recycle volume bound by controller
+			"6-1 - successful recycle",
+			newVolumeArray("volume6-1", "1Gi", "uid6-1", "claim6-1", api.VolumeBound, api.PersistentVolumeReclaimRecycle, annBoundByController),
+			newVolumeArray("volume6-1", "1Gi", "", "", api.VolumeAvailable, api.PersistentVolumeReclaimRecycle),
+			noclaims,
+			noclaims,
+			noevents, noerrors,
+			// Inject recycler into the controller and call syncVolume. The
+			// recycler simulates one recycle() call that succeeds.
+			wrapTestWithControllerConfig(operationRecycle, []error{nil}, testSyncVolume),
+		},
+		{
+			// recycle volume bound by user
+			"6-2 - successful recycle with prebound volume",
+			newVolumeArray("volume6-2", "1Gi", "uid6-2", "claim6-2", api.VolumeBound, api.PersistentVolumeReclaimRecycle),
+			newVolumeArray("volume6-2", "1Gi", "", "claim6-2", api.VolumeAvailable, api.PersistentVolumeReclaimRecycle),
+			noclaims,
+			noclaims,
+			noevents, noerrors,
+			// Inject recycler into the controller and call syncVolume. The
+			// recycler simulates one recycle() call that succeeds.
+			wrapTestWithControllerConfig(operationRecycle, []error{nil}, testSyncVolume),
+		},
+		{
+			// recycle failure - plugin not found
+			"6-3 - plugin not found",
+			newVolumeArray("volume6-3", "1Gi", "uid6-3", "claim6-3", api.VolumeBound, api.PersistentVolumeReclaimRecycle),
+			newVolumeArray("volume6-3", "1Gi", "uid6-3", "claim6-3", api.VolumeFailed, api.PersistentVolumeReclaimRecycle),
+			noclaims,
+			noclaims,
+			[]string{"Warning VolumeFailedRecycle"}, noerrors, testSyncVolume,
+		},
+		{
+			// recycle failure - newRecycler returns error
+			"6-4 - newRecycler returns error",
+			newVolumeArray("volume6-4", "1Gi", "uid6-4", "claim6-4", api.VolumeBound, api.PersistentVolumeReclaimRecycle),
+			newVolumeArray("volume6-4", "1Gi", "uid6-4", "claim6-4", api.VolumeFailed, api.PersistentVolumeReclaimRecycle),
+			noclaims,
+			noclaims,
+			[]string{"Warning VolumeFailedRecycle"}, noerrors,
+			wrapTestWithControllerConfig(operationRecycle, []error{}, testSyncVolume),
+		},
+		{
+			// recycle failure - recycle returns error
+			"6-5 - recycle returns error",
+			newVolumeArray("volume6-5", "1Gi", "uid6-5", "claim6-5", api.VolumeBound, api.PersistentVolumeReclaimRecycle),
+			newVolumeArray("volume6-5", "1Gi", "uid6-5", "claim6-5", api.VolumeFailed, api.PersistentVolumeReclaimRecycle),
+			noclaims,
+			noclaims,
+			[]string{"Warning VolumeFailedRecycle"}, noerrors,
+			wrapTestWithControllerConfig(operationRecycle, []error{errors.New("Mock recycle error")}, testSyncVolume),
+		},
+		{
+			// recycle success(?) - volume is deleted before doRecycle() starts
+			"6-6 - volume is deleted before recycling",
+			newVolumeArray("volume6-6", "1Gi", "uid6-6", "claim6-6", api.VolumeBound, api.PersistentVolumeReclaimRecycle),
+			novolumes,
+			noclaims,
+			noclaims,
+			noevents, noerrors,
+			wrapTestWithInjectedOperation(wrapTestWithControllerConfig(operationRecycle, []error{}, testSyncVolume), func(ctrl *PersistentVolumeController, reactor *volumeReactor) {
+				// Delete the volume before recycle operation starts
+				reactor.lock.Lock()
+				delete(reactor.volumes, "volume6-6")
+				reactor.lock.Unlock()
+			}),
+		},
+		{
+			// recycle success(?) - volume is recycled by previous recycler just
+			// at the time new doRecycle() starts. This simulates "volume no
+			// longer needs recycling, skipping".
+			"6-7 - volume is deleted before recycling",
+			newVolumeArray("volume6-7", "1Gi", "uid6-7", "claim6-7", api.VolumeBound, api.PersistentVolumeReclaimRecycle, annBoundByController),
+			newVolumeArray("volume6-7", "1Gi", "", "", api.VolumeAvailable, api.PersistentVolumeReclaimRecycle),
+			noclaims,
+			noclaims,
+			noevents, noerrors,
+			wrapTestWithInjectedOperation(wrapTestWithControllerConfig(operationRecycle, []error{}, testSyncVolume), func(ctrl *PersistentVolumeController, reactor *volumeReactor) {
+				// Mark the volume as Available before the recycler starts
+				reactor.lock.Lock()
+				volume := reactor.volumes["volume6-7"]
+				volume.Spec.ClaimRef = nil
+				volume.Status.Phase = api.VolumeAvailable
+				volume.Annotations = nil
+				reactor.lock.Unlock()
+			}),
+		},
+		{
+			// recycle success(?) - volume bound by user is recycled by previous
+			// recycler just at the time new doRecycle() starts. This simulates
+			// "volume no longer needs recycling, skipping" with volume bound by
+			// user.
+			"6-8 - prebound volume is deleted before recycling",
+			newVolumeArray("volume6-8", "1Gi", "uid6-8", "claim6-8", api.VolumeBound, api.PersistentVolumeReclaimRecycle),
+			newVolumeArray("volume6-8", "1Gi", "", "claim6-8", api.VolumeAvailable, api.PersistentVolumeReclaimRecycle),
+			noclaims,
+			noclaims,
+			noevents, noerrors,
+			wrapTestWithInjectedOperation(wrapTestWithControllerConfig(operationRecycle, []error{}, testSyncVolume), func(ctrl *PersistentVolumeController, reactor *volumeReactor) {
+				// Mark the volume as Available before the recycler starts
+				reactor.lock.Lock()
+				volume := reactor.volumes["volume6-8"]
+				volume.Spec.ClaimRef.UID = ""
+				volume.Status.Phase = api.VolumeAvailable
+				reactor.lock.Unlock()
+			}),
+		},
+		{
+			// recycle success - volume bound by user is recycled, while a new
+			// claim is created with another UID.
+			"6-9 - prebound volume is recycled while the claim exists",
+			newVolumeArray("volume6-9", "1Gi", "uid6-9", "claim6-9", api.VolumeBound, api.PersistentVolumeReclaimRecycle),
+			newVolumeArray("volume6-9", "1Gi", "", "claim6-9", api.VolumeAvailable, api.PersistentVolumeReclaimRecycle),
+			newClaimArray("claim6-9", "uid6-9-x", "10Gi", "", api.ClaimPending),
+			newClaimArray("claim6-9", "uid6-9-x", "10Gi", "", api.ClaimPending),
+			noevents, noerrors,
+			// Inject recycler into the controller and call syncVolume. The
+			// recycler simulates one recycle() call that succeeds.
+			wrapTestWithControllerConfig(operationRecycle, []error{nil}, testSyncVolume),
+		},
+		{
+			// volume has unknown reclaim policy - failure expected
+			"6-10 - unknown reclaim policy",
+			newVolumeArray("volume6-10", "1Gi", "uid6-10", "claim6-10", api.VolumeBound, "Unknown"),
+			newVolumeArray("volume6-10", "1Gi", "uid6-10", "claim6-10", api.VolumeFailed, "Unknown"),
+			noclaims,
+			noclaims,
+			[]string{"Warning VolumeUnknownReclaimPolicy"}, noerrors, 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 TestRecycleMultiSync(t *testing.T) {
+	tests := []controllerTest{
+		{
+			// recycle failure - recycle returns error. The controller should
+			// try again.
+			"7-1 - recycle returns error",
+			newVolumeArray("volume7-1", "1Gi", "uid7-1", "claim7-1", api.VolumeBound, api.PersistentVolumeReclaimRecycle),
+			newVolumeArray("volume7-1", "1Gi", "", "claim7-1", api.VolumeAvailable, api.PersistentVolumeReclaimRecycle),
+			noclaims,
+			noclaims,
+			[]string{"Warning VolumeFailedRecycle"}, noerrors,
+			wrapTestWithControllerConfig(operationRecycle, []error{errors.New("Mock recycle error"), nil}, testSyncVolume),
+		},
+	}
+
+	runMultisyncTests(t, tests)
+}

pkg/controller/persistentvolume/volume_host.go

+/*
+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 (
+	"fmt"
+
+	"k8s.io/kubernetes/pkg/api"
+	clientset "k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset"
+	"k8s.io/kubernetes/pkg/cloudprovider"
+	"k8s.io/kubernetes/pkg/types"
+	"k8s.io/kubernetes/pkg/util/io"
+	"k8s.io/kubernetes/pkg/util/mount"
+	vol "k8s.io/kubernetes/pkg/volume"
+)
+
+// VolumeHost interface implementation for PersistentVolumeController.
+
+var _ vol.VolumeHost = &PersistentVolumeController{}
+
+func (ctrl *PersistentVolumeController) GetPluginDir(pluginName string) string {
+	return ""
+}
+
+func (ctrl *PersistentVolumeController) GetPodVolumeDir(podUID types.UID, pluginName string, volumeName string) string {
+	return ""
+}
+
+func (ctrl *PersistentVolumeController) GetPodPluginDir(podUID types.UID, pluginName string) string {
+	return ""
+}
+
+func (ctrl *PersistentVolumeController) GetKubeClient() clientset.Interface {
+	return ctrl.kubeClient
+}
+
+func (ctrl *PersistentVolumeController) NewWrapperMounter(volName string, spec vol.Spec, pod *api.Pod, opts vol.VolumeOptions) (vol.Mounter, error) {
+	return nil, fmt.Errorf("PersistentVolumeController.NewWrapperMounter is not implemented")
+}
+
+func (ctrl *PersistentVolumeController) NewWrapperUnmounter(volName string, spec vol.Spec, podUID types.UID) (vol.Unmounter, error) {
+	return nil, fmt.Errorf("PersistentVolumeController.NewWrapperMounter is not implemented")
+}
+
+func (ctrl *PersistentVolumeController) GetCloudProvider() cloudprovider.Interface {
+	return ctrl.cloud
+}
+
+func (ctrl *PersistentVolumeController) GetMounter() mount.Interface {
+	return nil
+}
+
+func (ctrl *PersistentVolumeController) GetWriter() io.Writer {
+	return nil
+}
+
+func (ctrl *PersistentVolumeController) GetHostName() string {
+	return ""
+}

pkg/kubectl/describe.go

@@ -774,6 +774,8 @@ func (d *PersistentVolumeClaimDescriber) Describe(namespace, name string, descri
 		capacity = storage.String()
 	}
 
+	events, _ := d.Events(namespace).Search(pvc)
+
 	return tabbedString(func(out io.Writer) error {
 		fmt.Fprintf(out, "Name:\t%s\n", pvc.Name)
 		fmt.Fprintf(out, "Namespace:\t%s\n", pvc.Namespace)
@@ -782,6 +784,10 @@ func (d *PersistentVolumeClaimDescriber) Describe(namespace, name string, descri
 		printLabelsMultiline(out, "Labels", pvc.Labels)
 		fmt.Fprintf(out, "Capacity:\t%s\n", capacity)
 		fmt.Fprintf(out, "Access Modes:\t%s\n", accessModes)
+		if events != nil {
+			DescribeEvents(events, out)
+		}
+
 		return nil
 	})
 }

pkg/volume/aws_ebs/aws_ebs.go

@@ -408,35 +408,16 @@ type awsElasticBlockStoreProvisioner struct {
 
 var _ volume.Provisioner = &awsElasticBlockStoreProvisioner{}
 
-func (c *awsElasticBlockStoreProvisioner) Provision(pv *api.PersistentVolume) error {
+func (c *awsElasticBlockStoreProvisioner) Provision() (*api.PersistentVolume, error) {
 	volumeID, sizeGB, labels, err := c.manager.CreateVolume(c)
 	if err != nil {
-		return err
-	}
-	pv.Spec.PersistentVolumeSource.AWSElasticBlockStore.VolumeID = volumeID
-	pv.Spec.Capacity = api.ResourceList{
-		api.ResourceName(api.ResourceStorage): resource.MustParse(fmt.Sprintf("%dGi", sizeGB)),
+		return nil, err
 	}
 
-	if len(labels) != 0 {
-		if pv.Labels == nil {
-			pv.Labels = make(map[string]string)
-		}
-		for k, v := range labels {
-			pv.Labels[k] = v
-		}
-	}
-
-	return nil
-}
-
-func (c *awsElasticBlockStoreProvisioner) NewPersistentVolumeTemplate() (*api.PersistentVolume, error) {
-	// Provide dummy api.PersistentVolume.Spec, it will be filled in
-	// awsElasticBlockStoreProvisioner.Provision()
-	return &api.PersistentVolume{
+	pv := &api.PersistentVolume{
 		ObjectMeta: api.ObjectMeta{
-			GenerateName: "pv-aws-",
-			Labels:       map[string]string{},
+			Name:   c.options.PVName,
+			Labels: map[string]string{},
 			Annotations: map[string]string{
 				"kubernetes.io/createdby": "aws-ebs-dynamic-provisioner",
 			},
@@ -445,16 +426,27 @@ func (c *awsElasticBlockStoreProvisioner) NewPersistentVolumeTemplate() (*api.Pe
 			PersistentVolumeReclaimPolicy: c.options.PersistentVolumeReclaimPolicy,
 			AccessModes:                   c.options.AccessModes,
 			Capacity: api.ResourceList{
-				api.ResourceName(api.ResourceStorage): c.options.Capacity,
+				api.ResourceName(api.ResourceStorage): resource.MustParse(fmt.Sprintf("%dGi", sizeGB)),
 			},
 			PersistentVolumeSource: api.PersistentVolumeSource{
 				AWSElasticBlockStore: &api.AWSElasticBlockStoreVolumeSource{
-					VolumeID:  volume.ProvisionedVolumeName,
+					VolumeID:  volumeID,
 					FSType:    "ext4",
 					Partition: 0,
 					ReadOnly:  false,
 				},
 			},
 		},
-	}, nil
+	}
+
+	if len(labels) != 0 {
+		if pv.Labels == nil {
+			pv.Labels = make(map[string]string)
+		}
+		for k, v := range labels {
+			pv.Labels[k] = v
+		}
+	}
+
+	return pv, nil
 }

pkg/volume/aws_ebs/aws_ebs_test.go

@@ -220,14 +220,7 @@ func TestPlugin(t *testing.T) {
 		PersistentVolumeReclaimPolicy: api.PersistentVolumeReclaimDelete,
 	}
 	provisioner, err := plug.(*awsElasticBlockStorePlugin).newProvisionerInternal(options, &fakePDManager{})
-	persistentSpec, err := provisioner.NewPersistentVolumeTemplate()
-	if err != nil {
-		t.Errorf("NewPersistentVolumeTemplate() failed: %v", err)
-	}
-
-	// get 2nd Provisioner - persistent volume controller will do the same
-	provisioner, err = plug.(*awsElasticBlockStorePlugin).newProvisionerInternal(options, &fakePDManager{})
-	err = provisioner.Provision(persistentSpec)
+	persistentSpec, err := provisioner.Provision()
 	if err != nil {
 		t.Errorf("Provision() failed: %v", err)
 	}

pkg/volume/cinder/cinder.go

@@ -427,25 +427,16 @@ type cinderVolumeProvisioner struct {
 
 var _ volume.Provisioner = &cinderVolumeProvisioner{}
 
-func (c *cinderVolumeProvisioner) Provision(pv *api.PersistentVolume) error {
+func (c *cinderVolumeProvisioner) Provision() (*api.PersistentVolume, error) {
 	volumeID, sizeGB, err := c.manager.CreateVolume(c)
 	if err != nil {
-		return err
-	}
-	pv.Spec.PersistentVolumeSource.Cinder.VolumeID = volumeID
-	pv.Spec.Capacity = api.ResourceList{
-		api.ResourceName(api.ResourceStorage): resource.MustParse(fmt.Sprintf("%dGi", sizeGB)),
+		return nil, err
 	}
-	return nil
-}
 
-func (c *cinderVolumeProvisioner) NewPersistentVolumeTemplate() (*api.PersistentVolume, error) {
-	// Provide dummy api.PersistentVolume.Spec, it will be filled in
-	// cinderVolumeProvisioner.Provision()
-	return &api.PersistentVolume{
+	pv := &api.PersistentVolume{
 		ObjectMeta: api.ObjectMeta{
-			GenerateName: "pv-cinder-",
-			Labels:       map[string]string{},
+			Name:   c.options.PVName,
+			Labels: map[string]string{},
 			Annotations: map[string]string{
 				"kubernetes.io/createdby": "cinder-dynamic-provisioner",
 			},
@@ -454,16 +445,16 @@ func (c *cinderVolumeProvisioner) NewPersistentVolumeTemplate() (*api.Persistent
 			PersistentVolumeReclaimPolicy: c.options.PersistentVolumeReclaimPolicy,
 			AccessModes:                   c.options.AccessModes,
 			Capacity: api.ResourceList{
-				api.ResourceName(api.ResourceStorage): c.options.Capacity,
+				api.ResourceName(api.ResourceStorage): resource.MustParse(fmt.Sprintf("%dGi", sizeGB)),
 			},
 			PersistentVolumeSource: api.PersistentVolumeSource{
 				Cinder: &api.CinderVolumeSource{
-					VolumeID: volume.ProvisionedVolumeName,
+					VolumeID: volumeID,
 					FSType:   "ext4",
 					ReadOnly: false,
 				},
 			},
 		},
-	}, nil
-
+	}
+	return pv, nil
 }

pkg/volume/cinder/cinder_test.go

@@ -212,14 +212,7 @@ func TestPlugin(t *testing.T) {
 		PersistentVolumeReclaimPolicy: api.PersistentVolumeReclaimDelete,
 	}
 	provisioner, err := plug.(*cinderPlugin).newProvisionerInternal(options, &fakePDManager{0})
-	persistentSpec, err := provisioner.NewPersistentVolumeTemplate()
-	if err != nil {
-		t.Errorf("NewPersistentVolumeTemplate() failed: %v", err)
-	}
-
-	// get 2nd Provisioner - persistent volume controller will do the same
-	provisioner, err = plug.(*cinderPlugin).newProvisionerInternal(options, &fakePDManager{0})
-	err = provisioner.Provision(persistentSpec)
+	persistentSpec, err := provisioner.Provision()
 	if err != nil {
 		t.Errorf("Provision() failed: %v", err)
 	}

pkg/volume/gce_pd/gce_pd.go

@@ -370,35 +370,16 @@ type gcePersistentDiskProvisioner struct {
 
 var _ volume.Provisioner = &gcePersistentDiskProvisioner{}
 
-func (c *gcePersistentDiskProvisioner) Provision(pv *api.PersistentVolume) error {
+func (c *gcePersistentDiskProvisioner) Provision() (*api.PersistentVolume, error) {
 	volumeID, sizeGB, labels, err := c.manager.CreateVolume(c)
 	if err != nil {
-		return err
-	}
-	pv.Spec.PersistentVolumeSource.GCEPersistentDisk.PDName = volumeID
-	pv.Spec.Capacity = api.ResourceList{
-		api.ResourceName(api.ResourceStorage): resource.MustParse(fmt.Sprintf("%dGi", sizeGB)),
+		return nil, err
 	}
 
-	if len(labels) != 0 {
-		if pv.Labels == nil {
-			pv.Labels = make(map[string]string)
-		}
-		for k, v := range labels {
-			pv.Labels[k] = v
-		}
-	}
-
-	return nil
-}
-
-func (c *gcePersistentDiskProvisioner) NewPersistentVolumeTemplate() (*api.PersistentVolume, error) {
-	// Provide dummy api.PersistentVolume.Spec, it will be filled in
-	// gcePersistentDiskProvisioner.Provision()
-	return &api.PersistentVolume{
+	pv := &api.PersistentVolume{
 		ObjectMeta: api.ObjectMeta{
-			GenerateName: "pv-gce-",
-			Labels:       map[string]string{},
+			Name:   c.options.PVName,
+			Labels: map[string]string{},
 			Annotations: map[string]string{
 				"kubernetes.io/createdby": "gce-pd-dynamic-provisioner",
 			},
@@ -407,16 +388,27 @@ func (c *gcePersistentDiskProvisioner) NewPersistentVolumeTemplate() (*api.Persi
 			PersistentVolumeReclaimPolicy: c.options.PersistentVolumeReclaimPolicy,
 			AccessModes:                   c.options.AccessModes,
 			Capacity: api.ResourceList{
-				api.ResourceName(api.ResourceStorage): c.options.Capacity,
+				api.ResourceName(api.ResourceStorage): resource.MustParse(fmt.Sprintf("%dGi", sizeGB)),
 			},
 			PersistentVolumeSource: api.PersistentVolumeSource{
 				GCEPersistentDisk: &api.GCEPersistentDiskVolumeSource{
-					PDName:    volume.ProvisionedVolumeName,
+					PDName:    volumeID,
 					FSType:    "ext4",
 					Partition: 0,
 					ReadOnly:  false,
 				},
 			},
 		},
-	}, nil
+	}
+
+	if len(labels) != 0 {
+		if pv.Labels == nil {
+			pv.Labels = make(map[string]string)
+		}
+		for k, v := range labels {
+			pv.Labels[k] = v
+		}
+	}
+
+	return pv, nil
 }

pkg/volume/gce_pd/gce_pd_test.go

@@ -216,14 +216,7 @@ func TestPlugin(t *testing.T) {
 		PersistentVolumeReclaimPolicy: api.PersistentVolumeReclaimDelete,
 	}
 	provisioner, err := plug.(*gcePersistentDiskPlugin).newProvisionerInternal(options, &fakePDManager{})
-	persistentSpec, err := provisioner.NewPersistentVolumeTemplate()
-	if err != nil {
-		t.Errorf("NewPersistentVolumeTemplate() failed: %v", err)
-	}
-
-	// get 2nd Provisioner - persistent volume controller will do the same
-	provisioner, err = plug.(*gcePersistentDiskPlugin).newProvisionerInternal(options, &fakePDManager{})
-	err = provisioner.Provision(persistentSpec)
+	persistentSpec, err := provisioner.Provision()
 	if err != nil {
 		t.Errorf("Provision() failed: %v", err)
 	}

pkg/volume/host_path/host_path.go

@@ -252,18 +252,12 @@ type hostPathProvisioner struct {
 
 // Create for hostPath simply creates a local /tmp/hostpath_pv/%s directory as a new PersistentVolume.
 // This Provisioner is meant for development and testing only and WILL NOT WORK in a multi-node cluster.
-func (r *hostPathProvisioner) Provision(pv *api.PersistentVolume) error {
-	if pv.Spec.HostPath == nil {
-		return fmt.Errorf("pv.Spec.HostPath cannot be nil")
-	}
-	return os.MkdirAll(pv.Spec.HostPath.Path, 0750)
-}
-
-func (r *hostPathProvisioner) NewPersistentVolumeTemplate() (*api.PersistentVolume, error) {
+func (r *hostPathProvisioner) Provision() (*api.PersistentVolume, error) {
 	fullpath := fmt.Sprintf("/tmp/hostpath_pv/%s", util.NewUUID())
-	return &api.PersistentVolume{
+
+	pv := &api.PersistentVolume{
 		ObjectMeta: api.ObjectMeta{
-			GenerateName: "pv-hostpath-",
+			Name: r.options.PVName,
 			Annotations: map[string]string{
 				"kubernetes.io/createdby": "hostpath-dynamic-provisioner",
 			},
@@ -280,7 +274,9 @@ func (r *hostPathProvisioner) NewPersistentVolumeTemplate() (*api.PersistentVolu
 				},
 			},
 		},
-	}, nil
+	}
+
+	return pv, os.MkdirAll(pv.Spec.HostPath.Path, 0750)
 }
 
 // hostPathDeleter deletes a hostPath PV from the cluster.

pkg/volume/host_path/host_path_test.go

@@ -163,7 +163,7 @@ func TestProvisioner(t *testing.T) {
 	if err != nil {
 		t.Errorf("Failed to make a new Provisioner: %v", err)
 	}
-	pv, err := creater.NewPersistentVolumeTemplate()
+	pv, err := creater.Provision()
 	if err != nil {
 		t.Errorf("Unexpected error creating volume: %v", err)
 	}

pkg/volume/testing/testing.go

@@ -340,11 +340,12 @@ type FakeProvisioner struct {
 	Host    VolumeHost
 }
 
-func (fc *FakeProvisioner) NewPersistentVolumeTemplate() (*api.PersistentVolume, error) {
+func (fc *FakeProvisioner) Provision() (*api.PersistentVolume, error) {
 	fullpath := fmt.Sprintf("/tmp/hostpath_pv/%s", util.NewUUID())
-	return &api.PersistentVolume{
+
+	pv := &api.PersistentVolume{
 		ObjectMeta: api.ObjectMeta{
-			GenerateName: "pv-fakeplugin-",
+			Name: fc.Options.PVName,
 			Annotations: map[string]string{
 				"kubernetes.io/createdby": "fakeplugin-provisioner",
 			},
@@ -361,11 +362,9 @@ func (fc *FakeProvisioner) NewPersistentVolumeTemplate() (*api.PersistentVolume,
 				},
 			},
 		},
-	}, nil
-}
+	}
 
-func (fc *FakeProvisioner) Provision(pv *api.PersistentVolume) error {
-	return nil
+	return pv, nil
 }
 
 // FindEmptyDirectoryUsageOnTmpfs finds the expected usage of an empty directory existing on

pkg/volume/volume.go

@@ -111,13 +111,10 @@ type Recycler interface {
 // Provisioner is an interface that creates templates for PersistentVolumes and can create the volume
 // as a new resource in the infrastructure provider.
 type Provisioner interface {
-	// Provision creates the resource by allocating the underlying volume in a storage system.
-	// This method should block until completion.
-	Provision(*api.PersistentVolume) error
-	// NewPersistentVolumeTemplate creates a new PersistentVolume to be used as a template before saving.
-	// The provisioner will want to tweak its properties, assign correct annotations, etc.
-	// This func should *NOT* persist the PV in the API.  That is left to the caller.
-	NewPersistentVolumeTemplate() (*api.PersistentVolume, error)
+	// Provision creates the resource by allocating the underlying volume in a
+	// storage system. This method should block until completion and returns
+	// PersistentVolume representing the created storage resource.
+	Provision() (*api.PersistentVolume, error)
 }
 
 // Deleter removes the resource from the underlying storage provider.  Calls to this method should block until

test/integration/persistent_volumes_test.go

@@ -49,21 +49,15 @@ func TestPersistentVolumeRecycler(t *testing.T) {
 	deleteAllEtcdKeys()
 	// Use higher QPS and Burst, there is a test for race condition below, which
 	// creates many claims and default values were too low.
-	binderClient := clientset.NewForConfigOrDie(&restclient.Config{Host: s.URL, ContentConfig: restclient.ContentConfig{GroupVersion: testapi.Default.GroupVersion()}, QPS: 1000, Burst: 100000})
-	recyclerClient := clientset.NewForConfigOrDie(&restclient.Config{Host: s.URL, ContentConfig: restclient.ContentConfig{GroupVersion: testapi.Default.GroupVersion()}, QPS: 1000, Burst: 100000})
 	testClient := clientset.NewForConfigOrDie(&restclient.Config{Host: s.URL, ContentConfig: restclient.ContentConfig{GroupVersion: testapi.Default.GroupVersion()}, QPS: 1000, Burst: 100000})
 	host := volumetest.NewFakeVolumeHost("/tmp/fake", nil, nil)
 
 	plugins := []volume.VolumePlugin{&volumetest.FakeVolumePlugin{"plugin-name", host, volume.VolumeConfig{}, volume.VolumeOptions{}, 0, 0, nil, nil, nil, nil}}
 	cloud := &fake_cloud.FakeCloud{}
 
-	binder := persistentvolumecontroller.NewPersistentVolumeClaimBinder(binderClient, 10*time.Second)
-	binder.Run()
-	defer binder.Stop()
-
-	recycler, _ := persistentvolumecontroller.NewPersistentVolumeRecycler(recyclerClient, 30*time.Second, 3, plugins, cloud)
-	recycler.Run()
-	defer recycler.Stop()
+	ctrl := persistentvolumecontroller.NewPersistentVolumeController(testClient, 10*time.Second, nil, plugins, cloud, "", nil, nil, nil)
+	ctrl.Run()
+	defer ctrl.Stop()
 
 	// This PV will be claimed, released, and recycled.
 	pv := &api.PersistentVolume{