Skip to content

Commit bafcae4

Browse files
gabemonterogabemontero
committed
adjust newapp/newbuild error messages (arg classification vs. actual processing)
1 parent 1d4c029 commit bafcae4

14 files changed

+205
-58
lines changed

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

+44-21
Original file line numberDiff line numberDiff line change
@@ -605,7 +605,20 @@ 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, classErrs := range config.ComponentInputClassificationErrors {
611+
fmt.Fprintf(buf, "%s:\n", argName)
612+
for _, classErr := range classErrs {
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+
fmt.Fprintln(buf)
620+
}
621+
return kcmdutil.UsageErrorf(c, heredoc.Docf(buf.String()))
609622
}
610623

611624
if config.AllowMissingImages && config.AsSearch {
@@ -701,7 +714,7 @@ func retryBuildConfig(info *resource.Info, err error) runtime.Object {
701714
return nil
702715
}
703716

704-
func handleError(err error, baseName, commandName, commandPath string, config *newcmd.AppConfig, transformError func(err error, baseName, commandName, commandPath string, groups errorGroups)) error {
717+
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 {
705718
if err == nil {
706719
return nil
707720
}
@@ -711,23 +724,19 @@ func handleError(err error, baseName, commandName, commandPath string, config *n
711724
}
712725
groups := errorGroups{}
713726
for _, err := range errs {
714-
transformError(err, baseName, commandName, commandPath, groups)
727+
transformError(err, baseName, commandName, commandPath, groups, config)
715728
}
716729
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-
}
728730
for _, group := range groups {
729731
fmt.Fprint(buf, kcmdutil.MultipleErrors("error: ", group.errs))
732+
if len(group.classification) > 0 {
733+
fmt.Fprintln(buf)
734+
}
735+
fmt.Fprintf(buf, group.classification)
730736
if len(group.suggestion) > 0 {
737+
if len(group.classification) > 0 {
738+
fmt.Fprintln(buf)
739+
}
731740
fmt.Fprintln(buf)
732741
}
733742
fmt.Fprint(buf, group.suggestion)
@@ -736,20 +745,22 @@ func handleError(err error, baseName, commandName, commandPath string, config *n
736745
}
737746

738747
type errorGroup struct {
739-
errs []error
740-
suggestion string
748+
errs []error
749+
suggestion string
750+
classification string
741751
}
742752
type errorGroups map[string]errorGroup
743753

744-
func (g errorGroups) Add(group string, suggestion string, err error, errs ...error) {
754+
func (g errorGroups) Add(group string, suggestion string, classification string, err error, errs ...error) {
745755
all := g[group]
746756
all.errs = append(all.errs, errs...)
747757
all.errs = append(all.errs, err)
748758
all.suggestion = suggestion
759+
all.classification = classification
749760
g[group] = all
750761
}
751762

752-
func transformRunError(err error, baseName, commandName, commandPath string, groups errorGroups) {
763+
func transformRunError(err error, baseName, commandName, commandPath string, groups errorGroups, config *newcmd.AppConfig) {
753764
switch t := err.(type) {
754765
case newcmd.ErrRequiresExplicitAccess:
755766
if t.Input.Token != nil && t.Input.Token.ServiceAccount {
@@ -762,6 +773,7 @@ func transformRunError(err error, baseName, commandName, commandPath string, gro
762773
You can see more information about the image by adding the --dry-run flag.
763774
If you trust the provided image, include the flag --grant-install-rights.`,
764775
),
776+
"",
765777
fmt.Errorf("installing %q requires an 'installer' service account with project editor access", t.Match.Value),
766778
)
767779
} else {
@@ -774,11 +786,13 @@ func transformRunError(err error, baseName, commandName, commandPath string, gro
774786
You can see more information about the image by adding the --dry-run flag.
775787
If you trust the provided image, include the flag --grant-install-rights.`,
776788
),
789+
"",
777790
fmt.Errorf("installing %q requires that you grant the image access to run with your credentials", t.Match.Value),
778791
)
779792
}
780793
return
781794
case newapp.ErrNoMatch:
795+
classification, _ := config.ComponentInputClassificationWinners[t.Value]
782796
groups.Add(
783797
"no-matches",
784798
heredoc.Docf(`
@@ -794,11 +808,13 @@ func transformRunError(err error, baseName, commandName, commandPath string, gro
794808
795809
See '%[1]s -h' for examples.`, commandPath,
796810
),
811+
heredoc.Docf(classification),
797812
t,
798813
t.Errs...,
799814
)
800815
return
801816
case newapp.ErrMultipleMatches:
817+
classification, _ := config.ComponentInputClassificationWinners[t.Value]
802818
buf := &bytes.Buffer{}
803819
for i, match := range t.Matches {
804820

@@ -812,6 +828,7 @@ func transformRunError(err error, baseName, commandName, commandPath string, gro
812828
813829
%[2]sTo view a full list of matches, use '%[3]s %[4]s -S %[1]s'`, t.Value, buf.String(), baseName, commandName,
814830
),
831+
classification,
815832
t,
816833
t.Errs...,
817834
)
@@ -830,11 +847,13 @@ func transformRunError(err error, baseName, commandName, commandPath string, gro
830847
831848
%[2]s`, t.Value, buf.String(),
832849
),
850+
classification,
833851
t,
834852
t.Errs...,
835853
)
836854
return
837855
case newapp.ErrPartialMatch:
856+
classification, _ := config.ComponentInputClassificationWinners[t.Value]
838857
buf := &bytes.Buffer{}
839858
fmt.Fprintf(buf, "* %s\n", t.Match.Description)
840859
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
846865
847866
%[2]s`, t.Value, buf.String(),
848867
),
868+
classification,
849869
t,
850870
t.Errs...,
851871
)
852872
return
853873
case newapp.ErrNoTagsFound:
874+
classification, _ := config.ComponentInputClassificationWinners[t.Value]
854875
buf := &bytes.Buffer{}
855876
fmt.Fprintf(buf, " Use --allow-missing-imagestream-tags to use this image stream\n\n")
856877
groups.Add(
@@ -860,6 +881,7 @@ func transformRunError(err error, baseName, commandName, commandPath string, gro
860881
861882
%[2]s`, t.Match.Name, buf.String(),
862883
),
884+
classification,
863885
t,
864886
t.Errs...,
865887
)
@@ -868,13 +890,14 @@ func transformRunError(err error, baseName, commandName, commandPath string, gro
868890
switch err {
869891
case errNoTokenAvailable:
870892
// 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)"))
893+
groups.Add("", "", "", fmt.Errorf("to install components you must be logged in with an OAuth token (instead of only a certificate)"))
872894
case newcmd.ErrNoInputs:
873895
// TODO: suggest things to the user
874-
groups.Add("", "", usageError(commandPath, newAppNoInput, baseName, commandName))
896+
groups.Add("", "", "", usageError(commandPath, newAppNoInput, baseName, commandName))
875897
default:
876-
groups.Add("", "", err)
898+
groups.Add("", "", "", err)
877899
}
900+
return
878901
}
879902

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

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

+6-3
Original file line numberDiff line numberDiff line change
@@ -212,9 +212,10 @@ 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.ComponentInputClassificationWinners[t.Value]
218219
groups.Add(
219220
"no-matches",
220221
heredoc.Docf(`
@@ -229,15 +230,17 @@ func transformBuildError(err error, baseName, commandName, commandPath string, g
229230
230231
See '%[1]s -h' for examples.`, commandPath,
231232
),
233+
classification,
232234
t,
233235
t.Errs...,
234236
)
235237
return
236238
}
237239
switch err {
238240
case newcmd.ErrNoInputs:
239-
groups.Add("", "", usageError(commandPath, newBuildNoInput, baseName, commandName))
241+
groups.Add("", "", "", usageError(commandPath, newBuildNoInput, baseName, commandName))
240242
return
241243
}
242-
transformRunError(err, baseName, commandName, commandPath, groups)
244+
transformRunError(err, baseName, commandName, commandPath, groups, config)
245+
return
243246
}

‎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++ {

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

+16
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,10 @@ type DockerClientSearcher struct {
4141
AllowMissingImages bool
4242
}
4343

44+
func (r DockerClientSearcher) Type() string {
45+
return "local docker images"
46+
}
47+
4448
// Search searches all images in local docker server for images that match terms
4549
func (r DockerClientSearcher) Search(precise bool, terms ...string) (ComponentMatches, []error) {
4650
componentMatches := ComponentMatches{}
@@ -164,6 +168,10 @@ func (r DockerClientSearcher) Search(precise bool, terms ...string) (ComponentMa
164168
type MissingImageSearcher struct {
165169
}
166170

171+
func (r MissingImageSearcher) Type() string {
172+
return "images not found in docker registry nor locally"
173+
}
174+
167175
// Search always returns an exact match for the search terms.
168176
func (r MissingImageSearcher) Search(precise bool, terms ...string) (ComponentMatches, []error) {
169177
componentMatches := ComponentMatches{}
@@ -184,6 +192,10 @@ type ImageImportSearcher struct {
184192
Fallback Searcher
185193
}
186194

195+
func (s ImageImportSearcher) Type() string {
196+
return "images via the image stream import API"
197+
}
198+
187199
// Search invokes the new ImageStreamImport API to have the server look up Docker images for the user,
188200
// using secrets stored on the server.
189201
func (s ImageImportSearcher) Search(precise bool, terms ...string) (ComponentMatches, []error) {
@@ -267,6 +279,10 @@ type DockerRegistrySearcher struct {
267279
AllowInsecure bool
268280
}
269281

282+
func (r DockerRegistrySearcher) Type() string {
283+
return "images in the docker registry"
284+
}
285+
270286
// Search searches in the Docker registry for images that match terms
271287
func (r DockerRegistrySearcher) Search(precise bool, terms ...string) (ComponentMatches, []error) {
272288
componentMatches := ComponentMatches{}

0 commit comments

Comments
 (0)