From 9eb2831954508202210fc09c51a767c54b664711 Mon Sep 17 00:00:00 2001 From: Xiang Li Date: Mon, 11 Jul 2016 19:22:14 -0700 Subject: [PATCH] controller/volume: simplify sync logic in syncUnboundClaim --- .../volume/persistentvolume/controller.go | 163 ++++++++---------- 1 file changed, 75 insertions(+), 88 deletions(-) diff --git a/pkg/controller/volume/persistentvolume/controller.go b/pkg/controller/volume/persistentvolume/controller.go index 0fff15b19c3..abbd2a9cc53 100644 --- a/pkg/controller/volume/persistentvolume/controller.go +++ b/pkg/controller/volume/persistentvolume/controller.go @@ -150,9 +150,8 @@ func (ctrl *PersistentVolumeController) syncClaim(claim *api.PersistentVolumeCla if !hasAnnotation(claim.ObjectMeta, annBindCompleted) { return ctrl.syncUnboundClaim(claim) - } else { - return ctrl.syncBoundClaim(claim) } + return ctrl.syncBoundClaim(claim) } // syncUnboundClaim is the main controller method to decide what to do with an @@ -168,98 +167,86 @@ func (ctrl *PersistentVolumeController) syncUnboundClaim(claim *api.PersistentVo glog.V(2).Infof("synchronizing unbound PersistentVolumeClaim[%s]: Error finding PV for claim: %v", claimToClaimKey(claim), err) return fmt.Errorf("Error finding PV for claim %q: %v", claimToClaimKey(claim), err) } - if volume == nil { - glog.V(4).Infof("synchronizing unbound PersistentVolumeClaim[%s]: no volume found", claimToClaimKey(claim)) - // No PV could be found - // OBSERVATION: pvc is "Pending", will retry - if hasAnnotation(claim.ObjectMeta, annClass) { - if err = ctrl.provisionClaim(claim); err != nil { - return err - } - return nil - } - // Mark the claim as Pending and try to find a match in the next - // periodic syncClaim - if _, err = ctrl.updateClaimPhase(claim, api.ClaimPending); err != nil { - return err - } - return nil - } else /* pv != nil */ { + + if volume != nil { // Found a PV for this claim // OBSERVATION: pvc is "Pending", pv is "Available" glog.V(4).Infof("synchronizing unbound PersistentVolumeClaim[%s]: volume %q found: %s", claimToClaimKey(claim), volume.Name, getVolumeStatusForLogging(volume)) - if err = ctrl.bind(volume, claim); err != nil { - // On any error saving the volume or the claim, subsequent - // syncClaim will finish the binding. - return err - } - // OBSERVATION: claim is "Bound", pv is "Bound" - return nil - } - } else /* pvc.Spec.VolumeName != nil */ { - // [Unit test set 2] - // User asked for a specific PV. - glog.V(4).Infof("synchronizing unbound PersistentVolumeClaim[%s]: volume %q requested", claimToClaimKey(claim), claim.Spec.VolumeName) - obj, found, err := ctrl.volumes.store.GetByKey(claim.Spec.VolumeName) - if err != nil { - return err - } - if !found { - // User asked for a PV that does not exist. - // OBSERVATION: pvc is "Pending" - // Retry later. - glog.V(4).Infof("synchronizing unbound PersistentVolumeClaim[%s]: volume %q requested and not found, will try again next time", claimToClaimKey(claim), claim.Spec.VolumeName) - if _, err = ctrl.updateClaimPhase(claim, api.ClaimPending); err != nil { - return err - } - return nil - } else { - volume, ok := obj.(*api.PersistentVolume) - if !ok { - return fmt.Errorf("Cannot convert object from volume cache to volume %q!?: %+v", claim.Spec.VolumeName, obj) - } - glog.V(4).Infof("synchronizing unbound PersistentVolumeClaim[%s]: volume %q requested and found: %s", claimToClaimKey(claim), claim.Spec.VolumeName, getVolumeStatusForLogging(volume)) - if volume.Spec.ClaimRef == nil { - // User asked for a PV that is not claimed - // OBSERVATION: pvc is "Pending", pv is "Available" - glog.V(4).Infof("synchronizing unbound PersistentVolumeClaim[%s]: volume is unbound, binding", claimToClaimKey(claim)) - if err = ctrl.bind(volume, claim); err != nil { - // On any error saving the volume or the claim, subsequent - // syncClaim will finish the binding. - return err - } - // OBSERVATION: pvc is "Bound", pv is "Bound" - return nil - } else if isVolumeBoundToClaim(volume, claim) { - // User asked for a PV that is claimed by this PVC - // OBSERVATION: pvc is "Pending", pv is "Bound" - glog.V(4).Infof("synchronizing unbound PersistentVolumeClaim[%s]: volume already bound, finishing the binding", claimToClaimKey(claim)) - // Finish the volume binding by adding claim UID. - if err = ctrl.bind(volume, claim); err != nil { - return err - } - // OBSERVATION: pvc is "Bound", pv is "Bound" - return nil - } else { - // User asked for a PV that is claimed by someone else - // OBSERVATION: pvc is "Pending", pv is "Bound" - if !hasAnnotation(claim.ObjectMeta, annBoundByController) { - glog.V(4).Infof("synchronizing unbound PersistentVolumeClaim[%s]: volume already bound to different claim by user, will retry later", claimToClaimKey(claim)) - // User asked for a specific PV, retry later - if _, err = ctrl.updateClaimPhase(claim, api.ClaimPending); err != nil { - return err - } - return nil - } else { - // This should never happen because someone had to remove - // annBindCompleted annotation on the claim. - glog.V(4).Infof("synchronizing unbound PersistentVolumeClaim[%s]: volume already bound to different claim %q by controller, THIS SHOULD NEVER HAPPEN", claimToClaimKey(claim), claimrefToClaimKey(volume.Spec.ClaimRef)) - return fmt.Errorf("Invalid binding of claim %q to volume %q: volume already claimed by %q", claimToClaimKey(claim), claim.Spec.VolumeName, claimrefToClaimKey(volume.Spec.ClaimRef)) - } - } + // On any error saving the volume or the claim, subsequent + // syncClaim will finish the binding. + // OBSERVATION: claim is "Bound", pv is "Bound" when no error + return ctrl.bind(volume, claim) } + + glog.V(4).Infof("synchronizing unbound PersistentVolumeClaim[%s]: no volume found", claimToClaimKey(claim)) + // No PV could be found + // OBSERVATION: pvc is "Pending", will retry + if hasAnnotation(claim.ObjectMeta, annClass) { + return ctrl.provisionClaim(claim) + } + // Mark the claim as Pending and try to find a match in the next periodic syncClaim + _, err = ctrl.updateClaimPhase(claim, api.ClaimPending) + return err } + + /* pvc.Spec.VolumeName != nil */ + // [Unit test set 2] + // User asked for a specific PV. + glog.V(4).Infof("synchronizing unbound PersistentVolumeClaim[%s]: volume %q requested", claimToClaimKey(claim), claim.Spec.VolumeName) + obj, found, err := ctrl.volumes.store.GetByKey(claim.Spec.VolumeName) + if err != nil { + return err + } + if !found { + // User asked for a PV that does not exist. + // OBSERVATION: pvc is "Pending" + // Retry later. + glog.V(4).Infof("synchronizing unbound PersistentVolumeClaim[%s]: volume %q requested and not found, will try again next time", claimToClaimKey(claim), claim.Spec.VolumeName) + _, err = ctrl.updateClaimPhase(claim, api.ClaimPending) + return err + } + + volume, ok := obj.(*api.PersistentVolume) + if !ok { + return fmt.Errorf("Cannot convert object from volume cache to volume %q!?: %+v", claim.Spec.VolumeName, obj) + } + glog.V(4).Infof("synchronizing unbound PersistentVolumeClaim[%s]: volume %q requested and found: %s", claimToClaimKey(claim), claim.Spec.VolumeName, getVolumeStatusForLogging(volume)) + + if volume.Spec.ClaimRef == nil { + // User asked for a PV that is not claimed + // OBSERVATION: pvc is "Pending", pv is "Available" + glog.V(4).Infof("synchronizing unbound PersistentVolumeClaim[%s]: volume is unbound, binding", claimToClaimKey(claim)) + + // On any error saving the volume or the claim, subsequent + // syncClaim will finish the binding. + // OBSERVATION: pvc is "Bound", pv is "Bound" when no error + return ctrl.bind(volume, claim) + } + + if isVolumeBoundToClaim(volume, claim) { + // User asked for a PV that is claimed by this PVC + // OBSERVATION: pvc is "Pending", pv is "Bound" + glog.V(4).Infof("synchronizing unbound PersistentVolumeClaim[%s]: volume already bound, finishing the binding", claimToClaimKey(claim)) + + // Finish the volume binding by adding claim UID. + // OBSERVATION: pvc is "Bound", pv is "Bound" when no error + return ctrl.bind(volume, claim) + } + + // User asked for a PV that is claimed by someone else + // OBSERVATION: pvc is "Pending", pv is "Bound" + if !hasAnnotation(claim.ObjectMeta, annBoundByController) { + glog.V(4).Infof("synchronizing unbound PersistentVolumeClaim[%s]: volume already bound to different claim by user, will retry later", claimToClaimKey(claim)) + // User asked for a specific PV, retry later + _, err = ctrl.updateClaimPhase(claim, api.ClaimPending) + return err + } + + // This should never happen because someone had to remove + // annBindCompleted annotation on the claim. + glog.V(4).Infof("synchronizing unbound PersistentVolumeClaim[%s]: volume already bound to different claim %q by controller, THIS SHOULD NEVER HAPPEN", claimToClaimKey(claim), claimrefToClaimKey(volume.Spec.ClaimRef)) + return fmt.Errorf("Invalid binding of claim %q to volume %q: volume already claimed by %q", claimToClaimKey(claim), claim.Spec.VolumeName, claimrefToClaimKey(volume.Spec.ClaimRef)) } // syncBoundClaim is the main controller method to decide what to do with a