Fix code to handle pod updates

kubernetes-csi · Jun 4, 2020 · 1bc70d8 · 1bc70d8
1 parent f7cf14d
commit 1bc70d8
Show file tree

Hide file tree

Showing 2 changed files with 42 additions and 34 deletions.
diff --git 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,20 +131,29 @@ 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)
 }
 
 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
@@ -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)