Fix review comments and add tests

kubernetes-csi · Jan 18, 2022 · 15fcebc · 15fcebc
1 parent 9f08c69
commit 15fcebc
Show file tree

Hide file tree

Showing 3 changed files with 36 additions and 24 deletions.
diff --git a/pkg/controller/expand_and_recover.go b/pkg/controller/expand_and_recover.go
@@ -1,5 +1,5 @@
 /*
-Copyright 2018 The Kubernetes Authors.
+Copyright 2022 The Kubernetes Authors.
 
 Licensed under the Apache License, Version 2.0 (the "License");
 you may not use this file except in compliance with the License.
@@ -31,6 +31,8 @@ func (ctrl *resizeController) expandAndRecover(pvc *v1.PersistentVolumeClaim, pv
 		klog.V(4).Infof("No need to resize PV %q", pv.Name)
 		return pvc, pv, nil, false
 	}
+	// only used as a sentinel value when function returns without
+	// actually performing expansion on the volume.
 	resizeNotCalled := false
 
 	// if we are here that already means pvc.Spec.Size > pvc.Status.Size
@@ -132,7 +134,7 @@ func (ctrl *resizeController) expandAndRecover(pvc *v1.PersistentVolumeClaim, pv
 		// Record an event to indicate that resizer is not expanding the pvc
 		msg := fmt.Sprintf("Unable to expand %s because CSI driver %s only supports offline expansion and volume is currently in-use", util.PVCKey(pvc), ctrl.resizer.Name())
 		ctrl.eventRecorder.Event(pvc, v1.EventTypeWarning, util.VolumeResizeFailed, msg)
-		return pvc, pv, fmt.Errorf(msg), false
+		return pvc, pv, fmt.Errorf(msg), resizeNotCalled
 	}
 
 	// Record an event to indicate that external resizer is resizing this volume.

diff --git a/pkg/controller/expand_and_recover_test.go b/pkg/controller/expand_and_recover_test.go
@@ -19,76 +19,86 @@ import (
 func TestExpandAndRecover(t *testing.T) {
 	fsVolumeMode := v1.PersistentVolumeFilesystem
 	var tests = []struct {
-		name                 string
-		pvc                  *v1.PersistentVolumeClaim
-		pv                   *v1.PersistentVolume
-		recoverFeatureGate   bool
-		disableNodeExpansion bool
+		name                       string
+		pvc                        *v1.PersistentVolumeClaim
+		pv                         *v1.PersistentVolume
+		disableNodeExpansion       bool
+		disableControllerExpansion bool
 		// expectations of test
 		expectedResizeStatus  v1.PersistentVolumeClaimResizeStatus
 		expectedAllocatedSize resource.Quantity
 		expectResizeCall      bool
 	}{
 		{
-			name:                  "pvc.spec.size > pv.spec.size, recover_expansion=on",
+			name:                  "pvc.spec.size > pv.spec.size, resize_status=node_expansion_inprogress",
 			pvc:                   getTestPVC("test-vol0", "2G", "1G", "", v1.PersistentVolumeClaimNoExpansionInProgress),
 			pv:                    createPV(1, "claim01", defaultNS, "test-uid", &fsVolumeMode),
-			recoverFeatureGate:    true,
 			expectedResizeStatus:  v1.PersistentVolumeClaimNodeExpansionPending,
 			expectedAllocatedSize: resource.MustParse("2G"),
 			expectResizeCall:      true,
 		},
 		{
-			name:                  "pvc.spec.size = pv.spec.size, recover_expansion=on",
+			name:                  "pvc.spec.size = pv.spec.size, resize_status=no_expansion_inprogress",
 			pvc:                   getTestPVC("test-vol0", "1G", "1G", "", v1.PersistentVolumeClaimNoExpansionInProgress),
 			pv:                    createPV(1, "claim01", defaultNS, "test-uid", &fsVolumeMode),
-			recoverFeatureGate:    true,
 			expectedResizeStatus:  v1.PersistentVolumeClaimNodeExpansionPending,
 			expectedAllocatedSize: resource.MustParse("1G"),
 			expectResizeCall:      true,
 		},
 		{
-			name:                  "pvc.spec.size = pv.spec.size, recover_expansion=on",
+			name:                  "pvc.spec.size > pv.spec.size, resize_status=controller_expansion_failed",
+			pvc:                   getTestPVC("test-vol0", "5G" /*specSize*/, "3G" /*statusSize*/, "10G" /*allocatedSize*/, v1.PersistentVolumeClaimControllerExpansionFailed),
+			pv:                    createPV(3, "claim01", defaultNS, "test-uid", &fsVolumeMode),
+			expectedResizeStatus:  v1.PersistentVolumeClaimNodeExpansionPending,
+			expectedAllocatedSize: resource.MustParse("5G"),
+			expectResizeCall:      true,
+		},
+		{
+			name:                       "pvc.spec.size = pv.spec.size, resize_status=node_expansion_failed, disable_controller_expansion=true",
+			pvc:                        getTestPVC("test-vol0", "5G" /*specSize*/, "3G" /*statusSize*/, "10G" /*allocatedSize*/, v1.PersistentVolumeClaimNodeExpansionFailed),
+			pv:                         createPV(10, "claim01", defaultNS, "test-uid", &fsVolumeMode),
+			disableControllerExpansion: true,
+			expectedResizeStatus:       v1.PersistentVolumeClaimNodeExpansionPending,
+			expectedAllocatedSize:      resource.MustParse("5G"),
+			expectResizeCall:           true,
+		},
+		{
+			name:                  "pvc.spec.size = pv.spec.size, resize_status=node_expansion_pending",
 			pvc:                   getTestPVC("test-vol0", "1G", "1G", "1G", v1.PersistentVolumeClaimNodeExpansionPending),
 			pv:                    createPV(1, "claim01", defaultNS, "test-uid", &fsVolumeMode),
-			recoverFeatureGate:    true,
 			expectedResizeStatus:  v1.PersistentVolumeClaimNodeExpansionPending,
 			expectedAllocatedSize: resource.MustParse("1G"),
 			expectResizeCall:      false,
 		},
 		{
-			name:                  "pvc.spec.size > pv.spec.size, recover_expansion=on, disable_node_expansion=true",
+			name:                  "pvc.spec.size > pv.spec.size, disable_node_expansion=true, resize_status=no_expansion_inprogress",
 			pvc:                   getTestPVC("test-vol0", "2G", "1G", "", v1.PersistentVolumeClaimNoExpansionInProgress),
 			pv:                    createPV(1, "claim01", defaultNS, "test-uid", &fsVolumeMode),
 			disableNodeExpansion:  true,
-			recoverFeatureGate:    true,
 			expectedResizeStatus:  v1.PersistentVolumeClaimNoExpansionInProgress,
 			expectedAllocatedSize: resource.MustParse("2G"),
 			expectResizeCall:      true,
 		},
 		{
-			name:                  "pv.spec.size >= pvc.spec.size, recover_expansion=on, resize_status=node_expansion_failed",
+			name:                  "pv.spec.size >= pvc.spec.size, resize_status=node_expansion_failed",
 			pvc:                   getTestPVC("test-vol0", "2G", "1G", "2G", v1.PersistentVolumeClaimNodeExpansionFailed),
 			pv:                    createPV(2, "claim01", defaultNS, "test-uid", &fsVolumeMode),
-			recoverFeatureGate:    true,
 			expectedResizeStatus:  v1.PersistentVolumeClaimNodeExpansionPending,
 			expectedAllocatedSize: resource.MustParse("2G"),
 			expectResizeCall:      true,
 		},
 		{
-			name:                  "pvc.spec.size > pv.spec.size, recover_expansion=on, resize_status-node_expansion_pending",
+			name:                  "pvc.spec.size > pv.spec.size, resize_status-node_expansion_pending",
 			pvc:                   getTestPVC("test-vol0", "10G", "1G", "3G", v1.PersistentVolumeClaimNodeExpansionPending),
 			pv:                    createPV(3, "claim01", defaultNS, "test-uid", &fsVolumeMode),
-			recoverFeatureGate:    true,
 			expectedResizeStatus:  v1.PersistentVolumeClaimNodeExpansionPending,
 			expectedAllocatedSize: resource.MustParse("3G"),
 			expectResizeCall:      false,
 		},
 		{
-			name:                  "pvc.spec.size > pv.spec.size, recover_expansion=on, resize_status-node_expansion_inprogress",
+			name:                  "pvc.spec.size > pv.spec.size, resize_status-node_expansion_inprogress",
 			pvc:                   getTestPVC("test-vol0", "10G", "1G", "3G", v1.PersistentVolumeClaimNodeExpansionInProgress),
 			pv:                    createPV(3, "claim01", defaultNS, "test-uid", &fsVolumeMode),
-			recoverFeatureGate:    true,
 			expectedResizeStatus:  v1.PersistentVolumeClaimNodeExpansionInProgress,
 			expectedAllocatedSize: resource.MustParse("3G"),
 			expectResizeCall:      false,
@@ -97,8 +107,8 @@ func TestExpandAndRecover(t *testing.T) {
 	for i := range tests {
 		test := tests[i]
 		t.Run(test.name, func(t *testing.T) {
-			defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.RecoverVolumeExpansionFailure, test.recoverFeatureGate)()
-			client := csi.NewMockClient("foo", !test.disableNodeExpansion, true, true, true)
+			defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.RecoverVolumeExpansionFailure, true)()
+			client := csi.NewMockClient("foo", !test.disableNodeExpansion, !test.disableControllerExpansion, true, true)
 			driverName, _ := client.GetDriverName(context.TODO())
 
 			var initialObjects []runtime.Object

diff --git a/pkg/controller/resize_status.go b/pkg/controller/resize_status.go
@@ -1,5 +1,5 @@
 /*
-Copyright 2018 The Kubernetes Authors.
+Copyright 2022 The Kubernetes Authors.
 
 Licensed under the Apache License, Version 2.0 (the "License");
 you may not use this file except in compliance with the License.