Remove ExecWithResult to enable testing

Author: Harvey Lowndes

pkg/flexvolume/flexvolume.go

@@ -86,26 +86,6 @@ type Driver interface {
 	Unmount(mountDir string) DriverStatus
 }
 
-// ExitWithResult outputs the given Result and exits with the appropriate exit
-// code.
-func ExitWithResult(result DriverStatus) {
-	code := 1
-	if result.Status == StatusSuccess || result.Status == StatusNotSupported {
-		code = 0
-	}
-
-	res, err := json.Marshal(result)
-	if err != nil {
-		log.Printf("Error marshaling result: %v", err)
-		fmt.Fprintln(out, `{"status":"Failure","message":"Error marshaling result to JSON"}`)
-	} else {
-		s := string(res)
-		log.Printf("Command result: %s", s)
-		fmt.Fprintln(out, s)
-	}
-	exit(code)
-}
-
 // Fail creates a StatusFailure Result with a given message.
 func Fail(a ...interface{}) DriverStatus {
 	msg := fmt.Sprint(a...)
@@ -115,6 +95,15 @@ func Fail(a ...interface{}) DriverStatus {
 	}
 }
 
+// Failf creates a StatusFailure Result with a given message.
+func Failf(s string, a ...interface{}) DriverStatus {
+	msg := fmt.Sprintf(s, a...)
+	return DriverStatus{
+		Status:  StatusFailure,
+		Message: msg,
+	}
+}
+
 // Succeed creates a StatusSuccess Result with a given message.
 func Succeed(a ...interface{}) DriverStatus {
 	return DriverStatus{
@@ -123,6 +112,14 @@ func Succeed(a ...interface{}) DriverStatus {
 	}
 }
 
+// NotSupportedf creates a StatusNotSupported Result with a given message.
+func NotSupportedf(s string, a ...interface{}) DriverStatus {
+	return DriverStatus{
+		Status:  StatusNotSupported,
+		Message: fmt.Sprintf(s, a...),
+	}
+}
+
 // NotSupported creates a StatusNotSupported Result with a given message.
 func NotSupported(a ...interface{}) DriverStatus {
 	return DriverStatus{
@@ -147,9 +144,9 @@ func processOpts(optsStr string) (Options, error) {
 
 // ExecDriver executes the appropriate FlexvolumeDriver command based on
 // recieved call-out.
-func ExecDriver(drivers map[string]Driver, args []string) {
+func ExecDriver(drivers map[string]Driver, args []string) DriverStatus {
 	if len(args) < 2 {
-		ExitWithResult(Fail("Expected at least one argument"))
+		return Failf("Expected at least one argument")
 	}
 
 	log.Printf("'%s %s' called with %s", args[0], args[1], args[2:])
@@ -161,127 +158,130 @@ func ExecDriver(drivers map[string]Driver, args []string) {
 
 	if dir != "oci" && dir != DefaultSymlinkDirectory {
 		driver = drivers[dir]
-		if driver == nil {
-			ExitWithResult(Fail("No driver found for ", dir))
-		}
+	}
+
+	// Moved outside the above if to catch errors in code.
+	if driver == nil {
+		Failf("No driver found for ", dir)
 	}
 
 	log.Printf("Using %s driver", dir)
 
 	switch args[1] {
 	// <driver executable> init
 	case "init":
-		ExitWithResult(driver.Init())
+		return driver.Init()
 
 	// <driver executable> getvolumename <json options>
 	// Currently broken as of lates kube release (1.6.4). Work around hardcodes
 	// exiting with StatusNotSupported.
 	// TODO(apryde): Investigate current situation and version support
 	// requirements.
 	case "getvolumename":
-		ExitWithResult(NotSupported("getvolumename is broken as of kube 1.6.4"))
+		return NotSupportedf("getvolumename is broken as of kube 1.6.4")
 
 	// <driver executable> attach <json options> <node name>
 	case "attach":
 		if len(args) != 4 {
-			ExitWithResult(Fail("attach expected exactly 4 arguments; got ", args))
+			Failf("attach expected exactly 4 arguments; got ", args)
 		}
 
 		opts, err := processOpts(args[2])
 		if err != nil {
-			ExitWithResult(Fail(err))
+			return Failf(err.Error())
 		}
 
 		nodeName := args[3]
-		ExitWithResult(driver.Attach(opts, nodeName))
+		return driver.Attach(opts, nodeName)
 
 	// <driver executable> detach <mount device> <node name>
 	case "detach":
 		if len(args) != 4 {
-			ExitWithResult(Fail("detach expected exactly 4 arguments; got ", args))
+			return Failf("detach expected exactly 4 arguments; got ", args)
 		}
 
 		mountDevice := args[2]
 		nodeName := args[3]
-		ExitWithResult(driver.Detach(mountDevice, nodeName))
+		return driver.Detach(mountDevice, nodeName)
 
 	// <driver executable> waitforattach <mount device> <json options>
 	case "waitforattach":
 		if len(args) != 4 {
-			ExitWithResult(Fail("waitforattach expected exactly 4 arguments; got ", args))
+			return Failf("waitforattach expected exactly 4 arguments; got ", args)
 		}
 
 		mountDevice := args[2]
 		opts, err := processOpts(args[3])
 		if err != nil {
-			ExitWithResult(Fail(err))
+			return Failf(err.Error())
 		}
 
-		ExitWithResult(driver.WaitForAttach(mountDevice, opts))
+		return driver.WaitForAttach(mountDevice, opts)
 
 	// <driver executable> isattached <json options> <node name>
 	case "isattached":
 		if len(args) != 4 {
-			ExitWithResult(Fail("isattached expected exactly 4 arguments; got ", args))
+			return Failf("isattached expected exactly 4 arguments; got ", args)
 		}
 
 		opts, err := processOpts(args[2])
 		if err != nil {
-			ExitWithResult(Fail(err))
+			return Failf(err.Error())
 		}
 		nodeName := args[3]
-		ExitWithResult(driver.IsAttached(opts, nodeName))
+		return driver.IsAttached(opts, nodeName)
 
 	// <driver executable> mountdevice <mount dir> <mount device> <json options>
 	case "mountdevice":
 		if len(args) != 5 {
-			ExitWithResult(Fail("mountdevice expected exactly 5 arguments; got ", args))
+			return Failf("mountdevice expected exactly 5 arguments; got ", args)
 		}
 
 		mountDir := args[2]
 		mountDevice := args[3]
 
 		opts, err := processOpts(args[4])
 		if err != nil {
-			ExitWithResult(Fail(err))
+			return Failf(err.Error())
 		}
 
-		ExitWithResult(driver.MountDevice(mountDir, mountDevice, opts))
+		return driver.MountDevice(mountDir, mountDevice, opts)
 
 	// <driver executable> unmountdevice <mount dir>
 	case "unmountdevice":
 		if len(args) != 3 {
-			ExitWithResult(Fail("unmountdevice expected exactly 3 arguments; got ", args))
+			return Failf("unmountdevice expected exactly 3 arguments; got ", args)
 		}
 
 		mountDir := args[2]
-		ExitWithResult(driver.UnmountDevice(mountDir))
+		return driver.UnmountDevice(mountDir)
 
 	// <driver executable> mount <mount dir> <json options>
 	case "mount":
 		if len(args) != 4 {
-			ExitWithResult(Fail("mount expected exactly 4 arguments; got ", args))
+			return Failf("mount expected exactly 4 arguments; got ", args)
 		}
 
 		mountDir := args[2]
 
 		opts, err := processOpts(args[3])
 		if err != nil {
-			ExitWithResult(Fail(err))
+			return Failf(err.Error())
 		}
 
-		ExitWithResult(driver.Mount(mountDir, opts))
+		return driver.Mount(mountDir, opts)
 
 	// <driver executable> unmount <mount dir>
 	case "unmount":
 		if len(args) != 3 {
-			ExitWithResult(Fail("mount expected exactly 3 arguments; got ", args))
+			return Failf("mount expected exactly 3 arguments; got ", args)
 		}
 
 		mountDir := args[2]
-		ExitWithResult(driver.Unmount(mountDir))
+		return driver.Unmount(mountDir)
 
 	default:
-		ExitWithResult(Fail("Invalid command; got ", args))
+		return Failf("invalid command; got ", args)
 	}
+	return Failf("Unexpected condition")
 }

pkg/flexvolume/flexvolume_test.go

@@ -15,74 +15,60 @@
 package flexvolume
 
 import (
-	"bytes"
 	"testing"
 )
 
-const defaultTestOps = `{"kubernetes.io/fsType":"ext4","kubernetes.io/readwrite":"rw"}`
+const defaultTestOps = `{"kubernetes.io/fsType":"ext4","kubernetes.io/readwrite":"rw","kubernetes.io/pvOrVolumeName":"mockvolumeid"}`
+const noVolIDTestOps = `{"kubernetes.io/fsType":"ext4","kubernetes.io/readwrite":"rw"}`
 
-func TestInit(t *testing.T) {
-	bak := out
-	out = new(bytes.Buffer)
-	defer func() { out = bak }()
+func assertSuccess(t *testing.T, expected DriverStatus, status DriverStatus) {
+	if status != expected {
+		t.Fatalf(`Expected '%#v' got '%#v'`, expected, status)
+	}
+}
 
-	code := 0
-	osexit := exit
-	exit = func(c int) { code = c }
-	defer func() { exit = osexit }()
+func assertFailure(t *testing.T, expected DriverStatus, status DriverStatus) {
+	if status != expected {
+		t.Fatalf(`Expected '%#v' got '%#v'`, expected, status)
+	}
+}
 
-	ExecDriver(map[string]Driver{"oci-bvs": mockFlexvolumeDriver{}}, []string{"oci", "init"})
+func TestInit(t *testing.T) {
+	status := ExecDriver(map[string]Driver{"oci-bvs": mockFlexvolumeDriver{}},
+		[]string{"oci", "init"})
 
-	if out.(*bytes.Buffer).String() != `{"status":"Success"}`+"\n" {
-		t.Fatalf(`Expected '{"status":"Success"}'; got %s`, out.(*bytes.Buffer).String())
-	}
+	expected := DriverStatus{Status: "Success"}
 
-	if code != 0 {
-		t.Fatalf("Expected 'exit 0'; got 'exit %d'", code)
-	}
+	assertSuccess(t, expected, status)
 }
 
 // TestVolumeName tests that the getvolumename call-out results in
 // StatusNotSupported as the call-out is broken as of the latest stable Kube
 // release (1.6.4).
 func TestGetVolumeName(t *testing.T) {
-	bak := out
-	out = new(bytes.Buffer)
-	defer func() { out = bak }()
+	status := ExecDriver(map[string]Driver{"oci-bvs": mockFlexvolumeDriver{}},
+		[]string{"oci", "getvolumename", defaultTestOps})
 
-	code := 0
-	osexit := exit
-	exit = func(c int) { code = c }
-	defer func() { exit = osexit }()
+	expected := DriverStatus{Status: "Not supported", Message: "getvolumename is broken as of kube 1.6.4"}
 
-	ExecDriver(map[string]Driver{"oci-bvs": mockFlexvolumeDriver{}}, []string{"oci", "getvolumename", defaultTestOps})
+	assertFailure(t, expected, status)
+}
 
-	if out.(*bytes.Buffer).String() != `{"status":"Not supported","message":"getvolumename is broken as of kube 1.6.4"}`+"\n" {
-		t.Fatalf(`Expected '{"status":"Not supported","message":"getvolumename is broken as of kube 1.6.4"}}'; got %s`, out.(*bytes.Buffer).String())
-	}
+func TestNoVolumeIDDispatch(t *testing.T) {
+	status := ExecDriver(map[string]Driver{"oci-bvs": mockFlexvolumeDriver{}},
+		[]string{"oci", "attach", noVolIDTestOps, "nodeName"})
 
-	if code != 0 {
-		t.Fatalf("Expected 'exit 0'; got 'exit %d'", code)
+	expected := DriverStatus{
+		Status:  "Failure",
+		Message: "key 'kubernetes.io/pvOrVolumeName' not found in attach options",
 	}
+	assertFailure(t, expected, status)
 }
 
 func TestAttachUnsuported(t *testing.T) {
-	bak := out
-	out = new(bytes.Buffer)
-	defer func() { out = bak }()
-
-	code := 0
-	osexit := exit
-	exit = func(c int) { code = c }
-	defer func() { exit = osexit }()
-
-	ExecDriver(map[string]Driver{"oci-bvs": mockFlexvolumeDriver{}}, []string{"oci", "attach", defaultTestOps, "nodeName"})
+	status := ExecDriver(map[string]Driver{"oci-bvs": mockFlexvolumeDriver{}},
+		[]string{"oci", "attach", defaultTestOps, "nodeName"})
 
-	if out.(*bytes.Buffer).String() != `{"status":"Not supported"}`+"\n" {
-		t.Fatalf(`Expected '{"status":"Not supported""}'; got %s`, out.(*bytes.Buffer).String())
-	}
-
-	if code != 0 {
-		t.Fatalf("Expected 'exit 0'; got 'exit %d'", code)
-	}
+	expected := DriverStatus{Status: "Not supported"}
+	assertFailure(t, expected, status)
 }