From 7563b4d01b5e5dde148f746ccb1ff47c5b5b9272 Mon Sep 17 00:00:00 2001 From: Hemant Kumar Date: Wed, 29 May 2019 11:01:45 -0400 Subject: [PATCH] Address comments about retry in online resizing block --- .../volume/expand/expand_controller_test.go | 3 ++ .../operationexecutor/operation_generator.go | 52 ++++++++++++------- 2 files changed, 37 insertions(+), 18 deletions(-) diff --git a/pkg/controller/volume/expand/expand_controller_test.go b/pkg/controller/volume/expand/expand_controller_test.go index 15cd524f30b..5226e94e897 100644 --- a/pkg/controller/volume/expand/expand_controller_test.go +++ b/pkg/controller/volume/expand/expand_controller_test.go @@ -131,6 +131,9 @@ func TestSyncHandler(t *testing.T) { if test.csiMigrationEnabled { defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.CSIMigration, true)() defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.CSIMigrationAWS, true)() + } else { + defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.CSIMigration, false)() + defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.CSIMigrationAWS, false)() } var expController *expandController diff --git a/pkg/volume/util/operationexecutor/operation_generator.go b/pkg/volume/util/operationexecutor/operation_generator.go index 9ae816bffd9..5704be54258 100644 --- a/pkg/volume/util/operationexecutor/operation_generator.go +++ b/pkg/volume/util/operationexecutor/operation_generator.go @@ -1577,25 +1577,23 @@ func (og *operationGenerator) GenerateExpandVolumeFunc( func (og *operationGenerator) GenerateExpandVolumeFSWithoutUnmountingFunc( volumeToMount VolumeToMount, actualStateOfWorld ActualStateOfWorldMounterUpdater) (volumetypes.GeneratedOperations, error) { - // Need to translate the spec here if the plugin is migrated so that the metrics - // emitted show the correct (migrated) plugin - if useCSIPlugin(og.volumePluginMgr, volumeToMount.VolumeSpec) { - csiSpec, err := translateSpec(volumeToMount.VolumeSpec) - if err == nil { - volumeToMount.VolumeSpec = csiSpec - } - // If we have an error here we ignore it, the metric emitted will then be for the - // in-tree plugin. This error case(skipped one) will also trigger an error - // while the generated function is executed. And those errors will be handled during the execution of the generated - // function with a back off policy. - } - volumePlugin, err := - og.volumePluginMgr.FindPluginBySpec(volumeToMount.VolumeSpec) - if err != nil || volumePlugin == nil { - return volumetypes.GeneratedOperations{}, volumeToMount.GenerateErrorDetailed("VolumeFSResize.FindPluginBySpec failed", err) - } fsResizeFunc := func() (error, error) { + // Need to translate the spec here if the plugin is migrated so that the metrics + // emitted show the correct (migrated) plugin + if useCSIPlugin(og.volumePluginMgr, volumeToMount.VolumeSpec) { + csiSpec, err := translateSpec(volumeToMount.VolumeSpec) + if err != nil { + return volumeToMount.GenerateError("VolumeFSResize.translateSpec failed", err) + } + volumeToMount.VolumeSpec = csiSpec + } + volumePlugin, err := + og.volumePluginMgr.FindPluginBySpec(volumeToMount.VolumeSpec) + if err != nil || volumePlugin == nil { + return volumeToMount.GenerateError("VolumeFSResize.FindPluginBySpec failed", err) + } + var resizeDone bool var simpleErr, detailedErr error resizeOptions := volume.NodeResizeOptions{ @@ -1643,7 +1641,7 @@ func (og *operationGenerator) GenerateExpandVolumeFSWithoutUnmountingFunc( return nil, nil } // This is a placeholder error - we should NEVER reach here. - err := fmt.Errorf("volume resizing failed for unknown reason") + err = fmt.Errorf("volume resizing failed for unknown reason") return volumeToMount.GenerateError("VolumeFSResize.resizeFileSystem failed to resize volume", err) } @@ -1653,6 +1651,24 @@ func (og *operationGenerator) GenerateExpandVolumeFSWithoutUnmountingFunc( } } + // Need to translate the spec here if the plugin is migrated so that the metrics + // emitted show the correct (migrated) plugin + if useCSIPlugin(og.volumePluginMgr, volumeToMount.VolumeSpec) { + csiSpec, err := translateSpec(volumeToMount.VolumeSpec) + if err == nil { + volumeToMount.VolumeSpec = csiSpec + } + // If we have an error here we ignore it, the metric emitted will then be for the + // in-tree plugin. This error case(skipped one) will also trigger an error + // while the generated function is executed. And those errors will be handled during the execution of the generated + // function with a back off policy. + } + volumePlugin, err := + og.volumePluginMgr.FindPluginBySpec(volumeToMount.VolumeSpec) + if err != nil || volumePlugin == nil { + return volumetypes.GeneratedOperations{}, volumeToMount.GenerateErrorDetailed("VolumeFSResize.FindPluginBySpec failed", err) + } + return volumetypes.GeneratedOperations{ OperationName: "volume_fs_resize", OperationFunc: fsResizeFunc,