Merge pull request #33234 from caesarxuchao/retry-discovery-failure

Automatic merge from submit-queue

Discovery client retry when failed to discovery resrouces

Fix #32308

`ServerPreferredNamespacedResources()` fails if in the middle of its execution, the TPR e2e tests change the supported resources on the API server. This PR let the e2e test framework retry `ServerPreferredNamespacedResources()`.

cc @lavalamp

pkg/client/typed/discovery/discovery_client.go

@@ -199,39 +199,46 @@ func IsGroupDiscoveryFailedError(err error) bool {
 // 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) {
-	results := []unversioned.GroupVersionResource{}
-	serverGroupList, err := d.ServerGroups()
-	if err != nil {
-		return results, err
-	}
-
+	// retry in case the groups supported by the server change after ServerGroup() returns.
+	const maxRetries = 2
 	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)
+	var results []unversioned.GroupVersionResource
+RetrieveGroups:
+	for i := 0; i < maxRetries; i++ {
+		results = []unversioned.GroupVersionResource{}
+		failedGroups = make(map[unversioned.GroupVersion]error)
+		serverGroupList, err := d.ServerGroups()
 		if err != nil {
-			if failedGroups == nil {
-				failedGroups = make(map[unversioned.GroupVersion]error)
-			}
-			failedGroups[groupVersion] = err
-			continue
+			return results, err
 		}
-		for _, apiResource := range apiResourceList.APIResources {
-			// ignore the root scoped resources if "namespaced" is true.
-			if namespaced && !apiResource.Namespaced {
+
+		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 {
+				if i < maxRetries-1 {
+					continue RetrieveGroups
+				}
+				failedGroups[groupVersion] = err
 				continue
 			}
-			if strings.Contains(apiResource.Name, "/") {
-				continue
+			for _, apiResource := range apiResourceList.APIResources {
+				// ignore the root scoped resources if "namespaced" is true.
+				if namespaced && !apiResource.Namespaced {
+					continue
+				}
+				if strings.Contains(apiResource.Name, "/") {
+					continue
+				}
+				results = append(results, groupVersion.WithResource(apiResource.Name))
 			}
-			results = append(results, groupVersion.WithResource(apiResource.Name))
+		}
+		if len(failedGroups) == 0 {
+			return results, nil
 		}
 	}
-	if len(failedGroups) > 0 {
-		return results, &ErrGroupDiscoveryFailed{Groups: failedGroups}
-	}
-	return results, nil
+	return results, &ErrGroupDiscoveryFailed{Groups: failedGroups}
 }
 
 // ServerPreferredResources returns the supported resources with the version preferred by the

pkg/client/typed/discovery/discovery_client_test.go

@@ -454,3 +454,102 @@ func TestGetServerPreferredResources(t *testing.T) {
 		server.Close()
 	}
 }
+
+func TestGetServerPreferredResourcesRetries(t *testing.T) {
+	stable := unversioned.APIResourceList{
+		GroupVersion: "v1",
+		APIResources: []unversioned.APIResource{
+			{Name: "pods", Namespaced: true, Kind: "Pod"},
+		},
+	}
+	beta := unversioned.APIResourceList{
+		GroupVersion: "extensions/v1",
+		APIResources: []unversioned.APIResource{
+			{Name: "deployments", Namespaced: true, Kind: "Deployment"},
+		},
+	}
+
+	response := func(numErrors int) http.HandlerFunc {
+		var i = 0
+		return func(w http.ResponseWriter, req *http.Request) {
+			var list interface{}
+			switch req.URL.Path {
+			case "/apis/extensions/v1beta1":
+				if i < numErrors {
+					i++
+					w.WriteHeader(http.StatusInternalServerError)
+					return
+				}
+				list = &beta
+			case "/api/v1":
+				list = &stable
+			case "/api":
+				list = &unversioned.APIVersions{
+					Versions: []string{
+						"v1",
+					},
+				}
+			case "/apis":
+				list = &unversioned.APIGroupList{
+					Groups: []unversioned.APIGroup{
+						{
+							Name: "extensions",
+							Versions: []unversioned.GroupVersionForDiscovery{
+								{GroupVersion: "extensions/v1beta1"},
+							},
+							PreferredVersion: unversioned.GroupVersionForDiscovery{
+								GroupVersion: "extensions/v1beta1",
+								Version:      "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)
+		}
+	}
+	tests := []struct {
+		responseErrors  int
+		expectResources int
+		expectedError   func(err error) bool
+	}{
+		{
+			responseErrors:  1,
+			expectResources: 2,
+			expectedError: func(err error) bool {
+				return err == nil
+			},
+		},
+		{
+			responseErrors:  2,
+			expectResources: 1,
+			expectedError:   IsGroupDiscoveryFailedError,
+		},
+	}
+
+	for i, tc := range tests {
+		server := httptest.NewServer(http.HandlerFunc(response(tc.responseErrors)))
+		defer server.Close()
+
+		client := NewDiscoveryClientForConfigOrDie(&restclient.Config{Host: server.URL})
+		got, err := client.ServerPreferredResources()
+		if !tc.expectedError(err) {
+			t.Errorf("case %d: unexpected error: %v", i, err)
+		}
+		if len(got) != tc.expectResources {
+			t.Errorf("case %d: expect %d resources, got %#v", i, tc.expectResources, got)
+		}
+		server.Close()
+	}
+}