Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

adjust newapp/newbuild error messages (arg classification vs. actual … #18272

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
80 changes: 59 additions & 21 deletions pkg/oc/cli/cmd/newapp.go
Original file line number Diff line number Diff line change
Expand Up @@ -605,7 +605,29 @@ 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 := range unknown {
fmt.Fprintf(buf, "%s:\n", argName)
for _, classErr := range config.EnvironmentClassificationErrors {
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))
}
}
for _, classErr := range config.SourceClassificationErrors {
fmt.Fprintf(buf, fmt.Sprintf("%s: %v\n", classErr.Key, classErr.Value))
}
for _, classErr := range config.TemplateClassificationErrors {
fmt.Fprintf(buf, fmt.Sprintf("%s: %v\n", classErr.Key, classErr.Value))
}
for _, classErr := range config.ComponentClassificationErrors {
fmt.Fprintf(buf, fmt.Sprintf("%s: %v\n", classErr.Key, classErr.Value))
}
fmt.Fprintln(buf)
}
return kcmdutil.UsageErrorf(c, heredoc.Docf(buf.String()))
}

if config.AllowMissingImages && config.AsSearch {
Expand Down Expand Up @@ -701,7 +723,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
}
Expand All @@ -711,23 +733,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)
Expand All @@ -736,20 +754,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 {
Expand All @@ -762,6 +782,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 {
Expand All @@ -774,11 +795,19 @@ 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.ClassificationWinners[t.Value]
if classification.IncludeGitErrors {
notGitRepo, ok := config.SourceClassificationErrors[t.Value]
if ok {
t.Errs = append(t.Errs, notGitRepo.Value)
}
}
groups.Add(
"no-matches",
heredoc.Docf(`
Expand All @@ -794,11 +823,13 @@ func transformRunError(err error, baseName, commandName, commandPath string, gro

See '%[1]s -h' for examples.`, commandPath,
),
heredoc.Docf(classification.String()),
t,
t.Errs...,
)
return
case newapp.ErrMultipleMatches:
classification, _ := config.ClassificationWinners[t.Value]
buf := &bytes.Buffer{}
for i, match := range t.Matches {

Expand All @@ -812,6 +843,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.String(),
t,
t.Errs...,
)
Expand All @@ -830,11 +862,13 @@ func transformRunError(err error, baseName, commandName, commandPath string, gro

%[2]s`, t.Value, buf.String(),
),
classification.String(),
t,
t.Errs...,
)
return
case newapp.ErrPartialMatch:
classification, _ := config.ClassificationWinners[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)
Expand All @@ -846,11 +880,13 @@ func transformRunError(err error, baseName, commandName, commandPath string, gro

%[2]s`, t.Value, buf.String(),
),
classification.String(),
t,
t.Errs...,
)
return
case newapp.ErrNoTagsFound:
classification, _ := config.ClassificationWinners[t.Value]
buf := &bytes.Buffer{}
fmt.Fprintf(buf, " Use --allow-missing-imagestream-tags to use this image stream\n\n")
groups.Add(
Expand All @@ -860,6 +896,7 @@ func transformRunError(err error, baseName, commandName, commandPath string, gro

%[2]s`, t.Match.Name, buf.String(),
),
classification.String(),
t,
t.Errs...,
)
Expand All @@ -868,13 +905,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 {
Expand Down
15 changes: 12 additions & 3 deletions pkg/oc/cli/cmd/newbuild.go
Original file line number Diff line number Diff line change
Expand Up @@ -212,9 +212,16 @@ 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.ClassificationWinners[t.Value]
if classification.IncludeGitErrors {
notGitRepo, ok := config.SourceClassificationErrors[t.Value]
if ok {
t.Errs = append(t.Errs, notGitRepo.Value)
}
}
groups.Add(
"no-matches",
heredoc.Docf(`
Expand All @@ -229,15 +236,17 @@ func transformBuildError(err error, baseName, commandName, commandPath string, g

See '%[1]s -h' for examples.`, commandPath,
),
classification.String(),
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
}
4 changes: 4 additions & 0 deletions pkg/oc/cli/cmd/newbuild_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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...)
Expand Down
30 changes: 25 additions & 5 deletions pkg/oc/generate/app/componentresolvers.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package app

import (
"sort"
"strings"

"github.com/golang/glog"

Expand All @@ -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
Expand All @@ -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
Expand All @@ -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) {
Expand All @@ -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 {
Expand Down Expand Up @@ -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)
}
Expand All @@ -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)
Expand All @@ -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:
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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
Expand All @@ -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{}
Expand Down
4 changes: 4 additions & 0 deletions pkg/oc/generate/app/componentresolvers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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++ {
Expand Down
Loading