From fc245b339b3a72307435f51c02e783eac7b66825 Mon Sep 17 00:00:00 2001 From: Jan Safranek Date: Wed, 19 Oct 2022 14:52:08 +0200 Subject: [PATCH] Refactor ConstructVolumeSpec Return a struct from ConstructVolumeSpec to be able to add more fields to it later. --- .../attachdetach/testing/testvolumespec.go | 6 ++- .../volume/persistentvolume/framework_test.go | 4 +- .../volumemanager/reconciler/reconstruct.go | 8 ++- .../reconciler/reconstruct_common.go | 5 +- pkg/volume/awsebs/aws_ebs.go | 12 +++-- pkg/volume/azure_file/azure_file.go | 6 ++- pkg/volume/azuredd/azure_dd.go | 10 ++-- pkg/volume/cephfs/cephfs.go | 6 ++- pkg/volume/cephfs/cephfs_test.go | 6 +-- pkg/volume/configmap/configmap.go | 6 ++- pkg/volume/csi/csi_plugin.go | 12 +++-- pkg/volume/csi/csi_plugin_test.go | 50 +++++++++---------- pkg/volume/csi/csi_test.go | 6 +-- pkg/volume/downwardapi/downwardapi.go | 6 ++- pkg/volume/emptydir/empty_dir.go | 6 ++- pkg/volume/fc/fc.go | 14 +++--- pkg/volume/flexvolume/plugin.go | 6 ++- pkg/volume/gcepd/gce_pd.go | 10 ++-- pkg/volume/git_repo/git_repo.go | 6 ++- pkg/volume/hostpath/host_path.go | 6 ++- pkg/volume/iscsi/iscsi.go | 20 ++++---- pkg/volume/local/local.go | 10 ++-- pkg/volume/local/local_test.go | 16 +++--- pkg/volume/nfs/nfs.go | 6 ++- pkg/volume/noop_expandable_plugin.go | 4 +- pkg/volume/plugins.go | 8 ++- pkg/volume/plugins_test.go | 4 +- pkg/volume/portworx/portworx.go | 6 ++- pkg/volume/projected/projected.go | 6 ++- pkg/volume/rbd/rbd.go | 14 +++--- pkg/volume/rbd/rbd_test.go | 10 ++-- pkg/volume/secret/secret.go | 6 ++- pkg/volume/testing/testing.go | 16 +++--- .../operationexecutor/operation_executor.go | 16 +++--- pkg/volume/vsphere_volume/vsphere_volume.go | 10 ++-- 35 files changed, 202 insertions(+), 141 deletions(-) diff --git a/pkg/controller/volume/attachdetach/testing/testvolumespec.go b/pkg/controller/volume/attachdetach/testing/testvolumespec.go index 0af3f0b52d4..8cee67a64cb 100644 --- a/pkg/controller/volume/attachdetach/testing/testvolumespec.go +++ b/pkg/controller/volume/attachdetach/testing/testvolumespec.go @@ -424,7 +424,7 @@ func (plugin *TestPlugin) NewUnmounter(name string, podUID types.UID) (volume.Un return nil, nil } -func (plugin *TestPlugin) ConstructVolumeSpec(volumeName, mountPath string) (*volume.Spec, error) { +func (plugin *TestPlugin) ConstructVolumeSpec(volumeName, mountPath string) (volume.ReconstructedVolume, error) { fakeVolume := &v1.Volume{ Name: volumeName, VolumeSource: v1.VolumeSource{ @@ -435,7 +435,9 @@ func (plugin *TestPlugin) ConstructVolumeSpec(volumeName, mountPath string) (*vo }, }, } - return volume.NewSpecFromVolume(fakeVolume), nil + return volume.ReconstructedVolume{ + Spec: volume.NewSpecFromVolume(fakeVolume), + }, nil } func (plugin *TestPlugin) NewAttacher() (volume.Attacher, error) { diff --git a/pkg/controller/volume/persistentvolume/framework_test.go b/pkg/controller/volume/persistentvolume/framework_test.go index ea109e3af88..e5a3130084a 100644 --- a/pkg/controller/volume/persistentvolume/framework_test.go +++ b/pkg/controller/volume/persistentvolume/framework_test.go @@ -967,8 +967,8 @@ func (plugin *mockVolumePlugin) SupportsBulkVolumeVerification() bool { return false } -func (plugin *mockVolumePlugin) ConstructVolumeSpec(volumeName, mountPath string) (*volume.Spec, error) { - return nil, nil +func (plugin *mockVolumePlugin) ConstructVolumeSpec(volumeName, mountPath string) (volume.ReconstructedVolume, error) { + return volume.ReconstructedVolume{}, nil } func (plugin *mockVolumePlugin) SupportsSELinuxContextMount(spec *volume.Spec) (bool, error) { diff --git a/pkg/kubelet/volumemanager/reconciler/reconstruct.go b/pkg/kubelet/volumemanager/reconciler/reconstruct.go index ce503c5a824..6c0d2b1a4dc 100644 --- a/pkg/kubelet/volumemanager/reconciler/reconstruct.go +++ b/pkg/kubelet/volumemanager/reconciler/reconstruct.go @@ -1,9 +1,12 @@ /* -Copyright 2022 The Kubernetes Authors. +Copyright 2016 The Kubernetes Authors. + Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. You may obtain a copy of the License at + http://www.apache.org/licenses/LICENSE-2.0 + Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. @@ -136,7 +139,7 @@ func (rc *reconciler) reconstructVolume(volume podVolume) (*reconstructedVolume, return nil, fmt.Errorf("could not find block volume plugin %q (spec.Name: %q) pod %q (UID: %q)", volume.pluginName, volume.volumeSpecName, volume.podName, pod.UID) } - volumeSpec, err := rc.operationExecutor.ReconstructVolumeOperation( + reconstructed, err := rc.operationExecutor.ReconstructVolumeOperation( volume.volumeMode, plugin, mapperPlugin, @@ -148,6 +151,7 @@ func (rc *reconciler) reconstructVolume(volume podVolume) (*reconstructedVolume, if err != nil { return nil, err } + volumeSpec := reconstructed.Spec // We have to find the plugins by volume spec (NOT by plugin name) here // in order to correctly reconstruct ephemeral volume types. diff --git a/pkg/kubelet/volumemanager/reconciler/reconstruct_common.go b/pkg/kubelet/volumemanager/reconciler/reconstruct_common.go index 6088fd5b0b6..88a6581f232 100644 --- a/pkg/kubelet/volumemanager/reconciler/reconstruct_common.go +++ b/pkg/kubelet/volumemanager/reconciler/reconstruct_common.go @@ -1,9 +1,12 @@ /* -Copyright 2022 The Kubernetes Authors. +Copyright 2016 The Kubernetes Authors. + Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. You may obtain a copy of the License at + http://www.apache.org/licenses/LICENSE-2.0 + Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. diff --git a/pkg/volume/awsebs/aws_ebs.go b/pkg/volume/awsebs/aws_ebs.go index 0f135179e3d..0801982ffcd 100644 --- a/pkg/volume/awsebs/aws_ebs.go +++ b/pkg/volume/awsebs/aws_ebs.go @@ -249,25 +249,27 @@ func getVolumeSource( return nil, false, fmt.Errorf("Spec does not reference an AWS EBS volume type") } -func (plugin *awsElasticBlockStorePlugin) ConstructVolumeSpec(volName, mountPath string) (*volume.Spec, error) { +func (plugin *awsElasticBlockStorePlugin) ConstructVolumeSpec(volName, mountPath string) (volume.ReconstructedVolume, error) { mounter := plugin.host.GetMounter(plugin.GetPluginName()) kvh, ok := plugin.host.(volume.KubeletVolumeHost) if !ok { - return nil, fmt.Errorf("plugin volume host does not implement KubeletVolumeHost interface") + return volume.ReconstructedVolume{}, fmt.Errorf("plugin volume host does not implement KubeletVolumeHost interface") } hu := kvh.GetHostUtil() pluginMntDir := util.GetPluginMountDir(plugin.host, plugin.GetPluginName()) volumeID, err := hu.GetDeviceNameFromMount(mounter, mountPath, pluginMntDir) if err != nil { - return nil, err + return volume.ReconstructedVolume{}, err } volumeID, err = formatVolumeID(volumeID) if err != nil { - return nil, fmt.Errorf("failed to get AWS volume id from mount path %q: %v", mountPath, err) + return volume.ReconstructedVolume{}, fmt.Errorf("failed to get AWS volume id from mount path %q: %v", mountPath, err) } file := v1.PersistentVolumeFilesystem - return newAWSVolumeSpec(volName, volumeID, file), nil + return volume.ReconstructedVolume{ + Spec: newAWSVolumeSpec(volName, volumeID, file), + }, nil } func (plugin *awsElasticBlockStorePlugin) RequiresFSResize() bool { diff --git a/pkg/volume/azure_file/azure_file.go b/pkg/volume/azure_file/azure_file.go index df331ea9f48..8c00c41b579 100644 --- a/pkg/volume/azure_file/azure_file.go +++ b/pkg/volume/azure_file/azure_file.go @@ -202,7 +202,7 @@ func (plugin *azureFilePlugin) ExpandVolumeDevice( return newSize, nil } -func (plugin *azureFilePlugin) ConstructVolumeSpec(volName, mountPath string) (*volume.Spec, error) { +func (plugin *azureFilePlugin) ConstructVolumeSpec(volName, mountPath string) (volume.ReconstructedVolume, error) { azureVolume := &v1.Volume{ Name: volName, VolumeSource: v1.VolumeSource{ @@ -212,7 +212,9 @@ func (plugin *azureFilePlugin) ConstructVolumeSpec(volName, mountPath string) (* }, }, } - return volume.NewSpecFromVolume(azureVolume), nil + return volume.ReconstructedVolume{ + Spec: volume.NewSpecFromVolume(azureVolume), + }, nil } // azureFile volumes represent mount of an AzureFile share. diff --git a/pkg/volume/azuredd/azure_dd.go b/pkg/volume/azuredd/azure_dd.go index 25dd97af883..99ec690b2dc 100644 --- a/pkg/volume/azuredd/azure_dd.go +++ b/pkg/volume/azuredd/azure_dd.go @@ -316,18 +316,18 @@ func (plugin *azureDataDiskPlugin) NodeExpand(resizeOptions volume.NodeResizeOpt var _ volume.NodeExpandableVolumePlugin = &azureDataDiskPlugin{} -func (plugin *azureDataDiskPlugin) ConstructVolumeSpec(volumeName, mountPath string) (*volume.Spec, error) { +func (plugin *azureDataDiskPlugin) ConstructVolumeSpec(volumeName, mountPath string) (volume.ReconstructedVolume, error) { mounter := plugin.host.GetMounter(plugin.GetPluginName()) kvh, ok := plugin.host.(volume.KubeletVolumeHost) if !ok { - return nil, fmt.Errorf("plugin volume host does not implement KubeletVolumeHost interface") + return volume.ReconstructedVolume{}, fmt.Errorf("plugin volume host does not implement KubeletVolumeHost interface") } hu := kvh.GetHostUtil() pluginMntDir := util.GetPluginMountDir(plugin.host, plugin.GetPluginName()) sourceName, err := hu.GetDeviceNameFromMount(mounter, mountPath, pluginMntDir) if err != nil { - return nil, err + return volume.ReconstructedVolume{}, err } azureVolume := &v1.Volume{ @@ -338,7 +338,9 @@ func (plugin *azureDataDiskPlugin) ConstructVolumeSpec(volumeName, mountPath str }, }, } - return volume.NewSpecFromVolume(azureVolume), nil + return volume.ReconstructedVolume{ + Spec: volume.NewSpecFromVolume(azureVolume), + }, nil } func (plugin *azureDataDiskPlugin) GetDeviceMountRefs(deviceMountPath string) ([]string, error) { diff --git a/pkg/volume/cephfs/cephfs.go b/pkg/volume/cephfs/cephfs.go index 2fc27d6248b..c4480e8447a 100644 --- a/pkg/volume/cephfs/cephfs.go +++ b/pkg/volume/cephfs/cephfs.go @@ -173,7 +173,7 @@ func (plugin *cephfsPlugin) newUnmounterInternal(volName string, podUID types.UI }, nil } -func (plugin *cephfsPlugin) ConstructVolumeSpec(volumeName, mountPath string) (*volume.Spec, error) { +func (plugin *cephfsPlugin) ConstructVolumeSpec(volumeName, mountPath string) (volume.ReconstructedVolume, error) { cephfsVolume := &v1.Volume{ Name: volumeName, VolumeSource: v1.VolumeSource{ @@ -183,7 +183,9 @@ func (plugin *cephfsPlugin) ConstructVolumeSpec(volumeName, mountPath string) (* }, }, } - return volume.NewSpecFromVolume(cephfsVolume), nil + return volume.ReconstructedVolume{ + Spec: volume.NewSpecFromVolume(cephfsVolume), + }, nil } // CephFS volumes represent a bare host file or directory mount of an CephFS export. diff --git a/pkg/volume/cephfs/cephfs_test.go b/pkg/volume/cephfs/cephfs_test.go index 9c671842f59..874395b60ba 100644 --- a/pkg/volume/cephfs/cephfs_test.go +++ b/pkg/volume/cephfs/cephfs_test.go @@ -128,13 +128,13 @@ func TestConstructVolumeSpec(t *testing.T) { t.Errorf("can't find cephfs plugin by name") } - cephfsSpec, err := plug.(*cephfsPlugin).ConstructVolumeSpec("cephfsVolume", "/cephfsVolume/") + cephfsVol, err := plug.(*cephfsPlugin).ConstructVolumeSpec("cephfsVolume", "/cephfsVolume/") if err != nil { t.Errorf("ConstructVolumeSpec() failed: %v", err) } - if cephfsSpec.Name() != "cephfsVolume" { - t.Errorf("Get wrong cephfs spec name, got: %s", cephfsSpec.Name()) + if cephfsVol.Spec.Name() != "cephfsVolume" { + t.Errorf("Get wrong cephfs spec name, got: %s", cephfsVol.Spec.Name()) } } diff --git a/pkg/volume/configmap/configmap.go b/pkg/volume/configmap/configmap.go index 8aca9bc2314..cdfba9480cd 100644 --- a/pkg/volume/configmap/configmap.go +++ b/pkg/volume/configmap/configmap.go @@ -122,14 +122,16 @@ func (plugin *configMapPlugin) NewUnmounter(volName string, podUID types.UID) (v }, nil } -func (plugin *configMapPlugin) ConstructVolumeSpec(volumeName, mountPath string) (*volume.Spec, error) { +func (plugin *configMapPlugin) ConstructVolumeSpec(volumeName, mountPath string) (volume.ReconstructedVolume, error) { configMapVolume := &v1.Volume{ Name: volumeName, VolumeSource: v1.VolumeSource{ ConfigMap: &v1.ConfigMapVolumeSource{}, }, } - return volume.NewSpecFromVolume(configMapVolume), nil + return volume.ReconstructedVolume{ + Spec: volume.NewSpecFromVolume(configMapVolume), + }, nil } type configMapVolume struct { diff --git a/pkg/volume/csi/csi_plugin.go b/pkg/volume/csi/csi_plugin.go index ce7a543c94f..c4bb214e6cf 100644 --- a/pkg/volume/csi/csi_plugin.go +++ b/pkg/volume/csi/csi_plugin.go @@ -448,12 +448,12 @@ func (p *csiPlugin) NewUnmounter(specName string, podUID types.UID) (volume.Unmo return unmounter, nil } -func (p *csiPlugin) ConstructVolumeSpec(volumeName, mountPath string) (*volume.Spec, error) { +func (p *csiPlugin) ConstructVolumeSpec(volumeName, mountPath string) (volume.ReconstructedVolume, error) { klog.V(4).Info(log("plugin.ConstructVolumeSpec [pv.Name=%v, path=%v]", volumeName, mountPath)) volData, err := loadVolumeData(mountPath, volDataFileName) if err != nil { - return nil, errors.New(log("plugin.ConstructVolumeSpec failed loading volume data using [%s]: %v", mountPath, err)) + return volume.ReconstructedVolume{}, errors.New(log("plugin.ConstructVolumeSpec failed loading volume data using [%s]: %v", mountPath, err)) } klog.V(4).Info(log("plugin.ConstructVolumeSpec extracted [%#v]", volData)) @@ -464,11 +464,13 @@ func (p *csiPlugin) ConstructVolumeSpec(volumeName, mountPath string) (*volume.S // use constructPVSourceSpec to construct volume construct pv source spec. if storage.VolumeLifecycleMode(volData[volDataKey.volumeLifecycleMode]) == storage.VolumeLifecycleEphemeral { spec = p.constructVolSourceSpec(volData[volDataKey.specVolID], volData[volDataKey.driverName]) - return spec, nil + return volume.ReconstructedVolume{Spec: spec}, nil } - spec = p.constructPVSourceSpec(volData[volDataKey.specVolID], volData[volDataKey.driverName], volData[volDataKey.volHandle]) - return spec, nil + spec = p.constructPVSourceSpec(volData[volDataKey.specVolID], volData[volDataKey.driverName], volData[volDataKey.volHandle]) + return volume.ReconstructedVolume{ + Spec: spec, + }, nil } // constructVolSourceSpec constructs volume.Spec with CSIVolumeSource diff --git a/pkg/volume/csi/csi_plugin_test.go b/pkg/volume/csi/csi_plugin_test.go index 9774be800e0..e874eaba7b8 100644 --- a/pkg/volume/csi/csi_plugin_test.go +++ b/pkg/volume/csi/csi_plugin_test.go @@ -362,38 +362,38 @@ func TestPluginConstructVolumeSpec(t *testing.T) { } // rebuild spec - spec, err := plug.ConstructVolumeSpec("test-pv", filepath.Dir(csiMounter.GetPath())) + rec, err := plug.ConstructVolumeSpec("test-pv", filepath.Dir(csiMounter.GetPath())) if err != nil { t.Fatal(err) } - if spec == nil { + if rec.Spec == nil { t.Fatal("nil volume.Spec constructed") } // inspect spec - if spec.PersistentVolume == nil || spec.PersistentVolume.Spec.CSI == nil { + if rec.Spec.PersistentVolume == nil || rec.Spec.PersistentVolume.Spec.CSI == nil { t.Fatal("CSIPersistentVolume not found in constructed spec ") } - volHandle := spec.PersistentVolume.Spec.CSI.VolumeHandle + volHandle := rec.Spec.PersistentVolume.Spec.CSI.VolumeHandle if volHandle != tc.originSpec.PersistentVolume.Spec.CSI.VolumeHandle { t.Error("unexpected volumeHandle constructed:", volHandle) } - driverName := spec.PersistentVolume.Spec.CSI.Driver + driverName := rec.Spec.PersistentVolume.Spec.CSI.Driver if driverName != tc.originSpec.PersistentVolume.Spec.CSI.Driver { t.Error("unexpected driverName constructed:", driverName) } - if spec.PersistentVolume.Spec.VolumeMode == nil { + if rec.Spec.PersistentVolume.Spec.VolumeMode == nil { t.Fatalf("Volume mode has not been set.") } - if *spec.PersistentVolume.Spec.VolumeMode != api.PersistentVolumeFilesystem { - t.Errorf("Unexpected volume mode %q", *spec.PersistentVolume.Spec.VolumeMode) + if *rec.Spec.PersistentVolume.Spec.VolumeMode != api.PersistentVolumeFilesystem { + t.Errorf("Unexpected volume mode %q", *rec.Spec.PersistentVolume.Spec.VolumeMode) } - if spec.Name() != tc.specVolID { - t.Errorf("Unexpected spec name constructed %s", spec.Name()) + if rec.Spec.Name() != tc.specVolID { + t.Errorf("Unexpected spec name constructed %s", rec.Spec.Name()) } }) } @@ -496,44 +496,44 @@ func TestPluginConstructVolumeSpecWithInline(t *testing.T) { } // rebuild spec - spec, err := plug.ConstructVolumeSpec("test-pv", filepath.Dir(csiMounter.GetPath())) + rec, err := plug.ConstructVolumeSpec("test-pv", filepath.Dir(csiMounter.GetPath())) if err != nil { t.Fatal(err) } - if spec == nil { + if rec.Spec == nil { t.Fatal("nil volume.Spec constructed") } - if spec.Name() != tc.specVolID { - t.Errorf("unexpected spec name constructed volume.Spec: %s", spec.Name()) + if rec.Spec.Name() != tc.specVolID { + t.Errorf("unexpected spec name constructed volume.Spec: %s", rec.Spec.Name()) } switch { - case spec.Volume != nil: - if spec.Volume.CSI == nil { + case rec.Spec.Volume != nil: + if rec.Spec.Volume.CSI == nil { t.Error("missing CSIVolumeSource in constructed volume.Spec") } - if spec.Volume.CSI.Driver != tc.originSpec.Volume.CSI.Driver { - t.Error("unexpected driver in constructed volume source:", spec.Volume.CSI.Driver) + if rec.Spec.Volume.CSI.Driver != tc.originSpec.Volume.CSI.Driver { + t.Error("unexpected driver in constructed volume source:", rec.Spec.Volume.CSI.Driver) } - case spec.PersistentVolume != nil: - if spec.PersistentVolume.Spec.CSI == nil { + case rec.Spec.PersistentVolume != nil: + if rec.Spec.PersistentVolume.Spec.CSI == nil { t.Fatal("missing CSIPersistentVolumeSource in constructed volume.spec") } - volHandle := spec.PersistentVolume.Spec.CSI.VolumeHandle + volHandle := rec.Spec.PersistentVolume.Spec.CSI.VolumeHandle if volHandle != tc.originSpec.PersistentVolume.Spec.CSI.VolumeHandle { t.Error("unexpected volumeHandle constructed in persistent volume source:", volHandle) } - driverName := spec.PersistentVolume.Spec.CSI.Driver + driverName := rec.Spec.PersistentVolume.Spec.CSI.Driver if driverName != tc.originSpec.PersistentVolume.Spec.CSI.Driver { t.Error("unexpected driverName constructed in persistent volume source:", driverName) } - if spec.PersistentVolume.Spec.VolumeMode == nil { + if rec.Spec.PersistentVolume.Spec.VolumeMode == nil { t.Fatalf("Volume mode has not been set.") } - if *spec.PersistentVolume.Spec.VolumeMode != api.PersistentVolumeFilesystem { - t.Errorf("Unexpected volume mode %q", *spec.PersistentVolume.Spec.VolumeMode) + if *rec.Spec.PersistentVolume.Spec.VolumeMode != api.PersistentVolumeFilesystem { + t.Errorf("Unexpected volume mode %q", *rec.Spec.PersistentVolume.Spec.VolumeMode) } default: t.Fatal("invalid volume.Spec constructed") diff --git a/pkg/volume/csi/csi_test.go b/pkg/volume/csi/csi_test.go index 55ea7c23822..da6d28046cd 100644 --- a/pkg/volume/csi/csi_test.go +++ b/pkg/volume/csi/csi_test.go @@ -443,14 +443,14 @@ func TestCSI_VolumeAll(t *testing.T) { // ******** Volume Reconstruction ************* // volPath := filepath.Dir(csiMounter.GetPath()) t.Log("csiTest.VolumeAll entering plugin.ConstructVolumeSpec for path", volPath) - spec, err := volPlug.ConstructVolumeSpec(test.volName, volPath) + rec, err := volPlug.ConstructVolumeSpec(test.volName, volPath) if err != nil { t.Fatalf("csiTest.VolumeAll plugin.ConstructVolumeSpec failed: %s", err) } else { - if spec == nil { + if rec.Spec == nil { t.Fatalf("csiTest.VolumeAll plugin.ConstructVolumeSpec returned nil spec") } else { - volSpec = spec + volSpec = rec.Spec if test.isInline { if volSpec.Volume == nil || volSpec.Volume.CSI == nil { diff --git a/pkg/volume/downwardapi/downwardapi.go b/pkg/volume/downwardapi/downwardapi.go index 714254c5c79..8338850ac42 100644 --- a/pkg/volume/downwardapi/downwardapi.go +++ b/pkg/volume/downwardapi/downwardapi.go @@ -123,14 +123,16 @@ func (plugin *downwardAPIPlugin) NewUnmounter(volName string, podUID types.UID) }, nil } -func (plugin *downwardAPIPlugin) ConstructVolumeSpec(volumeName, mountPath string) (*volume.Spec, error) { +func (plugin *downwardAPIPlugin) ConstructVolumeSpec(volumeName, mountPath string) (volume.ReconstructedVolume, error) { downwardAPIVolume := &v1.Volume{ Name: volumeName, VolumeSource: v1.VolumeSource{ DownwardAPI: &v1.DownwardAPIVolumeSource{}, }, } - return volume.NewSpecFromVolume(downwardAPIVolume), nil + return volume.ReconstructedVolume{ + Spec: volume.NewSpecFromVolume(downwardAPIVolume), + }, nil } // downwardAPIVolume retrieves downward API data and placing them into the volume on the host. diff --git a/pkg/volume/emptydir/empty_dir.go b/pkg/volume/emptydir/empty_dir.go index cb10674cb83..3fcf4472059 100644 --- a/pkg/volume/emptydir/empty_dir.go +++ b/pkg/volume/emptydir/empty_dir.go @@ -188,14 +188,16 @@ func (plugin *emptyDirPlugin) newUnmounterInternal(volName string, podUID types. return ed, nil } -func (plugin *emptyDirPlugin) ConstructVolumeSpec(volName, mountPath string) (*volume.Spec, error) { +func (plugin *emptyDirPlugin) ConstructVolumeSpec(volName, mountPath string) (volume.ReconstructedVolume, error) { emptyDirVolume := &v1.Volume{ Name: volName, VolumeSource: v1.VolumeSource{ EmptyDir: &v1.EmptyDirVolumeSource{}, }, } - return volume.NewSpecFromVolume(emptyDirVolume), nil + return volume.ReconstructedVolume{ + Spec: volume.NewSpecFromVolume(emptyDirVolume), + }, nil } // mountDetector abstracts how to find what kind of mount a path is backed by. diff --git a/pkg/volume/fc/fc.go b/pkg/volume/fc/fc.go index ed9912aa690..a29e85a1d9d 100644 --- a/pkg/volume/fc/fc.go +++ b/pkg/volume/fc/fc.go @@ -240,7 +240,7 @@ func (plugin *fcPlugin) newUnmapperInternal(volName string, podUID types.UID, ma }, nil } -func (plugin *fcPlugin) ConstructVolumeSpec(volumeName, mountPath string) (*volume.Spec, error) { +func (plugin *fcPlugin) ConstructVolumeSpec(volumeName, mountPath string) (volume.ReconstructedVolume, error) { // Find globalPDPath from pod volume directory(mountPath) // examples: // mountPath: pods/{podUid}/volumes/kubernetes.io~fc/{volumeName} @@ -256,10 +256,10 @@ func (plugin *fcPlugin) ConstructVolumeSpec(volumeName, mountPath string) (*volu if io.IsInconsistentReadError(err) { klog.Errorf("Failed to read mount refs from /proc/mounts for %s: %s", mountPath, err) klog.Errorf("Kubelet cannot unmount volume at %s, please unmount it manually", mountPath) - return nil, err + return volume.ReconstructedVolume{}, err } if err != nil { - return nil, err + return volume.ReconstructedVolume{}, err } for _, path := range paths { if strings.Contains(path, plugin.host.GetPluginDir(fcPluginName)) { @@ -269,12 +269,12 @@ func (plugin *fcPlugin) ConstructVolumeSpec(volumeName, mountPath string) (*volu } // Couldn't fetch globalPDPath if len(globalPDPath) == 0 { - return nil, fmt.Errorf("couldn't fetch globalPDPath. failed to obtain volume spec") + return volume.ReconstructedVolume{}, fmt.Errorf("couldn't fetch globalPDPath. failed to obtain volume spec") } wwns, lun, wwids, err := parsePDName(globalPDPath) if err != nil { - return nil, fmt.Errorf("failed to retrieve volume plugin information from globalPDPath: %s", err) + return volume.ReconstructedVolume{}, fmt.Errorf("failed to retrieve volume plugin information from globalPDPath: %s", err) } // Create volume from wwn+lun or wwid fcVolume := &v1.Volume{ @@ -285,7 +285,9 @@ func (plugin *fcPlugin) ConstructVolumeSpec(volumeName, mountPath string) (*volu } klog.V(5).Infof("ConstructVolumeSpec: TargetWWNs: %v, Lun: %v, WWIDs: %v", fcVolume.VolumeSource.FC.TargetWWNs, *fcVolume.VolumeSource.FC.Lun, fcVolume.VolumeSource.FC.WWIDs) - return volume.NewSpecFromVolume(fcVolume), nil + return volume.ReconstructedVolume{ + Spec: volume.NewSpecFromVolume(fcVolume), + }, nil } // ConstructBlockVolumeSpec creates a new volume.Spec with following steps. diff --git a/pkg/volume/flexvolume/plugin.go b/pkg/volume/flexvolume/plugin.go index 92e5c8d91db..b2449f4541c 100644 --- a/pkg/volume/flexvolume/plugin.go +++ b/pkg/volume/flexvolume/plugin.go @@ -260,7 +260,7 @@ func (plugin *flexVolumeAttachablePlugin) CanDeviceMount(spec *volume.Spec) (boo } // ConstructVolumeSpec is part of the volume.AttachableVolumePlugin interface. -func (plugin *flexVolumePlugin) ConstructVolumeSpec(volumeName, mountPath string) (*volume.Spec, error) { +func (plugin *flexVolumePlugin) ConstructVolumeSpec(volumeName, mountPath string) (volume.ReconstructedVolume, error) { flexVolume := &api.Volume{ Name: volumeName, VolumeSource: api.VolumeSource{ @@ -269,7 +269,9 @@ func (plugin *flexVolumePlugin) ConstructVolumeSpec(volumeName, mountPath string }, }, } - return volume.NewSpecFromVolume(flexVolume), nil + return volume.ReconstructedVolume{ + Spec: volume.NewSpecFromVolume(flexVolume), + }, nil } func (plugin *flexVolumePlugin) SupportsMountOption() bool { diff --git a/pkg/volume/gcepd/gce_pd.go b/pkg/volume/gcepd/gce_pd.go index cb33c1e3dd3..0df9ef10f5a 100644 --- a/pkg/volume/gcepd/gce_pd.go +++ b/pkg/volume/gcepd/gce_pd.go @@ -299,17 +299,17 @@ func (plugin *gcePersistentDiskPlugin) NodeExpand(resizeOptions volume.NodeResiz var _ volume.NodeExpandableVolumePlugin = &gcePersistentDiskPlugin{} -func (plugin *gcePersistentDiskPlugin) ConstructVolumeSpec(volumeName, mountPath string) (*volume.Spec, error) { +func (plugin *gcePersistentDiskPlugin) ConstructVolumeSpec(volumeName, mountPath string) (volume.ReconstructedVolume, error) { mounter := plugin.host.GetMounter(plugin.GetPluginName()) kvh, ok := plugin.host.(volume.KubeletVolumeHost) if !ok { - return nil, fmt.Errorf("plugin volume host does not implement KubeletVolumeHost interface") + return volume.ReconstructedVolume{}, fmt.Errorf("plugin volume host does not implement KubeletVolumeHost interface") } hu := kvh.GetHostUtil() pluginMntDir := util.GetPluginMountDir(plugin.host, plugin.GetPluginName()) sourceName, err := hu.GetDeviceNameFromMount(mounter, mountPath, pluginMntDir) if err != nil { - return nil, err + return volume.ReconstructedVolume{}, err } gceVolume := &v1.Volume{ Name: volumeName, @@ -319,7 +319,9 @@ func (plugin *gcePersistentDiskPlugin) ConstructVolumeSpec(volumeName, mountPath }, }, } - return volume.NewSpecFromVolume(gceVolume), nil + return volume.ReconstructedVolume{ + Spec: volume.NewSpecFromVolume(gceVolume), + }, nil } // Abstract interface to PD operations. diff --git a/pkg/volume/git_repo/git_repo.go b/pkg/volume/git_repo/git_repo.go index 76dafd7c839..fe890032e27 100644 --- a/pkg/volume/git_repo/git_repo.go +++ b/pkg/volume/git_repo/git_repo.go @@ -123,14 +123,16 @@ func (plugin *gitRepoPlugin) NewUnmounter(volName string, podUID types.UID) (vol }, nil } -func (plugin *gitRepoPlugin) ConstructVolumeSpec(volumeName, mountPath string) (*volume.Spec, error) { +func (plugin *gitRepoPlugin) ConstructVolumeSpec(volumeName, mountPath string) (volume.ReconstructedVolume, error) { gitVolume := &v1.Volume{ Name: volumeName, VolumeSource: v1.VolumeSource{ GitRepo: &v1.GitRepoVolumeSource{}, }, } - return volume.NewSpecFromVolume(gitVolume), nil + return volume.ReconstructedVolume{ + Spec: volume.NewSpecFromVolume(gitVolume), + }, nil } // gitRepo volumes are directories which are pre-filled from a git repository. diff --git a/pkg/volume/hostpath/host_path.go b/pkg/volume/hostpath/host_path.go index c6f5b4c779c..b6e36bd3cce 100644 --- a/pkg/volume/hostpath/host_path.go +++ b/pkg/volume/hostpath/host_path.go @@ -181,7 +181,7 @@ func (plugin *hostPathPlugin) NewProvisioner(options volume.VolumeOptions) (volu return newProvisioner(options, plugin.host, plugin) } -func (plugin *hostPathPlugin) ConstructVolumeSpec(volumeName, mountPath string) (*volume.Spec, error) { +func (plugin *hostPathPlugin) ConstructVolumeSpec(volumeName, mountPath string) (volume.ReconstructedVolume, error) { hostPathVolume := &v1.Volume{ Name: volumeName, VolumeSource: v1.VolumeSource{ @@ -190,7 +190,9 @@ func (plugin *hostPathPlugin) ConstructVolumeSpec(volumeName, mountPath string) }, }, } - return volume.NewSpecFromVolume(hostPathVolume), nil + return volume.ReconstructedVolume{ + Spec: volume.NewSpecFromVolume(hostPathVolume), + }, nil } func newDeleter(spec *volume.Spec, host volume.VolumeHost) (volume.Deleter, error) { diff --git a/pkg/volume/iscsi/iscsi.go b/pkg/volume/iscsi/iscsi.go index 7f24a4e32ff..791527a3c98 100644 --- a/pkg/volume/iscsi/iscsi.go +++ b/pkg/volume/iscsi/iscsi.go @@ -221,7 +221,7 @@ func (plugin *iscsiPlugin) newUnmapperInternal(volName string, podUID types.UID, }, nil } -func (plugin *iscsiPlugin) ConstructVolumeSpec(volumeName, mountPath string) (*volume.Spec, error) { +func (plugin *iscsiPlugin) ConstructVolumeSpec(volumeName, mountPath string) (volume.ReconstructedVolume, error) { // Find globalPDPath from pod volume directory(mountPath) var globalPDPath string mounter := plugin.host.GetMounter(plugin.GetPluginName()) @@ -233,10 +233,10 @@ func (plugin *iscsiPlugin) ConstructVolumeSpec(volumeName, mountPath string) (*v if io.IsInconsistentReadError(err) { klog.Errorf("Failed to read mount refs from /proc/mounts for %s: %s", mountPath, err) klog.Errorf("Kubelet cannot unmount volume at %s, please unmount it and all mounts of the same device manually.", mountPath) - return nil, err + return volume.ReconstructedVolume{}, err } if err != nil { - return nil, err + return volume.ReconstructedVolume{}, err } for _, path := range paths { @@ -247,25 +247,25 @@ func (plugin *iscsiPlugin) ConstructVolumeSpec(volumeName, mountPath string) (*v } // Couldn't fetch globalPDPath if len(globalPDPath) == 0 { - return nil, fmt.Errorf("couldn't fetch globalPDPath. failed to obtain volume spec") + return volume.ReconstructedVolume{}, fmt.Errorf("couldn't fetch globalPDPath. failed to obtain volume spec") } // Obtain iscsi disk configurations from globalPDPath device, _, err := extractDeviceAndPrefix(globalPDPath) if err != nil { - return nil, err + return volume.ReconstructedVolume{}, err } bkpPortal, iqn, err := extractPortalAndIqn(device) if err != nil { - return nil, err + return volume.ReconstructedVolume{}, err } arr := strings.Split(device, "-lun-") if len(arr) < 2 { - return nil, fmt.Errorf("failed to retrieve lun from globalPDPath: %v", globalPDPath) + return volume.ReconstructedVolume{}, fmt.Errorf("failed to retrieve lun from globalPDPath: %v", globalPDPath) } lun, err := strconv.Atoi(arr[1]) if err != nil { - return nil, err + return volume.ReconstructedVolume{}, err } iface, _ := extractIface(globalPDPath) iscsiVolume := &v1.Volume{ @@ -279,7 +279,9 @@ func (plugin *iscsiPlugin) ConstructVolumeSpec(volumeName, mountPath string) (*v }, }, } - return volume.NewSpecFromVolume(iscsiVolume), nil + return volume.ReconstructedVolume{ + Spec: volume.NewSpecFromVolume(iscsiVolume), + }, nil } func (plugin *iscsiPlugin) ConstructBlockVolumeSpec(podUID types.UID, volumeName, mapPath string) (*volume.Spec, error) { diff --git a/pkg/volume/local/local.go b/pkg/volume/local/local.go index ab2af54c2e4..c55a3502e79 100644 --- a/pkg/volume/local/local.go +++ b/pkg/volume/local/local.go @@ -197,7 +197,7 @@ func (plugin *localVolumePlugin) NewBlockVolumeUnmapper(volName string, } // TODO: check if no path and no topology constraints are ok -func (plugin *localVolumePlugin) ConstructVolumeSpec(volumeName, mountPath string) (*volume.Spec, error) { +func (plugin *localVolumePlugin) ConstructVolumeSpec(volumeName, mountPath string) (volume.ReconstructedVolume, error) { fs := v1.PersistentVolumeFilesystem // The main purpose of reconstructed volume is to clean unused mount points // and directories. @@ -209,7 +209,7 @@ func (plugin *localVolumePlugin) ConstructVolumeSpec(volumeName, mountPath strin mounter := plugin.host.GetMounter(plugin.GetPluginName()) refs, err := mounter.GetMountRefs(mountPath) if err != nil { - return nil, err + return volume.ReconstructedVolume{}, err } baseMountPath := plugin.generateBlockDeviceBaseGlobalPath() for _, ref := range refs { @@ -221,7 +221,7 @@ func (plugin *localVolumePlugin) ConstructVolumeSpec(volumeName, mountPath strin // source and can be used in reconstructed volume. path, _, err = mount.GetDeviceNameFromMount(mounter, ref) if err != nil { - return nil, err + return volume.ReconstructedVolume{}, err } klog.V(4).Infof("local: reconstructing volume %q (pod volume mount: %q) with device %q", volumeName, mountPath, path) break @@ -240,7 +240,9 @@ func (plugin *localVolumePlugin) ConstructVolumeSpec(volumeName, mountPath strin VolumeMode: &fs, }, } - return volume.NewSpecFromPersistentVolume(localVolume, false), nil + return volume.ReconstructedVolume{ + Spec: volume.NewSpecFromPersistentVolume(localVolume, false), + }, nil } func (plugin *localVolumePlugin) ConstructBlockVolumeSpec(podUID types.UID, volumeName, diff --git a/pkg/volume/local/local_test.go b/pkg/volume/local/local_test.go index e9d6b19de68..bd47ab1f520 100644 --- a/pkg/volume/local/local_test.go +++ b/pkg/volume/local/local_test.go @@ -543,34 +543,34 @@ func TestConstructVolumeSpec(t *testing.T) { } mounter.(*mount.FakeMounter).MountPoints = fakeMountPoints volPath := filepath.Join(tmpDir, testMountPath) - spec, err := plug.ConstructVolumeSpec(testPVName, volPath) + rec, err := plug.ConstructVolumeSpec(testPVName, volPath) if err != nil { t.Errorf("ConstructVolumeSpec() failed: %v", err) } - if spec == nil { + if rec.Spec == nil { t.Fatalf("ConstructVolumeSpec() returned nil") } - volName := spec.Name() + volName := rec.Spec.Name() if volName != testPVName { t.Errorf("Expected volume name %q, got %q", testPVName, volName) } - if spec.Volume != nil { + if rec.Spec.Volume != nil { t.Errorf("Volume object returned, expected nil") } - pv := spec.PersistentVolume + pv := rec.Spec.PersistentVolume if pv == nil { t.Fatalf("PersistentVolume object nil") } - if spec.PersistentVolume.Spec.VolumeMode == nil { + if rec.Spec.PersistentVolume.Spec.VolumeMode == nil { t.Fatalf("Volume mode has not been set.") } - if *spec.PersistentVolume.Spec.VolumeMode != v1.PersistentVolumeFilesystem { - t.Errorf("Unexpected volume mode %q", *spec.PersistentVolume.Spec.VolumeMode) + if *rec.Spec.PersistentVolume.Spec.VolumeMode != v1.PersistentVolumeFilesystem { + t.Errorf("Unexpected volume mode %q", *rec.Spec.PersistentVolume.Spec.VolumeMode) } ls := pv.Spec.PersistentVolumeSource.Local diff --git a/pkg/volume/nfs/nfs.go b/pkg/volume/nfs/nfs.go index f292be4a506..2de888feca7 100644 --- a/pkg/volume/nfs/nfs.go +++ b/pkg/volume/nfs/nfs.go @@ -176,7 +176,7 @@ func (plugin *nfsPlugin) Recycle(pvName string, spec *volume.Spec, eventRecorder return recyclerclient.RecycleVolumeByWatchingPodUntilCompletion(pvName, pod, plugin.host.GetKubeClient(), eventRecorder) } -func (plugin *nfsPlugin) ConstructVolumeSpec(volumeName, mountPath string) (*volume.Spec, error) { +func (plugin *nfsPlugin) ConstructVolumeSpec(volumeName, mountPath string) (volume.ReconstructedVolume, error) { nfsVolume := &v1.Volume{ Name: volumeName, VolumeSource: v1.VolumeSource{ @@ -185,7 +185,9 @@ func (plugin *nfsPlugin) ConstructVolumeSpec(volumeName, mountPath string) (*vol }, }, } - return volume.NewSpecFromVolume(nfsVolume), nil + return volume.ReconstructedVolume{ + Spec: volume.NewSpecFromVolume(nfsVolume), + }, nil } // NFS volumes represent a bare host file or directory mount of an NFS export. diff --git a/pkg/volume/noop_expandable_plugin.go b/pkg/volume/noop_expandable_plugin.go index 8e3872e3712..9db19f16b0e 100644 --- a/pkg/volume/noop_expandable_plugin.go +++ b/pkg/volume/noop_expandable_plugin.go @@ -60,8 +60,8 @@ func (n *noopExpandableVolumePluginInstance) NewUnmounter(name string, podUID ty return nil, nil } -func (n *noopExpandableVolumePluginInstance) ConstructVolumeSpec(volumeName, mountPath string) (*Spec, error) { - return n.spec, nil +func (n *noopExpandableVolumePluginInstance) ConstructVolumeSpec(volumeName, mountPath string) (ReconstructedVolume, error) { + return ReconstructedVolume{Spec: n.spec}, nil } func (n *noopExpandableVolumePluginInstance) SupportsMountOption() bool { diff --git a/pkg/volume/plugins.go b/pkg/volume/plugins.go index 272b88ed9e7..d469a89b617 100644 --- a/pkg/volume/plugins.go +++ b/pkg/volume/plugins.go @@ -166,7 +166,7 @@ type VolumePlugin interface { // and volumePath. The spec may have incomplete information due to limited // information from input. This function is used by volume manager to reconstruct // volume spec by reading the volume directories from disk - ConstructVolumeSpec(volumeName, volumePath string) (*Spec, error) + ConstructVolumeSpec(volumeName, volumePath string) (ReconstructedVolume, error) // SupportsMountOption returns true if volume plugins supports Mount options // Specifying mount options in a volume plugin that doesn't support @@ -570,6 +570,12 @@ type VolumeConfig struct { ProvisioningEnabled bool } +// ReconstructedVolume contains information about a volume reconstructed by +// ConstructVolumeSpec(). +type ReconstructedVolume struct { + Spec *Spec +} + // NewSpecFromVolume creates an Spec from an v1.Volume func NewSpecFromVolume(vs *v1.Volume) *Spec { return &Spec{ diff --git a/pkg/volume/plugins_test.go b/pkg/volume/plugins_test.go index dabb421a344..8d00bb03c34 100644 --- a/pkg/volume/plugins_test.go +++ b/pkg/volume/plugins_test.go @@ -99,8 +99,8 @@ func (plugin *testPlugins) NewUnmounter(name string, podUID types.UID) (Unmounte return nil, nil } -func (plugin *testPlugins) ConstructVolumeSpec(volumeName, mountPath string) (*Spec, error) { - return nil, nil +func (plugin *testPlugins) ConstructVolumeSpec(volumeName, mountPath string) (ReconstructedVolume, error) { + return ReconstructedVolume{}, nil } func newTestPlugin() []VolumePlugin { diff --git a/pkg/volume/portworx/portworx.go b/pkg/volume/portworx/portworx.go index 417929b780d..6cba5032716 100644 --- a/pkg/volume/portworx/portworx.go +++ b/pkg/volume/portworx/portworx.go @@ -210,7 +210,7 @@ func (plugin *portworxVolumePlugin) ExpandVolumeDevice( return newSize, nil } -func (plugin *portworxVolumePlugin) ConstructVolumeSpec(volumeName, mountPath string) (*volume.Spec, error) { +func (plugin *portworxVolumePlugin) ConstructVolumeSpec(volumeName, mountPath string) (volume.ReconstructedVolume, error) { portworxVolume := &v1.Volume{ Name: volumeName, VolumeSource: v1.VolumeSource{ @@ -219,7 +219,9 @@ func (plugin *portworxVolumePlugin) ConstructVolumeSpec(volumeName, mountPath st }, }, } - return volume.NewSpecFromVolume(portworxVolume), nil + return volume.ReconstructedVolume{ + Spec: volume.NewSpecFromVolume(portworxVolume), + }, nil } func (plugin *portworxVolumePlugin) SupportsMountOption() bool { diff --git a/pkg/volume/projected/projected.go b/pkg/volume/projected/projected.go index ecbe408098e..a48567b2a6a 100644 --- a/pkg/volume/projected/projected.go +++ b/pkg/volume/projected/projected.go @@ -135,7 +135,7 @@ func (plugin *projectedPlugin) NewUnmounter(volName string, podUID types.UID) (v }, nil } -func (plugin *projectedPlugin) ConstructVolumeSpec(volumeName, mountPath string) (*volume.Spec, error) { +func (plugin *projectedPlugin) ConstructVolumeSpec(volumeName, mountPath string) (volume.ReconstructedVolume, error) { projectedVolume := &v1.Volume{ Name: volumeName, VolumeSource: v1.VolumeSource{ @@ -143,7 +143,9 @@ func (plugin *projectedPlugin) ConstructVolumeSpec(volumeName, mountPath string) }, } - return volume.NewSpecFromVolume(projectedVolume), nil + return volume.ReconstructedVolume{ + Spec: volume.NewSpecFromVolume(projectedVolume), + }, nil } type projectedVolume struct { diff --git a/pkg/volume/rbd/rbd.go b/pkg/volume/rbd/rbd.go index 242d59663da..93747df81a5 100644 --- a/pkg/volume/rbd/rbd.go +++ b/pkg/volume/rbd/rbd.go @@ -386,17 +386,17 @@ func (plugin *rbdPlugin) newUnmounterInternal(volName string, podUID types.UID, }, nil } -func (plugin *rbdPlugin) ConstructVolumeSpec(volumeName, mountPath string) (*volume.Spec, error) { +func (plugin *rbdPlugin) ConstructVolumeSpec(volumeName, mountPath string) (volume.ReconstructedVolume, error) { mounter := plugin.host.GetMounter(plugin.GetPluginName()) kvh, ok := plugin.host.(volume.KubeletVolumeHost) if !ok { - return nil, fmt.Errorf("plugin volume host does not implement KubeletVolumeHost interface") + return volume.ReconstructedVolume{}, fmt.Errorf("plugin volume host does not implement KubeletVolumeHost interface") } hu := kvh.GetHostUtil() pluginMntDir := volutil.GetPluginMountDir(plugin.host, plugin.GetPluginName()) sourceName, err := hu.GetDeviceNameFromMount(mounter, mountPath, pluginMntDir) if err != nil { - return nil, err + return volume.ReconstructedVolume{}, err } s := dstrings.Split(sourceName, "-image-") if len(s) != 2 { @@ -414,11 +414,11 @@ func (plugin *rbdPlugin) ConstructVolumeSpec(volumeName, mountPath string) (*vol klog.V(3).Infof("SourceName %s wrong, fallback to old format", sourceName) sourceName, err = plugin.getDeviceNameFromOldMountPath(mounter, mountPath) if err != nil { - return nil, err + return volume.ReconstructedVolume{}, err } s = dstrings.Split(sourceName, "-image-") if len(s) != 2 { - return nil, fmt.Errorf("sourceName %s wrong, should be pool+\"-image-\"+imageName", sourceName) + return volume.ReconstructedVolume{}, fmt.Errorf("sourceName %s wrong, should be pool+\"-image-\"+imageName", sourceName) } } rbdVolume := &v1.Volume{ @@ -430,7 +430,9 @@ func (plugin *rbdPlugin) ConstructVolumeSpec(volumeName, mountPath string) (*vol }, }, } - return volume.NewSpecFromVolume(rbdVolume), nil + return volume.ReconstructedVolume{ + Spec: volume.NewSpecFromVolume(rbdVolume), + }, nil } func (plugin *rbdPlugin) ConstructBlockVolumeSpec(podUID types.UID, volumeName, mapPath string) (*volume.Spec, error) { diff --git a/pkg/volume/rbd/rbd_test.go b/pkg/volume/rbd/rbd_test.go index 91cceaba3a3..1c7fc245932 100644 --- a/pkg/volume/rbd/rbd_test.go +++ b/pkg/volume/rbd/rbd_test.go @@ -631,15 +631,15 @@ func TestConstructVolumeSpec(t *testing.T) { if err = fakeMounter.Mount(c.targetPath, podMountPath, "fake", []string{"bind"}); err != nil { t.Fatalf("Mount %s to %s failed: %v", c.targetPath, podMountPath, err) } - spec, err := plug.ConstructVolumeSpec(c.volumeName, podMountPath) + rec, err := plug.ConstructVolumeSpec(c.volumeName, podMountPath) if err != nil { t.Errorf("ConstructVolumeSpec failed: %v", err) } else { - if spec.Volume.RBD.RBDPool != pool { - t.Errorf("Mismatch rbd pool: wanted %s, got %s", pool, spec.Volume.RBD.RBDPool) + if rec.Spec.Volume.RBD.RBDPool != pool { + t.Errorf("Mismatch rbd pool: wanted %s, got %s", pool, rec.Spec.Volume.RBD.RBDPool) } - if spec.Volume.RBD.RBDImage != image { - t.Fatalf("Mismatch rbd image: wanted %s, got %s", image, spec.Volume.RBD.RBDImage) + if rec.Spec.Volume.RBD.RBDImage != image { + t.Fatalf("Mismatch rbd image: wanted %s, got %s", image, rec.Spec.Volume.RBD.RBDImage) } } if err = fakeMounter.Unmount(podMountPath); err != nil { diff --git a/pkg/volume/secret/secret.go b/pkg/volume/secret/secret.go index a8a2d633b5c..fa06d879303 100644 --- a/pkg/volume/secret/secret.go +++ b/pkg/volume/secret/secret.go @@ -125,7 +125,7 @@ func (plugin *secretPlugin) NewUnmounter(volName string, podUID types.UID) (volu }, nil } -func (plugin *secretPlugin) ConstructVolumeSpec(volName, mountPath string) (*volume.Spec, error) { +func (plugin *secretPlugin) ConstructVolumeSpec(volName, mountPath string) (volume.ReconstructedVolume, error) { secretVolume := &v1.Volume{ Name: volName, VolumeSource: v1.VolumeSource{ @@ -134,7 +134,9 @@ func (plugin *secretPlugin) ConstructVolumeSpec(volName, mountPath string) (*vol }, }, } - return volume.NewSpecFromVolume(secretVolume), nil + return volume.ReconstructedVolume{ + Spec: volume.NewSpecFromVolume(secretVolume), + }, nil } type secretVolume struct { diff --git a/pkg/volume/testing/testing.go b/pkg/volume/testing/testing.go index 7e2f9cac554..ce6a3cc43db 100644 --- a/pkg/volume/testing/testing.go +++ b/pkg/volume/testing/testing.go @@ -451,10 +451,12 @@ func (plugin *FakeVolumePlugin) GetAccessModes() []v1.PersistentVolumeAccessMode return []v1.PersistentVolumeAccessMode{} } -func (plugin *FakeVolumePlugin) ConstructVolumeSpec(volumeName, mountPath string) (*volume.Spec, error) { - return &volume.Spec{ - Volume: &v1.Volume{ - Name: volumeName, +func (plugin *FakeVolumePlugin) ConstructVolumeSpec(volumeName, mountPath string) (volume.ReconstructedVolume, error) { + return volume.ReconstructedVolume{ + Spec: &volume.Spec{ + Volume: &v1.Volume{ + Name: volumeName, + }, }, }, nil } @@ -526,7 +528,7 @@ func (f *FakeBasicVolumePlugin) CanSupport(spec *volume.Spec) bool { return strings.HasPrefix(spec.Name(), f.GetPluginName()) } -func (f *FakeBasicVolumePlugin) ConstructVolumeSpec(ame, mountPath string) (*volume.Spec, error) { +func (f *FakeBasicVolumePlugin) ConstructVolumeSpec(ame, mountPath string) (volume.ReconstructedVolume, error) { return f.Plugin.ConstructVolumeSpec(ame, mountPath) } @@ -647,8 +649,8 @@ func (plugin *FakeFileVolumePlugin) NewUnmounter(name string, podUID types.UID) return nil, nil } -func (plugin *FakeFileVolumePlugin) ConstructVolumeSpec(volumeName, mountPath string) (*volume.Spec, error) { - return nil, nil +func (plugin *FakeFileVolumePlugin) ConstructVolumeSpec(volumeName, mountPath string) (volume.ReconstructedVolume, error) { + return volume.ReconstructedVolume{}, nil } func NewFakeFileVolumePlugin() []volume.VolumePlugin { diff --git a/pkg/volume/util/operationexecutor/operation_executor.go b/pkg/volume/util/operationexecutor/operation_executor.go index d16e287be68..6785f58aab4 100644 --- a/pkg/volume/util/operationexecutor/operation_executor.go +++ b/pkg/volume/util/operationexecutor/operation_executor.go @@ -150,7 +150,7 @@ type OperationExecutor interface { // ExpandInUseVolume will resize volume's file system to expected size without unmounting the volume. ExpandInUseVolume(volumeToMount VolumeToMount, actualStateOfWorld ActualStateOfWorldMounterUpdater, currentSize resource.Quantity) error // ReconstructVolumeOperation construct a new volumeSpec and returns it created by plugin - ReconstructVolumeOperation(volumeMode v1.PersistentVolumeMode, plugin volume.VolumePlugin, mapperPlugin volume.BlockVolumePlugin, uid types.UID, podName volumetypes.UniquePodName, volumeSpecName string, volumePath string, pluginName string) (*volume.Spec, error) + ReconstructVolumeOperation(volumeMode v1.PersistentVolumeMode, plugin volume.VolumePlugin, mapperPlugin volume.BlockVolumePlugin, uid types.UID, podName volumetypes.UniquePodName, volumeSpecName string, volumePath string, pluginName string) (volume.ReconstructedVolume, error) // CheckVolumeExistenceOperation checks volume existence CheckVolumeExistenceOperation(volumeSpec *volume.Spec, mountPath, volumeName string, mounter mount.Interface, uniqueVolumeName v1.UniqueVolumeName, podName volumetypes.UniquePodName, podUID types.UID, attachable volume.AttachableVolumePlugin) (bool, error) } @@ -1061,17 +1061,17 @@ func (oe *operationExecutor) ReconstructVolumeOperation( podName volumetypes.UniquePodName, volumeSpecName string, volumePath string, - pluginName string) (*volume.Spec, error) { + pluginName string) (volume.ReconstructedVolume, error) { // Filesystem Volume case if volumeMode == v1.PersistentVolumeFilesystem { // Create volumeSpec from mount path klog.V(5).Infof("Starting operationExecutor.ReconstructVolume for file volume on pod %q", podName) - volumeSpec, err := plugin.ConstructVolumeSpec(volumeSpecName, volumePath) + reconstructed, err := plugin.ConstructVolumeSpec(volumeSpecName, volumePath) if err != nil { - return nil, err + return volume.ReconstructedVolume{}, err } - return volumeSpec, nil + return reconstructed, nil } // Block Volume case @@ -1083,9 +1083,11 @@ func (oe *operationExecutor) ReconstructVolumeOperation( // ex. volumePath: pods/{podUid}}/{DefaultKubeletVolumeDevicesDirName}/{escapeQualifiedPluginName}/{volumeName} volumeSpec, err := mapperPlugin.ConstructBlockVolumeSpec(uid, volumeSpecName, volumePath) if err != nil { - return nil, err + return volume.ReconstructedVolume{}, err } - return volumeSpec, nil + return volume.ReconstructedVolume{ + Spec: volumeSpec, + }, nil } // CheckVolumeExistenceOperation checks mount path directory if volume still exists diff --git a/pkg/volume/vsphere_volume/vsphere_volume.go b/pkg/volume/vsphere_volume/vsphere_volume.go index 15072690373..6f7d02974c5 100644 --- a/pkg/volume/vsphere_volume/vsphere_volume.go +++ b/pkg/volume/vsphere_volume/vsphere_volume.go @@ -153,17 +153,17 @@ func (plugin *vsphereVolumePlugin) newUnmounterInternal(volName string, podUID t }}, nil } -func (plugin *vsphereVolumePlugin) ConstructVolumeSpec(volumeName, mountPath string) (*volume.Spec, error) { +func (plugin *vsphereVolumePlugin) ConstructVolumeSpec(volumeName, mountPath string) (volume.ReconstructedVolume, error) { mounter := plugin.host.GetMounter(plugin.GetPluginName()) kvh, ok := plugin.host.(volume.KubeletVolumeHost) if !ok { - return nil, fmt.Errorf("plugin volume host does not implement KubeletVolumeHost interface") + return volume.ReconstructedVolume{}, fmt.Errorf("plugin volume host does not implement KubeletVolumeHost interface") } hu := kvh.GetHostUtil() pluginMntDir := util.GetPluginMountDir(plugin.host, plugin.GetPluginName()) volumePath, err := hu.GetDeviceNameFromMount(mounter, mountPath, pluginMntDir) if err != nil { - return nil, err + return volume.ReconstructedVolume{}, err } volumePath = strings.Replace(volumePath, "\\040", " ", -1) klog.V(5).Infof("vSphere volume path is %q", volumePath) @@ -175,7 +175,9 @@ func (plugin *vsphereVolumePlugin) ConstructVolumeSpec(volumeName, mountPath str }, }, } - return volume.NewSpecFromVolume(vsphereVolume), nil + return volume.ReconstructedVolume{ + Spec: volume.NewSpecFromVolume(vsphereVolume), + }, nil } // Abstract interface to disk operations.