Merge pull request #11802 from jiangyaoguo/new-builder-cleaner-for-iscis

pkg/volume/iscsi/disk_manager.go

@@ -27,14 +27,14 @@ import (
 type diskManager interface {
 	MakeGlobalPDName(disk iscsiDisk) string
 	// Attaches the disk to the kubelet's host machine.
-	AttachDisk(disk iscsiDisk) error
+	AttachDisk(b iscsiDiskBuilder) error
 	// Detaches the disk from the kubelet's host machine.
-	DetachDisk(disk iscsiDisk, mntPath string) error
+	DetachDisk(disk iscsiDiskCleaner, mntPath string) error
 }
 
 // utility to mount a disk based filesystem
-func diskSetUp(manager diskManager, disk iscsiDisk, volPath string, mounter mount.Interface) error {
-	globalPDPath := manager.MakeGlobalPDName(disk)
+func diskSetUp(manager diskManager, b iscsiDiskBuilder, volPath string, mounter mount.Interface) error {
+	globalPDPath := manager.MakeGlobalPDName(*b.iscsiDisk)
 	// TODO: handle failed mounts here.
 	mountpoint, err := mounter.IsMountPoint(volPath)
 
@@ -45,7 +45,7 @@ func diskSetUp(manager diskManager, disk iscsiDisk, volPath string, mounter moun
 	if mountpoint {
 		return nil
 	}
-	if err := manager.AttachDisk(disk); err != nil {
+	if err := manager.AttachDisk(b); err != nil {
 		glog.Errorf("failed to attach disk")
 		return err
 	}
@@ -56,7 +56,7 @@ func diskSetUp(manager diskManager, disk iscsiDisk, volPath string, mounter moun
 	}
 	// Perform a bind mount to the full path to allow duplicate mounts of the same disk.
 	options := []string{"bind"}
-	if disk.readOnly {
+	if b.readOnly {
 		options = append(options, "ro")
 	}
 	err = mounter.Mount(globalPDPath, volPath, "", options)
@@ -68,7 +68,7 @@ func diskSetUp(manager diskManager, disk iscsiDisk, volPath string, mounter moun
 }
 
 // utility to tear down a disk based filesystem
-func diskTearDown(manager diskManager, disk iscsiDisk, volPath string, mounter mount.Interface) error {
+func diskTearDown(manager diskManager, c iscsiDiskCleaner, volPath string, mounter mount.Interface) error {
 	mountpoint, err := mounter.IsMountPoint(volPath)
 	if err != nil {
 		glog.Errorf("cannot validate mountpoint %s", volPath)
@@ -91,7 +91,7 @@ func diskTearDown(manager diskManager, disk iscsiDisk, volPath string, mounter m
 	// remaining reference is the global mount. It is safe to detach.
 	if len(refs) == 1 {
 		mntPath := refs[0]
-		if err := manager.DetachDisk(disk, mntPath); err != nil {
+		if err := manager.DetachDisk(c, mntPath); err != nil {
 			glog.Errorf("failed to detach disk from %s", mntPath)
 			return err
 		}

pkg/volume/iscsi/iscsi.go

@@ -89,17 +89,19 @@ func (plugin *iscsiPlugin) newBuilderInternal(spec *volume.Spec, podUID types.UI
 
 	lun := strconv.Itoa(iscsi.Lun)
 
-	return &iscsiDisk{
-		podUID:   podUID,
-		volName:  spec.Name,
-		portal:   iscsi.TargetPortal,
-		iqn:      iscsi.IQN,
-		lun:      lun,
+	return &iscsiDiskBuilder{
+		iscsiDisk: &iscsiDisk{
+			podUID:  podUID,
+			volName: spec.Name,
+			portal:  iscsi.TargetPortal,
+			iqn:     iscsi.IQN,
+			lun:     lun,
+			manager: manager,
+			mounter: mounter,
+			plugin:  plugin},
+
 		fsType:   iscsi.FSType,
 		readOnly: iscsi.ReadOnly,
-		manager:  manager,
-		mounter:  mounter,
-		plugin:   plugin,
 	}, nil
 }
 
@@ -109,13 +111,13 @@ func (plugin *iscsiPlugin) NewCleaner(volName string, podUID types.UID, mounter
 }
 
 func (plugin *iscsiPlugin) newCleanerInternal(volName string, podUID types.UID, manager diskManager, mounter mount.Interface) (volume.Cleaner, error) {
-	return &iscsiDisk{
+	return &iscsiDiskCleaner{&iscsiDisk{
 		podUID:  podUID,
 		volName: volName,
 		manager: manager,
 		mounter: mounter,
 		plugin:  plugin,
-	}, nil
+	}}, nil
 }
 
 func (plugin *iscsiPlugin) execCommand(command string, args []string) ([]byte, error) {
@@ -124,15 +126,13 @@ func (plugin *iscsiPlugin) execCommand(command string, args []string) ([]byte, e
 }
 
 type iscsiDisk struct {
-	volName  string
-	podUID   types.UID
-	portal   string
-	iqn      string
-	readOnly bool
-	lun      string
-	fsType   string
-	plugin   *iscsiPlugin
-	mounter  mount.Interface
+	volName string
+	podUID  types.UID
+	portal  string
+	iqn     string
+	lun     string
+	plugin  *iscsiPlugin
+	mounter mount.Interface
 	// Utility interface that provides API calls to the provider to attach/detach disks.
 	manager diskManager
 }
@@ -143,33 +143,47 @@ func (iscsi *iscsiDisk) GetPath() string {
 	return iscsi.plugin.host.GetPodVolumeDir(iscsi.podUID, util.EscapeQualifiedNameForDisk(name), iscsi.volName)
 }
 
-func (iscsi *iscsiDisk) SetUp() error {
-	return iscsi.SetUpAt(iscsi.GetPath())
+type iscsiDiskBuilder struct {
+	*iscsiDisk
+	readOnly bool
+	fsType   string
+}
+
+var _ volume.Builder = &iscsiDiskBuilder{}
+
+func (b *iscsiDiskBuilder) SetUp() error {
+	return b.SetUpAt(b.GetPath())
 }
 
-func (iscsi *iscsiDisk) SetUpAt(dir string) error {
+func (b *iscsiDiskBuilder) SetUpAt(dir string) error {
 	// diskSetUp checks mountpoints and prevent repeated calls
-	err := diskSetUp(iscsi.manager, *iscsi, dir, iscsi.mounter)
+	err := diskSetUp(b.manager, *b, dir, b.mounter)
 	if err != nil {
 		glog.Errorf("iscsi: failed to setup")
 		return err
 	}
-	globalPDPath := iscsi.manager.MakeGlobalPDName(*iscsi)
+	globalPDPath := b.manager.MakeGlobalPDName(*b.iscsiDisk)
 	var options []string
-	if iscsi.readOnly {
+	if b.readOnly {
 		options = []string{"remount", "ro"}
 	} else {
 		options = []string{"remount", "rw"}
 	}
-	return iscsi.mounter.Mount(globalPDPath, dir, "", options)
+	return b.mounter.Mount(globalPDPath, dir, "", options)
 }
 
+type iscsiDiskCleaner struct {
+	*iscsiDisk
+}
+
+var _ volume.Cleaner = &iscsiDiskCleaner{}
+
 // Unmounts the bind mount, and detaches the disk only if the disk
 // resource was the last reference to that disk on the kubelet.
-func (iscsi *iscsiDisk) TearDown() error {
-	return iscsi.TearDownAt(iscsi.GetPath())
+func (c *iscsiDiskCleaner) TearDown() error {
+	return c.TearDownAt(c.GetPath())
 }
 
-func (iscsi *iscsiDisk) TearDownAt(dir string) error {
-	return diskTearDown(iscsi.manager, *iscsi, dir, iscsi.mounter)
+func (c *iscsiDiskCleaner) TearDownAt(dir string) error {
+	return diskTearDown(c.manager, *c, dir, c.mounter)
 }

pkg/volume/iscsi/iscsi_test.go

@@ -72,22 +72,22 @@ type fakeDiskManager struct {
 func (fake *fakeDiskManager) MakeGlobalPDName(disk iscsiDisk) string {
 	return "/tmp/fake_iscsi_path"
 }
-func (fake *fakeDiskManager) AttachDisk(disk iscsiDisk) error {
-	globalPath := disk.manager.MakeGlobalPDName(disk)
+func (fake *fakeDiskManager) AttachDisk(b iscsiDiskBuilder) error {
+	globalPath := b.manager.MakeGlobalPDName(*b.iscsiDisk)
 	err := os.MkdirAll(globalPath, 0750)
 	if err != nil {
 		return err
 	}
 	// Simulate the global mount so that the fakeMounter returns the
 	// expected number of mounts for the attached disk.
-	disk.mounter.Mount(globalPath, globalPath, disk.fsType, nil)
+	b.mounter.Mount(globalPath, globalPath, b.fsType, nil)
 
 	fake.attachCalled = true
 	return nil
 }
 
-func (fake *fakeDiskManager) DetachDisk(disk iscsiDisk, mntPath string) error {
-	globalPath := disk.manager.MakeGlobalPDName(disk)
+func (fake *fakeDiskManager) DetachDisk(c iscsiDiskCleaner, mntPath string) error {
+	globalPath := c.manager.MakeGlobalPDName(*c.iscsiDisk)
 	err := os.RemoveAll(globalPath)
 	if err != nil {
 		return err

pkg/volume/iscsi/iscsi_util.go

@@ -78,18 +78,18 @@ func (util *ISCSIUtil) MakeGlobalPDName(iscsi iscsiDisk) string {
 	return makePDNameInternal(iscsi.plugin.host, iscsi.portal, iscsi.iqn, iscsi.lun)
 }
 
-func (util *ISCSIUtil) AttachDisk(iscsi iscsiDisk) error {
-	devicePath := strings.Join([]string{"/dev/disk/by-path/ip", iscsi.portal, "iscsi", iscsi.iqn, "lun", iscsi.lun}, "-")
+func (util *ISCSIUtil) AttachDisk(b iscsiDiskBuilder) error {
+	devicePath := strings.Join([]string{"/dev/disk/by-path/ip", b.portal, "iscsi", b.iqn, "lun", b.lun}, "-")
 	exist := waitForPathToExist(devicePath, 1)
 	if exist == false {
 		// discover iscsi target
-		out, err := iscsi.plugin.execCommand("iscsiadm", []string{"-m", "discovery", "-t", "sendtargets", "-p", iscsi.portal})
+		out, err := b.plugin.execCommand("iscsiadm", []string{"-m", "discovery", "-t", "sendtargets", "-p", b.portal})
 		if err != nil {
-			glog.Errorf("iscsi: failed to sendtargets to portal %s error: %s", iscsi.portal, string(out))
+			glog.Errorf("iscsi: failed to sendtargets to portal %s error: %s", b.portal, string(out))
 			return err
 		}
 		// login to iscsi target
-		out, err = iscsi.plugin.execCommand("iscsiadm", []string{"-m", "node", "-p", iscsi.portal, "-T", iscsi.iqn, "--login"})
+		out, err = b.plugin.execCommand("iscsiadm", []string{"-m", "node", "-p", b.portal, "-T", b.iqn, "--login"})
 		if err != nil {
 			glog.Errorf("iscsi: failed to attach disk:Error: %s (%v)", string(out), err)
 			return err
@@ -100,8 +100,8 @@ func (util *ISCSIUtil) AttachDisk(iscsi iscsiDisk) error {
 		}
 	}
 	// mount it
-	globalPDPath := iscsi.manager.MakeGlobalPDName(iscsi)
-	mountpoint, err := iscsi.mounter.IsMountPoint(globalPDPath)
+	globalPDPath := b.manager.MakeGlobalPDName(*b.iscsiDisk)
+	mountpoint, err := b.mounter.IsMountPoint(globalPDPath)
 	if mountpoint {
 		glog.Infof("iscsi: %s already mounted", globalPDPath)
 		return nil
@@ -112,21 +112,21 @@ func (util *ISCSIUtil) AttachDisk(iscsi iscsiDisk) error {
 		return err
 	}
 
-	err = iscsi.mounter.Mount(devicePath, globalPDPath, iscsi.fsType, nil)
+	err = b.mounter.Mount(devicePath, globalPDPath, b.fsType, nil)
 	if err != nil {
-		glog.Errorf("iscsi: failed to mount iscsi volume %s [%s] to %s, error %v", devicePath, iscsi.fsType, globalPDPath, err)
+		glog.Errorf("iscsi: failed to mount iscsi volume %s [%s] to %s, error %v", devicePath, b.fsType, globalPDPath, err)
 	}
 
 	return err
 }
 
-func (util *ISCSIUtil) DetachDisk(iscsi iscsiDisk, mntPath string) error {
-	device, cnt, err := mount.GetDeviceNameFromMount(iscsi.mounter, mntPath)
+func (util *ISCSIUtil) DetachDisk(c iscsiDiskCleaner, mntPath string) error {
+	device, cnt, err := mount.GetDeviceNameFromMount(c.mounter, mntPath)
 	if err != nil {
 		glog.Errorf("iscsi detach disk: failed to get device from mnt: %s\nError: %v", mntPath, err)
 		return err
 	}
-	if err = iscsi.mounter.Unmount(mntPath); err != nil {
+	if err = c.mounter.Unmount(mntPath); err != nil {
 		glog.Errorf("iscsi detach disk: failed to unmount: %s\nError: %v", mntPath, err)
 		return err
 	}
@@ -136,7 +136,7 @@ func (util *ISCSIUtil) DetachDisk(iscsi iscsiDisk, mntPath string) error {
 		// strip -lun- from device path
 		ind := strings.LastIndex(device, "-lun-")
 		prefix := device[:(ind - 1)]
-		refCount, err := getDevicePrefixRefCount(iscsi.mounter, prefix)
+		refCount, err := getDevicePrefixRefCount(c.mounter, prefix)
 
 		if err == nil && refCount == 0 {
 			// this portal/iqn are no longer referenced, log out
@@ -146,7 +146,7 @@ func (util *ISCSIUtil) DetachDisk(iscsi iscsiDisk, mntPath string) error {
 			iqn := device[ind1+len("-iscsi-") : ind]
 
 			glog.Infof("iscsi: log out target %s iqn %s", portal, iqn)
-			out, err := iscsi.plugin.execCommand("iscsiadm", []string{"-m", "node", "-p", portal, "-T", iqn, "--logout"})
+			out, err := c.plugin.execCommand("iscsiadm", []string{"-m", "node", "-p", portal, "-T", iqn, "--logout"})
 			if err != nil {
 				glog.Errorf("iscsi: failed to detach disk Error: %s", string(out))
 			}