Skip to content

Commit 2785d75

Browse files
gabemonterogabemontero
committed
adjust newapp/newbuild error messages (arg classification vs. actual processing)
1 parent 6ff50ac commit 2785d75

14 files changed

+241
-60
lines changed

‎pkg/oc/cli/cmd/newapp.go

+59-21
Original file line numberDiff line numberDiff line change
@@ -605,7 +605,29 @@ func CompleteAppConfig(config *newcmd.AppConfig, f *clientcmd.Factory, c *cobra.
605605

606606
unknown := config.AddArguments(args)
607607
if len(unknown) != 0 {
608-
return kcmdutil.UsageErrorf(c, "Did not recognize the following arguments: %v", unknown)
608+
buf := &bytes.Buffer{}
609+
fmt.Fprintf(buf, "Did not recognize the following arguments: %v\n\n", unknown)
610+
for _, argName := range unknown {
611+
fmt.Fprintf(buf, "%s:\n", argName)
612+
for _, classErr := range config.EnvironmentClassificationErrors {
613+
if classErr.Value != nil {
614+
fmt.Fprintf(buf, fmt.Sprintf("%s: %v\n", classErr.Key, classErr.Value))
615+
} else {
616+
fmt.Fprintf(buf, fmt.Sprintf("%s\n", classErr.Key))
617+
}
618+
}
619+
for _, classErr := range config.SourceClassificationErrors {
620+
fmt.Fprintf(buf, fmt.Sprintf("%s: %v\n", classErr.Key, classErr.Value))
621+
}
622+
for _, classErr := range config.TemplateClassificationErrors {
623+
fmt.Fprintf(buf, fmt.Sprintf("%s: %v\n", classErr.Key, classErr.Value))
624+
}
625+
for _, classErr := range config.ComponentClassificationErrors {
626+
fmt.Fprintf(buf, fmt.Sprintf("%s: %v\n", classErr.Key, classErr.Value))
627+
}
628+
fmt.Fprintln(buf)
629+
}
630+
return kcmdutil.UsageErrorf(c, heredoc.Docf(buf.String()))
609631
}
610632

611633
if config.AllowMissingImages && config.AsSearch {
@@ -701,7 +723,7 @@ func retryBuildConfig(info *resource.Info, err error) runtime.Object {
701723
return nil
702724
}
703725

704-
func handleError(err error, baseName, commandName, commandPath string, config *newcmd.AppConfig, transformError func(err error, baseName, commandName, commandPath string, groups errorGroups)) error {
726+
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 {
705727
if err == nil {
706728
return nil
707729
}
@@ -711,23 +733,19 @@ func handleError(err error, baseName, commandName, commandPath string, config *n
711733
}
712734
groups := errorGroups{}
713735
for _, err := range errs {
714-
transformError(err, baseName, commandName, commandPath, groups)
736+
transformError(err, baseName, commandName, commandPath, groups, config)
715737
}
716738
buf := &bytes.Buffer{}
717-
if len(config.ArgumentClassificationErrors) > 0 {
718-
fmt.Fprintf(buf, "Errors occurred while determining argument types:\n")
719-
for _, classErr := range config.ArgumentClassificationErrors {
720-
fmt.Fprintf(buf, fmt.Sprintf("\n%s: %v\n", classErr.Key, classErr.Value))
721-
}
722-
fmt.Fprint(buf, "\n")
723-
// this print serves as a header for the printing of the errorGroups, but
724-
// only print it if we precede with classification errors, to help distinguish
725-
// between the two
726-
fmt.Fprintln(buf, "Errors occurred during resource creation:")
727-
}
728739
for _, group := range groups {
729740
fmt.Fprint(buf, kcmdutil.MultipleErrors("error: ", group.errs))
741+
if len(group.classification) > 0 {
742+
fmt.Fprintln(buf)
743+
}
744+
fmt.Fprintf(buf, group.classification)
730745
if len(group.suggestion) > 0 {
746+
if len(group.classification) > 0 {
747+
fmt.Fprintln(buf)
748+
}
731749
fmt.Fprintln(buf)
732750
}
733751
fmt.Fprint(buf, group.suggestion)
@@ -736,20 +754,22 @@ func handleError(err error, baseName, commandName, commandPath string, config *n
736754
}
737755

738756
type errorGroup struct {
739-
errs []error
740-
suggestion string
757+
errs []error
758+
suggestion string
759+
classification string
741760
}
742761
type errorGroups map[string]errorGroup
743762

744-
func (g errorGroups) Add(group string, suggestion string, err error, errs ...error) {
763+
func (g errorGroups) Add(group string, suggestion string, classification string, err error, errs ...error) {
745764
all := g[group]
746765
all.errs = append(all.errs, errs...)
747766
all.errs = append(all.errs, err)
748767
all.suggestion = suggestion
768+
all.classification = classification
749769
g[group] = all
750770
}
751771

752-
func transformRunError(err error, baseName, commandName, commandPath string, groups errorGroups) {
772+
func transformRunError(err error, baseName, commandName, commandPath string, groups errorGroups, config *newcmd.AppConfig) {
753773
switch t := err.(type) {
754774
case newcmd.ErrRequiresExplicitAccess:
755775
if t.Input.Token != nil && t.Input.Token.ServiceAccount {
@@ -762,6 +782,7 @@ func transformRunError(err error, baseName, commandName, commandPath string, gro
762782
You can see more information about the image by adding the --dry-run flag.
763783
If you trust the provided image, include the flag --grant-install-rights.`,
764784
),
785+
"",
765786
fmt.Errorf("installing %q requires an 'installer' service account with project editor access", t.Match.Value),
766787
)
767788
} else {
@@ -774,11 +795,19 @@ func transformRunError(err error, baseName, commandName, commandPath string, gro
774795
You can see more information about the image by adding the --dry-run flag.
775796
If you trust the provided image, include the flag --grant-install-rights.`,
776797
),
798+
"",
777799
fmt.Errorf("installing %q requires that you grant the image access to run with your credentials", t.Match.Value),
778800
)
779801
}
780802
return
781803
case newapp.ErrNoMatch:
804+
classification, _ := config.ClassificationWinners[t.Value]
805+
if classification.IncludeGitErrors {
806+
notGitRepo, ok := config.SourceClassificationErrors[t.Value]
807+
if ok {
808+
t.Errs = append(t.Errs, notGitRepo.Value)
809+
}
810+
}
782811
groups.Add(
783812
"no-matches",
784813
heredoc.Docf(`
@@ -794,11 +823,13 @@ func transformRunError(err error, baseName, commandName, commandPath string, gro
794823
795824
See '%[1]s -h' for examples.`, commandPath,
796825
),
826+
heredoc.Docf(classification.String()),
797827
t,
798828
t.Errs...,
799829
)
800830
return
801831
case newapp.ErrMultipleMatches:
832+
classification, _ := config.ClassificationWinners[t.Value]
802833
buf := &bytes.Buffer{}
803834
for i, match := range t.Matches {
804835

@@ -812,6 +843,7 @@ func transformRunError(err error, baseName, commandName, commandPath string, gro
812843
813844
%[2]sTo view a full list of matches, use '%[3]s %[4]s -S %[1]s'`, t.Value, buf.String(), baseName, commandName,
814845
),
846+
classification.String(),
815847
t,
816848
t.Errs...,
817849
)
@@ -830,11 +862,13 @@ func transformRunError(err error, baseName, commandName, commandPath string, gro
830862
831863
%[2]s`, t.Value, buf.String(),
832864
),
865+
classification.String(),
833866
t,
834867
t.Errs...,
835868
)
836869
return
837870
case newapp.ErrPartialMatch:
871+
classification, _ := config.ClassificationWinners[t.Value]
838872
buf := &bytes.Buffer{}
839873
fmt.Fprintf(buf, "* %s\n", t.Match.Description)
840874
fmt.Fprintf(buf, " Use %[1]s to specify this image or template\n\n", t.Match.Argument)
@@ -846,11 +880,13 @@ func transformRunError(err error, baseName, commandName, commandPath string, gro
846880
847881
%[2]s`, t.Value, buf.String(),
848882
),
883+
classification.String(),
849884
t,
850885
t.Errs...,
851886
)
852887
return
853888
case newapp.ErrNoTagsFound:
889+
classification, _ := config.ClassificationWinners[t.Value]
854890
buf := &bytes.Buffer{}
855891
fmt.Fprintf(buf, " Use --allow-missing-imagestream-tags to use this image stream\n\n")
856892
groups.Add(
@@ -860,6 +896,7 @@ func transformRunError(err error, baseName, commandName, commandPath string, gro
860896
861897
%[2]s`, t.Match.Name, buf.String(),
862898
),
899+
classification.String(),
863900
t,
864901
t.Errs...,
865902
)
@@ -868,13 +905,14 @@ func transformRunError(err error, baseName, commandName, commandPath string, gro
868905
switch err {
869906
case errNoTokenAvailable:
870907
// TODO: improve by allowing token generation
871-
groups.Add("", "", fmt.Errorf("to install components you must be logged in with an OAuth token (instead of only a certificate)"))
908+
groups.Add("", "", "", fmt.Errorf("to install components you must be logged in with an OAuth token (instead of only a certificate)"))
872909
case newcmd.ErrNoInputs:
873910
// TODO: suggest things to the user
874-
groups.Add("", "", usageError(commandPath, newAppNoInput, baseName, commandName))
911+
groups.Add("", "", "", usageError(commandPath, newAppNoInput, baseName, commandName))
875912
default:
876-
groups.Add("", "", err)
913+
groups.Add("", "", "", err)
877914
}
915+
return
878916
}
879917

880918
func usageError(commandPath, format string, args ...interface{}) error {

‎pkg/oc/cli/cmd/newbuild.go

+12-3
Original file line numberDiff line numberDiff line change
@@ -212,9 +212,16 @@ func (o *NewBuildOptions) RunNewBuild() error {
212212
return nil
213213
}
214214

215-
func transformBuildError(err error, baseName, commandName, commandPath string, groups errorGroups) {
215+
func transformBuildError(err error, baseName, commandName, commandPath string, groups errorGroups, config *newcmd.AppConfig) {
216216
switch t := err.(type) {
217217
case newapp.ErrNoMatch:
218+
classification, _ := config.ClassificationWinners[t.Value]
219+
if classification.IncludeGitErrors {
220+
notGitRepo, ok := config.SourceClassificationErrors[t.Value]
221+
if ok {
222+
t.Errs = append(t.Errs, notGitRepo.Value)
223+
}
224+
}
218225
groups.Add(
219226
"no-matches",
220227
heredoc.Docf(`
@@ -229,15 +236,17 @@ func transformBuildError(err error, baseName, commandName, commandPath string, g
229236
230237
See '%[1]s -h' for examples.`, commandPath,
231238
),
239+
classification.String(),
232240
t,
233241
t.Errs...,
234242
)
235243
return
236244
}
237245
switch err {
238246
case newcmd.ErrNoInputs:
239-
groups.Add("", "", usageError(commandPath, newBuildNoInput, baseName, commandName))
247+
groups.Add("", "", "", usageError(commandPath, newBuildNoInput, baseName, commandName))
240248
return
241249
}
242-
transformRunError(err, baseName, commandName, commandPath, groups)
250+
transformRunError(err, baseName, commandName, commandPath, groups, config)
251+
return
243252
}

‎pkg/oc/cli/cmd/newbuild_test.go

+4
Original file line numberDiff line numberDiff line change
@@ -108,6 +108,10 @@ type MockSearcher struct {
108108
OnSearch func(precise bool, terms ...string) (app.ComponentMatches, []error)
109109
}
110110

111+
func (m MockSearcher) Type() string {
112+
return ""
113+
}
114+
111115
// Search mocks a search.
112116
func (m MockSearcher) Search(precise bool, terms ...string) (app.ComponentMatches, []error) {
113117
return m.OnSearch(precise, terms...)

‎pkg/oc/generate/app/componentresolvers.go

+25-5
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package app
22

33
import (
44
"sort"
5+
"strings"
56

67
"github.com/golang/glog"
78

@@ -24,6 +25,7 @@ type Resolver interface {
2425
// matches
2526
type Searcher interface {
2627
Search(precise bool, terms ...string) (ComponentMatches, []error)
28+
Type() string
2729
}
2830

2931
// WeightedResolver is a resolver identified as exact or not, depending on its weight
@@ -42,6 +44,7 @@ type PerfectMatchWeightedResolver []WeightedResolver
4244
// Resolve resolves the provided input and returns only exact matches
4345
func (r PerfectMatchWeightedResolver) Resolve(value string) (*ComponentMatch, error) {
4446
var errs []error
47+
types := []string{}
4548
candidates := ScoredComponentMatches{}
4649
var group MultiSimpleSearcher
4750
var groupWeight float32 = 0.0
@@ -59,6 +62,7 @@ func (r PerfectMatchWeightedResolver) Resolve(value string) (*ComponentMatch, er
5962
glog.V(5).Infof("Error from resolver: %v\n", err)
6063
errs = append(errs, err...)
6164
}
65+
types = append(types, group.Type())
6266

6367
sort.Sort(ScoredComponentMatches(matches))
6468
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
7579

7680
switch len(candidates) {
7781
case 0:
78-
return nil, ErrNoMatch{Value: value, Errs: errs}
82+
return nil, ErrNoMatch{Value: value, Errs: errs, Type: strings.Join(types, ", ")}
7983
case 1:
8084
if candidates[0].Score != 0.0 {
8185
if candidates[0].NoTagsFound {
@@ -108,7 +112,7 @@ type FirstMatchResolver struct {
108112
func (r FirstMatchResolver) Resolve(value string) (*ComponentMatch, error) {
109113
matches, err := r.Searcher.Search(true, value)
110114
if len(matches) == 0 {
111-
return nil, ErrNoMatch{Value: value, Errs: err}
115+
return nil, ErrNoMatch{Value: value, Errs: err, Type: r.Searcher.Type()}
112116
}
113117
return matches[0], errors.NewAggregate(err)
114118
}
@@ -125,7 +129,7 @@ type HighestScoreResolver struct {
125129
func (r HighestScoreResolver) Resolve(value string) (*ComponentMatch, error) {
126130
matches, err := r.Searcher.Search(true, value)
127131
if len(matches) == 0 {
128-
return nil, ErrNoMatch{Value: value, Errs: err}
132+
return nil, ErrNoMatch{Value: value, Errs: err, Type: r.Searcher.Type()}
129133
}
130134
sort.Sort(ScoredComponentMatches(matches))
131135
return matches[0], errors.NewAggregate(err)
@@ -146,7 +150,7 @@ func (r HighestUniqueScoreResolver) Resolve(value string) (*ComponentMatch, erro
146150
sort.Sort(ScoredComponentMatches(matches))
147151
switch len(matches) {
148152
case 0:
149-
return nil, ErrNoMatch{Value: value, Errs: err}
153+
return nil, ErrNoMatch{Value: value, Errs: err, Type: r.Searcher.Type()}
150154
case 1:
151155
return matches[0], errors.NewAggregate(err)
152156
default:
@@ -184,7 +188,7 @@ func (r UniqueExactOrInexactMatchResolver) Resolve(value string) (*ComponentMatc
184188
inexact := matches.Inexact()
185189
switch len(inexact) {
186190
case 0:
187-
return nil, ErrNoMatch{Value: value, Errs: err}
191+
return nil, ErrNoMatch{Value: value, Errs: err, Type: r.Searcher.Type()}
188192
case 1:
189193
return inexact[0], errors.NewAggregate(err)
190194
default:
@@ -213,6 +217,14 @@ func (r PipelineResolver) Resolve(value string) (*ComponentMatch, error) {
213217
// MultiSimpleSearcher is a set of searchers
214218
type MultiSimpleSearcher []Searcher
215219

220+
func (s MultiSimpleSearcher) Type() string {
221+
t := []string{}
222+
for _, searcher := range s {
223+
t = append(t, searcher.Type())
224+
}
225+
return strings.Join(t, ", ")
226+
}
227+
216228
// Search searches using all searchers it holds
217229
func (s MultiSimpleSearcher) Search(precise bool, terms ...string) (ComponentMatches, []error) {
218230
var errs []error
@@ -239,6 +251,14 @@ type WeightedSearcher struct {
239251
// priority in search results
240252
type MultiWeightedSearcher []WeightedSearcher
241253

254+
func (s MultiWeightedSearcher) Type() string {
255+
t := []string{}
256+
for _, searcher := range s {
257+
t = append(t, searcher.Type())
258+
}
259+
return strings.Join(t, ", ")
260+
}
261+
242262
// Search searches using all searchers it holds and score according to searcher height
243263
func (s MultiWeightedSearcher) Search(precise bool, terms ...string) (ComponentMatches, []error) {
244264
componentMatches := ComponentMatches{}

‎pkg/oc/generate/app/componentresolvers_test.go

+4
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,10 @@ type mockSearcher struct {
99
numResults int
1010
}
1111

12+
func (m mockSearcher) Type() string {
13+
return ""
14+
}
15+
1216
func (m mockSearcher) Search(precise bool, terms ...string) (ComponentMatches, []error) {
1317
results := ComponentMatches{}
1418
for i := 0; i < m.numResults; i++ {

0 commit comments

Comments
 (0)