From 823d92942119dfa07fd763bf6306bd67686811a8 Mon Sep 17 00:00:00 2001 From: Igor Sutton Date: Tue, 17 May 2022 20:00:57 +0200 Subject: [PATCH] Add --burst-limit option for client-side throttling limit configuration (#10842) * feat: add configuration for client-side throttling limit Client-side throttling seems to be an issue in larger environments such as OpenShift clusters, where it is common to have several hundreds CRDs out-of-the-box. From this view point, it is fair that clients should be able to fine tune this accordingly should the environment they work on evolves, which is currently not possible, and quite frustrating. This change introduces the --default-burst-limit option to helm (and its counterpart HELM_DEFAULT_BURST_LIMIT environment variable) to address that issue, allowing clients to properly tune their client usage as their environment evolves. Signed-off-by: Igor Sutton * chore: change DefaultBurstLimit to BurstLimit Signed-off-by: Igor Sutton * chore: add HELM_BURST_LIMIT to golden file Signed-off-by: Igor Sutton * chore: add burst limit tests Signed-off-by: Igor Sutton * docs: add burst limit default value to documentation Signed-off-by: Igor Sutton * refactor: change burst limit default value to 100 per review instructions Signed-off-by: Igor Sutton --- cmd/helm/root.go | 45 ++++++++++++++------------- cmd/helm/testdata/output/env-comp.txt | 1 + pkg/cli/environment.go | 13 ++++++++ pkg/cli/environment_test.go | 16 +++++++--- 4 files changed, 49 insertions(+), 26 deletions(-) diff --git a/cmd/helm/root.go b/cmd/helm/root.go index 394f241d5..53b0c0ce8 100644 --- a/cmd/helm/root.go +++ b/cmd/helm/root.go @@ -45,28 +45,29 @@ Common actions for Helm: Environment variables: -| Name | Description | -|------------------------------------|-----------------------------------------------------------------------------------| -| $HELM_CACHE_HOME | set an alternative location for storing cached files. | -| $HELM_CONFIG_HOME | set an alternative location for storing Helm configuration. | -| $HELM_DATA_HOME | set an alternative location for storing Helm data. | -| $HELM_DEBUG | indicate whether or not Helm is running in Debug mode | -| $HELM_DRIVER | set the backend storage driver. Values are: configmap, secret, memory, sql. | -| $HELM_DRIVER_SQL_CONNECTION_STRING | set the connection string the SQL storage driver should use. | -| $HELM_MAX_HISTORY | set the maximum number of helm release history. | -| $HELM_NAMESPACE | set the namespace used for the helm operations. | -| $HELM_NO_PLUGINS | disable plugins. Set HELM_NO_PLUGINS=1 to disable plugins. | -| $HELM_PLUGINS | set the path to the plugins directory | -| $HELM_REGISTRY_CONFIG | set the path to the registry config file. | -| $HELM_REPOSITORY_CACHE | set the path to the repository cache directory | -| $HELM_REPOSITORY_CONFIG | set the path to the repositories file. | -| $KUBECONFIG | set an alternative Kubernetes configuration file (default "~/.kube/config") | -| $HELM_KUBEAPISERVER | set the Kubernetes API Server Endpoint for authentication | -| $HELM_KUBECAFILE | set the Kubernetes certificate authority file. | -| $HELM_KUBEASGROUPS | set the Groups to use for impersonation using a comma-separated list. | -| $HELM_KUBEASUSER | set the Username to impersonate for the operation. | -| $HELM_KUBECONTEXT | set the name of the kubeconfig context. | -| $HELM_KUBETOKEN | set the Bearer KubeToken used for authentication. | +| Name | Description | +|------------------------------------|---------------------------------------------------------------------------------------------------| +| $HELM_CACHE_HOME | set an alternative location for storing cached files. | +| $HELM_CONFIG_HOME | set an alternative location for storing Helm configuration. | +| $HELM_DATA_HOME | set an alternative location for storing Helm data. | +| $HELM_DEBUG | indicate whether or not Helm is running in Debug mode | +| $HELM_DRIVER | set the backend storage driver. Values are: configmap, secret, memory, sql. | +| $HELM_DRIVER_SQL_CONNECTION_STRING | set the connection string the SQL storage driver should use. | +| $HELM_MAX_HISTORY | set the maximum number of helm release history. | +| $HELM_NAMESPACE | set the namespace used for the helm operations. | +| $HELM_NO_PLUGINS | disable plugins. Set HELM_NO_PLUGINS=1 to disable plugins. | +| $HELM_PLUGINS | set the path to the plugins directory | +| $HELM_REGISTRY_CONFIG | set the path to the registry config file. | +| $HELM_REPOSITORY_CACHE | set the path to the repository cache directory | +| $HELM_REPOSITORY_CONFIG | set the path to the repositories file. | +| $KUBECONFIG | set an alternative Kubernetes configuration file (default "~/.kube/config") | +| $HELM_KUBEAPISERVER | set the Kubernetes API Server Endpoint for authentication | +| $HELM_KUBECAFILE | set the Kubernetes certificate authority file. | +| $HELM_KUBEASGROUPS | set the Groups to use for impersonation using a comma-separated list. | +| $HELM_KUBEASUSER | set the Username to impersonate for the operation. | +| $HELM_KUBECONTEXT | set the name of the kubeconfig context. | +| $HELM_KUBETOKEN | set the Bearer KubeToken used for authentication. | +| $HELM_BURST_LIMIT | set the default burst limit in the case the server contains many CRDs (default 100, -1 to disable)| Helm stores cache, configuration, and data based on the following configuration order: diff --git a/cmd/helm/testdata/output/env-comp.txt b/cmd/helm/testdata/output/env-comp.txt index b7befd69e..e14e83472 100644 --- a/cmd/helm/testdata/output/env-comp.txt +++ b/cmd/helm/testdata/output/env-comp.txt @@ -1,4 +1,5 @@ HELM_BIN +HELM_BURST_LIMIT HELM_CACHE_HOME HELM_CONFIG_HOME HELM_DATA_HOME diff --git a/pkg/cli/environment.go b/pkg/cli/environment.go index d5b208015..b17172db4 100644 --- a/pkg/cli/environment.go +++ b/pkg/cli/environment.go @@ -30,6 +30,7 @@ import ( "github.com/spf13/pflag" "k8s.io/cli-runtime/pkg/genericclioptions" + "k8s.io/client-go/rest" "helm.sh/helm/v3/pkg/helmpath" ) @@ -37,6 +38,9 @@ import ( // defaultMaxHistory sets the maximum number of releases to 0: unlimited const defaultMaxHistory = 10 +// defaultBurstLimit sets the default client-side throttling limit +const defaultBurstLimit = 100 + // EnvSettings describes all of the environment settings. type EnvSettings struct { namespace string @@ -68,6 +72,8 @@ type EnvSettings struct { PluginsDirectory string // MaxHistory is the max release history maintained. MaxHistory int + // BurstLimit is the default client-side throttling limit. + BurstLimit int } func New() *EnvSettings { @@ -84,6 +90,7 @@ func New() *EnvSettings { RegistryConfig: envOr("HELM_REGISTRY_CONFIG", helmpath.ConfigPath("registry/config.json")), RepositoryConfig: envOr("HELM_REPOSITORY_CONFIG", helmpath.ConfigPath("repositories.yaml")), RepositoryCache: envOr("HELM_REPOSITORY_CACHE", helmpath.CachePath("repository")), + BurstLimit: envIntOr("HELM_BURST_LIMIT", defaultBurstLimit), } env.Debug, _ = strconv.ParseBool(os.Getenv("HELM_DEBUG")) @@ -97,6 +104,10 @@ func New() *EnvSettings { KubeConfig: &env.KubeConfig, Impersonate: &env.KubeAsUser, ImpersonateGroup: &env.KubeAsGroups, + WrapConfigFn: func(config *rest.Config) *rest.Config { + config.Burst = env.BurstLimit + return config + }, } return env } @@ -115,6 +126,7 @@ func (s *EnvSettings) AddFlags(fs *pflag.FlagSet) { fs.StringVar(&s.RegistryConfig, "registry-config", s.RegistryConfig, "path to the registry config file") fs.StringVar(&s.RepositoryConfig, "repository-config", s.RepositoryConfig, "path to the file containing repository names and URLs") fs.StringVar(&s.RepositoryCache, "repository-cache", s.RepositoryCache, "path to the file containing cached repository indexes") + fs.IntVar(&s.BurstLimit, "burst-limit", s.BurstLimit, "client-side default throttling limit") } func envOr(name, def string) string { @@ -157,6 +169,7 @@ func (s *EnvSettings) EnvVars() map[string]string { "HELM_REPOSITORY_CONFIG": s.RepositoryConfig, "HELM_NAMESPACE": s.Namespace(), "HELM_MAX_HISTORY": strconv.Itoa(s.MaxHistory), + "HELM_BURST_LIMIT": strconv.Itoa(s.BurstLimit), // broken, these are populated from helm flags and not kubeconfig. "HELM_KUBECONTEXT": s.KubeContext, diff --git a/pkg/cli/environment_test.go b/pkg/cli/environment_test.go index d2affa79d..d77799c9d 100644 --- a/pkg/cli/environment_test.go +++ b/pkg/cli/environment_test.go @@ -54,27 +54,31 @@ func TestEnvSettings(t *testing.T) { kubeAsUser string kubeAsGroups []string kubeCaFile string + burstLimit int }{ { name: "defaults", ns: "default", maxhistory: defaultMaxHistory, + burstLimit: defaultBurstLimit, }, { name: "with flags set", - args: "--debug --namespace=myns --kube-as-user=poro --kube-as-group=admins --kube-as-group=teatime --kube-as-group=snackeaters --kube-ca-file=/tmp/ca.crt", + args: "--debug --namespace=myns --kube-as-user=poro --kube-as-group=admins --kube-as-group=teatime --kube-as-group=snackeaters --kube-ca-file=/tmp/ca.crt --burst-limit 100", ns: "myns", debug: true, maxhistory: defaultMaxHistory, + burstLimit: 100, kubeAsUser: "poro", kubeAsGroups: []string{"admins", "teatime", "snackeaters"}, kubeCaFile: "/tmp/ca.crt", }, { name: "with envvars set", - envvars: map[string]string{"HELM_DEBUG": "1", "HELM_NAMESPACE": "yourns", "HELM_KUBEASUSER": "pikachu", "HELM_KUBEASGROUPS": ",,,operators,snackeaters,partyanimals", "HELM_MAX_HISTORY": "5", "HELM_KUBECAFILE": "/tmp/ca.crt"}, + envvars: map[string]string{"HELM_DEBUG": "1", "HELM_NAMESPACE": "yourns", "HELM_KUBEASUSER": "pikachu", "HELM_KUBEASGROUPS": ",,,operators,snackeaters,partyanimals", "HELM_MAX_HISTORY": "5", "HELM_KUBECAFILE": "/tmp/ca.crt", "HELM_BURST_LIMIT": "150"}, ns: "yourns", maxhistory: 5, + burstLimit: 150, debug: true, kubeAsUser: "pikachu", kubeAsGroups: []string{"operators", "snackeaters", "partyanimals"}, @@ -82,11 +86,12 @@ func TestEnvSettings(t *testing.T) { }, { name: "with flags and envvars set", - args: "--debug --namespace=myns --kube-as-user=poro --kube-as-group=admins --kube-as-group=teatime --kube-as-group=snackeaters --kube-ca-file=/my/ca.crt", - envvars: map[string]string{"HELM_DEBUG": "1", "HELM_NAMESPACE": "yourns", "HELM_KUBEASUSER": "pikachu", "HELM_KUBEASGROUPS": ",,,operators,snackeaters,partyanimals", "HELM_MAX_HISTORY": "5", "HELM_KUBECAFILE": "/tmp/ca.crt"}, + args: "--debug --namespace=myns --kube-as-user=poro --kube-as-group=admins --kube-as-group=teatime --kube-as-group=snackeaters --kube-ca-file=/my/ca.crt --burst-limit 175", + envvars: map[string]string{"HELM_DEBUG": "1", "HELM_NAMESPACE": "yourns", "HELM_KUBEASUSER": "pikachu", "HELM_KUBEASGROUPS": ",,,operators,snackeaters,partyanimals", "HELM_MAX_HISTORY": "5", "HELM_KUBECAFILE": "/tmp/ca.crt", "HELM_BURST_LIMIT": "200"}, ns: "myns", debug: true, maxhistory: 5, + burstLimit: 175, kubeAsUser: "poro", kubeAsGroups: []string{"admins", "teatime", "snackeaters"}, kubeCaFile: "/my/ca.crt", @@ -128,6 +133,9 @@ func TestEnvSettings(t *testing.T) { if tt.kubeCaFile != settings.KubeCaFile { t.Errorf("expected kCaFile %q, got %q", tt.kubeCaFile, settings.KubeCaFile) } + if tt.burstLimit != settings.BurstLimit { + t.Errorf("expected BurstLimit %d, got %d", tt.burstLimit, settings.BurstLimit) + } }) } }