diff --git a/hack/.staticcheck_failures b/hack/.staticcheck_failures index a90443e253d..7931c65fae6 100644 --- a/hack/.staticcheck_failures +++ b/hack/.staticcheck_failures @@ -53,7 +53,6 @@ pkg/volume/flexvolume pkg/volume/flocker pkg/volume/hostpath pkg/volume/iscsi -pkg/volume/local pkg/volume/portworx pkg/volume/quobyte pkg/volume/rbd diff --git a/pkg/controller/volume/attachdetach/testing/testvolumespec.go b/pkg/controller/volume/attachdetach/testing/testvolumespec.go index 22d689f2803..eac942b0895 100644 --- a/pkg/controller/volume/attachdetach/testing/testvolumespec.go +++ b/pkg/controller/volume/attachdetach/testing/testvolumespec.go @@ -435,7 +435,7 @@ func (attacher *testPluginAttacher) GetDeviceMountPath(spec *volume.Spec) (strin return "", nil } -func (attacher *testPluginAttacher) MountDevice(spec *volume.Spec, devicePath string, deviceMountPath string) error { +func (attacher *testPluginAttacher) MountDevice(spec *volume.Spec, devicePath string, deviceMountPath string) (volumetypes.OperationStatus, error) { attacher.pluginLock.Lock() defer attacher.pluginLock.Unlock() if spec == nil { diff --git a/pkg/kubelet/kubelet_volumes_test.go b/pkg/kubelet/kubelet_volumes_test.go index b5d2c168e55..85e86ca55c6 100644 --- a/pkg/kubelet/kubelet_volumes_test.go +++ b/pkg/kubelet/kubelet_volumes_test.go @@ -531,18 +531,14 @@ func (f *stubVolume) CanMount() error { return nil } -func (f *stubVolume) SetUp(mounterArgs volume.MounterArgs) error { - return nil +func (f *stubVolume) SetUp(mounterArgs volume.MounterArgs) (volumetypes.OperationStatus, error) { + return volumetypes.OperationFinished, nil } func (f *stubVolume) SetUpAt(dir string, mounterArgs volume.MounterArgs) error { return nil } -func (f *stubVolume) SetUpWithStatusTracking(mountArgs volume.MounterArgs) (volumetypes.OperationStatus, error) { - return volumetypes.OperationFinished, nil -} - type stubBlockVolume struct { dirPath string volName string diff --git a/pkg/kubelet/volumemanager/cache/actual_state_of_world.go b/pkg/kubelet/volumemanager/cache/actual_state_of_world.go index e68b3380480..1aedc92af8d 100644 --- a/pkg/kubelet/volumemanager/cache/actual_state_of_world.go +++ b/pkg/kubelet/volumemanager/cache/actual_state_of_world.go @@ -173,7 +173,8 @@ type AttachedVolume struct { DeviceMountState operationexecutor.DeviceMountState } -// DeviceMayBeMounted returns true if device may be mounted in global path. +// DeviceMayBeMounted returns true if device is mounted in global path or is in +// uncertain state. func (av AttachedVolume) DeviceMayBeMounted() bool { return av.DeviceMountState == operationexecutor.DeviceGloballyMounted || av.DeviceMountState == operationexecutor.DeviceMountUncertain diff --git a/pkg/volume/awsebs/aws_ebs.go b/pkg/volume/awsebs/aws_ebs.go index 23e81f0ebcc..11bbd2d17f0 100644 --- a/pkg/volume/awsebs/aws_ebs.go +++ b/pkg/volume/awsebs/aws_ebs.go @@ -366,13 +366,8 @@ func (b *awsElasticBlockStoreMounter) CanMount() error { } // SetUp attaches the disk and bind mounts to the volume path. -func (b *awsElasticBlockStoreMounter) SetUp(mounterArgs volume.MounterArgs) error { - return b.SetUpAt(b.GetPath(), mounterArgs) -} - -// SetupWithStatusTracking attaches the disk and bind mounts to the volume path. -func (b *awsElasticBlockStoreMounter) SetUpWithStatusTracking(mounterArgs volume.MounterArgs) (volumetypes.OperationStatus, error) { - err := b.SetUp(mounterArgs) +func (b *awsElasticBlockStoreMounter) SetUp(mounterArgs volume.MounterArgs) (volumetypes.OperationStatus, error) { + err := b.SetUpAt(b.GetPath(), mounterArgs) return volumetypes.OperationFinished, err } diff --git a/pkg/volume/awsebs/aws_ebs_test.go b/pkg/volume/awsebs/aws_ebs_test.go index 021ecc46dc5..db0fcca91cb 100644 --- a/pkg/volume/awsebs/aws_ebs_test.go +++ b/pkg/volume/awsebs/aws_ebs_test.go @@ -141,7 +141,7 @@ func TestPlugin(t *testing.T) { t.Errorf("Got unexpected path: %s", path) } - if err := mounter.SetUp(volume.MounterArgs{}); err != nil { + if _, err := mounter.SetUp(volume.MounterArgs{}); err != nil { t.Errorf("Expected success, got: %v", err) } if _, err := os.Stat(path); err != nil { @@ -372,7 +372,7 @@ func TestMountOptions(t *testing.T) { t.Errorf("Got a nil Mounter") } - if err := mounter.SetUp(volume.MounterArgs{}); err != nil { + if _, err := mounter.SetUp(volume.MounterArgs{}); err != nil { t.Errorf("Expected success, got: %v", err) } mountOptions := fakeMounter.MountPoints[0].Opts diff --git a/pkg/volume/azure_dd/azure_mounter.go b/pkg/volume/azure_dd/azure_mounter.go index c7d5f23441a..f641e78e789 100644 --- a/pkg/volume/azure_dd/azure_mounter.go +++ b/pkg/volume/azure_dd/azure_mounter.go @@ -66,12 +66,8 @@ func (m *azureDiskMounter) CanMount() error { return nil } -func (m *azureDiskMounter) SetUp(mounterArgs volume.MounterArgs) error { - return m.SetUpAt(m.GetPath(), mounterArgs) -} - -func (m *azureDiskMounter) SetUpWithStatusTracking(mounterArgs volume.MounterArgs) (volumetypes.OperationStatus, error) { - err := m.SetUp(mounterArgs) +func (m *azureDiskMounter) SetUp(mounterArgs volume.MounterArgs) (volumetypes.OperationStatus, error) { + err := m.SetUpAt(m.GetPath(), mounterArgs) return volumetypes.OperationFinished, err } diff --git a/pkg/volume/azure_file/azure_file.go b/pkg/volume/azure_file/azure_file.go index 56b88600751..427e88e1043 100644 --- a/pkg/volume/azure_file/azure_file.go +++ b/pkg/volume/azure_file/azure_file.go @@ -236,13 +236,8 @@ func (b *azureFileMounter) CanMount() error { } // SetUp attaches the disk and bind mounts to the volume path. -func (b *azureFileMounter) SetUp(mounterArgs volume.MounterArgs) error { - return b.SetUpAt(b.GetPath(), mounterArgs) -} - -// SetUp attaches the disk and bind mounts to the volume path. -func (b *azureFileMounter) SetUpWithStatusTracking(mounterArgs volume.MounterArgs) (volumetypes.OperationStatus, error) { - err := b.SetUp(mounterArgs) +func (b *azureFileMounter) SetUp(mounterArgs volume.MounterArgs) (volumetypes.OperationStatus, error) { + err := b.SetUpAt(b.GetPath(), mounterArgs) return volumetypes.OperationFinished, err } diff --git a/pkg/volume/azure_file/azure_file_test.go b/pkg/volume/azure_file/azure_file_test.go index 6d42eb9ad8c..d3407b8959a 100644 --- a/pkg/volume/azure_file/azure_file_test.go +++ b/pkg/volume/azure_file/azure_file_test.go @@ -154,7 +154,7 @@ func testPlugin(t *testing.T, tmpDir string, volumeHost volume.VolumeHost) { t.Errorf("Got unexpected path: %s", path) } - if err := mounter.SetUp(volume.MounterArgs{}); err != nil { + if _, err := mounter.SetUp(volume.MounterArgs{}); err != nil { t.Errorf("Expected success, got: %v", err) } if _, err := os.Stat(path); err != nil { diff --git a/pkg/volume/cephfs/cephfs.go b/pkg/volume/cephfs/cephfs.go index bfa86d8c6a1..f9cd9be0d49 100644 --- a/pkg/volume/cephfs/cephfs.go +++ b/pkg/volume/cephfs/cephfs.go @@ -220,13 +220,8 @@ func (cephfsVolume *cephfsMounter) CanMount() error { } // SetUp attaches the disk and bind mounts to the volume path. -func (cephfsVolume *cephfsMounter) SetUp(mounterArgs volume.MounterArgs) error { - return cephfsVolume.SetUpAt(cephfsVolume.GetPath(), mounterArgs) -} - -// SetUp attaches the disk and bind mounts to the volume path. -func (cephfsVolume *cephfsMounter) SetUpWithStatusTracking(mounterArgs volume.MounterArgs) (volumetypes.OperationStatus, error) { - err := cephfsVolume.SetUp(mounterArgs) +func (cephfsVolume *cephfsMounter) SetUp(mounterArgs volume.MounterArgs) (volumetypes.OperationStatus, error) { + err := cephfsVolume.SetUpAt(cephfsVolume.GetPath(), mounterArgs) return volumetypes.OperationFinished, err } diff --git a/pkg/volume/cephfs/cephfs_test.go b/pkg/volume/cephfs/cephfs_test.go index abe498c5f10..ea674d80ff8 100644 --- a/pkg/volume/cephfs/cephfs_test.go +++ b/pkg/volume/cephfs/cephfs_test.go @@ -88,7 +88,7 @@ func TestPlugin(t *testing.T) { if volumePath != volpath { t.Errorf("Got unexpected path: %s", volumePath) } - if err := mounter.SetUp(volume.MounterArgs{}); err != nil { + if _, err := mounter.SetUp(volume.MounterArgs{}); err != nil { t.Errorf("Expected success, got: %v", err) } if _, err := os.Stat(volumePath); err != nil { diff --git a/pkg/volume/cinder/cinder.go b/pkg/volume/cinder/cinder.go index 8a60ac8a8a8..d694775f35a 100644 --- a/pkg/volume/cinder/cinder.go +++ b/pkg/volume/cinder/cinder.go @@ -390,12 +390,8 @@ func (b *cinderVolumeMounter) CanMount() error { return nil } -func (b *cinderVolumeMounter) SetUp(mounterArgs volume.MounterArgs) error { - return b.SetUpAt(b.GetPath(), mounterArgs) -} - -func (b *cinderVolumeMounter) SetUpWithStatusTracking(mounterArgs volume.MounterArgs) (volumetypes.OperationStatus, error) { - err := b.SetUp(mounterArgs) +func (b *cinderVolumeMounter) SetUp(mounterArgs volume.MounterArgs) (volumetypes.OperationStatus, error) { + err := b.SetUpAt(b.GetPath(), mounterArgs) return volumetypes.OperationFinished, err } diff --git a/pkg/volume/cinder/cinder_test.go b/pkg/volume/cinder/cinder_test.go index bd0d0c6b929..5a151f5fc33 100644 --- a/pkg/volume/cinder/cinder_test.go +++ b/pkg/volume/cinder/cinder_test.go @@ -169,7 +169,7 @@ func TestPlugin(t *testing.T) { t.Errorf("Got unexpected path: %s", path) } - if err := mounter.SetUp(volume.MounterArgs{}); err != nil { + if _, err := mounter.SetUp(volume.MounterArgs{}); err != nil { t.Errorf("Expected success, got: %v", err) } if _, err := os.Stat(path); err != nil { diff --git a/pkg/volume/configmap/configmap.go b/pkg/volume/configmap/configmap.go index 0c80c34f6b9..4347b4b2dad 100644 --- a/pkg/volume/configmap/configmap.go +++ b/pkg/volume/configmap/configmap.go @@ -181,12 +181,8 @@ func (b *configMapVolumeMounter) CanMount() error { return nil } -func (b *configMapVolumeMounter) SetUp(mounterArgs volume.MounterArgs) error { - return b.SetUpAt(b.GetPath(), mounterArgs) -} - -func (b *configMapVolumeMounter) SetUpWithStatusTracking(mounterArgs volume.MounterArgs) (volumetypes.OperationStatus, error) { - err := b.SetUp(mounterArgs) +func (b *configMapVolumeMounter) SetUp(mounterArgs volume.MounterArgs) (volumetypes.OperationStatus, error) { + err := b.SetUpAt(b.GetPath(), mounterArgs) return volumetypes.OperationFinished, err } diff --git a/pkg/volume/configmap/configmap_test.go b/pkg/volume/configmap/configmap_test.go index e03d39d1a25..0e7b2172ba7 100644 --- a/pkg/volume/configmap/configmap_test.go +++ b/pkg/volume/configmap/configmap_test.go @@ -368,7 +368,7 @@ func TestPlugin(t *testing.T) { var mounterArgs volume.MounterArgs group := int64(1001) mounterArgs.FsGroup = &group - err = mounter.SetUp(mounterArgs) + _, err = mounter.SetUp(mounterArgs) if err != nil { t.Errorf("Failed to setup volume: %v", err) } @@ -428,7 +428,7 @@ func TestPluginReboot(t *testing.T) { var mounterArgs volume.MounterArgs group := int64(1001) mounterArgs.FsGroup = &group - err = mounter.SetUp(mounterArgs) + _, err = mounter.SetUp(mounterArgs) if err != nil { t.Errorf("Failed to setup volume: %v", err) } @@ -492,7 +492,7 @@ func TestPluginOptional(t *testing.T) { var mounterArgs volume.MounterArgs group := int64(1001) mounterArgs.FsGroup = &group - err = mounter.SetUp(mounterArgs) + _, err = mounter.SetUp(mounterArgs) if err != nil { t.Errorf("Failed to setup volume: %v", err) } @@ -591,7 +591,7 @@ func TestPluginKeysOptional(t *testing.T) { var mounterArgs volume.MounterArgs group := int64(1001) mounterArgs.FsGroup = &group - err = mounter.SetUp(mounterArgs) + _, err = mounter.SetUp(mounterArgs) if err != nil { t.Errorf("Failed to setup volume: %v", err) } @@ -671,7 +671,7 @@ func TestInvalidConfigMapSetup(t *testing.T) { var mounterArgs volume.MounterArgs group := int64(1001) mounterArgs.FsGroup = &group - err = mounter.SetUp(mounterArgs) + _, err = mounter.SetUp(mounterArgs) if err == nil { t.Errorf("Expected setup to fail") } diff --git a/pkg/volume/csi/csi_attacher_test.go b/pkg/volume/csi/csi_attacher_test.go index 6e4bcc258fa..22b4de03eea 100644 --- a/pkg/volume/csi/csi_attacher_test.go +++ b/pkg/volume/csi/csi_attacher_test.go @@ -1199,7 +1199,7 @@ func TestAttacherMountDevice(t *testing.T) { } // Run - exitStatus, err := csiAttacher.MountDeviceWithStatusTracking(tc.spec, tc.devicePath, tc.deviceMountPath) + exitStatus, err := csiAttacher.MountDevice(tc.spec, tc.devicePath, tc.deviceMountPath) // Verify if err != nil { diff --git a/pkg/volume/csi/csi_client.go b/pkg/volume/csi/csi_client.go index aa00d85e859..939e4d3a42b 100644 --- a/pkg/volume/csi/csi_client.go +++ b/pkg/volume/csi/csi_client.go @@ -632,18 +632,18 @@ func isFinalError(err error) bool { if !ok { // This is not gRPC error. The operation must have failed before gRPC // method was called, otherwise we would get gRPC error. - // We don't know if any previous CreateVolume is in progress, be on the safe side. + // We don't know if any previous volume operation is in progress, be on the safe side. return false } switch st.Code() { case codes.Canceled, // gRPC: Client Application cancelled the request codes.DeadlineExceeded, // gRPC: Timeout - codes.Unavailable, // gRPC: Server shutting down, TCP connection broken - previous CreateVolume() may be still in progress. - codes.ResourceExhausted, // gRPC: Server temporarily out of resources - previous Publish operation may be still in progress. + codes.Unavailable, // gRPC: Server shutting down, TCP connection broken - previous volume operation may be still in progress. + codes.ResourceExhausted, // gRPC: Server temporarily out of resources - previous volume operation may be still in progress. codes.Aborted: // CSI: Operation pending for volume return false } - // All other errors mean that provisioning either did not + // All other errors mean that operation either did not // even start or failed. It is for sure not in progress. return true } diff --git a/pkg/volume/csi/csi_mounter.go b/pkg/volume/csi/csi_mounter.go index 22a5085bd03..1915adcac5d 100644 --- a/pkg/volume/csi/csi_mounter.go +++ b/pkg/volume/csi/csi_mounter.go @@ -99,11 +99,7 @@ func (c *csiMountMgr) CanMount() error { return nil } -func (c *csiMountMgr) SetUp(mounterArgs volume.MounterArgs) error { - return c.SetUpAt(c.GetPath(), mounterArgs) -} - -func (c *csiMountMgr) SetUpWithStatusTracking(mounterArgs volume.MounterArgs) (volumetypes.OperationStatus, error) { +func (c *csiMountMgr) SetUp(mounterArgs volume.MounterArgs) (volumetypes.OperationStatus, error) { opExitStatus, err := c.setupUtil(c.GetPath(), mounterArgs) return opExitStatus, err } diff --git a/pkg/volume/csi/csi_mounter_test.go b/pkg/volume/csi/csi_mounter_test.go index c397ddf2bae..a8e9d544671 100644 --- a/pkg/volume/csi/csi_mounter_test.go +++ b/pkg/volume/csi/csi_mounter_test.go @@ -221,7 +221,7 @@ func MounterSetUpTests(t *testing.T, podInfoEnabled bool) { var mounterArgs volume.MounterArgs fsGroup := int64(2000) mounterArgs.FsGroup = &fsGroup - if err := csiMounter.SetUp(mounterArgs); err != nil { + if _, err := csiMounter.SetUp(mounterArgs); err != nil { t.Fatalf("mounter.Setup failed: %v", err) } @@ -361,7 +361,7 @@ func TestMounterSetUpSimple(t *testing.T) { } // Mounter.SetUp() - if err := csiMounter.SetUp(volume.MounterArgs{}); err != nil { + if _, err := csiMounter.SetUp(volume.MounterArgs{}); err != nil { t.Fatalf("mounter.Setup failed: %v", err) } @@ -488,7 +488,7 @@ func TestMounterSetupWithStatusTracking(t *testing.T) { } } - opExistStatus, err := csiMounter.SetUpWithStatusTracking(volume.MounterArgs{}) + opExistStatus, err := csiMounter.SetUp(volume.MounterArgs{}) if opExistStatus != tc.exitStatus { t.Fatalf("expected exitStatus: %v but got %v", tc.exitStatus, opExistStatus) @@ -604,7 +604,7 @@ func TestMounterSetUpWithInline(t *testing.T) { } // Mounter.SetUp() - if err := csiMounter.SetUp(volume.MounterArgs{}); err != nil { + if _, err := csiMounter.SetUp(volume.MounterArgs{}); err != nil { t.Fatalf("mounter.Setup failed: %v", err) } @@ -757,7 +757,7 @@ func TestMounterSetUpWithFSGroup(t *testing.T) { fsGroupPtr = &fsGroup } mounterArgs.FsGroup = fsGroupPtr - if err := csiMounter.SetUp(mounterArgs); err != nil { + if _, err := csiMounter.SetUp(mounterArgs); err != nil { t.Fatalf("mounter.Setup failed: %v", err) } diff --git a/pkg/volume/csi/csi_test.go b/pkg/volume/csi/csi_test.go index 5d310c81308..0ee4858f955 100644 --- a/pkg/volume/csi/csi_test.go +++ b/pkg/volume/csi/csi_test.go @@ -417,7 +417,7 @@ func TestCSI_VolumeAll(t *testing.T) { csiMounter.csiClient = csiClient var mounterArgs volume.MounterArgs mounterArgs.FsGroup = fsGroup - if err := csiMounter.SetUp(mounterArgs); err != nil { + if _, err := csiMounter.SetUp(mounterArgs); err != nil { t.Fatalf("csiTest.VolumeAll mounter.Setup(fsGroup) failed: %s", err) } t.Log("csiTest.VolumeAll mounter.Setup(fsGroup) done OK") diff --git a/pkg/volume/downwardapi/downwardapi.go b/pkg/volume/downwardapi/downwardapi.go index caee1fd4027..83c8b11fc3d 100644 --- a/pkg/volume/downwardapi/downwardapi.go +++ b/pkg/volume/downwardapi/downwardapi.go @@ -171,12 +171,8 @@ func (b *downwardAPIVolumeMounter) CanMount() error { // This function is not idempotent by design. We want the data to be refreshed periodically. // The internal sync interval of kubelet will drive the refresh of data. // TODO: Add volume specific ticker and refresh loop -func (b *downwardAPIVolumeMounter) SetUp(mounterArgs volume.MounterArgs) error { - return b.SetUpAt(b.GetPath(), mounterArgs) -} - -func (b *downwardAPIVolumeMounter) SetUpWithStatusTracking(mounterArgs volume.MounterArgs) (volumetypes.OperationStatus, error) { - err := b.SetUp(mounterArgs) +func (b *downwardAPIVolumeMounter) SetUp(mounterArgs volume.MounterArgs) (volumetypes.OperationStatus, error) { + err := b.SetUpAt(b.GetPath(), mounterArgs) return volumetypes.OperationFinished, err } diff --git a/pkg/volume/downwardapi/downwardapi_test.go b/pkg/volume/downwardapi/downwardapi_test.go index 647ebe3ff56..8584e812d5c 100644 --- a/pkg/volume/downwardapi/downwardapi_test.go +++ b/pkg/volume/downwardapi/downwardapi_test.go @@ -253,7 +253,7 @@ func newDownwardAPITest(t *testing.T, name string, volumeFiles, podLabels, podAn volumePath := mounter.GetPath() - err = mounter.SetUp(volume.MounterArgs{}) + _, err = mounter.SetUp(volume.MounterArgs{}) if err != nil { t.Errorf("Failed to setup volume: %v", err) } @@ -380,7 +380,7 @@ func (step reSetUp) run(test *downwardAPITest) { } // now re-run Setup - if err = test.mounter.SetUp(volume.MounterArgs{}); err != nil { + if _, err = test.mounter.SetUp(volume.MounterArgs{}); err != nil { test.t.Errorf("Failed to re-setup volume: %v", err) } diff --git a/pkg/volume/emptydir/empty_dir.go b/pkg/volume/emptydir/empty_dir.go index b5cc0803627..2e64bff6c4d 100644 --- a/pkg/volume/emptydir/empty_dir.go +++ b/pkg/volume/emptydir/empty_dir.go @@ -193,12 +193,8 @@ func (ed *emptyDir) CanMount() error { } // SetUp creates new directory. -func (ed *emptyDir) SetUp(mounterArgs volume.MounterArgs) error { - return ed.SetUpAt(ed.GetPath(), mounterArgs) -} - -func (ed *emptyDir) SetUpWithStatusTracking(mounterArgs volume.MounterArgs) (volumetypes.OperationStatus, error) { - err := ed.SetUp(mounterArgs) +func (ed *emptyDir) SetUp(mounterArgs volume.MounterArgs) (volumetypes.OperationStatus, error) { + err := ed.SetUpAt(ed.GetPath(), mounterArgs) return volumetypes.OperationFinished, err } diff --git a/pkg/volume/emptydir/empty_dir_test.go b/pkg/volume/emptydir/empty_dir_test.go index 892ec1ec342..4c4bcac53af 100644 --- a/pkg/volume/emptydir/empty_dir_test.go +++ b/pkg/volume/emptydir/empty_dir_test.go @@ -164,7 +164,7 @@ func doTestPlugin(t *testing.T, config pluginTestConfig) { t.Errorf("Got unexpected path: %s", volPath) } - if err := mounter.SetUp(volume.MounterArgs{}); err != nil { + if _, err := mounter.SetUp(volume.MounterArgs{}); err != nil { t.Errorf("Expected success, got: %v", err) } diff --git a/pkg/volume/fc/fc.go b/pkg/volume/fc/fc.go index b5940251460..259d1b8bbbe 100644 --- a/pkg/volume/fc/fc.go +++ b/pkg/volume/fc/fc.go @@ -370,12 +370,8 @@ func (b *fcDiskMounter) CanMount() error { return nil } -func (b *fcDiskMounter) SetUp(mounterArgs volume.MounterArgs) error { - return b.SetUpAt(b.GetPath(), mounterArgs) -} - -func (b *fcDiskMounter) SetUpWithStatusTracking(mounterArgs volume.MounterArgs) (volumetypes.OperationStatus, error) { - err := b.SetUp(mounterArgs) +func (b *fcDiskMounter) SetUp(mounterArgs volume.MounterArgs) (volumetypes.OperationStatus, error) { + err := b.SetUpAt(b.GetPath(), mounterArgs) return volumetypes.OperationFinished, err } diff --git a/pkg/volume/fc/fc_test.go b/pkg/volume/fc/fc_test.go index 63d5a08e09a..32dc1b51d9e 100644 --- a/pkg/volume/fc/fc_test.go +++ b/pkg/volume/fc/fc_test.go @@ -181,7 +181,7 @@ func doTestPlugin(t *testing.T, spec *volume.Spec) { t.Errorf("Unexpected path, expected %q, got: %q", expectedPath, path) } - if err := mounter.SetUp(volume.MounterArgs{}); err != nil { + if _, err := mounter.SetUp(volume.MounterArgs{}); err != nil { t.Errorf("Expected success, got: %v", err) } if _, err := os.Stat(path); err != nil { diff --git a/pkg/volume/flexvolume/mounter.go b/pkg/volume/flexvolume/mounter.go index 47def08da8e..084e7ff7701 100644 --- a/pkg/volume/flexvolume/mounter.go +++ b/pkg/volume/flexvolume/mounter.go @@ -40,13 +40,8 @@ var _ volume.Mounter = &flexVolumeMounter{} // Mounter interface // SetUp creates new directory. -func (f *flexVolumeMounter) SetUp(mounterArgs volume.MounterArgs) error { - return f.SetUpAt(f.GetPath(), mounterArgs) -} - -// SetUp creates new directory. -func (f *flexVolumeMounter) SetUpWithStatusTracking(mounterArgs volume.MounterArgs) (volumetypes.OperationStatus, error) { - err := f.SetUp(mounterArgs) +func (f *flexVolumeMounter) SetUp(mounterArgs volume.MounterArgs) (volumetypes.OperationStatus, error) { + err := f.SetUpAt(f.GetPath(), mounterArgs) return volumetypes.OperationFinished, err } diff --git a/pkg/volume/flocker/flocker.go b/pkg/volume/flocker/flocker.go index 8adb9186ce5..2c951e68709 100644 --- a/pkg/volume/flocker/flocker.go +++ b/pkg/volume/flocker/flocker.go @@ -232,12 +232,8 @@ func (b *flockerVolumeMounter) GetPath() string { } // SetUp bind mounts the disk global mount to the volume path. -func (b *flockerVolumeMounter) SetUp(mounterArgs volume.MounterArgs) error { - return b.SetUpAt(b.GetPath(), mounterArgs) -} - -func (b *flockerVolumeMounter) SetUpWithStatusTracking(mounterArgs volume.MounterArgs) (volumetypes.OperationStatus, error) { - err := b.SetUp(mounterArgs) +func (b *flockerVolumeMounter) SetUp(mounterArgs volume.MounterArgs) (volumetypes.OperationStatus, error) { + err := b.SetUpAt(b.GetPath(), mounterArgs) return volumetypes.OperationFinished, err } diff --git a/pkg/volume/gcepd/gce_pd.go b/pkg/volume/gcepd/gce_pd.go index 1d8409706e4..a43ca3868fa 100644 --- a/pkg/volume/gcepd/gce_pd.go +++ b/pkg/volume/gcepd/gce_pd.go @@ -369,13 +369,8 @@ func (b *gcePersistentDiskMounter) CanMount() error { } // SetUp bind mounts the disk global mount to the volume path. -func (b *gcePersistentDiskMounter) SetUp(mounterArgs volume.MounterArgs) error { - return b.SetUpAt(b.GetPath(), mounterArgs) -} - -// SetUp bind mounts the disk global mount to the volume path. -func (b *gcePersistentDiskMounter) SetUpWithStatusTracking(mounterArgs volume.MounterArgs) (volumetypes.OperationStatus, error) { - err := b.SetUp(mounterArgs) +func (b *gcePersistentDiskMounter) SetUp(mounterArgs volume.MounterArgs) (volumetypes.OperationStatus, error) { + err := b.SetUpAt(b.GetPath(), mounterArgs) return volumetypes.OperationFinished, err } diff --git a/pkg/volume/gcepd/gce_pd_test.go b/pkg/volume/gcepd/gce_pd_test.go index d45de1be877..6102930b1c5 100644 --- a/pkg/volume/gcepd/gce_pd_test.go +++ b/pkg/volume/gcepd/gce_pd_test.go @@ -144,7 +144,7 @@ func TestPlugin(t *testing.T) { t.Errorf("Got unexpected path: %s", path) } - if err := mounter.SetUp(volume.MounterArgs{}); err != nil { + if _, err := mounter.SetUp(volume.MounterArgs{}); err != nil { t.Errorf("Expected success, got: %v", err) } if _, err := os.Stat(path); err != nil { @@ -282,7 +282,7 @@ func TestMountOptions(t *testing.T) { t.Errorf("Got a nil Mounter") } - if err := mounter.SetUp(volume.MounterArgs{}); err != nil { + if _, err := mounter.SetUp(volume.MounterArgs{}); err != nil { t.Errorf("Expected success, got: %v", err) } mountOptions := fakeMounter.MountPoints[0].Opts diff --git a/pkg/volume/git_repo/git_repo.go b/pkg/volume/git_repo/git_repo.go index d982cae4d9b..aacc389194d 100644 --- a/pkg/volume/git_repo/git_repo.go +++ b/pkg/volume/git_repo/git_repo.go @@ -176,13 +176,8 @@ func (b *gitRepoVolumeMounter) CanMount() error { } // SetUp creates new directory and clones a git repo. -func (b *gitRepoVolumeMounter) SetUp(mounterArgs volume.MounterArgs) error { - return b.SetUpAt(b.GetPath(), mounterArgs) -} - -// SetUp creates new directory and clones a git repo. -func (b *gitRepoVolumeMounter) SetUpWithStatusTracking(mounterArgs volume.MounterArgs) (volumetypes.OperationStatus, error) { - err := b.SetUp(mounterArgs) +func (b *gitRepoVolumeMounter) SetUp(mounterArgs volume.MounterArgs) (volumetypes.OperationStatus, error) { + err := b.SetUpAt(b.GetPath(), mounterArgs) return volumetypes.OperationFinished, err } diff --git a/pkg/volume/glusterfs/glusterfs.go b/pkg/volume/glusterfs/glusterfs.go index c9164d08b93..c42edb94197 100644 --- a/pkg/volume/glusterfs/glusterfs.go +++ b/pkg/volume/glusterfs/glusterfs.go @@ -270,13 +270,8 @@ func (b *glusterfsMounter) CanMount() error { } // SetUp attaches the disk and bind mounts to the volume path. -func (b *glusterfsMounter) SetUp(mounterArgs volume.MounterArgs) error { - return b.SetUpAt(b.GetPath(), mounterArgs) -} - -// SetUp attaches the disk and bind mounts to the volume path. -func (b *glusterfsMounter) SetUpWithStatusTracking(mounterArgs volume.MounterArgs) (volumetypes.OperationStatus, error) { - err := b.SetUp(mounterArgs) +func (b *glusterfsMounter) SetUp(mounterArgs volume.MounterArgs) (volumetypes.OperationStatus, error) { + err := b.SetUpAt(b.GetPath(), mounterArgs) return volumetypes.OperationFinished, err } diff --git a/pkg/volume/glusterfs/glusterfs_test.go b/pkg/volume/glusterfs/glusterfs_test.go index 25e199d2ad9..1eba9fbc9a8 100644 --- a/pkg/volume/glusterfs/glusterfs_test.go +++ b/pkg/volume/glusterfs/glusterfs_test.go @@ -119,7 +119,7 @@ func doTestPlugin(t *testing.T, spec *volume.Spec) { if volumePath != expectedPath { t.Errorf("Unexpected path, expected %q, got: %q", expectedPath, volumePath) } - if err := mounter.SetUp(volume.MounterArgs{}); err != nil { + if _, err := mounter.SetUp(volume.MounterArgs{}); err != nil { t.Errorf("Expected success, got: %v", err) } if _, err := os.Stat(volumePath); err != nil { diff --git a/pkg/volume/hostpath/host_path.go b/pkg/volume/hostpath/host_path.go index 47c8601f542..649a0f07836 100644 --- a/pkg/volume/hostpath/host_path.go +++ b/pkg/volume/hostpath/host_path.go @@ -227,22 +227,20 @@ func (b *hostPathMounter) CanMount() error { } // SetUp does nothing. -func (b *hostPathMounter) SetUp(mounterArgs volume.MounterArgs) error { - err := validation.ValidatePathNoBacksteps(b.GetPath()) - if err != nil { - return fmt.Errorf("invalid HostPath `%s`: %v", b.GetPath(), err) - } +func (b *hostPathMounter) SetUp(mounterArgs volume.MounterArgs) (volumetypes.OperationStatus, error) { + internalSetup := func() error { + err := validation.ValidatePathNoBacksteps(b.GetPath()) + if err != nil { + return fmt.Errorf("invalid HostPath `%s`: %v", b.GetPath(), err) + } - if *b.pathType == v1.HostPathUnset { - return nil + if *b.pathType == v1.HostPathUnset { + return nil + } + return checkType(b.GetPath(), b.pathType, b.hu) } - return checkType(b.GetPath(), b.pathType, b.hu) -} + return volumetypes.OperationFinished, internalSetup() -// SetUpWithStatusTracking calls setup and returns additional information about operation state -func (b *hostPathMounter) SetUpWithStatusTracking(mounterArgs volume.MounterArgs) (volumetypes.OperationStatus, error) { - err := b.SetUp(mounterArgs) - return volumetypes.OperationFinished, err } // SetUpAt does not make sense for host paths - probably programmer error. diff --git a/pkg/volume/hostpath/host_path_test.go b/pkg/volume/hostpath/host_path_test.go index fa94755b1bc..2191d0d6148 100644 --- a/pkg/volume/hostpath/host_path_test.go +++ b/pkg/volume/hostpath/host_path_test.go @@ -219,7 +219,7 @@ func TestInvalidHostPath(t *testing.T) { t.Fatal(err) } - err = mounter.SetUp(volume.MounterArgs{}) + _, err = mounter.SetUp(volume.MounterArgs{}) expectedMsg := "invalid HostPath `/no/backsteps/allowed/..`: must not contain '..'" if err.Error() != expectedMsg { t.Fatalf("expected error `%s` but got `%s`", expectedMsg, err) @@ -255,7 +255,7 @@ func TestPlugin(t *testing.T) { t.Errorf("Got unexpected path: %s", path) } - if err := mounter.SetUp(volume.MounterArgs{}); err != nil { + if _, err := mounter.SetUp(volume.MounterArgs{}); err != nil { t.Errorf("Expected success, got: %v", err) } diff --git a/pkg/volume/iscsi/iscsi.go b/pkg/volume/iscsi/iscsi.go index 51fbdb20121..1e46cbba7b0 100644 --- a/pkg/volume/iscsi/iscsi.go +++ b/pkg/volume/iscsi/iscsi.go @@ -339,12 +339,8 @@ func (b *iscsiDiskMounter) CanMount() error { return nil } -func (b *iscsiDiskMounter) SetUp(mounterArgs volume.MounterArgs) error { - return b.SetUpAt(b.GetPath(), mounterArgs) -} - -func (b *iscsiDiskMounter) SetUpWithStatusTracking(mounterArgs volume.MounterArgs) (volumetypes.OperationStatus, error) { - err := b.SetUp(mounterArgs) +func (b *iscsiDiskMounter) SetUp(mounterArgs volume.MounterArgs) (volumetypes.OperationStatus, error) { + err := b.SetUpAt(b.GetPath(), mounterArgs) return volumetypes.OperationFinished, err } diff --git a/pkg/volume/iscsi/iscsi_test.go b/pkg/volume/iscsi/iscsi_test.go index a062067e123..924855f55c3 100644 --- a/pkg/volume/iscsi/iscsi_test.go +++ b/pkg/volume/iscsi/iscsi_test.go @@ -177,7 +177,7 @@ func doTestPlugin(t *testing.T, spec *volume.Spec) { t.Errorf("Unexpected path, expected %q, got: %q", expectedPath, path) } - if err := mounter.SetUp(volume.MounterArgs{}); err != nil { + if _, err := mounter.SetUp(volume.MounterArgs{}); err != nil { t.Errorf("Expected success, got: %v", err) } if _, err := os.Stat(path); err != nil { diff --git a/pkg/volume/local/local.go b/pkg/volume/local/local.go index 1aa6b78fde2..7f0a8e41f32 100644 --- a/pkg/volume/local/local.go +++ b/pkg/volume/local/local.go @@ -473,13 +473,8 @@ func (m *localVolumeMounter) CanMount() error { } // SetUp bind mounts the directory to the volume path -func (m *localVolumeMounter) SetUp(mounterArgs volume.MounterArgs) error { - return m.SetUpAt(m.GetPath(), mounterArgs) -} - -// SetUp bind mounts the directory to the volume path -func (m *localVolumeMounter) SetUpWithStatusTracking(mounterArgs volume.MounterArgs) (volumetypes.OperationStatus, error) { - err := m.SetUp(mounterArgs) +func (m *localVolumeMounter) SetUp(mounterArgs volume.MounterArgs) (volumetypes.OperationStatus, error) { + err := m.SetUpAt(m.GetPath(), mounterArgs) return volumetypes.OperationFinished, err } diff --git a/pkg/volume/local/local_test.go b/pkg/volume/local/local_test.go index bb96a4fac31..c48e3bbffe9 100644 --- a/pkg/volume/local/local_test.go +++ b/pkg/volume/local/local_test.go @@ -201,7 +201,7 @@ func TestInvalidLocalPath(t *testing.T) { t.Fatal(err) } - err = mounter.SetUp(volume.MounterArgs{}) + _, err = mounter.SetUp(volume.MounterArgs{}) expectedMsg := "invalid path: /no/backsteps/allowed/.. must not contain '..'" if err.Error() != expectedMsg { t.Fatalf("expected error `%s` but got `%s`", expectedMsg, err) @@ -308,7 +308,7 @@ func TestMountUnmount(t *testing.T) { t.Errorf("Got unexpected path: %s", path) } - if err := mounter.SetUp(volume.MounterArgs{}); err != nil { + if _, err := mounter.SetUp(volume.MounterArgs{}); err != nil { t.Errorf("Expected success, got: %v", err) } @@ -429,7 +429,7 @@ func testFSGroupMount(plug volume.VolumePlugin, pod *v1.Pod, tmpDir string, fsGr var mounterArgs volume.MounterArgs mounterArgs.FsGroup = &fsGroup - if err := mounter.SetUp(mounterArgs); err != nil { + if _, err := mounter.SetUp(mounterArgs); err != nil { return err } return nil @@ -587,7 +587,7 @@ func TestMountOptions(t *testing.T) { fakeMounter := mount.NewFakeMounter(nil) mounter.(*localVolumeMounter).mounter = fakeMounter - if err := mounter.SetUp(volume.MounterArgs{}); err != nil { + if _, err := mounter.SetUp(volume.MounterArgs{}); err != nil { t.Errorf("Expected success, got: %v", err) } mountOptions := fakeMounter.MountPoints[0].Opts diff --git a/pkg/volume/nfs/nfs.go b/pkg/volume/nfs/nfs.go index 5077f91a4d2..c02b256dd23 100644 --- a/pkg/volume/nfs/nfs.go +++ b/pkg/volume/nfs/nfs.go @@ -238,12 +238,8 @@ func (nfsMounter *nfsMounter) GetAttributes() volume.Attributes { } // SetUp attaches the disk and bind mounts to the volume path. -func (nfsMounter *nfsMounter) SetUp(mounterArgs volume.MounterArgs) error { - return nfsMounter.SetUpAt(nfsMounter.GetPath(), mounterArgs) -} - -func (nfsMounter *nfsMounter) SetUpWithStatusTracking(mounterArgs volume.MounterArgs) (volumetypes.OperationStatus, error) { - err := nfsMounter.SetUp(mounterArgs) +func (nfsMounter *nfsMounter) SetUp(mounterArgs volume.MounterArgs) (volumetypes.OperationStatus, error) { + err := nfsMounter.SetUpAt(nfsMounter.GetPath(), mounterArgs) return volumetypes.OperationFinished, err } diff --git a/pkg/volume/nfs/nfs_test.go b/pkg/volume/nfs/nfs_test.go index f2b0e519060..298c1ff5d1d 100644 --- a/pkg/volume/nfs/nfs_test.go +++ b/pkg/volume/nfs/nfs_test.go @@ -123,7 +123,7 @@ func doTestPlugin(t *testing.T, spec *volume.Spec) { if volumePath != expectedPath { t.Errorf("Unexpected path, expected %q, got: %q", expectedPath, volumePath) } - if err := mounter.SetUp(volume.MounterArgs{}); err != nil { + if _, err := mounter.SetUp(volume.MounterArgs{}); err != nil { t.Errorf("Expected success, got: %v", err) } if _, err := os.Stat(volumePath); err != nil { diff --git a/pkg/volume/portworx/portworx.go b/pkg/volume/portworx/portworx.go index 31e253340a4..87d34363faa 100644 --- a/pkg/volume/portworx/portworx.go +++ b/pkg/volume/portworx/portworx.go @@ -296,12 +296,8 @@ func (b *portworxVolumeMounter) CanMount() error { } // SetUp attaches the disk and bind mounts to the volume path. -func (b *portworxVolumeMounter) SetUp(mounterArgs volume.MounterArgs) error { - return b.SetUpAt(b.GetPath(), mounterArgs) -} - -func (b *portworxVolumeMounter) SetUpWithStatusTracking(mounterArgs volume.MounterArgs) (volumetypes.OperationStatus, error) { - err := b.SetUp(mounterArgs) +func (b *portworxVolumeMounter) SetUp(mounterArgs volume.MounterArgs) (volumetypes.OperationStatus, error) { + err := b.SetUpAt(b.GetPath(), mounterArgs) return volumetypes.OperationFinished, err } diff --git a/pkg/volume/portworx/portworx_test.go b/pkg/volume/portworx/portworx_test.go index 02492f6d99d..d70c036e3c5 100644 --- a/pkg/volume/portworx/portworx_test.go +++ b/pkg/volume/portworx/portworx_test.go @@ -164,7 +164,7 @@ func TestPlugin(t *testing.T) { t.Errorf("Got unexpected path: %s", path) } - if err := mounter.SetUp(volume.MounterArgs{}); err != nil { + if _, err := mounter.SetUp(volume.MounterArgs{}); err != nil { t.Errorf("Expected success, got: %v", err) } if _, err := os.Stat(path); err != nil { diff --git a/pkg/volume/projected/projected.go b/pkg/volume/projected/projected.go index 31b30eb0050..dcbc0f478bd 100644 --- a/pkg/volume/projected/projected.go +++ b/pkg/volume/projected/projected.go @@ -185,12 +185,8 @@ func (s *projectedVolumeMounter) CanMount() error { return nil } -func (s *projectedVolumeMounter) SetUp(mounterArgs volume.MounterArgs) error { - return s.SetUpAt(s.GetPath(), mounterArgs) -} - -func (s *projectedVolumeMounter) SetUpWithStatusTracking(mounterArgs volume.MounterArgs) (volumetypes.OperationStatus, error) { - err := s.SetUp(mounterArgs) +func (s *projectedVolumeMounter) SetUp(mounterArgs volume.MounterArgs) (volumetypes.OperationStatus, error) { + err := s.SetUpAt(s.GetPath(), mounterArgs) return volumetypes.OperationFinished, err } diff --git a/pkg/volume/projected/projected_test.go b/pkg/volume/projected/projected_test.go index ecf9b694073..bd882305df0 100644 --- a/pkg/volume/projected/projected_test.go +++ b/pkg/volume/projected/projected_test.go @@ -887,7 +887,7 @@ func TestPlugin(t *testing.T) { t.Errorf("Got unexpected path: %s", volumePath) } - err = mounter.SetUp(volume.MounterArgs{}) + _, err = mounter.SetUp(volume.MounterArgs{}) if err != nil { t.Errorf("Failed to setup volume: %v", err) } @@ -953,7 +953,7 @@ func TestInvalidPathProjected(t *testing.T) { } var mounterArgs volume.MounterArgs - err = mounter.SetUp(mounterArgs) + _, err = mounter.SetUp(mounterArgs) if err == nil { t.Errorf("Expected error while setting up secret") } @@ -1004,7 +1004,7 @@ func TestPluginReboot(t *testing.T) { t.Errorf("Got unexpected path: %s", volumePath) } - err = mounter.SetUp(volume.MounterArgs{}) + _, err = mounter.SetUp(volume.MounterArgs{}) if err != nil { t.Errorf("Failed to setup volume: %v", err) } @@ -1056,7 +1056,7 @@ func TestPluginOptional(t *testing.T) { t.Errorf("Got unexpected path: %s", volumePath) } - err = mounter.SetUp(volume.MounterArgs{}) + _, err = mounter.SetUp(volume.MounterArgs{}) if err != nil { t.Errorf("Failed to setup volume: %v", err) } @@ -1154,7 +1154,7 @@ func TestPluginOptionalKeys(t *testing.T) { t.Errorf("Got unexpected path: %s", volumePath) } - err = mounter.SetUp(volume.MounterArgs{}) + _, err = mounter.SetUp(volume.MounterArgs{}) if err != nil { t.Errorf("Failed to setup volume: %v", err) } diff --git a/pkg/volume/quobyte/quobyte.go b/pkg/volume/quobyte/quobyte.go index d488d238bac..19e6f2924c0 100644 --- a/pkg/volume/quobyte/quobyte.go +++ b/pkg/volume/quobyte/quobyte.go @@ -235,14 +235,9 @@ func (mounter *quobyteMounter) CanMount() error { } // SetUp attaches the disk and bind mounts to the volume path. -func (mounter *quobyteMounter) SetUp(mounterArgs volume.MounterArgs) error { +func (mounter *quobyteMounter) SetUp(mounterArgs volume.MounterArgs) (volumetypes.OperationStatus, error) { pluginDir := mounter.plugin.host.GetPluginDir(utilstrings.EscapeQualifiedName(quobytePluginName)) - return mounter.SetUpAt(pluginDir, mounterArgs) -} - -// SetUpWithStatusTracking attaches the disk and bind mounts to the volume path. -func (mounter *quobyteMounter) SetUpWithStatusTracking(mounterArgs volume.MounterArgs) (volumetypes.OperationStatus, error) { - err := mounter.SetUp(mounterArgs) + err := mounter.SetUpAt(pluginDir, mounterArgs) return volumetypes.OperationFinished, err } diff --git a/pkg/volume/quobyte/quobyte_test.go b/pkg/volume/quobyte/quobyte_test.go index eb3ef2bbba8..d24ca69a265 100644 --- a/pkg/volume/quobyte/quobyte_test.go +++ b/pkg/volume/quobyte/quobyte_test.go @@ -102,7 +102,7 @@ func doTestPlugin(t *testing.T, spec *volume.Spec) { if volumePath != fmt.Sprintf("%s/plugins/kubernetes.io~quobyte/root#root@vol", tmpDir) { t.Errorf("Got unexpected path: %s expected: %s", volumePath, fmt.Sprintf("%s/plugins/kubernetes.io~quobyte/root#root@vol", tmpDir)) } - if err := mounter.SetUp(volume.MounterArgs{}); err != nil { + if _, err := mounter.SetUp(volume.MounterArgs{}); err != nil { t.Errorf("Expected success, got: %v", err) } unmounter, err := plug.(*quobytePlugin).newUnmounterInternal("vol", types.UID("poduid"), mount.NewFakeMounter(nil)) diff --git a/pkg/volume/rbd/rbd.go b/pkg/volume/rbd/rbd.go index 57fdf0021e6..751909b69fe 100644 --- a/pkg/volume/rbd/rbd.go +++ b/pkg/volume/rbd/rbd.go @@ -837,12 +837,8 @@ func (b *rbdMounter) CanMount() error { return nil } -func (b *rbdMounter) SetUp(mounterArgs volume.MounterArgs) error { - return b.SetUpAt(b.GetPath(), mounterArgs) -} - -func (b *rbdMounter) SetUpWithStatusTracking(mounterArgs volume.MounterArgs) (volumetypes.OperationStatus, error) { - err := b.SetUp(mounterArgs) +func (b *rbdMounter) SetUp(mounterArgs volume.MounterArgs) (volumetypes.OperationStatus, error) { + err := b.SetUpAt(b.GetPath(), mounterArgs) return volumetypes.OperationFinished, err } diff --git a/pkg/volume/rbd/rbd_test.go b/pkg/volume/rbd/rbd_test.go index 311b854b6b1..fa07b6b5ba1 100644 --- a/pkg/volume/rbd/rbd_test.go +++ b/pkg/volume/rbd/rbd_test.go @@ -307,7 +307,7 @@ func doTestPlugin(t *testing.T, c *testcase) { t.Errorf("Unexpected path, expected %q, got: %q", c.expectedPodMountPath, path) } - if err := mounter.SetUp(volume.MounterArgs{}); err != nil { + if _, err := mounter.SetUp(volume.MounterArgs{}); err != nil { t.Errorf("Expected success, got: %v", err) } if _, err := os.Stat(path); err != nil { diff --git a/pkg/volume/scaleio/sio_volume.go b/pkg/volume/scaleio/sio_volume.go index 368dec3dd23..72c20aefa1d 100644 --- a/pkg/volume/scaleio/sio_volume.go +++ b/pkg/volume/scaleio/sio_volume.go @@ -79,12 +79,8 @@ func (v *sioVolume) CanMount() error { return nil } -func (v *sioVolume) SetUp(mounterArgs volume.MounterArgs) error { - return v.SetUpAt(v.GetPath(), mounterArgs) -} - -func (v *sioVolume) SetUpWithStatusTracking(mounterArgs volume.MounterArgs) (volumetypes.OperationStatus, error) { - err := v.SetUp(mounterArgs) +func (v *sioVolume) SetUp(mounterArgs volume.MounterArgs) (volumetypes.OperationStatus, error) { + err := v.SetUpAt(v.GetPath(), mounterArgs) return volumetypes.OperationFinished, err } diff --git a/pkg/volume/scaleio/sio_volume_test.go b/pkg/volume/scaleio/sio_volume_test.go index ca871817589..e3ac12440d5 100644 --- a/pkg/volume/scaleio/sio_volume_test.go +++ b/pkg/volume/scaleio/sio_volume_test.go @@ -190,7 +190,7 @@ func TestVolumeMounterUnmounter(t *testing.T) { t.Errorf("Got unexpected path: %s", path) } - if err := sioMounter.SetUp(volume.MounterArgs{}); err != nil { + if _, err := sioMounter.SetUp(volume.MounterArgs{}); err != nil { t.Errorf("Expected success, got: %v", err) } if _, err := os.Stat(path); err != nil { @@ -344,7 +344,7 @@ func TestVolumeProvisioner(t *testing.T) { t.Fatalf("failed to create sio mgr: %v", err) } sioVol.sioMgr.client = sio - if err := sioMounter.SetUp(volume.MounterArgs{}); err != nil { + if _, err := sioMounter.SetUp(volume.MounterArgs{}); err != nil { t.Fatalf("Expected success, got: %v", err) } diff --git a/pkg/volume/secret/secret.go b/pkg/volume/secret/secret.go index 31662addbf3..dbaab27995d 100644 --- a/pkg/volume/secret/secret.go +++ b/pkg/volume/secret/secret.go @@ -176,12 +176,8 @@ func (b *secretVolumeMounter) CanMount() error { return nil } -func (b *secretVolumeMounter) SetUp(mounterArgs volume.MounterArgs) error { - return b.SetUpAt(b.GetPath(), mounterArgs) -} - -func (b *secretVolumeMounter) SetUpWithStatusTracking(mounterArgs volume.MounterArgs) (volumetypes.OperationStatus, error) { - err := b.SetUp(mounterArgs) +func (b *secretVolumeMounter) SetUp(mounterArgs volume.MounterArgs) (volumetypes.OperationStatus, error) { + err := b.SetUpAt(b.GetPath(), mounterArgs) return volumetypes.OperationFinished, err } diff --git a/pkg/volume/secret/secret_test.go b/pkg/volume/secret/secret_test.go index 7d60d7b7d6c..fa7e74fc5ee 100644 --- a/pkg/volume/secret/secret_test.go +++ b/pkg/volume/secret/secret_test.go @@ -327,7 +327,7 @@ func TestPlugin(t *testing.T) { t.Errorf("Got unexpected path: %s", volumePath) } - err = mounter.SetUp(volume.MounterArgs{}) + _, err = mounter.SetUp(volume.MounterArgs{}) if err != nil { t.Errorf("Failed to setup volume: %v", err) } @@ -402,7 +402,7 @@ func TestInvalidPathSecret(t *testing.T) { } var mounterArgs volume.MounterArgs - err = mounter.SetUp(mounterArgs) + _, err = mounter.SetUp(mounterArgs) if err == nil { t.Errorf("Expected error while setting up secret") } @@ -453,7 +453,7 @@ func TestPluginReboot(t *testing.T) { t.Errorf("Got unexpected path: %s", volumePath) } - err = mounter.SetUp(volume.MounterArgs{}) + _, err = mounter.SetUp(volume.MounterArgs{}) if err != nil { t.Errorf("Failed to setup volume: %v", err) } @@ -505,7 +505,7 @@ func TestPluginOptional(t *testing.T) { t.Errorf("Got unexpected path: %s", volumePath) } - err = mounter.SetUp(volume.MounterArgs{}) + _, err = mounter.SetUp(volume.MounterArgs{}) if err != nil { t.Errorf("Failed to setup volume: %v", err) } @@ -603,7 +603,7 @@ func TestPluginOptionalKeys(t *testing.T) { t.Errorf("Got unexpected path: %s", volumePath) } - err = mounter.SetUp(volume.MounterArgs{}) + _, err = mounter.SetUp(volume.MounterArgs{}) if err != nil { t.Errorf("Failed to setup volume: %v", err) } diff --git a/pkg/volume/storageos/storageos.go b/pkg/volume/storageos/storageos.go index 3def2c36eed..27ac988868f 100644 --- a/pkg/volume/storageos/storageos.go +++ b/pkg/volume/storageos/storageos.go @@ -344,42 +344,40 @@ func (b *storageosMounter) CanMount() error { } // SetUp attaches the disk and bind mounts to the volume path. -func (b *storageosMounter) SetUp(mounterArgs volume.MounterArgs) error { - // Need a namespace to find the volume, try pod's namespace if not set. - if b.volNamespace == "" { - klog.V(2).Infof("Setting StorageOS volume namespace to pod namespace: %s", b.podNamespace) - b.volNamespace = b.podNamespace +func (b *storageosMounter) SetUp(mounterArgs volume.MounterArgs) (volumetypes.OperationStatus, error) { + internalSetup := func() error { + // Need a namespace to find the volume, try pod's namespace if not set. + if b.volNamespace == "" { + klog.V(2).Infof("Setting StorageOS volume namespace to pod namespace: %s", b.podNamespace) + b.volNamespace = b.podNamespace + } + + targetPath := makeGlobalPDName(b.plugin.host, b.pvName, b.volNamespace, b.volName) + + // Attach the device to the host. + if err := b.manager.AttachDevice(b, targetPath); err != nil { + klog.Errorf("Failed to attach device at %s: %s", targetPath, err.Error()) + return err + } + + // Attach the StorageOS volume as a block device + devicePath, err := b.manager.AttachVolume(b) + if err != nil { + klog.Errorf("Failed to attach StorageOS volume %s: %s", b.volName, err.Error()) + return err + } + + // Mount the loop device into the plugin's disk global mount dir. + err = b.manager.MountVolume(b, devicePath, targetPath) + if err != nil { + return err + } + klog.V(4).Infof("Successfully mounted StorageOS volume %s into global mount directory", b.volName) + + // Bind mount the volume into the pod + return b.SetUpAt(b.GetPath(), mounterArgs) } - - targetPath := makeGlobalPDName(b.plugin.host, b.pvName, b.volNamespace, b.volName) - - // Attach the device to the host. - if err := b.manager.AttachDevice(b, targetPath); err != nil { - klog.Errorf("Failed to attach device at %s: %s", targetPath, err.Error()) - return err - } - - // Attach the StorageOS volume as a block device - devicePath, err := b.manager.AttachVolume(b) - if err != nil { - klog.Errorf("Failed to attach StorageOS volume %s: %s", b.volName, err.Error()) - return err - } - - // Mount the loop device into the plugin's disk global mount dir. - err = b.manager.MountVolume(b, devicePath, targetPath) - if err != nil { - return err - } - klog.V(4).Infof("Successfully mounted StorageOS volume %s into global mount directory", b.volName) - - // Bind mount the volume into the pod - return b.SetUpAt(b.GetPath(), mounterArgs) -} - -func (b *storageosMounter) SetUpWithStatusTracking(mounterArgs volume.MounterArgs) (volumetypes.OperationStatus, error) { - err := b.SetUp(mounterArgs) - return volumetypes.OperationFinished, err + return volumetypes.OperationFinished, internalSetup() } // SetUp bind mounts the disk global mount to the give volume path. diff --git a/pkg/volume/storageos/storageos_test.go b/pkg/volume/storageos/storageos_test.go index d26f24af7c5..3758c9debcc 100644 --- a/pkg/volume/storageos/storageos_test.go +++ b/pkg/volume/storageos/storageos_test.go @@ -210,7 +210,7 @@ func TestPlugin(t *testing.T) { t.Errorf("Expected path: '%s' got: '%s'", expectedPath, volPath) } - if err := mounter.SetUp(volume.MounterArgs{}); err != nil { + if _, err := mounter.SetUp(volume.MounterArgs{}); err != nil { t.Errorf("Expected success, got: %v", err) } if _, err := os.Stat(volPath); err != nil { diff --git a/pkg/volume/testing/testing.go b/pkg/volume/testing/testing.go index ffc68b154a9..69c21256e65 100644 --- a/pkg/volume/testing/testing.go +++ b/pkg/volume/testing/testing.go @@ -843,18 +843,17 @@ func (fv *FakeVolume) CanMount() error { return nil } -func (fv *FakeVolume) SetUp(mounterArgs MounterArgs) error { - fv.Lock() - defer fv.Unlock() - if fv.VolName == TimeoutOnSetupVolumeName { - return volumetypes.NewOperationTimedOutError("time out on setup") +func (fv *FakeVolume) SetUp(mounterArgs MounterArgs) (volumetypes.OperationStatus, error) { + internalSetup := func() error { + fv.Lock() + defer fv.Unlock() + if fv.VolName == TimeoutOnSetupVolumeName { + return volumetypes.NewOperationTimedOutError("time out on setup") + } + fv.SetUpCallCount++ + return fv.SetUpAt(fv.getPath(), mounterArgs) } - fv.SetUpCallCount++ - return fv.SetUpAt(fv.getPath(), mounterArgs) -} - -func (fv *FakeVolume) SetUpWithStatusTracking(mounterArgs MounterArgs) (volumetypes.OperationStatus, error) { - err := fv.SetUp(mounterArgs) + err := internalSetup() if volumetypes.IsOperationTimeOutError(err) { return volumetypes.OperationInProgress, err } diff --git a/pkg/volume/util/operationexecutor/operation_generator.go b/pkg/volume/util/operationexecutor/operation_generator.go index 932d2ba5104..8b080560127 100644 --- a/pkg/volume/util/operationexecutor/operation_generator.go +++ b/pkg/volume/util/operationexecutor/operation_generator.go @@ -629,7 +629,7 @@ func (og *operationGenerator) GenerateMountVolumeFunc( } // Execute mount - opExitStatus, mountErr := volumeMounter.SetUpWithStatusTracking(volume.MounterArgs{ + opExitStatus, mountErr := volumeMounter.SetUp(volume.MounterArgs{ FsGroup: fsGroup, DesiredSize: volumeToMount.DesiredSizeLimit, }) diff --git a/pkg/volume/volume.go b/pkg/volume/volume.go index d6ffe91df2e..c751f9c959b 100644 --- a/pkg/volume/volume.go +++ b/pkg/volume/volume.go @@ -129,11 +129,7 @@ type Mounter interface { // content should be owned by 'fsGroup' so that it can be // accessed by the pod. This may be called more than once, so // implementations must be idempotent. - SetUp(mounterArgs MounterArgs) error - - // SetupWithStatusTracking is similar to SetUp function except it - // also return operation status as a return value - SetUpWithStatusTracking(mounterArgs MounterArgs) (volumetypes.OperationStatus, error) + SetUp(mounterArgs MounterArgs) (volumetypes.OperationStatus, error) // SetUpAt prepares and mounts/unpacks the volume to the // specified directory path, which may or may not exist yet. diff --git a/pkg/volume/vsphere_volume/vsphere_volume.go b/pkg/volume/vsphere_volume/vsphere_volume.go index a521f1d7bf5..2d68ee6136f 100644 --- a/pkg/volume/vsphere_volume/vsphere_volume.go +++ b/pkg/volume/vsphere_volume/vsphere_volume.go @@ -213,13 +213,8 @@ func (b *vsphereVolumeMounter) GetAttributes() volume.Attributes { } // SetUp attaches the disk and bind mounts to the volume path. -func (b *vsphereVolumeMounter) SetUp(mounterArgs volume.MounterArgs) error { - return b.SetUpAt(b.GetPath(), mounterArgs) -} - -// SetUp attaches the disk and bind mounts to the volume path. -func (b *vsphereVolumeMounter) SetUpWithStatusTracking(mounterArgs volume.MounterArgs) (volumetypes.OperationStatus, error) { - err := b.SetUp(mounterArgs) +func (b *vsphereVolumeMounter) SetUp(mounterArgs volume.MounterArgs) (volumetypes.OperationStatus, error) { + err := b.SetUpAt(b.GetPath(), mounterArgs) return volumetypes.OperationFinished, err } diff --git a/pkg/volume/vsphere_volume/vsphere_volume_test.go b/pkg/volume/vsphere_volume/vsphere_volume_test.go index ae59c319813..ac282ea3cf6 100644 --- a/pkg/volume/vsphere_volume/vsphere_volume_test.go +++ b/pkg/volume/vsphere_volume/vsphere_volume_test.go @@ -126,7 +126,7 @@ func TestPlugin(t *testing.T) { t.Errorf("Got unexpected path: %s", path) } - if err := mounter.SetUp(volume.MounterArgs{}); err != nil { + if _, err := mounter.SetUp(volume.MounterArgs{}); err != nil { t.Errorf("Expected success, got: %v", err) }