From 42bd794654b303258621f3230e651ae64cf0b3a9 Mon Sep 17 00:00:00 2001 From: chentao1596 Date: Mon, 6 Nov 2017 15:26:09 +0800 Subject: [PATCH] fix-bug: version info should be printed when failed to execute 'kubectl apply -f XXXXX' --- pkg/controller/garbagecollector/operations.go | 2 +- .../k8s.io/apimachinery/pkg/api/meta/BUILD | 1 + .../apimachinery/pkg/api/meta/errors.go | 20 ++++- .../pkg/api/meta/multirestmapper.go | 4 +- .../pkg/api/meta/multirestmapper_test.go | 84 +++++++++++++------ .../apimachinery/pkg/api/meta/priority.go | 2 +- .../pkg/api/meta/priority_test.go | 6 +- .../apimachinery/pkg/api/meta/restmapper.go | 4 +- 8 files changed, 88 insertions(+), 35 deletions(-) diff --git a/pkg/controller/garbagecollector/operations.go b/pkg/controller/garbagecollector/operations.go index 16f631f489..1c898431dc 100644 --- a/pkg/controller/garbagecollector/operations.go +++ b/pkg/controller/garbagecollector/operations.go @@ -34,7 +34,7 @@ import ( // namespace> tuple to a unversioned.APIResource struct. func (gc *GarbageCollector) apiResource(apiVersion, kind string) (*metav1.APIResource, error) { fqKind := schema.FromAPIVersionAndKind(apiVersion, kind) - mapping, err := gc.restMapper.RESTMapping(fqKind.GroupKind(), apiVersion) + mapping, err := gc.restMapper.RESTMapping(fqKind.GroupKind(), fqKind.Version) if err != nil { return nil, newRESTMappingError(kind, apiVersion) } diff --git a/staging/src/k8s.io/apimachinery/pkg/api/meta/BUILD b/staging/src/k8s.io/apimachinery/pkg/api/meta/BUILD index aeff38a57b..21097f9b9d 100644 --- a/staging/src/k8s.io/apimachinery/pkg/api/meta/BUILD +++ b/staging/src/k8s.io/apimachinery/pkg/api/meta/BUILD @@ -52,6 +52,7 @@ go_library( "//vendor/k8s.io/apimachinery/pkg/runtime/schema:go_default_library", "//vendor/k8s.io/apimachinery/pkg/types:go_default_library", "//vendor/k8s.io/apimachinery/pkg/util/errors:go_default_library", + "//vendor/k8s.io/apimachinery/pkg/util/sets:go_default_library", ], ) diff --git a/staging/src/k8s.io/apimachinery/pkg/api/meta/errors.go b/staging/src/k8s.io/apimachinery/pkg/api/meta/errors.go index 1503bd6d84..cbf5d0263c 100644 --- a/staging/src/k8s.io/apimachinery/pkg/api/meta/errors.go +++ b/staging/src/k8s.io/apimachinery/pkg/api/meta/errors.go @@ -20,6 +20,7 @@ import ( "fmt" "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apimachinery/pkg/util/sets" ) // AmbiguousResourceError is returned if the RESTMapper finds multiple matches for a resource @@ -85,11 +86,26 @@ func (e *NoResourceMatchError) Error() string { // NoKindMatchError is returned if the RESTMapper can't find any match for a kind type NoKindMatchError struct { - PartialKind schema.GroupVersionKind + // GroupKind is the API group and kind that was searched + GroupKind schema.GroupKind + // SearchedVersions is the optional list of versions the search was restricted to + SearchedVersions []string } func (e *NoKindMatchError) Error() string { - return fmt.Sprintf("no matches for %v", e.PartialKind) + searchedVersions := sets.NewString() + for _, v := range e.SearchedVersions { + searchedVersions.Insert(schema.GroupVersion{Group: e.GroupKind.Group, Version: v}.String()) + } + + switch len(searchedVersions) { + case 0: + return fmt.Sprintf("no matches for kind %q in group %q", e.GroupKind.Kind, e.GroupKind.Group) + case 1: + return fmt.Sprintf("no matches for kind %q in version %q", e.GroupKind.Kind, searchedVersions.List()[0]) + default: + return fmt.Sprintf("no matches for kind %q in versions %q", e.GroupKind.Kind, searchedVersions.List()) + } } func IsNoMatchError(err error) bool { diff --git a/staging/src/k8s.io/apimachinery/pkg/api/meta/multirestmapper.go b/staging/src/k8s.io/apimachinery/pkg/api/meta/multirestmapper.go index 679098fe56..6b01bf197f 100644 --- a/staging/src/k8s.io/apimachinery/pkg/api/meta/multirestmapper.go +++ b/staging/src/k8s.io/apimachinery/pkg/api/meta/multirestmapper.go @@ -179,7 +179,7 @@ func (m MultiRESTMapper) RESTMapping(gk schema.GroupKind, versions ...string) (* if len(errors) > 0 { return nil, utilerrors.NewAggregate(errors) } - return nil, &NoKindMatchError{PartialKind: gk.WithVersion("")} + return nil, &NoKindMatchError{GroupKind: gk, SearchedVersions: versions} } // RESTMappings returns all possible RESTMappings for the provided group kind, or an error @@ -204,7 +204,7 @@ func (m MultiRESTMapper) RESTMappings(gk schema.GroupKind, versions ...string) ( return nil, utilerrors.NewAggregate(errors) } if len(allMappings) == 0 { - return nil, &NoKindMatchError{PartialKind: gk.WithVersion("")} + return nil, &NoKindMatchError{GroupKind: gk, SearchedVersions: versions} } return allMappings, nil } diff --git a/staging/src/k8s.io/apimachinery/pkg/api/meta/multirestmapper_test.go b/staging/src/k8s.io/apimachinery/pkg/api/meta/multirestmapper_test.go index dec07a16f7..b71ca468d3 100644 --- a/staging/src/k8s.io/apimachinery/pkg/api/meta/multirestmapper_test.go +++ b/staging/src/k8s.io/apimachinery/pkg/api/meta/multirestmapper_test.go @@ -261,42 +261,78 @@ func TestMultiRESTMapperRESTMappings(t *testing.T) { tcs := []struct { name string - mapper MultiRESTMapper - input schema.GroupKind - result []*RESTMapping - err error + mapper MultiRESTMapper + groupKind schema.GroupKind + versions []string + result []*RESTMapping + err error }{ { - name: "empty", - mapper: MultiRESTMapper{}, - input: schema.GroupKind{Kind: "Foo"}, - result: nil, - err: &NoKindMatchError{PartialKind: schema.GroupVersionKind{Kind: "Foo"}}, + name: "empty with no versions", + mapper: MultiRESTMapper{}, + groupKind: schema.GroupKind{Kind: "Foo"}, + result: nil, + err: &NoKindMatchError{GroupKind: schema.GroupKind{Kind: "Foo"}}, }, { - name: "ignore not found", - mapper: MultiRESTMapper{fixedRESTMapper{err: &NoKindMatchError{PartialKind: schema.GroupVersionKind{Kind: "IGNORE_THIS"}}}}, - input: schema.GroupKind{Kind: "Foo"}, - result: nil, - err: &NoKindMatchError{PartialKind: schema.GroupVersionKind{Kind: "Foo"}}, + name: "empty with one version", + mapper: MultiRESTMapper{}, + groupKind: schema.GroupKind{Kind: "Foo"}, + versions: []string{"v1beta"}, + result: nil, + err: &NoKindMatchError{GroupKind: schema.GroupKind{Kind: "Foo"}, SearchedVersions: []string{"v1beta"}}, }, { - name: "accept first failure", - mapper: MultiRESTMapper{fixedRESTMapper{err: errors.New("fail on this")}, fixedRESTMapper{mappings: []*RESTMapping{mapping1}}}, - input: schema.GroupKind{Kind: "Foo"}, - result: nil, - err: errors.New("fail on this"), + name: "empty with multi(two) vesions", + mapper: MultiRESTMapper{}, + groupKind: schema.GroupKind{Kind: "Foo"}, + versions: []string{"v1beta", "v2"}, + result: nil, + err: &NoKindMatchError{GroupKind: schema.GroupKind{Kind: "Foo"}, SearchedVersions: []string{"v1beta", "v2"}}, }, { - name: "return both", - mapper: MultiRESTMapper{fixedRESTMapper{mappings: []*RESTMapping{mapping1}}, fixedRESTMapper{mappings: []*RESTMapping{mapping2}}}, - input: schema.GroupKind{Kind: "Foo"}, - result: []*RESTMapping{mapping1, mapping2}, + name: "ignore not found with kind not exist", + mapper: MultiRESTMapper{fixedRESTMapper{err: &NoKindMatchError{GroupKind: schema.GroupKind{Kind: "IGNORE_THIS"}}}}, + groupKind: schema.GroupKind{Kind: "Foo"}, + versions: nil, + result: nil, + err: &NoKindMatchError{GroupKind: schema.GroupKind{Kind: "Foo"}}, + }, + { + name: "ignore not found with version not exist", + mapper: MultiRESTMapper{fixedRESTMapper{err: &NoKindMatchError{GroupKind: schema.GroupKind{Kind: "Foo"}, SearchedVersions: []string{"v1"}}}}, + groupKind: schema.GroupKind{Kind: "Foo"}, + versions: []string{"v1beta"}, + result: nil, + err: &NoKindMatchError{GroupKind: schema.GroupKind{Kind: "Foo"}, SearchedVersions: []string{"v1beta"}}, + }, + { + name: "ignore not found with multi versions not exist", + mapper: MultiRESTMapper{fixedRESTMapper{err: &NoKindMatchError{GroupKind: schema.GroupKind{Kind: "Foo"}, SearchedVersions: []string{"v1"}}}}, + groupKind: schema.GroupKind{Kind: "Foo"}, + versions: []string{"v1beta", "v2"}, + result: nil, + err: &NoKindMatchError{GroupKind: schema.GroupKind{Kind: "Foo"}, SearchedVersions: []string{"v1beta", "v2"}}, + }, + { + name: "accept first failure", + mapper: MultiRESTMapper{fixedRESTMapper{err: errors.New("fail on this")}, fixedRESTMapper{mappings: []*RESTMapping{mapping1}}}, + groupKind: schema.GroupKind{Kind: "Foo"}, + versions: []string{"v1beta"}, + result: nil, + err: errors.New("fail on this"), + }, + { + name: "return both", + mapper: MultiRESTMapper{fixedRESTMapper{mappings: []*RESTMapping{mapping1}}, fixedRESTMapper{mappings: []*RESTMapping{mapping2}}}, + groupKind: schema.GroupKind{Kind: "Foo"}, + versions: []string{"v1beta"}, + result: []*RESTMapping{mapping1, mapping2}, }, } for _, tc := range tcs { - actualResult, actualErr := tc.mapper.RESTMappings(tc.input) + actualResult, actualErr := tc.mapper.RESTMappings(tc.groupKind, tc.versions...) if e, a := tc.result, actualResult; !reflect.DeepEqual(e, a) { t.Errorf("%s: expected %v, got %v", tc.name, e, a) } diff --git a/staging/src/k8s.io/apimachinery/pkg/api/meta/priority.go b/staging/src/k8s.io/apimachinery/pkg/api/meta/priority.go index 2a14aa7ab1..df28e64ffa 100644 --- a/staging/src/k8s.io/apimachinery/pkg/api/meta/priority.go +++ b/staging/src/k8s.io/apimachinery/pkg/api/meta/priority.go @@ -153,7 +153,7 @@ func kindMatches(pattern schema.GroupVersionKind, kind schema.GroupVersionKind) } func (m PriorityRESTMapper) RESTMapping(gk schema.GroupKind, versions ...string) (mapping *RESTMapping, err error) { - mappings, err := m.Delegate.RESTMappings(gk) + mappings, err := m.Delegate.RESTMappings(gk, versions...) if err != nil { return nil, err } diff --git a/staging/src/k8s.io/apimachinery/pkg/api/meta/priority_test.go b/staging/src/k8s.io/apimachinery/pkg/api/meta/priority_test.go index f273a39f9f..098d53bd51 100644 --- a/staging/src/k8s.io/apimachinery/pkg/api/meta/priority_test.go +++ b/staging/src/k8s.io/apimachinery/pkg/api/meta/priority_test.go @@ -234,13 +234,13 @@ func TestPriorityRESTMapperRESTMapping(t *testing.T) { name: "empty", mapper: PriorityRESTMapper{Delegate: MultiRESTMapper{}}, input: schema.GroupKind{Kind: "Foo"}, - err: &NoKindMatchError{PartialKind: schema.GroupVersionKind{Kind: "Foo"}}, + err: &NoKindMatchError{GroupKind: schema.GroupKind{Kind: "Foo"}}, }, { name: "ignore not found", - mapper: PriorityRESTMapper{Delegate: MultiRESTMapper{fixedRESTMapper{err: &NoKindMatchError{PartialKind: schema.GroupVersionKind{Kind: "IGNORE_THIS"}}}}}, + mapper: PriorityRESTMapper{Delegate: MultiRESTMapper{fixedRESTMapper{err: &NoKindMatchError{GroupKind: schema.GroupKind{Kind: "IGNORE_THIS"}}}}}, input: schema.GroupKind{Kind: "Foo"}, - err: &NoKindMatchError{PartialKind: schema.GroupVersionKind{Kind: "Foo"}}, + err: &NoKindMatchError{GroupKind: schema.GroupKind{Kind: "Foo"}}, }, { name: "accept first failure", diff --git a/staging/src/k8s.io/apimachinery/pkg/api/meta/restmapper.go b/staging/src/k8s.io/apimachinery/pkg/api/meta/restmapper.go index 55155a6e43..ff945acd14 100644 --- a/staging/src/k8s.io/apimachinery/pkg/api/meta/restmapper.go +++ b/staging/src/k8s.io/apimachinery/pkg/api/meta/restmapper.go @@ -472,7 +472,7 @@ func (m *DefaultRESTMapper) RESTMapping(gk schema.GroupKind, versions ...string) return nil, err } if len(mappings) == 0 { - return nil, &NoKindMatchError{PartialKind: gk.WithVersion("")} + return nil, &NoKindMatchError{GroupKind: gk, SearchedVersions: versions} } // since we rely on RESTMappings method // take the first match and return to the caller @@ -510,7 +510,7 @@ func (m *DefaultRESTMapper) RESTMappings(gk schema.GroupKind, versions ...string } if len(potentialGVK) == 0 { - return nil, &NoKindMatchError{PartialKind: gk.WithVersion("")} + return nil, &NoKindMatchError{GroupKind: gk, SearchedVersions: versions} } for _, gvk := range potentialGVK {