diff --git a/hack/.golint_failures b/hack/.golint_failures index 347bc3ea36..ddc2440969 100644 --- a/hack/.golint_failures +++ b/hack/.golint_failures @@ -390,7 +390,6 @@ pkg/volume/configmap pkg/volume/csi/fake pkg/volume/empty_dir pkg/volume/fc -pkg/volume/flexvolume pkg/volume/flocker pkg/volume/gce_pd pkg/volume/git_repo diff --git a/pkg/volume/flexvolume/attacher.go b/pkg/volume/flexvolume/attacher.go index f7767d3894..4d2169f36f 100644 --- a/pkg/volume/flexvolume/attacher.go +++ b/pkg/volume/flexvolume/attacher.go @@ -91,9 +91,8 @@ func (a *flexVolumeAttacher) MountDevice(spec *volume.Spec, devicePath string, d // plugin does not implement attach interface. if devicePath != "" { return (*attacherDefaults)(a).MountDevice(spec, devicePath, deviceMountPath, a.plugin.host.GetMounter(a.plugin.GetPluginName())) - } else { - return nil } + return nil } return err } diff --git a/pkg/volume/flexvolume/attacher_test.go b/pkg/volume/flexvolume/attacher_test.go index f4b4ab7b83..8472886dad 100644 --- a/pkg/volume/flexvolume/attacher_test.go +++ b/pkg/volume/flexvolume/attacher_test.go @@ -30,7 +30,7 @@ func TestAttach(t *testing.T) { plugin, _ := testPlugin() plugin.runner = fakeRunner( assertDriverCall(t, notSupportedOutput(), attachCmd, - specJson(plugin, spec, nil), "localhost"), + specJSON(plugin, spec, nil), "localhost"), ) a, _ := plugin.NewAttacher() @@ -43,7 +43,7 @@ func TestWaitForAttach(t *testing.T) { plugin, _ := testPlugin() plugin.runner = fakeRunner( assertDriverCall(t, notSupportedOutput(), waitForAttachCmd, "/dev/sdx", - specJson(plugin, spec, nil)), + specJSON(plugin, spec, nil)), ) a, _ := plugin.NewAttacher() @@ -56,7 +56,7 @@ func TestMountDevice(t *testing.T) { plugin, rootDir := testPlugin() plugin.runner = fakeRunner( assertDriverCall(t, notSupportedOutput(), mountDeviceCmd, rootDir+"/mount-dir", "/dev/sdx", - specJson(plugin, spec, nil)), + specJSON(plugin, spec, nil)), ) a, _ := plugin.NewAttacher() @@ -68,7 +68,7 @@ func TestIsVolumeAttached(t *testing.T) { plugin, _ := testPlugin() plugin.runner = fakeRunner( - assertDriverCall(t, notSupportedOutput(), isAttached, specJson(plugin, spec, nil), "localhost"), + assertDriverCall(t, notSupportedOutput(), isAttached, specJSON(plugin, spec, nil), "localhost"), ) a, _ := plugin.NewAttacher() specs := []*volume.Spec{spec} diff --git a/pkg/volume/flexvolume/common_test.go b/pkg/volume/flexvolume/common_test.go index 9300c2a765..fb99d1add4 100644 --- a/pkg/volume/flexvolume/common_test.go +++ b/pkg/volume/flexvolume/common_test.go @@ -129,7 +129,7 @@ func fakePersistentVolumeSpec() *volume.Spec { return volume.NewSpecFromPersistentVolume(vol, false) } -func specJson(plugin *flexVolumeAttachablePlugin, spec *volume.Spec, extraOptions map[string]string) string { +func specJSON(plugin *flexVolumeAttachablePlugin, spec *volume.Spec, extraOptions map[string]string) string { o, err := NewOptionsForDriver(spec, plugin.host, extraOptions) if err != nil { panic("Failed to convert spec: " + err.Error()) diff --git a/pkg/volume/flexvolume/driver-call.go b/pkg/volume/flexvolume/driver-call.go index bd5a5cdb4f..d2706ffa0e 100644 --- a/pkg/volume/flexvolume/driver-call.go +++ b/pkg/volume/flexvolume/driver-call.go @@ -67,7 +67,7 @@ const ( ) var ( - TimeoutError = fmt.Errorf("Timeout") + errTimeout = fmt.Errorf("Timeout") ) // DriverCall implements the basic contract between FlexVolume and its driver. @@ -92,10 +92,12 @@ func (plugin *flexVolumePlugin) NewDriverCallWithTimeout(command string, timeout } } +// Append appends arg into driver call argument list func (dc *DriverCall) Append(arg string) { dc.args = append(dc.args, arg) } +// AppendSpec appends volume spec to driver call argument list func (dc *DriverCall) AppendSpec(spec *volume.Spec, host volume.VolumeHost, extraOptions map[string]string) error { optionsForDriver, err := NewOptionsForDriver(spec, host, extraOptions) if err != nil { @@ -111,6 +113,7 @@ func (dc *DriverCall) AppendSpec(spec *volume.Spec, host volume.VolumeHost, extr return nil } +// Run executes the driver call func (dc *DriverCall) Run() (*DriverStatus, error) { if dc.plugin.isUnsupported(dc.Command) { return nil, errors.New(StatusNotSupported) @@ -131,7 +134,7 @@ func (dc *DriverCall) Run() (*DriverStatus, error) { output, execErr := cmd.CombinedOutput() if execErr != nil { if timeout { - return nil, TimeoutError + return nil, errTimeout } _, err := handleCmdResponse(dc.Command, output) if err == nil { @@ -160,6 +163,7 @@ func (dc *DriverCall) Run() (*DriverStatus, error) { // OptionsForDriver represents the spec given to the driver. type OptionsForDriver map[string]string +// NewOptionsForDriver create driver options given volume spec func NewOptionsForDriver(spec *volume.Spec, host volume.VolumeHost, extraOptions map[string]string) (OptionsForDriver, error) { volSourceFSType, err := getFSType(spec) @@ -219,6 +223,7 @@ type DriverStatus struct { Capabilities *DriverCapabilities `json:",omitempty"` } +// DriverCapabilities represents what driver can do type DriverCapabilities struct { Attach bool `json:"attach"` SELinuxRelabel bool `json:"selinuxRelabel"` diff --git a/pkg/volume/flexvolume/fake_watcher.go b/pkg/volume/flexvolume/fake_watcher.go index b427f379ea..ce834043fb 100644 --- a/pkg/volume/flexvolume/fake_watcher.go +++ b/pkg/volume/flexvolume/fake_watcher.go @@ -29,7 +29,7 @@ type fakeWatcher struct { var _ utilfs.FSWatcher = &fakeWatcher{} -func NewFakeWatcher() *fakeWatcher { +func newFakeWatcher() *fakeWatcher { return &fakeWatcher{ watches: nil, } diff --git a/pkg/volume/flexvolume/mounter_test.go b/pkg/volume/flexvolume/mounter_test.go index 5d08971c3f..70dac00728 100644 --- a/pkg/volume/flexvolume/mounter_test.go +++ b/pkg/volume/flexvolume/mounter_test.go @@ -44,7 +44,7 @@ func TestSetUpAt(t *testing.T) { plugin.runner = fakeRunner( // first call without fsGroup assertDriverCall(t, successOutput(), mountCmd, rootDir+"/mount-dir", - specJson(plugin, spec, map[string]string{ + specJSON(plugin, spec, map[string]string{ optionKeyPodName: "my-pod", optionKeyPodNamespace: "my-ns", optionKeyPodUID: "my-uid", @@ -53,7 +53,7 @@ func TestSetUpAt(t *testing.T) { // second test has fsGroup assertDriverCall(t, notSupportedOutput(), mountCmd, rootDir+"/mount-dir", - specJson(plugin, spec, map[string]string{ + specJSON(plugin, spec, map[string]string{ optionFSGroup: "42", optionKeyPodName: "my-pod", optionKeyPodNamespace: "my-ns", @@ -61,7 +61,7 @@ func TestSetUpAt(t *testing.T) { optionKeyServiceAccountName: "my-sa", })), assertDriverCall(t, fakeVolumeNameOutput("sdx"), getVolumeNameCmd, - specJson(plugin, spec, nil)), + specJSON(plugin, spec, nil)), ) m, _ := plugin.newMounterInternal(spec, pod, mounter, plugin.runner) diff --git a/pkg/volume/flexvolume/plugin.go b/pkg/volume/flexvolume/plugin.go index 6ddd455490..9e59b5ccfe 100644 --- a/pkg/volume/flexvolume/plugin.go +++ b/pkg/volume/flexvolume/plugin.go @@ -60,6 +60,7 @@ var _ volume.PersistentVolumePlugin = &flexVolumePlugin{} var _ volume.DeviceMountableVolumePlugin = &flexVolumeAttachablePlugin{} +// PluginFactory create flex volume plugin type PluginFactory interface { NewFlexVolumePlugin(pluginDir, driverName string, runner exec.Interface) (volume.VolumePlugin, error) } @@ -89,9 +90,8 @@ func (pluginFactory) NewFlexVolumePlugin(pluginDir, name string, runner exec.Int if flexPlugin.capabilities.Attach { // Plugin supports attach/detach, so return flexVolumeAttachablePlugin return &flexVolumeAttachablePlugin{flexVolumePlugin: flexPlugin}, nil - } else { - return flexPlugin, nil } + return flexPlugin, nil } // Init is part of the volume.VolumePlugin interface. diff --git a/pkg/volume/flexvolume/plugin_test.go b/pkg/volume/flexvolume/plugin_test.go index 17c540a874..ae267fbd9f 100644 --- a/pkg/volume/flexvolume/plugin_test.go +++ b/pkg/volume/flexvolume/plugin_test.go @@ -42,7 +42,7 @@ func TestGetVolumeName(t *testing.T) { plugin, _ := testPlugin() plugin.runner = fakeRunner( assertDriverCall(t, fakeVolumeNameOutput(spec.Name()), getVolumeNameCmd, - specJson(plugin, spec, nil)), + specJSON(plugin, spec, nil)), ) name, err := plugin.GetVolumeName(spec) diff --git a/pkg/volume/flexvolume/probe.go b/pkg/volume/flexvolume/probe.go index 40d2dd3300..2a792c6f40 100644 --- a/pkg/volume/flexvolume/probe.go +++ b/pkg/volume/flexvolume/probe.go @@ -46,6 +46,7 @@ type flexVolumeProber struct { eventsMap map[string]volume.ProbeOperation // the key is the driver directory path, the value is the coresponding operation } +// GetDynamicPluginProber creates dynamic plugin prober func GetDynamicPluginProber(pluginDir string, runner exec.Interface) volume.DynamicPluginProber { return &flexVolumeProber{ pluginDir: pluginDir, diff --git a/pkg/volume/flexvolume/probe_test.go b/pkg/volume/flexvolume/probe_test.go index 305c370f6e..fe3590f2f8 100644 --- a/pkg/volume/flexvolume/probe_test.go +++ b/pkg/volume/flexvolume/probe_test.go @@ -174,7 +174,7 @@ func TestProberAddRemoveDriver(t *testing.T) { func TestEmptyPluginDir(t *testing.T) { // Arrange fs := utilfs.NewFakeFs() - watcher := NewFakeWatcher() + watcher := newFakeWatcher() prober := &flexVolumeProber{ pluginDir: pluginDir, watcher: watcher, @@ -268,7 +268,7 @@ func TestProberMultipleEvents(t *testing.T) { func TestProberError(t *testing.T) { fs := utilfs.NewFakeFs() - watcher := NewFakeWatcher() + watcher := newFakeWatcher() prober := &flexVolumeProber{ pluginDir: pluginDir, watcher: watcher, @@ -296,7 +296,7 @@ func initTestEnvironment(t *testing.T) ( watcher *fakeWatcher, prober volume.DynamicPluginProber) { fs = utilfs.NewFakeFs() - watcher = NewFakeWatcher() + watcher = newFakeWatcher() prober = &flexVolumeProber{ pluginDir: pluginDir, watcher: watcher, diff --git a/pkg/volume/flexvolume/util.go b/pkg/volume/flexvolume/util.go index b706712101..1efdb92112 100644 --- a/pkg/volume/flexvolume/util.go +++ b/pkg/volume/flexvolume/util.go @@ -55,7 +55,7 @@ func addSecretsToOptions(options map[string]string, spec *volume.Spec, namespace return nil } -var notFlexVolume = fmt.Errorf("not a flex volume") +var errNotFlexVolume = fmt.Errorf("not a flex volume") func getDriver(spec *volume.Spec) (string, error) { if spec.Volume != nil && spec.Volume.FlexVolume != nil { @@ -64,7 +64,7 @@ func getDriver(spec *volume.Spec) (string, error) { if spec.PersistentVolume != nil && spec.PersistentVolume.Spec.FlexVolume != nil { return spec.PersistentVolume.Spec.FlexVolume.Driver, nil } - return "", notFlexVolume + return "", errNotFlexVolume } func getFSType(spec *volume.Spec) (string, error) { @@ -74,7 +74,7 @@ func getFSType(spec *volume.Spec) (string, error) { if spec.PersistentVolume != nil && spec.PersistentVolume.Spec.FlexVolume != nil { return spec.PersistentVolume.Spec.FlexVolume.FSType, nil } - return "", notFlexVolume + return "", errNotFlexVolume } func getSecretNameAndNamespace(spec *volume.Spec, podNamespace string) (string, string, error) { @@ -95,7 +95,7 @@ func getSecretNameAndNamespace(spec *volume.Spec, podNamespace string) (string, } return secretName, secretNamespace, nil } - return "", "", notFlexVolume + return "", "", errNotFlexVolume } func getReadOnly(spec *volume.Spec) (bool, error) { @@ -106,7 +106,7 @@ func getReadOnly(spec *volume.Spec) (bool, error) { // ReadOnly is specified at the PV level return spec.ReadOnly, nil } - return false, notFlexVolume + return false, errNotFlexVolume } func getOptions(spec *volume.Spec) (map[string]string, error) { @@ -116,7 +116,7 @@ func getOptions(spec *volume.Spec) (map[string]string, error) { if spec.PersistentVolume != nil && spec.PersistentVolume.Spec.FlexVolume != nil { return spec.PersistentVolume.Spec.FlexVolume.Options, nil } - return nil, notFlexVolume + return nil, errNotFlexVolume } func prepareForMount(mounter mount.Interface, deviceMountPath string) (bool, error) {