Automated cherry pick of #66932 #67433: Include unavailable API services in discovery response, allow failed discovery on initial quota controller start

pkg/controller/resourcequota/resource_quota_controller.go

@@ -161,11 +161,14 @@ func NewResourceQuotaController(options *ResourceQuotaControllerOptions) (*Resou
 
 		rq.quotaMonitor = qm
 
-		// do initial quota monitor setup
+		// do initial quota monitor setup.  If we have a discovery failure here, it's ok. We'll discover more resources when a later sync happens.
 		resources, err := GetQuotableResources(options.DiscoveryFunc)
-		if err != nil {
+		if discovery.IsGroupDiscoveryFailedError(err) {
+			utilruntime.HandleError(fmt.Errorf("initial discovery check failure, continuing and counting on future sync update: %v", err))
+		} else if err != nil {
 			return nil, err
 		}
+
 		if err = qm.SyncMonitors(resources); err != nil {
 			utilruntime.HandleError(fmt.Errorf("initial monitor sync has error: %v", err))
 		}
@@ -425,7 +428,16 @@ func (rq *ResourceQuotaController) Sync(discoveryFunc NamespacedResourcesFunc, p
 		newResources, err := GetQuotableResources(discoveryFunc)
 		if err != nil {
 			utilruntime.HandleError(err)
-			return
+
+			if discovery.IsGroupDiscoveryFailedError(err) && len(newResources) > 0 {
+				// In partial discovery cases, don't remove any existing informers, just add new ones
+				for k, v := range oldResources {
+					newResources[k] = v
+				}
+			} else {
+				// short circuit in non-discovery error cases or if discovery returned zero resources
+				return
+			}
 		}
 
 		// Decide whether discovery has reported a change.
@@ -470,15 +482,18 @@ func (rq *ResourceQuotaController) resyncMonitors(resources map[schema.GroupVers
 
 // GetQuotableResources returns all resources that the quota system should recognize.
 // It requires a resource supports the following verbs: 'create','list','delete'
+// This function may return both results and an error.  If that happens, it means that the discovery calls were only
+// partially successful.  A decision about whether to proceed or not is left to the caller.
 func GetQuotableResources(discoveryFunc NamespacedResourcesFunc) (map[schema.GroupVersionResource]struct{}, error) {
-	possibleResources, err := discoveryFunc()
-	if err != nil {
-		return nil, fmt.Errorf("failed to discover resources: %v", err)
+	possibleResources, discoveryErr := discoveryFunc()
+	if discoveryErr != nil && len(possibleResources) == 0 {
+		return nil, fmt.Errorf("failed to discover resources: %v", discoveryErr)
 	}
 	quotableResources := discovery.FilteredBy(discovery.SupportsAllVerbs{Verbs: []string{"create", "list", "watch", "delete"}}, possibleResources)
 	quotableGroupVersionResources, err := discovery.GroupVersionResources(quotableResources)
 	if err != nil {
 		return nil, fmt.Errorf("Failed to parse resources: %v", err)
 	}
-	return quotableGroupVersionResources, nil
+	// return the original discovery error (if any) in addition to the list
+	return quotableGroupVersionResources, discoveryErr
 }

staging/src/k8s.io/kube-aggregator/pkg/apiserver/handler_apis.go

@@ -92,10 +92,6 @@ func convertToDiscoveryAPIGroup(apiServices []*apiregistrationapi.APIService) *m
 	var discoveryGroup *metav1.APIGroup
 
 	for _, apiService := range apiServicesByGroup {
-		if !apiregistrationapi.IsAPIServiceConditionTrue(apiService, apiregistrationapi.Available) {
-			continue
-		}
-
 		// the first APIService which is valid becomes the default
 		if discoveryGroup == nil {
 			discoveryGroup = &metav1.APIGroup{

staging/src/k8s.io/kube-aggregator/pkg/apiserver/handler_apis_test.go

@@ -236,6 +236,47 @@ func TestAPIs(t *testing.T) {
 				},
 			},
 		},
+		{
+			name: "unavailable service",
+			apiservices: []*apiregistration.APIService{
+				{
+					ObjectMeta: metav1.ObjectMeta{Name: "v1.foo"},
+					Spec: apiregistration.APIServiceSpec{
+						Service: &apiregistration.ServiceReference{
+							Namespace: "ns",
+							Name:      "api",
+						},
+						Group:                "foo",
+						Version:              "v1",
+						GroupPriorityMinimum: 11,
+					},
+					Status: apiregistration.APIServiceStatus{
+						Conditions: []apiregistration.APIServiceCondition{
+							{Type: apiregistration.Available, Status: apiregistration.ConditionFalse},
+						},
+					},
+				},
+			},
+			expected: &metav1.APIGroupList{
+				TypeMeta: metav1.TypeMeta{Kind: "APIGroupList", APIVersion: "v1"},
+				Groups: []metav1.APIGroup{
+					discoveryGroup,
+					{
+						Name: "foo",
+						Versions: []metav1.GroupVersionForDiscovery{
+							{
+								GroupVersion: "foo/v1",
+								Version:      "v1",
+							},
+						},
+						PreferredVersion: metav1.GroupVersionForDiscovery{
+							GroupVersion: "foo/v1",
+							Version:      "v1",
+						},
+					},
+				},
+			},
+		},
 	}
 
 	for _, tc := range tests {

test/integration/examples/BUILD

@@ -25,12 +25,15 @@ go_test(
         "//vendor/github.com/stretchr/testify/assert:go_default_library",
         "//vendor/k8s.io/api/admissionregistration/v1beta1:go_default_library",
         "//vendor/k8s.io/api/core/v1:go_default_library",
+        "//vendor/k8s.io/apimachinery/pkg/api/errors:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
+        "//vendor/k8s.io/apimachinery/pkg/runtime/schema:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/util/wait:go_default_library",
         "//vendor/k8s.io/apiserver/pkg/apis/audit:go_default_library",
         "//vendor/k8s.io/apiserver/pkg/authorization/authorizer:go_default_library",
         "//vendor/k8s.io/apiserver/pkg/server:go_default_library",
         "//vendor/k8s.io/apiserver/pkg/server/options:go_default_library",
+        "//vendor/k8s.io/client-go/discovery:go_default_library",
         "//vendor/k8s.io/client-go/kubernetes:go_default_library",
         "//vendor/k8s.io/client-go/rest:go_default_library",
         "//vendor/k8s.io/client-go/tools/clientcmd:go_default_library",

test/integration/examples/apiserver_test.go

@@ -31,10 +31,13 @@ import (
 
 	"github.com/stretchr/testify/assert"
 
+	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/apimachinery/pkg/util/wait"
 	genericapiserver "k8s.io/apiserver/pkg/server"
 	genericapiserveroptions "k8s.io/apiserver/pkg/server/options"
+	discovery "k8s.io/client-go/discovery"
 	client "k8s.io/client-go/kubernetes"
 	"k8s.io/client-go/rest"
 	"k8s.io/client-go/tools/clientcmd"
@@ -334,9 +337,8 @@ func TestAggregatedAPIServer(t *testing.T) {
 	// this is ugly, but sleep just a little bit so that the watch is probably observed.  Since nothing will actually be added to discovery
 	// (the service is missing), we don't have an external signal.
 	time.Sleep(100 * time.Millisecond)
-	if _, err := aggregatorDiscoveryClient.Discovery().ServerResources(); err != nil {
-		t.Fatal(err)
-	}
+	_, err = aggregatorDiscoveryClient.Discovery().ServerResources()
+	assertWardleUnavailableDiscoveryError(t, err)
 
 	_, err = aggregatorClient.ApiregistrationV1beta1().APIServices().Create(&apiregistrationv1beta1.APIService{
 		ObjectMeta: metav1.ObjectMeta{Name: "v1."},
@@ -357,13 +359,32 @@ func TestAggregatedAPIServer(t *testing.T) {
 	// (the service is missing), we don't have an external signal.
 	time.Sleep(100 * time.Millisecond)
 	_, err = aggregatorDiscoveryClient.Discovery().ServerResources()
-	if err != nil {
-		t.Fatal(err)
-	}
+	assertWardleUnavailableDiscoveryError(t, err)
 
 	// TODO figure out how to turn on enough of services and dns to run more
 }
 
+func assertWardleUnavailableDiscoveryError(t *testing.T, err error) {
+	if err == nil {
+		t.Fatal("Discovery call expected to return failed unavailable service")
+	}
+	if !discovery.IsGroupDiscoveryFailedError(err) {
+		t.Fatalf("Unexpected error: %T, %v", err, err)
+	}
+	discoveryErr := err.(*discovery.ErrGroupDiscoveryFailed)
+	if len(discoveryErr.Groups) != 1 {
+		t.Fatalf("Unexpected failed groups: %v", err)
+	}
+	groupVersion := schema.GroupVersion{Group: "wardle.k8s.io", Version: "v1alpha1"}
+	groupVersionErr, ok := discoveryErr.Groups[groupVersion]
+	if !ok {
+		t.Fatalf("Unexpected failed group version: %v", err)
+	}
+	if !apierrors.IsServiceUnavailable(groupVersionErr) {
+		t.Fatalf("Unexpected failed group version error: %v", err)
+	}
+}
+
 func createKubeConfig(clientCfg *rest.Config) *clientcmdapi.Config {
 	clusterNick := "cluster"
 	userNick := "user"