Skip to content

Commit 5cc5fe8

Browse files
sunnylovestiramisusunnylovestiramisu
committed
Enable Rollback Support of VAC with Infeasible Errors
1 parent 3429c44 commit 5cc5fe8

File tree

3 files changed

+46
-17
lines changed

3 files changed

+46
-17
lines changed

‎pkg/modifycontroller/controller_test.go

+12-11
Original file line numberDiff line numberDiff line change
@@ -4,13 +4,14 @@ import (
44
"context"
55
"errors"
66
"fmt"
7+
"testing"
8+
"time"
9+
710
"github.com/kubernetes-csi/external-resizer/pkg/util"
811
"google.golang.org/grpc/codes"
912
"google.golang.org/grpc/status"
1013
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
1114
"k8s.io/client-go/tools/cache"
12-
"testing"
13-
"time"
1415

1516
"github.com/kubernetes-csi/external-resizer/pkg/features"
1617

@@ -27,7 +28,7 @@ import (
2728
)
2829

2930
func TestController(t *testing.T) {
30-
basePVC := createTestPVC(pvcName, testVac /*vacName*/, testVac /*curVacName*/, testVac /*targetVacName*/)
31+
basePVC := createTestPVC(pvcName, testVac /*vacName*/, testVac /*curVacName*/, testVac /*targetVacName*/, "" /*modifyVolumeStatus*/)
3132
basePV := createTestPV(1, pvcName, pvcNamespace, "foobaz" /*pvcUID*/, &fsVolumeMode, testVac)
3233
firstTimePV := basePV.DeepCopy()
3334
firstTimePV.Spec.VolumeAttributesClassName = nil
@@ -44,7 +45,7 @@ func TestController(t *testing.T) {
4445
}{
4546
{
4647
name: "Modify called",
47-
pvc: createTestPVC(pvcName, targetVac /*vacName*/, testVac /*curVacName*/, testVac /*targetVacName*/),
48+
pvc: createTestPVC(pvcName, targetVac /*vacName*/, testVac /*curVacName*/, testVac /*targetVacName*/, "" /*modifyVolumeStatus*/),
4849
pv: basePV,
4950
vacExists: true,
5051
callCSIModify: true,
@@ -104,14 +105,14 @@ func TestModifyPVC(t *testing.T) {
104105
}{
105106
{
106107
name: "Modify succeeded",
107-
pvc: createTestPVC(pvcName, targetVac /*vacName*/, testVac /*curVacName*/, testVac /*targetVacName*/),
108+
pvc: createTestPVC(pvcName, targetVac /*vacName*/, testVac /*curVacName*/, testVac /*targetVacName*/, "" /*modifyVolumeStatus*/),
108109
pv: basePV,
109110
modifyFailure: false,
110111
expectFailure: false,
111112
},
112113
{
113114
name: "Modify failed",
114-
pvc: createTestPVC(pvcName, targetVac /*vacName*/, testVac /*curVacName*/, testVac /*targetVacName*/),
115+
pvc: createTestPVC(pvcName, targetVac /*vacName*/, testVac /*curVacName*/, testVac /*targetVacName*/, "" /*modifyVolumeStatus*/),
115116
pv: basePV,
116117
modifyFailure: true,
117118
expectFailure: true,
@@ -145,16 +146,16 @@ func TestModifyPVC(t *testing.T) {
145146
}
146147

147148
func TestSyncPVC(t *testing.T) {
148-
basePVC := createTestPVC(pvcName, targetVac /*vacName*/, testVac /*curVacName*/, testVac /*targetVacName*/)
149+
basePVC := createTestPVC(pvcName, targetVac /*vacName*/, testVac /*curVacName*/, testVac /*targetVacName*/, "" /*modifyVolumeStatus*/)
149150
basePV := createTestPV(1, pvcName, pvcNamespace, "foobaz" /*pvcUID*/, &fsVolumeMode, testVac)
150151

151152
otherDriverPV := createTestPV(1, pvcName, pvcNamespace, "foobaz" /*pvcUID*/, &fsVolumeMode, testVac)
152153
otherDriverPV.Spec.PersistentVolumeSource.CSI.Driver = "some-other-driver"
153154

154-
unboundPVC := createTestPVC(pvcName, targetVac /*vacName*/, testVac /*curVacName*/, testVac /*targetVacName*/)
155+
unboundPVC := createTestPVC(pvcName, targetVac /*vacName*/, testVac /*curVacName*/, testVac /*targetVacName*/, "" /*modifyVolumeStatus*/)
155156
unboundPVC.Status.Phase = v1.ClaimPending
156157

157-
pvcWithUncreatedPV := createTestPVC(pvcName, targetVac /*vacName*/, testVac /*curVacName*/, testVac /*targetVacName*/)
158+
pvcWithUncreatedPV := createTestPVC(pvcName, targetVac /*vacName*/, testVac /*curVacName*/, testVac /*targetVacName*/, "" /*modifyVolumeStatus*/)
158159
pvcWithUncreatedPV.Spec.VolumeName = ""
159160

160161
nonCSIPVC := &v1.PersistentVolumeClaim{
@@ -196,7 +197,7 @@ func TestSyncPVC(t *testing.T) {
196197
},
197198
{
198199
name: "Should NOT modify if PVC has empty Spec.VACName",
199-
pvc: createTestPVC(pvcName, "" /*vacName*/, testVac /*curVacName*/, testVac /*targetVacName*/),
200+
pvc: createTestPVC(pvcName, "" /*vacName*/, testVac /*curVacName*/, testVac /*targetVacName*/, "" /*modifyVolumeStatus*/),
200201
pv: basePV,
201202
callCSIModify: false,
202203
},
@@ -247,7 +248,7 @@ func TestSyncPVC(t *testing.T) {
247248

248249
// TestInfeasibleRetry tests that sidecar doesn't spam plugin upon infeasible error code (e.g. invalid VAC parameter)
249250
func TestInfeasibleRetry(t *testing.T) {
250-
basePVC := createTestPVC(pvcName, targetVac /*vacName*/, testVac /*curVacName*/, testVac /*targetVacName*/)
251+
basePVC := createTestPVC(pvcName, targetVac /*vacName*/, testVac /*curVacName*/, testVac /*targetVacName*/, "" /*modifyVolumeStatus*/)
251252
basePV := createTestPV(1, pvcName, pvcNamespace, "foobaz" /*pvcUID*/, &fsVolumeMode, testVac)
252253

253254
tests := []struct {

‎pkg/modifycontroller/modify_volume.go

+18
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,24 @@ func (ctrl *modifyController) modify(pvc *v1.PersistentVolumeClaim, pv *v1.Persi
6565
}
6666
return ctrl.controllerModifyVolumeWithTarget(pvc, pv, vac, pvcSpecVacName)
6767
}
68+
} else if pvcSpecVacName != nil && curVacName != nil && pvc.Status.ModifyVolumeStatus != nil && *pvcSpecVacName == *curVacName {
69+
targetVacName := pvc.Status.ModifyVolumeStatus.TargetVolumeAttributesClassName
70+
// Allow rollback of VAC if it is in infeasible state
71+
if targetVacName != "" && targetVacName != *curVacName && pvc.Status.ModifyVolumeStatus.Status == v1.PersistentVolumeClaimModifyVolumeInfeasible {
72+
vac, err := ctrl.vacLister.Get(*pvcSpecVacName)
73+
if err != nil {
74+
return pvc, pv, err, false
75+
}
76+
// Mark pvc.Status.ModifyVolumeStatus as in progress
77+
pvc, err = ctrl.markControllerModifyVolumeStatus(pvc, v1.PersistentVolumeClaimModifyVolumeInProgress, nil)
78+
if err != nil {
79+
return pvc, pv, err, false
80+
}
81+
// Record an event to indicate that external resizer is rolling back VAC in this volume.
82+
ctrl.eventRecorder.Event(pvc, v1.EventTypeNormal, util.VolumeModify,
83+
fmt.Sprintf("external resizer is rolling back a modification of the volume %s to vac %s", pvc.Name, *pvcSpecVacName))
84+
return ctrl.controllerModifyVolumeWithTarget(pvc, pv, vac, pvcSpecVacName)
85+
}
6886
}
6987

7088
// No modification required

‎pkg/modifycontroller/modify_volume_test.go

+16-6
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ var (
3535
)
3636

3737
func TestModify(t *testing.T) {
38-
basePVC := createTestPVC(pvcName, testVac /*vacName*/, testVac /*curVacName*/, testVac /*targetVacName*/)
38+
basePVC := createTestPVC(pvcName, testVac /*vacName*/, testVac /*curVacName*/, testVac /*targetVacName*/, "" /*modifyVolumeStatus*/)
3939
basePV := createTestPV(1, pvcName, pvcNamespace, "foobaz" /*pvcUID*/, &fsVolumeMode, testVac)
4040

4141
var tests = []struct {
@@ -61,7 +61,7 @@ func TestModify(t *testing.T) {
6161
},
6262
{
6363
name: "vac does not exist, no modification and set ModifyVolumeStatus to pending",
64-
pvc: createTestPVC(pvcName, targetVac /*vacName*/, testVac /*curVacName*/, testVac /*targetVacName*/),
64+
pvc: createTestPVC(pvcName, targetVac /*vacName*/, testVac /*curVacName*/, testVac /*targetVacName*/, "" /*modifyVolumeStatus*/),
6565
pv: basePV,
6666
expectModifyCall: false,
6767
expectedModifyVolumeStatus: &v1.ModifyVolumeStatus{
@@ -73,7 +73,7 @@ func TestModify(t *testing.T) {
7373
},
7474
{
7575
name: "modify volume success",
76-
pvc: createTestPVC(pvcName, targetVac /*vacName*/, testVac /*curVacName*/, testVac /*targetVacName*/),
76+
pvc: createTestPVC(pvcName, targetVac /*vacName*/, testVac /*curVacName*/, testVac /*targetVacName*/, "" /*modifyVolumeStatus*/),
7777
pv: basePV,
7878
vacExists: true,
7979
expectModifyCall: true,
@@ -83,7 +83,7 @@ func TestModify(t *testing.T) {
8383
},
8484
{
8585
name: "modify volume success with extra metadata",
86-
pvc: createTestPVC(pvcName, targetVac /*vacName*/, testVac /*curVacName*/, testVac /*targetVacName*/),
86+
pvc: createTestPVC(pvcName, targetVac /*vacName*/, testVac /*curVacName*/, testVac /*targetVacName*/, "" /*modifyVolumeStatus*/),
8787
pv: basePV,
8888
vacExists: true,
8989
expectModifyCall: true,
@@ -98,6 +98,16 @@ func TestModify(t *testing.T) {
9898
"csi.storage.k8s.io/pv/name": "testPV",
9999
},
100100
},
101+
{
102+
name: "modify volume rollback succeeds for infeasible errors",
103+
pvc: createTestPVC(pvcName, testVac /*vacName*/, targetVac /*curVacName*/, testVac /*targetVacName*/, v1.PersistentVolumeClaimModifyVolumeInfeasible),
104+
pv: basePV,
105+
vacExists: true,
106+
expectModifyCall: true,
107+
expectedModifyVolumeStatus: nil,
108+
expectedCurrentVolumeAttributesClassName: &testVac,
109+
expectedPVVolumeAttributesClassName: &testVac,
110+
},
101111
}
102112

103113
for i := range tests {
@@ -154,7 +164,7 @@ func TestModify(t *testing.T) {
154164
}
155165
}
156166

157-
func createTestPVC(pvcName string, vacName string, curVacName string, targetVacName string) *v1.PersistentVolumeClaim {
167+
func createTestPVC(pvcName string, vacName string, curVacName string, targetVacName string, modifyVolumeStatus v1.PersistentVolumeClaimModifyVolumeStatus) *v1.PersistentVolumeClaim {
158168
pvc := &v1.PersistentVolumeClaim{
159169
ObjectMeta: metav1.ObjectMeta{Name: pvcName, Namespace: pvcNamespace},
160170
Spec: v1.PersistentVolumeClaimSpec{
@@ -178,7 +188,7 @@ func createTestPVC(pvcName string, vacName string, curVacName string, targetVacN
178188
CurrentVolumeAttributesClassName: &curVacName,
179189
ModifyVolumeStatus: &v1.ModifyVolumeStatus{
180190
TargetVolumeAttributesClassName: targetVacName,
181-
Status: "",
191+
Status: modifyVolumeStatus,
182192
},
183193
},
184194
}

0 commit comments

Comments
 (0)