Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix PVC.Status.Capacity and AccessModes after binding #29982

Merged
merged 1 commit into from
Aug 8, 2016
Merged

Fix PVC.Status.Capacity and AccessModes after binding #29982

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 14 additions & 14 deletions pkg/controller/volume/persistentvolume/binder_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ func TestSync(t *testing.T) {
newVolume("volume1-5_2", "1Gi", "", "", api.VolumePending, api.PersistentVolumeReclaimRetain),
},
newClaimArray("claim1-5", "uid1-5", "1Gi", "", api.ClaimPending),
newClaimArray("claim1-5", "uid1-5", "1Gi", "volume1-5_1", api.ClaimBound, annBoundByController, annBindCompleted),
withExpectedCapacity("10Gi", newClaimArray("claim1-5", "uid1-5", "1Gi", "volume1-5_1", api.ClaimBound, annBoundByController, annBindCompleted)),
noevents, noerrors, testSyncClaim,
},
{
Expand All @@ -109,7 +109,7 @@ func TestSync(t *testing.T) {
newVolume("volume1-6_2", "1Gi", "", "", api.VolumePending, api.PersistentVolumeReclaimRetain),
},
newClaimArray("claim1-6", "uid1-6", "1Gi", "", api.ClaimPending),
newClaimArray("claim1-6", "uid1-6", "1Gi", "volume1-6_1", api.ClaimBound, annBoundByController, annBindCompleted),
withExpectedCapacity("10Gi", newClaimArray("claim1-6", "uid1-6", "1Gi", "volume1-6_1", api.ClaimBound, annBoundByController, annBindCompleted)),
noevents, noerrors, testSyncClaim,
},
{
Expand Down Expand Up @@ -199,8 +199,8 @@ func TestSync(t *testing.T) {
"2-3 - claim prebound to unbound volume",
newVolumeArray("volume2-3", "1Gi", "", "", api.VolumePending, api.PersistentVolumeReclaimRetain),
newVolumeArray("volume2-3", "1Gi", "uid2-3", "claim2-3", api.VolumeBound, api.PersistentVolumeReclaimRetain, annBoundByController),
newClaimArray("claim2-3", "uid2-3", "10Gi", "volume2-3", api.ClaimPending),
newClaimArray("claim2-3", "uid2-3", "10Gi", "volume2-3", api.ClaimBound, annBindCompleted),
newClaimArray("claim2-3", "uid2-3", "1Gi", "volume2-3", api.ClaimPending),
newClaimArray("claim2-3", "uid2-3", "1Gi", "volume2-3", api.ClaimBound, annBindCompleted),
noevents, noerrors, testSyncClaim,
},
{
Expand All @@ -209,8 +209,8 @@ func TestSync(t *testing.T) {
"2-4 - claim prebound to prebound volume by name",
newVolumeArray("volume2-4", "1Gi", "", "claim2-4", api.VolumePending, api.PersistentVolumeReclaimRetain),
newVolumeArray("volume2-4", "1Gi", "uid2-4", "claim2-4", api.VolumeBound, api.PersistentVolumeReclaimRetain),
newClaimArray("claim2-4", "uid2-4", "10Gi", "volume2-4", api.ClaimPending),
newClaimArray("claim2-4", "uid2-4", "10Gi", "volume2-4", api.ClaimBound, annBindCompleted),
newClaimArray("claim2-4", "uid2-4", "1Gi", "volume2-4", api.ClaimPending),
newClaimArray("claim2-4", "uid2-4", "1Gi", "volume2-4", api.ClaimBound, annBindCompleted),
noevents, noerrors, testSyncClaim,
},
{
Expand All @@ -220,8 +220,8 @@ func TestSync(t *testing.T) {
"2-5 - claim prebound to prebound volume by UID",
newVolumeArray("volume2-5", "1Gi", "uid2-5", "claim2-5", api.VolumePending, api.PersistentVolumeReclaimRetain),
newVolumeArray("volume2-5", "1Gi", "uid2-5", "claim2-5", api.VolumeBound, api.PersistentVolumeReclaimRetain),
newClaimArray("claim2-5", "uid2-5", "10Gi", "volume2-5", api.ClaimPending),
newClaimArray("claim2-5", "uid2-5", "10Gi", "volume2-5", api.ClaimBound, annBindCompleted),
newClaimArray("claim2-5", "uid2-5", "1Gi", "volume2-5", api.ClaimPending),
newClaimArray("claim2-5", "uid2-5", "1Gi", "volume2-5", api.ClaimBound, annBindCompleted),
noevents, noerrors, testSyncClaim,
},
{
Expand All @@ -230,8 +230,8 @@ func TestSync(t *testing.T) {
"2-6 - claim prebound to already bound volume",
newVolumeArray("volume2-6", "1Gi", "uid2-6_1", "claim2-6_1", api.VolumeBound, api.PersistentVolumeReclaimRetain),
newVolumeArray("volume2-6", "1Gi", "uid2-6_1", "claim2-6_1", api.VolumeBound, api.PersistentVolumeReclaimRetain),
newClaimArray("claim2-6", "uid2-6", "10Gi", "volume2-6", api.ClaimBound),
newClaimArray("claim2-6", "uid2-6", "10Gi", "volume2-6", api.ClaimPending),
newClaimArray("claim2-6", "uid2-6", "1Gi", "volume2-6", api.ClaimBound),
newClaimArray("claim2-6", "uid2-6", "1Gi", "volume2-6", api.ClaimPending),
noevents, noerrors, testSyncClaim,
},
{
Expand All @@ -240,8 +240,8 @@ func TestSync(t *testing.T) {
"2-7 - claim bound by controller to already bound volume",
newVolumeArray("volume2-7", "1Gi", "uid2-7_1", "claim2-7_1", api.VolumeBound, api.PersistentVolumeReclaimRetain),
newVolumeArray("volume2-7", "1Gi", "uid2-7_1", "claim2-7_1", api.VolumeBound, api.PersistentVolumeReclaimRetain),
newClaimArray("claim2-7", "uid2-7", "10Gi", "volume2-7", api.ClaimBound, annBoundByController),
newClaimArray("claim2-7", "uid2-7", "10Gi", "volume2-7", api.ClaimBound, annBoundByController),
newClaimArray("claim2-7", "uid2-7", "1Gi", "volume2-7", api.ClaimBound, annBoundByController),
newClaimArray("claim2-7", "uid2-7", "1Gi", "volume2-7", api.ClaimBound, annBoundByController),
noevents, noerrors, testSyncClaimError,
},
{
Expand All @@ -251,8 +251,8 @@ func TestSync(t *testing.T) {
"2-8 - claim prebound to unbound volume that does not match the selector",
newVolumeArray("volume2-3", "1Gi", "", "", api.VolumePending, api.PersistentVolumeReclaimRetain),
newVolumeArray("volume2-3", "1Gi", "uid2-3", "claim2-3", api.VolumeBound, api.PersistentVolumeReclaimRetain, annBoundByController),
withLabelSelector(labels, newClaimArray("claim2-3", "uid2-3", "10Gi", "volume2-3", api.ClaimPending)),
withLabelSelector(labels, newClaimArray("claim2-3", "uid2-3", "10Gi", "volume2-3", api.ClaimBound, annBindCompleted)),
withLabelSelector(labels, newClaimArray("claim2-3", "uid2-3", "1Gi", "volume2-3", api.ClaimPending)),
withLabelSelector(labels, newClaimArray("claim2-3", "uid2-3", "1Gi", "volume2-3", api.ClaimBound, annBindCompleted)),
noevents, noerrors, testSyncClaim,
},

Expand Down
95 changes: 70 additions & 25 deletions pkg/controller/volume/persistentvolume/controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package persistentvolume

import (
"fmt"
"reflect"
"time"

"k8s.io/kubernetes/pkg/api"
Expand Down Expand Up @@ -215,7 +216,7 @@ func (ctrl *PersistentVolumeController) syncUnboundClaim(claim *api.PersistentVo
}
// 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 {
if _, err = ctrl.updateClaimStatus(claim, api.ClaimPending, nil); err != nil {
return err
}
return nil
Expand Down Expand Up @@ -244,7 +245,7 @@ func (ctrl *PersistentVolumeController) syncUnboundClaim(claim *api.PersistentVo
// 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 {
if _, err = ctrl.updateClaimStatus(claim, api.ClaimPending, nil); err != nil {
return err
}
return nil
Expand Down Expand Up @@ -282,7 +283,7 @@ func (ctrl *PersistentVolumeController) syncUnboundClaim(claim *api.PersistentVo
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 {
if _, err = ctrl.updateClaimStatus(claim, api.ClaimPending, nil); err != nil {
return err
}
return nil
Expand All @@ -306,7 +307,7 @@ func (ctrl *PersistentVolumeController) syncBoundClaim(claim *api.PersistentVolu
// [Unit test set 3]
if claim.Spec.VolumeName == "" {
// Claim was bound before but not any more.
if _, err := ctrl.updateClaimPhaseWithEvent(claim, api.ClaimLost, api.EventTypeWarning, "ClaimLost", "Bound claim has lost reference to PersistentVolume. Data on the volume is lost!"); err != nil {
if _, err := ctrl.updateClaimStatusWithEvent(claim, api.ClaimLost, nil, api.EventTypeWarning, "ClaimLost", "Bound claim has lost reference to PersistentVolume. Data on the volume is lost!"); err != nil {
return err
}
return nil
Expand All @@ -317,7 +318,7 @@ func (ctrl *PersistentVolumeController) syncBoundClaim(claim *api.PersistentVolu
}
if !found {
// Claim is bound to a non-existing volume.
if _, err = ctrl.updateClaimPhaseWithEvent(claim, api.ClaimLost, api.EventTypeWarning, "ClaimLost", "Bound claim has lost its PersistentVolume. Data on the volume is lost!"); err != nil {
if _, err = ctrl.updateClaimStatusWithEvent(claim, api.ClaimLost, nil, api.EventTypeWarning, "ClaimLost", "Bound claim has lost its PersistentVolume. Data on the volume is lost!"); err != nil {
return err
}
return nil
Expand Down Expand Up @@ -354,7 +355,7 @@ func (ctrl *PersistentVolumeController) syncBoundClaim(claim *api.PersistentVolu
// Claim is bound but volume has a different claimant.
// Set the claim phase to 'Lost', which is a terminal
// phase.
if _, err = ctrl.updateClaimPhaseWithEvent(claim, api.ClaimLost, api.EventTypeWarning, "ClaimMisbound", "Two claims are bound to the same volume, this one is bound incorrectly"); err != nil {
if _, err = ctrl.updateClaimStatusWithEvent(claim, api.ClaimLost, nil, api.EventTypeWarning, "ClaimMisbound", "Two claims are bound to the same volume, this one is bound incorrectly"); err != nil {
return err
}
return nil
Expand Down Expand Up @@ -517,14 +518,15 @@ func (ctrl *PersistentVolumeController) syncVolume(volume *api.PersistentVolume)
}
}

// updateClaimPhase saves new claim phase to API server.
func (ctrl *PersistentVolumeController) updateClaimPhase(claim *api.PersistentVolumeClaim, phase api.PersistentVolumeClaimPhase) (*api.PersistentVolumeClaim, error) {
glog.V(4).Infof("updating PersistentVolumeClaim[%s]: set phase %s", claimToClaimKey(claim), phase)
if claim.Status.Phase == phase {
// Nothing to do.
glog.V(4).Infof("updating PersistentVolumeClaim[%s]: phase %s already set", claimToClaimKey(claim), phase)
return claim, nil
}
// updateClaimStatus saves new claim.Status to API server.
// Parameters:
// claim - claim to update
// phasephase - phase to set
// volume - volume which Capacity is set into claim.Status.Capacity
func (ctrl *PersistentVolumeController) updateClaimStatus(claim *api.PersistentVolumeClaim, phase api.PersistentVolumeClaimPhase, volume *api.PersistentVolume) (*api.PersistentVolumeClaim, error) {
glog.V(4).Infof("updating PersistentVolumeClaim[%s] status: set phase %s", claimToClaimKey(claim), phase)

dirty := false

clone, err := conversion.NewCloner().DeepCopy(claim)
if err != nil {
Expand All @@ -535,33 +537,76 @@ func (ctrl *PersistentVolumeController) updateClaimPhase(claim *api.PersistentVo
return nil, fmt.Errorf("Unexpected claim cast error : %v", claimClone)
}

claimClone.Status.Phase = phase
if claim.Status.Phase != phase {
claimClone.Status.Phase = phase
dirty = true
}

if volume == nil {
// Need to reset AccessModes and Capacity
if claim.Status.AccessModes != nil {
claimClone.Status.AccessModes = nil
dirty = true
}
if claim.Status.Capacity != nil {
claimClone.Status.Capacity = nil
dirty = true
}
} else {
// Need to update AccessModes and Capacity
if !reflect.DeepEqual(claim.Status.AccessModes, volume.Spec.AccessModes) {
claimClone.Status.AccessModes = volume.Spec.AccessModes
dirty = true
}

volumeCap, ok := volume.Spec.Capacity[api.ResourceStorage]
if !ok {
return nil, fmt.Errorf("PersistentVolume %q is without a storage capacity", volume.Name)
}
claimCap, ok := claim.Status.Capacity[api.ResourceStorage]
if !ok || volumeCap.Cmp(claimCap) != 0 {
claimClone.Status.Capacity = volume.Spec.Capacity
dirty = true
}
}

if !dirty {
// Nothing to do.
glog.V(4).Infof("updating PersistentVolumeClaim[%s] status: phase %s already set", claimToClaimKey(claim), phase)
return claim, nil
}

newClaim, err := ctrl.kubeClient.Core().PersistentVolumeClaims(claimClone.Namespace).UpdateStatus(claimClone)
if err != nil {
glog.V(4).Infof("updating PersistentVolumeClaim[%s]: set phase %s failed: %v", claimToClaimKey(claim), phase, err)
glog.V(4).Infof("updating PersistentVolumeClaim[%s] status: set phase %s failed: %v", claimToClaimKey(claim), phase, err)
return newClaim, err
}
_, err = ctrl.storeClaimUpdate(newClaim)
if err != nil {
glog.V(4).Infof("updating PersistentVolumeClaim[%s]: cannot update internal cache: %v", claimToClaimKey(claim), err)
glog.V(4).Infof("updating PersistentVolumeClaim[%s] status: cannot update internal cache: %v", claimToClaimKey(claim), err)
return newClaim, err
}
glog.V(2).Infof("claim %q entered phase %q", claimToClaimKey(claim), phase)
return newClaim, nil
}

// updateClaimPhaseWithEvent saves new claim phase to API server and emits given
// event on the claim. It saves the phase and emits the event only when the
// phase has actually changed from the version saved in API server.
func (ctrl *PersistentVolumeController) updateClaimPhaseWithEvent(claim *api.PersistentVolumeClaim, phase api.PersistentVolumeClaimPhase, eventtype, reason, message string) (*api.PersistentVolumeClaim, error) {
glog.V(4).Infof("updating updateClaimPhaseWithEvent[%s]: set phase %s", claimToClaimKey(claim), phase)
// updateClaimStatusWithEvent saves new claim.Status to API server and emits
// given event on the claim. It saves the status and emits the event only when
// the status has actually changed from the version saved in API server.
// Parameters:
// claim - claim to update
// phasephase - phase to set
// volume - volume which Capacity is set into claim.Status.Capacity
// eventtype, reason, message - event to send, see EventRecorder.Event()
func (ctrl *PersistentVolumeController) updateClaimStatusWithEvent(claim *api.PersistentVolumeClaim, phase api.PersistentVolumeClaimPhase, volume *api.PersistentVolume, eventtype, reason, message string) (*api.PersistentVolumeClaim, error) {
glog.V(4).Infof("updating updateClaimStatusWithEvent[%s]: set phase %s", claimToClaimKey(claim), phase)
if claim.Status.Phase == phase {
// Nothing to do.
glog.V(4).Infof("updating updateClaimPhaseWithEvent[%s]: phase %s already set", claimToClaimKey(claim), phase)
glog.V(4).Infof("updating updateClaimStatusWithEvent[%s]: phase %s already set", claimToClaimKey(claim), phase)
return claim, nil
}

newClaim, err := ctrl.updateClaimPhase(claim, phase)
newClaim, err := ctrl.updateClaimStatus(claim, phase, volume)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -791,7 +836,7 @@ func (ctrl *PersistentVolumeController) bind(volume *api.PersistentVolume, claim
}
claim = updatedClaim

if updatedClaim, err = ctrl.updateClaimPhase(claim, api.ClaimBound); err != nil {
if updatedClaim, err = ctrl.updateClaimStatus(claim, api.ClaimBound, volume); err != nil {
glog.V(3).Infof("error binding volume %q to claim %q: failed saving the claim status: %v", volume.Name, claimToClaimKey(claim), err)
return err
}
Expand Down
Loading