Make ContinueOnError actually work

In resource.Builder ContinueOnError() should actually do so.

Reorganized util.CheckErr() to make it display bulk errors more
effectively and be more reusable. Clarified that CheckErr is not
specific to kubectl in Godoc. Changed the "Error: " prefix to
"error: " to more closely match Unix conventions.

pkg/kubectl/cmd/util/helpers.go

@@ -46,62 +46,113 @@ type debugError interface {
 	DebugError() (msg string, args []interface{})
 }
 
+// CheckErr prints a user friendly error to STDERR and exits with a non-zero
+// exit code. Unrecognized errors will be printed with an "error: " prefix.
+//
+// This method is generic to the command in use and may be used by non-Kubectl
+// commands.
 func CheckErr(err error) {
-	if err != nil {
+	if err == nil {
-		if debugErr, ok := err.(debugError); ok {
+		return
-			glog.V(4).Infof(debugErr.DebugError())
-		}
-		_, isStatus := err.(client.APIStatus)
-		switch {
-		case clientcmd.IsConfigurationInvalid(err):
-			fatal(MultilineError("Error in configuration: ", err))
-		case isStatus:
-			fatal(fmt.Sprintf("Error from server: %s", err.Error()))
-		case errors.IsUnexpectedObjectError(err):
-			fatal(fmt.Sprintf("Server returned an unexpected response: %s", err.Error()))
-		}
-		switch t := err.(type) {
-		case *url.Error:
-			glog.V(4).Infof("Connection error: %s %s: %v", t.Op, t.URL, t.Err)
-			switch {
-			case strings.Contains(t.Err.Error(), "connection refused"):
-				host := t.URL
-				if server, err := url.Parse(t.URL); err == nil {
-					host = server.Host
-				}
-				fatal(fmt.Sprintf("The connection to the server %s was refused - did you specify the right host or port?", host))
-			}
-			fatal(fmt.Sprintf("Unable to connect to the server: %v", t.Err))
-		}
-		fatal(fmt.Sprintf("Error: %s", err.Error()))
 	}
+
+	// handle multiline errors
+	if clientcmd.IsConfigurationInvalid(err) {
+		fatal(MultilineError("Error in configuration: ", err))
+	}
+	if agg, ok := err.(utilerrors.Aggregate); ok && len(agg.Errors()) > 0 {
+		fatal(MultipleErrors("", agg.Errors()))
+	}
+
+	msg, ok := StandardErrorMessage(err)
+	if !ok {
+		msg = fmt.Sprintf("error: %s\n", err.Error())
+	}
+	fatal(msg)
 }
 
+// StandardErrorMessage translates common errors into a human readable message, or returns
+// false if the error is not one of the recognized types. It may also log extended
+// information to glog.
+//
+// This method is generic to the command in use and may be used by non-Kubectl
+// commands.
+func StandardErrorMessage(err error) (string, bool) {
+	if debugErr, ok := err.(debugError); ok {
+		glog.V(4).Infof(debugErr.DebugError())
+	}
+	_, isStatus := err.(client.APIStatus)
+	switch {
+	case isStatus:
+		return fmt.Sprintf("Error from server: %s", err.Error()), true
+	case errors.IsUnexpectedObjectError(err):
+		return fmt.Sprintf("Server returned an unexpected response: %s", err.Error()), true
+	}
+	switch t := err.(type) {
+	case *url.Error:
+		glog.V(4).Infof("Connection error: %s %s: %v", t.Op, t.URL, t.Err)
+		switch {
+		case strings.Contains(t.Err.Error(), "connection refused"):
+			host := t.URL
+			if server, err := url.Parse(t.URL); err == nil {
+				host = server.Host
+			}
+			return fmt.Sprintf("The connection to the server %s was refused - did you specify the right host or port?", host), true
+		}
+		return fmt.Sprintf("Unable to connect to the server: %v", t.Err), true
+	}
+	return "", false
+}
+
+// MultilineError returns a string representing an error that splits sub errors into their own
+// lines. The returned string will end with a newline.
 func MultilineError(prefix string, err error) string {
 	if agg, ok := err.(utilerrors.Aggregate); ok {
-		errs := agg.Errors()
+		errs := utilerrors.Flatten(agg).Errors()
 		buf := &bytes.Buffer{}
 		switch len(errs) {
 		case 0:
-			return fmt.Sprintf("%s%v", prefix, err)
+			return fmt.Sprintf("%s%v\n", prefix, err)
 		case 1:
-			return fmt.Sprintf("%s%v", prefix, errs[0])
+			return fmt.Sprintf("%s%v\n", prefix, messageForError(errs[0]))
 		default:
 			fmt.Fprintln(buf, prefix)
 			for _, err := range errs {
-				fmt.Fprintf(buf, "* %v\n", err)
+				fmt.Fprintf(buf, "* %v\n", messageForError(err))
 			}
 			return buf.String()
 		}
 	}
-	return fmt.Sprintf("%s%s", prefix, err)
+	return fmt.Sprintf("%s%s\n", prefix, err)
 }
 
+// MultipleErrors returns a newline delimited string containing
+// the prefix and referenced errors in standard form.
+func MultipleErrors(prefix string, errs []error) string {
+	buf := &bytes.Buffer{}
+	for _, err := range errs {
+		fmt.Fprintf(buf, "%s%v\n", prefix, messageForError(err))
+	}
+	return buf.String()
+}
+
+// messageForError returns the string representing the error.
+func messageForError(err error) string {
+	msg, ok := StandardErrorMessage(err)
+	if !ok {
+		msg = err.Error()
+	}
+	return msg
+}
+
+// fatal prints the message and then exits. If V(2) or greater, glog.Fatal
+// is invoked for extended information. The provided msg should end in a
+// newline.
 func fatal(msg string) {
 	if glog.V(2) {
 		glog.FatalDepth(2, msg)
 	}
-	fmt.Fprintln(os.Stderr, msg)
+	fmt.Fprint(os.Stderr, msg)
 	os.Exit(1)
 }
 

pkg/kubectl/resource/builder.go

@@ -614,6 +614,9 @@ func (b *Builder) Do() *Result {
 		helpers = append(helpers, RetrieveLazy)
 	}
 	r.visitor = NewDecoratedVisitor(r.visitor, helpers...)
+	if b.continueOnError {
+		r.visitor = ContinueOnErrorVisitor{r.visitor}
+	}
 	return r
 }
 

pkg/kubectl/resource/builder_test.go

@@ -18,6 +18,7 @@ package resource
 
 import (
 	"bytes"
+	"fmt"
 	"io"
 	"io/ioutil"
 	"net/http"
@@ -606,6 +607,36 @@ func TestMultipleObject(t *testing.T) {
 	}
 }
 
+func TestContinueOnErrorVisitor(t *testing.T) {
+	r, _, _ := streamTestData()
+	req := NewBuilder(latest.RESTMapper, api.Scheme, fakeClient()).
+		ContinueOnError().
+		NamespaceParam("test").Stream(r, "STDIN").Flatten().
+		Do()
+	count := 0
+	testErr := fmt.Errorf("test error")
+	err := req.Visit(func(_ *Info) error {
+		count++
+		if count > 1 {
+			return testErr
+		}
+		return nil
+	})
+	if err == nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	if count != 3 {
+		t.Fatalf("did not visit all infos: %d", count)
+	}
+	agg, ok := err.(errors.Aggregate)
+	if !ok {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	if len(agg.Errors()) != 2 || agg.Errors()[0] != testErr || agg.Errors()[1] != testErr {
+		t.Fatalf("unexpected error: %v", err)
+	}
+}
+
 func TestSingularObject(t *testing.T) {
 	obj, err := NewBuilder(latest.RESTMapper, api.Scheme, fakeClient()).
 		NamespaceParam("test").DefaultNamespace().

pkg/kubectl/resource/visitor.go

@@ -350,6 +350,36 @@ func (v DecoratedVisitor) Visit(fn VisitorFunc) error {
 	})
 }
 
+// ContinueOnErrorVisitor visits each item and, if an error occurs on
+// any individual item, returns an aggregate error after all items
+// are visited.
+type ContinueOnErrorVisitor struct {
+	Visitor
+}
+
+// Visit returns nil if no error occurs during traversal, a regular
+// error if one occurs, or if multiple errors occur, an aggregate
+// error.  If the provided visitor fails on any individual item it
+// will not prevent the remaining items from being visited. An error
+// returned by the visitor directly may still result in some items
+// not being visited.
+func (v ContinueOnErrorVisitor) Visit(fn VisitorFunc) error {
+	errs := []error{}
+	err := v.Visitor.Visit(func(info *Info) error {
+		if err := fn(info); err != nil {
+			errs = append(errs, err)
+		}
+		return nil
+	})
+	if err != nil {
+		errs = append(errs, err)
+	}
+	if len(errs) == 1 {
+		return errs[0]
+	}
+	return errors.NewAggregate(errs)
+}
+
 // FlattenListVisitor flattens any objects that runtime.ExtractList recognizes as a list
 // - has an "Items" public field that is a slice of runtime.Objects or objects satisfying
 // that interface - into multiple Infos. An error on any sub item (for instance, if a List