From ce52ae782d99d863cbfeae151ccf6f1be6a4cb40 Mon Sep 17 00:00:00 2001 From: jiangyaoguo Date: Fri, 24 Jul 2015 16:55:56 +0800 Subject: [PATCH] refector iscsi volume to seperate builder and cleaner --- pkg/volume/iscsi/disk_manager.go | 16 +++---- pkg/volume/iscsi/iscsi.go | 76 +++++++++++++++++++------------- pkg/volume/iscsi/iscsi_test.go | 10 ++--- pkg/volume/iscsi/iscsi_util.go | 28 ++++++------ 4 files changed, 72 insertions(+), 58 deletions(-) diff --git a/pkg/volume/iscsi/disk_manager.go b/pkg/volume/iscsi/disk_manager.go index eb7259aa274..17b453f6703 100644 --- a/pkg/volume/iscsi/disk_manager.go +++ b/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 } diff --git a/pkg/volume/iscsi/iscsi.go b/pkg/volume/iscsi/iscsi.go index 8a7c55a0fea..99064aa9df9 100644 --- a/pkg/volume/iscsi/iscsi.go +++ b/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 } -func (iscsi *iscsiDisk) SetUpAt(dir string) error { +var _ volume.Builder = &iscsiDiskBuilder{} + +func (b *iscsiDiskBuilder) SetUp() error { + return b.SetUpAt(b.GetPath()) +} + +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) } diff --git a/pkg/volume/iscsi/iscsi_test.go b/pkg/volume/iscsi/iscsi_test.go index 3bd8e77e790..5a66dc265f1 100644 --- a/pkg/volume/iscsi/iscsi_test.go +++ b/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 diff --git a/pkg/volume/iscsi/iscsi_util.go b/pkg/volume/iscsi/iscsi_util.go index 1d4f812840b..cfd5c269646 100644 --- a/pkg/volume/iscsi/iscsi_util.go +++ b/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)) }