Skip to content

Commit 099f112

Browse files
authored
Merge pull request #513 from huww98/rollback
modify: support rollback
2 parents 75970d4 + d03a1e7 commit 099f112

File tree

13 files changed

+551
-437
lines changed

13 files changed

+551
-437
lines changed

go.mod

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,6 @@ require (
66
github.com/container-storage-interface/spec v1.11.0
77
github.com/google/go-cmp v0.7.0
88
github.com/kubernetes-csi/csi-lib-utils v0.22.0
9-
golang.org/x/oauth2 v0.27.0 // indirect
10-
golang.org/x/term v0.31.0 // indirect
119
google.golang.org/grpc v1.72.1
1210
k8s.io/api v0.34.0
1311
k8s.io/apimachinery v0.34.0
@@ -16,10 +14,9 @@ require (
1614
k8s.io/component-base v0.34.0
1715
k8s.io/csi-translation-lib v0.34.0
1816
k8s.io/klog/v2 v2.130.1
17+
k8s.io/utils v0.0.0-20250604170112-4c0f3b243397
1918
)
2019

21-
require k8s.io/utils v0.0.0-20250604170112-4c0f3b243397
22-
2320
require (
2421
cel.dev/expr v0.24.0 // indirect
2522
github.com/NYTimes/gziphandler v1.1.1 // indirect
@@ -87,8 +84,10 @@ require (
8784
golang.org/x/crypto v0.37.0 // indirect
8885
golang.org/x/exp v0.0.0-20240719175910-8a7402abbf56 // indirect
8986
golang.org/x/net v0.39.0 // indirect
87+
golang.org/x/oauth2 v0.27.0 // indirect
9088
golang.org/x/sync v0.13.0 // indirect
9189
golang.org/x/sys v0.32.0 // indirect
90+
golang.org/x/term v0.31.0 // indirect
9291
golang.org/x/text v0.24.0 // indirect
9392
golang.org/x/time v0.9.0 // indirect
9493
google.golang.org/genproto/googleapis/api v0.0.0-20250303144028-a0af3efb3deb // indirect

pkg/controller/controller.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -293,15 +293,15 @@ func (ctrl *resizeController) Run(workers int, ctx context.Context, wg *sync.Wai
293293
}
294294

295295
if utilfeature.DefaultFeatureGate.Enabled(features.ReleaseLeaderElectionOnExit) {
296-
for i := 0; i < workers; i++ {
296+
for range workers {
297297
wg.Add(1)
298298
go func() {
299299
defer wg.Done()
300300
wait.Until(ctrl.syncPVCs, 0, stopCh)
301301
}()
302302
}
303303
} else {
304-
for i := 0; i < workers; i++ {
304+
for range workers {
305305
go wait.Until(ctrl.syncPVCs, 0, stopCh)
306306
}
307307
}

pkg/controller/resize_status_test.go

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -30,42 +30,42 @@ func TestResizeFunctions(t *testing.T) {
3030
}{
3131
{
3232
name: "mark fs resize, with no other conditions",
33-
pvc: basePVC.Get(),
34-
expectedPVC: basePVC.WithStorageResourceStatus(v1.PersistentVolumeClaimNodeResizePending).Get(),
33+
pvc: basePVC().Get(),
34+
expectedPVC: basePVC().WithStorageResourceStatus(v1.PersistentVolumeClaimNodeResizePending).Get(),
3535
testFunc: func(pvc *v1.PersistentVolumeClaim, ctrl *resizeController, size resource.Quantity) (*v1.PersistentVolumeClaim, error) {
3636
return ctrl.markForPendingNodeExpansion(pvc)
3737
},
3838
},
3939
{
4040
name: "mark fs resize, when other resource statuses are present",
41-
pvc: basePVC.WithResourceStatus(v1.ResourceCPU, v1.PersistentVolumeClaimControllerResizeInfeasible).Get(),
42-
expectedPVC: basePVC.WithResourceStatus(v1.ResourceCPU, v1.PersistentVolumeClaimControllerResizeInfeasible).
41+
pvc: basePVC().WithResourceStatus(v1.ResourceCPU, v1.PersistentVolumeClaimControllerResizeInfeasible).Get(),
42+
expectedPVC: basePVC().WithResourceStatus(v1.ResourceCPU, v1.PersistentVolumeClaimControllerResizeInfeasible).
4343
WithStorageResourceStatus(v1.PersistentVolumeClaimNodeResizePending).Get(),
4444
testFunc: func(pvc *v1.PersistentVolumeClaim, ctrl *resizeController, _ resource.Quantity) (*v1.PersistentVolumeClaim, error) {
4545
return ctrl.markForPendingNodeExpansion(pvc)
4646
},
4747
},
4848
{
4949
name: "mark controller resize in-progress",
50-
pvc: basePVC.Get(),
51-
expectedPVC: basePVC.WithStorageResourceStatus(v1.PersistentVolumeClaimControllerResizeInProgress).Get(),
50+
pvc: basePVC().Get(),
51+
expectedPVC: basePVC().WithStorageResourceStatus(v1.PersistentVolumeClaimControllerResizeInProgress).Get(),
5252
testFunc: func(pvc *v1.PersistentVolumeClaim, ctrl *resizeController, q resource.Quantity) (*v1.PersistentVolumeClaim, error) {
5353
return ctrl.markControllerResizeInProgress(pvc, q, true)
5454
},
5555
},
5656
{
5757
name: "mark controller resize failed",
58-
pvc: basePVC.Get(),
59-
expectedPVC: basePVC.WithStorageResourceStatus(v1.PersistentVolumeClaimControllerResizeInfeasible).Get(),
58+
pvc: basePVC().Get(),
59+
expectedPVC: basePVC().WithStorageResourceStatus(v1.PersistentVolumeClaimControllerResizeInfeasible).Get(),
6060
testFunc: func(pvc *v1.PersistentVolumeClaim, ctrl *resizeController, q resource.Quantity) (*v1.PersistentVolumeClaim, error) {
6161
return ctrl.markControllerExpansionInfeasible(pvc, fmt.Errorf("things failed"))
6262
},
6363
},
6464
{
6565
name: "mark resize finished",
66-
pvc: basePVC.WithResourceStatus(v1.ResourceCPU, v1.PersistentVolumeClaimControllerResizeInfeasible).
66+
pvc: basePVC().WithResourceStatus(v1.ResourceCPU, v1.PersistentVolumeClaimControllerResizeInfeasible).
6767
WithStorageResourceStatus(v1.PersistentVolumeClaimNodeResizePending).Get(),
68-
expectedPVC: basePVC.WithResourceStatus(v1.ResourceCPU, v1.PersistentVolumeClaimControllerResizeInfeasible).
68+
expectedPVC: basePVC().WithResourceStatus(v1.ResourceCPU, v1.PersistentVolumeClaimControllerResizeInfeasible).
6969
WithStorageResourceStatus("").Get(),
7070
testFunc: func(pvc *v1.PersistentVolumeClaim, ctrl *resizeController, q resource.Quantity) (*v1.PersistentVolumeClaim, error) {
7171
return ctrl.markOverallExpansionAsFinished(pvc, q)

pkg/modifycontroller/controller.go

Lines changed: 24 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ import (
2828
"github.com/kubernetes-csi/csi-lib-utils/slowset"
2929
"github.com/kubernetes-csi/external-resizer/pkg/modifier"
3030
v1 "k8s.io/api/core/v1"
31+
apierrors "k8s.io/apimachinery/pkg/api/errors"
3132
"k8s.io/apimachinery/pkg/labels"
3233
"k8s.io/apimachinery/pkg/util/wait"
3334
utilfeature "k8s.io/apiserver/pkg/util/feature"
@@ -41,6 +42,7 @@ import (
4142
"k8s.io/client-go/tools/record"
4243
"k8s.io/client-go/util/workqueue"
4344
"k8s.io/klog/v2"
45+
"k8s.io/utils/ptr"
4446
)
4547

4648
// ModifyController watches PVCs and checks if they are requesting an modify operation.
@@ -63,8 +65,11 @@ type modifyController struct {
6365
vacLister storagev1listers.VolumeAttributesClassLister
6466
vacListerSynced cache.InformerSynced
6567
extraModifyMetadata bool
66-
// the key of the map is {PVC_NAMESPACE}/{PVC_NAME}
67-
uncertainPVCs map[string]v1.PersistentVolumeClaim
68+
// uncertainPVCs tracks PVCs that failed with non-final errors.
69+
// We must not change the target when retrying.
70+
// All in-progress PVCs are added here on initialization.
71+
// The key of the map is {PVC_NAMESPACE}/{PVC_NAME}, value is not important now.
72+
uncertainPVCs sync.Map
6873
// slowSet tracks PVCs for which modification failed with infeasible error and should be retried at slower rate.
6974
slowSet *slowset.SlowSet
7075
}
@@ -124,19 +129,18 @@ func NewModifyController(
124129
}
125130

126131
func (ctrl *modifyController) initUncertainPVCs() error {
127-
ctrl.uncertainPVCs = make(map[string]v1.PersistentVolumeClaim)
128132
allPVCs, err := ctrl.pvcLister.List(labels.Everything())
129133
if err != nil {
130134
klog.Errorf("Failed to list pvcs when init uncertain pvcs: %v", err)
131135
return err
132136
}
133137
for _, pvc := range allPVCs {
134-
if pvc.Status.ModifyVolumeStatus != nil && (pvc.Status.ModifyVolumeStatus.Status == v1.PersistentVolumeClaimModifyVolumeInProgress || pvc.Status.ModifyVolumeStatus.Status == v1.PersistentVolumeClaimModifyVolumeInfeasible) {
138+
if pvc.Status.ModifyVolumeStatus != nil && (pvc.Status.ModifyVolumeStatus.Status == v1.PersistentVolumeClaimModifyVolumeInProgress) {
135139
pvcKey, err := cache.MetaNamespaceKeyFunc(pvc)
136140
if err != nil {
137141
return err
138142
}
139-
ctrl.uncertainPVCs[pvcKey] = *pvc.DeepCopy()
143+
ctrl.uncertainPVCs.Store(pvcKey, pvc)
140144
}
141145
}
142146

@@ -163,18 +167,18 @@ func (ctrl *modifyController) updatePVC(oldObj, newObj interface{}) {
163167
}
164168

165169
// Only trigger modify volume if the following conditions are met
166-
// 1. Non empty vac name
167-
// 2. oldVacName != newVacName
168-
// 3. PVC is in Bound state
169-
oldVacName := oldPVC.Spec.VolumeAttributesClassName
170-
newVacName := newPVC.Spec.VolumeAttributesClassName
171-
if newVacName != nil && *newVacName != "" && (oldVacName == nil || *newVacName != *oldVacName) && oldPVC.Status.Phase == v1.ClaimBound {
170+
// 1. VAC changed or modify finished (check pending modify request while we are modifying)
171+
// 2. PVC is in Bound state
172+
oldVacName := ptr.Deref(oldPVC.Spec.VolumeAttributesClassName, "")
173+
newVacName := ptr.Deref(newPVC.Spec.VolumeAttributesClassName, "")
174+
if (newVacName != oldVacName || newPVC.Status.ModifyVolumeStatus == nil) && newPVC.Status.Phase == v1.ClaimBound {
172175
_, err := ctrl.pvLister.Get(oldPVC.Spec.VolumeName)
173176
if err != nil {
174177
klog.Errorf("Get PV %q of pvc %q in PVInformer cache failed: %v", oldPVC.Spec.VolumeName, klog.KObj(oldPVC), err)
175178
return
176179
}
177180
// Handle modify volume by adding to the claimQueue to avoid race conditions
181+
klog.V(4).InfoS("Enqueueing PVC for modify", "PVC", klog.KObj(newPVC))
178182
ctrl.addPVC(newObj)
179183
} else {
180184
klog.V(4).InfoS("No need to modify PVC", "PVC", klog.KObj(newPVC))
@@ -190,10 +194,7 @@ func (ctrl *modifyController) deletePVC(obj interface{}) {
190194
}
191195

192196
func (ctrl *modifyController) init(ctx context.Context) bool {
193-
informersSyncd := []cache.InformerSynced{ctrl.pvListerSynced, ctrl.pvcListerSynced}
194-
informersSyncd = append(informersSyncd, ctrl.vacListerSynced)
195-
196-
if !cache.WaitForCacheSync(ctx.Done(), informersSyncd...) {
197+
if !cache.WaitForCacheSync(ctx.Done(), ctrl.pvListerSynced, ctrl.pvcListerSynced, ctrl.vacListerSynced) {
197198
klog.ErrorS(nil, "Cannot sync pod, pv, pvc or vac caches")
198199
return false
199200
}
@@ -224,15 +225,15 @@ func (ctrl *modifyController) Run(
224225
go ctrl.slowSet.Run(stopCh)
225226

226227
if utilfeature.DefaultFeatureGate.Enabled(features.ReleaseLeaderElectionOnExit) {
227-
for i := 0; i < workers; i++ {
228+
for range workers {
228229
wg.Add(1)
229230
go func() {
230231
defer wg.Done()
231232
wait.Until(ctrl.sync, 0, stopCh)
232233
}()
233234
}
234235
} else {
235-
for i := 0; i < workers; i++ {
236+
for range workers {
236237
go wait.Until(ctrl.sync, 0, stopCh)
237238
}
238239
}
@@ -268,6 +269,10 @@ func (ctrl *modifyController) syncPVC(key string) error {
268269

269270
pvc, err := ctrl.pvcLister.PersistentVolumeClaims(namespace).Get(name)
270271
if err != nil {
272+
if apierrors.IsNotFound(err) {
273+
klog.V(3).InfoS("PVC is deleted or does not exist", "PVC", klog.KRef(namespace, name))
274+
return nil
275+
}
271276
return fmt.Errorf("getting PVC %s/%s failed: %v", namespace, name, err)
272277
}
273278

@@ -283,15 +288,13 @@ func (ctrl *modifyController) syncPVC(key string) error {
283288

284289
// Only trigger modify volume if the following conditions are met
285290
// 1. PV provisioned by CSI driver AND driver name matches local driver
286-
// 2. Non-empty vac name
287-
// 3. PVC is in Bound state
291+
// 2. PVC is in Bound state
288292
if pv.Spec.CSI == nil || pv.Spec.CSI.Driver != ctrl.name {
289293
klog.V(7).InfoS("Skipping PV provisioned by different driver", "PV", klog.KObj(pv))
290294
return nil
291295
}
292296

293-
vacName := pvc.Spec.VolumeAttributesClassName
294-
if vacName != nil && *vacName != "" && pvc.Status.Phase == v1.ClaimBound {
297+
if pvc.Status.Phase == v1.ClaimBound {
295298
_, _, err, _ := ctrl.modify(pvc, pv)
296299
if err != nil {
297300
return err

0 commit comments

Comments
 (0)