mock symlink evaluation

resource/lvm/lowlevel/exec.go

@@ -19,6 +19,7 @@ import (
 	"io/ioutil"
 	"os"
 	"os/exec"
+	"path/filepath"
 	"strings"
 	"syscall"
 
@@ -44,6 +45,9 @@ type Exec interface {
 	WriteFile(fn string, c []byte, p os.FileMode) error
 	MkdirAll(path string, perm os.FileMode) error
 	Exists(path string) (bool, error)
+
+	// Local Filesystem Functions
+	EvalSymlinks(string) (string, error)
 }
 
 type osExec struct {
@@ -54,6 +58,10 @@ func MakeOsExec() Exec {
 	return &osExec{}
 }
 
+func (*osExec) EvalSymlinks(path string) (string, error) {
+	return filepath.EvalSymlinks(path)
+}
+
 func (*osExec) Run(prog string, args []string) error {
 	log.WithField("module", "lvm").Infof("Executing %s: %v", prog, args)
 	e := exec.Command(prog, args...).Run()

resource/lvm/lowlevel/util.go

@@ -16,6 +16,7 @@ package lowlevel
 
 import (
 	"fmt"
+	"path/filepath"
 
 	"github.com/asteris-llc/converge/resource/wait"
 	"github.com/pkg/errors"
@@ -66,28 +67,52 @@ func MakeRealLVM(backend Exec) LVM {
 
 func (lvm *realLVM) CreateVolumeGroup(vg string, devs []string) error {
 	args := []string{vg}
-	args = append(args, devs...)
+	var canonicalDevs []string
+	for _, dev := range devs {
+		canonicalDev, err := lvm.backend.EvalSymlinks(dev)
+		if err != nil {
+			return errors.Wrap(err, "resolving symlink for "+dev)
+		}
+		canonicalDevs = append(canonicalDevs, canonicalDev)
+	}
+	args = append(args, canonicalDevs...)
 	return lvm.backend.Run("vgcreate", args)
 }
 
 func (lvm *realLVM) ExtendVolumeGroup(vg string, dev string) error {
-	return lvm.backend.Run("vgextend", []string{vg, dev})
+	canonicalDev, err := lvm.backend.EvalSymlinks(dev)
+	if err != nil {
+		return err
+	}
+	return lvm.backend.Run("vgextend", []string{vg, canonicalDev})
 }
 
 func (lvm *realLVM) ReduceVolumeGroup(vg string, dev string) error {
-	return lvm.backend.Run("vgreduce", []string{vg, dev})
+	canonicalDev, err := lvm.backend.EvalSymlinks(dev)
+	if err != nil {
+		return err
+	}
+	return lvm.backend.Run("vgreduce", []string{vg, canonicalDev})
 }
 
 func (lvm *realLVM) CreatePhysicalVolume(dev string) error {
-	return lvm.backend.Run("pvcreate", []string{dev})
+	canonicalDev, err := lvm.backend.EvalSymlinks(dev)
+	if err != nil {
+		return err
+	}
+	return lvm.backend.Run("pvcreate", []string{canonicalDev})
 }
 
 func (lvm *realLVM) RemovePhysicalVolume(dev string, force bool) error {
+	canonicalDev, err := lvm.backend.EvalSymlinks(dev)
+	if err != nil {
+		return err
+	}
 	args := []string{}
 	if force {
 		args = append(args, "--force", "--force", "--yes")
 	}
-	args = append(args, dev)
+	args = append(args, canonicalDev)
 	return lvm.backend.Run("pvremove", args)
 }
 
@@ -98,7 +123,11 @@ func (lvm *realLVM) CreateLogicalVolume(group string, volume string, size *LvmSi
 }
 
 func (lvm *realLVM) Mkfs(dev string, fstype string) error {
-	return lvm.backend.Run("mkfs", []string{"-t", fstype, dev})
+	canonicalDev, err := lvm.backend.EvalSymlinks(dev)
+	if err != nil {
+		return err
+	}
+	return lvm.backend.Run("mkfs", []string{"-t", fstype, canonicalDev})
 }
 
 func (lvm *realLVM) Mountpoint(path string) (bool, error) {
@@ -153,3 +182,17 @@ func (lvm *realLVM) WaitForDevice(path string) error {
 	}
 	return nil
 }
+
+// evalDeviceSymlinks returns the real path of deach device (otherwise it breaks
+// on GCE)
+func evalDeviceSymlinks(devices []string) ([]string, error) {
+	realpaths := make([]string, len(devices))
+	for idx, dev := range devices {
+		realpath, err := filepath.EvalSymlinks(dev)
+		if err != nil {
+			return realpaths, errors.Wrap(err, "unable to resolve path: "+dev)
+		}
+		realpaths[idx] = realpath
+	}
+	return realpaths, nil
+}

resource/lvm/testhelpers/lvm.go

@@ -15,6 +15,8 @@
 package testhelpers
 
 import (
+	"fmt"
+
 	"github.com/asteris-llc/converge/resource/lvm/lowlevel"
 	"github.com/stretchr/testify/mock"
 )
@@ -31,6 +33,12 @@ func MakeFakeLvm() (lowlevel.LVM, *FakeLVM) {
 	return lvm, lvm
 }
 
+// EvalSymlinks mocks symlink evaluation
+func (*FakeLVM) EvalSymlinks(s string) (string, error) {
+	fmt.Println("calling fakelvm eval symlinks with ", s)
+	return s, nil
+}
+
 // Check is mock for LVM.Check()
 func (f *FakeLVM) Check() error {
 	return f.Called().Error(0)

resource/lvm/testhelpers/mock.go

@@ -15,9 +15,10 @@
 package testhelpers
 
 import (
+	"os"
+
 	"github.com/asteris-llc/converge/resource/lvm/lowlevel"
 	"github.com/stretchr/testify/mock"
-	"os"
 )
 
 // MockExecutor is a lowlevel.Exec impleentation for faking system interoperation
@@ -96,3 +97,8 @@ func (mex *MockExecutor) Exists(path string) (bool, error) {
 func (mex *MockExecutor) Getuid() int {
 	return mex.Called().Int(0)
 }
+
+// EvalSymlinks mocks symlink evaluation
+func (mex *MockExecutor) EvalSymlinks(s string) (string, error) {
+	return s, nil
+}

resource/lvm/vg/preparer.go

@@ -15,8 +15,6 @@
 package vg
 
 import (
-	"path/filepath"
-
 	"golang.org/x/net/context"
 
 	"github.com/asteris-llc/converge/load/registry"
@@ -46,18 +44,7 @@ type Preparer struct {
 
 // Prepare a new task
 func (p *Preparer) Prepare(_ context.Context, render resource.Renderer) (resource.Task, error) {
-	// Device paths need to be real devices, not symlinks
-	// (otherwise it breaks on GCE)
-	devices := make([]string, len(p.Devices))
-	for i, dev := range p.Devices {
-		var err error
-		devices[i], err = filepath.EvalSymlinks(dev)
-		if err != nil {
-			return nil, err
-		}
-	}
-
-	rvg := NewResourceVG(lowlevel.MakeLvmBackend(), p.Name, devices, p.Remove, p.ForceRemove)
+	rvg := NewResourceVG(lowlevel.MakeLvmBackend(), p.Name, p.Devices, p.Remove, p.ForceRemove)
 	return rvg, nil
 }
 

resource/lvm/vg/vg_test.go

@@ -205,13 +205,14 @@ func TestCreateVolume(t *testing.T) {
 
 		me.On("Read", "pvs", mock.Anything).Return("", nil)
 		me.On("Read", "vgs", mock.Anything).Return("", nil)
+
 		me.On("Run", "vgcreate", []string{"vg0", "/dev/sda1"}).Return(nil)
 
 		fr := fakerenderer.New()
 
 		r := vg.NewResourceVG(lvm, "vg0", []string{"/dev/sda1"}, false, false)
 		status, err := r.Check(context.Background(), fr)
-		assert.NoError(t, err)
+		require.NoError(t, err)
 		assert.True(t, status.HasChanges())
 		comparison.AssertDiff(t, status.Diffs(), "vg0", "<not exists>", "/dev/sda1")