Gracefully handle failed API groups from the discovery API (#1293)

* log details and continue executing if error of type ErrGroupDiscoveryFailed is returned by discovery API

Signed-off-by: fabito <fuechi@ciandt.com>

changelogs/unreleased/1293-fabito

@@ -0,0 +1 @@
+gracefully handle failed API groups from the discovery API

pkg/discovery/helper.go

@@ -51,6 +51,10 @@ type Helper interface {
 	APIGroups() []metav1.APIGroup
 }
 
+type serverResourcesInterface interface {
+	ServerPreferredResources() ([]*metav1.APIResourceList, error)
+}
+
 type helper struct {
 	discoveryClient discovery.DiscoveryInterface
 	logger          logrus.FieldLogger
@@ -107,7 +111,7 @@ func (h *helper) Refresh() error {
 	}
 	h.mapper = shortcutExpander
 
-	preferredResources, err := h.discoveryClient.ServerPreferredResources()
+	preferredResources, err := refreshServerPreferredResources(h.discoveryClient, h.logger)
 	if err != nil {
 		return errors.WithStack(err)
 	}
@@ -141,6 +145,19 @@ func (h *helper) Refresh() error {
 	return nil
 }
 
+func refreshServerPreferredResources(discoveryClient serverResourcesInterface, logger logrus.FieldLogger) ([]*metav1.APIResourceList, error) {
+	preferredResources, err := discoveryClient.ServerPreferredResources()
+	if err != nil {
+		if discoveryErr, ok := err.(*discovery.ErrGroupDiscoveryFailed); ok {
+			for groupVersion, err := range discoveryErr.Groups {
+				logger.WithError(err).Warnf("Failed to discover group: %v", groupVersion)
+			}
+			return preferredResources, nil
+		}
+	}
+	return preferredResources, err
+}
+
 func filterByVerbs(groupVersion string, r *metav1.APIResource) bool {
 	return discovery.SupportsAllVerbs{Verbs: []string{"list", "create", "get", "delete"}}.Match(groupVersion, r)
 }

pkg/discovery/helper_test.go

@@ -17,10 +17,16 @@ limitations under the License.
 package discovery
 
 import (
+	"errors"
 	"testing"
 
+	"github.com/sirupsen/logrus"
 	"github.com/stretchr/testify/assert"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/runtime/schema"
+
+	"github.com/heptio/velero/pkg/util/logging"
+	velerotest "github.com/heptio/velero/pkg/util/test"
 )
 
 func TestSortResources(t *testing.T) {
@@ -140,3 +146,53 @@ func TestFilteringByVerbs(t *testing.T) {
 		})
 	}
 }
+
+func TestRefreshServerPreferredResources(t *testing.T) {
+	tests := []struct {
+		name         string
+		resourceList []*metav1.APIResourceList
+		failedGroups map[schema.GroupVersion]error
+		returnError  error
+	}{
+		{
+			name: "all groups discovered, no error is returned",
+			resourceList: []*metav1.APIResourceList{
+				{GroupVersion: "groupB/v1"},
+				{GroupVersion: "apps/v1beta1"},
+				{GroupVersion: "extensions/v1beta1"},
+			},
+		},
+		{
+			name: "failed to discover some groups, no error is returned",
+			resourceList: []*metav1.APIResourceList{
+				{GroupVersion: "groupB/v1"},
+				{GroupVersion: "apps/v1beta1"},
+				{GroupVersion: "extensions/v1beta1"},
+			},
+			failedGroups: map[schema.GroupVersion]error{
+				{Group: "groupA", Version: "v1"}: errors.New("Fake error"),
+				{Group: "groupC", Version: "v2"}: errors.New("Fake error"),
+			},
+		},
+		{
+			name:        "non ErrGroupDiscoveryFailed error, returns error",
+			returnError: errors.New("Generic error"),
+		},
+	}
+
+	for _, test := range tests {
+		fakeServer := velerotest.NewFakeServerResourcesInterface(test.resourceList, test.failedGroups, test.returnError)
+		t.Run(test.name, func(t *testing.T) {
+			resources, err := refreshServerPreferredResources(fakeServer, logging.DefaultLogger(logrus.DebugLevel))
+			if test.returnError != nil {
+				assert.NotNil(t, err)
+			} else {
+				assert.Nil(t, err)
+				assert.Equal(t, test.returnError, err)
+			}
+
+			assert.Equal(t, test.resourceList, resources)
+		})
+	}
+
+}

pkg/util/test/fake_discovery_helper.go

@@ -23,6 +23,7 @@ import (
 	"k8s.io/apimachinery/pkg/api/meta"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime/schema"
+	"k8s.io/client-go/discovery"
 )
 
 type FakeDiscoveryHelper struct {
@@ -123,3 +124,28 @@ func (dh *FakeDiscoveryHelper) ResourceFor(input schema.GroupVersionResource) (s
 func (dh *FakeDiscoveryHelper) APIGroups() []metav1.APIGroup {
 	return dh.APIGroupsList
 }
+
+type FakeServerResourcesInterface struct {
+	ResourceList []*metav1.APIResourceList
+	FailedGroups map[schema.GroupVersion]error
+	ReturnError  error
+}
+
+func (di *FakeServerResourcesInterface) ServerPreferredResources() ([]*metav1.APIResourceList, error) {
+	if di.ReturnError != nil {
+		return di.ResourceList, di.ReturnError
+	}
+	if di.FailedGroups == nil || len(di.FailedGroups) == 0 {
+		return di.ResourceList, nil
+	}
+	return di.ResourceList, &discovery.ErrGroupDiscoveryFailed{Groups: di.FailedGroups}
+}
+
+func NewFakeServerResourcesInterface(resourceList []*metav1.APIResourceList, failedGroups map[schema.GroupVersion]error, returnError error) *FakeServerResourcesInterface {
+	helper := &FakeServerResourcesInterface{
+		ResourceList: resourceList,
+		FailedGroups: failedGroups,
+		ReturnError:  returnError,
+	}
+	return helper
+}