fixup! Set Azure Rate Limit Defaults from Env

Signed-off-by: Marc Sensenich <[email protected]>
kubernetes · May 9, 2020 · 1d6f18f · 1d6f18f
1 parent 0dd4891
commit 1d6f18f
Show file tree

Hide file tree

Showing 2 changed files with 108 additions and 13 deletions.
diff --git 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,52 +159,48 @@ 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
 		}
 	}
 
 	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
 		}
 	}
 
 	// 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
@@ -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)
+}