From 4b8e552a8882b205c5365b7ea0b042f969579a4e Mon Sep 17 00:00:00 2001 From: Hemant Kumar Date: Thu, 14 Nov 2019 18:55:46 -0500 Subject: [PATCH] Use typed errors for special casing volume progress Use typed errors rather than operation status for indicating operation progress --- hack/.staticcheck_failures | 1 + .../volume/attachdetach/testing/BUILD | 1 - .../attachdetach/testing/testvolumespec.go | 7 +- pkg/kubelet/BUILD | 1 - pkg/kubelet/kubelet_volumes_test.go | 5 +- pkg/volume/BUILD | 1 - pkg/volume/awsebs/BUILD | 1 - pkg/volume/awsebs/attacher.go | 13 ++- pkg/volume/awsebs/aws_ebs.go | 6 +- pkg/volume/awsebs/aws_ebs_test.go | 4 +- pkg/volume/azure_dd/BUILD | 1 - pkg/volume/azure_dd/attacher.go | 7 +- pkg/volume/azure_dd/azure_mounter.go | 6 +- pkg/volume/azure_file/BUILD | 1 - pkg/volume/azure_file/azure_file.go | 6 +- pkg/volume/azure_file/azure_file_test.go | 2 +- pkg/volume/cephfs/BUILD | 1 - pkg/volume/cephfs/cephfs.go | 6 +- pkg/volume/cephfs/cephfs_test.go | 2 +- pkg/volume/cinder/BUILD | 1 - pkg/volume/cinder/attacher.go | 8 +- pkg/volume/cinder/cinder.go | 6 +- pkg/volume/cinder/cinder_test.go | 2 +- pkg/volume/configmap/BUILD | 1 - pkg/volume/configmap/configmap.go | 6 +- pkg/volume/configmap/configmap_test.go | 10 +-- pkg/volume/csi/csi_attacher.go | 39 ++++----- pkg/volume/csi/csi_attacher_test.go | 24 +++--- pkg/volume/csi/csi_client.go | 4 +- pkg/volume/csi/csi_client_test.go | 14 +--- pkg/volume/csi/csi_mounter.go | 68 ++++++--------- pkg/volume/csi/csi_mounter_test.go | 26 +++--- pkg/volume/csi/csi_test.go | 4 +- pkg/volume/downwardapi/BUILD | 1 - pkg/volume/downwardapi/downwardapi.go | 6 +- pkg/volume/downwardapi/downwardapi_test.go | 4 +- pkg/volume/emptydir/BUILD | 1 - pkg/volume/emptydir/empty_dir.go | 6 +- pkg/volume/emptydir/empty_dir_test.go | 2 +- pkg/volume/fc/BUILD | 1 - pkg/volume/fc/attacher.go | 60 +++++++------- pkg/volume/fc/fc.go | 6 +- pkg/volume/fc/fc_test.go | 2 +- pkg/volume/flexvolume/BUILD | 1 - pkg/volume/flexvolume/attacher.go | 50 +++++------ pkg/volume/flexvolume/mounter.go | 6 +- pkg/volume/flocker/BUILD | 1 - pkg/volume/flocker/flocker.go | 6 +- pkg/volume/gcepd/BUILD | 1 - pkg/volume/gcepd/attacher.go | 8 +- pkg/volume/gcepd/gce_pd.go | 6 +- pkg/volume/gcepd/gce_pd_test.go | 4 +- pkg/volume/git_repo/BUILD | 1 - pkg/volume/git_repo/git_repo.go | 6 +- pkg/volume/glusterfs/BUILD | 1 - pkg/volume/glusterfs/glusterfs.go | 6 +- pkg/volume/glusterfs/glusterfs_test.go | 2 +- pkg/volume/hostpath/BUILD | 1 - pkg/volume/hostpath/host_path.go | 21 ++--- pkg/volume/hostpath/host_path_test.go | 4 +- pkg/volume/iscsi/BUILD | 1 - pkg/volume/iscsi/attacher.go | 9 +- pkg/volume/iscsi/iscsi.go | 6 +- pkg/volume/iscsi/iscsi_test.go | 2 +- pkg/volume/local/BUILD | 1 - pkg/volume/local/local.go | 47 +++++------ pkg/volume/local/local_test.go | 12 +-- pkg/volume/nfs/BUILD | 1 - pkg/volume/nfs/nfs.go | 6 +- pkg/volume/nfs/nfs_test.go | 2 +- pkg/volume/portworx/BUILD | 1 - pkg/volume/portworx/portworx.go | 6 +- pkg/volume/portworx/portworx_test.go | 2 +- pkg/volume/projected/BUILD | 1 - pkg/volume/projected/projected.go | 6 +- pkg/volume/projected/projected_test.go | 10 +-- pkg/volume/quobyte/BUILD | 1 - pkg/volume/quobyte/quobyte.go | 6 +- pkg/volume/quobyte/quobyte_test.go | 2 +- pkg/volume/rbd/BUILD | 1 - pkg/volume/rbd/attacher.go | 14 +--- pkg/volume/rbd/rbd.go | 6 +- pkg/volume/rbd/rbd_test.go | 4 +- pkg/volume/scaleio/BUILD | 1 - pkg/volume/scaleio/sio_volume.go | 6 +- pkg/volume/scaleio/sio_volume_test.go | 4 +- pkg/volume/secret/BUILD | 1 - pkg/volume/secret/secret.go | 6 +- pkg/volume/secret/secret_test.go | 10 +-- pkg/volume/storageos/BUILD | 1 - pkg/volume/storageos/storageos.go | 64 +++++++------- pkg/volume/storageos/storageos_test.go | 2 +- pkg/volume/testing/testing.go | 69 +++++++-------- .../operationexecutor/operation_generator.go | 83 +++++++++---------- pkg/volume/util/types/types.go | 68 ++++++++------- pkg/volume/volume.go | 15 +++- pkg/volume/vsphere_volume/BUILD | 1 - pkg/volume/vsphere_volume/attacher.go | 10 +-- pkg/volume/vsphere_volume/vsphere_volume.go | 7 +- .../vsphere_volume/vsphere_volume_test.go | 2 +- 100 files changed, 418 insertions(+), 578 deletions(-) diff --git a/hack/.staticcheck_failures b/hack/.staticcheck_failures index 7931c65fae6..a90443e253d 100644 --- a/hack/.staticcheck_failures +++ b/hack/.staticcheck_failures @@ -53,6 +53,7 @@ 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/BUILD b/pkg/controller/volume/attachdetach/testing/BUILD index 03a639344d3..e063b6b5fe2 100644 --- a/pkg/controller/volume/attachdetach/testing/BUILD +++ b/pkg/controller/volume/attachdetach/testing/BUILD @@ -12,7 +12,6 @@ go_library( deps = [ "//pkg/volume:go_default_library", "//pkg/volume/util:go_default_library", - "//pkg/volume/util/types:go_default_library", "//staging/src/k8s.io/api/core/v1:go_default_library", "//staging/src/k8s.io/api/storage/v1:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library", diff --git a/pkg/controller/volume/attachdetach/testing/testvolumespec.go b/pkg/controller/volume/attachdetach/testing/testvolumespec.go index eac942b0895..86b66f8f244 100644 --- a/pkg/controller/volume/attachdetach/testing/testvolumespec.go +++ b/pkg/controller/volume/attachdetach/testing/testvolumespec.go @@ -32,7 +32,6 @@ import ( "k8s.io/klog" "k8s.io/kubernetes/pkg/volume" "k8s.io/kubernetes/pkg/volume/util" - volumetypes "k8s.io/kubernetes/pkg/volume/util/types" ) const TestPluginName = "kubernetes.io/testPlugin" @@ -435,15 +434,15 @@ func (attacher *testPluginAttacher) GetDeviceMountPath(spec *volume.Spec) (strin return "", nil } -func (attacher *testPluginAttacher) MountDevice(spec *volume.Spec, devicePath string, deviceMountPath string) (volumetypes.OperationStatus, error) { +func (attacher *testPluginAttacher) MountDevice(spec *volume.Spec, devicePath string, deviceMountPath string) error { attacher.pluginLock.Lock() defer attacher.pluginLock.Unlock() if spec == nil { *attacher.ErrorEncountered = true klog.Errorf("MountDevice called with nil volume spec") - return volumetypes.OperationFinished, fmt.Errorf("MountDevice called with nil volume spec") + return fmt.Errorf("MountDevice called with nil volume spec") } - return volumetypes.OperationFinished, nil + return nil } // Detacher diff --git a/pkg/kubelet/BUILD b/pkg/kubelet/BUILD index f6681956119..f942cf4c2aa 100644 --- a/pkg/kubelet/BUILD +++ b/pkg/kubelet/BUILD @@ -224,7 +224,6 @@ go_test( "//pkg/volume/util:go_default_library", "//pkg/volume/util/hostutil:go_default_library", "//pkg/volume/util/subpath:go_default_library", - "//pkg/volume/util/types:go_default_library", "//staging/src/k8s.io/api/core/v1:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/api/equality:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/api/errors:go_default_library", diff --git a/pkg/kubelet/kubelet_volumes_test.go b/pkg/kubelet/kubelet_volumes_test.go index 85e86ca55c6..bfc5c8b7c22 100644 --- a/pkg/kubelet/kubelet_volumes_test.go +++ b/pkg/kubelet/kubelet_volumes_test.go @@ -29,7 +29,6 @@ import ( "k8s.io/kubernetes/pkg/volume" volumetest "k8s.io/kubernetes/pkg/volume/testing" "k8s.io/kubernetes/pkg/volume/util" - volumetypes "k8s.io/kubernetes/pkg/volume/util/types" ) func TestListVolumesForPod(t *testing.T) { @@ -531,8 +530,8 @@ func (f *stubVolume) CanMount() error { return nil } -func (f *stubVolume) SetUp(mounterArgs volume.MounterArgs) (volumetypes.OperationStatus, error) { - return volumetypes.OperationFinished, nil +func (f *stubVolume) SetUp(mounterArgs volume.MounterArgs) error { + return nil } func (f *stubVolume) SetUpAt(dir string, mounterArgs volume.MounterArgs) error { diff --git a/pkg/volume/BUILD b/pkg/volume/BUILD index 88919d029a4..8e608ecc467 100644 --- a/pkg/volume/BUILD +++ b/pkg/volume/BUILD @@ -23,7 +23,6 @@ go_library( "//pkg/volume/util/hostutil:go_default_library", "//pkg/volume/util/recyclerclient:go_default_library", "//pkg/volume/util/subpath:go_default_library", - "//pkg/volume/util/types:go_default_library", "//staging/src/k8s.io/api/authentication/v1:go_default_library", "//staging/src/k8s.io/api/core/v1:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/api/resource:go_default_library", diff --git a/pkg/volume/awsebs/BUILD b/pkg/volume/awsebs/BUILD index a9dfc8b4361..0a64e9943ea 100644 --- a/pkg/volume/awsebs/BUILD +++ b/pkg/volume/awsebs/BUILD @@ -23,7 +23,6 @@ go_library( "//pkg/features:go_default_library", "//pkg/volume:go_default_library", "//pkg/volume/util:go_default_library", - "//pkg/volume/util/types:go_default_library", "//pkg/volume/util/volumepathhandler:go_default_library", "//staging/src/k8s.io/api/core/v1:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/api/resource:go_default_library", diff --git a/pkg/volume/awsebs/attacher.go b/pkg/volume/awsebs/attacher.go index a13e5738faa..35bf55df8fe 100644 --- a/pkg/volume/awsebs/attacher.go +++ b/pkg/volume/awsebs/attacher.go @@ -34,7 +34,6 @@ import ( "k8s.io/apimachinery/pkg/types" "k8s.io/kubernetes/pkg/volume" volumeutil "k8s.io/kubernetes/pkg/volume/util" - volumetypes "k8s.io/kubernetes/pkg/volume/util/types" "k8s.io/legacy-cloud-providers/aws" ) @@ -207,7 +206,7 @@ func (attacher *awsElasticBlockStoreAttacher) GetDeviceMountPath( } // FIXME: this method can be further pruned. -func (attacher *awsElasticBlockStoreAttacher) MountDevice(spec *volume.Spec, devicePath string, deviceMountPath string) (volumetypes.OperationStatus, error) { +func (attacher *awsElasticBlockStoreAttacher) MountDevice(spec *volume.Spec, devicePath string, deviceMountPath string) error { mounter := attacher.host.GetMounter(awsElasticBlockStorePluginName) notMnt, err := mounter.IsLikelyNotMountPoint(deviceMountPath) if err != nil { @@ -222,17 +221,17 @@ func (attacher *awsElasticBlockStoreAttacher) MountDevice(spec *volume.Spec, dev dir = filepath.Dir(deviceMountPath) } if err := os.MkdirAll(dir, 0750); err != nil { - return volumetypes.OperationFinished, fmt.Errorf("making dir %s failed with %s", dir, err) + return fmt.Errorf("making dir %s failed with %s", dir, err) } notMnt = true } else { - return volumetypes.OperationFinished, err + return err } } volumeSource, readOnly, err := getVolumeSource(spec) if err != nil { - return volumetypes.OperationFinished, err + return err } options := []string{} @@ -245,10 +244,10 @@ func (attacher *awsElasticBlockStoreAttacher) MountDevice(spec *volume.Spec, dev err = diskMounter.FormatAndMount(devicePath, deviceMountPath, volumeSource.FSType, mountOptions) if err != nil { os.Remove(deviceMountPath) - return volumetypes.OperationFinished, err + return err } } - return volumetypes.OperationFinished, nil + return nil } type awsElasticBlockStoreDetacher struct { diff --git a/pkg/volume/awsebs/aws_ebs.go b/pkg/volume/awsebs/aws_ebs.go index 11bbd2d17f0..7bae0358e34 100644 --- a/pkg/volume/awsebs/aws_ebs.go +++ b/pkg/volume/awsebs/aws_ebs.go @@ -39,7 +39,6 @@ import ( "k8s.io/kubernetes/pkg/features" "k8s.io/kubernetes/pkg/volume" "k8s.io/kubernetes/pkg/volume/util" - volumetypes "k8s.io/kubernetes/pkg/volume/util/types" "k8s.io/legacy-cloud-providers/aws" utilstrings "k8s.io/utils/strings" ) @@ -366,9 +365,8 @@ func (b *awsElasticBlockStoreMounter) CanMount() error { } // SetUp attaches the disk and bind mounts to the volume path. -func (b *awsElasticBlockStoreMounter) SetUp(mounterArgs volume.MounterArgs) (volumetypes.OperationStatus, error) { - err := b.SetUpAt(b.GetPath(), mounterArgs) - return volumetypes.OperationFinished, err +func (b *awsElasticBlockStoreMounter) SetUp(mounterArgs volume.MounterArgs) error { + return b.SetUpAt(b.GetPath(), mounterArgs) } // SetUpAt attaches the disk and bind mounts to the volume path. diff --git a/pkg/volume/awsebs/aws_ebs_test.go b/pkg/volume/awsebs/aws_ebs_test.go index db0fcca91cb..021ecc46dc5 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/BUILD b/pkg/volume/azure_dd/BUILD index b2839f87927..9e607d94ef0 100644 --- a/pkg/volume/azure_dd/BUILD +++ b/pkg/volume/azure_dd/BUILD @@ -27,7 +27,6 @@ go_library( "//pkg/features:go_default_library", "//pkg/volume:go_default_library", "//pkg/volume/util:go_default_library", - "//pkg/volume/util/types:go_default_library", "//pkg/volume/util/volumepathhandler:go_default_library", "//staging/src/k8s.io/api/core/v1:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/api/resource:go_default_library", diff --git a/pkg/volume/azure_dd/attacher.go b/pkg/volume/azure_dd/attacher.go index 290ca67e122..42296172f0d 100644 --- a/pkg/volume/azure_dd/attacher.go +++ b/pkg/volume/azure_dd/attacher.go @@ -37,7 +37,6 @@ import ( cloudprovider "k8s.io/cloud-provider" "k8s.io/kubernetes/pkg/volume" "k8s.io/kubernetes/pkg/volume/util" - volumetypes "k8s.io/kubernetes/pkg/volume/util/types" "k8s.io/legacy-cloud-providers/azure" ) @@ -199,11 +198,7 @@ func (a *azureDiskAttacher) GetDeviceMountPath(spec *volume.Spec) (string, error return makeGlobalPDPath(a.plugin.host, volumeSource.DataDiskURI, isManagedDisk) } -func (attacher *azureDiskAttacher) MountDevice(spec *volume.Spec, devicePath string, deviceMountPath string) (volumetypes.OperationStatus, error) { - return volumetypes.OperationFinished, attacher.mountDeviceInternal(spec, devicePath, deviceMountPath) -} - -func (attacher *azureDiskAttacher) mountDeviceInternal(spec *volume.Spec, devicePath string, deviceMountPath string) error { +func (attacher *azureDiskAttacher) MountDevice(spec *volume.Spec, devicePath string, deviceMountPath string) error { mounter := attacher.plugin.host.GetMounter(azureDataDiskPluginName) notMnt, err := mounter.IsLikelyNotMountPoint(deviceMountPath) diff --git a/pkg/volume/azure_dd/azure_mounter.go b/pkg/volume/azure_dd/azure_mounter.go index f641e78e789..2f8d38bd0aa 100644 --- a/pkg/volume/azure_dd/azure_mounter.go +++ b/pkg/volume/azure_dd/azure_mounter.go @@ -29,7 +29,6 @@ import ( v1 "k8s.io/api/core/v1" "k8s.io/kubernetes/pkg/volume" "k8s.io/kubernetes/pkg/volume/util" - volumetypes "k8s.io/kubernetes/pkg/volume/util/types" ) type azureDiskMounter struct { @@ -66,9 +65,8 @@ func (m *azureDiskMounter) CanMount() error { return nil } -func (m *azureDiskMounter) SetUp(mounterArgs volume.MounterArgs) (volumetypes.OperationStatus, error) { - err := m.SetUpAt(m.GetPath(), mounterArgs) - return volumetypes.OperationFinished, err +func (m *azureDiskMounter) SetUp(mounterArgs volume.MounterArgs) error { + return m.SetUpAt(m.GetPath(), mounterArgs) } func (m *azureDiskMounter) GetPath() string { diff --git a/pkg/volume/azure_file/BUILD b/pkg/volume/azure_file/BUILD index f425722d0b9..1c4640f78a0 100644 --- a/pkg/volume/azure_file/BUILD +++ b/pkg/volume/azure_file/BUILD @@ -13,7 +13,6 @@ go_library( deps = [ "//pkg/volume:go_default_library", "//pkg/volume/util:go_default_library", - "//pkg/volume/util/types:go_default_library", "//staging/src/k8s.io/api/core/v1:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/api/errors:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/api/resource:go_default_library", diff --git a/pkg/volume/azure_file/azure_file.go b/pkg/volume/azure_file/azure_file.go index 427e88e1043..cd1a13ba369 100644 --- a/pkg/volume/azure_file/azure_file.go +++ b/pkg/volume/azure_file/azure_file.go @@ -36,7 +36,6 @@ import ( volumehelpers "k8s.io/cloud-provider/volume/helpers" "k8s.io/kubernetes/pkg/volume" volutil "k8s.io/kubernetes/pkg/volume/util" - volumetypes "k8s.io/kubernetes/pkg/volume/util/types" "k8s.io/legacy-cloud-providers/azure" ) @@ -236,9 +235,8 @@ func (b *azureFileMounter) CanMount() error { } // SetUp attaches the disk and bind mounts to the volume path. -func (b *azureFileMounter) SetUp(mounterArgs volume.MounterArgs) (volumetypes.OperationStatus, error) { - err := b.SetUpAt(b.GetPath(), mounterArgs) - return volumetypes.OperationFinished, err +func (b *azureFileMounter) SetUp(mounterArgs volume.MounterArgs) error { + return b.SetUpAt(b.GetPath(), mounterArgs) } func (b *azureFileMounter) SetUpAt(dir string, mounterArgs volume.MounterArgs) error { diff --git a/pkg/volume/azure_file/azure_file_test.go b/pkg/volume/azure_file/azure_file_test.go index d3407b8959a..6d42eb9ad8c 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/BUILD b/pkg/volume/cephfs/BUILD index f3ed56c1c85..b521e4b48c9 100644 --- a/pkg/volume/cephfs/BUILD +++ b/pkg/volume/cephfs/BUILD @@ -16,7 +16,6 @@ go_library( deps = [ "//pkg/volume:go_default_library", "//pkg/volume/util:go_default_library", - "//pkg/volume/util/types:go_default_library", "//staging/src/k8s.io/api/core/v1:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/types:go_default_library", diff --git a/pkg/volume/cephfs/cephfs.go b/pkg/volume/cephfs/cephfs.go index f9cd9be0d49..4ba29214fd9 100644 --- a/pkg/volume/cephfs/cephfs.go +++ b/pkg/volume/cephfs/cephfs.go @@ -33,7 +33,6 @@ import ( "k8s.io/apimachinery/pkg/types" "k8s.io/kubernetes/pkg/volume" "k8s.io/kubernetes/pkg/volume/util" - volumetypes "k8s.io/kubernetes/pkg/volume/util/types" ) // ProbeVolumePlugins is the primary entrypoint for volume plugins. @@ -220,9 +219,8 @@ func (cephfsVolume *cephfsMounter) CanMount() error { } // SetUp attaches the disk and bind mounts to the volume path. -func (cephfsVolume *cephfsMounter) SetUp(mounterArgs volume.MounterArgs) (volumetypes.OperationStatus, error) { - err := cephfsVolume.SetUpAt(cephfsVolume.GetPath(), mounterArgs) - return volumetypes.OperationFinished, err +func (cephfsVolume *cephfsMounter) SetUp(mounterArgs volume.MounterArgs) error { + return cephfsVolume.SetUpAt(cephfsVolume.GetPath(), mounterArgs) } // SetUpAt attaches the disk and bind mounts to the volume path. diff --git a/pkg/volume/cephfs/cephfs_test.go b/pkg/volume/cephfs/cephfs_test.go index ea674d80ff8..abe498c5f10 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/BUILD b/pkg/volume/cinder/BUILD index 06278d9ddeb..6d8674c91d3 100644 --- a/pkg/volume/cinder/BUILD +++ b/pkg/volume/cinder/BUILD @@ -20,7 +20,6 @@ go_library( "//pkg/features:go_default_library", "//pkg/volume:go_default_library", "//pkg/volume/util:go_default_library", - "//pkg/volume/util/types:go_default_library", "//pkg/volume/util/volumepathhandler:go_default_library", "//staging/src/k8s.io/api/core/v1:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/api/resource:go_default_library", diff --git a/pkg/volume/cinder/attacher.go b/pkg/volume/cinder/attacher.go index 21af74720b0..ebf84d31d0f 100644 --- a/pkg/volume/cinder/attacher.go +++ b/pkg/volume/cinder/attacher.go @@ -34,7 +34,6 @@ import ( "k8s.io/kubernetes/pkg/volume" volumeutil "k8s.io/kubernetes/pkg/volume/util" - volumetypes "k8s.io/kubernetes/pkg/volume/util/types" ) type cinderDiskAttacher struct { @@ -269,7 +268,7 @@ func (attacher *cinderDiskAttacher) GetDeviceMountPath( } // FIXME: this method can be further pruned. -func (attacher *cinderDiskAttacher) mountDeviceInternal(spec *volume.Spec, devicePath string, deviceMountPath string) error { +func (attacher *cinderDiskAttacher) MountDevice(spec *volume.Spec, devicePath string, deviceMountPath string) error { mounter := attacher.host.GetMounter(cinderVolumePluginName) notMnt, err := mounter.IsLikelyNotMountPoint(deviceMountPath) if err != nil { @@ -304,11 +303,6 @@ func (attacher *cinderDiskAttacher) mountDeviceInternal(spec *volume.Spec, devic return nil } -func (attacher *cinderDiskAttacher) MountDevice(spec *volume.Spec, devicePath string, deviceMountPath string) (volumetypes.OperationStatus, error) { - err := attacher.mountDeviceInternal(spec, devicePath, deviceMountPath) - return volumetypes.OperationFinished, err -} - type cinderDiskDetacher struct { mounter mount.Interface cinderProvider BlockStorageProvider diff --git a/pkg/volume/cinder/cinder.go b/pkg/volume/cinder/cinder.go index d694775f35a..d09baeb0caa 100644 --- a/pkg/volume/cinder/cinder.go +++ b/pkg/volume/cinder/cinder.go @@ -39,7 +39,6 @@ import ( "k8s.io/kubernetes/pkg/features" "k8s.io/kubernetes/pkg/volume" "k8s.io/kubernetes/pkg/volume/util" - volumetypes "k8s.io/kubernetes/pkg/volume/util/types" "k8s.io/legacy-cloud-providers/openstack" ) @@ -390,9 +389,8 @@ func (b *cinderVolumeMounter) CanMount() error { return nil } -func (b *cinderVolumeMounter) SetUp(mounterArgs volume.MounterArgs) (volumetypes.OperationStatus, error) { - err := b.SetUpAt(b.GetPath(), mounterArgs) - return volumetypes.OperationFinished, err +func (b *cinderVolumeMounter) SetUp(mounterArgs volume.MounterArgs) error { + return b.SetUpAt(b.GetPath(), mounterArgs) } // SetUp bind mounts to the volume path. diff --git a/pkg/volume/cinder/cinder_test.go b/pkg/volume/cinder/cinder_test.go index 5a151f5fc33..bd0d0c6b929 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/BUILD b/pkg/volume/configmap/BUILD index 3300373c646..ab7d2571142 100644 --- a/pkg/volume/configmap/BUILD +++ b/pkg/volume/configmap/BUILD @@ -16,7 +16,6 @@ go_library( deps = [ "//pkg/volume:go_default_library", "//pkg/volume/util:go_default_library", - "//pkg/volume/util/types:go_default_library", "//staging/src/k8s.io/api/core/v1:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/api/errors:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library", diff --git a/pkg/volume/configmap/configmap.go b/pkg/volume/configmap/configmap.go index 4347b4b2dad..7b0d900f16e 100644 --- a/pkg/volume/configmap/configmap.go +++ b/pkg/volume/configmap/configmap.go @@ -29,7 +29,6 @@ import ( "k8s.io/apimachinery/pkg/types" "k8s.io/kubernetes/pkg/volume" volumeutil "k8s.io/kubernetes/pkg/volume/util" - volumetypes "k8s.io/kubernetes/pkg/volume/util/types" ) // ProbeVolumePlugins is the entry point for plugin detection in a package. @@ -181,9 +180,8 @@ func (b *configMapVolumeMounter) CanMount() error { return nil } -func (b *configMapVolumeMounter) SetUp(mounterArgs volume.MounterArgs) (volumetypes.OperationStatus, error) { - err := b.SetUpAt(b.GetPath(), mounterArgs) - return volumetypes.OperationFinished, err +func (b *configMapVolumeMounter) SetUp(mounterArgs volume.MounterArgs) error { + return b.SetUpAt(b.GetPath(), mounterArgs) } func (b *configMapVolumeMounter) SetUpAt(dir string, mounterArgs volume.MounterArgs) error { diff --git a/pkg/volume/configmap/configmap_test.go b/pkg/volume/configmap/configmap_test.go index 0e7b2172ba7..e03d39d1a25 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.go b/pkg/volume/csi/csi_attacher.go index c1fc1ec385e..fbe710fbfb5 100644 --- a/pkg/volume/csi/csi_attacher.go +++ b/pkg/volume/csi/csi_attacher.go @@ -220,40 +220,38 @@ func (c *csiAttacher) GetDeviceMountPath(spec *volume.Spec) (string, error) { return deviceMountPath, nil } -func (c *csiAttacher) MountDevice(spec *volume.Spec, devicePath string, deviceMountPath string) (volumetypes.OperationStatus, error) { +func (c *csiAttacher) MountDevice(spec *volume.Spec, devicePath string, deviceMountPath string) error { klog.V(4).Infof(log("attacher.MountDevice(%s, %s)", devicePath, deviceMountPath)) - // lets default to operation as finished state - opExitStatus := volumetypes.OperationFinished if deviceMountPath == "" { - return opExitStatus, errors.New(log("attacher.MountDevice failed, deviceMountPath is empty")) + return errors.New(log("attacher.MountDevice failed, deviceMountPath is empty")) } mounted, err := isDirMounted(c.plugin, deviceMountPath) if err != nil { klog.Error(log("attacher.MountDevice failed while checking mount status for dir [%s]", deviceMountPath)) - return opExitStatus, err + return err } if mounted { klog.V(4).Info(log("attacher.MountDevice skipping mount, dir already mounted [%s]", deviceMountPath)) - return opExitStatus, nil + return nil } // Setup if spec == nil { - return opExitStatus, errors.New(log("attacher.MountDevice failed, spec is nil")) + return errors.New(log("attacher.MountDevice failed, spec is nil")) } csiSource, err := getPVSourceFromSpec(spec) if err != nil { - return opExitStatus, errors.New(log("attacher.MountDevice failed to get CSIPersistentVolumeSource: %v", err)) + return errors.New(log("attacher.MountDevice failed to get CSIPersistentVolumeSource: %v", err)) } // lets check if node/unstage is supported if c.csiClient == nil { c.csiClient, err = newCsiDriverClient(csiDriverName(csiSource.Driver)) if err != nil { - return opExitStatus, errors.New(log("attacher.MountDevice failed to create newCsiDriverClient: %v", err)) + return errors.New(log("attacher.MountDevice failed to create newCsiDriverClient: %v", err)) } } csi := c.csiClient @@ -263,7 +261,7 @@ func (c *csiAttacher) MountDevice(spec *volume.Spec, devicePath string, deviceMo // Check whether "STAGE_UNSTAGE_VOLUME" is set stageUnstageSet, err := csi.NodeSupportsStageUnstage(ctx) if err != nil { - return opExitStatus, err + return err } // Get secrets and publish context required for mountDevice @@ -271,8 +269,7 @@ func (c *csiAttacher) MountDevice(spec *volume.Spec, devicePath string, deviceMo publishContext, err := c.plugin.getPublishContext(c.k8s, csiSource.VolumeHandle, csiSource.Driver, nodeName) if err != nil { - opExitStatus = volumetypes.OperationStateNoChange - return opExitStatus, err + return volumetypes.NewTransientOperationFailure(err.Error()) } nodeStageSecrets := map[string]string{} @@ -283,15 +280,14 @@ func (c *csiAttacher) MountDevice(spec *volume.Spec, devicePath string, deviceMo err = fmt.Errorf("fetching NodeStageSecretRef %s/%s failed: %v", csiSource.NodeStageSecretRef.Namespace, csiSource.NodeStageSecretRef.Name, err) // if we failed to fetch secret then that could be a transient error - opExitStatus = volumetypes.OperationStateNoChange - return opExitStatus, err + return volumetypes.NewTransientOperationFailure(err.Error()) } } // Store volume metadata for UnmountDevice. Keep it around even if the // driver does not support NodeStage, UnmountDevice still needs it. if err = os.MkdirAll(deviceMountPath, 0750); err != nil { - return opExitStatus, errors.New(log("attacher.MountDevice failed to create dir %#v: %v", deviceMountPath, err)) + return errors.New(log("attacher.MountDevice failed to create dir %#v: %v", deviceMountPath, err)) } klog.V(4).Info(log("created target path successfully [%s]", deviceMountPath)) dataDir := filepath.Dir(deviceMountPath) @@ -304,12 +300,12 @@ func (c *csiAttacher) MountDevice(spec *volume.Spec, devicePath string, deviceMo if cleanErr := os.RemoveAll(dataDir); cleanErr != nil { klog.Error(log("failed to remove dir after error [%s]: %v", dataDir, cleanErr)) } - return opExitStatus, err + return err } defer func() { // Only if there was an error and volume operation was considered // finished, we should remove the directory. - if err != nil && opExitStatus == volumetypes.OperationFinished { + if err != nil && volumetypes.IsOperationFinishedError(err) { // clean up metadata klog.Errorf(log("attacher.MountDevice failed: %v", err)) if err := removeMountDir(c.plugin, deviceMountPath); err != nil { @@ -321,7 +317,7 @@ func (c *csiAttacher) MountDevice(spec *volume.Spec, devicePath string, deviceMo if !stageUnstageSet { klog.Infof(log("attacher.MountDevice STAGE_UNSTAGE_VOLUME capability not set. Skipping MountDevice...")) // defer does *not* remove the metadata file and it's correct - UnmountDevice needs it there. - return opExitStatus, nil + return nil } //TODO (vladimirvivien) implement better AccessModes mapping between k8s and CSI @@ -347,14 +343,11 @@ func (c *csiAttacher) MountDevice(spec *volume.Spec, devicePath string, deviceMo mountOptions) if err != nil { - if volumetypes.IsOperationTimeOutError(err) { - opExitStatus = volumetypes.OperationInProgress - } - return opExitStatus, err + return err } klog.V(4).Infof(log("attacher.MountDevice successfully requested NodeStageVolume [%s]", deviceMountPath)) - return opExitStatus, err + return err } var _ volume.Detacher = &csiAttacher{} diff --git a/pkg/volume/csi/csi_attacher_test.go b/pkg/volume/csi/csi_attacher_test.go index 22b4de03eea..41c646cf839 100644 --- a/pkg/volume/csi/csi_attacher_test.go +++ b/pkg/volume/csi/csi_attacher_test.go @@ -1056,6 +1056,9 @@ func TestAttacherGetDeviceMountPath(t *testing.T) { func TestAttacherMountDevice(t *testing.T) { pvName := "test-pv" + nonFinalError := volumetypes.NewUncertainProgressError("") + transientError := volumetypes.NewTransientOperationFailure("") + testCases := []struct { testName string volName string @@ -1064,7 +1067,7 @@ func TestAttacherMountDevice(t *testing.T) { stageUnstageSet bool shouldFail bool createAttachment bool - exitStatus volumetypes.OperationStatus + exitError error spec *volume.Spec }{ { @@ -1075,7 +1078,6 @@ func TestAttacherMountDevice(t *testing.T) { stageUnstageSet: true, createAttachment: true, spec: volume.NewSpecFromPersistentVolume(makeTestPV(pvName, 10, testDriver, "test-vol1"), false), - exitStatus: volumetypes.OperationFinished, }, { testName: "normal PV with mount options", @@ -1084,7 +1086,6 @@ func TestAttacherMountDevice(t *testing.T) { deviceMountPath: "path2", stageUnstageSet: true, createAttachment: true, - exitStatus: volumetypes.OperationFinished, spec: volume.NewSpecFromPersistentVolume(makeTestPVWithMountOptions(pvName, 10, testDriver, "test-vol1", []string{"test-op"}), false), }, { @@ -1095,7 +1096,7 @@ func TestAttacherMountDevice(t *testing.T) { stageUnstageSet: true, createAttachment: false, shouldFail: true, - exitStatus: volumetypes.OperationStateNoChange, + exitError: transientError, spec: volume.NewSpecFromPersistentVolume(makeTestPVWithMountOptions(pvName, 10, testDriver, "test-vol1", []string{"test-op"}), false), }, { @@ -1106,7 +1107,6 @@ func TestAttacherMountDevice(t *testing.T) { stageUnstageSet: true, shouldFail: true, createAttachment: true, - exitStatus: volumetypes.OperationFinished, spec: volume.NewSpecFromPersistentVolume(makeTestPV(pvName, 10, testDriver, ""), false), }, { @@ -1117,7 +1117,6 @@ func TestAttacherMountDevice(t *testing.T) { stageUnstageSet: true, shouldFail: false, createAttachment: true, - exitStatus: volumetypes.OperationFinished, spec: volume.NewSpecFromPersistentVolume(makeTestPV(pvName, 10, testDriver, "test-vol1"), false), }, { @@ -1128,7 +1127,6 @@ func TestAttacherMountDevice(t *testing.T) { stageUnstageSet: true, shouldFail: true, createAttachment: true, - exitStatus: volumetypes.OperationFinished, spec: volume.NewSpecFromPersistentVolume(makeTestPV(pvName, 10, testDriver, "test-vol1"), false), }, { @@ -1138,7 +1136,6 @@ func TestAttacherMountDevice(t *testing.T) { deviceMountPath: "path2", stageUnstageSet: false, createAttachment: true, - exitStatus: volumetypes.OperationFinished, spec: volume.NewSpecFromPersistentVolume(makeTestPV(pvName, 10, testDriver, "test-vol1"), false), }, { @@ -1148,7 +1145,6 @@ func TestAttacherMountDevice(t *testing.T) { deviceMountPath: "path2", shouldFail: true, createAttachment: true, - exitStatus: volumetypes.OperationFinished, spec: volume.NewSpecFromVolume(makeTestVol(pvName, testDriver)), }, { @@ -1159,7 +1155,7 @@ func TestAttacherMountDevice(t *testing.T) { stageUnstageSet: true, createAttachment: true, spec: volume.NewSpecFromPersistentVolume(makeTestPV(pvName, 10, testDriver, fakecsi.NodeStageTimeOut_VolumeID), false), - exitStatus: volumetypes.OperationInProgress, + exitError: nonFinalError, shouldFail: true, }, } @@ -1199,7 +1195,7 @@ func TestAttacherMountDevice(t *testing.T) { } // Run - exitStatus, err := csiAttacher.MountDevice(tc.spec, tc.devicePath, tc.deviceMountPath) + err := csiAttacher.MountDevice(tc.spec, tc.devicePath, tc.deviceMountPath) // Verify if err != nil { @@ -1212,8 +1208,8 @@ func TestAttacherMountDevice(t *testing.T) { t.Errorf("test should fail, but no error occurred") } - if exitStatus != tc.exitStatus { - t.Fatalf("expected exitStatus: %v got: %v", tc.exitStatus, exitStatus) + if tc.exitError != nil && reflect.TypeOf(tc.exitError) != reflect.TypeOf(err) { + t.Fatalf("expected exitError: %v got: %v", tc.exitError, err) } // Verify call goes through all the way @@ -1348,7 +1344,7 @@ func TestAttacherMountDeviceWithInline(t *testing.T) { }() // Run - _, err = csiAttacher.MountDevice(tc.spec, tc.devicePath, tc.deviceMountPath) + 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 939e4d3a42b..9da810cd3c0 100644 --- a/pkg/volume/csi/csi_client.go +++ b/pkg/volume/csi/csi_client.go @@ -260,7 +260,7 @@ func (c *csiDriverClient) NodePublishVolume( _, err = nodeClient.NodePublishVolume(ctx, req) if err != nil && !isFinalError(err) { - return volumetypes.NewOperationTimedOutError(err.Error()) + return volumetypes.NewUncertainProgressError(err.Error()) } return nil } @@ -382,7 +382,7 @@ func (c *csiDriverClient) NodeStageVolume(ctx context.Context, _, err = nodeClient.NodeStageVolume(ctx, req) if err != nil && !isFinalError(err) { - return volumetypes.NewOperationTimedOutError(err.Error()) + return volumetypes.NewUncertainProgressError(err.Error()) } return err } diff --git a/pkg/volume/csi/csi_client_test.go b/pkg/volume/csi/csi_client_test.go index 3724b2cb574..83080da2575 100644 --- a/pkg/volume/csi/csi_client_test.go +++ b/pkg/volume/csi/csi_client_test.go @@ -157,11 +157,8 @@ func (c *fakeCsiDriverClient) NodePublishVolume( } _, err := c.nodeClient.NodePublishVolume(ctx, req) - if err != nil { - if isFinalError(err) { - return err - } - return volumetypes.NewOperationTimedOutError(err.Error()) + if err != nil && !isFinalError(err) { + return volumetypes.NewUncertainProgressError(err.Error()) } return err } @@ -208,11 +205,8 @@ func (c *fakeCsiDriverClient) NodeStageVolume(ctx context.Context, } _, err := c.nodeClient.NodeStageVolume(ctx, req) - if err != nil { - if isFinalError(err) { - return err - } - return volumetypes.NewOperationTimedOutError(err.Error()) + if err != nil && !isFinalError(err) { + return volumetypes.NewUncertainProgressError(err.Error()) } return err } diff --git a/pkg/volume/csi/csi_mounter.go b/pkg/volume/csi/csi_mounter.go index 35bc07328f3..ad53d124b78 100644 --- a/pkg/volume/csi/csi_mounter.go +++ b/pkg/volume/csi/csi_mounter.go @@ -99,42 +99,34 @@ func (c *csiMountMgr) CanMount() error { return nil } -func (c *csiMountMgr) SetUp(mounterArgs volume.MounterArgs) (volumetypes.OperationStatus, error) { - opExitStatus, err := c.setupInternal(c.GetPath(), mounterArgs) - return opExitStatus, err +func (c *csiMountMgr) SetUp(mounterArgs volume.MounterArgs) error { + return c.SetUpAt(c.GetPath(), mounterArgs) } func (c *csiMountMgr) SetUpAt(dir string, mounterArgs volume.MounterArgs) error { - _, err := c.setupInternal(dir, mounterArgs) - return err -} - -func (c *csiMountMgr) setupInternal(dir string, mounterArgs volume.MounterArgs) (volumetypes.OperationStatus, error) { klog.V(4).Infof(log("Mounter.SetUpAt(%s)", dir)) - // default to finished operation status - opExitStatus := volumetypes.OperationFinished mounted, err := isDirMounted(c.plugin, dir) if err != nil { - return opExitStatus, errors.New(log("mounter.SetUpAt failed while checking mount status for dir [%s]: %v", dir, err)) + return errors.New(log("mounter.SetUpAt failed while checking mount status for dir [%s]: %v", dir, err)) } if mounted { klog.V(4).Info(log("mounter.SetUpAt skipping mount, dir already mounted [%s]", dir)) - return opExitStatus, nil + return nil } 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)) + return volumetypes.NewTransientOperationFailure(log("mounter.SetUpAt failed to get CSI client: %v", err)) + } ctx, cancel := context.WithTimeout(context.Background(), csiTimeout) defer cancel() volSrc, pvSrc, err := getSourceFromSpec(c.spec) if err != nil { - return opExitStatus, errors.New(log("mounter.SetupAt failed to get CSI persistent source: %v", err)) + return errors.New(log("mounter.SetupAt failed to get CSI persistent source: %v", err)) } driverName := c.driverName @@ -155,10 +147,10 @@ func (c *csiMountMgr) setupInternal(dir string, mounterArgs volume.MounterArgs) switch { case volSrc != nil: if !utilfeature.DefaultFeatureGate.Enabled(features.CSIInlineVolume) { - return opExitStatus, fmt.Errorf("CSIInlineVolume feature required") + return fmt.Errorf("CSIInlineVolume feature required") } if c.volumeLifecycleMode != storage.VolumeLifecycleEphemeral { - return opExitStatus, fmt.Errorf("unexpected volume mode: %s", c.volumeLifecycleMode) + return fmt.Errorf("unexpected volume mode: %s", c.volumeLifecycleMode) } if volSrc.FSType != nil { fsType = *volSrc.FSType @@ -173,7 +165,7 @@ func (c *csiMountMgr) setupInternal(dir string, mounterArgs volume.MounterArgs) } case pvSrc != nil: if c.volumeLifecycleMode != storage.VolumeLifecyclePersistent { - return opExitStatus, fmt.Errorf("unexpected driver mode: %s", c.volumeLifecycleMode) + return fmt.Errorf("unexpected driver mode: %s", c.volumeLifecycleMode) } fsType = pvSrc.FSType @@ -194,13 +186,13 @@ func (c *csiMountMgr) setupInternal(dir string, mounterArgs volume.MounterArgs) // Check for STAGE_UNSTAGE_VOLUME set and populate deviceMountPath if so stageUnstageSet, err := csi.NodeSupportsStageUnstage(ctx) if err != nil { - return opExitStatus, errors.New(log("mounter.SetUpAt failed to check for STAGE_UNSTAGE_VOLUME capability: %v", err)) + return errors.New(log("mounter.SetUpAt failed to check for STAGE_UNSTAGE_VOLUME capability: %v", err)) } if stageUnstageSet { deviceMountPath, err = makeDeviceMountPath(c.plugin, c.spec) if err != nil { - return opExitStatus, errors.New(log("mounter.SetUpAt failed to make device mount path: %v", err)) + return errors.New(log("mounter.SetUpAt failed to make device mount path: %v", err)) } } @@ -210,19 +202,18 @@ func (c *csiMountMgr) setupInternal(dir string, mounterArgs volume.MounterArgs) c.publishContext, err = c.plugin.getPublishContext(c.k8s, volumeHandle, string(driverName), nodeName) if err != nil { // we could have a transient error associated with fetching publish context - opExitStatus = volumetypes.OperationStateNoChange - return opExitStatus, err + return volumetypes.NewTransientOperationFailure(log("mounter.SetUpAt failed to fetch publishContext: %v", err)) } publishContext = c.publishContext } default: - return opExitStatus, fmt.Errorf("volume source not found in volume.Spec") + return fmt.Errorf("volume source not found in volume.Spec") } // create target_dir before call to NodePublish if err := os.MkdirAll(dir, 0750); err != nil { - return opExitStatus, errors.New(log("mounter.SetUpAt failed to create dir %#v: %v", dir, err)) + return errors.New(log("mounter.SetUpAt failed to create dir %#v: %v", dir, err)) } klog.V(4).Info(log("created target path successfully [%s]", dir)) @@ -230,9 +221,8 @@ func (c *csiMountMgr) setupInternal(dir string, mounterArgs volume.MounterArgs) if secretRef != nil { nodePublishSecrets, err = getCredentialsFromSecret(c.k8s, secretRef) if err != nil { - opExitStatus = volumetypes.OperationStateNoChange - return opExitStatus, fmt.Errorf("fetching NodePublishSecretRef %s/%s failed: %v", - secretRef.Namespace, secretRef.Name, err) + return volumetypes.NewTransientOperationFailure(fmt.Sprintf("fetching NodePublishSecretRef %s/%s failed: %v", + secretRef.Namespace, secretRef.Name, err)) } } @@ -240,8 +230,7 @@ func (c *csiMountMgr) setupInternal(dir string, mounterArgs volume.MounterArgs) // Inject pod information into volume_attributes podAttrs, err := c.podAttributes() if err != nil { - opExitStatus = volumetypes.OperationStateNoChange - return opExitStatus, errors.New(log("mounter.SetUpAt failed to assemble volume attributes: %v", err)) + return volumetypes.NewTransientOperationFailure(log("mounter.SetUpAt failed to assemble volume attributes: %v", err)) } if podAttrs != nil { if volAttribs == nil { @@ -268,16 +257,13 @@ func (c *csiMountMgr) setupInternal(dir string, mounterArgs volume.MounterArgs) ) if err != nil { - if volumetypes.IsOperationTimeOutError(err) { - opExitStatus = volumetypes.OperationInProgress - } // If operation finished with error then we can remove the mount directory. - if opExitStatus == volumetypes.OperationFinished { + if volumetypes.IsOperationFinishedError(err) { if removeMountDirErr := removeMountDir(c.plugin, dir); removeMountDirErr != nil { klog.Error(log("mounter.SetupAt failed to remove mount dir after a NodePublish() error [%s]: %v", dir, removeMountDirErr)) } } - return opExitStatus, errors.New(log("mounter.SetupAt failed: %v", err)) + return err } c.supportsSELinux, err = c.kubeVolHost.GetHostUtil().GetSELinuxSupport(dir) @@ -289,19 +275,17 @@ func (c *csiMountMgr) setupInternal(dir string, mounterArgs volume.MounterArgs) // The following logic is derived from https://github.com/kubernetes/kubernetes/issues/66323 // if fstype is "", then skip fsgroup (could be indication of non-block filesystem) // if fstype is provided and pv.AccessMode == ReadWriteOnly, then apply fsgroup - err = c.applyFSGroup(fsType, mounterArgs.FsGroup) if err != nil { - // If we are here that means volume was mounted correctly and it must at least be unmounted - // before it can be used by someone else. - opExitStatus = volumetypes.OperationInProgress - // attempt to rollback mount. - fsGrpErr := fmt.Errorf("applyFSGroup failed for vol %s: %v", c.volumeID, err) - return opExitStatus, fsGrpErr + // At this point mount operation is successful: + // 1. Since volume can not be used by the pod because of invalid permissions, we must return error + // 2. Since mount is successful, we must record volume as mounted in uncertain state, so it can be + // cleaned up. + return volumetypes.NewUncertainProgressError(fmt.Sprintf("applyFSGroup failed for vol %s: %v", c.volumeID, err)) } klog.V(4).Infof(log("mounter.SetUp successfully requested NodePublish [%s]", dir)) - return opExitStatus, nil + return nil } func (c *csiMountMgr) podAttributes() (map[string]string, error) { diff --git a/pkg/volume/csi/csi_mounter_test.go b/pkg/volume/csi/csi_mounter_test.go index a8e9d544671..ef73b31ad97 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) } @@ -402,13 +402,15 @@ func TestMounterSetupWithStatusTracking(t *testing.T) { fakeClient := fakeclient.NewSimpleClientset() plug, tmpDir := newTestPlugin(t, fakeClient) defer os.RemoveAll(tmpDir) + nonFinalError := volumetypes.NewUncertainProgressError("non-final-error") + transientError := volumetypes.NewTransientOperationFailure("transient-error") testCases := []struct { name string podUID types.UID spec func(string, []string) *volume.Spec shouldFail bool - exitStatus volumetypes.OperationStatus + exitError error createAttachment bool }{ { @@ -420,7 +422,6 @@ func TestMounterSetupWithStatusTracking(t *testing.T) { pvSrc.Spec.MountOptions = options return volume.NewSpecFromPersistentVolume(pvSrc, false) }, - exitStatus: volumetypes.OperationFinished, createAttachment: true, }, { @@ -429,7 +430,7 @@ func TestMounterSetupWithStatusTracking(t *testing.T) { spec: func(fsType string, options []string) *volume.Spec { return volume.NewSpecFromPersistentVolume(makeTestPV("pv3", 20, testDriver, "vol4"), false) }, - exitStatus: volumetypes.OperationStateNoChange, + exitError: transientError, createAttachment: false, shouldFail: true, }, @@ -440,7 +441,7 @@ func TestMounterSetupWithStatusTracking(t *testing.T) { return volume.NewSpecFromPersistentVolume(makeTestPV("pv4", 20, testDriver, fakecsi.NodePublishTimeOut_VolumeID), false) }, createAttachment: true, - exitStatus: volumetypes.OperationInProgress, + exitError: nonFinalError, shouldFail: true, }, { @@ -454,7 +455,7 @@ func TestMounterSetupWithStatusTracking(t *testing.T) { } return volume.NewSpecFromPersistentVolume(pv, false) }, - exitStatus: volumetypes.OperationStateNoChange, + exitError: transientError, createAttachment: true, shouldFail: true, }, @@ -487,11 +488,10 @@ func TestMounterSetupWithStatusTracking(t *testing.T) { t.Fatalf("failed to setup VolumeAttachment: %v", err) } } + err = csiMounter.SetUp(volume.MounterArgs{}) - opExistStatus, err := csiMounter.SetUp(volume.MounterArgs{}) - - if opExistStatus != tc.exitStatus { - t.Fatalf("expected exitStatus: %v but got %v", tc.exitStatus, opExistStatus) + if tc.exitError != nil && reflect.TypeOf(tc.exitError) != reflect.TypeOf(err) { + t.Fatalf("expected exitError: %+v got: %+v", tc.exitError, err) } if tc.shouldFail && err == nil { @@ -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 0ee4858f955..3ec20217760 100644 --- a/pkg/volume/csi/csi_test.go +++ b/pkg/volume/csi/csi_test.go @@ -360,7 +360,7 @@ func TestCSI_VolumeAll(t *testing.T) { if err != nil { t.Fatalf("csiTest.VolumeAll deviceMounter.GetdeviceMountPath failed %s", err) } - if _, err := csiDevMounter.MountDevice(volSpec, devicePath, devMountPath); err != nil { + if err := csiDevMounter.MountDevice(volSpec, devicePath, devMountPath); err != nil { t.Fatalf("csiTest.VolumeAll deviceMounter.MountDevice failed: %v", err) } t.Log("csiTest.VolumeAll device mounted at path:", devMountPath) @@ -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/BUILD b/pkg/volume/downwardapi/BUILD index 081164f60a4..cece3c521e2 100644 --- a/pkg/volume/downwardapi/BUILD +++ b/pkg/volume/downwardapi/BUILD @@ -15,7 +15,6 @@ go_library( "//pkg/fieldpath:go_default_library", "//pkg/volume:go_default_library", "//pkg/volume/util:go_default_library", - "//pkg/volume/util/types:go_default_library", "//staging/src/k8s.io/api/core/v1:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/types:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/util/errors:go_default_library", diff --git a/pkg/volume/downwardapi/downwardapi.go b/pkg/volume/downwardapi/downwardapi.go index 83c8b11fc3d..12746696888 100644 --- a/pkg/volume/downwardapi/downwardapi.go +++ b/pkg/volume/downwardapi/downwardapi.go @@ -28,7 +28,6 @@ import ( "k8s.io/kubernetes/pkg/fieldpath" "k8s.io/kubernetes/pkg/volume" volumeutil "k8s.io/kubernetes/pkg/volume/util" - volumetypes "k8s.io/kubernetes/pkg/volume/util/types" utilstrings "k8s.io/utils/strings" ) @@ -171,9 +170,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) (volumetypes.OperationStatus, error) { - err := b.SetUpAt(b.GetPath(), mounterArgs) - return volumetypes.OperationFinished, err +func (b *downwardAPIVolumeMounter) SetUp(mounterArgs volume.MounterArgs) error { + return b.SetUpAt(b.GetPath(), mounterArgs) } func (b *downwardAPIVolumeMounter) SetUpAt(dir string, mounterArgs volume.MounterArgs) error { diff --git a/pkg/volume/downwardapi/downwardapi_test.go b/pkg/volume/downwardapi/downwardapi_test.go index 8584e812d5c..647ebe3ff56 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/BUILD b/pkg/volume/emptydir/BUILD index 13ba66ba972..c4a58fda540 100644 --- a/pkg/volume/emptydir/BUILD +++ b/pkg/volume/emptydir/BUILD @@ -20,7 +20,6 @@ go_library( "//pkg/volume:go_default_library", "//pkg/volume/util:go_default_library", "//pkg/volume/util/fsquota:go_default_library", - "//pkg/volume/util/types:go_default_library", "//staging/src/k8s.io/api/core/v1:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/api/resource:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library", diff --git a/pkg/volume/emptydir/empty_dir.go b/pkg/volume/emptydir/empty_dir.go index 2e64bff6c4d..607705aea9f 100644 --- a/pkg/volume/emptydir/empty_dir.go +++ b/pkg/volume/emptydir/empty_dir.go @@ -33,7 +33,6 @@ import ( "k8s.io/kubernetes/pkg/volume" volumeutil "k8s.io/kubernetes/pkg/volume/util" "k8s.io/kubernetes/pkg/volume/util/fsquota" - volumetypes "k8s.io/kubernetes/pkg/volume/util/types" ) // TODO: in the near future, this will be changed to be more restrictive @@ -193,9 +192,8 @@ func (ed *emptyDir) CanMount() error { } // SetUp creates new directory. -func (ed *emptyDir) SetUp(mounterArgs volume.MounterArgs) (volumetypes.OperationStatus, error) { - err := ed.SetUpAt(ed.GetPath(), mounterArgs) - return volumetypes.OperationFinished, err +func (ed *emptyDir) SetUp(mounterArgs volume.MounterArgs) error { + return ed.SetUpAt(ed.GetPath(), mounterArgs) } // SetUpAt creates new directory. diff --git a/pkg/volume/emptydir/empty_dir_test.go b/pkg/volume/emptydir/empty_dir_test.go index 4c4bcac53af..892ec1ec342 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/BUILD b/pkg/volume/fc/BUILD index 158fc17f212..dd16c9f27b3 100644 --- a/pkg/volume/fc/BUILD +++ b/pkg/volume/fc/BUILD @@ -20,7 +20,6 @@ go_library( "//pkg/features:go_default_library", "//pkg/volume:go_default_library", "//pkg/volume/util:go_default_library", - "//pkg/volume/util/types:go_default_library", "//pkg/volume/util/volumepathhandler:go_default_library", "//staging/src/k8s.io/api/core/v1:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library", diff --git a/pkg/volume/fc/attacher.go b/pkg/volume/fc/attacher.go index 09b663b6866..4a431fd9928 100644 --- a/pkg/volume/fc/attacher.go +++ b/pkg/volume/fc/attacher.go @@ -32,7 +32,6 @@ import ( "k8s.io/kubernetes/pkg/features" "k8s.io/kubernetes/pkg/volume" volumeutil "k8s.io/kubernetes/pkg/volume/util" - volumetypes "k8s.io/kubernetes/pkg/volume/util/types" ) type fcAttacher struct { @@ -97,42 +96,39 @@ func (attacher *fcAttacher) GetDeviceMountPath( return attacher.manager.MakeGlobalPDName(*mounter.fcDisk), nil } -func (attacher *fcAttacher) MountDevice(spec *volume.Spec, devicePath string, deviceMountPath string) (volumetypes.OperationStatus, error) { - mountInternal := func() error { - mounter := attacher.host.GetMounter(fcPluginName) - notMnt, err := mounter.IsLikelyNotMountPoint(deviceMountPath) - if err != nil { - if os.IsNotExist(err) { - if err := os.MkdirAll(deviceMountPath, 0750); err != nil { - return err - } - notMnt = true - } else { +func (attacher *fcAttacher) MountDevice(spec *volume.Spec, devicePath string, deviceMountPath string) error { + mounter := attacher.host.GetMounter(fcPluginName) + notMnt, err := mounter.IsLikelyNotMountPoint(deviceMountPath) + if err != nil { + if os.IsNotExist(err) { + if err := os.MkdirAll(deviceMountPath, 0750); err != nil { return err } - } - - volumeSource, readOnly, err := getVolumeSource(spec) - if err != nil { + notMnt = true + } else { return err } - - options := []string{} - if readOnly { - options = append(options, "ro") - } - if notMnt { - diskMounter := &mount.SafeFormatAndMount{Interface: mounter, Exec: attacher.host.GetExec(fcPluginName)} - mountOptions := volumeutil.MountOptionFromSpec(spec, options...) - err = diskMounter.FormatAndMount(devicePath, deviceMountPath, volumeSource.FSType, mountOptions) - if err != nil { - os.Remove(deviceMountPath) - return err - } - } - return nil } - return volumetypes.OperationFinished, mountInternal() + + volumeSource, readOnly, err := getVolumeSource(spec) + if err != nil { + return err + } + + options := []string{} + if readOnly { + options = append(options, "ro") + } + if notMnt { + diskMounter := &mount.SafeFormatAndMount{Interface: mounter, Exec: attacher.host.GetExec(fcPluginName)} + mountOptions := volumeutil.MountOptionFromSpec(spec, options...) + err = diskMounter.FormatAndMount(devicePath, deviceMountPath, volumeSource.FSType, mountOptions) + if err != nil { + os.Remove(deviceMountPath) + return err + } + } + return nil } type fcDetacher struct { diff --git a/pkg/volume/fc/fc.go b/pkg/volume/fc/fc.go index 259d1b8bbbe..dfc2aa9d062 100644 --- a/pkg/volume/fc/fc.go +++ b/pkg/volume/fc/fc.go @@ -35,7 +35,6 @@ import ( "k8s.io/kubernetes/pkg/features" "k8s.io/kubernetes/pkg/volume" "k8s.io/kubernetes/pkg/volume/util" - volumetypes "k8s.io/kubernetes/pkg/volume/util/types" "k8s.io/kubernetes/pkg/volume/util/volumepathhandler" ) @@ -370,9 +369,8 @@ func (b *fcDiskMounter) CanMount() error { return nil } -func (b *fcDiskMounter) SetUp(mounterArgs volume.MounterArgs) (volumetypes.OperationStatus, error) { - err := b.SetUpAt(b.GetPath(), mounterArgs) - return volumetypes.OperationFinished, err +func (b *fcDiskMounter) SetUp(mounterArgs volume.MounterArgs) error { + return b.SetUpAt(b.GetPath(), mounterArgs) } func (b *fcDiskMounter) SetUpAt(dir string, mounterArgs volume.MounterArgs) error { diff --git a/pkg/volume/fc/fc_test.go b/pkg/volume/fc/fc_test.go index 32dc1b51d9e..63d5a08e09a 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/BUILD b/pkg/volume/flexvolume/BUILD index 777ede4375d..9cb8aceaa93 100644 --- a/pkg/volume/flexvolume/BUILD +++ b/pkg/volume/flexvolume/BUILD @@ -32,7 +32,6 @@ go_library( "//pkg/util/filesystem:go_default_library", "//pkg/volume:go_default_library", "//pkg/volume/util:go_default_library", - "//pkg/volume/util/types:go_default_library", "//staging/src/k8s.io/api/core/v1:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/api/resource:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/types:go_default_library", diff --git a/pkg/volume/flexvolume/attacher.go b/pkg/volume/flexvolume/attacher.go index d526ed23c1b..3b98eefa079 100644 --- a/pkg/volume/flexvolume/attacher.go +++ b/pkg/volume/flexvolume/attacher.go @@ -23,7 +23,6 @@ import ( "k8s.io/apimachinery/pkg/types" "k8s.io/klog" "k8s.io/kubernetes/pkg/volume" - volumetypes "k8s.io/kubernetes/pkg/volume/util/types" ) type flexVolumeAttacher struct { @@ -71,34 +70,31 @@ func (a *flexVolumeAttacher) GetDeviceMountPath(spec *volume.Spec) (string, erro } // MountDevice is part of the volume.Attacher interface -func (a *flexVolumeAttacher) MountDevice(spec *volume.Spec, devicePath string, deviceMountPath string) (volumetypes.OperationStatus, error) { - mountInternal := func() error { - // Mount only once. - alreadyMounted, err := prepareForMount(a.plugin.host.GetMounter(a.plugin.GetPluginName()), deviceMountPath) - if err != nil { - return err - } - if alreadyMounted { - return nil - } - - call := a.plugin.NewDriverCall(mountDeviceCmd) - call.Append(deviceMountPath) - call.Append(devicePath) - call.AppendSpec(spec, a.plugin.host, nil) - - _, err = call.Run() - if isCmdNotSupportedErr(err) { - // Devicepath is empty if the plugin does not support attach calls. Ignore mountDevice calls if the - // plugin does not implement attach interface. - if devicePath != "" { - return (*attacherDefaults)(a).MountDevice(spec, devicePath, deviceMountPath, a.plugin.host.GetMounter(a.plugin.GetPluginName())) - } - return nil - } +func (a *flexVolumeAttacher) MountDevice(spec *volume.Spec, devicePath string, deviceMountPath string) error { + // Mount only once. + alreadyMounted, err := prepareForMount(a.plugin.host.GetMounter(a.plugin.GetPluginName()), deviceMountPath) + if err != nil { return err } - return volumetypes.OperationFinished, mountInternal() + if alreadyMounted { + return nil + } + + call := a.plugin.NewDriverCall(mountDeviceCmd) + call.Append(deviceMountPath) + call.Append(devicePath) + call.AppendSpec(spec, a.plugin.host, nil) + + _, err = call.Run() + if isCmdNotSupportedErr(err) { + // Devicepath is empty if the plugin does not support attach calls. Ignore mountDevice calls if the + // plugin does not implement attach interface. + if devicePath != "" { + return (*attacherDefaults)(a).MountDevice(spec, devicePath, deviceMountPath, a.plugin.host.GetMounter(a.plugin.GetPluginName())) + } + return nil + } + return err } func (a *flexVolumeAttacher) VolumesAreAttached(specs []*volume.Spec, nodeName types.NodeName) (map[*volume.Spec]bool, error) { diff --git a/pkg/volume/flexvolume/mounter.go b/pkg/volume/flexvolume/mounter.go index 084e7ff7701..94229d0d833 100644 --- a/pkg/volume/flexvolume/mounter.go +++ b/pkg/volume/flexvolume/mounter.go @@ -21,7 +21,6 @@ import ( "strconv" "k8s.io/kubernetes/pkg/volume" - volumetypes "k8s.io/kubernetes/pkg/volume/util/types" "k8s.io/utils/exec" ) @@ -40,9 +39,8 @@ var _ volume.Mounter = &flexVolumeMounter{} // Mounter interface // SetUp creates new directory. -func (f *flexVolumeMounter) SetUp(mounterArgs volume.MounterArgs) (volumetypes.OperationStatus, error) { - err := f.SetUpAt(f.GetPath(), mounterArgs) - return volumetypes.OperationFinished, err +func (f *flexVolumeMounter) SetUp(mounterArgs volume.MounterArgs) error { + return f.SetUpAt(f.GetPath(), mounterArgs) } // SetUpAt creates new directory. diff --git a/pkg/volume/flocker/BUILD b/pkg/volume/flocker/BUILD index e67799ee70e..276ada39eed 100644 --- a/pkg/volume/flocker/BUILD +++ b/pkg/volume/flocker/BUILD @@ -19,7 +19,6 @@ go_library( "//pkg/util/env:go_default_library", "//pkg/volume:go_default_library", "//pkg/volume/util:go_default_library", - "//pkg/volume/util/types:go_default_library", "//staging/src/k8s.io/api/core/v1:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/api/resource:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library", diff --git a/pkg/volume/flocker/flocker.go b/pkg/volume/flocker/flocker.go index 2c951e68709..723b28de4db 100644 --- a/pkg/volume/flocker/flocker.go +++ b/pkg/volume/flocker/flocker.go @@ -31,7 +31,6 @@ import ( "k8s.io/apimachinery/pkg/types" "k8s.io/kubernetes/pkg/util/env" "k8s.io/kubernetes/pkg/volume" - volumetypes "k8s.io/kubernetes/pkg/volume/util/types" ) // ProbeVolumePlugins is the primary entrypoint for volume plugins. @@ -232,9 +231,8 @@ func (b *flockerVolumeMounter) GetPath() string { } // SetUp bind mounts the disk global mount to the volume path. -func (b *flockerVolumeMounter) SetUp(mounterArgs volume.MounterArgs) (volumetypes.OperationStatus, error) { - err := b.SetUpAt(b.GetPath(), mounterArgs) - return volumetypes.OperationFinished, err +func (b *flockerVolumeMounter) SetUp(mounterArgs volume.MounterArgs) error { + return b.SetUpAt(b.GetPath(), mounterArgs) } // newFlockerClient uses environment variables and pod attributes to return a diff --git a/pkg/volume/gcepd/BUILD b/pkg/volume/gcepd/BUILD index 2ef8af55718..cff495a05d4 100644 --- a/pkg/volume/gcepd/BUILD +++ b/pkg/volume/gcepd/BUILD @@ -20,7 +20,6 @@ go_library( "//pkg/features:go_default_library", "//pkg/volume:go_default_library", "//pkg/volume/util:go_default_library", - "//pkg/volume/util/types:go_default_library", "//pkg/volume/util/volumepathhandler:go_default_library", "//staging/src/k8s.io/api/core/v1:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/api/resource:go_default_library", diff --git a/pkg/volume/gcepd/attacher.go b/pkg/volume/gcepd/attacher.go index 4b492edabec..43c03564c59 100644 --- a/pkg/volume/gcepd/attacher.go +++ b/pkg/volume/gcepd/attacher.go @@ -38,7 +38,6 @@ import ( "k8s.io/apimachinery/pkg/util/sets" "k8s.io/kubernetes/pkg/volume" volumeutil "k8s.io/kubernetes/pkg/volume/util" - volumetypes "k8s.io/kubernetes/pkg/volume/util/types" "k8s.io/legacy-cloud-providers/gce" ) @@ -287,7 +286,7 @@ func (attacher *gcePersistentDiskAttacher) GetDeviceMountPath( return makeGlobalPDName(attacher.host, volumeSource.PDName), nil } -func (attacher *gcePersistentDiskAttacher) mountDeviceInternal(spec *volume.Spec, devicePath string, deviceMountPath string) error { +func (attacher *gcePersistentDiskAttacher) MountDevice(spec *volume.Spec, devicePath string, deviceMountPath string) error { // Only mount the PD globally once. mounter := attacher.host.GetMounter(gcePersistentDiskPluginName) notMnt, err := mounter.IsLikelyNotMountPoint(deviceMountPath) @@ -329,11 +328,6 @@ func (attacher *gcePersistentDiskAttacher) mountDeviceInternal(spec *volume.Spec return nil } -func (attacher *gcePersistentDiskAttacher) MountDevice(spec *volume.Spec, devicePath string, deviceMountPath string) (volumetypes.OperationStatus, error) { - err := attacher.mountDeviceInternal(spec, devicePath, deviceMountPath) - return volumetypes.OperationFinished, err -} - type gcePersistentDiskDetacher struct { host volume.VolumeHost gceDisks gce.Disks diff --git a/pkg/volume/gcepd/gce_pd.go b/pkg/volume/gcepd/gce_pd.go index a43ca3868fa..a1f4eff61b1 100644 --- a/pkg/volume/gcepd/gce_pd.go +++ b/pkg/volume/gcepd/gce_pd.go @@ -40,7 +40,6 @@ import ( "k8s.io/kubernetes/pkg/features" "k8s.io/kubernetes/pkg/volume" "k8s.io/kubernetes/pkg/volume/util" - volumetypes "k8s.io/kubernetes/pkg/volume/util/types" gcecloud "k8s.io/legacy-cloud-providers/gce" ) @@ -369,9 +368,8 @@ func (b *gcePersistentDiskMounter) CanMount() error { } // SetUp bind mounts the disk global mount to the volume path. -func (b *gcePersistentDiskMounter) SetUp(mounterArgs volume.MounterArgs) (volumetypes.OperationStatus, error) { - err := b.SetUpAt(b.GetPath(), mounterArgs) - return volumetypes.OperationFinished, err +func (b *gcePersistentDiskMounter) SetUp(mounterArgs volume.MounterArgs) error { + return b.SetUpAt(b.GetPath(), mounterArgs) } // SetUp bind mounts the disk global mount to the give volume path. diff --git a/pkg/volume/gcepd/gce_pd_test.go b/pkg/volume/gcepd/gce_pd_test.go index 6102930b1c5..d45de1be877 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/BUILD b/pkg/volume/git_repo/BUILD index 467410ea49b..57a9e29bdf2 100644 --- a/pkg/volume/git_repo/BUILD +++ b/pkg/volume/git_repo/BUILD @@ -16,7 +16,6 @@ go_library( deps = [ "//pkg/volume:go_default_library", "//pkg/volume/util:go_default_library", - "//pkg/volume/util/types:go_default_library", "//staging/src/k8s.io/api/core/v1:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/types:go_default_library", "//vendor/k8s.io/utils/exec:go_default_library", diff --git a/pkg/volume/git_repo/git_repo.go b/pkg/volume/git_repo/git_repo.go index aacc389194d..aee38f5bbf9 100644 --- a/pkg/volume/git_repo/git_repo.go +++ b/pkg/volume/git_repo/git_repo.go @@ -26,7 +26,6 @@ import ( "k8s.io/apimachinery/pkg/types" "k8s.io/kubernetes/pkg/volume" volumeutil "k8s.io/kubernetes/pkg/volume/util" - volumetypes "k8s.io/kubernetes/pkg/volume/util/types" "k8s.io/utils/exec" utilstrings "k8s.io/utils/strings" ) @@ -176,9 +175,8 @@ func (b *gitRepoVolumeMounter) CanMount() error { } // SetUp creates new directory and clones a git repo. -func (b *gitRepoVolumeMounter) SetUp(mounterArgs volume.MounterArgs) (volumetypes.OperationStatus, error) { - err := b.SetUpAt(b.GetPath(), mounterArgs) - return volumetypes.OperationFinished, err +func (b *gitRepoVolumeMounter) SetUp(mounterArgs volume.MounterArgs) error { + return b.SetUpAt(b.GetPath(), mounterArgs) } // SetUpAt creates new directory and clones a git repo. diff --git a/pkg/volume/glusterfs/BUILD b/pkg/volume/glusterfs/BUILD index a8a73afe7d5..a7b6e671934 100644 --- a/pkg/volume/glusterfs/BUILD +++ b/pkg/volume/glusterfs/BUILD @@ -19,7 +19,6 @@ go_library( "//pkg/apis/core/v1/helper:go_default_library", "//pkg/volume:go_default_library", "//pkg/volume/util:go_default_library", - "//pkg/volume/util/types:go_default_library", "//staging/src/k8s.io/api/core/v1:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/api/errors:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/api/resource:go_default_library", diff --git a/pkg/volume/glusterfs/glusterfs.go b/pkg/volume/glusterfs/glusterfs.go index c42edb94197..8067ba83344 100644 --- a/pkg/volume/glusterfs/glusterfs.go +++ b/pkg/volume/glusterfs/glusterfs.go @@ -47,7 +47,6 @@ import ( v1helper "k8s.io/kubernetes/pkg/apis/core/v1/helper" "k8s.io/kubernetes/pkg/volume" volutil "k8s.io/kubernetes/pkg/volume/util" - volumetypes "k8s.io/kubernetes/pkg/volume/util/types" ) // ProbeVolumePlugins is the primary entrypoint for volume plugins. @@ -270,9 +269,8 @@ func (b *glusterfsMounter) CanMount() error { } // SetUp attaches the disk and bind mounts to the volume path. -func (b *glusterfsMounter) SetUp(mounterArgs volume.MounterArgs) (volumetypes.OperationStatus, error) { - err := b.SetUpAt(b.GetPath(), mounterArgs) - return volumetypes.OperationFinished, err +func (b *glusterfsMounter) SetUp(mounterArgs volume.MounterArgs) error { + return b.SetUpAt(b.GetPath(), mounterArgs) } func (b *glusterfsMounter) SetUpAt(dir string, mounterArgs volume.MounterArgs) error { diff --git a/pkg/volume/glusterfs/glusterfs_test.go b/pkg/volume/glusterfs/glusterfs_test.go index 1eba9fbc9a8..25e199d2ad9 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/BUILD b/pkg/volume/hostpath/BUILD index 5aae5056809..0c2bd65bf3a 100644 --- a/pkg/volume/hostpath/BUILD +++ b/pkg/volume/hostpath/BUILD @@ -18,7 +18,6 @@ go_library( "//pkg/volume/util:go_default_library", "//pkg/volume/util/hostutil:go_default_library", "//pkg/volume/util/recyclerclient:go_default_library", - "//pkg/volume/util/types:go_default_library", "//pkg/volume/validation:go_default_library", "//staging/src/k8s.io/api/core/v1:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library", diff --git a/pkg/volume/hostpath/host_path.go b/pkg/volume/hostpath/host_path.go index 649a0f07836..af10f49ed54 100644 --- a/pkg/volume/hostpath/host_path.go +++ b/pkg/volume/hostpath/host_path.go @@ -31,7 +31,6 @@ import ( "k8s.io/kubernetes/pkg/volume/util" "k8s.io/kubernetes/pkg/volume/util/hostutil" "k8s.io/kubernetes/pkg/volume/util/recyclerclient" - volumetypes "k8s.io/kubernetes/pkg/volume/util/types" "k8s.io/kubernetes/pkg/volume/validation" ) @@ -227,20 +226,16 @@ func (b *hostPathMounter) CanMount() error { } // SetUp does nothing. -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 - } - return checkType(b.GetPath(), b.pathType, b.hu) +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) } - return volumetypes.OperationFinished, internalSetup() + if *b.pathType == v1.HostPathUnset { + return nil + } + return checkType(b.GetPath(), b.pathType, b.hu) } // 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 2191d0d6148..fa94755b1bc 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/BUILD b/pkg/volume/iscsi/BUILD index 68c19fa25f4..c7ac6a79bf9 100644 --- a/pkg/volume/iscsi/BUILD +++ b/pkg/volume/iscsi/BUILD @@ -21,7 +21,6 @@ go_library( "//pkg/kubelet/config:go_default_library", "//pkg/volume:go_default_library", "//pkg/volume/util:go_default_library", - "//pkg/volume/util/types:go_default_library", "//pkg/volume/util/volumepathhandler:go_default_library", "//staging/src/k8s.io/api/core/v1:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library", diff --git a/pkg/volume/iscsi/attacher.go b/pkg/volume/iscsi/attacher.go index 20c801126ee..7b3d439077e 100644 --- a/pkg/volume/iscsi/attacher.go +++ b/pkg/volume/iscsi/attacher.go @@ -31,8 +31,6 @@ import ( "k8s.io/kubernetes/pkg/features" "k8s.io/kubernetes/pkg/volume" volumeutil "k8s.io/kubernetes/pkg/volume/util" - volumetypes "k8s.io/kubernetes/pkg/volume/util/types" - "k8s.io/utils/keymutex" ) type iscsiAttacher struct { @@ -102,7 +100,7 @@ func (attacher *iscsiAttacher) GetDeviceMountPath( return attacher.manager.MakeGlobalPDName(*mounter.iscsiDisk), nil } -func (attacher *iscsiAttacher) mountDeviceInternal(spec *volume.Spec, devicePath string, deviceMountPath string) error { +func (attacher *iscsiAttacher) MountDevice(spec *volume.Spec, devicePath string, deviceMountPath string) error { mounter := attacher.host.GetMounter(iscsiPluginName) notMnt, err := mounter.IsLikelyNotMountPoint(deviceMountPath) if err != nil { @@ -136,11 +134,6 @@ func (attacher *iscsiAttacher) mountDeviceInternal(spec *volume.Spec, devicePath return nil } -func (attacher *iscsiAttacher) MountDevice(spec *volume.Spec, devicePath string, deviceMountPath string) (volumetypes.OperationStatus, error) { - err := attacher.mountDeviceInternal(spec, devicePath, deviceMountPath) - return volumetypes.OperationFinished, err -} - type iscsiDetacher struct { host volume.VolumeHost mounter mount.Interface diff --git a/pkg/volume/iscsi/iscsi.go b/pkg/volume/iscsi/iscsi.go index 1e46cbba7b0..957a90a331a 100644 --- a/pkg/volume/iscsi/iscsi.go +++ b/pkg/volume/iscsi/iscsi.go @@ -34,7 +34,6 @@ import ( "k8s.io/apimachinery/pkg/types" "k8s.io/kubernetes/pkg/volume" ioutil "k8s.io/kubernetes/pkg/volume/util" - volumetypes "k8s.io/kubernetes/pkg/volume/util/types" "k8s.io/kubernetes/pkg/volume/util/volumepathhandler" ) @@ -339,9 +338,8 @@ func (b *iscsiDiskMounter) CanMount() error { return nil } -func (b *iscsiDiskMounter) SetUp(mounterArgs volume.MounterArgs) (volumetypes.OperationStatus, error) { - err := b.SetUpAt(b.GetPath(), mounterArgs) - return volumetypes.OperationFinished, err +func (b *iscsiDiskMounter) SetUp(mounterArgs volume.MounterArgs) error { + return b.SetUpAt(b.GetPath(), mounterArgs) } func (b *iscsiDiskMounter) SetUpAt(dir string, mounterArgs volume.MounterArgs) error { diff --git a/pkg/volume/iscsi/iscsi_test.go b/pkg/volume/iscsi/iscsi_test.go index 924855f55c3..a062067e123 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/BUILD b/pkg/volume/local/BUILD index 1115ac9314b..1fcbe6c957f 100644 --- a/pkg/volume/local/BUILD +++ b/pkg/volume/local/BUILD @@ -13,7 +13,6 @@ go_library( "//pkg/volume:go_default_library", "//pkg/volume/util:go_default_library", "//pkg/volume/util/hostutil:go_default_library", - "//pkg/volume/util/types:go_default_library", "//pkg/volume/validation:go_default_library", "//staging/src/k8s.io/api/core/v1:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library", diff --git a/pkg/volume/local/local.go b/pkg/volume/local/local.go index 7f0a8e41f32..b79180a6c87 100644 --- a/pkg/volume/local/local.go +++ b/pkg/volume/local/local.go @@ -33,7 +33,6 @@ import ( "k8s.io/kubernetes/pkg/volume" "k8s.io/kubernetes/pkg/volume/util" "k8s.io/kubernetes/pkg/volume/util/hostutil" - volumetypes "k8s.io/kubernetes/pkg/volume/util/types" "k8s.io/kubernetes/pkg/volume/validation" "k8s.io/utils/keymutex" "k8s.io/utils/mount" @@ -349,29 +348,26 @@ func (dm *deviceMounter) mountLocalBlockDevice(spec *volume.Spec, devicePath str return nil } -func (dm *deviceMounter) MountDevice(spec *volume.Spec, devicePath string, deviceMountPath string) (volumetypes.OperationStatus, error) { - mountInternal := func() error { - if spec.PersistentVolume.Spec.Local == nil || len(spec.PersistentVolume.Spec.Local.Path) == 0 { - return fmt.Errorf("local volume source is nil or local path is not set") - } - fileType, err := dm.hostUtil.GetFileType(spec.PersistentVolume.Spec.Local.Path) - if err != nil { - return err - } - - switch fileType { - case hostutil.FileTypeBlockDev: - // local volume plugin does not implement AttachableVolumePlugin interface, so set devicePath to Path in PV spec directly - devicePath = spec.PersistentVolume.Spec.Local.Path - return dm.mountLocalBlockDevice(spec, devicePath, deviceMountPath) - case hostutil.FileTypeDirectory: - // if the given local volume path is of already filesystem directory, return directly - return nil - default: - return fmt.Errorf("only directory and block device are supported") - } +func (dm *deviceMounter) MountDevice(spec *volume.Spec, devicePath string, deviceMountPath string) error { + if spec.PersistentVolume.Spec.Local == nil || len(spec.PersistentVolume.Spec.Local.Path) == 0 { + return fmt.Errorf("local volume source is nil or local path is not set") + } + fileType, err := dm.hostUtil.GetFileType(spec.PersistentVolume.Spec.Local.Path) + if err != nil { + return err + } + + switch fileType { + case hostutil.FileTypeBlockDev: + // local volume plugin does not implement AttachableVolumePlugin interface, so set devicePath to Path in PV spec directly + devicePath = spec.PersistentVolume.Spec.Local.Path + return dm.mountLocalBlockDevice(spec, devicePath, deviceMountPath) + case hostutil.FileTypeDirectory: + // if the given local volume path is of already filesystem directory, return directly + return nil + default: + return fmt.Errorf("only directory and block device are supported") } - return volumetypes.OperationFinished, mountInternal() } func getVolumeSourceFSType(spec *volume.Spec) (string, error) { @@ -473,9 +469,8 @@ func (m *localVolumeMounter) CanMount() error { } // SetUp bind mounts the directory to the volume path -func (m *localVolumeMounter) SetUp(mounterArgs volume.MounterArgs) (volumetypes.OperationStatus, error) { - err := m.SetUpAt(m.GetPath(), mounterArgs) - return volumetypes.OperationFinished, err +func (m *localVolumeMounter) SetUp(mounterArgs volume.MounterArgs) error { + return m.SetUpAt(m.GetPath(), mounterArgs) } // SetUpAt bind mounts the directory to the volume path and sets up volume ownership diff --git a/pkg/volume/local/local_test.go b/pkg/volume/local/local_test.go index c48e3bbffe9..301cff5f1dd 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) @@ -231,7 +231,7 @@ func TestBlockDeviceGlobalPathAndMountDevice(t *testing.T) { fmt.Println("expected global path is:", expectedGlobalPath) - _, err = dm.MountDevice(pvSpec, tmpBlockDir, expectedGlobalPath) + err = dm.MountDevice(pvSpec, tmpBlockDir, expectedGlobalPath) if err != nil { t.Fatal(err) } @@ -276,7 +276,7 @@ func TestFSGlobalPathAndMountDevice(t *testing.T) { } // Actually, we will do nothing if the local path is FS type - _, err = dm.MountDevice(pvSpec, tmpFSDir, expectedGlobalPath) + err = dm.MountDevice(pvSpec, tmpFSDir, expectedGlobalPath) if err != nil { t.Fatal(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/BUILD b/pkg/volume/nfs/BUILD index 268871a739a..624a7898a2e 100644 --- a/pkg/volume/nfs/BUILD +++ b/pkg/volume/nfs/BUILD @@ -17,7 +17,6 @@ go_library( "//pkg/volume:go_default_library", "//pkg/volume/util:go_default_library", "//pkg/volume/util/recyclerclient:go_default_library", - "//pkg/volume/util/types:go_default_library", "//staging/src/k8s.io/api/core/v1:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/types:go_default_library", diff --git a/pkg/volume/nfs/nfs.go b/pkg/volume/nfs/nfs.go index c02b256dd23..41b2e1dd9bb 100644 --- a/pkg/volume/nfs/nfs.go +++ b/pkg/volume/nfs/nfs.go @@ -31,7 +31,6 @@ import ( "k8s.io/kubernetes/pkg/volume" "k8s.io/kubernetes/pkg/volume/util" "k8s.io/kubernetes/pkg/volume/util/recyclerclient" - volumetypes "k8s.io/kubernetes/pkg/volume/util/types" ) func getPath(uid types.UID, volName string, host volume.VolumeHost) string { @@ -238,9 +237,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) (volumetypes.OperationStatus, error) { - err := nfsMounter.SetUpAt(nfsMounter.GetPath(), mounterArgs) - return volumetypes.OperationFinished, err +func (nfsMounter *nfsMounter) SetUp(mounterArgs volume.MounterArgs) error { + return nfsMounter.SetUpAt(nfsMounter.GetPath(), mounterArgs) } func (nfsMounter *nfsMounter) SetUpAt(dir string, mounterArgs volume.MounterArgs) error { diff --git a/pkg/volume/nfs/nfs_test.go b/pkg/volume/nfs/nfs_test.go index 298c1ff5d1d..f2b0e519060 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/BUILD b/pkg/volume/portworx/BUILD index 41e6240d7ee..06f8c232f02 100644 --- a/pkg/volume/portworx/BUILD +++ b/pkg/volume/portworx/BUILD @@ -33,7 +33,6 @@ go_library( "//pkg/apis/core:go_default_library", "//pkg/volume:go_default_library", "//pkg/volume/util:go_default_library", - "//pkg/volume/util/types:go_default_library", "//staging/src/k8s.io/api/core/v1:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/api/resource:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library", diff --git a/pkg/volume/portworx/portworx.go b/pkg/volume/portworx/portworx.go index 87d34363faa..da6cfbe7088 100644 --- a/pkg/volume/portworx/portworx.go +++ b/pkg/volume/portworx/portworx.go @@ -31,7 +31,6 @@ import ( "k8s.io/apimachinery/pkg/types" "k8s.io/kubernetes/pkg/volume" "k8s.io/kubernetes/pkg/volume/util" - volumetypes "k8s.io/kubernetes/pkg/volume/util/types" ) const ( @@ -296,9 +295,8 @@ func (b *portworxVolumeMounter) CanMount() error { } // SetUp attaches the disk and bind mounts to the volume path. -func (b *portworxVolumeMounter) SetUp(mounterArgs volume.MounterArgs) (volumetypes.OperationStatus, error) { - err := b.SetUpAt(b.GetPath(), mounterArgs) - return volumetypes.OperationFinished, err +func (b *portworxVolumeMounter) SetUp(mounterArgs volume.MounterArgs) error { + return b.SetUpAt(b.GetPath(), mounterArgs) } // SetUpAt attaches the disk and bind mounts to the volume path. diff --git a/pkg/volume/portworx/portworx_test.go b/pkg/volume/portworx/portworx_test.go index d70c036e3c5..02492f6d99d 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/BUILD b/pkg/volume/projected/BUILD index 3dfe5afe15a..d3685747bf5 100644 --- a/pkg/volume/projected/BUILD +++ b/pkg/volume/projected/BUILD @@ -41,7 +41,6 @@ go_library( "//pkg/volume/downwardapi:go_default_library", "//pkg/volume/secret:go_default_library", "//pkg/volume/util:go_default_library", - "//pkg/volume/util/types:go_default_library", "//staging/src/k8s.io/api/authentication/v1:go_default_library", "//staging/src/k8s.io/api/core/v1:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/api/errors:go_default_library", diff --git a/pkg/volume/projected/projected.go b/pkg/volume/projected/projected.go index dcbc0f478bd..65e1ac5e2f1 100644 --- a/pkg/volume/projected/projected.go +++ b/pkg/volume/projected/projected.go @@ -33,7 +33,6 @@ import ( "k8s.io/kubernetes/pkg/volume/downwardapi" "k8s.io/kubernetes/pkg/volume/secret" volumeutil "k8s.io/kubernetes/pkg/volume/util" - volumetypes "k8s.io/kubernetes/pkg/volume/util/types" utilstrings "k8s.io/utils/strings" ) @@ -185,9 +184,8 @@ func (s *projectedVolumeMounter) CanMount() error { return nil } -func (s *projectedVolumeMounter) SetUp(mounterArgs volume.MounterArgs) (volumetypes.OperationStatus, error) { - err := s.SetUpAt(s.GetPath(), mounterArgs) - return volumetypes.OperationFinished, err +func (s *projectedVolumeMounter) SetUp(mounterArgs volume.MounterArgs) error { + return s.SetUpAt(s.GetPath(), mounterArgs) } func (s *projectedVolumeMounter) SetUpAt(dir string, mounterArgs volume.MounterArgs) error { diff --git a/pkg/volume/projected/projected_test.go b/pkg/volume/projected/projected_test.go index bd882305df0..ecf9b694073 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/BUILD b/pkg/volume/quobyte/BUILD index e8f05722228..60bc5a55ab1 100644 --- a/pkg/volume/quobyte/BUILD +++ b/pkg/volume/quobyte/BUILD @@ -17,7 +17,6 @@ go_library( deps = [ "//pkg/volume:go_default_library", "//pkg/volume/util:go_default_library", - "//pkg/volume/util/types:go_default_library", "//staging/src/k8s.io/api/core/v1:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/api/resource:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library", diff --git a/pkg/volume/quobyte/quobyte.go b/pkg/volume/quobyte/quobyte.go index 19e6f2924c0..33d730da522 100644 --- a/pkg/volume/quobyte/quobyte.go +++ b/pkg/volume/quobyte/quobyte.go @@ -33,7 +33,6 @@ import ( "k8s.io/apimachinery/pkg/types" "k8s.io/kubernetes/pkg/volume" "k8s.io/kubernetes/pkg/volume/util" - volumetypes "k8s.io/kubernetes/pkg/volume/util/types" ) // ProbeVolumePlugins is the primary entrypoint for volume plugins. @@ -235,10 +234,9 @@ func (mounter *quobyteMounter) CanMount() error { } // SetUp attaches the disk and bind mounts to the volume path. -func (mounter *quobyteMounter) SetUp(mounterArgs volume.MounterArgs) (volumetypes.OperationStatus, error) { +func (mounter *quobyteMounter) SetUp(mounterArgs volume.MounterArgs) error { pluginDir := mounter.plugin.host.GetPluginDir(utilstrings.EscapeQualifiedName(quobytePluginName)) - err := mounter.SetUpAt(pluginDir, mounterArgs) - return volumetypes.OperationFinished, err + return mounter.SetUpAt(pluginDir, mounterArgs) } func (mounter *quobyteMounter) SetUpAt(dir string, mounterArgs volume.MounterArgs) error { diff --git a/pkg/volume/quobyte/quobyte_test.go b/pkg/volume/quobyte/quobyte_test.go index d24ca69a265..eb3ef2bbba8 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/BUILD b/pkg/volume/rbd/BUILD index f26fdbb9895..09226ab256b 100644 --- a/pkg/volume/rbd/BUILD +++ b/pkg/volume/rbd/BUILD @@ -21,7 +21,6 @@ go_library( "//pkg/util/node:go_default_library", "//pkg/volume:go_default_library", "//pkg/volume/util:go_default_library", - "//pkg/volume/util/types:go_default_library", "//pkg/volume/util/volumepathhandler:go_default_library", "//staging/src/k8s.io/api/core/v1:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/api/resource:go_default_library", diff --git a/pkg/volume/rbd/attacher.go b/pkg/volume/rbd/attacher.go index 354100c1b39..12c2e77c935 100644 --- a/pkg/volume/rbd/attacher.go +++ b/pkg/volume/rbd/attacher.go @@ -28,7 +28,6 @@ import ( "k8s.io/apimachinery/pkg/types" "k8s.io/kubernetes/pkg/volume" volutil "k8s.io/kubernetes/pkg/volume/util" - volumetypes "k8s.io/kubernetes/pkg/volume/util/types" ) // NewAttacher implements AttachableVolumePlugin.NewAttacher. @@ -144,7 +143,10 @@ func (attacher *rbdAttacher) GetDeviceMountPath(spec *volume.Spec) (string, erro return makePDNameInternal(attacher.plugin.host, pool, img), nil } -func (attacher *rbdAttacher) mountDeviceInternal(spec *volume.Spec, devicePath string, deviceMountPath string) error { +// MountDevice implements Attacher.MountDevice. It is called by the kubelet to +// mount device at the given mount path. +// This method is idempotent, callers are responsible for retrying on failure. +func (attacher *rbdAttacher) MountDevice(spec *volume.Spec, devicePath string, deviceMountPath string) error { klog.V(4).Infof("rbd: mouting device %s to %s", devicePath, deviceMountPath) notMnt, err := attacher.mounter.IsLikelyNotMountPoint(deviceMountPath) if err != nil { @@ -182,14 +184,6 @@ func (attacher *rbdAttacher) mountDeviceInternal(spec *volume.Spec, devicePath s return nil } -// MountDevice implements Attacher.MountDevice. It is called by the kubelet to -// mount device at the given mount path. -// This method is idempotent, callers are responsible for retrying on failure. -func (attacher *rbdAttacher) MountDevice(spec *volume.Spec, devicePath string, deviceMountPath string) (volumetypes.OperationStatus, error) { - err := attacher.mountDeviceInternal(spec, devicePath, deviceMountPath) - return volumetypes.OperationFinished, err -} - // rbdDetacher implements volume.Detacher interface. type rbdDetacher struct { plugin *rbdPlugin diff --git a/pkg/volume/rbd/rbd.go b/pkg/volume/rbd/rbd.go index 751909b69fe..a07fe8bf06d 100644 --- a/pkg/volume/rbd/rbd.go +++ b/pkg/volume/rbd/rbd.go @@ -40,7 +40,6 @@ import ( "k8s.io/kubernetes/pkg/volume" "k8s.io/kubernetes/pkg/volume/util" volutil "k8s.io/kubernetes/pkg/volume/util" - volumetypes "k8s.io/kubernetes/pkg/volume/util/types" "k8s.io/kubernetes/pkg/volume/util/volumepathhandler" ) @@ -837,9 +836,8 @@ func (b *rbdMounter) CanMount() error { return nil } -func (b *rbdMounter) SetUp(mounterArgs volume.MounterArgs) (volumetypes.OperationStatus, error) { - err := b.SetUpAt(b.GetPath(), mounterArgs) - return volumetypes.OperationFinished, err +func (b *rbdMounter) SetUp(mounterArgs volume.MounterArgs) error { + return b.SetUpAt(b.GetPath(), mounterArgs) } func (b *rbdMounter) SetUpAt(dir string, mounterArgs volume.MounterArgs) error { diff --git a/pkg/volume/rbd/rbd_test.go b/pkg/volume/rbd/rbd_test.go index fa07b6b5ba1..e58020ddadb 100644 --- a/pkg/volume/rbd/rbd_test.go +++ b/pkg/volume/rbd/rbd_test.go @@ -281,7 +281,7 @@ func doTestPlugin(t *testing.T, c *testcase) { if deviceMountPath != c.expectedDeviceMountPath { t.Errorf("Unexpected mount path, expected %q, not: %q", c.expectedDeviceMountPath, deviceMountPath) } - _, err = attacher.MountDevice(c.spec, devicePath, deviceMountPath) + err = attacher.MountDevice(c.spec, devicePath, deviceMountPath) if err != nil { t.Fatal(err) } @@ -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/BUILD b/pkg/volume/scaleio/BUILD index 5af6e3ecf91..f1410f50657 100644 --- a/pkg/volume/scaleio/BUILD +++ b/pkg/volume/scaleio/BUILD @@ -42,7 +42,6 @@ go_library( deps = [ "//pkg/volume:go_default_library", "//pkg/volume/util:go_default_library", - "//pkg/volume/util/types:go_default_library", "//staging/src/k8s.io/api/core/v1:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/api/resource:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library", diff --git a/pkg/volume/scaleio/sio_volume.go b/pkg/volume/scaleio/sio_volume.go index 72c20aefa1d..9904147be53 100644 --- a/pkg/volume/scaleio/sio_volume.go +++ b/pkg/volume/scaleio/sio_volume.go @@ -35,7 +35,6 @@ import ( volumehelpers "k8s.io/cloud-provider/volume/helpers" "k8s.io/kubernetes/pkg/volume" "k8s.io/kubernetes/pkg/volume/util" - volumetypes "k8s.io/kubernetes/pkg/volume/util/types" ) type sioVolume struct { @@ -79,9 +78,8 @@ func (v *sioVolume) CanMount() error { return nil } -func (v *sioVolume) SetUp(mounterArgs volume.MounterArgs) (volumetypes.OperationStatus, error) { - err := v.SetUpAt(v.GetPath(), mounterArgs) - return volumetypes.OperationFinished, err +func (v *sioVolume) SetUp(mounterArgs volume.MounterArgs) error { + return v.SetUpAt(v.GetPath(), mounterArgs) } // SetUp bind mounts the disk global mount to the volume path. diff --git a/pkg/volume/scaleio/sio_volume_test.go b/pkg/volume/scaleio/sio_volume_test.go index e3ac12440d5..ca871817589 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/BUILD b/pkg/volume/secret/BUILD index a34ad5fd998..cb168c9532d 100644 --- a/pkg/volume/secret/BUILD +++ b/pkg/volume/secret/BUILD @@ -16,7 +16,6 @@ go_library( deps = [ "//pkg/volume:go_default_library", "//pkg/volume/util:go_default_library", - "//pkg/volume/util/types:go_default_library", "//staging/src/k8s.io/api/core/v1:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/api/errors:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library", diff --git a/pkg/volume/secret/secret.go b/pkg/volume/secret/secret.go index dbaab27995d..3eee6d91827 100644 --- a/pkg/volume/secret/secret.go +++ b/pkg/volume/secret/secret.go @@ -29,7 +29,6 @@ import ( "k8s.io/apimachinery/pkg/types" "k8s.io/kubernetes/pkg/volume" volumeutil "k8s.io/kubernetes/pkg/volume/util" - volumetypes "k8s.io/kubernetes/pkg/volume/util/types" ) // ProbeVolumePlugins is the entry point for plugin detection in a package. @@ -176,9 +175,8 @@ func (b *secretVolumeMounter) CanMount() error { return nil } -func (b *secretVolumeMounter) SetUp(mounterArgs volume.MounterArgs) (volumetypes.OperationStatus, error) { - err := b.SetUpAt(b.GetPath(), mounterArgs) - return volumetypes.OperationFinished, err +func (b *secretVolumeMounter) SetUp(mounterArgs volume.MounterArgs) error { + return b.SetUpAt(b.GetPath(), mounterArgs) } func (b *secretVolumeMounter) SetUpAt(dir string, mounterArgs volume.MounterArgs) error { diff --git a/pkg/volume/secret/secret_test.go b/pkg/volume/secret/secret_test.go index fa7e74fc5ee..7d60d7b7d6c 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/BUILD b/pkg/volume/storageos/BUILD index eea84684a13..8d1b5730cbe 100644 --- a/pkg/volume/storageos/BUILD +++ b/pkg/volume/storageos/BUILD @@ -17,7 +17,6 @@ go_library( deps = [ "//pkg/volume:go_default_library", "//pkg/volume/util:go_default_library", - "//pkg/volume/util/types:go_default_library", "//staging/src/k8s.io/api/core/v1:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/api/resource:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library", diff --git a/pkg/volume/storageos/storageos.go b/pkg/volume/storageos/storageos.go index 27ac988868f..a90e384e9a6 100644 --- a/pkg/volume/storageos/storageos.go +++ b/pkg/volume/storageos/storageos.go @@ -36,7 +36,6 @@ import ( volumehelpers "k8s.io/cloud-provider/volume/helpers" "k8s.io/kubernetes/pkg/volume" "k8s.io/kubernetes/pkg/volume/util" - volumetypes "k8s.io/kubernetes/pkg/volume/util/types" ) // ProbeVolumePlugins is the primary entrypoint for volume plugins. @@ -344,40 +343,37 @@ func (b *storageosMounter) CanMount() error { } // SetUp attaches the disk and bind mounts to the volume path. -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) +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 } - return volumetypes.OperationFinished, internalSetup() + + 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) } // 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 3758c9debcc..d26f24af7c5 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 b027fa673d0..0bdad8924ba 100644 --- a/pkg/volume/testing/testing.go +++ b/pkg/volume/testing/testing.go @@ -88,6 +88,14 @@ const ( SuccessAndTimeoutDeviceName = "success-and-timeout-device-name" // SuccessAndFailOnMountDeviceName will cause first mount operation to succeed but subsequent attempts to fail SuccessAndFailOnMountDeviceName = "success-and-failed-mount-device-name" + + deviceNotMounted = "deviceNotMounted" + deviceMountUncertain = "deviceMountUncertain" + deviceMounted = "deviceMounted" + + volumeNotMounted = "volumeNotMounted" + volumeMountUncertain = "volumeMountUncertain" + volumeMounted = "volumeMounted" ) // fakeVolumeHost is useful for testing volume plugins. @@ -406,8 +414,8 @@ func (plugin *FakeVolumePlugin) getFakeVolume(list *[]*FakeVolume) *FakeVolume { UnmountDeviceHook: plugin.UnmountDeviceHook, } volume.VolumesAttached = make(map[string]types.NodeName) - volume.DeviceMountState = make(map[string]volumetypes.OperationStatus) - volume.VolumeMountState = make(map[string]volumetypes.OperationStatus) + volume.DeviceMountState = make(map[string]string) + volume.VolumeMountState = make(map[string]string) *list = append(*list, volume) return volume } @@ -810,8 +818,8 @@ type FakeVolume struct { Plugin *FakeVolumePlugin MetricsNil VolumesAttached map[string]types.NodeName - DeviceMountState map[string]volumetypes.OperationStatus - VolumeMountState map[string]volumetypes.OperationStatus + DeviceMountState map[string]string + VolumeMountState map[string]string // Add callbacks as needed WaitForAttachHook func(spec *Spec, devicePath string, pod *v1.Pod, spectimeout time.Duration) (string, error) @@ -859,34 +867,32 @@ func (fv *FakeVolume) CanMount() error { return nil } -func (fv *FakeVolume) SetUp(mounterArgs MounterArgs) (volumetypes.OperationStatus, error) { +func (fv *FakeVolume) SetUp(mounterArgs MounterArgs) error { fv.Lock() defer fv.Unlock() err := fv.setupInternal(mounterArgs) fv.SetUpCallCount++ - if volumetypes.IsOperationTimeOutError(err) { - return volumetypes.OperationInProgress, err - } - return volumetypes.OperationFinished, err + return err } func (fv *FakeVolume) setupInternal(mounterArgs MounterArgs) error { if fv.VolName == TimeoutOnSetupVolumeName { - fv.VolumeMountState[fv.VolName] = volumetypes.OperationInProgress - return volumetypes.NewOperationTimedOutError("time out on setup") + fv.VolumeMountState[fv.VolName] = volumeMountUncertain + return volumetypes.NewUncertainProgressError("time out on setup") } if fv.VolName == FailOnSetupVolumeName { + fv.VolumeMountState[fv.VolName] = volumeNotMounted return fmt.Errorf("mounting volume failed") } if fv.VolName == TimeoutAndFailOnSetupVolumeName { _, ok := fv.VolumeMountState[fv.VolName] if !ok { - fv.VolumeMountState[fv.VolName] = volumetypes.OperationInProgress - return volumetypes.NewOperationTimedOutError("time out on setup") + fv.VolumeMountState[fv.VolName] = volumeMountUncertain + return volumetypes.NewUncertainProgressError("time out on setup") } - fv.VolumeMountState[fv.VolName] = volumetypes.OperationFinished + fv.VolumeMountState[fv.VolName] = volumeNotMounted return fmt.Errorf("mounting volume failed") } @@ -894,7 +900,7 @@ func (fv *FakeVolume) setupInternal(mounterArgs MounterArgs) error { if fv.VolName == SuccessAndFailOnSetupVolumeName { _, ok := fv.VolumeMountState[fv.VolName] if ok { - fv.VolumeMountState[fv.VolName] = volumetypes.OperationFinished + fv.VolumeMountState[fv.VolName] = volumeNotMounted return fmt.Errorf("mounting volume failed") } } @@ -902,13 +908,12 @@ func (fv *FakeVolume) setupInternal(mounterArgs MounterArgs) error { if fv.VolName == SuccessAndTimeoutSetupVolumeName { _, ok := fv.VolumeMountState[fv.VolName] if ok { - fv.VolumeMountState[fv.VolName] = volumetypes.OperationInProgress - return volumetypes.NewOperationTimedOutError("time out on setup") + fv.VolumeMountState[fv.VolName] = volumeMountUncertain + return volumetypes.NewUncertainProgressError("time out on setup") } } - fv.VolumeMountState[fv.VolName] = volumetypes.OperationFinished - + fv.VolumeMountState[fv.VolName] = volumeNotMounted return fv.SetUpAt(fv.getPath(), mounterArgs) } @@ -1113,30 +1118,30 @@ func (fv *FakeVolume) mountDeviceInternal(spec *Spec, devicePath string, deviceM fv.Lock() defer fv.Unlock() if spec.Name() == TimeoutOnMountDeviceVolumeName { - fv.DeviceMountState[spec.Name()] = volumetypes.OperationInProgress - return volumetypes.NewOperationTimedOutError("error mounting device") + fv.DeviceMountState[spec.Name()] = deviceMountUncertain + return volumetypes.NewUncertainProgressError("mount failed") } if spec.Name() == FailMountDeviceVolumeName { - fv.DeviceMountState[spec.Name()] = volumetypes.OperationFinished + fv.DeviceMountState[spec.Name()] = deviceNotMounted return fmt.Errorf("error mounting disk: %s", devicePath) } if spec.Name() == TimeoutAndFailOnMountDeviceVolumeName { _, ok := fv.DeviceMountState[spec.Name()] if !ok { - fv.DeviceMountState[spec.Name()] = volumetypes.OperationInProgress - return volumetypes.NewOperationTimedOutError("timed out mounting error") + fv.DeviceMountState[spec.Name()] = deviceMountUncertain + return volumetypes.NewUncertainProgressError("timed out mounting error") } - fv.DeviceMountState[spec.Name()] = volumetypes.OperationFinished + fv.DeviceMountState[spec.Name()] = deviceNotMounted return fmt.Errorf("error mounting disk: %s", devicePath) } if spec.Name() == SuccessAndTimeoutDeviceName { _, ok := fv.DeviceMountState[spec.Name()] if ok { - fv.DeviceMountState[spec.Name()] = volumetypes.OperationInProgress - return volumetypes.NewOperationTimedOutError("error mounting state") + fv.DeviceMountState[spec.Name()] = deviceMountUncertain + return volumetypes.NewUncertainProgressError("error mounting state") } } @@ -1146,17 +1151,13 @@ func (fv *FakeVolume) mountDeviceInternal(spec *Spec, devicePath string, deviceM return fmt.Errorf("error mounting disk: %s", devicePath) } } - fv.DeviceMountState[spec.Name()] = volumetypes.OperationFinished + fv.DeviceMountState[spec.Name()] = deviceMounted fv.MountDeviceCallCount++ return nil } -func (fv *FakeVolume) MountDevice(spec *Spec, devicePath string, deviceMountPath string) (volumetypes.OperationStatus, error) { - err := fv.mountDeviceInternal(spec, devicePath, deviceMountPath) - if volumetypes.IsOperationTimeOutError(err) { - return volumetypes.OperationInProgress, err - } - return volumetypes.OperationFinished, err +func (fv *FakeVolume) MountDevice(spec *Spec, devicePath string, deviceMountPath string) error { + return fv.mountDeviceInternal(spec, devicePath, deviceMountPath) } func (fv *FakeVolume) GetMountDeviceCallCount() int { diff --git a/pkg/volume/util/operationexecutor/operation_generator.go b/pkg/volume/util/operationexecutor/operation_generator.go index 7be76b9e34d..d453c049608 100644 --- a/pkg/volume/util/operationexecutor/operation_generator.go +++ b/pkg/volume/util/operationexecutor/operation_generator.go @@ -575,12 +575,12 @@ func (og *operationGenerator) GenerateMountVolumeFunc( } // Mount device to global mount path - operationState, err := volumeDeviceMounter.MountDevice( + err = volumeDeviceMounter.MountDevice( volumeToMount.VolumeSpec, devicePath, deviceMountPath) if err != nil { - og.markDeviceErrorState(volumeToMount, devicePath, deviceMountPath, operationState, actualStateOfWorld) + og.markDeviceErrorState(volumeToMount, devicePath, deviceMountPath, err, actualStateOfWorld) // On failure, return error. Caller will log and retry. return volumeToMount.GenerateError("MountVolume.MountDevice failed", err) } @@ -618,7 +618,7 @@ func (og *operationGenerator) GenerateMountVolumeFunc( } // Execute mount - opExitStatus, mountErr := volumeMounter.SetUp(volume.MounterArgs{ + mountErr := volumeMounter.SetUp(volume.MounterArgs{ FsGroup: fsGroup, DesiredSize: volumeToMount.DesiredSizeLimit, }) @@ -630,11 +630,11 @@ func (og *operationGenerator) GenerateMountVolumeFunc( Mounter: volumeMounter, OuterVolumeSpecName: volumeToMount.OuterVolumeSpecName, VolumeGidVolume: volumeToMount.VolumeGidValue, - VolumeSpec: originalSpec, + VolumeSpec: volumeToMount.VolumeSpec, VolumeMountState: VolumeMounted, } if mountErr != nil { - og.markVolumeErrorState(volumeToMount, markOpts, opExitStatus, actualStateOfWorld) + og.markVolumeErrorState(volumeToMount, markOpts, mountErr, actualStateOfWorld) // On failure, return error. Caller will log and retry. return volumeToMount.GenerateError("MountVolume.SetUp failed", mountErr) } @@ -660,17 +660,6 @@ func (og *operationGenerator) GenerateMountVolumeFunc( } } - // Update actual state of world - markOpts := MarkVolumeMountedOpts{ - PodName: volumeToMount.PodName, - PodUID: volumeToMount.Pod.UID, - VolumeName: volumeToMount.VolumeName, - Mounter: volumeMounter, - OuterVolumeSpecName: volumeToMount.OuterVolumeSpecName, - VolumeGidVolume: volumeToMount.VolumeGidValue, - VolumeSpec: volumeToMount.VolumeSpec, - VolumeMountState: VolumeMounted, - } markVolMountedErr := actualStateOfWorld.MarkVolumeAsMounted(markOpts) if markVolMountedErr != nil { // On failure, return error. Caller will log and retry. @@ -694,45 +683,47 @@ func (og *operationGenerator) GenerateMountVolumeFunc( } } -func (og *operationGenerator) markDeviceErrorState(volumeToMount VolumeToMount, devicePath, deviceMountPath string, operationState volumetypes.OperationStatus, actualStateOfWorld ActualStateOfWorldMounterUpdater) { - switch operationState { - case volumetypes.OperationInProgress: +func (og *operationGenerator) markDeviceErrorState(volumeToMount VolumeToMount, devicePath, deviceMountPath string, mountError error, actualStateOfWorld ActualStateOfWorldMounterUpdater) { + if volumetypes.IsOperationFinishedError(mountError) && + actualStateOfWorld.GetDeviceMountState(volumeToMount.VolumeName) == DeviceMountUncertain { + // Only devices which were uncertain can be marked as unmounted + markDeviceUnmountError := actualStateOfWorld.MarkDeviceAsUnmounted(volumeToMount.VolumeName) + if markDeviceUnmountError != nil { + klog.Errorf(volumeToMount.GenerateErrorDetailed("MountDevice.MarkDeviceAsUnmounted failed", markDeviceUnmountError).Error()) + } + return + } + + if volumetypes.IsUncertainProgressError(mountError) && + actualStateOfWorld.GetDeviceMountState(volumeToMount.VolumeName) == DeviceNotMounted { // only devices which are not mounted can be marked as uncertain. We do not want to mark a device // which was previously marked as mounted here as uncertain. - if actualStateOfWorld.GetDeviceMountState(volumeToMount.VolumeName) == DeviceNotMounted { - markDeviceUncertainError := actualStateOfWorld.MarkDeviceAsUncertain(volumeToMount.VolumeName, devicePath, deviceMountPath) - if markDeviceUncertainError != nil { - klog.Errorf(volumeToMount.GenerateErrorDetailed("MountDevice.MarkDeviceAsUncertain failed", markDeviceUncertainError).Error()) - } - } - case volumetypes.OperationFinished: - // Similarly only devices which were uncertain can be marked as unmounted - if actualStateOfWorld.GetDeviceMountState(volumeToMount.VolumeName) == DeviceMountUncertain { - markDeviceUnmountError := actualStateOfWorld.MarkDeviceAsUnmounted(volumeToMount.VolumeName) - if markDeviceUnmountError != nil { - klog.Errorf(volumeToMount.GenerateErrorDetailed("MountDevice.MarkDeviceAsUnmounted failed", markDeviceUnmountError).Error()) - } + markDeviceUncertainError := actualStateOfWorld.MarkDeviceAsUncertain(volumeToMount.VolumeName, devicePath, deviceMountPath) + if markDeviceUncertainError != nil { + klog.Errorf(volumeToMount.GenerateErrorDetailed("MountDevice.MarkDeviceAsUncertain failed", markDeviceUncertainError).Error()) } } + } -func (og *operationGenerator) markVolumeErrorState(volumeToMount VolumeToMount, markOpts MarkVolumeOpts, operationState volumetypes.OperationStatus, actualStateOfWorld ActualStateOfWorldMounterUpdater) { - switch operationState { - case volumetypes.OperationInProgress: - if actualStateOfWorld.GetVolumeMountState(volumeToMount.VolumeName, markOpts.PodName) == VolumeNotMounted { - t := actualStateOfWorld.MarkVolumeMountAsUncertain(markOpts) - if t != nil { - klog.Errorf(volumeToMount.GenerateErrorDetailed("MountVolume.MarkVolumeMountAsUncertain failed", t).Error()) - } +func (og *operationGenerator) markVolumeErrorState(volumeToMount VolumeToMount, markOpts MarkVolumeOpts, mountError error, actualStateOfWorld ActualStateOfWorldMounterUpdater) { + if volumetypes.IsOperationFinishedError(mountError) && + actualStateOfWorld.GetVolumeMountState(volumeToMount.VolumeName, markOpts.PodName) == VolumeMountUncertain { + t := actualStateOfWorld.MarkVolumeAsUnmounted(volumeToMount.PodName, volumeToMount.VolumeName) + if t != nil { + klog.Errorf(volumeToMount.GenerateErrorDetailed("MountVolume.MarkVolumeAsUnmounted failed", t).Error()) } - case volumetypes.OperationFinished: - if actualStateOfWorld.GetVolumeMountState(volumeToMount.VolumeName, markOpts.PodName) == VolumeMountUncertain { - t := actualStateOfWorld.MarkVolumeAsUnmounted(volumeToMount.PodName, volumeToMount.VolumeName) - if t != nil { - klog.Errorf(volumeToMount.GenerateErrorDetailed("MountVolume.MarkVolumeAsUnmounted failed", t).Error()) - } + return + } + + if volumetypes.IsUncertainProgressError(mountError) && + actualStateOfWorld.GetVolumeMountState(volumeToMount.VolumeName, markOpts.PodName) == VolumeNotMounted { + t := actualStateOfWorld.MarkVolumeMountAsUncertain(markOpts) + if t != nil { + klog.Errorf(volumeToMount.GenerateErrorDetailed("MountVolume.MarkVolumeMountAsUncertain failed", t).Error()) } } + } func (og *operationGenerator) GenerateUnmountVolumeFunc( diff --git a/pkg/volume/util/types/types.go b/pkg/volume/util/types/types.go index 4cbcd2c22f3..199365c3520 100644 --- a/pkg/volume/util/types/types.go +++ b/pkg/volume/util/types/types.go @@ -51,44 +51,52 @@ func (o *GeneratedOperations) Run() (eventErr, detailedErr error) { return o.OperationFunc() } -// OperationStatus is used to store status of a volume operation -type OperationStatus string - -const ( - // OperationFinished means volume operation has been finished - OperationFinished OperationStatus = "Finished" - - // OperationInProgress means volume operation has been started and - // is in-progress. This state does not indicate if operation will succeed or fail but - // merely it has been started and is in-progress. - OperationInProgress OperationStatus = "InProgress" - - // OperationStateNoChange indicates it is unchanged from previous state. - // This can be used to indicate transient failures for an operation which - // was in-progress previously. - OperationStateNoChange OperationStatus = "NoChange" -) - -// OperationTimedOutError indicates a particular volume operation has timed out. -type OperationTimedOutError struct { +// TransientOperationFailure indicates operation failed with a transient error +// and may fix itself when retried. +type TransientOperationFailure struct { msg string } -func (err *OperationTimedOutError) Error() string { +func (err *TransientOperationFailure) Error() string { return err.msg } -// NewOperationTimedOutError returns a new instance of OperationTimedOutError -func NewOperationTimedOutError(msg string) *OperationTimedOutError { - return &OperationTimedOutError{ - msg: msg, - } +// NewTransientOperationFailure creates an instance of TransientOperationFailure error +func NewTransientOperationFailure(msg string) *TransientOperationFailure { + return &TransientOperationFailure{msg: msg} } -// IsOperationTimeOutError returns true if volume operation could have timed out for client but possibly -// still running or being processed by the volume plugin. -func IsOperationTimeOutError(err error) bool { - if _, ok := err.(*OperationTimedOutError); ok { +// UncertainProgressError indicates operation failed with a non-final error +// and operation may be in-progress in background. +type UncertainProgressError struct { + msg string +} + +func (err *UncertainProgressError) Error() string { + return err.msg +} + +// NewUncertainProgressError creates an instance of UncertainProgressError type +func NewUncertainProgressError(msg string) *UncertainProgressError { + return &UncertainProgressError{msg: msg} +} + +// IsOperationFinishedError checks if given error is of type that indicates +// operation is finished with an error. +func IsOperationFinishedError(err error) bool { + if _, ok := err.(*UncertainProgressError); ok { + return false + } + if _, ok := err.(*TransientOperationFailure); ok { + return false + } + return true +} + +// IsUncertainProgressError checks if given error is of type that indicates +// operation might be in-progress in background. +func IsUncertainProgressError(err error) bool { + if _, ok := err.(*UncertainProgressError); ok { return true } return false diff --git a/pkg/volume/volume.go b/pkg/volume/volume.go index c751f9c959b..0efabf92508 100644 --- a/pkg/volume/volume.go +++ b/pkg/volume/volume.go @@ -19,11 +19,10 @@ package volume import ( "time" - "k8s.io/api/core/v1" + v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" - volumetypes "k8s.io/kubernetes/pkg/volume/util/types" ) // Volume represents a directory used by pods or hosts on a node. All method @@ -129,7 +128,11 @@ 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) (volumetypes.OperationStatus, error) + // It could return following types of errors: + // - TransientOperationFailure + // - UncertainProgressError + // - Error of any other type should be considered a final error + SetUp(mounterArgs MounterArgs) error // SetUpAt prepares and mounts/unpacks the volume to the // specified directory path, which may or may not exist yet. @@ -249,7 +252,11 @@ type DeviceMounter interface { // MountDevice mounts the disk to a global path which // individual pods can then bind mount // Note that devicePath can be empty if the volume plugin does not implement any of Attach and WaitForAttach methods. - MountDevice(spec *Spec, devicePath string, deviceMountPath string) (volumetypes.OperationStatus, error) + // It could return following types of errors: + // - TransientOperationFailure + // - UncertainProgressError + // - Error of any other type should be considered a final error + MountDevice(spec *Spec, devicePath string, deviceMountPath string) error } type BulkVolumeVerifier interface { diff --git a/pkg/volume/vsphere_volume/BUILD b/pkg/volume/vsphere_volume/BUILD index a2f466175d9..b4bab5c5ef9 100644 --- a/pkg/volume/vsphere_volume/BUILD +++ b/pkg/volume/vsphere_volume/BUILD @@ -23,7 +23,6 @@ go_library( "//pkg/features:go_default_library", "//pkg/volume:go_default_library", "//pkg/volume/util:go_default_library", - "//pkg/volume/util/types:go_default_library", "//pkg/volume/util/volumepathhandler:go_default_library", "//staging/src/k8s.io/api/core/v1:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/api/resource:go_default_library", diff --git a/pkg/volume/vsphere_volume/attacher.go b/pkg/volume/vsphere_volume/attacher.go index 3f5e56142b7..8c247167858 100644 --- a/pkg/volume/vsphere_volume/attacher.go +++ b/pkg/volume/vsphere_volume/attacher.go @@ -33,7 +33,6 @@ import ( "k8s.io/apimachinery/pkg/types" "k8s.io/kubernetes/pkg/volume" volumeutil "k8s.io/kubernetes/pkg/volume/util" - volumetypes "k8s.io/kubernetes/pkg/volume/util/types" "k8s.io/legacy-cloud-providers/vsphere" ) @@ -208,7 +207,8 @@ func (plugin *vsphereVolumePlugin) GetDeviceMountRefs(deviceMountPath string) ([ return mounter.GetMountRefs(deviceMountPath) } -func (attacher *vsphereVMDKAttacher) mountDeviceInternal(spec *volume.Spec, devicePath string, deviceMountPath string) error { +// MountDevice mounts device to global mount point. +func (attacher *vsphereVMDKAttacher) MountDevice(spec *volume.Spec, devicePath string, deviceMountPath string) error { klog.Info("vsphere MountDevice", devicePath, deviceMountPath) mounter := attacher.host.GetMounter(vsphereVolumePluginName) notMnt, err := mounter.IsLikelyNotMountPoint(deviceMountPath) @@ -248,12 +248,6 @@ func (attacher *vsphereVMDKAttacher) mountDeviceInternal(spec *volume.Spec, devi return nil } -// MountDevice mounts device to global mount point. -func (attacher *vsphereVMDKAttacher) MountDevice(spec *volume.Spec, devicePath string, deviceMountPath string) (volumetypes.OperationStatus, error) { - err := attacher.mountDeviceInternal(spec, devicePath, deviceMountPath) - return volumetypes.OperationFinished, err -} - type vsphereVMDKDetacher struct { mounter mount.Interface vsphereVolumes vsphere.Volumes diff --git a/pkg/volume/vsphere_volume/vsphere_volume.go b/pkg/volume/vsphere_volume/vsphere_volume.go index 2d68ee6136f..2d023da3b1f 100644 --- a/pkg/volume/vsphere_volume/vsphere_volume.go +++ b/pkg/volume/vsphere_volume/vsphere_volume.go @@ -38,7 +38,7 @@ import ( "k8s.io/kubernetes/pkg/features" "k8s.io/kubernetes/pkg/volume" "k8s.io/kubernetes/pkg/volume/util" - volumetypes "k8s.io/kubernetes/pkg/volume/util/types" +) // This is the primary entrypoint for volume plugins. func ProbeVolumePlugins() []volume.VolumePlugin { @@ -213,9 +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) (volumetypes.OperationStatus, error) { - err := b.SetUpAt(b.GetPath(), mounterArgs) - return volumetypes.OperationFinished, err +func (b *vsphereVolumeMounter) SetUp(mounterArgs volume.MounterArgs) error { + return b.SetUpAt(b.GetPath(), mounterArgs) } // Checks prior to mount operations to verify that the required components (binaries, etc.) diff --git a/pkg/volume/vsphere_volume/vsphere_volume_test.go b/pkg/volume/vsphere_volume/vsphere_volume_test.go index ac282ea3cf6..ae59c319813 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) }