From 46850966d9fca0fae8b94e5ddc9a9ec4a74bf677 Mon Sep 17 00:00:00 2001 From: Marc Sensenich Date: Mon, 4 May 2020 22:22:37 -0400 Subject: [PATCH 1/3] Set Azure Rate Limit Defaults from Env Signed-off-by: Marc Sensenich --- .../cloudprovider/azure/azure_manager.go | 61 ++++++++++++++++--- 1 file changed, 53 insertions(+), 8 deletions(-) diff --git a/cluster-autoscaler/cloudprovider/azure/azure_manager.go b/cluster-autoscaler/cloudprovider/azure/azure_manager.go index 7427addc5058..3b460b9f3161 100644 --- a/cluster-autoscaler/cloudprovider/azure/azure_manager.go +++ b/cluster-autoscaler/cloudprovider/azure/azure_manager.go @@ -148,24 +148,63 @@ type Config struct { } // InitializeCloudProviderRateLimitConfig initializes rate limit configs. -func InitializeCloudProviderRateLimitConfig(config *CloudProviderRateLimitConfig) { +func InitializeCloudProviderRateLimitConfig(config *CloudProviderRateLimitConfig) error { if config == nil { - return + return nil } // Assign read rate limit defaults if no configuration was passed in. if config.CloudProviderRateLimitQPS == 0 { - config.CloudProviderRateLimitQPS = rateLimitQPSDefault + if rateLimitQPSFromEnv := os.Getenv("RATE_LIMIT_READ_QPS"); rateLimitQPSFromEnv != "" { + rateLimitQPS, err := strconv.ParseFloat(rateLimitQPSFromEnv, 0) + if err != nil { + return fmt.Errorf("failed to parse RATE_LIMIT_READ_QPS: %q, %v", rateLimitQPSFromEnv, err) + } + klog.V(4).Infof("Set read rate limit QPS to %f", rateLimitQPS) + config.CloudProviderRateLimitQPS = float32(rateLimitQPS) + } else { + config.CloudProviderRateLimitQPS = rateLimitQPSDefault + } } + if config.CloudProviderRateLimitBucket == 0 { - config.CloudProviderRateLimitBucket = rateLimitBucketDefault + if rateLimitBucketFromEnv := os.Getenv("RATE_LIMIT_READ_BUCKETS"); rateLimitBucketFromEnv != "" { + rateLimitBucket, err := strconv.ParseInt(rateLimitBucketFromEnv, 10, 0) + if err != nil { + return fmt.Errorf("failed to parse RATE_LIMIT_READ_BUCKETS: %q, %v", rateLimitBucketFromEnv, err) + } + config.CloudProviderRateLimitBucket = int(rateLimitBucket) + klog.V(4).Infof("Set read rate limit buckets to %d", rateLimitBucket) + } else { + config.CloudProviderRateLimitBucket = rateLimitBucketDefault + } } - // Assing write rate limit defaults if no configuration was passed in. + + // Assign write rate limit defaults if no configuration was passed in. if config.CloudProviderRateLimitQPSWrite == 0 { - config.CloudProviderRateLimitQPSWrite = config.CloudProviderRateLimitQPS + if rateLimitQPSWriteFromEnv := os.Getenv("RATE_LIMIT_WRITE_QPS"); rateLimitQPSWriteFromEnv != "" { + rateLimitQPSWrite, err := strconv.ParseFloat(rateLimitQPSWriteFromEnv, 0) + if err != nil { + return fmt.Errorf("failed to parse RATE_LIMIT_WRITE_QPS: %q, %v", rateLimitQPSWriteFromEnv, err) + } + config.CloudProviderRateLimitQPSWrite = float32(rateLimitQPSWrite) + klog.V(4).Infof("Set write rate limit QPS to %f", rateLimitQPSWrite) + } else { + config.CloudProviderRateLimitQPSWrite = config.CloudProviderRateLimitQPS + } } + if config.CloudProviderRateLimitBucketWrite == 0 { - config.CloudProviderRateLimitBucketWrite = config.CloudProviderRateLimitBucket + if rateLimitBucketWriteFromEnv := os.Getenv("RATE_LIMIT_WRITE_BUCKETS"); rateLimitBucketWriteFromEnv != "" { + rateLimitBucketWrite, err := strconv.ParseInt(rateLimitBucketWriteFromEnv, 10, 0) + if err != nil { + return fmt.Errorf("failed to parse RATE_LIMIT_WRITE_BUCKET: %q, %v", rateLimitBucketWriteFromEnv, err) + } + klog.V(4).Infof("Set write rate limit buckets to %d", rateLimitBucketWrite) + config.CloudProviderRateLimitBucketWrite = int(rateLimitBucketWrite) + } else { + config.CloudProviderRateLimitBucketWrite = config.CloudProviderRateLimitBucket + } } config.InterfaceRateLimit = overrideDefaultRateLimitConfig(&config.RateLimitConfig, config.InterfaceRateLimit) @@ -173,6 +212,8 @@ func InitializeCloudProviderRateLimitConfig(config *CloudProviderRateLimitConfig config.StorageAccountRateLimit = overrideDefaultRateLimitConfig(&config.RateLimitConfig, config.StorageAccountRateLimit) config.DiskRateLimit = overrideDefaultRateLimitConfig(&config.RateLimitConfig, config.DiskRateLimit) config.VirtualMachineScaleSetRateLimit = overrideDefaultRateLimitConfig(&config.RateLimitConfig, config.VirtualMachineScaleSetRateLimit) + + return nil } // overrideDefaultRateLimitConfig overrides the default CloudProviderRateLimitConfig. @@ -358,7 +399,11 @@ func CreateAzureManager(configReader io.Reader, discoveryOpts cloudprovider.Node return nil, fmt.Errorf("failed to parse CLOUD_PROVIDER_RATE_LIMIT: %q, %v", cloudProviderRateLimit, err) } } - InitializeCloudProviderRateLimitConfig(&cfg.CloudProviderRateLimitConfig) + + err = InitializeCloudProviderRateLimitConfig(&cfg.CloudProviderRateLimitConfig) + if err != nil { + return nil, err + } // Defaulting vmType to vmss. if cfg.VMType == "" { From 0dd48913e728ec79520c99364d8fc5639b522b54 Mon Sep 17 00:00:00 2001 From: Marc Sensenich Date: Tue, 5 May 2020 07:01:08 -0400 Subject: [PATCH 2/3] Update Azure README to include new environment Signed-off-by: Marc Sensenich --- cluster-autoscaler/cloudprovider/azure/README.md | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/cluster-autoscaler/cloudprovider/azure/README.md b/cluster-autoscaler/cloudprovider/azure/README.md index 9cab61ee767a..a4019467ec2f 100644 --- a/cluster-autoscaler/cloudprovider/azure/README.md +++ b/cluster-autoscaler/cloudprovider/azure/README.md @@ -260,12 +260,12 @@ The new version of [Azure client][] supports rate limit and back-off retries whe | CloudProviderBackoffDuration | 5 | BACKOFF_DURATION | cloudProviderBackoffDuration | | CloudProviderBackoffJitter | 1.0 | BACKOFF_JITTER | cloudProviderBackoffJitter | | CloudProviderRateLimit * | false | CLOUD_PROVIDER_RATE_LIMIT | cloudProviderRateLimit | -| CloudProviderRateLimitQPS * | 1 | | cloudProviderRateLimitQPS | -| CloudProviderRateLimitBucket * | 5 | | cloudProviderRateLimitBucket | -| CloudProviderRateLimitQPSWrite * | 1 | | cloudProviderRateLimitQPSWrite | -| CloudProviderRateLimitBucketWrite * | 5 | | cloudProviderRateLimitBucketWrite | +| CloudProviderRateLimitQPS * | 1 | RATE_LIMIT_READ_QPS | cloudProviderRateLimitQPS | +| CloudProviderRateLimitBucket * | 5 | RATE_LIMIT_READ_BUCKETS | cloudProviderRateLimitBucket | +| CloudProviderRateLimitQPSWrite * | 1 | RATE_LIMIT_WRITE_QPS | cloudProviderRateLimitQPSWrite | +| CloudProviderRateLimitBucketWrite * | 5 | RATE_LIMIT_WRITE_BUCKETS | cloudProviderRateLimitBucketWrite | -> **_NOTE_**: * These rate limit configs can be set per-client. Customizing `QPS` and `Bucket` through environment variables is not supported. +> **_NOTE_**: * These rate limit configs can be set per-client. Customizing `QPS` and `Bucket` through environment variables per client is not supported. [AKS]: https://docs.microsoft.com/azure/aks/ [AKS autoscaler documentation]: https://docs.microsoft.com/azure/aks/autoscaler From 1d6f18ff3cb577b0c1f476c59359d75d20324e25 Mon Sep 17 00:00:00 2001 From: Marc Sensenich Date: Sat, 9 May 2020 17:35:47 -0400 Subject: [PATCH 3/3] fixup! Set Azure Rate Limit Defaults from Env Signed-off-by: Marc Sensenich --- .../cloudprovider/azure/azure_manager.go | 26 ++--- .../cloudprovider/azure/azure_manager_test.go | 95 +++++++++++++++++++ 2 files changed, 108 insertions(+), 13 deletions(-) diff --git a/cluster-autoscaler/cloudprovider/azure/azure_manager.go b/cluster-autoscaler/cloudprovider/azure/azure_manager.go index 3b460b9f3161..a6b2ce0849df 100644 --- a/cluster-autoscaler/cloudprovider/azure/azure_manager.go +++ b/cluster-autoscaler/cloudprovider/azure/azure_manager.go @@ -66,8 +66,12 @@ const ( backoffJitterDefault = 1.0 // rate limit - rateLimitQPSDefault = 1.0 + rateLimitQPSDefault float32 = 1.0 rateLimitBucketDefault = 5 + rateLimitReadQPSEnvVar = "RATE_LIMIT_READ_QPS" + rateLimitReadBucketsEnvVar = "RATE_LIMIT_READ_BUCKETS" + rateLimitWriteQPSEnvVar = "RATE_LIMIT_WRITE_QPS" + rateLimitWriteBucketsEnvVar = "RATE_LIMIT_WRITE_BUCKETS" ) var validLabelAutoDiscovererKeys = strings.Join([]string{ @@ -155,12 +159,11 @@ func InitializeCloudProviderRateLimitConfig(config *CloudProviderRateLimitConfig // Assign read rate limit defaults if no configuration was passed in. if config.CloudProviderRateLimitQPS == 0 { - if rateLimitQPSFromEnv := os.Getenv("RATE_LIMIT_READ_QPS"); rateLimitQPSFromEnv != "" { + if rateLimitQPSFromEnv := os.Getenv(rateLimitReadQPSEnvVar); rateLimitQPSFromEnv != "" { rateLimitQPS, err := strconv.ParseFloat(rateLimitQPSFromEnv, 0) if err != nil { - return fmt.Errorf("failed to parse RATE_LIMIT_READ_QPS: %q, %v", rateLimitQPSFromEnv, err) + return fmt.Errorf("failed to parse %s: %q, %v", rateLimitReadQPSEnvVar, rateLimitQPSFromEnv, err) } - klog.V(4).Infof("Set read rate limit QPS to %f", rateLimitQPS) config.CloudProviderRateLimitQPS = float32(rateLimitQPS) } else { config.CloudProviderRateLimitQPS = rateLimitQPSDefault @@ -168,13 +171,12 @@ func InitializeCloudProviderRateLimitConfig(config *CloudProviderRateLimitConfig } if config.CloudProviderRateLimitBucket == 0 { - if rateLimitBucketFromEnv := os.Getenv("RATE_LIMIT_READ_BUCKETS"); rateLimitBucketFromEnv != "" { + if rateLimitBucketFromEnv := os.Getenv(rateLimitReadBucketsEnvVar); rateLimitBucketFromEnv != "" { rateLimitBucket, err := strconv.ParseInt(rateLimitBucketFromEnv, 10, 0) if err != nil { - return fmt.Errorf("failed to parse RATE_LIMIT_READ_BUCKETS: %q, %v", rateLimitBucketFromEnv, err) + return fmt.Errorf("failed to parse %s: %q, %v", rateLimitReadBucketsEnvVar, rateLimitBucketFromEnv, err) } config.CloudProviderRateLimitBucket = int(rateLimitBucket) - klog.V(4).Infof("Set read rate limit buckets to %d", rateLimitBucket) } else { config.CloudProviderRateLimitBucket = rateLimitBucketDefault } @@ -182,25 +184,23 @@ func InitializeCloudProviderRateLimitConfig(config *CloudProviderRateLimitConfig // Assign write rate limit defaults if no configuration was passed in. if config.CloudProviderRateLimitQPSWrite == 0 { - if rateLimitQPSWriteFromEnv := os.Getenv("RATE_LIMIT_WRITE_QPS"); rateLimitQPSWriteFromEnv != "" { + if rateLimitQPSWriteFromEnv := os.Getenv(rateLimitWriteQPSEnvVar); rateLimitQPSWriteFromEnv != "" { rateLimitQPSWrite, err := strconv.ParseFloat(rateLimitQPSWriteFromEnv, 0) if err != nil { - return fmt.Errorf("failed to parse RATE_LIMIT_WRITE_QPS: %q, %v", rateLimitQPSWriteFromEnv, err) + return fmt.Errorf("failed to parse %s: %q, %v", rateLimitWriteQPSEnvVar, rateLimitQPSWriteFromEnv, err) } config.CloudProviderRateLimitQPSWrite = float32(rateLimitQPSWrite) - klog.V(4).Infof("Set write rate limit QPS to %f", rateLimitQPSWrite) } else { config.CloudProviderRateLimitQPSWrite = config.CloudProviderRateLimitQPS } } if config.CloudProviderRateLimitBucketWrite == 0 { - if rateLimitBucketWriteFromEnv := os.Getenv("RATE_LIMIT_WRITE_BUCKETS"); rateLimitBucketWriteFromEnv != "" { + if rateLimitBucketWriteFromEnv := os.Getenv(rateLimitWriteBucketsEnvVar); rateLimitBucketWriteFromEnv != "" { rateLimitBucketWrite, err := strconv.ParseInt(rateLimitBucketWriteFromEnv, 10, 0) if err != nil { - return fmt.Errorf("failed to parse RATE_LIMIT_WRITE_BUCKET: %q, %v", rateLimitBucketWriteFromEnv, err) + return fmt.Errorf("failed to parse %s: %q, %v", rateLimitWriteBucketsEnvVar, rateLimitBucketWriteFromEnv, err) } - klog.V(4).Infof("Set write rate limit buckets to %d", rateLimitBucketWrite) config.CloudProviderRateLimitBucketWrite = int(rateLimitBucketWrite) } else { config.CloudProviderRateLimitBucketWrite = config.CloudProviderRateLimitBucket diff --git a/cluster-autoscaler/cloudprovider/azure/azure_manager_test.go b/cluster-autoscaler/cloudprovider/azure/azure_manager_test.go index 69787faa29eb..646715d9097d 100644 --- a/cluster-autoscaler/cloudprovider/azure/azure_manager_test.go +++ b/cluster-autoscaler/cloudprovider/azure/azure_manager_test.go @@ -18,6 +18,7 @@ package azure import ( "fmt" + "os" "reflect" "strings" "testing" @@ -366,3 +367,97 @@ func TestFetchAutoAsgsVmss(t *testing.T) { assert.Equal(t, minVal, asgs[0].MinSize()) assert.Equal(t, maxVal, asgs[0].MaxSize()) } + +func TestInitializeCloudProviderRateLimitConfigWithNoConfigReturnsNoError(t *testing.T) { + err := InitializeCloudProviderRateLimitConfig(nil) + assert.Nil(t, err, "err should be nil") +} + +func TestInitializeCloudProviderRateLimitConfigWithNoRateLimitSettingsReturnsDefaults(t *testing.T) { + emptyConfig := &CloudProviderRateLimitConfig{} + err := InitializeCloudProviderRateLimitConfig(emptyConfig) + + assert.NoError(t, err) + assert.Equal(t, emptyConfig.CloudProviderRateLimitQPS, rateLimitQPSDefault) + assert.Equal(t, emptyConfig.CloudProviderRateLimitBucket, rateLimitBucketDefault) + assert.Equal(t, emptyConfig.CloudProviderRateLimitQPSWrite, rateLimitQPSDefault) + assert.Equal(t, emptyConfig.CloudProviderRateLimitBucketWrite, rateLimitBucketDefault) +} + +func TestInitializeCloudProviderRateLimitConfigWithReadRateLimitSettingsFromEnv(t *testing.T) { + emptyConfig := &CloudProviderRateLimitConfig{} + var rateLimitReadQPS float32 = 3.0 + rateLimitReadBuckets := 10 + os.Setenv(rateLimitReadQPSEnvVar, fmt.Sprintf("%.1f", rateLimitReadQPS)) + os.Setenv(rateLimitReadBucketsEnvVar, fmt.Sprintf("%d", rateLimitReadBuckets)) + + err := InitializeCloudProviderRateLimitConfig(emptyConfig) + assert.NoError(t, err) + assert.Equal(t, emptyConfig.CloudProviderRateLimitQPS, rateLimitReadQPS) + assert.Equal(t, emptyConfig.CloudProviderRateLimitBucket, rateLimitReadBuckets) + assert.Equal(t, emptyConfig.CloudProviderRateLimitQPSWrite, rateLimitReadQPS) + assert.Equal(t, emptyConfig.CloudProviderRateLimitBucketWrite, rateLimitReadBuckets) + + os.Unsetenv(rateLimitReadBucketsEnvVar) + os.Unsetenv(rateLimitReadQPSEnvVar) +} + +func TestInitializeCloudProviderRateLimitConfigWithReadAndWriteRateLimitSettingsFromEnv(t *testing.T) { + emptyConfig := &CloudProviderRateLimitConfig{} + var rateLimitReadQPS float32 = 3.0 + rateLimitReadBuckets := 10 + var rateLimitWriteQPS float32 = 6.0 + rateLimitWriteBuckets := 20 + + os.Setenv(rateLimitReadQPSEnvVar, fmt.Sprintf("%.1f", rateLimitReadQPS)) + os.Setenv(rateLimitReadBucketsEnvVar, fmt.Sprintf("%d", rateLimitReadBuckets)) + os.Setenv(rateLimitWriteQPSEnvVar, fmt.Sprintf("%.1f", rateLimitWriteQPS)) + os.Setenv(rateLimitWriteBucketsEnvVar, fmt.Sprintf("%d", rateLimitWriteBuckets)) + + err := InitializeCloudProviderRateLimitConfig(emptyConfig) + + assert.NoError(t, err) + assert.Equal(t, emptyConfig.CloudProviderRateLimitQPS, rateLimitReadQPS) + assert.Equal(t, emptyConfig.CloudProviderRateLimitBucket, rateLimitReadBuckets) + assert.Equal(t, emptyConfig.CloudProviderRateLimitQPSWrite, rateLimitWriteQPS) + assert.Equal(t, emptyConfig.CloudProviderRateLimitBucketWrite, rateLimitWriteBuckets) + + os.Unsetenv(rateLimitReadQPSEnvVar) + os.Unsetenv(rateLimitReadBucketsEnvVar) + os.Unsetenv(rateLimitWriteQPSEnvVar) + os.Unsetenv(rateLimitWriteBucketsEnvVar) +} + +func TestInitializeCloudProviderRateLimitConfigWithReadAndWriteRateLimitAlreadySetInConfig(t *testing.T) { + var rateLimitReadQPS float32 = 3.0 + rateLimitReadBuckets := 10 + var rateLimitWriteQPS float32 = 6.0 + rateLimitWriteBuckets := 20 + + configWithRateLimits := &CloudProviderRateLimitConfig{ + RateLimitConfig: azclients.RateLimitConfig{ + CloudProviderRateLimitBucket: rateLimitReadBuckets, + CloudProviderRateLimitBucketWrite: rateLimitWriteBuckets, + CloudProviderRateLimitQPS: rateLimitReadQPS, + CloudProviderRateLimitQPSWrite: rateLimitWriteQPS, + }, + } + + os.Setenv(rateLimitReadQPSEnvVar, "99") + os.Setenv(rateLimitReadBucketsEnvVar, "99") + os.Setenv(rateLimitWriteQPSEnvVar, "99") + os.Setenv(rateLimitWriteBucketsEnvVar, "99") + + err := InitializeCloudProviderRateLimitConfig(configWithRateLimits) + + assert.NoError(t, err) + assert.Equal(t, configWithRateLimits.CloudProviderRateLimitQPS, rateLimitReadQPS) + assert.Equal(t, configWithRateLimits.CloudProviderRateLimitBucket, rateLimitReadBuckets) + assert.Equal(t, configWithRateLimits.CloudProviderRateLimitQPSWrite, rateLimitWriteQPS) + assert.Equal(t, configWithRateLimits.CloudProviderRateLimitBucketWrite, rateLimitWriteBuckets) + + os.Unsetenv(rateLimitReadQPSEnvVar) + os.Unsetenv(rateLimitReadBucketsEnvVar) + os.Unsetenv(rateLimitWriteQPSEnvVar) + os.Unsetenv(rateLimitWriteBucketsEnvVar) +}