Make kubectl commands return errors and centralize exit handling

pkg/kubectl/cmd/util/helpers.go

@@ -31,10 +31,11 @@ import (
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/api/latest"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/client"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/runtime"
-
 	"github.com/evanphx/json-patch"
+
 	"github.com/golang/glog"
 	"github.com/spf13/cobra"
+	"github.com/spf13/pflag"
 )
 
 func CheckErr(err error) {
@@ -52,25 +53,26 @@ func CheckErr(err error) {
 	}
 }
 
-func usageError(cmd *cobra.Command, format string, args ...interface{}) {
-	glog.Errorf(format, args...)
-	glog.Errorf("See '%s -h' for help.", cmd.CommandPath())
-	os.Exit(1)
+func UsageError(cmd *cobra.Command, format string, args ...interface{}) error {
+	msg := fmt.Sprintf(format, args...)
+	return fmt.Errorf("%s\nsee '%s -h' for help.", msg, cmd.CommandPath())
+}
+
+func getFlag(cmd *cobra.Command, flag string) *pflag.Flag {
+	f := cmd.Flags().Lookup(flag)
+	if f == nil {
+		glog.Fatalf("flag accessed but not defined for command %s: %s", cmd.Name(), flag)
+	}
+	return f
 }
 
 func GetFlagString(cmd *cobra.Command, flag string) string {
-	f := cmd.Flags().Lookup(flag)
-	if f == nil {
-		glog.Fatalf("Flag accessed but not defined for command %s: %s", cmd.Name(), flag)
-	}
+	f := getFlag(cmd, flag)
 	return f.Value.String()
 }
 
 func GetFlagBool(cmd *cobra.Command, flag string) bool {
-	f := cmd.Flags().Lookup(flag)
-	if f == nil {
-		glog.Fatalf("Flag accessed but not defined for command %s: %s", cmd.Name(), flag)
-	}
+	f := getFlag(cmd, flag)
 	result, err := strconv.ParseBool(f.Value.String())
 	if err != nil {
 		glog.Fatalf("Invalid value for a boolean flag: %s", f.Value.String())
@@ -80,24 +82,22 @@ func GetFlagBool(cmd *cobra.Command, flag string) bool {
 
 // Assumes the flag has a default value.
 func GetFlagInt(cmd *cobra.Command, flag string) int {
-	f := cmd.Flags().Lookup(flag)
-	if f == nil {
-		glog.Fatalf("Flag accessed but not defined for command %s: %s", cmd.Name(), flag)
-	}
+	f := getFlag(cmd, flag)
 	v, err := strconv.Atoi(f.Value.String())
 	// This is likely not a sufficiently friendly error message, but cobra
 	// should prevent non-integer values from reaching here.
-	CheckErr(err)
+	if err != nil {
+		glog.Fatalf("unable to convert flag value to int: %v", err)
+	}
 	return v
 }
 
 func GetFlagDuration(cmd *cobra.Command, flag string) time.Duration {
-	f := cmd.Flags().Lookup(flag)
-	if f == nil {
-		glog.Fatalf("Flag accessed but not defined for command %s: %s", cmd.Name(), flag)
-	}
+	f := getFlag(cmd, flag)
 	v, err := time.ParseDuration(f.Value.String())
-	CheckErr(err)
+	if err != nil {
+		glog.Fatalf("unable to convert flag value to Duration: %v", err)
+	}
 	return v
 }
 

pkg/kubectl/cmd/util/resource.go

@@ -27,26 +27,29 @@ import (
 )
 
 // ResourceFromArgs expects two arguments with a given type, and extracts the fields necessary
-// to uniquely locate a resource. Displays a usageError if that contract is not satisfied, or
+// to uniquely locate a resource. Displays a UsageError if that contract is not satisfied, or
 // a generic error if any other problems occur.
 // DEPRECATED: Use resource.Builder
-func ResourceFromArgs(cmd *cobra.Command, args []string, mapper meta.RESTMapper, cmdNamespace string) (mapping *meta.RESTMapping, namespace, name string) {
+func ResourceFromArgs(cmd *cobra.Command, args []string, mapper meta.RESTMapper, cmdNamespace string) (mapping *meta.RESTMapping, namespace, name string, err error) {
 	if len(args) != 2 {
-		usageError(cmd, "Must provide resource and name command line params")
+		err = UsageError(cmd, "Must provide resource and name command line params")
+		return
 	}
 
 	resource := args[0]
 	namespace = cmdNamespace
 	name = args[1]
 	if len(name) == 0 || len(resource) == 0 {
-		usageError(cmd, "Must provide resource and name command line params")
+		err = UsageError(cmd, "Must provide resource and name command line params")
+		return
 	}
 
 	version, kind, err := mapper.VersionAndKindForResource(resource)
-	CheckErr(err)
+	if err != nil {
+		return
+	}
 
 	mapping, err = mapper.RESTMapping(kind, version)
-	CheckErr(err)
 	return
 }
 
@@ -54,41 +57,52 @@ func ResourceFromArgs(cmd *cobra.Command, args []string, mapper meta.RESTMapper,
 // resolve to a known type an error is returned. The returned mapping can be used to determine
 // the correct REST endpoint to modify this resource with.
 // DEPRECATED: Use resource.Builder
-func ResourceFromFile(filename string, typer runtime.ObjectTyper, mapper meta.RESTMapper, schema validation.Schema, cmdVersion string) (mapping *meta.RESTMapping, namespace, name string, data []byte) {
-	configData, err := ReadConfigData(filename)
-	CheckErr(err)
-	data = configData
+func ResourceFromFile(filename string, typer runtime.ObjectTyper, mapper meta.RESTMapper, schema validation.Schema, cmdVersion string) (mapping *meta.RESTMapping, namespace, name string, data []byte, err error) {
+	data, err = ReadConfigData(filename)
+	if err != nil {
+		return
+	}
 
 	objVersion, kind, err := typer.DataVersionAndKind(data)
-	CheckErr(err)
+	if err != nil {
+		return
+	}
 
 	// TODO: allow unversioned objects?
 	if len(objVersion) == 0 {
-		CheckErr(fmt.Errorf("the resource in the provided file has no apiVersion defined"))
+		err = fmt.Errorf("the resource in the provided file has no apiVersion defined")
 	}
 
 	err = schema.ValidateBytes(data)
-	CheckErr(err)
+	if err != nil {
+		return
+	}
 
 	// decode using the version stored with the object (allows codec to vary across versions)
 	mapping, err = mapper.RESTMapping(kind, objVersion)
-	CheckErr(err)
+	if err != nil {
+		return
+	}
 
 	obj, err := mapping.Codec.Decode(data)
-	CheckErr(err)
+	if err != nil {
+		return
+	}
 
 	meta := mapping.MetadataAccessor
 	namespace, err = meta.Namespace(obj)
-	CheckErr(err)
+	if err != nil {
+		return
+	}
 	name, err = meta.Name(obj)
-	CheckErr(err)
+	if err != nil {
+		return
+	}
 
 	// if the preferred API version differs, get a different mapper
 	if cmdVersion != objVersion {
 		mapping, err = mapper.RESTMapping(kind, cmdVersion)
-		CheckErr(err)
 	}
-
 	return
 }