From 5ff9dc0b3bf3d4f73814b34250360d0471e25b1d Mon Sep 17 00:00:00 2001 From: mtanino Date: Wed, 9 Aug 2017 10:24:29 -0400 Subject: [PATCH] WaitForAttach refactoring for iSCSI attacher/detacher This change is prerequisite for implementing iSCSI attacher and detacher. In order to use chap authentication at iSCSI plugin after implementing attacher and detacher, secret is needed at AttachDisk() which is called from WaitForAttach(). To obtain secret, pod information is required, but WaitForAttach() doesn't pass pod information inside. This patch adds 'pod' as an argument of WaitForAttach() and adds changes to drivers who implements WaitForAttach(). Fixes #48953 --- .../volume/attachdetach/testing/testvolumespec.go | 2 +- pkg/volume/aws_ebs/attacher.go | 3 ++- pkg/volume/azure_dd/attacher.go | 2 +- pkg/volume/cinder/attacher.go | 3 ++- pkg/volume/fc/attacher.go | 3 ++- pkg/volume/flexvolume/attacher.go | 3 ++- pkg/volume/flexvolume/attacher_test.go | 8 +++++--- pkg/volume/gce_pd/attacher.go | 3 ++- pkg/volume/photon_pd/attacher.go | 3 ++- pkg/volume/testing/testing.go | 2 +- pkg/volume/util/operationexecutor/operation_generator.go | 2 +- pkg/volume/volume.go | 2 +- pkg/volume/vsphere_volume/attacher.go | 3 ++- 13 files changed, 24 insertions(+), 15 deletions(-) diff --git a/pkg/controller/volume/attachdetach/testing/testvolumespec.go b/pkg/controller/volume/attachdetach/testing/testvolumespec.go index a77eddfed4e..a20f4507421 100644 --- a/pkg/controller/volume/attachdetach/testing/testvolumespec.go +++ b/pkg/controller/volume/attachdetach/testing/testvolumespec.go @@ -377,7 +377,7 @@ func (attacher *testPluginAttacher) VolumesAreAttached(specs []*volume.Spec, nod return nil, nil } -func (attacher *testPluginAttacher) WaitForAttach(spec *volume.Spec, devicePath string, timeout time.Duration) (string, error) { +func (attacher *testPluginAttacher) WaitForAttach(spec *volume.Spec, devicePath string, pod *v1.Pod, timeout time.Duration) (string, error) { attacher.pluginLock.Lock() defer attacher.pluginLock.Unlock() if spec == nil { diff --git a/pkg/volume/aws_ebs/attacher.go b/pkg/volume/aws_ebs/attacher.go index 2ffa14f4122..bf141424690 100644 --- a/pkg/volume/aws_ebs/attacher.go +++ b/pkg/volume/aws_ebs/attacher.go @@ -24,6 +24,7 @@ import ( "time" "github.com/golang/glog" + "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/types" "k8s.io/kubernetes/pkg/cloudprovider/providers/aws" "k8s.io/kubernetes/pkg/util/mount" @@ -142,7 +143,7 @@ func (attacher *awsElasticBlockStoreAttacher) BulkVerifyVolumes(volumesByNode ma return volumesAttachedCheck, nil } -func (attacher *awsElasticBlockStoreAttacher) WaitForAttach(spec *volume.Spec, devicePath string, timeout time.Duration) (string, error) { +func (attacher *awsElasticBlockStoreAttacher) WaitForAttach(spec *volume.Spec, devicePath string, _ *v1.Pod, timeout time.Duration) (string, error) { volumeSource, _, err := getVolumeSource(spec) if err != nil { return "", err diff --git a/pkg/volume/azure_dd/attacher.go b/pkg/volume/azure_dd/attacher.go index f473bc4ffb1..0ef1c6c0477 100644 --- a/pkg/volume/azure_dd/attacher.go +++ b/pkg/volume/azure_dd/attacher.go @@ -150,7 +150,7 @@ func (a *azureDiskAttacher) VolumesAreAttached(specs []*volume.Spec, nodeName ty return volumesAttachedCheck, nil } -func (a *azureDiskAttacher) WaitForAttach(spec *volume.Spec, devicePath string, timeout time.Duration) (string, error) { +func (a *azureDiskAttacher) WaitForAttach(spec *volume.Spec, devicePath string, _ *v1.Pod, timeout time.Duration) (string, error) { var err error lun, err := strconv.Atoi(devicePath) if err != nil { diff --git a/pkg/volume/cinder/attacher.go b/pkg/volume/cinder/attacher.go index 53dc22a8e4e..f6d1e4be699 100644 --- a/pkg/volume/cinder/attacher.go +++ b/pkg/volume/cinder/attacher.go @@ -24,6 +24,7 @@ import ( "time" "github.com/golang/glog" + "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/wait" "k8s.io/kubernetes/pkg/cloudprovider" @@ -220,7 +221,7 @@ func (attacher *cinderDiskAttacher) VolumesAreAttached(specs []*volume.Spec, nod return volumesAttachedCheck, nil } -func (attacher *cinderDiskAttacher) WaitForAttach(spec *volume.Spec, devicePath string, timeout time.Duration) (string, error) { +func (attacher *cinderDiskAttacher) WaitForAttach(spec *volume.Spec, devicePath string, _ *v1.Pod, timeout time.Duration) (string, error) { // NOTE: devicePath is is path as reported by Cinder, which may be incorrect and should not be used. See Issue #33128 volumeSource, _, err := getVolumeSource(spec) if err != nil { diff --git a/pkg/volume/fc/attacher.go b/pkg/volume/fc/attacher.go index 4c855d56bc5..4a8121faee8 100644 --- a/pkg/volume/fc/attacher.go +++ b/pkg/volume/fc/attacher.go @@ -24,6 +24,7 @@ import ( "time" "github.com/golang/glog" + "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/types" "k8s.io/kubernetes/pkg/util/mount" "k8s.io/kubernetes/pkg/volume" @@ -65,7 +66,7 @@ func (attacher *fcAttacher) VolumesAreAttached(specs []*volume.Spec, nodeName ty return volumesAttachedCheck, nil } -func (attacher *fcAttacher) WaitForAttach(spec *volume.Spec, devicePath string, timeout time.Duration) (string, error) { +func (attacher *fcAttacher) WaitForAttach(spec *volume.Spec, devicePath string, _ *v1.Pod, timeout time.Duration) (string, error) { mounter, err := volumeSpecToMounter(spec, attacher.host) if err != nil { glog.Warningf("failed to get fc mounter: %v", err) diff --git a/pkg/volume/flexvolume/attacher.go b/pkg/volume/flexvolume/attacher.go index 00f1d5d487d..c2efcffa1c3 100644 --- a/pkg/volume/flexvolume/attacher.go +++ b/pkg/volume/flexvolume/attacher.go @@ -20,6 +20,7 @@ import ( "time" "github.com/golang/glog" + "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/types" "k8s.io/kubernetes/pkg/volume" ) @@ -47,7 +48,7 @@ func (a *flexVolumeAttacher) Attach(spec *volume.Spec, hostName types.NodeName) } // WaitForAttach is part of the volume.Attacher interface -func (a *flexVolumeAttacher) WaitForAttach(spec *volume.Spec, devicePath string, timeout time.Duration) (string, error) { +func (a *flexVolumeAttacher) WaitForAttach(spec *volume.Spec, devicePath string, _ *v1.Pod, timeout time.Duration) (string, error) { call := a.plugin.NewDriverCallWithTimeout(waitForAttachCmd, timeout) call.Append(devicePath) call.AppendSpec(spec, a.plugin.host, nil) diff --git a/pkg/volume/flexvolume/attacher_test.go b/pkg/volume/flexvolume/attacher_test.go index 8807cc81622..f4b4ab7b839 100644 --- a/pkg/volume/flexvolume/attacher_test.go +++ b/pkg/volume/flexvolume/attacher_test.go @@ -17,9 +17,11 @@ limitations under the License. package flexvolume import ( - "k8s.io/kubernetes/pkg/volume" "testing" "time" + + "k8s.io/api/core/v1" + "k8s.io/kubernetes/pkg/volume" ) func TestAttach(t *testing.T) { @@ -37,7 +39,7 @@ func TestAttach(t *testing.T) { func TestWaitForAttach(t *testing.T) { spec := fakeVolumeSpec() - + var pod *v1.Pod plugin, _ := testPlugin() plugin.runner = fakeRunner( assertDriverCall(t, notSupportedOutput(), waitForAttachCmd, "/dev/sdx", @@ -45,7 +47,7 @@ func TestWaitForAttach(t *testing.T) { ) a, _ := plugin.NewAttacher() - a.WaitForAttach(spec, "/dev/sdx", 1*time.Second) + a.WaitForAttach(spec, "/dev/sdx", pod, 1*time.Second) } func TestMountDevice(t *testing.T) { diff --git a/pkg/volume/gce_pd/attacher.go b/pkg/volume/gce_pd/attacher.go index e60f3c6c2f0..588d9aabe76 100644 --- a/pkg/volume/gce_pd/attacher.go +++ b/pkg/volume/gce_pd/attacher.go @@ -25,6 +25,7 @@ import ( "time" "github.com/golang/glog" + "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/sets" "k8s.io/kubernetes/pkg/cloudprovider/providers/gce" @@ -130,7 +131,7 @@ func (attacher *gcePersistentDiskAttacher) VolumesAreAttached(specs []*volume.Sp return volumesAttachedCheck, nil } -func (attacher *gcePersistentDiskAttacher) WaitForAttach(spec *volume.Spec, devicePath string, timeout time.Duration) (string, error) { +func (attacher *gcePersistentDiskAttacher) WaitForAttach(spec *volume.Spec, devicePath string, _ *v1.Pod, timeout time.Duration) (string, error) { ticker := time.NewTicker(checkSleepDuration) defer ticker.Stop() timer := time.NewTimer(timeout) diff --git a/pkg/volume/photon_pd/attacher.go b/pkg/volume/photon_pd/attacher.go index 8bdaba3c10f..4af726716e6 100644 --- a/pkg/volume/photon_pd/attacher.go +++ b/pkg/volume/photon_pd/attacher.go @@ -25,6 +25,7 @@ import ( "time" "github.com/golang/glog" + "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/types" "k8s.io/kubernetes/pkg/cloudprovider/providers/photon" "k8s.io/kubernetes/pkg/util/mount" @@ -121,7 +122,7 @@ func (attacher *photonPersistentDiskAttacher) VolumesAreAttached(specs []*volume return volumesAttachedCheck, nil } -func (attacher *photonPersistentDiskAttacher) WaitForAttach(spec *volume.Spec, devicePath string, timeout time.Duration) (string, error) { +func (attacher *photonPersistentDiskAttacher) WaitForAttach(spec *volume.Spec, devicePath string, _ *v1.Pod, timeout time.Duration) (string, error) { volumeSource, _, err := getVolumeSource(spec) if err != nil { glog.Errorf("Photon Controller attacher: WaitForAttach failed to get volume source") diff --git a/pkg/volume/testing/testing.go b/pkg/volume/testing/testing.go index 435f5683a7d..d2903f5f144 100644 --- a/pkg/volume/testing/testing.go +++ b/pkg/volume/testing/testing.go @@ -435,7 +435,7 @@ func (fv *FakeVolume) GetAttachCallCount() int { return fv.AttachCallCount } -func (fv *FakeVolume) WaitForAttach(spec *Spec, devicePath string, spectimeout time.Duration) (string, error) { +func (fv *FakeVolume) WaitForAttach(spec *Spec, devicePath string, pod *v1.Pod, spectimeout time.Duration) (string, error) { fv.Lock() defer fv.Unlock() fv.WaitForAttachCallCount++ diff --git a/pkg/volume/util/operationexecutor/operation_generator.go b/pkg/volume/util/operationexecutor/operation_generator.go index cceabde48cf..2750b99ea32 100644 --- a/pkg/volume/util/operationexecutor/operation_generator.go +++ b/pkg/volume/util/operationexecutor/operation_generator.go @@ -408,7 +408,7 @@ func (og *operationGenerator) GenerateMountVolumeFunc( glog.Infof(volumeToMount.GenerateMsgDetailed("MountVolume.WaitForAttach entering", fmt.Sprintf("DevicePath %q", volumeToMount.DevicePath))) devicePath, err := volumeAttacher.WaitForAttach( - volumeToMount.VolumeSpec, volumeToMount.DevicePath, waitForAttachTimeout) + volumeToMount.VolumeSpec, volumeToMount.DevicePath, volumeToMount.Pod, waitForAttachTimeout) if err != nil { // On failure, return error. Caller will log and retry. return volumeToMount.GenerateErrorDetailed("MountVolume.WaitForAttach failed", err) diff --git a/pkg/volume/volume.go b/pkg/volume/volume.go index 899cf8aac7f..78cb21a32a7 100644 --- a/pkg/volume/volume.go +++ b/pkg/volume/volume.go @@ -173,7 +173,7 @@ type Attacher interface { // node. If it successfully attaches, the path to the device // is returned. Otherwise, if the device does not attach after // the given timeout period, an error will be returned. - WaitForAttach(spec *Spec, devicePath string, timeout time.Duration) (string, error) + WaitForAttach(spec *Spec, devicePath string, pod *v1.Pod, timeout time.Duration) (string, error) // GetDeviceMountPath returns a path where the device should // be mounted after it is attached. This is a global mount diff --git a/pkg/volume/vsphere_volume/attacher.go b/pkg/volume/vsphere_volume/attacher.go index e87b07d50c5..19edc58ca04 100644 --- a/pkg/volume/vsphere_volume/attacher.go +++ b/pkg/volume/vsphere_volume/attacher.go @@ -23,6 +23,7 @@ import ( "time" "github.com/golang/glog" + "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/types" "k8s.io/kubernetes/pkg/cloudprovider/providers/vsphere" "k8s.io/kubernetes/pkg/util/keymutex" @@ -118,7 +119,7 @@ func (attacher *vsphereVMDKAttacher) VolumesAreAttached(specs []*volume.Spec, no return volumesAttachedCheck, nil } -func (attacher *vsphereVMDKAttacher) WaitForAttach(spec *volume.Spec, devicePath string, timeout time.Duration) (string, error) { +func (attacher *vsphereVMDKAttacher) WaitForAttach(spec *volume.Spec, devicePath string, _ *v1.Pod, timeout time.Duration) (string, error) { volumeSource, _, err := getVolumeSource(spec) if err != nil { return "", err