From 0ad846cb18df4ae387b3f44647ada67b7f3dc0e2 Mon Sep 17 00:00:00 2001 From: Hemant Kumar Date: Tue, 26 Sep 2017 22:15:00 -0400 Subject: [PATCH] Add documentation comments for volume expand controller These comments help clear out some of the design choices made in code. --- pkg/controller/volume/expand/cache/volume_resize_map.go | 5 +++++ pkg/controller/volume/expand/pvc_populator.go | 5 +++++ pkg/volume/util/operationexecutor/operation_generator.go | 5 +++++ 3 files changed, 15 insertions(+) diff --git a/pkg/controller/volume/expand/cache/volume_resize_map.go b/pkg/controller/volume/expand/cache/volume_resize_map.go index 5676192bcc..2645a294c2 100644 --- a/pkg/controller/volume/expand/cache/volume_resize_map.go +++ b/pkg/controller/volume/expand/cache/volume_resize_map.go @@ -88,6 +88,11 @@ func NewVolumeResizeMap(kubeClient clientset.Interface) VolumeResizeMap { } // AddPVCUpdate adds pvc for resizing +// This function intentionally allows addition of PVCs for which pv.Spec.Size >= pvc.Spec.Size, +// the reason being - lack of transaction in k8s means after successful resize, we can't guarantee that when we update PV, +// pvc update will be successful too and after resize we alyways update PV first. +// If for some reason we weren't able to update PVC after successful resize, then we are going to reprocess +// the PVC and hopefully after a no-op resize in volume plugin, PVC will be updated with right values as well. func (resizeMap *volumeResizeMap) AddPVCUpdate(pvc *v1.PersistentVolumeClaim, pv *v1.PersistentVolume) { if pv.Spec.ClaimRef == nil || pvc.Namespace != pv.Spec.ClaimRef.Namespace || pvc.Name != pv.Spec.ClaimRef.Name { glog.V(4).Infof("Persistent Volume is not bound to PVC being updated : %s", util.ClaimToClaimKey(pvc)) diff --git a/pkg/controller/volume/expand/pvc_populator.go b/pkg/controller/volume/expand/pvc_populator.go index 220e1092f7..bf46175f1e 100644 --- a/pkg/controller/volume/expand/pvc_populator.go +++ b/pkg/controller/volume/expand/pvc_populator.go @@ -80,6 +80,11 @@ func (populator *pvcPopulator) Sync() { glog.V(5).Infof("Error getting persistent volume for pvc %q : %v", pvc.UID, err) continue } + // We are only going to add PVCs which are: + // - bound + // - pvc.Spec.Size > pvc.Status.Size + // These 2 checks are already performed in AddPVCUpdate function before adding pvc for resize + // and hence we do not repeat those checks here. populator.resizeMap.AddPVCUpdate(pvc, pv) } } diff --git a/pkg/volume/util/operationexecutor/operation_generator.go b/pkg/volume/util/operationexecutor/operation_generator.go index 93b05ae6dd..e7e289b93f 100644 --- a/pkg/volume/util/operationexecutor/operation_generator.go +++ b/pkg/volume/util/operationexecutor/operation_generator.go @@ -756,6 +756,9 @@ func (og *operationGenerator) GenerateExpandVolumeFunc( return expandErr } newSize = updatedSize + // k8s doesn't have transactions, we can't guarantee that after updating PV - updating PVC will be + // successful, that is why all PVCs for which pvc.Spec.Size > pvc.Status.Size must be reprocessed + // until they reflect user requested size in pvc.Status.Size updateErr := resizeMap.UpdatePVSize(pvcWithResizeRequest, newSize) if updateErr != nil { @@ -766,6 +769,8 @@ func (og *operationGenerator) GenerateExpandVolumeFunc( } // No Cloudprovider resize needed, lets mark resizing as done + // Rest of the volume expand controller code will assume PVC as *not* resized until pvc.Status.Size + // reflects user requested size. if !volumePlugin.RequiresFSResize() { glog.V(4).Infof("Controller resizing done for PVC %s", pvcWithResizeRequest.QualifiedName()) err := resizeMap.MarkAsResized(pvcWithResizeRequest, newSize)