Skip to content
This repository has been archived by the owner on Nov 1, 2022. It is now read-only.

Ignore discovery errors for metrics resources #2009

Merged
merged 1 commit into from
May 6, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions cluster/kubernetes/sync.go
Original file line number Diff line number Diff line change
Expand Up @@ -206,6 +206,11 @@ func (c *Cluster) getAllowedResourcesBySelector(selector string) (map[string]*ku
return nil, err
}
for gv, e := range discErr.Groups {
if strings.HasSuffix(gv.Group, "metrics.k8s.io") {
// The Metrics API tends to be misconfigured, causing errors.
// We just ignore them, since it doesn't make sense to sync metrics anyways.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@stefanprodan can you confirm whether this is true? I am not that familiar with the metrics API.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

GKE comes with the metrics server and it registers the metrics.k8s.io at bootstrap. For EKS and AKS where you would install the metrics server with Helm, I don't think it will be an issue and for GKE ignoring it will not cause any problems as far as I can tell.

continue
}
// Tolerate empty GroupVersions due to e.g. misconfigured custom metrics
if e.Error() != fmt.Sprintf("Got empty response for: %v", gv) {
return nil, err
Expand Down
34 changes: 33 additions & 1 deletion cluster/kubernetes/sync_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -266,7 +266,7 @@ func TestSyncTolerateEmptyGroupVersion(t *testing.T) {

// Add a GroupVersion without API Resources
fakeClient := kube.client.coreClient.(*corefake.Clientset)
fakeClient.Resources = append(fakeClient.Resources, &metav1.APIResourceList{GroupVersion: "custom.metrics.k8s.io/v1beta1"})
fakeClient.Resources = append(fakeClient.Resources, &metav1.APIResourceList{GroupVersion: "foo.bar/v1"})

// We should tolerate the error caused in the cache due to the
// GroupVersion being empty
Expand All @@ -278,6 +278,38 @@ func TestSyncTolerateEmptyGroupVersion(t *testing.T) {
assert.NoError(t, err)
}

type failingDiscoveryClient struct {
discovery.DiscoveryInterface
}

func (d *failingDiscoveryClient) ServerResourcesForGroupVersion(groupVersion string) (*metav1.APIResourceList, error) {
return nil, errors.NewServiceUnavailable("")
}

func TestSyncTolerateMetricsErrors(t *testing.T) {
kube, _, cancel := setup(t)

// Replace the discovery client by one returning errors when asking for resources
cancel()
crdClient := crdfake.NewSimpleClientset()
shutdown := make(chan struct{})
defer close(shutdown)
newDiscoveryClient := &failingDiscoveryClient{kube.client.coreClient.Discovery()}
kube.client.discoveryClient = MakeCachedDiscovery(newDiscoveryClient, crdClient, shutdown)

// Check that syncing results in an error for groups other than metrics
fakeClient := kube.client.coreClient.(*corefake.Clientset)
fakeClient.Resources = []*metav1.APIResourceList{{GroupVersion: "foo.bar/v1"}}
err := kube.Sync(cluster.SyncSet{})
assert.Error(t, err)

// Check that syncing doesn't result in an error for a metrics group
kube.client.discoveryClient.(*cachedDiscovery).CachedDiscoveryInterface.Invalidate()
fakeClient.Resources = []*metav1.APIResourceList{{GroupVersion: "custom.metrics.k8s.io/v1"}}
err = kube.Sync(cluster.SyncSet{})
assert.NoError(t, err)
}

func TestSync(t *testing.T) {
const ns1 = `---
apiVersion: v1
Expand Down