From 1bc70d8971a252c6a7df0413b364e801cb4d37eb Mon Sep 17 00:00:00 2001 From: Hemant Kumar Date: Thu, 4 Jun 2020 15:31:37 -0400 Subject: [PATCH] Fix code to handle pod updates --- pkg/controller/controller.go | 25 ++++++++++----- pkg/controller/controller_test.go | 51 +++++++++++++++---------------- 2 files changed, 42 insertions(+), 34 deletions(-) diff --git a/pkg/controller/controller.go b/pkg/controller/controller.go index 45e0958aa..94dd6f749 100644 --- a/pkg/controller/controller.go +++ b/pkg/controller/controller.go @@ -115,6 +115,7 @@ func NewResizeController( podInformer.Informer().AddEventHandlerWithResyncPeriod(cache.ResourceEventHandlerFuncs{ AddFunc: ctrl.addPod, DeleteFunc: ctrl.deletePod, + UpdateFunc: ctrl.updatePod, }, resyncPeriod) return ctrl @@ -130,7 +131,7 @@ func (ctrl *resizeController) addPVC(obj interface{}) { func (ctrl *resizeController) addPod(obj interface{}) { pod := parsePod(obj) - if pod != nil { + if pod == nil { return } ctrl.usedPVCs.addPod(pod) @@ -138,12 +139,21 @@ func (ctrl *resizeController) addPod(obj interface{}) { func (ctrl *resizeController) deletePod(obj interface{}) { pod := parsePod(obj) - if pod != nil { + if pod == nil { return } ctrl.usedPVCs.removePod(pod) } +func (ctrl *resizeController) updatePod(oldObj, newObj interface{}) { + pod := parsePod(newObj) + if pod == nil { + return + } + + ctrl.usedPVCs.addPod(pod) +} + func (ctrl *resizeController) updatePVC(oldObj, newObj interface{}) { oldPVC, ok := oldObj.(*v1.PersistentVolumeClaim) if !ok || oldPVC == nil { @@ -361,9 +371,9 @@ func (ctrl *resizeController) resizePVC(pvc *v1.PersistentVolumeClaim, pv *v1.Pe // we must not try expansion here if ctrl.usedPVCs.hasInUseErrors(pvc) && ctrl.usedPVCs.checkForUse(pvc) { // Record an event to indicate that resizer is not expanding the pvc - ctrl.eventRecorder.Event(pvc, v1.EventTypeWarning, util.VolumeResizeFailed, - fmt.Sprintf("CSI resizer is not expanding %s because it is in-use", pv.Name)) - return fmt.Errorf("csi resizer is not expanding %s because it is in-use", pv.Name) + msg := fmt.Sprintf("Unable to expand %s because CSI driver %s only supports offline expansion and volume is currently in-use", pv.Name, ctrl.resizer.Name()) + ctrl.eventRecorder.Event(pvc, v1.EventTypeWarning, util.VolumeResizeFailed, msg) + return fmt.Errorf(msg) } // Record an event to indicate that external resizer is resizing this volume. @@ -485,12 +495,12 @@ func parsePod(obj interface{}) *v1.Pod { } pod, ok := obj.(*v1.Pod) if !ok { - tombstone, ok := obj.(cache.DeletedFinalStateUnknown) + staleObj, ok := obj.(cache.DeletedFinalStateUnknown) if !ok { utilruntime.HandleError(fmt.Errorf("couldn't get object from tombstone %#v", obj)) return nil } - pod, ok = tombstone.Obj.(*v1.Pod) + pod, ok = staleObj.Obj.(*v1.Pod) if !ok { utilruntime.HandleError(fmt.Errorf("tombstone contained object that is not a Pod %#v", obj)) return nil @@ -507,6 +517,7 @@ func inUseError(err error) bool { } // if this is a failed precondition error then that means driver does not support expansion // of in-use volumes + // More info - https://github.com/container-storage-interface/spec/blob/master/spec.md#controllerexpandvolume-errors if st.Code() == codes.FailedPrecondition { return true } diff --git a/pkg/controller/controller_test.go b/pkg/controller/controller_test.go index 48836a4a4..7bd57f49b 100644 --- a/pkg/controller/controller_test.go +++ b/pkg/controller/controller_test.go @@ -2,7 +2,6 @@ package controller import ( "context" - "fmt" "testing" "time" @@ -34,7 +33,6 @@ func TestController(t *testing.T) { NodeResize bool CallCSIExpand bool expectBlockVolume bool - expectError bool // is PVC being expanded in-use pvcInUse bool @@ -113,7 +111,6 @@ func TestController(t *testing.T) { CallCSIExpand: false, pvcHasInUseErrors: true, pvcInUse: true, - expectError: true, }, { Name: "Resize PVC, no FS resize, pvc-inuse but no failedprecondition error", @@ -148,20 +145,15 @@ func TestController(t *testing.T) { } } + if test.pvcInUse { + pod := withPVC(test.PVC.Name, pod()) + initialObjects = append(initialObjects, pod) + } + kubeClient, informerFactory := fakeK8s(initialObjects) pvInformer := informerFactory.Core().V1().PersistentVolumes() pvcInformer := informerFactory.Core().V1().PersistentVolumeClaims() - - for _, obj := range initialObjects { - switch obj.(type) { - case *v1.PersistentVolume: - pvInformer.Informer().GetStore().Add(obj) - case *v1.PersistentVolumeClaim: - pvcInformer.Informer().GetStore().Add(obj) - default: - t.Fatalf("Test %s: Unknown initalObject type: %+v", test.Name, obj) - } - } + podInformer := informerFactory.Core().V1().Pods() metricsManager := metrics.NewCSIMetricsManager("" /* driverName */) metricsAddress := "" @@ -182,23 +174,28 @@ func TestController(t *testing.T) { } } - if test.pvcInUse { - pod := withPVC(test.PVC.Name, pod()) - ctrlInstance.usedPVCs.addPod(pod) - if !ctrlInstance.usedPVCs.checkForUse(test.PVC) { - t.Fatalf("pvc %s is not in use", test.PVC.Name) - } - } + stopCh := make(chan struct{}) + informerFactory.Start(stopCh) - err = ctrlInstance.syncPVC(fmt.Sprintf("%s/%s", test.PVC.Namespace, test.PVC.Name)) - if err != nil && !test.expectError { - t.Fatalf("Test %s: Unexpected error: %v", test.Name, err) - } + ctx := context.TODO() + defer ctx.Done() + go controller.Run(1, ctx) - if test.expectError && err == nil { - t.Fatalf("Test %s: expected error got no none", test.Name) + for _, obj := range initialObjects { + switch obj.(type) { + case *v1.PersistentVolume: + pvInformer.Informer().GetStore().Add(obj) + case *v1.PersistentVolumeClaim: + pvcInformer.Informer().GetStore().Add(obj) + case *v1.Pod: + podInformer.Informer().GetStore().Add(obj) + default: + t.Fatalf("Test %s: Unknown initalObject type: %+v", test.Name, obj) + } } + time.Sleep(time.Second * 2) + expandCallCount := client.GetExpandCount() if test.CallCSIExpand && expandCallCount == 0 { t.Fatalf("for %s: expected csi expand call, no csi expand call was made", test.Name)