From d620adb93c7b12ac590cca97fd94ee7e8ca57479 Mon Sep 17 00:00:00 2001 From: Grant He Date: Thu, 14 Aug 2025 06:57:33 +0000 Subject: [PATCH 1/3] Address various linter (staticcheck, modernize, etc.) comments. --- pkg/controller/controller.go | 18 +++++++------- pkg/csi/mock_client.go | 2 +- pkg/modifier/csi_modifier_test.go | 5 ---- pkg/modifycontroller/controller.go | 8 +++--- pkg/modifycontroller/modify_status_test.go | 29 ++++++++++------------ pkg/resizer/csi_resizer.go | 8 +++--- pkg/resizer/csi_resizer_test.go | 4 +-- pkg/util/util.go | 6 ++--- pkg/util/util_test.go | 4 +-- 9 files changed, 38 insertions(+), 46 deletions(-) diff --git a/pkg/controller/controller.go b/pkg/controller/controller.go index 60afed470..dde4bfef0 100644 --- a/pkg/controller/controller.go +++ b/pkg/controller/controller.go @@ -146,7 +146,7 @@ func NewResizeController( return ctrl } -func (ctrl *resizeController) addPVC(obj interface{}) { +func (ctrl *resizeController) addPVC(obj any) { objKey, err := util.GetObjectKey(obj) if err != nil { return @@ -154,7 +154,7 @@ func (ctrl *resizeController) addPVC(obj interface{}) { ctrl.claimQueue.Add(objKey) } -func (ctrl *resizeController) addPod(obj interface{}) { +func (ctrl *resizeController) addPod(obj any) { pod := parsePod(obj) if pod == nil { return @@ -162,7 +162,7 @@ func (ctrl *resizeController) addPod(obj interface{}) { ctrl.usedPVCs.addPod(pod) } -func (ctrl *resizeController) deletePod(obj interface{}) { +func (ctrl *resizeController) deletePod(obj any) { pod := parsePod(obj) if pod == nil { return @@ -170,7 +170,7 @@ func (ctrl *resizeController) deletePod(obj interface{}) { ctrl.usedPVCs.removePod(pod) } -func (ctrl *resizeController) updatePod(oldObj, newObj interface{}) { +func (ctrl *resizeController) updatePod(oldObj, newObj any) { pod := parsePod(newObj) if pod == nil { return @@ -183,7 +183,7 @@ func (ctrl *resizeController) updatePod(oldObj, newObj interface{}) { } } -func (ctrl *resizeController) updatePVC(oldObj, newObj interface{}) { +func (ctrl *resizeController) updatePVC(oldObj, newObj any) { oldPVC, ok := oldObj.(*v1.PersistentVolumeClaim) if !ok || oldPVC == nil { return @@ -262,7 +262,7 @@ func (ctrl *resizeController) updatePVC(oldObj, newObj interface{}) { } } -func (ctrl *resizeController) deletePVC(obj interface{}) { +func (ctrl *resizeController) deletePVC(obj any) { objKey, err := util.GetObjectKey(obj) if err != nil { return @@ -577,7 +577,7 @@ func (ctrl *resizeController) markPVCResizeInProgress(pvc *v1.PersistentVolumeCl updatedPVC, err := util.PatchClaim(ctrl.kubeClient, pvc, newPVC, true /* addResourceVersionCheck */) if err != nil { - return updatedPVC, fmt.Errorf("Mark PVC %q as resize as in progress failed: %v", klog.KObj(pvc), err) + return updatedPVC, fmt.Errorf("mark PVC %q as resize as in progress failed: %v", klog.KObj(pvc), err) } err = ctrl.claims.Update(updatedPVC) if err != nil { @@ -595,7 +595,7 @@ func (ctrl *resizeController) markPVCResizeFinished( updatedPVC, err := util.PatchClaim(ctrl.kubeClient, pvc, newPVC, true /* addResourceVersionCheck */) if err != nil { - return fmt.Errorf("Mark PVC %q as resize finished failed: %w", klog.KObj(pvc), err) + return fmt.Errorf("mark PVC %q as resize finished failed: %w", klog.KObj(pvc), err) } err = ctrl.claims.Update(updatedPVC) @@ -647,7 +647,7 @@ func (ctrl *resizeController) updatePVCapacity( return updatedPV, nil } -func parsePod(obj interface{}) *v1.Pod { +func parsePod(obj any) *v1.Pod { if obj == nil { return nil } diff --git a/pkg/csi/mock_client.go b/pkg/csi/mock_client.go index dc8a03b7e..946501f45 100644 --- a/pkg/csi/mock_client.go +++ b/pkg/csi/mock_client.go @@ -100,7 +100,7 @@ func (c *MockClient) Expand( additionalInfoVal := additionalInfo.(connection.AdditionalInfo) migrated := additionalInfoVal.Migrated if migrated != "true" { - err := fmt.Errorf("Expected value of migrated label: true, Actual value: %s", migrated) + err := fmt.Errorf("migrated label expected value: true, actual value: %s", migrated) return requestBytes, c.supportsNodeResize, err } } diff --git a/pkg/modifier/csi_modifier_test.go b/pkg/modifier/csi_modifier_test.go index 179eb0341..35afc9f8b 100644 --- a/pkg/modifier/csi_modifier_test.go +++ b/pkg/modifier/csi_modifier_test.go @@ -1,7 +1,6 @@ package modifier import ( - "errors" "testing" "github.com/kubernetes-csi/external-resizer/pkg/csi" @@ -10,10 +9,6 @@ import ( "k8s.io/client-go/kubernetes/fake" ) -var ( - controllerServiceNotSupportErr = errors.New("CSI driver does not support controller service") -) - func TestNewModifier(t *testing.T) { for i, c := range []struct { SupportsControllerModify bool diff --git a/pkg/modifycontroller/controller.go b/pkg/modifycontroller/controller.go index 68f386d5e..2983e9d53 100644 --- a/pkg/modifycontroller/controller.go +++ b/pkg/modifycontroller/controller.go @@ -143,7 +143,7 @@ func (ctrl *modifyController) initUncertainPVCs() error { return nil } -func (ctrl *modifyController) addPVC(obj interface{}) { +func (ctrl *modifyController) addPVC(obj any) { objKey, err := util.GetObjectKey(obj) if err != nil { return @@ -151,7 +151,7 @@ func (ctrl *modifyController) addPVC(obj interface{}) { ctrl.claimQueue.Add(objKey) } -func (ctrl *modifyController) updatePVC(oldObj, newObj interface{}) { +func (ctrl *modifyController) updatePVC(oldObj, newObj any) { oldPVC, ok := oldObj.(*v1.PersistentVolumeClaim) if !ok || oldPVC == nil { return @@ -181,7 +181,7 @@ func (ctrl *modifyController) updatePVC(oldObj, newObj interface{}) { } } -func (ctrl *modifyController) deletePVC(obj interface{}) { +func (ctrl *modifyController) deletePVC(obj any) { objKey, err := util.GetObjectKey(obj) if err != nil { return @@ -278,7 +278,7 @@ func (ctrl *modifyController) syncPVC(key string) error { pv, err := ctrl.pvLister.Get(pvc.Spec.VolumeName) if err != nil { - return fmt.Errorf("Get PV %q of pvc %q in PVInformer cache failed: %v", pvc.Spec.VolumeName, klog.KObj(pvc), err) + return fmt.Errorf("get pv %q of pvc %q in PVInformer cache failed: %v", pvc.Spec.VolumeName, klog.KObj(pvc), err) } // Only trigger modify volume if the following conditions are met diff --git a/pkg/modifycontroller/modify_status_test.go b/pkg/modifycontroller/modify_status_test.go index c9e8b6ad0..17a30bafe 100644 --- a/pkg/modifycontroller/modify_status_test.go +++ b/pkg/modifycontroller/modify_status_test.go @@ -2,7 +2,7 @@ package modifycontroller import ( "context" - "reflect" + "errors" "testing" "time" @@ -37,7 +37,6 @@ var ( targetVac = "target-vac" testDriverName = "mock" infeasibleErr = status.Errorf(codes.InvalidArgument, "Parameters in VolumeAttributesClass is invalid") - finalErr = status.Errorf(codes.Internal, "Final error") pvcConditionInProgress = v1.PersistentVolumeClaimCondition{ Type: v1.PersistentVolumeClaimVolumeModifyingVolume, Status: v1.ConditionTrue, @@ -83,7 +82,7 @@ func TestMarkControllerModifyVolumeStatus(t *testing.T) { pvc: basePVC.Get(), expectedPVC: basePVC.WithModifyVolumeStatus(v1.PersistentVolumeClaimModifyVolumeInfeasible).Get(), expectedConditions: []v1.PersistentVolumeClaimCondition{pvcConditionInfeasible}, - expectedErr: infeasibleErr, + expectedErr: nil, testFunc: func(pvc *v1.PersistentVolumeClaim, ctrl *modifyController) (*v1.PersistentVolumeClaim, error) { return ctrl.markControllerModifyVolumeStatus(pvc, v1.PersistentVolumeClaimModifyVolumeInfeasible, infeasibleErr) }, @@ -126,14 +125,14 @@ func TestMarkControllerModifyVolumeStatus(t *testing.T) { ctrlInstance, _ := controller.(*modifyController) pvc, err = tc.testFunc(pvc, ctrlInstance) - if err != nil && !reflect.DeepEqual(tc.expectedErr, err) { - t.Errorf("Expected error to be %v but got %v", tc.expectedErr, err) + if !errors.Is(err, tc.expectedErr) { + t.Errorf("expected error: %v, got: %v", tc.expectedErr, err) } realStatus := pvc.Status.ModifyVolumeStatus.Status expectedStatus := tc.expectedPVC.Status.ModifyVolumeStatus.Status - if !reflect.DeepEqual(realStatus, expectedStatus) { - t.Errorf("expected modify volume status %+v got %+v", expectedStatus, realStatus) + if diff := cmp.Diff(expectedStatus, realStatus); diff != "" { + t.Errorf("unexpected modify volume status (-want +got):\n%s", diff) } realConditions := pvc.Status.Conditions @@ -167,8 +166,6 @@ func TestUpdateConditionBasedOnError(t *testing.T) { client := csi.NewMockClient("foo", true, true, true, true, true) driverName, _ := client.GetDriverName(context.TODO()) - pvc := test.pvc - var initialObjects []runtime.Object initialObjects = append(initialObjects, test.pvc) @@ -185,9 +182,9 @@ func TestUpdateConditionBasedOnError(t *testing.T) { ctrlInstance, _ := controller.(*modifyController) - pvc, err = ctrlInstance.updateConditionBasedOnError(tc.pvc, err) - if err != nil && !reflect.DeepEqual(tc.expectedErr, err) { - t.Errorf("Expected error to be %v but got %v", tc.expectedErr, err) + pvc, err := ctrlInstance.updateConditionBasedOnError(tc.pvc, err) + if !errors.Is(err, tc.expectedErr) { + t.Errorf("expected error: %v, got: %v", tc.expectedErr, err) } if !testutil.CompareConditions(pvc.Status.Conditions, tc.expectedConditions) { @@ -254,8 +251,8 @@ func TestMarkControllerModifyVolumeCompleted(t *testing.T) { ctrlInstance, _ := controller.(*modifyController) actualPVC, pv, err := ctrlInstance.markControllerModifyVolumeCompleted(tc.pvc, tc.pv) - if err != nil && !reflect.DeepEqual(tc.expectedErr, err) { - t.Errorf("Expected error to be %v but got %v", tc.expectedErr, err) + if !errors.Is(err, tc.expectedErr) { + t.Errorf("expected error: %v, got: %v", tc.expectedErr, err) } if len(actualPVC.Status.Conditions) == 0 { @@ -263,11 +260,11 @@ func TestMarkControllerModifyVolumeCompleted(t *testing.T) { } if diff := cmp.Diff(tc.expectedPVC, actualPVC); diff != "" { - t.Errorf("expected pvc %+v got %+v, diff is: %v", tc.expectedPVC, actualPVC, diff) + t.Errorf("unexpected pvc (-want +got):\n%s", diff) } if diff := cmp.Diff(tc.expectedPV, pv); diff != "" { - t.Errorf("expected pvc %+v got %+v, diff is: %v", tc.expectedPV, pv, diff) + t.Errorf("unexpected pv (-want +got):\n%s", diff) } }) } diff --git a/pkg/resizer/csi_resizer.go b/pkg/resizer/csi_resizer.go index a95aa0c98..7774c35fa 100644 --- a/pkg/resizer/csi_resizer.go +++ b/pkg/resizer/csi_resizer.go @@ -37,8 +37,8 @@ import ( ) var ( - controllerServiceNotSupportErr = errors.New("CSI driver does not support controller service") - resizeNotSupportErr = errors.New("CSI driver neither supports controller resize nor node resize") + errControllerServiceNotSupport = errors.New("CSI driver does not support controller service") + errResizeNotSupport = errors.New("CSI driver neither supports controller resize nor node resize") ) func NewResizerFromClient( @@ -53,7 +53,7 @@ func NewResizerFromClient( } if !supportControllerService { - return nil, controllerServiceNotSupportErr + return nil, errControllerServiceNotSupport } supportControllerResize, err := supportsControllerResize(csiClient, timeout) @@ -70,7 +70,7 @@ func NewResizerFromClient( klog.InfoS("The CSI driver supports node resize only, using trivial resizer to handle resize requests") return newTrivialResizer(driverName), nil } - return nil, resizeNotSupportErr + return nil, errResizeNotSupport } _, err = supportsControllerSingleNodeMultiWriter(csiClient, timeout) diff --git a/pkg/resizer/csi_resizer_test.go b/pkg/resizer/csi_resizer_test.go index ea6b9e023..c9ccd7d6d 100644 --- a/pkg/resizer/csi_resizer_test.go +++ b/pkg/resizer/csi_resizer_test.go @@ -42,7 +42,7 @@ func TestNewResizer(t *testing.T) { SupportsPluginControllerService: false, SupportsControllerSingleNodeMultiWriter: true, - Error: controllerServiceNotSupportErr, + Error: errControllerServiceNotSupport, }, // Controller modify not supported. { @@ -69,7 +69,7 @@ func TestNewResizer(t *testing.T) { SupportsPluginControllerService: true, SupportsControllerSingleNodeMultiWriter: true, - Error: resizeNotSupportErr, + Error: errResizeNotSupport, }, } { client := csi.NewMockClient("mock", c.SupportsNodeResize, c.SupportsControllerResize, false, c.SupportsPluginControllerService, c.SupportsControllerSingleNodeMultiWriter) diff --git a/pkg/util/util.go b/pkg/util/util.go index 5bb0c10ca..70fd29353 100644 --- a/pkg/util/util.go +++ b/pkg/util/util.go @@ -139,7 +139,7 @@ func GetPVCPatchData(oldPVC, newPVC *v1.PersistentVolumeClaim, addResourceVersio } func addResourceVersion(patchBytes []byte, resourceVersion string) ([]byte, error) { - var patchMap map[string]interface{} + var patchMap map[string]any err := json.Unmarshal(patchBytes, &patchMap) if err != nil { return nil, fmt.Errorf("error unmarshalling patch with %v", err) @@ -157,7 +157,7 @@ func addResourceVersion(patchBytes []byte, resourceVersion string) ([]byte, erro return versionBytes, nil } -func GetPatchData(oldObj, newObj interface{}) ([]byte, error) { +func GetPatchData(oldObj, newObj any) ([]byte, error) { oldData, err := json.Marshal(oldObj) if err != nil { return nil, fmt.Errorf("marshal old object failed: %v", err) @@ -195,7 +195,7 @@ func SanitizeName(name string) string { return name } -func GetObjectKey(obj interface{}) (string, error) { +func GetObjectKey(obj any) (string, error) { if unknown, ok := obj.(cache.DeletedFinalStateUnknown); ok && unknown.Obj != nil { obj = unknown.Obj } diff --git a/pkg/util/util_test.go b/pkg/util/util_test.go index ebe91b8b8..6a2ad1dec 100644 --- a/pkg/util/util_test.go +++ b/pkg/util/util_test.go @@ -60,13 +60,13 @@ func TestGetPVCPatchData(t *testing.T) { t.Errorf("Case %d: Get patch data failed: %v", i, err) } - var patchMap map[string]interface{} + var patchMap map[string]any err = json.Unmarshal(patchBytes, &patchMap) if err != nil { t.Errorf("Case %d: unmarshalling json patch failed: %v", i, err) } - metadata, exist := patchMap["metadata"].(map[string]interface{}) + metadata, exist := patchMap["metadata"].(map[string]any) if !exist { t.Errorf("Case %d: ResourceVersion should exist in patch data", i) } From 1e69bc0bb5b8b68262427bf9e61fec69a1ff0176 Mon Sep 17 00:00:00 2001 From: Grant He Date: Thu, 14 Aug 2025 22:01:04 +0000 Subject: [PATCH 2/3] Improve PVC condition message for ModifyVolume failing with infeasible errors. --- pkg/modifycontroller/modify_status.go | 2 +- pkg/modifycontroller/modify_status_test.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/modifycontroller/modify_status.go b/pkg/modifycontroller/modify_status.go index eb7c70320..9cfcc8954 100644 --- a/pkg/modifycontroller/modify_status.go +++ b/pkg/modifycontroller/modify_status.go @@ -48,7 +48,7 @@ func (ctrl *modifyController) markControllerModifyVolumeStatus( case v1.PersistentVolumeClaimModifyVolumeInProgress: conditionMessage = "ModifyVolume operation in progress." case v1.PersistentVolumeClaimModifyVolumeInfeasible: - conditionMessage = "ModifyVolume failed with error" + err.Error() + ". Waiting for retry." + conditionMessage = "ModifyVolume failed with error: " + err.Error() + ". Waiting for retry." } pvcCondition.Message = conditionMessage // Do not change conditions for pending modifications and keep existing conditions diff --git a/pkg/modifycontroller/modify_status_test.go b/pkg/modifycontroller/modify_status_test.go index 17a30bafe..b738ece11 100644 --- a/pkg/modifycontroller/modify_status_test.go +++ b/pkg/modifycontroller/modify_status_test.go @@ -46,7 +46,7 @@ var ( pvcConditionInfeasible = v1.PersistentVolumeClaimCondition{ Type: v1.PersistentVolumeClaimVolumeModifyingVolume, Status: v1.ConditionTrue, - Message: "ModifyVolume failed with errorrpc error: code = InvalidArgument desc = Parameters in VolumeAttributesClass is invalid. Waiting for retry.", + Message: "ModifyVolume failed with error: rpc error: code = InvalidArgument desc = Parameters in VolumeAttributesClass is invalid. Waiting for retry.", } pvcConditionError = v1.PersistentVolumeClaimCondition{ From 12f2c2d44d2a09108af11fab968897548cd817db Mon Sep 17 00:00:00 2001 From: Grant He Date: Wed, 17 Sep 2025 15:59:16 +0000 Subject: [PATCH 3/3] Modernize for loops --- pkg/controller/controller.go | 4 ++-- pkg/modifycontroller/controller.go | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/pkg/controller/controller.go b/pkg/controller/controller.go index dde4bfef0..a96ab0845 100644 --- a/pkg/controller/controller.go +++ b/pkg/controller/controller.go @@ -293,7 +293,7 @@ func (ctrl *resizeController) Run(workers int, ctx context.Context, wg *sync.Wai } if utilfeature.DefaultFeatureGate.Enabled(features.ReleaseLeaderElectionOnExit) { - for i := 0; i < workers; i++ { + for range workers { wg.Add(1) go func() { defer wg.Done() @@ -301,7 +301,7 @@ func (ctrl *resizeController) Run(workers int, ctx context.Context, wg *sync.Wai }() } } else { - for i := 0; i < workers; i++ { + for range workers { go wait.Until(ctrl.syncPVCs, 0, stopCh) } } diff --git a/pkg/modifycontroller/controller.go b/pkg/modifycontroller/controller.go index 2983e9d53..57005d756 100644 --- a/pkg/modifycontroller/controller.go +++ b/pkg/modifycontroller/controller.go @@ -224,7 +224,7 @@ func (ctrl *modifyController) Run( go ctrl.slowSet.Run(stopCh) if utilfeature.DefaultFeatureGate.Enabled(features.ReleaseLeaderElectionOnExit) { - for i := 0; i < workers; i++ { + for range workers { wg.Add(1) go func() { defer wg.Done() @@ -232,7 +232,7 @@ func (ctrl *modifyController) Run( }() } } else { - for i := 0; i < workers; i++ { + for range workers { go wait.Until(ctrl.sync, 0, stopCh) } }