Merge pull request #25325 from swagiaal/attacher-interface-update

pkg/kubelet/kubelet.go

@@ -2011,11 +2011,11 @@ func (kl *Kubelet) cleanupOrphanedVolumes(pods []*api.Pod, runningPods []*kubeco
 					glog.Errorf("Could not detach volume %q at %q: %v", name, volumePath, err)
 				}
 
-				// TODO(swagiaal): This will block until the sync loop until device is attached
-				// so all of this should be moved to a mount/unmount manager which does it asynchronously
-				if err = detacher.WaitForDetach(devicePath, maxWaitForVolumeOps); err != nil {
-					glog.Errorf("Error while waiting for detach: %v", err)
-				}
+				go func() {
+					if err = detacher.WaitForDetach(devicePath, maxWaitForVolumeOps); err != nil {
+						glog.Errorf("Error while waiting for detach: %v", err)
+					}
+				}()
 			}
 		}
 	}

pkg/kubelet/kubelet_test.go

@@ -559,9 +559,6 @@ func TestCleanupOrphanedVolumes(t *testing.T) {
 	if detacher.DetachCallCount != 1 {
 		t.Errorf("Expected Detach to be called")
 	}
-	if detacher.WaitForDetachCallCount != 1 {
-		t.Errorf("Expected WaitForDetach to be called")
-	}
 	if detacher.UnmountDeviceCallCount != 1 {
 		t.Errorf("Expected UnmountDevice to be called")
 	}

pkg/kubelet/volumes.go

@@ -153,8 +153,8 @@ func (kl *Kubelet) mountExternalVolumes(pod *api.Pod) (kubecontainer.VolumeMap,
 				return nil, err
 			}
 
-			deviceMountPath := attacher.GetDeviceMountPath(volSpec)
-			if err = attacher.MountDevice(devicePath, deviceMountPath, kl.mounter); err != nil {
+			deviceMountPath := attacher.GetDeviceMountPath(&volumeHost{kl}, volSpec)
+			if err = attacher.MountDevice(volSpec, devicePath, deviceMountPath, kl.mounter); err != nil {
 				return nil, err
 			}
 		}
@@ -303,7 +303,7 @@ func (kl *Kubelet) newVolumeAttacherFromPlugins(spec *volume.Spec, pod *api.Pod,
 		return nil, nil
 	}
 
-	attacher, err := plugin.NewAttacher(spec)
+	attacher, err := plugin.NewAttacher()
 	if err != nil {
 		return nil, fmt.Errorf("failed to instantiate volume attacher for %s: %v", spec.Name(), err)
 	}
@@ -348,10 +348,9 @@ func (kl *Kubelet) newVolumeDetacherFromPlugins(kind string, name string, podUID
 		return nil, nil
 	}
 
-	detacher, err := plugin.NewDetacher(name, podUID)
+	detacher, err := plugin.NewDetacher()
 	if err != nil {
 		return nil, fmt.Errorf("failed to instantiate volume plugin for %s/%s: %v", podUID, kind, err)
 	}
-	glog.V(3).Infof("Used volume plugin %q to detach %s/%s", plugin.Name(), podUID, kind)
 	return detacher, nil
 }

pkg/volume/plugins.go

@@ -131,8 +131,8 @@ type ProvisionableVolumePlugin interface {
 // to a node before mounting.
 type AttachableVolumePlugin interface {
 	VolumePlugin
-	NewAttacher(spec *Spec) (Attacher, error)
-	NewDetacher(name string, podUID types.UID) (Detacher, error)
+	NewAttacher() (Attacher, error)
+	NewDetacher() (Detacher, error)
 }
 
 // VolumeHost is an interface that plugins can use to access the kubelet.

pkg/volume/testing/testing.go

@@ -189,12 +189,12 @@ func (plugin *FakeVolumePlugin) NewUnmounter(volName string, podUID types.UID) (
 	return volume, nil
 }
 
-func (plugin *FakeVolumePlugin) NewAttacher(spec *Spec) (Attacher, error) {
+func (plugin *FakeVolumePlugin) NewAttacher() (Attacher, error) {
 	plugin.NewAttacherCallCount = plugin.NewAttacherCallCount + 1
 	return plugin.getFakeVolume(&plugin.Attachers), nil
 }
 
-func (plugin *FakeVolumePlugin) NewDetacher(name string, podUID types.UID) (Detacher, error) {
+func (plugin *FakeVolumePlugin) NewDetacher() (Detacher, error) {
 	plugin.NewDetacherCallCount = plugin.NewDetacherCallCount + 1
 	return plugin.getFakeVolume(&plugin.Detachers), nil
 }
@@ -273,12 +273,12 @@ func (fv *FakeVolume) WaitForAttach(spec *Spec, spectimeout time.Duration) (stri
 	return "", nil
 }
 
-func (fv *FakeVolume) GetDeviceMountPath(spec *Spec) string {
+func (fv *FakeVolume) GetDeviceMountPath(host VolumeHost, spec *Spec) string {
 	fv.GetDeviceMountPathCallCount++
 	return ""
 }
 
-func (fv *FakeVolume) MountDevice(devicePath string, deviceMountPath string, mounter mount.Interface) error {
+func (fv *FakeVolume) MountDevice(spec *Spec, devicePath string, deviceMountPath string, mounter mount.Interface) error {
 	fv.MountDeviceCallCount++
 	return nil
 }

pkg/volume/volume.go

@@ -131,8 +131,6 @@ type Deleter interface {
 
 // Attacher can attach a volume to a node.
 type Attacher interface {
-	Volume
-
 	// Attach the volume specified by the given spec to the given host
 	Attach(spec *Spec, hostName string) error
 
@@ -145,11 +143,11 @@ type Attacher interface {
 	// GetDeviceMountPath returns a path where the device should
 	// be mounted after it is attached. This is a global mount
 	// point which should be bind mounted for individual volumes.
-	GetDeviceMountPath(spec *Spec) string
+	GetDeviceMountPath(host VolumeHost, spec *Spec) string
 
 	// MountDevice mounts the disk to a global path which
 	// individual pods can then bind mount
-	MountDevice(devicePath string, deviceMountPath string, mounter mount.Interface) error
+	MountDevice(spec *Spec, devicePath string, deviceMountPath string, mounter mount.Interface) error
 }
 
 // Detacher can detach a volume from a node.
@@ -159,14 +157,14 @@ type Detacher interface {
 	Detach(deviceMountPath string, hostName string) error
 
 	// WaitForDetach blocks until the device is detached from this
-	// node. If the device does not detach within the given timout
+	// node. If the device does not detach within the given timeout
 	// period an error is returned.
-	WaitForDetach(devicePath string, timout time.Duration) error
+	WaitForDetach(devicePath string, timeout time.Duration) error
 
 	// UnmountDevice unmounts the global mount of the disk. This
 	// should only be called once all bind mounts have been
 	// unmounted.
-	UnmountDevice(globalMountPath string, mounter mount.Interface) error
+	UnmountDevice(deviceMountPath string, mounter mount.Interface) error
 }
 
 func RenameDirectory(oldPath, newName string) (string, error) {