Skip to content

Commit

Permalink
add rate limit config check back
Browse files Browse the repository at this point in the history
  • Loading branch information
@britaniar
britaniar committed Nov 7, 2024
1 parent 71fab5c commit 7c21791
Show file tree
Hide file tree
Showing 2 changed files with 116 additions and 23 deletions.
7 changes: 6 additions & 1 deletion pkg/utils/cloudconfig/azure/config.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import (
type CloudConfig struct {
azclient.ARMClientConfig `json:",inline" mapstructure:",squash"`
azclient.AzureAuthConfig `json:",inline" mapstructure:",squash"`
ratelimit.Config `json:",inline" mapstructure:",squash"`
*ratelimit.Config `json:",inline" mapstructure:",squash"` // if nil, ratelimit config will be set using default value

// azure resource location
Location string `json:"location,omitempty" mapstructure:"location,omitempty"`
Expand Down Expand Up @@ -105,6 +105,11 @@ func (cfg *CloudConfig) validate() error {
}
}

// if not specified, apply default rate limit config
if cfg.Config == nil {
cfg.Config = &ratelimit.Config{CloudProviderRateLimit: true}
}

if cfg.CloudProviderRateLimit {
// Assign read rate limit defaults if no configuration was passed in.
if cfg.CloudProviderRateLimitQPS == 0 {
Expand Down
132 changes: 110 additions & 22 deletions pkg/utils/cloudconfig/azure/config_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"github.com/google/go-cmp/cmp"
"sigs.k8s.io/cloud-provider-azure/pkg/azclient"
"sigs.k8s.io/cloud-provider-azure/pkg/azclient/policy/ratelimit"
"sigs.k8s.io/cloud-provider-azure/pkg/consts"
)

func TestTrimSpace(t *testing.T) {
Expand Down Expand Up @@ -162,6 +163,52 @@ func TestValidate(t *testing.T) {
ResourceGroup: "v",
VnetName: "vn",
VnetResourceGroup: "",
Config: &ratelimit.Config{
CloudProviderRateLimit: true,
CloudProviderRateLimitQPS: 1.0,
CloudProviderRateLimitBucket: 5,
CloudProviderRateLimitQPSWrite: 1.0,
CloudProviderRateLimitBucketWrite: 5,
},
},
wantConfig: &CloudConfig{
ARMClientConfig: azclient.ARMClientConfig{
Cloud: "c",
},
AzureAuthConfig: azclient.AzureAuthConfig{
UseManagedIdentityExtension: true,
UserAssignedIdentityID: "a",
},
Location: "l",
SubscriptionID: "s",
ResourceGroup: "v",
VnetName: "vn",
VnetResourceGroup: "v",
Config: &ratelimit.Config{
CloudProviderRateLimit: true,
CloudProviderRateLimitQPS: consts.RateLimitQPSDefault,
CloudProviderRateLimitBucket: consts.RateLimitBucketDefault,
CloudProviderRateLimitBucketWrite: consts.RateLimitBucketDefault,
CloudProviderRateLimitQPSWrite: consts.RateLimitQPSDefault,
},
},
wantPass: true,
},
"ratelimit.Config nil": {
config: &CloudConfig{
ARMClientConfig: azclient.ARMClientConfig{
Cloud: "c",
},
AzureAuthConfig: azclient.AzureAuthConfig{
UseManagedIdentityExtension: true,
UserAssignedIdentityID: "a",
},
Location: "l",
SubscriptionID: "s",
ResourceGroup: "v",
VnetName: "vn",
VnetResourceGroup: "v",
Config: nil,
},
wantConfig: &CloudConfig{
ARMClientConfig: azclient.ARMClientConfig{
Expand All @@ -176,6 +223,13 @@ func TestValidate(t *testing.T) {
ResourceGroup: "v",
VnetName: "vn",
VnetResourceGroup: "v",
Config: &ratelimit.Config{
CloudProviderRateLimit: true,
CloudProviderRateLimitQPS: consts.RateLimitQPSDefault,
CloudProviderRateLimitBucket: consts.RateLimitBucketDefault,
CloudProviderRateLimitBucketWrite: consts.RateLimitBucketDefault,
CloudProviderRateLimitQPSWrite: consts.RateLimitQPSDefault,
},
},
wantPass: true,
},
Expand Down Expand Up @@ -231,7 +285,7 @@ func TestValidate(t *testing.T) {
},
wantPass: false,
},
"RateLimitConfig values are zero": {
"ratelimit.Config values are zero": {
config: &CloudConfig{
ARMClientConfig: azclient.ARMClientConfig{
Cloud: "c",
Expand All @@ -246,7 +300,7 @@ func TestValidate(t *testing.T) {
SubscriptionID: "s",
ResourceGroup: "v",
VnetName: "vn",
Config: ratelimit.Config{
Config: &ratelimit.Config{
CloudProviderRateLimit: true,
CloudProviderRateLimitQPS: 0,
CloudProviderRateLimitBucket: 0,
Expand All @@ -269,17 +323,17 @@ func TestValidate(t *testing.T) {
ResourceGroup: "v",
VnetName: "vn",
VnetResourceGroup: "v",
Config: ratelimit.Config{
Config: &ratelimit.Config{
CloudProviderRateLimit: true,
CloudProviderRateLimitQPS: 1.0,
CloudProviderRateLimitBucket: 5,
CloudProviderRateLimitQPSWrite: 1.0,
CloudProviderRateLimitBucketWrite: 5,
CloudProviderRateLimitQPS: consts.RateLimitQPSDefault,
CloudProviderRateLimitBucket: consts.RateLimitBucketDefault,
CloudProviderRateLimitBucketWrite: consts.RateLimitBucketDefault,
CloudProviderRateLimitQPSWrite: consts.RateLimitQPSDefault,
},
},
wantPass: true,
},
"RateLimitConfig with non-zero values": {
"ratelimit.Config with non-zero values": {
config: &CloudConfig{
ARMClientConfig: azclient.ARMClientConfig{
Cloud: "c",
Expand All @@ -294,7 +348,7 @@ func TestValidate(t *testing.T) {
SubscriptionID: "s",
ResourceGroup: "v",
VnetName: "vn",
Config: ratelimit.Config{
Config: &ratelimit.Config{
CloudProviderRateLimit: true,
CloudProviderRateLimitQPS: 2,
CloudProviderRateLimitBucket: 4,
Expand All @@ -317,7 +371,7 @@ func TestValidate(t *testing.T) {
ResourceGroup: "v",
VnetName: "vn",
VnetResourceGroup: "v",
Config: ratelimit.Config{
Config: &ratelimit.Config{
CloudProviderRateLimit: true,
CloudProviderRateLimitQPS: 2,
CloudProviderRateLimitBucket: 4,
Expand All @@ -342,7 +396,7 @@ func TestValidate(t *testing.T) {
SubscriptionID: "s",
ResourceGroup: "v",
VnetName: "vn",
Config: ratelimit.Config{
Config: &ratelimit.Config{
CloudProviderRateLimit: false,
},
},
Expand All @@ -361,7 +415,7 @@ func TestValidate(t *testing.T) {
ResourceGroup: "v",
VnetName: "vn",
VnetResourceGroup: "v",
Config: ratelimit.Config{
Config: &ratelimit.Config{
CloudProviderRateLimit: false,
},
},
Expand All @@ -378,10 +432,18 @@ func TestValidate(t *testing.T) {
AADClientID: "1",
AADClientSecret: "2",
},
Location: "l",
SubscriptionID: "s",
ResourceGroup: "v",
VnetName: "vn",
Location: "l",
SubscriptionID: "s",
ResourceGroup: "v",
VnetName: "vn",
VnetResourceGroup: "v",
Config: &ratelimit.Config{
CloudProviderRateLimit: true,
CloudProviderRateLimitQPS: consts.RateLimitQPSDefault,
CloudProviderRateLimitBucket: consts.RateLimitBucketDefault,
CloudProviderRateLimitBucketWrite: consts.RateLimitBucketDefault,
CloudProviderRateLimitQPSWrite: consts.RateLimitQPSDefault,
},
},
wantConfig: &CloudConfig{
ARMClientConfig: azclient.ARMClientConfig{
Expand All @@ -398,6 +460,13 @@ func TestValidate(t *testing.T) {
ResourceGroup: "v",
VnetName: "vn",
VnetResourceGroup: "v",
Config: &ratelimit.Config{
CloudProviderRateLimit: true,
CloudProviderRateLimitQPS: consts.RateLimitQPSDefault,
CloudProviderRateLimitBucket: consts.RateLimitBucketDefault,
CloudProviderRateLimitBucketWrite: consts.RateLimitBucketDefault,
CloudProviderRateLimitQPSWrite: consts.RateLimitQPSDefault,
},
},
wantPass: true,
},
Expand All @@ -410,10 +479,18 @@ func TestValidate(t *testing.T) {
UseManagedIdentityExtension: true,
UserAssignedIdentityID: "u",
},
Location: "l",
SubscriptionID: "s",
ResourceGroup: "v",
VnetName: "vn",
Location: "l",
SubscriptionID: "s",
ResourceGroup: "v",
VnetName: "vn",
VnetResourceGroup: "v",
Config: &ratelimit.Config{
CloudProviderRateLimit: true,
CloudProviderRateLimitQPS: consts.RateLimitQPSDefault,
CloudProviderRateLimitBucket: consts.RateLimitBucketDefault,
CloudProviderRateLimitBucketWrite: consts.RateLimitBucketDefault,
CloudProviderRateLimitQPSWrite: consts.RateLimitQPSDefault,
},
},
wantConfig: &CloudConfig{
ARMClientConfig: azclient.ARMClientConfig{
Expand All @@ -428,6 +505,13 @@ func TestValidate(t *testing.T) {
ResourceGroup: "v",
VnetName: "vn",
VnetResourceGroup: "v",
Config: &ratelimit.Config{
CloudProviderRateLimit: true,
CloudProviderRateLimitQPS: consts.RateLimitQPSDefault,
CloudProviderRateLimitBucket: consts.RateLimitBucketDefault,
CloudProviderRateLimitBucketWrite: consts.RateLimitBucketDefault,
CloudProviderRateLimitQPSWrite: consts.RateLimitQPSDefault,
},
},
wantPass: true,
},
Expand Down Expand Up @@ -489,8 +573,12 @@ func TestNewCloudConfigFromFile(t *testing.T) {
ResourceGroup: "test-rg",
VnetName: "test-vnet",
VnetResourceGroup: "test-rg",
Config: ratelimit.Config{
CloudProviderRateLimit: false,
Config: &ratelimit.Config{
CloudProviderRateLimit: true,
CloudProviderRateLimitQPS: consts.RateLimitQPSDefault,
CloudProviderRateLimitBucket: consts.RateLimitBucketDefault,
CloudProviderRateLimitBucketWrite: consts.RateLimitBucketDefault,
CloudProviderRateLimitQPSWrite: consts.RateLimitQPSDefault,
},
},
wantErr: false,
Expand Down

0 comments on commit 7c21791

Please sign in to comment.