Skip to content

Commit

Permalink
Use typed errors for special casing volume progress
Browse files Browse the repository at this point in the history 
Use typed errors rather than operation status for
indicating operation progress
  • Loading branch information
@gnufied
gnufied committed Dec 4, 2019
1 parent 0741f6f commit 4b8e552
Show file tree
Hide file tree
Showing 100 changed files with 405 additions and 565 deletions.
1 change: 1 addition & 0 deletions hack/.staticcheck_failures
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 0 additions & 1 deletion pkg/controller/volume/attachdetach/testing/BUILD
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
7 changes: 3 additions & 4 deletions pkg/controller/volume/attachdetach/testing/testvolumespec.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
Expand Down
1 change: 0 additions & 1 deletion pkg/kubelet/BUILD
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
5 changes: 2 additions & 3 deletions pkg/kubelet/kubelet_volumes_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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 {
Expand Down
1 change: 0 additions & 1 deletion pkg/volume/BUILD
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
1 change: 0 additions & 1 deletion pkg/volume/awsebs/BUILD
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
13 changes: 6 additions & 7 deletions pkg/volume/awsebs/attacher.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand Down Expand Up @@ -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 {
Expand All @@ -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{}
Expand All @@ -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 {
Expand Down
6 changes: 2 additions & 4 deletions pkg/volume/awsebs/aws_ebs.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand Down Expand Up @@ -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.
Expand Down
4 changes: 2 additions & 2 deletions pkg/volume/awsebs/aws_ebs_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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
Expand Down
1 change: 0 additions & 1 deletion pkg/volume/azure_dd/BUILD
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
7 changes: 1 addition & 6 deletions pkg/volume/azure_dd/attacher.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand Down Expand Up @@ -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)

Expand Down
6 changes: 2 additions & 4 deletions pkg/volume/azure_dd/azure_mounter.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down
1 change: 0 additions & 1 deletion pkg/volume/azure_file/BUILD
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
6 changes: 2 additions & 4 deletions pkg/volume/azure_file/azure_file.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand Down Expand Up @@ -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 {
Expand Down
2 changes: 1 addition & 1 deletion pkg/volume/azure_file/azure_file_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
1 change: 0 additions & 1 deletion pkg/volume/cephfs/BUILD
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
6 changes: 2 additions & 4 deletions pkg/volume/cephfs/cephfs.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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.
Expand Down
2 changes: 1 addition & 1 deletion pkg/volume/cephfs/cephfs_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
1 change: 0 additions & 1 deletion pkg/volume/cinder/BUILD
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
8 changes: 1 addition & 7 deletions pkg/volume/cinder/attacher.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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
Expand Down
6 changes: 2 additions & 4 deletions pkg/volume/cinder/cinder.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand Down Expand Up @@ -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.
Expand Down
2 changes: 1 addition & 1 deletion pkg/volume/cinder/cinder_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
1 change: 0 additions & 1 deletion pkg/volume/configmap/BUILD
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
6 changes: 2 additions & 4 deletions pkg/volume/configmap/configmap.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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 {
Expand Down
Loading

0 comments on commit 4b8e552

Please sign in to comment.