From 88b64a7a8209c2a0698ea4e33a002ad267f57074 Mon Sep 17 00:00:00 2001 From: Clayton Coleman Date: Wed, 7 Sep 2016 00:52:10 -0400 Subject: [PATCH] Use a structured error rather than an Aggregate error in discovery Should provide more information for debugging the root cause of discovery failures. --- .../typed/discovery/discovery_client.go | 42 +++++- ...lient_test.go => discovery_client_test.go} | 134 ++++++++++++++++++ 2 files changed, 169 insertions(+), 7 deletions(-) rename pkg/client/typed/discovery/{client_test.go => discovery_client_test.go} (71%) diff --git a/pkg/client/typed/discovery/discovery_client.go b/pkg/client/typed/discovery/discovery_client.go index bded981f031..714e6311e5e 100644 --- a/pkg/client/typed/discovery/discovery_client.go +++ b/pkg/client/typed/discovery/discovery_client.go @@ -20,6 +20,7 @@ import ( "encoding/json" "fmt" "net/url" + "sort" "strings" "github.com/emicklei/go-restful/swagger" @@ -31,7 +32,6 @@ import ( "k8s.io/kubernetes/pkg/client/restclient" "k8s.io/kubernetes/pkg/runtime" "k8s.io/kubernetes/pkg/runtime/serializer" - utilerrors "k8s.io/kubernetes/pkg/util/errors" "k8s.io/kubernetes/pkg/version" ) @@ -149,9 +149,8 @@ func (d *DiscoveryClient) ServerResourcesForGroupVersion(groupVersion string) (r // ignore 403 or 404 error to be compatible with an v1.0 server. if groupVersion == "v1" && (errors.IsNotFound(err) || errors.IsForbidden(err)) { return resources, nil - } else { - return nil, err } + return nil, err } return resources, nil } @@ -174,6 +173,29 @@ func (d *DiscoveryClient) ServerResources() (map[string]*unversioned.APIResource return result, nil } +// ErrGroupDiscoveryFailed is returned if one or more API groups fail to load. +type ErrGroupDiscoveryFailed struct { + // Groups is a list of the groups that failed to load and the error cause + Groups map[unversioned.GroupVersion]error +} + +// Error implements the error interface +func (e *ErrGroupDiscoveryFailed) Error() string { + var groups []string + for k, v := range e.Groups { + groups = append(groups, fmt.Sprintf("%s: %v", k, v)) + } + sort.Strings(groups) + return fmt.Sprintf("unable to retrieve the complete list of server APIs: %s", strings.Join(groups, ", ")) +} + +// IsGroupDiscoveryFailedError returns true if the provided error indicates the server was unable to discover +// a complete list of APIs for the client to use. +func IsGroupDiscoveryFailedError(err error) bool { + _, ok := err.(*ErrGroupDiscoveryFailed) + return err != nil && ok +} + // serverPreferredResources returns the supported resources with the version preferred by the // server. If namespaced is true, only namespaced resources will be returned. func (d *DiscoveryClient) serverPreferredResources(namespaced bool) ([]unversioned.GroupVersionResource, error) { @@ -183,15 +205,18 @@ func (d *DiscoveryClient) serverPreferredResources(namespaced bool) ([]unversion return results, err } - allErrs := []error{} + var failedGroups map[unversioned.GroupVersion]error for _, apiGroup := range serverGroupList.Groups { preferredVersion := apiGroup.PreferredVersion + groupVersion := unversioned.GroupVersion{Group: apiGroup.Name, Version: preferredVersion.Version} apiResourceList, err := d.ServerResourcesForGroupVersion(preferredVersion.GroupVersion) if err != nil { - allErrs = append(allErrs, err) + if failedGroups == nil { + failedGroups = make(map[unversioned.GroupVersion]error) + } + failedGroups[groupVersion] = err continue } - groupVersion := unversioned.GroupVersion{Group: apiGroup.Name, Version: preferredVersion.Version} for _, apiResource := range apiResourceList.APIResources { // ignore the root scoped resources if "namespaced" is true. if namespaced && !apiResource.Namespaced { @@ -203,7 +228,10 @@ func (d *DiscoveryClient) serverPreferredResources(namespaced bool) ([]unversion results = append(results, groupVersion.WithResource(apiResource.Name)) } } - return results, utilerrors.NewAggregate(allErrs) + if len(failedGroups) > 0 { + return results, &ErrGroupDiscoveryFailed{Groups: failedGroups} + } + return results, nil } // ServerPreferredResources returns the supported resources with the version preferred by the diff --git a/pkg/client/typed/discovery/client_test.go b/pkg/client/typed/discovery/discovery_client_test.go similarity index 71% rename from pkg/client/typed/discovery/client_test.go rename to pkg/client/typed/discovery/discovery_client_test.go index df6b86d34a9..1ede2500bdc 100644 --- a/pkg/client/typed/discovery/client_test.go +++ b/pkg/client/typed/discovery/discovery_client_test.go @@ -320,3 +320,137 @@ func TestGetSwaggerSchemaFail(t *testing.T) { t.Errorf("expected an error, got %v", err) } } + +func TestGetServerPreferredResources(t *testing.T) { + stable := unversioned.APIResourceList{ + GroupVersion: "v1", + APIResources: []unversioned.APIResource{ + {Name: "pods", Namespaced: true, Kind: "Pod"}, + {Name: "services", Namespaced: true, Kind: "Service"}, + {Name: "namespaces", Namespaced: false, Kind: "Namespace"}, + }, + } + /*beta := unversioned.APIResourceList{ + GroupVersion: "extensions/v1", + APIResources: []unversioned.APIResource{ + {Name: "deployments", Namespaced: true, Kind: "Deployment"}, + {Name: "ingresses", Namespaced: true, Kind: "Ingress"}, + {Name: "jobs", Namespaced: true, Kind: "Job"}, + }, + }*/ + tests := []struct { + resourcesList *unversioned.APIResourceList + response func(w http.ResponseWriter, req *http.Request) + expectErr func(err error) bool + }{ + { + resourcesList: &stable, + expectErr: IsGroupDiscoveryFailedError, + response: func(w http.ResponseWriter, req *http.Request) { + var list interface{} + switch req.URL.Path { + case "/apis/extensions/v1beta1": + w.WriteHeader(http.StatusInternalServerError) + return + case "/api/v1": + list = &stable + case "/api": + list = &unversioned.APIVersions{ + Versions: []string{ + "v1", + }, + } + case "/apis": + list = &unversioned.APIGroupList{ + Groups: []unversioned.APIGroup{ + { + Versions: []unversioned.GroupVersionForDiscovery{ + {GroupVersion: "extensions/v1beta1"}, + }, + }, + }, + } + default: + t.Logf("unexpected request: %s", req.URL.Path) + w.WriteHeader(http.StatusNotFound) + return + } + output, err := json.Marshal(list) + if err != nil { + t.Errorf("unexpected encoding error: %v", err) + return + } + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusOK) + w.Write(output) + }, + }, + { + resourcesList: nil, + expectErr: IsGroupDiscoveryFailedError, + response: func(w http.ResponseWriter, req *http.Request) { + var list interface{} + switch req.URL.Path { + case "/apis/extensions/v1beta1": + w.WriteHeader(http.StatusInternalServerError) + return + case "/api/v1": + w.WriteHeader(http.StatusInternalServerError) + case "/api": + list = &unversioned.APIVersions{ + Versions: []string{ + "v1", + }, + } + case "/apis": + list = &unversioned.APIGroupList{ + Groups: []unversioned.APIGroup{ + { + Versions: []unversioned.GroupVersionForDiscovery{ + {GroupVersion: "extensions/v1beta1"}, + }, + }, + }, + } + default: + t.Logf("unexpected request: %s", req.URL.Path) + w.WriteHeader(http.StatusNotFound) + return + } + output, err := json.Marshal(list) + if err != nil { + t.Errorf("unexpected encoding error: %v", err) + return + } + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusOK) + w.Write(output) + }, + }, + /*{ + resourcesList: &stable, + },*/ + } + for _, test := range tests { + server := httptest.NewServer(http.HandlerFunc(test.response)) + defer server.Close() + + client := NewDiscoveryClientForConfigOrDie(&restclient.Config{Host: server.URL}) + got, err := client.ServerPreferredResources() + if test.expectErr != nil { + if err == nil { + t.Error("unexpected non-error") + } + + continue + } + if err != nil { + t.Errorf("unexpected error: %v", err) + continue + } + if !reflect.DeepEqual(got, test.resourcesList) { + t.Errorf("expected:\n%v\ngot:\n%v\n", test.resourcesList, got) + } + server.Close() + } +}