adjust newapp/newbuild error messages (arg classification vs. actual processing)

Author: gabemontero

pkg/oc/cli/cmd/newapp.go

@@ -605,7 +605,20 @@ func CompleteAppConfig(config *newcmd.AppConfig, f *clientcmd.Factory, c *cobra.
 
 	unknown := config.AddArguments(args)
 	if len(unknown) != 0 {
-		return kcmdutil.UsageErrorf(c, "Did not recognize the following arguments: %v", unknown)
+		buf := &bytes.Buffer{}
+		fmt.Fprintf(buf, "Did not recognize the following arguments: %v\n\n", unknown)
+		for argName, classErrs := range config.ComponentInputClassificationErrors {
+			fmt.Fprintf(buf, "%s:\n", argName)
+			for _, classErr := range classErrs {
+				if classErr.Value != nil {
+					fmt.Fprintf(buf, fmt.Sprintf("%s:  %v\n", classErr.Key, classErr.Value))
+				} else {
+					fmt.Fprintf(buf, fmt.Sprintf("%s\n", classErr.Key))
+				}
+			}
+			fmt.Fprintln(buf)
+		}
+		return kcmdutil.UsageErrorf(c, heredoc.Docf(buf.String()))
 	}
 
 	if config.AllowMissingImages && config.AsSearch {
@@ -701,7 +714,7 @@ func retryBuildConfig(info *resource.Info, err error) runtime.Object {
 	return nil
 }
 
-func handleError(err error, baseName, commandName, commandPath string, config *newcmd.AppConfig, transformError func(err error, baseName, commandName, commandPath string, groups errorGroups)) error {
+func handleError(err error, baseName, commandName, commandPath string, config *newcmd.AppConfig, transformError func(err error, baseName, commandName, commandPath string, groups errorGroups, config *newcmd.AppConfig)) error {
 	if err == nil {
 		return nil
 	}
@@ -711,23 +724,19 @@ func handleError(err error, baseName, commandName, commandPath string, config *n
 	}
 	groups := errorGroups{}
 	for _, err := range errs {
-		transformError(err, baseName, commandName, commandPath, groups)
+		transformError(err, baseName, commandName, commandPath, groups, config)
 	}
 	buf := &bytes.Buffer{}
-	if len(config.ArgumentClassificationErrors) > 0 {
-		fmt.Fprintf(buf, "Errors occurred while determining argument types:\n")
-		for _, classErr := range config.ArgumentClassificationErrors {
-			fmt.Fprintf(buf, fmt.Sprintf("\n%s:  %v\n", classErr.Key, classErr.Value))
-		}
-		fmt.Fprint(buf, "\n")
-		// this print serves as a header for the printing of the errorGroups, but
-		// only print it if we precede with classification errors, to help distinguish
-		// between the two
-		fmt.Fprintln(buf, "Errors occurred during resource creation:")
-	}
 	for _, group := range groups {
 		fmt.Fprint(buf, kcmdutil.MultipleErrors("error: ", group.errs))
+		if len(group.classification) > 0 {
+			fmt.Fprintln(buf)
+		}
+		fmt.Fprintf(buf, group.classification)
 		if len(group.suggestion) > 0 {
+			if len(group.classification) > 0 {
+				fmt.Fprintln(buf)
+			}
 			fmt.Fprintln(buf)
 		}
 		fmt.Fprint(buf, group.suggestion)
@@ -736,20 +745,22 @@ func handleError(err error, baseName, commandName, commandPath string, config *n
 }
 
 type errorGroup struct {
-	errs       []error
-	suggestion string
+	errs           []error
+	suggestion     string
+	classification string
 }
 type errorGroups map[string]errorGroup
 
-func (g errorGroups) Add(group string, suggestion string, err error, errs ...error) {
+func (g errorGroups) Add(group string, suggestion string, classification string, err error, errs ...error) {
 	all := g[group]
 	all.errs = append(all.errs, errs...)
 	all.errs = append(all.errs, err)
 	all.suggestion = suggestion
+	all.classification = classification
 	g[group] = all
 }
 
-func transformRunError(err error, baseName, commandName, commandPath string, groups errorGroups) {
+func transformRunError(err error, baseName, commandName, commandPath string, groups errorGroups, config *newcmd.AppConfig) {
 	switch t := err.(type) {
 	case newcmd.ErrRequiresExplicitAccess:
 		if t.Input.Token != nil && t.Input.Token.ServiceAccount {
@@ -762,6 +773,7 @@ func transformRunError(err error, baseName, commandName, commandPath string, gro
 					You can see more information about the image by adding the --dry-run flag.
 					If you trust the provided image, include the flag --grant-install-rights.`,
 				),
+				"",
 				fmt.Errorf("installing %q requires an 'installer' service account with project editor access", t.Match.Value),
 			)
 		} else {
@@ -774,11 +786,13 @@ func transformRunError(err error, baseName, commandName, commandPath string, gro
 					You can see more information about the image by adding the --dry-run flag.
 					If you trust the provided image, include the flag --grant-install-rights.`,
 				),
+				"",
 				fmt.Errorf("installing %q requires that you grant the image access to run with your credentials", t.Match.Value),
 			)
 		}
 		return
 	case newapp.ErrNoMatch:
+		classification, _ := config.ComponentInputClassificationWinners[t.Value]
 		groups.Add(
 			"no-matches",
 			heredoc.Docf(`
@@ -794,11 +808,13 @@ func transformRunError(err error, baseName, commandName, commandPath string, gro
 
 				See '%[1]s -h' for examples.`, commandPath,
 			),
+			heredoc.Docf(classification),
 			t,
 			t.Errs...,
 		)
 		return
 	case newapp.ErrMultipleMatches:
+		classification, _ := config.ComponentInputClassificationWinners[t.Value]
 		buf := &bytes.Buffer{}
 		for i, match := range t.Matches {
 
@@ -812,6 +828,7 @@ func transformRunError(err error, baseName, commandName, commandPath string, gro
 
 						%[2]sTo view a full list of matches, use '%[3]s %[4]s -S %[1]s'`, t.Value, buf.String(), baseName, commandName,
 					),
+					classification,
 					t,
 					t.Errs...,
 				)
@@ -830,11 +847,13 @@ func transformRunError(err error, baseName, commandName, commandPath string, gro
 
 					%[2]s`, t.Value, buf.String(),
 			),
+			classification,
 			t,
 			t.Errs...,
 		)
 		return
 	case newapp.ErrPartialMatch:
+		classification, _ := config.ComponentInputClassificationWinners[t.Value]
 		buf := &bytes.Buffer{}
 		fmt.Fprintf(buf, "* %s\n", t.Match.Description)
 		fmt.Fprintf(buf, "  Use %[1]s to specify this image or template\n\n", t.Match.Argument)
@@ -846,11 +865,13 @@ func transformRunError(err error, baseName, commandName, commandPath string, gro
 
 					%[2]s`, t.Value, buf.String(),
 			),
+			classification,
 			t,
 			t.Errs...,
 		)
 		return
 	case newapp.ErrNoTagsFound:
+		classification, _ := config.ComponentInputClassificationWinners[t.Value]
 		buf := &bytes.Buffer{}
 		fmt.Fprintf(buf, "  Use --allow-missing-imagestream-tags to use this image stream\n\n")
 		groups.Add(
@@ -860,6 +881,7 @@ func transformRunError(err error, baseName, commandName, commandPath string, gro
 
 					%[2]s`, t.Match.Name, buf.String(),
 			),
+			classification,
 			t,
 			t.Errs...,
 		)
@@ -868,13 +890,14 @@ func transformRunError(err error, baseName, commandName, commandPath string, gro
 	switch err {
 	case errNoTokenAvailable:
 		// TODO: improve by allowing token generation
-		groups.Add("", "", fmt.Errorf("to install components you must be logged in with an OAuth token (instead of only a certificate)"))
+		groups.Add("", "", "", fmt.Errorf("to install components you must be logged in with an OAuth token (instead of only a certificate)"))
 	case newcmd.ErrNoInputs:
 		// TODO: suggest things to the user
-		groups.Add("", "", usageError(commandPath, newAppNoInput, baseName, commandName))
+		groups.Add("", "", "", usageError(commandPath, newAppNoInput, baseName, commandName))
 	default:
-		groups.Add("", "", err)
+		groups.Add("", "", "", err)
 	}
+	return
 }
 
 func usageError(commandPath, format string, args ...interface{}) error {

pkg/oc/cli/cmd/newbuild.go

@@ -212,9 +212,10 @@ func (o *NewBuildOptions) RunNewBuild() error {
 	return nil
 }
 
-func transformBuildError(err error, baseName, commandName, commandPath string, groups errorGroups) {
+func transformBuildError(err error, baseName, commandName, commandPath string, groups errorGroups, config *newcmd.AppConfig) {
 	switch t := err.(type) {
 	case newapp.ErrNoMatch:
+		classification, _ := config.ComponentInputClassificationWinners[t.Value]
 		groups.Add(
 			"no-matches",
 			heredoc.Docf(`
@@ -229,15 +230,17 @@ func transformBuildError(err error, baseName, commandName, commandPath string, g
 
 				See '%[1]s -h' for examples.`, commandPath,
 			),
+			classification,
 			t,
 			t.Errs...,
 		)
 		return
 	}
 	switch err {
 	case newcmd.ErrNoInputs:
-		groups.Add("", "", usageError(commandPath, newBuildNoInput, baseName, commandName))
+		groups.Add("", "", "", usageError(commandPath, newBuildNoInput, baseName, commandName))
 		return
 	}
-	transformRunError(err, baseName, commandName, commandPath, groups)
+	transformRunError(err, baseName, commandName, commandPath, groups, config)
+	return
 }

pkg/oc/cli/cmd/newbuild_test.go

@@ -108,6 +108,10 @@ type MockSearcher struct {
 	OnSearch func(precise bool, terms ...string) (app.ComponentMatches, []error)
 }
 
+func (m MockSearcher) Type() string {
+	return ""
+}
+
 // Search mocks a search.
 func (m MockSearcher) Search(precise bool, terms ...string) (app.ComponentMatches, []error) {
 	return m.OnSearch(precise, terms...)

pkg/oc/generate/app/componentresolvers.go

@@ -2,6 +2,7 @@ package app
 
 import (
 	"sort"
+	"strings"
 
 	"github.com/golang/glog"
 
@@ -24,6 +25,7 @@ type Resolver interface {
 // matches
 type Searcher interface {
 	Search(precise bool, terms ...string) (ComponentMatches, []error)
+	Type() string
 }
 
 // WeightedResolver is a resolver identified as exact or not, depending on its weight
@@ -42,6 +44,7 @@ type PerfectMatchWeightedResolver []WeightedResolver
 // Resolve resolves the provided input and returns only exact matches
 func (r PerfectMatchWeightedResolver) Resolve(value string) (*ComponentMatch, error) {
 	var errs []error
+	types := []string{}
 	candidates := ScoredComponentMatches{}
 	var group MultiSimpleSearcher
 	var groupWeight float32 = 0.0
@@ -59,6 +62,7 @@ func (r PerfectMatchWeightedResolver) Resolve(value string) (*ComponentMatch, er
 			glog.V(5).Infof("Error from resolver: %v\n", err)
 			errs = append(errs, err...)
 		}
+		types = append(types, group.Type())
 
 		sort.Sort(ScoredComponentMatches(matches))
 		if len(matches) > 0 && matches[0].Score == 0.0 && (len(matches) == 1 || matches[1].Score != 0.0) {
@@ -75,7 +79,7 @@ func (r PerfectMatchWeightedResolver) Resolve(value string) (*ComponentMatch, er
 
 	switch len(candidates) {
 	case 0:
-		return nil, ErrNoMatch{Value: value, Errs: errs}
+		return nil, ErrNoMatch{Value: value, Errs: errs, Type: strings.Join(types, ", ")}
 	case 1:
 		if candidates[0].Score != 0.0 {
 			if candidates[0].NoTagsFound {
@@ -108,7 +112,7 @@ type FirstMatchResolver struct {
 func (r FirstMatchResolver) Resolve(value string) (*ComponentMatch, error) {
 	matches, err := r.Searcher.Search(true, value)
 	if len(matches) == 0 {
-		return nil, ErrNoMatch{Value: value, Errs: err}
+		return nil, ErrNoMatch{Value: value, Errs: err, Type: r.Searcher.Type()}
 	}
 	return matches[0], errors.NewAggregate(err)
 }
@@ -125,7 +129,7 @@ type HighestScoreResolver struct {
 func (r HighestScoreResolver) Resolve(value string) (*ComponentMatch, error) {
 	matches, err := r.Searcher.Search(true, value)
 	if len(matches) == 0 {
-		return nil, ErrNoMatch{Value: value, Errs: err}
+		return nil, ErrNoMatch{Value: value, Errs: err, Type: r.Searcher.Type()}
 	}
 	sort.Sort(ScoredComponentMatches(matches))
 	return matches[0], errors.NewAggregate(err)
@@ -146,7 +150,7 @@ func (r HighestUniqueScoreResolver) Resolve(value string) (*ComponentMatch, erro
 	sort.Sort(ScoredComponentMatches(matches))
 	switch len(matches) {
 	case 0:
-		return nil, ErrNoMatch{Value: value, Errs: err}
+		return nil, ErrNoMatch{Value: value, Errs: err, Type: r.Searcher.Type()}
 	case 1:
 		return matches[0], errors.NewAggregate(err)
 	default:
@@ -184,7 +188,7 @@ func (r UniqueExactOrInexactMatchResolver) Resolve(value string) (*ComponentMatc
 		inexact := matches.Inexact()
 		switch len(inexact) {
 		case 0:
-			return nil, ErrNoMatch{Value: value, Errs: err}
+			return nil, ErrNoMatch{Value: value, Errs: err, Type: r.Searcher.Type()}
 		case 1:
 			return inexact[0], errors.NewAggregate(err)
 		default:
@@ -213,6 +217,14 @@ func (r PipelineResolver) Resolve(value string) (*ComponentMatch, error) {
 // MultiSimpleSearcher is a set of searchers
 type MultiSimpleSearcher []Searcher
 
+func (s MultiSimpleSearcher) Type() string {
+	t := []string{}
+	for _, searcher := range s {
+		t = append(t, searcher.Type())
+	}
+	return strings.Join(t, ", ")
+}
+
 // Search searches using all searchers it holds
 func (s MultiSimpleSearcher) Search(precise bool, terms ...string) (ComponentMatches, []error) {
 	var errs []error
@@ -239,6 +251,14 @@ type WeightedSearcher struct {
 // priority in search results
 type MultiWeightedSearcher []WeightedSearcher
 
+func (s MultiWeightedSearcher) Type() string {
+	t := []string{}
+	for _, searcher := range s {
+		t = append(t, searcher.Type())
+	}
+	return strings.Join(t, ", ")
+}
+
 // Search searches using all searchers it holds and score according to searcher height
 func (s MultiWeightedSearcher) Search(precise bool, terms ...string) (ComponentMatches, []error) {
 	componentMatches := ComponentMatches{}

pkg/oc/generate/app/componentresolvers_test.go

@@ -9,6 +9,10 @@ type mockSearcher struct {
 	numResults int
 }
 
+func (m mockSearcher) Type() string {
+	return ""
+}
+
 func (m mockSearcher) Search(precise bool, terms ...string) (ComponentMatches, []error) {
 	results := ComponentMatches{}
 	for i := 0; i < m.numResults; i++ {

pkg/oc/generate/app/dockerimagelookup.go

@@ -41,6 +41,10 @@ type DockerClientSearcher struct {
 	AllowMissingImages bool
 }
 
+func (r DockerClientSearcher) Type() string {
+	return "local docker images"
+}
+
 // Search searches all images in local docker server for images that match terms
 func (r DockerClientSearcher) Search(precise bool, terms ...string) (ComponentMatches, []error) {
 	componentMatches := ComponentMatches{}
@@ -164,6 +168,10 @@ func (r DockerClientSearcher) Search(precise bool, terms ...string) (ComponentMa
 type MissingImageSearcher struct {
 }
 
+func (r MissingImageSearcher) Type() string {
+	return "images not found in docker registry nor locally"
+}
+
 // Search always returns an exact match for the search terms.
 func (r MissingImageSearcher) Search(precise bool, terms ...string) (ComponentMatches, []error) {
 	componentMatches := ComponentMatches{}
@@ -184,6 +192,10 @@ type ImageImportSearcher struct {
 	Fallback      Searcher
 }
 
+func (s ImageImportSearcher) Type() string {
+	return "images via the image stream import API"
+}
+
 // Search invokes the new ImageStreamImport API to have the server look up Docker images for the user,
 // using secrets stored on the server.
 func (s ImageImportSearcher) Search(precise bool, terms ...string) (ComponentMatches, []error) {
@@ -267,6 +279,10 @@ type DockerRegistrySearcher struct {
 	AllowInsecure bool
 }
 
+func (r DockerRegistrySearcher) Type() string {
+	return "images in the docker registry"
+}
+
 // Search searches in the Docker registry for images that match terms
 func (r DockerRegistrySearcher) Search(precise bool, terms ...string) (ComponentMatches, []error) {
 	componentMatches := ComponentMatches{}