From 5feea93163b5f1a3cf80f76fa0109e46e9aa9a0b Mon Sep 17 00:00:00 2001 From: Hemant Kumar Date: Mon, 11 Nov 2019 18:27:54 -0500 Subject: [PATCH] Rename MarkVolumeMountedOpts to MarkVolumeOpts Also remove VolumeNotMounted state --- .../volumemanager/cache/actual_state_of_world.go | 9 ++++----- .../cache/actual_state_of_world_test.go | 12 ++++++------ pkg/kubelet/volumemanager/metrics/metrics_test.go | 2 +- .../desired_state_of_world_populator_test.go | 2 +- pkg/kubelet/volumemanager/reconciler/reconciler.go | 2 +- pkg/volume/csi/csi_mounter.go | 1 + .../util/operationexecutor/operation_executor.go | 11 ++++------- .../util/operationexecutor/operation_generator.go | 5 ++--- 8 files changed, 20 insertions(+), 24 deletions(-) diff --git a/pkg/kubelet/volumemanager/cache/actual_state_of_world.go b/pkg/kubelet/volumemanager/cache/actual_state_of_world.go index 1aedc92af8d..c6172af508f 100644 --- a/pkg/kubelet/volumemanager/cache/actual_state_of_world.go +++ b/pkg/kubelet/volumemanager/cache/actual_state_of_world.go @@ -59,7 +59,7 @@ type ActualStateOfWorld interface { // volume, reset the pod's remountRequired value. // If a volume with the name volumeName does not exist in the list of // attached volumes, an error is returned. - AddPodToVolume(operationexecutor.MarkVolumeMountedOpts) error + AddPodToVolume(operationexecutor.MarkVolumeOpts) error // MarkRemountRequired marks each volume that is successfully attached and // mounted for the specified pod as requiring remount (if the plugin for the @@ -313,7 +313,6 @@ type mountedPod struct { // volumeMountStateForPod stores state of volume mount for the pod. if it is: // - VolumeMounted: means volume for pod has been successfully mounted // - VolumeMountUncertain: means volume for pod may not be mounted, but it must be unmounted - // - VolumeNotMounted: means volume for pod has not been mounted volumeMountStateForPod operationexecutor.VolumeMountState } @@ -332,7 +331,7 @@ func (asw *actualStateOfWorld) MarkVolumeAsDetached( asw.DeleteVolume(volumeName) } -func (asw *actualStateOfWorld) MarkVolumeAsMounted(markVolumeOpts operationexecutor.MarkVolumeMountedOpts) error { +func (asw *actualStateOfWorld) MarkVolumeAsMounted(markVolumeOpts operationexecutor.MarkVolumeOpts) error { return asw.AddPodToVolume(markVolumeOpts) } @@ -360,7 +359,7 @@ func (asw *actualStateOfWorld) MarkDeviceAsUncertain( return asw.SetDeviceMountState(volumeName, operationexecutor.DeviceMountUncertain, devicePath, deviceMountPath) } -func (asw *actualStateOfWorld) MarkVolumeMountAsUncertain(markVolumeOpts operationexecutor.MarkVolumeMountedOpts) error { +func (asw *actualStateOfWorld) MarkVolumeMountAsUncertain(markVolumeOpts operationexecutor.MarkVolumeOpts) error { markVolumeOpts.VolumeMountState = operationexecutor.VolumeMountUncertain return asw.AddPodToVolume(markVolumeOpts) } @@ -428,7 +427,7 @@ func (asw *actualStateOfWorld) addVolume( return nil } -func (asw *actualStateOfWorld) AddPodToVolume(markVolumeOpts operationexecutor.MarkVolumeMountedOpts) error { +func (asw *actualStateOfWorld) AddPodToVolume(markVolumeOpts operationexecutor.MarkVolumeOpts) error { podName := markVolumeOpts.PodName podUID := markVolumeOpts.PodUID volumeName := markVolumeOpts.VolumeName diff --git a/pkg/kubelet/volumemanager/cache/actual_state_of_world_test.go b/pkg/kubelet/volumemanager/cache/actual_state_of_world_test.go index 684c02afe0d..e44f958a4ad 100644 --- a/pkg/kubelet/volumemanager/cache/actual_state_of_world_test.go +++ b/pkg/kubelet/volumemanager/cache/actual_state_of_world_test.go @@ -221,7 +221,7 @@ func Test_AddPodToVolume_Positive_ExistingVolumeNewNode(t *testing.T) { } // Act - markVolumeOpts := operationexecutor.MarkVolumeMountedOpts{ + markVolumeOpts := operationexecutor.MarkVolumeOpts{ PodName: podName, PodUID: pod.UID, VolumeName: generatedVolumeName, @@ -295,7 +295,7 @@ func Test_AddPodToVolume_Positive_ExistingVolumeExistingNode(t *testing.T) { t.Fatalf("NewBlockVolumeMapper failed. Expected: Actual: <%v>", err) } - markVolumeOpts := operationexecutor.MarkVolumeMountedOpts{ + markVolumeOpts := operationexecutor.MarkVolumeOpts{ PodName: podName, PodUID: pod.UID, VolumeName: generatedVolumeName, @@ -402,7 +402,7 @@ func Test_AddTwoPodsToVolume_Positive(t *testing.T) { t.Fatalf("NewBlockVolumeMapper failed. Expected: Actual: <%v>", err) } - markVolumeOpts1 := operationexecutor.MarkVolumeMountedOpts{ + markVolumeOpts1 := operationexecutor.MarkVolumeOpts{ PodName: podName1, PodUID: pod1.UID, VolumeName: generatedVolumeName1, @@ -428,7 +428,7 @@ func Test_AddTwoPodsToVolume_Positive(t *testing.T) { t.Fatalf("NewBlockVolumeMapper failed. Expected: Actual: <%v>", err) } - markVolumeOpts2 := operationexecutor.MarkVolumeMountedOpts{ + markVolumeOpts2 := operationexecutor.MarkVolumeOpts{ PodName: podName2, PodUID: pod2.UID, VolumeName: generatedVolumeName1, @@ -513,7 +513,7 @@ func Test_AddPodToVolume_Negative_VolumeDoesntExist(t *testing.T) { } // Act - markVolumeOpts := operationexecutor.MarkVolumeMountedOpts{ + markVolumeOpts := operationexecutor.MarkVolumeOpts{ PodName: podName, PodUID: pod.UID, VolumeName: volumeName, @@ -632,7 +632,7 @@ func TestUncertainVolumeMounts(t *testing.T) { t.Fatalf("NewMounter failed. Expected: Actual: <%v>", err) } - markVolumeOpts1 := operationexecutor.MarkVolumeMountedOpts{ + markVolumeOpts1 := operationexecutor.MarkVolumeOpts{ PodName: podName1, PodUID: pod1.UID, VolumeName: generatedVolumeName1, diff --git a/pkg/kubelet/volumemanager/metrics/metrics_test.go b/pkg/kubelet/volumemanager/metrics/metrics_test.go index 2af2c17cdf5..33f4d27eb01 100644 --- a/pkg/kubelet/volumemanager/metrics/metrics_test.go +++ b/pkg/kubelet/volumemanager/metrics/metrics_test.go @@ -78,7 +78,7 @@ func TestMetricCollection(t *testing.T) { t.Fatalf("MarkVolumeAsAttached failed. Expected: Actual: <%v>", err) } - markVolumeOpts := operationexecutor.MarkVolumeMountedOpts{ + markVolumeOpts := operationexecutor.MarkVolumeOpts{ PodName: podName, PodUID: pod.UID, VolumeName: generatedVolumeName, diff --git a/pkg/kubelet/volumemanager/populator/desired_state_of_world_populator_test.go b/pkg/kubelet/volumemanager/populator/desired_state_of_world_populator_test.go index 423ac509cae..9f887a18305 100644 --- a/pkg/kubelet/volumemanager/populator/desired_state_of_world_populator_test.go +++ b/pkg/kubelet/volumemanager/populator/desired_state_of_world_populator_test.go @@ -855,7 +855,7 @@ func reconcileASW(asw cache.ActualStateOfWorld, dsw cache.DesiredStateOfWorld, t if err != nil { t.Fatalf("Unexpected error when MarkVolumeAsAttached: %v", err) } - markVolumeOpts := operationexecutor.MarkVolumeMountedOpts{ + markVolumeOpts := operationexecutor.MarkVolumeOpts{ PodName: volumeToMount.PodName, PodUID: volumeToMount.Pod.UID, VolumeName: volumeToMount.VolumeName, diff --git a/pkg/kubelet/volumemanager/reconciler/reconciler.go b/pkg/kubelet/volumemanager/reconciler/reconciler.go index ec46ea492ac..adb2e2038cd 100644 --- a/pkg/kubelet/volumemanager/reconciler/reconciler.go +++ b/pkg/kubelet/volumemanager/reconciler/reconciler.go @@ -640,7 +640,7 @@ func (rc *reconciler) updateStates(volumesNeedUpdate map[v1.UniqueVolumeName]*re klog.Errorf("Could not add volume information to actual state of world: %v", err) continue } - markVolumeOpts := operationexecutor.MarkVolumeMountedOpts{ + markVolumeOpts := operationexecutor.MarkVolumeOpts{ PodName: volume.podName, PodUID: types.UID(volume.podName), VolumeName: volume.volumeName, diff --git a/pkg/volume/csi/csi_mounter.go b/pkg/volume/csi/csi_mounter.go index 1915adcac5d..9b9c4e76342 100644 --- a/pkg/volume/csi/csi_mounter.go +++ b/pkg/volume/csi/csi_mounter.go @@ -126,6 +126,7 @@ func (c *csiMountMgr) setupUtil(dir string, mounterArgs volume.MounterArgs) (vol csi, err := c.csiClientGetter.Get() if err != nil { + opExitStatus = volumetypes.OperationStateNoChange return opExitStatus, errors.New(log("mounter.SetUpAt failed to get CSI client: %v", err)) } ctx, cancel := context.WithTimeout(context.Background(), csiTimeout) diff --git a/pkg/volume/util/operationexecutor/operation_executor.go b/pkg/volume/util/operationexecutor/operation_executor.go index 010299cee41..8c0bdf0fca4 100644 --- a/pkg/volume/util/operationexecutor/operation_executor.go +++ b/pkg/volume/util/operationexecutor/operation_executor.go @@ -160,8 +160,8 @@ func NewOperationExecutor( } } -// MarkVolumeMountedOpts is an struct to pass arguments to MountVolume functions -type MarkVolumeMountedOpts struct { +// MarkVolumeOpts is an struct to pass arguments to MountVolume functions +type MarkVolumeOpts struct { PodName volumetypes.UniquePodName PodUID types.UID VolumeName v1.UniqueVolumeName @@ -177,13 +177,13 @@ type MarkVolumeMountedOpts struct { // state of the world cache after successful mount/unmount. type ActualStateOfWorldMounterUpdater interface { // Marks the specified volume as mounted to the specified pod - MarkVolumeAsMounted(markVolumeOpts MarkVolumeMountedOpts) error + MarkVolumeAsMounted(markVolumeOpts MarkVolumeOpts) error // Marks the specified volume as unmounted from the specified pod MarkVolumeAsUnmounted(podName volumetypes.UniquePodName, volumeName v1.UniqueVolumeName) error // MarkVolumeMountAsUncertain marks state of volume mount for the pod uncertain - MarkVolumeMountAsUncertain(markVolumeOpts MarkVolumeMountedOpts) error + MarkVolumeMountAsUncertain(markVolumeOpts MarkVolumeOpts) error // Marks the specified volume as having been globally mounted. MarkDeviceAsMounted(volumeName v1.UniqueVolumeName, devicePath, deviceMountPath string) error @@ -397,9 +397,6 @@ const ( // VolumeMountUncertain means volume may or may not be mounted in pods' local path VolumeMountUncertain VolumeMountState = "VolumeMountUncertain" - - // VolumeNotMounted means volume has not been mounted in pod's local path - VolumeNotMounted VolumeMountState = "VolumeNotMounted" ) // GenerateMsgDetailed returns detailed msgs for volumes to mount diff --git a/pkg/volume/util/operationexecutor/operation_generator.go b/pkg/volume/util/operationexecutor/operation_generator.go index 8b080560127..1da44f42b04 100644 --- a/pkg/volume/util/operationexecutor/operation_generator.go +++ b/pkg/volume/util/operationexecutor/operation_generator.go @@ -634,7 +634,7 @@ func (og *operationGenerator) GenerateMountVolumeFunc( DesiredSize: volumeToMount.DesiredSizeLimit, }) // Update actual state of world - markOpts := MarkVolumeMountedOpts{ + markOpts := MarkVolumeOpts{ PodName: volumeToMount.PodName, PodUID: volumeToMount.Pod.UID, VolumeName: volumeToMount.VolumeName, @@ -653,7 +653,6 @@ func (og *operationGenerator) GenerateMountVolumeFunc( klog.Errorf(volumeToMount.GenerateErrorDetailed("MountVolume.MarkVolumeMountAsUncertain failed", t).Error()) } case volumetypes.OperationFinished: - markOpts.VolumeMountState = VolumeNotMounted t := actualStateOfWorld.MarkVolumeAsUnmounted(volumeToMount.PodName, volumeToMount.VolumeName) if t != nil { klog.Errorf(volumeToMount.GenerateErrorDetailed("MountVolume.MarkVolumeAsUnmounted failed", t).Error()) @@ -1021,7 +1020,7 @@ func (og *operationGenerator) GenerateMapVolumeFunc( return volumeToMount.GenerateError("MapVolume.MarkVolumeAsMounted failed while expanding volume", resizeError) } - markVolumeOpts := MarkVolumeMountedOpts{ + markVolumeOpts := MarkVolumeOpts{ PodName: volumeToMount.PodName, PodUID: volumeToMount.Pod.UID, VolumeName: volumeToMount.VolumeName,