Refactored based on code review comments (WiP)

Signed-off-by: Mohamed Mohamed <mmohamed@us.ibm.com>

.gitignore

@@ -1,3 +1,4 @@
 pkg
 bin
 *coverprofile
+.DS_Store

client.go

@@ -10,6 +10,6 @@ type Manager interface {
 //go:generate counterfeiter -o volmanfakes/fake_driver_client.go . DriverPlugin
 
 type DriverPlugin interface {
-	ListDrivers(logger lager.Logger) ([]Driver, error)
-	Mount(logger lager.Logger, driver Driver, volumeId string, config string) (string, error)
+	Info(logger lager.Logger) (DriverInfo, error)
+	Mount(logger lager.Logger, volumeId string, config string) (string, error)
 }

cmd/volman/main_test.go

@@ -49,7 +49,7 @@ var _ = Describe("Volman", func() {
 		})
 		Context("driver installed in temp directory", func() {
 			BeforeEach(func() {
-				copyFile(fakeDriverPath, tmpDriversPath+"/fake_driver")
+				copyFile(fakeDriverPath, tmpDriversPath+"/fakedriver")
 			})
 
 			It("should return list of drivers for '/v1/drivers' (200 status)", func() {
@@ -59,7 +59,7 @@ var _ = Describe("Volman", func() {
 
 				Expect(err).NotTo(HaveOccurred())
 				Expect(len(drivers.Drivers)).To(Equal(1))
-				Expect(drivers.Drivers[0].Name).To(Equal("FakeDriver"))
+				Expect(drivers.Drivers[0].Name).To(Equal("fakedriver"))
 			})
 
 			It("should mount a volume, given a driver name, volume id, and opaque blob of configuration", func() {
@@ -69,7 +69,7 @@ var _ = Describe("Volman", func() {
 				volumeId := "fake-volume"
 				config := "Here is some config!"
 
-				mountPoint, err := client.Mount(testLogger, "FakeDriver", volumeId, config)
+				mountPoint, err := client.Mount(testLogger, "fakedriver", volumeId, config)
 
 				Expect(err).NotTo(HaveOccurred())
 				Expect(mountPoint.Path).NotTo(Equal(""))
@@ -91,7 +91,7 @@ var _ = Describe("Volman", func() {
 			})
 
 			AfterEach(func() {
-				os.Remove(tmpDriversPath + "/fake_driver")
+				os.Remove(tmpDriversPath + "/fakedriver")
 			})
 		})
 	})

fakedriver/fakedriver.go

@@ -15,11 +15,12 @@ type InfoCommand struct {
 }
 
 func (x *InfoCommand) Execute(args []string) error {
-	driver := volman.Driver{
-		Name: "FakeDriver",
+	driverInfo := volman.DriverInfo{
+		Name: "fakedriver",
+		Path: "/fake/path",
 	}
 
-	jsonBlob, err := json.Marshal(driver)
+	jsonBlob, err := json.Marshal(driverInfo)
 	if err != nil {
 		panic("Error Marshaling the driver")
 	}
@@ -35,7 +36,6 @@ type MountCommand struct {
 func (x *MountCommand) Execute(args []string) error {
 
 	tmpDriversPath, err := ioutil.TempDir("", "fakeDriverMountPoint")
-
 	mountPoint := volman.MountPointResponse{tmpDriversPath}
 
 	jsonBlob, err := json.Marshal(mountPoint)

resources.go

@@ -12,18 +12,15 @@ var Routes = rata.Routes{
 	{Path: "/drivers/mount/", Method: "POST", Name: MountRoute},
 }
 
-type Driver struct {
-	Name   string `json:"name"`
-	Binary string `json:"binary,omitempty"`
+type DriverInfo struct {
+	Name string `json:"name"`
+	Path string `json:"path,omitempty"`
 }
 
 type ListDriversResponse struct {
 	Drivers []DriverInfo `json:"drivers"`
 }
 
-type DriverInfo struct {
-	Name string `json:"name"`
-}
 
 type MountPointRequest struct {
 	DriverId string `json:"driverId"`

voldriver/driver.go

@@ -0,0 +1,49 @@
+package voldriver
+import (
+	"github.com/cloudfoundry-incubator/volman/system"
+	"github.com/cloudfoundry-incubator/volman"
+	"github.com/pivotal-golang/lager"
+	"fmt"
+)
+
+type DriverClientCli struct {
+	UseExec     system.Exec
+	DriversPath string
+	Name string
+}
+
+func NewDriverClientCli(path string, useExec system.Exec, name string) *DriverClientCli{
+	return &DriverClientCli{
+		UseExec:     useExec,
+		DriversPath: path,
+		Name: name,
+	}
+}
+
+func (client *DriverClientCli) Mount(logger lager.Logger, volumeId string, config string) (string, error) {
+	logger.Info("start")
+	defer logger.Info("end")
+	response := struct {
+		Path string `json:"path"`
+	}{}
+	invoker := NewCliInvoker(client.UseExec, fmt.Sprintf("%s/%s",client.DriversPath,client.Name), "mount", volumeId, config)
+	err := invoker.InvokeDriver(logger, &response)
+	if err != nil {
+		return "", err
+	}
+
+	return response.Path, nil
+}
+
+
+func (client *DriverClientCli) Info(logger lager.Logger) (volman.DriverInfo, error) {
+	logger.Info("start")
+	defer logger.Info("end")
+	response := volman.DriverInfo{}
+	invoker := NewCliInvoker(client.UseExec, fmt.Sprintf("%s/%s",client.DriversPath,client.Name), "info")
+	err := invoker.InvokeDriver(logger, &response)
+	if err != nil {
+		return volman.DriverInfo{}, err
+	}
+	return response, nil
+}

voldriver/driver_suite_test.go

@@ -0,0 +1,51 @@
+package voldriver_test
+
+import (
+	"fmt"
+	"io"
+	"path/filepath"
+	"strings"
+
+	"github.com/cloudfoundry-incubator/volman"
+	. "github.com/onsi/ginkgo"
+	. "github.com/onsi/gomega"
+	"github.com/onsi/gomega/gexec"
+
+	"testing"
+)
+
+var client volman.Manager
+var fakeDriverPath string
+
+func TestDriver(t *testing.T) {
+	RegisterFailHandler(Fail)
+	RunSpecs(t, "Driver Suite")
+}
+
+var _ = SynchronizedBeforeSuite(func() []byte {
+	var err error
+
+	fakeDriverPath, err = gexec.Build("github.com/cloudfoundry-incubator/volman/fakedriver", "-race")
+	Expect(err).NotTo(HaveOccurred())
+	return []byte(strings.Join([]string{fakeDriverPath}, ","))
+}, func(pathsByte []byte) {
+	path := string(pathsByte)
+	fakeDriverPath = filepath.Dir(strings.Split(path, ",")[0])
+})
+
+var _ = SynchronizedAfterSuite(func() {
+
+}, func() {
+	gexec.CleanupBuildArtifacts()
+})
+
+// testing support types:
+
+type errCloser struct{ io.Reader }
+
+func (errCloser) Close() error                     { return nil }
+func (errCloser) Read(p []byte) (n int, err error) { return 0, fmt.Errorf("any") }
+
+type stringCloser struct{ io.Reader }
+
+func (stringCloser) Close() error { return nil }

voldriver/driver_test.go

@@ -0,0 +1,95 @@
+package voldriver_test
+
+import (
+	"bytes"
+	"io"
+
+	"github.com/cloudfoundry-incubator/volman"
+	"github.com/cloudfoundry-incubator/volman/voldriver"
+	"github.com/cloudfoundry-incubator/volman/volmanfakes"
+	. "github.com/onsi/ginkgo"
+	. "github.com/onsi/gomega"
+	"github.com/pivotal-golang/lager/lagertest"
+
+)
+
+var _ = Describe("DriverClientCli", func() {
+	var client volman.DriverPlugin
+	var fakeCmd *volmanfakes.FakeCmd
+	var fakeExec *volmanfakes.FakeExec
+	var validDriverInfoResponse io.ReadCloser
+
+	// Context("when given invalid driver path", func() {
+	// 	BeforeEach(func() {
+	// 		fakeExec = new(volmanfakes.FakeExec)
+	// 		fakeCmd = new(volmanfakes.FakeCmd)
+	// 		fakeExec.CommandReturns(fakeCmd)
+
+	// 		validDriverInfoResponse = stringCloser{bytes.NewBufferString("{\"Name\":\"SomeDriver\"}")}
+
+	// 		client = &voldriver.DriverClientCli{fakeExec, fakeDriverPath}
+	// 	})
+
+
+	// 	It("should not be able to mount", func() {
+	// 		fakeCmd.StdoutPipeReturns(errCloser{bytes.NewBufferString("")}, nil)
+	// 		testLogger := lagertest.NewTestLogger("ClientTest")
+	// 		driver := volman.Driver{
+	// 			Name: "SomeDriver",
+	// 		}
+	// 		volumeId := "fake-volume"
+	// 		config := "Here is some config!"
+
+	// 		_, err := client.Mount(testLogger, driver, volumeId, config)
+	// 		Expect(err).To(HaveOccurred())
+	// 	})
+	// })
+
+	Context("when given valid driver path", func() {
+		BeforeEach(func() {
+			fakeExec = new(volmanfakes.FakeExec)
+			fakeCmd = new(volmanfakes.FakeCmd)
+			fakeExec.CommandReturns(fakeCmd)
+
+			validDriverInfoResponse = stringCloser{bytes.NewBufferString("{\"Name\":\"SomeDriver\"}")}
+
+			client = &voldriver.DriverClientCli{fakeExec,fakeDriverPath,"fakedriver"}
+		})
+
+		It("should not error on get driver info", func() {
+			var validDriverMountResponse = stringCloser{bytes.NewBufferString("{\"Path\":\"/MountPoint\"}")}
+			var stdOutResponses = [...]io.ReadCloser{validDriverMountResponse, validDriverInfoResponse}
+
+			calls := 0
+			fakeCmd.StdoutPipeStub = func() (io.ReadCloser, error) {
+				defer func() { calls++ }()
+				return stdOutResponses[calls], nil
+			}
+
+			testLogger := lagertest.NewTestLogger("ClientTest")
+
+			_,err := client.Info(testLogger)
+			Expect(err).NotTo(HaveOccurred())
+
+		})
+
+		It("should be able to mount", func() {
+			var validDriverMountResponse = stringCloser{bytes.NewBufferString("{\"Path\":\"/MountPoint\"}")}
+			var stdOutResponses = [...]io.ReadCloser{validDriverMountResponse, validDriverInfoResponse}
+
+			calls := 0
+			fakeCmd.StdoutPipeStub = func() (io.ReadCloser, error) {
+				defer func() { calls++ }()
+				return stdOutResponses[calls], nil
+			}
+
+			testLogger := lagertest.NewTestLogger("ClientTest")
+			volumeId := "fake-volume"
+			config := "Here is some config!"
+
+			mountPath, err := client.Mount(testLogger, volumeId, config)
+			Expect(err).NotTo(HaveOccurred())
+			Expect(mountPath).To(Equal("/MountPoint"))
+		})
+	})
+})

vollocal/driverclient/invoker_cli.go → voldriver/invoker_cli.go

@@ -1,4 +1,4 @@
-package driverclient
+package voldriver
 
 import (
 	"encoding/json"
@@ -21,8 +21,9 @@ func NewCliInvoker(useExec system.Exec, executable string, args ...string) CliIn
 }
 
 func (invoker *CliInvoker) InvokeDriver(logger lager.Logger, output interface{}) error {
+	logger.Info("start")
+	defer logger.Info("end")
 	cmd := invoker.UseCmd
-
 	stdout, err := cmd.StdoutPipe()
 	if err != nil {
 		return invoker.DriverError(logger, err, "fetching stdout")

vollocal/driverclient/invoker_cli_test.go → voldriver/invoker_cli_test.go

@@ -1,19 +1,19 @@
-package driverclient_test
+package voldriver_test
 
 import (
 	"bytes"
 	"fmt"
 	"io"
 
-	"github.com/cloudfoundry-incubator/volman/vollocal/driverclient"
+	"github.com/cloudfoundry-incubator/volman/voldriver"
 	"github.com/cloudfoundry-incubator/volman/volmanfakes"
 	. "github.com/onsi/ginkgo"
 	. "github.com/onsi/gomega"
 	"github.com/pivotal-golang/lager/lagertest"
 )
 
 var _ = Describe("Invoker CLI", func() {
-	var subject *driverclient.CliInvoker
+	var subject *voldriver.CliInvoker
 	var fakeCmd *volmanfakes.FakeCmd
 	var fakeExec *volmanfakes.FakeExec
 	var validJson io.ReadCloser
@@ -26,7 +26,7 @@ var _ = Describe("Invoker CLI", func() {
 
 			validJson = stringCloser{bytes.NewBufferString("{\"Name\":\"JSON\"}")}
 
-			subject = &driverclient.CliInvoker{
+			subject = &voldriver.CliInvoker{
 				UseExec: fakeExec,
 				UseCmd:  fakeCmd,
 			}

volhttp/handlers.go

@@ -20,6 +20,8 @@ func respondWithError(logger lager.Logger, info string, err error, w http.Respon
 
 func NewHandler(logger lager.Logger, driversPath string) (http.Handler, error) {
 	logger = logger.Session("server")
+	logger.Info("start")
+	defer logger.Info("end")
 	var handlers = rata.Handlers{
 		"drivers": http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) {
 			client := vollocal.NewLocalClient(driversPath)

volhttp/handlers_suite_test.go

@@ -3,11 +3,42 @@ package volhttp_test
 import (
 	. "github.com/onsi/ginkgo"
 	. "github.com/onsi/gomega"
-
 	"testing"
+	"io/ioutil"
+	"strings"
+	"github.com/onsi/gomega/gexec"
 )
 
+var tmpDriversPath string
+var fakeDriverPath string
+
 func TestHandlers(t *testing.T) {
 	RegisterFailHandler(Fail)
 	RunSpecs(t, "Handlers Suite")
 }
+var _ = SynchronizedBeforeSuite(func() []byte {
+	var err error
+
+
+	fakeDriverPath, err = gexec.Build("github.com/cloudfoundry-incubator/volman/fakedriver", "-race")
+	Expect(err).NotTo(HaveOccurred())
+	return []byte(strings.Join([]string{ fakeDriverPath}, ","))
+}, func(pathsByte []byte) {
+	path := string(pathsByte)
+	fakeDriverPath = strings.Split(path, ",")[0]
+})
+
+var _ = BeforeEach(func() {
+	var err error
+	tmpDriversPath, err = ioutil.TempDir("", "driversPath")
+	Expect(err).NotTo(HaveOccurred())
+
+})
+
+
+
+var _ = SynchronizedAfterSuite(func() {
+
+}, func() {
+	gexec.CleanupBuildArtifacts()
+})