From 711dc0b5b5e0d6d792e43ae1edd4a5dd1c5bdf2f Mon Sep 17 00:00:00 2001 From: Jordan Liggitt Date: Wed, 11 Dec 2019 11:36:51 -0500 Subject: [PATCH] Increase Burst limit for discovery client --- CHANGELOG-1.17.md | 1 + .../client-go/discovery/discovery_client.go | 7 ++ .../discovery/discovery_client_test.go | 112 +++++++++++++++++- 3 files changed, 118 insertions(+), 2 deletions(-) diff --git a/CHANGELOG-1.17.md b/CHANGELOG-1.17.md index 916ee8221a6..8ab52ea4d88 100644 --- a/CHANGELOG-1.17.md +++ b/CHANGELOG-1.17.md @@ -133,6 +133,7 @@ The Kubernetes in-tree storage plugin to Container Storage Interface (CSI) migra ## Known Issues - volumeDevices mapping ignored when container is privileged - The `Should recreate evicted statefulset` conformance [test]( https://github.com/kubernetes/kubernetes/blob/master/test/e2e/apps/statefulset.go) fails because `Pod ss-0 expected to be re-created at least once`. This was caused by the `Predicate PodFitsHostPorts failed` scheduling error. The root cause was a host port conflict for port `21017`. This port was in-use as an ephemeral port by another application running on the node. This will be looked at for the 1.18 release. +- client-go discovery clients constructed using `NewDiscoveryClientForConfig` or `NewDiscoveryClientForConfigOrDie` default to rate limits that cause normal discovery request patterns to take several seconds. This is fixed in https://issue.k8s.io/86168 and will be resolved in v1.17.1. As a workaround, the `Burst` value can be adjusted higher in the rest.Config passed into `NewDiscoveryClientForConfig` or `NewDiscoveryClientForConfigOrDie`. ## Urgent Upgrade Notes ### (No, really, you MUST read this before you upgrade) diff --git a/staging/src/k8s.io/client-go/discovery/discovery_client.go b/staging/src/k8s.io/client-go/discovery/discovery_client.go index 61b9c4481bc..5d89457cca1 100644 --- a/staging/src/k8s.io/client-go/discovery/discovery_client.go +++ b/staging/src/k8s.io/client-go/discovery/discovery_client.go @@ -463,6 +463,13 @@ func setDiscoveryDefaults(config *restclient.Config) error { if config.Timeout == 0 { config.Timeout = defaultTimeout } + if config.Burst == 0 && config.QPS < 100 { + // discovery is expected to be bursty, increase the default burst + // to accommodate looking up resource info for many API groups. + // matches burst set by ConfigFlags#ToDiscoveryClient(). + // see https://issue.k8s.io/86149 + config.Burst = 100 + } codec := runtime.NoopEncoder{Decoder: scheme.Codecs.UniversalDecoder()} config.NegotiatedSerializer = serializer.NegotiatedSerializerWrapper(runtime.SerializerInfo{Serializer: codec}) if len(config.UserAgent) == 0 { diff --git a/staging/src/k8s.io/client-go/discovery/discovery_client_test.go b/staging/src/k8s.io/client-go/discovery/discovery_client_test.go index 10e49432280..653c062adc7 100644 --- a/staging/src/k8s.io/client-go/discovery/discovery_client_test.go +++ b/staging/src/k8s.io/client-go/discovery/discovery_client_test.go @@ -24,6 +24,7 @@ import ( "net/http/httptest" "reflect" "testing" + "time" "github.com/gogo/protobuf/proto" "github.com/googleapis/gnostic/OpenAPIv2" @@ -198,6 +199,26 @@ func TestGetServerResources(t *testing.T) { {Name: "jobs", Namespaced: true, Kind: "Job"}, }, } + extensionsbeta3 := metav1.APIResourceList{GroupVersion: "extensions/v1beta3", APIResources: []metav1.APIResource{{Name: "deployments", Namespaced: true, Kind: "Deployment"}}} + extensionsbeta4 := metav1.APIResourceList{GroupVersion: "extensions/v1beta4", APIResources: []metav1.APIResource{{Name: "deployments", Namespaced: true, Kind: "Deployment"}}} + extensionsbeta5 := metav1.APIResourceList{GroupVersion: "extensions/v1beta5", APIResources: []metav1.APIResource{{Name: "deployments", Namespaced: true, Kind: "Deployment"}}} + extensionsbeta6 := metav1.APIResourceList{GroupVersion: "extensions/v1beta6", APIResources: []metav1.APIResource{{Name: "deployments", Namespaced: true, Kind: "Deployment"}}} + extensionsbeta7 := metav1.APIResourceList{GroupVersion: "extensions/v1beta7", APIResources: []metav1.APIResource{{Name: "deployments", Namespaced: true, Kind: "Deployment"}}} + extensionsbeta8 := metav1.APIResourceList{GroupVersion: "extensions/v1beta8", APIResources: []metav1.APIResource{{Name: "deployments", Namespaced: true, Kind: "Deployment"}}} + extensionsbeta9 := metav1.APIResourceList{GroupVersion: "extensions/v1beta9", APIResources: []metav1.APIResource{{Name: "deployments", Namespaced: true, Kind: "Deployment"}}} + extensionsbeta10 := metav1.APIResourceList{GroupVersion: "extensions/v1beta10", APIResources: []metav1.APIResource{{Name: "deployments", Namespaced: true, Kind: "Deployment"}}} + + appsbeta1 := metav1.APIResourceList{GroupVersion: "apps/v1beta1", APIResources: []metav1.APIResource{{Name: "deployments", Namespaced: true, Kind: "Deployment"}}} + appsbeta2 := metav1.APIResourceList{GroupVersion: "apps/v1beta2", APIResources: []metav1.APIResource{{Name: "deployments", Namespaced: true, Kind: "Deployment"}}} + appsbeta3 := metav1.APIResourceList{GroupVersion: "apps/v1beta3", APIResources: []metav1.APIResource{{Name: "deployments", Namespaced: true, Kind: "Deployment"}}} + appsbeta4 := metav1.APIResourceList{GroupVersion: "apps/v1beta4", APIResources: []metav1.APIResource{{Name: "deployments", Namespaced: true, Kind: "Deployment"}}} + appsbeta5 := metav1.APIResourceList{GroupVersion: "apps/v1beta5", APIResources: []metav1.APIResource{{Name: "deployments", Namespaced: true, Kind: "Deployment"}}} + appsbeta6 := metav1.APIResourceList{GroupVersion: "apps/v1beta6", APIResources: []metav1.APIResource{{Name: "deployments", Namespaced: true, Kind: "Deployment"}}} + appsbeta7 := metav1.APIResourceList{GroupVersion: "apps/v1beta7", APIResources: []metav1.APIResource{{Name: "deployments", Namespaced: true, Kind: "Deployment"}}} + appsbeta8 := metav1.APIResourceList{GroupVersion: "apps/v1beta8", APIResources: []metav1.APIResource{{Name: "deployments", Namespaced: true, Kind: "Deployment"}}} + appsbeta9 := metav1.APIResourceList{GroupVersion: "apps/v1beta9", APIResources: []metav1.APIResource{{Name: "deployments", Namespaced: true, Kind: "Deployment"}}} + appsbeta10 := metav1.APIResourceList{GroupVersion: "apps/v1beta10", APIResources: []metav1.APIResource{{Name: "deployments", Namespaced: true, Kind: "Deployment"}}} + tests := []struct { resourcesList *metav1.APIResourceList path string @@ -232,6 +253,42 @@ func TestGetServerResources(t *testing.T) { list = &beta case "/apis/extensions/v1beta2": list = &beta2 + case "/apis/extensions/v1beta3": + list = &extensionsbeta3 + case "/apis/extensions/v1beta4": + list = &extensionsbeta4 + case "/apis/extensions/v1beta5": + list = &extensionsbeta5 + case "/apis/extensions/v1beta6": + list = &extensionsbeta6 + case "/apis/extensions/v1beta7": + list = &extensionsbeta7 + case "/apis/extensions/v1beta8": + list = &extensionsbeta8 + case "/apis/extensions/v1beta9": + list = &extensionsbeta9 + case "/apis/extensions/v1beta10": + list = &extensionsbeta10 + case "/apis/apps/v1beta1": + list = &appsbeta1 + case "/apis/apps/v1beta2": + list = &appsbeta2 + case "/apis/apps/v1beta3": + list = &appsbeta3 + case "/apis/apps/v1beta4": + list = &appsbeta4 + case "/apis/apps/v1beta5": + list = &appsbeta5 + case "/apis/apps/v1beta6": + list = &appsbeta6 + case "/apis/apps/v1beta7": + list = &appsbeta7 + case "/apis/apps/v1beta8": + list = &appsbeta8 + case "/apis/apps/v1beta9": + list = &appsbeta9 + case "/apis/apps/v1beta10": + list = &appsbeta10 case "/api": list = &metav1.APIVersions{ Versions: []string{ @@ -241,11 +298,34 @@ func TestGetServerResources(t *testing.T) { case "/apis": list = &metav1.APIGroupList{ Groups: []metav1.APIGroup{ + { + Name: "apps", + Versions: []metav1.GroupVersionForDiscovery{ + {GroupVersion: "apps/v1beta1", Version: "v1beta1"}, + {GroupVersion: "apps/v1beta2", Version: "v1beta2"}, + {GroupVersion: "apps/v1beta3", Version: "v1beta3"}, + {GroupVersion: "apps/v1beta4", Version: "v1beta4"}, + {GroupVersion: "apps/v1beta5", Version: "v1beta5"}, + {GroupVersion: "apps/v1beta6", Version: "v1beta6"}, + {GroupVersion: "apps/v1beta7", Version: "v1beta7"}, + {GroupVersion: "apps/v1beta8", Version: "v1beta8"}, + {GroupVersion: "apps/v1beta9", Version: "v1beta9"}, + {GroupVersion: "apps/v1beta10", Version: "v1beta10"}, + }, + }, { Name: "extensions", Versions: []metav1.GroupVersionForDiscovery{ {GroupVersion: "extensions/v1beta1", Version: "v1beta1"}, {GroupVersion: "extensions/v1beta2", Version: "v1beta2"}, + {GroupVersion: "extensions/v1beta3", Version: "v1beta3"}, + {GroupVersion: "extensions/v1beta4", Version: "v1beta4"}, + {GroupVersion: "extensions/v1beta5", Version: "v1beta5"}, + {GroupVersion: "extensions/v1beta6", Version: "v1beta6"}, + {GroupVersion: "extensions/v1beta7", Version: "v1beta7"}, + {GroupVersion: "extensions/v1beta8", Version: "v1beta8"}, + {GroupVersion: "extensions/v1beta9", Version: "v1beta9"}, + {GroupVersion: "extensions/v1beta10", Version: "v1beta10"}, }, }, }, @@ -265,8 +345,8 @@ func TestGetServerResources(t *testing.T) { w.Write(output) })) defer server.Close() - client := NewDiscoveryClientForConfigOrDie(&restclient.Config{Host: server.URL}) for _, test := range tests { + client := NewDiscoveryClientForConfigOrDie(&restclient.Config{Host: server.URL}) got, err := client.ServerResourcesForGroupVersion(test.request) if test.expectErr { if err == nil { @@ -283,12 +363,40 @@ func TestGetServerResources(t *testing.T) { } } + client := NewDiscoveryClientForConfigOrDie(&restclient.Config{Host: server.URL}) + start := time.Now() serverResources, err := client.ServerResources() if err != nil { t.Errorf("unexpected error: %v", err) } + end := time.Now() + if d := end.Sub(start); d > time.Second { + t.Errorf("took too long to perform discovery: %s", d) + } serverGroupVersions := groupVersions(serverResources) - expectedGroupVersions := []string{"v1", "extensions/v1beta1", "extensions/v1beta2"} + expectedGroupVersions := []string{ + "v1", + "apps/v1beta1", + "apps/v1beta2", + "apps/v1beta3", + "apps/v1beta4", + "apps/v1beta5", + "apps/v1beta6", + "apps/v1beta7", + "apps/v1beta8", + "apps/v1beta9", + "apps/v1beta10", + "extensions/v1beta1", + "extensions/v1beta2", + "extensions/v1beta3", + "extensions/v1beta4", + "extensions/v1beta5", + "extensions/v1beta6", + "extensions/v1beta7", + "extensions/v1beta8", + "extensions/v1beta9", + "extensions/v1beta10", + } if !reflect.DeepEqual(expectedGroupVersions, serverGroupVersions) { t.Errorf("unexpected group versions: %v", diff.ObjectReflectDiff(expectedGroupVersions, serverGroupVersions)) }