diff --git a/pkg/controller/persistentvolume/persistentvolume_claim_binder_controller.go b/pkg/controller/persistentvolume/persistentvolume_claim_binder_controller.go index 4d963b22107..35e8994c5ef 100644 --- a/pkg/controller/persistentvolume/persistentvolume_claim_binder_controller.go +++ b/pkg/controller/persistentvolume/persistentvolume_claim_binder_controller.go @@ -230,15 +230,19 @@ func syncVolume(volumeIndex *persistentVolumeOrderedIndex, binderClient binderCl switch currentPhase { case api.VolumePending: - // 3 possible states: - // 1. ClaimRef != nil and Claim exists: Prebound to claim. Make volume available for binding (it will match PVC). - // 2. ClaimRef != nil and Claim !exists: Recently recycled. Remove bind. Make volume available for new claim. - // 3. ClaimRef == nil: Neither recycled nor prebound. Make volume available for binding. + // 4 possible states: + // 1. ClaimRef != nil, Claim exists, Claim UID == ClaimRef UID: Prebound to claim. Make volume available for binding (it will match PVC). + // 2. ClaimRef != nil, Claim exists, Claim UID != ClaimRef UID: Recently recycled. Remove bind. Make volume available for new claim. + // 3. ClaimRef != nil, Claim !exists: Recently recycled. Remove bind. Make volume available for new claim. + // 4. ClaimRef == nil: Neither recycled nor prebound. Make volume available for binding. nextPhase = api.VolumeAvailable if volume.Spec.ClaimRef != nil { claim, err := binderClient.GetPersistentVolumeClaim(volume.Spec.ClaimRef.Namespace, volume.Spec.ClaimRef.Name) - if errors.IsNotFound(err) || (claim != nil && claim.UID != volume.Spec.ClaimRef.UID) { + switch { + case err != nil && !errors.IsNotFound(err): + return fmt.Errorf("Error getting PersistentVolumeClaim[%s/%s]: %v", volume.Spec.ClaimRef.Namespace, volume.Spec.ClaimRef.Name, err) + case errors.IsNotFound(err) || (claim != nil && claim.UID != volume.Spec.ClaimRef.UID): if volume.Spec.PersistentVolumeReclaimPolicy == api.PersistentVolumeReclaimRecycle { // Pending volumes that have a ClaimRef where the claim is missing were recently recycled. // The Recycler set the phase to VolumePending to start the volume at the beginning of this lifecycle. @@ -265,8 +269,6 @@ func syncVolume(volumeIndex *persistentVolumeOrderedIndex, binderClient binderCl // Mark the volume as Released, it will be deleted. nextPhase = api.VolumeReleased } - } else if err != nil { - return fmt.Errorf("Error getting PersistentVolumeClaim[%s/%s]: %v", volume.Spec.ClaimRef.Namespace, volume.Spec.ClaimRef.Name, err) } // Dynamically provisioned claims remain Pending until its volume is completely provisioned. diff --git a/pkg/controller/persistentvolume/persistentvolume_claim_binder_controller_test.go b/pkg/controller/persistentvolume/persistentvolume_claim_binder_controller_test.go index 37ef08c15bd..f01908c7f7f 100644 --- a/pkg/controller/persistentvolume/persistentvolume_claim_binder_controller_test.go +++ b/pkg/controller/persistentvolume/persistentvolume_claim_binder_controller_test.go @@ -31,6 +31,7 @@ import ( "k8s.io/kubernetes/pkg/client/cache" "k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset/fake" "k8s.io/kubernetes/pkg/client/testing/core" + "k8s.io/kubernetes/pkg/types" utiltesting "k8s.io/kubernetes/pkg/util/testing" "k8s.io/kubernetes/pkg/volume" "k8s.io/kubernetes/pkg/volume/host_path" @@ -566,6 +567,107 @@ func TestCasting(t *testing.T) { binder.updateClaim(pvc, pvc) } +func TestRecycledPersistentVolumeUID(t *testing.T) { + tmpDir, err := utiltesting.MkTmpdir("claimbinder-test") + if err != nil { + t.Fatalf("error creating temp dir: %v", err) + } + defer os.RemoveAll(tmpDir) + + codec := api.Codecs.UniversalDecoder() + o := core.NewObjects(api.Scheme, codec) + if err := core.AddObjectsFromPath("../../../docs/user-guide/persistent-volumes/claims/claim-01.yaml", o, codec); err != nil { + t.Fatal(err) + } + if err := core.AddObjectsFromPath("../../../docs/user-guide/persistent-volumes/volumes/local-01.yaml", o, codec); err != nil { + t.Fatal(err) + } + + clientset := &fake.Clientset{} + clientset.AddReactor("*", "*", core.ObjectReaction(o, registered.RESTMapper())) + + pv, err := clientset.Core().PersistentVolumes().Get("any") + if err != nil { + t.Errorf("Unexpected error getting PV from client: %v", err) + } + pv.Spec.PersistentVolumeReclaimPolicy = api.PersistentVolumeReclaimRecycle + if err != nil { + t.Errorf("Unexpected error getting PV from client: %v", err) + } + pv.ObjectMeta.SelfLink = testapi.Default.SelfLink("pv", "") + + // the default value of the PV is Pending. if processed at least once, its status in etcd is Available. + // There was a bug where only Pending volumes were being indexed and made ready for claims. + // Test that !Pending gets correctly added + pv.Status.Phase = api.VolumeAvailable + + claim, error := clientset.Core().PersistentVolumeClaims("ns").Get("any") + if error != nil { + t.Errorf("Unexpected error getting PVC from client: %v", err) + } + claim.ObjectMeta.SelfLink = testapi.Default.SelfLink("pvc", "") + claim.ObjectMeta.UID = types.UID("uid1") + + volumeIndex := NewPersistentVolumeOrderedIndex() + mockClient := &mockBinderClient{ + volume: pv, + claim: claim, + } + + plugMgr := volume.VolumePluginMgr{} + plugMgr.InitPlugins(host_path.ProbeRecyclableVolumePlugins(newMockRecycler, volume.VolumeConfig{}), volumetest.NewFakeVolumeHost(tmpDir, nil, nil)) + + recycler := &PersistentVolumeRecycler{ + kubeClient: clientset, + client: mockClient, + pluginMgr: plugMgr, + } + + // adds the volume to the index, making the volume available + syncVolume(volumeIndex, mockClient, pv) + if mockClient.volume.Status.Phase != api.VolumeAvailable { + t.Errorf("Expected phase %s but got %s", api.VolumeAvailable, mockClient.volume.Status.Phase) + } + + // add the claim to fake API server + mockClient.UpdatePersistentVolumeClaim(claim) + // an initial sync for a claim will bind it to an unbound volume + syncClaim(volumeIndex, mockClient, claim) + + // pretend the user deleted their claim. periodic resync picks it up. + mockClient.claim = nil + syncVolume(volumeIndex, mockClient, mockClient.volume) + + if mockClient.volume.Status.Phase != api.VolumeReleased { + t.Errorf("Expected phase %s but got %s", api.VolumeReleased, mockClient.volume.Status.Phase) + } + + // released volumes with a PersistentVolumeReclaimPolicy (recycle/delete) can have further processing + err = recycler.reclaimVolume(mockClient.volume) + if err != nil { + t.Errorf("Unexpected error reclaiming volume: %+v", err) + } + if mockClient.volume.Status.Phase != api.VolumePending { + t.Errorf("Expected phase %s but got %s", api.VolumePending, mockClient.volume.Status.Phase) + } + + // after the recycling changes the phase to Pending, the binder picks up again + // to remove any vestiges of binding and make the volume Available again + // + // explicitly set the claim's UID to a different value to ensure that a new claim with the same + // name as what the PV was previously bound still yields an available volume + claim.ObjectMeta.UID = types.UID("uid2") + mockClient.claim = claim + syncVolume(volumeIndex, mockClient, mockClient.volume) + + if mockClient.volume.Status.Phase != api.VolumeAvailable { + t.Errorf("Expected phase %s but got %s", api.VolumeAvailable, mockClient.volume.Status.Phase) + } + if mockClient.volume.Spec.ClaimRef != nil { + t.Errorf("Expected nil ClaimRef: %+v", mockClient.volume.Spec.ClaimRef) + } +} + type mockBinderClient struct { volume *api.PersistentVolume claim *api.PersistentVolumeClaim