From ba0f44a8328c087cb42ba46675ea6a105e67c215 Mon Sep 17 00:00:00 2001 From: Tatenda Zifudzi Date: Wed, 3 Jul 2024 20:32:22 -0700 Subject: [PATCH] Add Windows secondary IP mode configurable options for managing IP address allocation #443 --- controllers/core/configmap_controller.go | 41 +- controllers/core/configmap_controller_test.go | 40 +- main.go | 5 +- pkg/config/loader.go | 99 +++-- pkg/config/loader_test.go | 66 ++- pkg/config/type.go | 1 + pkg/pool/pool.go | 98 ++++- pkg/pool/pool_test.go | 389 ++++++++++++++++-- pkg/provider/branch/provider.go | 6 +- pkg/provider/ip/provider.go | 6 +- pkg/provider/ip/provider_test.go | 46 ++- pkg/provider/prefix/provider.go | 19 +- pkg/provider/prefix/provider_test.go | 21 +- pkg/provider/provider.go | 26 +- pkg/provider/provider_test.go | 112 +++++ 15 files changed, 821 insertions(+), 154 deletions(-) create mode 100644 pkg/provider/provider_test.go diff --git a/controllers/core/configmap_controller.go b/controllers/core/configmap_controller.go index e56e3847..a35261b1 100644 --- a/controllers/core/configmap_controller.go +++ b/controllers/core/configmap_controller.go @@ -46,8 +46,8 @@ type ConfigMapReconciler struct { Condition condition.Conditions curWinIPAMEnabledCond bool curWinPrefixDelegationEnabledCond bool - curWinPDWarmIPTarget int - curWinPDMinIPTarget int + curWinWarmIPTarget int + curWinMinIPTarget int curWinPDWarmPrefixTarget int Context context.Context } @@ -116,21 +116,34 @@ func (r *ConfigMapReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( isPrefixFlagUpdated = true } - // Check if configurations for Windows prefix delegation have changed - var isPDConfigUpdated bool - warmIPTarget, minIPTarget, warmPrefixTarget := config.ParseWinPDTargets(r.Log, configmap) - if r.curWinPDWarmIPTarget != warmIPTarget || r.curWinPDMinIPTarget != minIPTarget || r.curWinPDWarmPrefixTarget != warmPrefixTarget { - r.curWinPDWarmIPTarget = warmIPTarget - r.curWinPDMinIPTarget = minIPTarget - r.curWinPDWarmPrefixTarget = warmPrefixTarget - logger.Info("updated PD configs from configmap", config.WarmIPTarget, r.curWinPDWarmIPTarget, - config.MinimumIPTarget, r.curWinPDMinIPTarget, config.WarmPrefixTarget, r.curWinPDWarmPrefixTarget) + // Check if Windows IP target configurations in ConfigMap have changed + var isWinIPConfigsUpdated bool - isPDConfigUpdated = true + warmIPTarget, minIPTarget, warmPrefixTarget, isPDEnabled := config.ParseWinIPTargetConfigs(r.Log, configmap) + var winMinIPTargetUpdated = r.curWinMinIPTarget != minIPTarget + var winWarmIPTargetUpdated = r.curWinWarmIPTarget != warmIPTarget + var winPDWarmPrefixTargetUpdated = r.curWinPDWarmPrefixTarget != warmPrefixTarget + if winWarmIPTargetUpdated || winMinIPTargetUpdated { + r.curWinWarmIPTarget = warmIPTarget + r.curWinMinIPTarget = minIPTarget + r.curWinPDWarmPrefixTarget = warmPrefixTarget + isWinIPConfigsUpdated = true + } + if isPDEnabled && winPDWarmPrefixTargetUpdated { + isWinIPConfigsUpdated = true + } + if isWinIPConfigsUpdated { + logger.Info( + "Detected update in Windows IP configuration parameter values in ConfigMap", + config.WinWarmIPTarget, r.curWinWarmIPTarget, + config.WinMinimumIPTarget, r.curWinMinIPTarget, + config.WinWarmPrefixTarget, r.curWinPDWarmPrefixTarget, + config.EnableWindowsPrefixDelegationKey, isPDEnabled, + ) } - // Flag is updated, update all nodes - if isIPAMFlagUpdated || isPrefixFlagUpdated || isPDConfigUpdated { + var nodesRequireUpdate = isIPAMFlagUpdated || isPrefixFlagUpdated || isWinIPConfigsUpdated + if nodesRequireUpdate { err := UpdateNodesOnConfigMapChanges(r.K8sAPI, r.NodeManager) if err != nil { // Error in updating nodes diff --git a/controllers/core/configmap_controller_test.go b/controllers/core/configmap_controller_test.go index 34635b3c..e3936d46 100644 --- a/controllers/core/configmap_controller_test.go +++ b/controllers/core/configmap_controller_test.go @@ -16,14 +16,9 @@ package controllers import ( "context" "errors" + "strconv" "testing" - mock_condition "github.com/aws/amazon-vpc-resource-controller-k8s/mocks/amazon-vcp-resource-controller-k8s/pkg/condition" - mock_k8s "github.com/aws/amazon-vpc-resource-controller-k8s/mocks/amazon-vcp-resource-controller-k8s/pkg/k8s" - mock_node "github.com/aws/amazon-vpc-resource-controller-k8s/mocks/amazon-vcp-resource-controller-k8s/pkg/node" - mock_manager "github.com/aws/amazon-vpc-resource-controller-k8s/mocks/amazon-vcp-resource-controller-k8s/pkg/node/manager" - "github.com/aws/amazon-vpc-resource-controller-k8s/pkg/config" - cooldown "github.com/aws/amazon-vpc-resource-controller-k8s/pkg/provider/branch/cooldown" "github.com/golang/mock/gomock" "github.com/stretchr/testify/assert" corev1 "k8s.io/api/core/v1" @@ -35,18 +30,35 @@ import ( fakeClient "sigs.k8s.io/controller-runtime/pkg/client/fake" "sigs.k8s.io/controller-runtime/pkg/log/zap" "sigs.k8s.io/controller-runtime/pkg/reconcile" + + mock_condition "github.com/aws/amazon-vpc-resource-controller-k8s/mocks/amazon-vcp-resource-controller-k8s/pkg/condition" + mock_k8s "github.com/aws/amazon-vpc-resource-controller-k8s/mocks/amazon-vcp-resource-controller-k8s/pkg/k8s" + mock_node "github.com/aws/amazon-vpc-resource-controller-k8s/mocks/amazon-vcp-resource-controller-k8s/pkg/node" + mock_manager "github.com/aws/amazon-vpc-resource-controller-k8s/mocks/amazon-vcp-resource-controller-k8s/pkg/node/manager" + "github.com/aws/amazon-vpc-resource-controller-k8s/pkg/config" + cooldown "github.com/aws/amazon-vpc-resource-controller-k8s/pkg/provider/branch/cooldown" ) var ( mockConfigMap = &corev1.ConfigMap{ TypeMeta: metav1.TypeMeta{}, ObjectMeta: metav1.ObjectMeta{Name: config.VpcCniConfigMapName, Namespace: config.KubeSystemNamespace}, - Data: map[string]string{config.EnableWindowsIPAMKey: "true", config.EnableWindowsPrefixDelegationKey: "true"}, + Data: map[string]string{ + config.EnableWindowsIPAMKey: "true", + config.EnableWindowsPrefixDelegationKey: "true", + config.WinMinimumIPTarget: strconv.Itoa(config.IPv4DefaultWinMinIPTarget), + config.WinWarmIPTarget: strconv.Itoa(config.IPv4DefaultWinWarmIPTarget), + }, } mockConfigMapPD = &corev1.ConfigMap{ TypeMeta: metav1.TypeMeta{}, ObjectMeta: metav1.ObjectMeta{Name: config.VpcCniConfigMapName, Namespace: config.KubeSystemNamespace}, - Data: map[string]string{config.EnableWindowsIPAMKey: "false", config.EnableWindowsPrefixDelegationKey: "true"}, + Data: map[string]string{ + config.EnableWindowsIPAMKey: "false", + config.EnableWindowsPrefixDelegationKey: "true", + config.WinMinimumIPTarget: strconv.Itoa(config.IPv4PDDefaultMinIPTargetSize), + config.WinWarmIPTarget: strconv.Itoa(config.IPv4PDDefaultWarmIPTargetSize), + }, } mockConfigMapReq = reconcile.Request{ NamespacedName: types.NamespacedName{ @@ -89,11 +101,13 @@ func NewConfigMapMock(ctrl *gomock.Controller, mockObjects ...client.Object) Con return ConfigMapMock{ MockNodeManager: mockNodeManager, ConfigMapReconciler: &ConfigMapReconciler{ - Client: client, - Log: zap.New(), - NodeManager: mockNodeManager, - K8sAPI: mockK8sWrapper, - Condition: mockCondition, + Client: client, + Log: zap.New(), + NodeManager: mockNodeManager, + K8sAPI: mockK8sWrapper, + Condition: mockCondition, + curWinMinIPTarget: config.IPv4DefaultWinMinIPTarget, + curWinWarmIPTarget: config.IPv4DefaultWinWarmIPTarget, }, MockNode: mockNode, MockK8sAPI: mockK8sWrapper, diff --git a/main.go b/main.go index 83043cb8..5016e99b 100644 --- a/main.go +++ b/main.go @@ -22,6 +22,7 @@ import ( "time" crdv1alpha1 "github.com/aws/amazon-vpc-cni-k8s/pkg/apis/crd/v1alpha1" + vpcresourcesv1alpha1 "github.com/aws/amazon-vpc-resource-controller-k8s/apis/vpcresources/v1alpha1" vpcresourcesv1beta1 "github.com/aws/amazon-vpc-resource-controller-k8s/apis/vpcresources/v1beta1" "github.com/aws/amazon-vpc-resource-controller-k8s/controllers/apps" @@ -143,8 +144,8 @@ func main() { "Port for serving the introspection API") flag.BoolVar(&enableWindowsPrefixDelegation, "enable-windows-prefix-delegation", false, "Enable the feature flag for Windows prefix delegation") - flag.StringVar(®ion, "aws-region", "", "The aws region of the k8s cluster") - flag.StringVar(&vpcID, "vpc-id", "", "The VPC ID where EKS cluster is deployed") + flag.StringVar(®ion, "aws-region", "us-west-2", "The aws region of the k8s cluster") + flag.StringVar(&vpcID, "vpc-id", "vpc-09efbf2426cd7dc1a", "The VPC ID where EKS cluster is deployed") flag.Parse() diff --git a/pkg/config/loader.go b/pkg/config/loader.go index 0a40ab02..e4ba4ed3 100644 --- a/pkg/config/loader.go +++ b/pkg/config/loader.go @@ -28,13 +28,14 @@ const ( // Default Configuration for Pod ENI resource type PodENIDefaultWorker = 30 - // Default Configuration for IPv4 resource type - IPv4DefaultWorker = 2 - IPv4DefaultWPSize = 3 - IPv4DefaultMaxDev = 1 - IPv4DefaultResSize = 0 - - // Default Configuration for IPv4 prefix resource type + // Default Windows Configuration for IPv4 resource type + IPv4DefaultWinWorkerCount = 2 + IPv4DefaultWinWarmIPTarget = 1 + IPv4DefaultWinMinIPTarget = 3 + IPv4DefaultWinMaxDev = 0 + IPv4DefaultWinResSize = 0 + + // Default Windows Configuration for IPv4 prefix resource type IPv4PDDefaultWorker = 2 IPv4PDDefaultWPSize = 1 IPv4PDDefaultMaxDev = 0 @@ -70,26 +71,43 @@ func LoadResourceConfig() map[string]ResourceConfig { func LoadResourceConfigFromConfigMap(log logr.Logger, vpcCniConfigMap *v1.ConfigMap) map[string]ResourceConfig { resourceConfig := getDefaultResourceConfig() - warmIPTarget, minIPTarget, warmPrefixTarget := ParseWinPDTargets(log, vpcCniConfigMap) + warmIPTarget, minIPTarget, warmPrefixTarget, isPDEnabled := ParseWinIPTargetConfigs(log, vpcCniConfigMap) // If no PD configuration is set in configMap or none is valid, return default resource config if warmIPTarget == 0 && minIPTarget == 0 && warmPrefixTarget == 0 { return resourceConfig } - resourceConfig[ResourceNameIPAddressFromPrefix].WarmPoolConfig.WarmIPTarget = warmIPTarget - resourceConfig[ResourceNameIPAddressFromPrefix].WarmPoolConfig.MinIPTarget = minIPTarget - resourceConfig[ResourceNameIPAddressFromPrefix].WarmPoolConfig.WarmPrefixTarget = warmPrefixTarget + if isPDEnabled { + resourceConfig[ResourceNameIPAddressFromPrefix].WarmPoolConfig.WarmIPTarget = warmIPTarget + resourceConfig[ResourceNameIPAddressFromPrefix].WarmPoolConfig.MinIPTarget = minIPTarget + resourceConfig[ResourceNameIPAddressFromPrefix].WarmPoolConfig.WarmPrefixTarget = warmPrefixTarget + } else { + resourceConfig[ResourceNameIPAddress].WarmPoolConfig.WarmIPTarget = warmIPTarget + resourceConfig[ResourceNameIPAddress].WarmPoolConfig.MinIPTarget = minIPTarget + resourceConfig[ResourceNameIPAddress].WarmPoolConfig.WarmPrefixTarget = warmPrefixTarget // ignore warm prefix in secondary IP mode + } return resourceConfig } -// ParseWinPDTargets parses config map for Windows prefix delegation configurations set by users -func ParseWinPDTargets(log logr.Logger, vpcCniConfigMap *v1.ConfigMap) (warmIPTarget int, minIPTarget int, warmPrefixTarget int) { +// ParseWinIPTargetConfigs parses Windows IP target configuration parameters in the amazon-vpc-cni ConfigMap +func ParseWinIPTargetConfigs(log logr.Logger, vpcCniConfigMap *v1.ConfigMap) (warmIPTarget int, minIPTarget int, warmPrefixTarget int, isPDEnabled bool) { warmIPTarget, minIPTarget, warmPrefixTarget = 0, 0, 0 if vpcCniConfigMap.Data == nil { - return warmIPTarget, minIPTarget, warmPrefixTarget + log.V(1).Info("No configuration found in ConfigMap, falling back to using secondary IP mode with default values") + isPDEnabled = false + minIPTarget = IPv4DefaultWinMinIPTarget + warmIPTarget = IPv4DefaultWinWarmIPTarget + warmPrefixTarget = 0 + return warmIPTarget, minIPTarget, warmPrefixTarget, isPDEnabled + } + + isPDEnabled, err := strconv.ParseBool(vpcCniConfigMap.Data[EnableWindowsPrefixDelegationKey]) + if err != nil { + log.V(1).Info("Failed to parse prefix delegation flag from ConfigMap, falling back to using secondary IP mode") + isPDEnabled = false } warmIPTargetStr, foundWarmIP := vpcCniConfigMap.Data[WarmIPTarget] @@ -105,36 +123,59 @@ func ParseWinPDTargets(log logr.Logger, vpcCniConfigMap *v1.ConfigMap) (warmIPTa warmPrefixTargetStr, foundWarmPrefix = vpcCniConfigMap.Data[WinWarmPrefixTarget] } - // If no configuration is found, return 0 - if !foundWarmIP && !foundMinIP && !foundWarmPrefix { - return warmIPTarget, minIPTarget, warmPrefixTarget - } - + // Handle the scenario where warm IP target is not configured if foundWarmIP { warmIPTargetInt, err := strconv.Atoi(warmIPTargetStr) if err != nil { - log.Error(err, "failed to parse warm ip target", "warm ip target", warmIPTargetStr) + log.Error(err, "failed to parse warm ip target, defaulting to zero", "warm ip target", warmIPTargetStr) + warmIPTarget = 0 } else { warmIPTarget = warmIPTargetInt } + } else { + log.V(1).Info("warm IP configuration not found in ConfigMap, will use default value") + if isPDEnabled { + warmIPTarget = IPv4PDDefaultWarmIPTargetSize + } else { + warmIPTarget = IPv4DefaultWinWarmIPTarget + } } + + // Handle the scenario where minimum IP target is not configured if foundMinIP { minIPTargetInt, err := strconv.Atoi(minIPTargetStr) if err != nil { - log.Error(err, "failed to parse minimum ip target", "minimum ip target", minIPTargetStr) + log.Error(err, "failed to parse minimum ip target, defaulting to zero", "minimum ip target", minIPTargetStr) + minIPTarget = 0 } else { minIPTarget = minIPTargetInt } + } else { + log.V(1).Info("minimum IP configuration not found in ConfigMap, will use default value") + if isPDEnabled { + minIPTarget = IPv4PDDefaultMinIPTargetSize + } else { + minIPTarget = IPv4DefaultWinMinIPTarget + } } - if foundWarmPrefix { + + // Handle the scenario where warm prefix target is not configured + if !isPDEnabled && foundWarmPrefix { + log.V(1).Info("warm prefix configuration not supported in secondary IP mode, will ignore warm prefix configuration") + } else if isPDEnabled && foundWarmPrefix { warmPrefixTargetInt, err := strconv.Atoi(warmPrefixTargetStr) if err != nil { - log.Error(err, "failed to parse warm prefix target", "warm prefix target", warmPrefixTargetStr) + log.Error(err, "failed to parse warm prefix target, defaulting to zero", "warm prefix target", warmPrefixTargetStr) + warmPrefixTarget = 0 } else { warmPrefixTarget = warmPrefixTargetInt } + } else if isPDEnabled && !foundWarmPrefix { + log.V(1).Info("warm prefix configuration not found in ConfigMap, will use default value") + warmPrefixTarget = IPv4PDDefaultWarmPrefixTargetSize } - return warmIPTarget, minIPTarget, warmPrefixTarget + + return warmIPTarget, minIPTarget, warmPrefixTarget, isPDEnabled } // getDefaultResourceConfig returns the default Resource Configuration. @@ -153,13 +194,15 @@ func getDefaultResourceConfig() map[string]ResourceConfig { // Create default configuration for IPv4 Resource ipV4WarmPoolConfig := WarmPoolConfig{ - DesiredSize: IPv4DefaultWPSize, - MaxDeviation: IPv4DefaultMaxDev, - ReservedSize: IPv4DefaultResSize, + DesiredSize: IPv4DefaultWinWarmIPTarget, + WarmIPTarget: IPv4DefaultWinWarmIPTarget, + MinIPTarget: IPv4DefaultWinMinIPTarget, + MaxDeviation: IPv4DefaultWinMaxDev, + ReservedSize: IPv4DefaultWinResSize, } ipV4Config := ResourceConfig{ Name: ResourceNameIPAddress, - WorkerCount: IPv4DefaultWorker, + WorkerCount: IPv4DefaultWinWorkerCount, SupportedOS: map[string]bool{OSWindows: true, OSLinux: false}, WarmPoolConfig: &ipV4WarmPoolConfig, } diff --git a/pkg/config/loader_test.go b/pkg/config/loader_test.go index 88fa4b33..ca8b759a 100644 --- a/pkg/config/loader_test.go +++ b/pkg/config/loader_test.go @@ -36,14 +36,15 @@ func TestLoadResourceConfig(t *testing.T) { // Verify default resource configuration for resource IPv4 Address ipV4Config := defaultResourceConfig[ResourceNameIPAddress] assert.Equal(t, ResourceNameIPAddress, ipV4Config.Name) - assert.Equal(t, IPv4DefaultWorker, ipV4Config.WorkerCount) + assert.Equal(t, IPv4DefaultWinWorkerCount, ipV4Config.WorkerCount) assert.Equal(t, map[string]bool{OSLinux: false, OSWindows: true}, ipV4Config.SupportedOS) // Verify default Warm pool configuration for IPv4 Address ipV4WPConfig := ipV4Config.WarmPoolConfig - assert.Equal(t, IPv4DefaultWPSize, ipV4WPConfig.DesiredSize) - assert.Equal(t, IPv4DefaultMaxDev, ipV4WPConfig.MaxDeviation) - assert.Equal(t, IPv4DefaultResSize, ipV4WPConfig.ReservedSize) + assert.Equal(t, IPv4DefaultWinWarmIPTarget, ipV4WPConfig.DesiredSize) + assert.Equal(t, IPv4DefaultWinMinIPTarget, ipV4WPConfig.MinIPTarget) + assert.Equal(t, IPv4DefaultWinMaxDev, ipV4WPConfig.MaxDeviation) + assert.Equal(t, IPv4DefaultWinResSize, ipV4WPConfig.ReservedSize) // Verify default resource configuration for prefix-deconstructed IPv4 Address prefixIPv4Config := defaultResourceConfig[ResourceNameIPAddressFromPrefix] @@ -74,14 +75,51 @@ func TestParseWinPDTargets(t *testing.T) { WarmPrefixTarget: strconv.Itoa(IPv4PDDefaultWarmPrefixTargetSize), }, } - warmIPTarget, minIPTarget, warmPrefixTarget := ParseWinPDTargets(log, vpcCNIConfig) + warmIPTarget, minIPTarget, warmPrefixTarget, _ := ParseWinIPTargetConfigs(log, vpcCNIConfig) assert.Equal(t, IPv4PDDefaultWarmIPTargetSize, warmIPTarget) assert.Equal(t, IPv4PDDefaultMinIPTargetSize, minIPTarget) assert.Equal(t, IPv4PDDefaultWarmPrefixTargetSize, warmPrefixTarget) } +func TestParseWinIPTargetConfigs_PDDisabledAndInvalidConfig_ReturnsZero(t *testing.T) { + log := zap.New(zap.UseDevMode(true)).WithName("loader test") + + vpcCNIConfig := &v1.ConfigMap{ + Data: map[string]string{ + EnableWindowsIPAMKey: "true", + EnableWindowsPrefixDelegationKey: "false", + WarmIPTarget: "Invalid string", + MinimumIPTarget: "Invalid string", + }, + } + + warmIPTarget, minIPTarget, _, isPDEnabled := ParseWinIPTargetConfigs(log, vpcCNIConfig) + assert.False(t, isPDEnabled) + assert.Equal(t, 0, warmIPTarget) + assert.Equal(t, 0, minIPTarget) +} + +// negative values are still read in but processed accordingly when it's used in the warm pool +func TestParseWinIPTargetConfigs_PDDisabledAndNegativeConfig_ReturnsOriginal(t *testing.T) { + log := zap.New(zap.UseDevMode(true)).WithName("loader test") + + vpcCNIConfig := &v1.ConfigMap{ + Data: map[string]string{ + EnableWindowsIPAMKey: "true", + EnableWindowsPrefixDelegationKey: "false", + WarmIPTarget: strconv.Itoa(-5), + MinimumIPTarget: strconv.Itoa(-5), + }, + } + + warmIPTarget, minIPTarget, _, isPDEnabled := ParseWinIPTargetConfigs(log, vpcCNIConfig) + assert.False(t, isPDEnabled) + assert.Equal(t, -5, warmIPTarget) + assert.Equal(t, -5, minIPTarget) +} + // TestParseWinPDTargets parses prefix delegation configurations with negative values and returns the same -func TestParseWinPDTargets_Negative(t *testing.T) { +func TestParseWinIPTargetConfigs_PDEnabled_Negative(t *testing.T) { log := zap.New(zap.UseDevMode(true)).WithName("loader test") vpcCNIConfig := &v1.ConfigMap{ @@ -93,15 +131,15 @@ func TestParseWinPDTargets_Negative(t *testing.T) { WarmPrefixTarget: strconv.Itoa(0), }, } - warmIPTarget, minIPTarget, warmPrefixTarget := ParseWinPDTargets(log, vpcCNIConfig) + warmIPTarget, minIPTarget, warmPrefixTarget, _ := ParseWinIPTargetConfigs(log, vpcCNIConfig) // negative values are still read in but processed when it's used in the warm pool assert.Equal(t, -10, warmIPTarget) assert.Equal(t, -100, minIPTarget) assert.Equal(t, 0, warmPrefixTarget) } -// TestParseWinPDTargets_Invalid parses prefix delegation configurations with invalid values and returns 0s as targets -func TestParseWinPDTargets_Invalid(t *testing.T) { +// TestParseWinIPTargetConfigs_PDEnabled_Invalid parses prefix delegation configurations with invalid values and returns 0s as targets +func TestParseWinIPTargetConfigs_PDEnabled_Invalid(t *testing.T) { log := zap.New(zap.UseDevMode(true)).WithName("loader test") vpcCNIConfig := &v1.ConfigMap{ @@ -113,7 +151,7 @@ func TestParseWinPDTargets_Invalid(t *testing.T) { WarmPrefixTarget: "can't parse", }, } - warmIPTarget, minIPTarget, warmPrefixTarget := ParseWinPDTargets(log, vpcCNIConfig) + warmIPTarget, minIPTarget, warmPrefixTarget, _ := ParseWinIPTargetConfigs(log, vpcCNIConfig) assert.Equal(t, 0, warmIPTarget) assert.Equal(t, 0, minIPTarget) assert.Equal(t, 0, warmPrefixTarget) @@ -148,14 +186,14 @@ func TestLoadResourceConfigFromConfigMap(t *testing.T) { // Verify default resource configuration for resource IPv4 Address ipV4Config := resourceConfig[ResourceNameIPAddress] assert.Equal(t, ResourceNameIPAddress, ipV4Config.Name) - assert.Equal(t, IPv4DefaultWorker, ipV4Config.WorkerCount) + assert.Equal(t, IPv4DefaultWinWorkerCount, ipV4Config.WorkerCount) assert.Equal(t, map[string]bool{OSLinux: false, OSWindows: true}, ipV4Config.SupportedOS) // Verify default Warm pool configuration for IPv4 Address ipV4WPConfig := ipV4Config.WarmPoolConfig - assert.Equal(t, IPv4DefaultWPSize, ipV4WPConfig.DesiredSize) - assert.Equal(t, IPv4DefaultMaxDev, ipV4WPConfig.MaxDeviation) - assert.Equal(t, IPv4DefaultResSize, ipV4WPConfig.ReservedSize) + assert.Equal(t, IPv4DefaultWinWarmIPTarget, ipV4WPConfig.DesiredSize) + assert.Equal(t, IPv4DefaultWinMaxDev, ipV4WPConfig.MaxDeviation) + assert.Equal(t, IPv4DefaultWinResSize, ipV4WPConfig.ReservedSize) // Verify default resource configuration for prefix-deconstructed IPv4 Address prefixIPv4Config := resourceConfig[ResourceNameIPAddressFromPrefix] diff --git a/pkg/config/type.go b/pkg/config/type.go index 894b57b8..b831045b 100644 --- a/pkg/config/type.go +++ b/pkg/config/type.go @@ -155,6 +155,7 @@ type ResourceConfig struct { // WarmPoolConfig is the configuration of Warm Pool of a resource type WarmPoolConfig struct { + // TODO: Deprecate DesiredSize in favour of using WarmIPTarget since historically they served the same purpose // Number of resources to keep in warm pool per node; for prefix IP pool, this is used to check if pool is active DesiredSize int // Number of resources not to use in the warm pool diff --git a/pkg/pool/pool.go b/pkg/pool/pool.go index fcb24df7..9cc3684b 100644 --- a/pkg/pool/pool.go +++ b/pkg/pool/pool.go @@ -18,10 +18,11 @@ import ( "sync" "time" + "github.com/go-logr/logr" + "github.com/aws/amazon-vpc-resource-controller-k8s/pkg/config" "github.com/aws/amazon-vpc-resource-controller-k8s/pkg/utils" "github.com/aws/amazon-vpc-resource-controller-k8s/pkg/worker" - "github.com/go-logr/logr" ) var ( @@ -423,6 +424,20 @@ func (p *pool) ReconcilePool() *worker.WarmPoolJob { len(p.usedResources), "pending create", p.pendingCreate, "pending delete", p.pendingDelete, "cool down queue", len(p.coolDownQueue), "total resources", totalCreatedResources, "capacity", p.capacity) + p.log.V(1).Info( + "Reconciling pool", + "reSyncRequired", p.reSyncRequired, + "minIPTarget", p.warmPoolConfig.MinIPTarget, + "warmIPTarget", p.warmPoolConfig.WarmIPTarget, + "numWarmResources", numWarmResources, + "used resouces", len(p.usedResources), + "cool down queue", len(p.coolDownQueue), + "total resources", totalCreatedResources, + "pendingCreate", p.pendingCreate, + "pendingDelete", p.pendingDelete, + "capacity", p.capacity, + ) + if p.reSyncRequired { // If Pending operations are present then we can't re-sync as the upstream // and pool could change during re-sync @@ -442,9 +457,14 @@ func (p *pool) ReconcilePool() *worker.WarmPoolJob { } // Consider pending create as well so we don't create multiple subsequent create request - deviation := p.warmPoolConfig.DesiredSize - (numWarmResources + p.pendingCreate) + // deviation represents the difference between the desired number of resources and the current state + // A negative deviation means IP resources need to be deleted to reach the desired state + // A positive deviation means IP resources need to be created to reach the desired state + var deviation int if p.isPDPool { deviation = p.getPDDeviation() + } else { + deviation = p.calculateSecondaryIPDeviation() } // Need to create more resources for warm pool @@ -715,6 +735,80 @@ func (p *pool) getPDDeviation() int { return deviationPrefix * NumIPv4AddrPerPrefix } +// calculateSecondaryIPDeviation calculates the deviation required to meet the desired state for secondary IP mode +// Returns a number of IPv4 addresses by taking into account the MinIPTarget and WarmIPTarget +func (p *pool) calculateSecondaryIPDeviation() int { + numWarmResources := numResourcesFromMap(p.warmResources) + numUsedResources := len(p.usedResources) + numAssignedResources := numUsedResources + numWarmResources + p.pendingCreate + len(p.coolDownQueue) + + // warm pool is in draining state, set targets to zero + if p.warmPoolConfig.DesiredSize == 0 { + p.log.V(1).Info("DesiredSize is zero, warmPool is in draining state") + p.warmPoolConfig.WarmIPTarget = 0 + p.warmPoolConfig.MinIPTarget = 0 + p.warmPoolConfig.WarmPrefixTarget = 0 + } + + isMinIPTargetInvalid := p.warmPoolConfig.MinIPTarget < 0 + isWarmIPTargetInvalid := p.warmPoolConfig.WarmIPTarget < 0 + // Handle scenario where MinIPTarget is configured to negative integer which is invalid + if isMinIPTargetInvalid { + p.log.V(1).Info( + "MinIPTarget value is invalid negative integer, setting MinIPTarget to default", + "IPv4DefaultWinMinIPTarget", config.IPv4DefaultWinMinIPTarget, + ) + p.warmPoolConfig.MinIPTarget = config.IPv4DefaultWinMinIPTarget + } + // Handle scenario where WarmIPTarget is configured to negative integer which is invalid + if isWarmIPTargetInvalid { + p.log.V(1).Info( + "WarmIPTarget value is invalid negative integer, setting warmIPTarget to default", + "IPv4DefaultWinWarmIPTarget", config.IPv4DefaultWinWarmIPTarget, + ) + p.warmPoolConfig.WarmIPTarget = config.IPv4DefaultWinWarmIPTarget + } + + // Handle scenario where WarmIPTarget is configured to zero for secondary IP mode + // There must always be 1 warm IP to ensure that the warmpool is never empty as per workflow which relies on warmpool being populated + if p.warmPoolConfig.WarmIPTarget == 0 { + p.log.V(1).Info("WarmIPTarget zero value not supported in secondary IP mode, will override with 1") + p.warmPoolConfig.WarmIPTarget = 1 + } + + availableResources := numWarmResources + p.pendingCreate - p.pendingDelete + + // Calculate how many IPs we're short of the warm target + resourcesShort := max(p.warmPoolConfig.WarmIPTarget-availableResources, 0) + + // Adjust short based on the minimum IP target + resourcesShort = max(resourcesShort, p.warmPoolConfig.MinIPTarget-numAssignedResources) + + // Calculate how many IPs we're over the warm target + resourcesOver := max(availableResources-p.warmPoolConfig.WarmIPTarget, 0) + + // Adjust over to not go below the minimum IP target + resourcesOver = max(min(resourcesOver, numAssignedResources-p.warmPoolConfig.MinIPTarget), 0) + + // The final deviation is the difference between short and over + deviation := resourcesShort - resourcesOver + + p.log.Info( + "Finished calculating IP deviation for secondary IP pool", + "minIPTarget", p.warmPoolConfig.MinIPTarget, + "warmIPTarget", p.warmPoolConfig.WarmIPTarget, + "numWarmResources", numWarmResources, + "numUsedResources", numUsedResources, + "numAssigned", numAssignedResources, + "availableResources", availableResources, + "resourcesShort", resourcesShort, + "resourcesOver", resourcesOver, + "deviationResult", deviation, + ) + + return deviation +} + // numResourcesFromMap returns total number of resources from a map of list of resources indexed by group id func numResourcesFromMap(resourceGroups map[string][]Resource) int { count := 0 diff --git a/pkg/pool/pool_test.go b/pkg/pool/pool_test.go index ec28c525..32c1e95d 100644 --- a/pkg/pool/pool_test.go +++ b/pkg/pool/pool_test.go @@ -25,11 +25,25 @@ import ( "sigs.k8s.io/controller-runtime/pkg/log/zap" ) +type deviationCalcTestCase struct { + warmPoolConfig *config.WarmPoolConfig + warmResources map[string][]Resource + usedResources map[string]Resource + capacity int + expectedWarmIPSize int + expectedMinIPSize int + expectedDeviation int + isPDPool bool + pendingCreate int +} + var ( poolConfig = &config.WarmPoolConfig{ - DesiredSize: 2, + DesiredSize: 1, ReservedSize: 1, - MaxDeviation: 1, + MaxDeviation: config.IPv4DefaultWinMaxDev, + WarmIPTarget: config.IPv4DefaultWinWarmIPTarget, + MinIPTarget: config.IPv4DefaultWinMinIPTarget, } nodeName = "node-name" @@ -345,57 +359,84 @@ func TestPool_ReconcilePool_MaxCapacity(t *testing.T) { assert.Equal(t, &worker.WarmPoolJob{Operations: worker.OperationReconcileNotRequired}, job) } -// TestPool_ReconcilePool_NotRequired tests if the deviation form warm pool is equal to or less than the max deviation +// TestPool_ReconcilePool_NotRequired tests if the deviation from warm pool is equal to or less than the max deviation // then reconciliation is not triggered func TestPool_ReconcilePool_NotRequired(t *testing.T) { - warmPool := getMockPool(poolConfig, usedResources, map[string][]Resource{}, 7, false) - warmPool.pendingCreate = 1 + usedResourcesEmpty := map[string]Resource{} + warmPoolResourcesEmpty := map[string][]Resource{} + poolConfig.WarmIPTarget = 2 + poolConfig.MinIPTarget = 0 + warmPool := getMockPool(poolConfig, usedResourcesEmpty, warmPoolResourcesEmpty, 7, false) + warmPool.pendingCreate = 2 job := warmPool.ReconcilePool() - // deviation = 2(desired WP) - 1(actual WP + pending create) = 1, (deviation)1 > (max deviation)1 => false, + // deviation = 2(warmIPTarget) - 2(actual warmpool size + pending create) = 0 + // 0(deviation) > 0(max deviation) => false, // so no need create right now assert.Equal(t, &worker.WarmPoolJob{Operations: worker.OperationReconcileNotRequired}, job) } // TestPool_ReconcilePool tests job with operation type create is returned when the warm pool deviates form max deviation func TestPool_ReconcilePool_Create(t *testing.T) { - warmPool := getMockPool(poolConfig, usedResources, map[string][]Resource{}, 7, false) + usedResourcesEmpty := map[string]Resource{} + warmPoolResourcesEmpty := map[string][]Resource{} + poolConfig.WarmIPTarget = 2 + poolConfig.MinIPTarget = 0 + warmPool := getMockPool(poolConfig, usedResourcesEmpty, warmPoolResourcesEmpty, 7, false) job := warmPool.ReconcilePool() - // deviation = 2(desired WP) - 0(actual WP + pending create) = 0, (deviation)0 > (max deviation)1 => true, + // deviation = 2(warmIPTarget) - 0(actual warm pool size + pending create) = 0 + // 2 (deviation) >= 0 (max deviation) => true, so need to create 2 resources // create (deviation)2 resources assert.Equal(t, &worker.WarmPoolJob{Operations: worker.OperationCreate, ResourceCount: 2}, job) - assert.Equal(t, warmPool.pendingCreate, 2) + assert.Equal(t, 2, warmPool.pendingCreate) } // TestPool_ReconcilePool_Create_LimitByMaxCapacity tests when the warm pool deviates from max deviation and the deviation // is greater than the capacity of the pool, then only resources upto the max capacity are created func TestPool_ReconcilePool_Create_LimitByMaxCapacity(t *testing.T) { - warmPool := getMockPool(poolConfig, usedResources, map[string][]Resource{}, 7, false) - warmPool.pendingDelete = 4 + usedResources6 := map[string]Resource{ + res1: {GroupID: grp1, ResourceID: res1}, + res2: {GroupID: grp2, ResourceID: res2}, + res3: {GroupID: grp3, ResourceID: res3}, + res4: {GroupID: grp4, ResourceID: res4}, + res5: {GroupID: grp5, ResourceID: res5}, + res6: {GroupID: grp6, ResourceID: res6}, + } + warmPoolResourcesEmpty := map[string][]Resource{} + poolConfig.WarmIPTarget = 2 + poolConfig.MinIPTarget = 0 + warmPool := getMockPool(poolConfig, usedResources6, warmPoolResourcesEmpty, 7, false) job := warmPool.ReconcilePool() - // deviation = 2(desired WP) - 0(actual WP + pending create) = 2, (deviation)2 >= (max deviation)1 => true, so - // need to create (deviation)2 resources. But since remaining capacity is just 1, so we create 1 resource instead + // deviation = 2(warmIPTarget) - 0(actual warmpool size + pending create) = 2 + // 2 (deviation) >= 0 (max deviation) => true, so need to create 2 resources + // 6 resources are already pending creation when the ENI has a capacity of 7 + // Since the remaining capacity is just 1, so we create 1 resource instead assert.Equal(t, &worker.WarmPoolJob{Operations: worker.OperationCreate, ResourceCount: 1}, job) - assert.Equal(t, warmPool.pendingCreate, 1) + assert.Equal(t, 1, warmPool.pendingCreate) } // TestPool_ReconcilePool_Delete_NotRequired tests that if the warm pool is over the desired warm pool size but has not // exceeded the max deviation then we don't return a delete job func TestPool_ReconcilePool_Delete_NotRequired(t *testing.T) { - warmResources := make(map[string][]Resource) - warmResources[res3] = []Resource{{GroupID: res3, ResourceID: res3}} - warmResources[res4] = []Resource{{GroupID: res4, ResourceID: res4}} - warmResources[res5] = []Resource{{GroupID: res5, ResourceID: res5}} - warmPool := getMockPool(poolConfig, usedResources, warmResources, 7, false) + usedResourcesEmpty := map[string]Resource{} + poolConfig.WarmIPTarget = 2 + poolConfig.MinIPTarget = 0 + poolConfig.MaxDeviation = 1 + warmResources3 := make(map[string][]Resource) + warmResources3[res3] = []Resource{{GroupID: res3, ResourceID: res3}} + warmResources3[res4] = []Resource{{GroupID: res4, ResourceID: res4}} + warmResources3[res5] = []Resource{{GroupID: res5, ResourceID: res5}} + warmPool := getMockPool(poolConfig, usedResourcesEmpty, warmResources3, 7, false) job := warmPool.ReconcilePool() - // deviation = 2(desired WP) - 3(actual WP) = -1, (-deviation)1 > (max deviation)1 => false, so no need delete + // deviation = 2(warmIPTarget) - 3(actual warmpool size) = -1, + // -1 (deviation) > 1 (max deviation) => false, so no need delete assert.Equal(t, &worker.WarmPoolJob{Operations: worker.OperationReconcileNotRequired}, job) assert.Equal(t, warmPool.pendingDelete, 0) } @@ -403,15 +444,20 @@ func TestPool_ReconcilePool_Delete_NotRequired(t *testing.T) { // TestPool_ReconcilePool_Delete tests that if the warm pool is over the desired warm pool size and has exceed the max // deviation then we issue a return a delete job func TestPool_ReconcilePool_Delete(t *testing.T) { - warmResources := make(map[string][]Resource) - warmResources[res3] = []Resource{{GroupID: res3, ResourceID: res3}} - warmResources[res4] = []Resource{{GroupID: res4, ResourceID: res4}} - warmResources[res5] = []Resource{{GroupID: res5, ResourceID: res5}} - warmResources[res6] = []Resource{{GroupID: res6, ResourceID: res6}} - warmPool := getMockPool(poolConfig, usedResources, warmResources, 7, false) + usedResourcesEmpty := make(map[string]Resource) + poolConfig.WarmIPTarget = 2 + poolConfig.MinIPTarget = 0 + warmResources4 := map[string][]Resource{ + res1: {{GroupID: res3, ResourceID: res1}}, + res2: {{GroupID: res3, ResourceID: res2}}, + res3: {{GroupID: res3, ResourceID: res3}}, + res4: {{GroupID: res3, ResourceID: res4}}, + } + warmPool := getMockPool(poolConfig, usedResourcesEmpty, warmResources4, 7, false) job := warmPool.ReconcilePool() - // deviation = 2(desired WP) - 4(actual WP) = -2, (-deviation)2 > (max deviation)1 => true, need to delete + // deviation = 2(warmIPTarget) - 4(actual warmpool) = -2, + //|-2| (deviation) > 0(max deviation) => true, need to delete // since the warm resources is a map, there is no particular order to delete ip address from secondary ip pool, // we can't assert which two ips would get deleted here assert.Equal(t, 2, job.ResourceCount) @@ -532,12 +578,23 @@ func TestPool_Introspect(t *testing.T) { } func TestPool_SetToDraining_SecondaryIP_Pool(t *testing.T) { - warmPool := getMockPool(poolConfig, usedResources, warmPoolResources, 7, false) + usedResourcesEmpty := map[string]Resource{} + warmPoolResources2 := map[string][]Resource{ + res2: {{GroupID: res2, ResourceID: res2}}, + res3: {{GroupID: res3, ResourceID: res3}}, + } + poolConfig.WarmIPTarget = 1 + poolConfig.MinIPTarget = 0 + warmPool := getMockPool(poolConfig, usedResourcesEmpty, warmPoolResources2, 7, false) job := warmPool.SetToDraining() - // only 1 warm resource, i.e. secondary IP address - assert.Equal(t, &worker.WarmPoolJob{Operations: worker.OperationDeleted, Resources: []string{"res-3"}, ResourceCount: 1}, job) - assert.Equal(t, 1, warmPool.pendingDelete) + // 1 warm secondary IP address needs to be deleted since there is excess + assert.Equal(t, worker.OperationDeleted, job.Operations) + expectedDeletedCount := 1 + assert.Equal(t, expectedDeletedCount, job.ResourceCount) + assert.Equal(t, expectedDeletedCount, len(job.Resources)) + assert.Equal(t, expectedDeletedCount, warmPool.pendingDelete) + assert.Equal(t, 0, warmPool.pendingCreate) } func TestPool_SetToDraining_PD_Pool(t *testing.T) { @@ -555,12 +612,22 @@ func TestPool_SetToDraining_PD_Pool(t *testing.T) { } func TestPool_SetToActive_SecondaryIP_Pool(t *testing.T) { - emptyConfig := &config.WarmPoolConfig{} - warmPool := getMockPool(emptyConfig, usedResources, nil, 7, false) - newConfig := &config.WarmPoolConfig{DesiredSize: config.IPv4DefaultWPSize, MaxDeviation: config.IPv4DefaultMaxDev} + usedResourcesEmpty := map[string]Resource{} + warmPoolResources1 := map[string][]Resource{ + res3: { + {GroupID: res3, ResourceID: res3}, + }, + } + poolConfig.WarmIPTarget = 1 + poolConfig.MinIPTarget = 0 + warmPool := getMockPool(poolConfig, usedResourcesEmpty, warmPoolResources1, 7, false) + newConfig := &config.WarmPoolConfig{ + DesiredSize: 4, + WarmIPTarget: 4, + } job := warmPool.SetToActive(newConfig) - // default desired size is 3 + // 3 secondary IP addresses need to be created assert.Equal(t, &worker.WarmPoolJob{Operations: worker.OperationCreate, ResourceCount: 3}, job) assert.Equal(t, 3, warmPool.pendingCreate) } @@ -809,3 +876,255 @@ func TestNumResourcesFromMap(t *testing.T) { count = numResourcesFromMap(map[string][]Resource{grp5: {}}) assert.Equal(t, 0, count) } + +// Zero value not permitted for warm IP, must be overridden with 1 +func TestCalcSecondaryIPDeviation_PDDisabledAndZeroWarmIP_ShouldBeOverridenWith1(t *testing.T) { + poolConfig.MinIPTarget = 0 + poolConfig.WarmIPTarget = 0 + pdPool := getMockPool(poolConfig, nil, warmPoolResourcesPrefix, 7, false) + + pdPool.calculateSecondaryIPDeviation() + assert.Equal(t, 1, pdPool.warmPoolConfig.WarmIPTarget) +} + +func TestCalcSecondaryIPDeviation_InvalidMinIPTargetSet_UsesDefaultConfig(t *testing.T) { + createTestCase := func(minIPTarget int) deviationCalcTestCase { + return deviationCalcTestCase{ + warmPoolConfig: &config.WarmPoolConfig{ + DesiredSize: 1, + MinIPTarget: minIPTarget, + }, + isPDPool: false, + warmResources: map[string][]Resource{}, + usedResources: map[string]Resource{}, + capacity: 14, + expectedMinIPSize: config.IPv4DefaultWinMinIPTarget, + expectedWarmIPSize: config.IPv4DefaultWinWarmIPTarget, + expectedDeviation: 0, + } + } + testCases := map[string]deviationCalcTestCase{ + "InvalidNegativeValue1": createTestCase(-1), + "InvalidNegativeValue2": createTestCase(-10), + "InvalidNegativeValue3": createTestCase(-100), + } + + for name, tc := range testCases { + t.Run(name, func(t *testing.T) { + sIPPool := getMockPool(tc.warmPoolConfig, tc.usedResources, tc.warmResources, tc.capacity, tc.isPDPool) + sIPPool.calculateSecondaryIPDeviation() + assert.Equal(t, tc.expectedMinIPSize, sIPPool.warmPoolConfig.MinIPTarget) + }) + } +} + +func TestCalcSecondaryIPDeviation_InvalidWarmIPTargetSet_UsesDefaultConfig(t *testing.T) { + createTestCase := func(warmIPTarget int) deviationCalcTestCase { + return deviationCalcTestCase{ + warmPoolConfig: &config.WarmPoolConfig{ + DesiredSize: 1, + WarmIPTarget: warmIPTarget, + }, + isPDPool: false, + warmResources: map[string][]Resource{}, + usedResources: map[string]Resource{}, + capacity: 14, + expectedMinIPSize: config.IPv4DefaultWinMinIPTarget, + expectedWarmIPSize: config.IPv4DefaultWinWarmIPTarget, + expectedDeviation: config.IPv4DefaultWinMinIPTarget, + } + } + testCases := map[string]deviationCalcTestCase{ + "InvalidNegativeValue1": createTestCase(-1), + "InvalidNegativeValue2": createTestCase(-10), + "InvalidNegativeValue3": createTestCase(-100), + } + + for name, tc := range testCases { + t.Run(name, func(t *testing.T) { + sIPPool := getMockPool(tc.warmPoolConfig, tc.usedResources, tc.warmResources, tc.capacity, tc.isPDPool) + sIPPool.calculateSecondaryIPDeviation() + assert.Equal(t, tc.expectedWarmIPSize, sIPPool.warmPoolConfig.WarmIPTarget) + }) + } +} + +func TestCalcSecondaryIPDeviation_OnlyWarmIPTargetSetToNonZero_ExpectedDeviation(t *testing.T) { + createTestCase := func(warmIPTarget int, pendingCreate int, expectedDeviation int, warmResources map[string][]Resource, usedResources map[string]Resource) deviationCalcTestCase { + return deviationCalcTestCase{ + warmPoolConfig: &config.WarmPoolConfig{ + DesiredSize: 1, + WarmIPTarget: warmIPTarget, + MinIPTarget: 0, + }, + isPDPool: false, + pendingCreate: pendingCreate, + warmResources: warmResources, + usedResources: usedResources, + expectedWarmIPSize: warmIPTarget, + expectedMinIPSize: 0, + expectedDeviation: expectedDeviation, + } + } + + usedResourcesEmpty := map[string]Resource{} + usedResources1 := map[string]Resource{ + pod1: {GroupID: grp1, ResourceID: res1}, + } + usedResources2 := map[string]Resource{ + pod1: {GroupID: grp1, ResourceID: res1}, + pod2: {GroupID: grp2, ResourceID: res2}, + } + warmResourcesEmpty := make(map[string][]Resource) + warmResources1 := make(map[string][]Resource) + warmResources1[res1] = []Resource{{GroupID: res1, ResourceID: res1}} + warmResources2 := make(map[string][]Resource) + warmResources2[res1] = []Resource{{GroupID: res1, ResourceID: res1}, {GroupID: res2, ResourceID: res2}} + + testCases := map[string]deviationCalcTestCase{ + "ResourcesNeedToBeDeleted1": createTestCase(1, 0, -1, warmResources2, usedResourcesEmpty), + "NoResourcesInUseNoWarmNoPendingCreate": createTestCase(3, 0, 3, warmResourcesEmpty, usedResourcesEmpty), + "SomePendingResourcesScenario1": createTestCase(2, 2, 0, warmResourcesEmpty, usedResourcesEmpty), + "SomePendingResourcesScenario2": createTestCase(2, 1, 1, warmResourcesEmpty, usedResourcesEmpty), + "SomeWarmResourcesScenario1": createTestCase(2, 0, 1, warmResources1, usedResourcesEmpty), + "SomeWarmResourcesScenario2": createTestCase(2, 0, 0, warmResources2, usedResourcesEmpty), + "SomeUsedResourcesScenario1": createTestCase(2, 0, 2, warmResourcesEmpty, usedResources1), + "SomeUsedResourcesScenario2": createTestCase(2, 0, 2, warmResourcesEmpty, usedResources2), + "ComplexScenario1": createTestCase(3, 1, 0, warmResources2, usedResources1), + "ComplexScenario2": createTestCase(3, 2, 0, warmResources1, usedResources2), + "ComplexScenario3": createTestCase(3, 1, 0, warmResources2, usedResources2), + } + + for name, tc := range testCases { + t.Run(name, func(t *testing.T) { + sIPPool := getMockPool(tc.warmPoolConfig, tc.usedResources, tc.warmResources, tc.capacity, tc.isPDPool) + sIPPool.pendingCreate = tc.pendingCreate + deviation := sIPPool.calculateSecondaryIPDeviation() + assert.Equal(t, tc.expectedWarmIPSize, sIPPool.warmPoolConfig.WarmIPTarget) + assert.Equal(t, tc.expectedMinIPSize, sIPPool.warmPoolConfig.MinIPTarget) + assert.Equal(t, tc.expectedDeviation, deviation) + }) + } +} + +func TestCalcSecondaryIPDeviation_OnlyMinIPTargetSetToNonZero_ExpectedDeviation(t *testing.T) { + createTestCase := func(minIPTarget int, pendingCreate int, expectedDeviation int, warmResources map[string][]Resource, usedResources map[string]Resource) deviationCalcTestCase { + return deviationCalcTestCase{ + warmPoolConfig: &config.WarmPoolConfig{ + DesiredSize: 1, + WarmIPTarget: 1, + MinIPTarget: minIPTarget, + }, + isPDPool: false, + pendingCreate: pendingCreate, + warmResources: warmResources, + usedResources: usedResources, + expectedMinIPSize: minIPTarget, + expectedWarmIPSize: 1, + expectedDeviation: expectedDeviation, + } + } + + usedResourcesEmpty := map[string]Resource{} + usedResources1 := map[string]Resource{ + pod1: {GroupID: grp1, ResourceID: res1}, + } + usedResources2 := map[string]Resource{ + pod1: {GroupID: grp1, ResourceID: res1}, + pod2: {GroupID: grp2, ResourceID: res2}, + } + warmResourcesEmpty := make(map[string][]Resource) + warmResources1 := make(map[string][]Resource) + warmResources1[res1] = []Resource{{GroupID: res1, ResourceID: res1}} + warmResources2 := make(map[string][]Resource) + warmResources2[res1] = []Resource{{GroupID: res1, ResourceID: res1}, {GroupID: res2, ResourceID: res2}} + + testCases := map[string]deviationCalcTestCase{ + "ResourcesNeedToBeDeleted": createTestCase(0, 0, -1, warmResources2, usedResourcesEmpty), + "NoResourcesInUseNoWarmNoPendingCreate": createTestCase(3, 0, 3, warmResourcesEmpty, usedResourcesEmpty), + "SomePendingResourcesScenario1": createTestCase(2, 2, 0, warmResourcesEmpty, usedResourcesEmpty), + "SomePendingResourcesScenario2": createTestCase(2, 1, 1, warmResourcesEmpty, usedResourcesEmpty), + "SomeWarmResourcesScenario1": createTestCase(2, 0, 1, warmResources1, usedResourcesEmpty), + "SomeWarmResourcesScenario2": createTestCase(2, 0, 0, warmResources2, usedResourcesEmpty), + "SomeUsedResourcesScenario1": createTestCase(2, 0, 1, warmResourcesEmpty, usedResources1), + "SomeUsedResourcesScenario2": createTestCase(2, 0, 1, warmResourcesEmpty, usedResources2), + "ComplexScenario1": createTestCase(5, 1, 2, warmResources1, usedResources1), + "ComplexScenario2": createTestCase(5, 3, -2, warmResources2, usedResources2), + "ComplexScenario3": createTestCase(5, 2, 0, warmResources2, usedResources1), + } + + for name, tc := range testCases { + t.Run(name, func(t *testing.T) { + sIPPool := getMockPool(tc.warmPoolConfig, tc.usedResources, tc.warmResources, tc.capacity, tc.isPDPool) + sIPPool.pendingCreate = tc.pendingCreate + deviation := sIPPool.calculateSecondaryIPDeviation() + assert.Equal(t, tc.expectedWarmIPSize, sIPPool.warmPoolConfig.WarmIPTarget) + assert.Equal(t, tc.expectedMinIPSize, sIPPool.warmPoolConfig.MinIPTarget) + assert.Equal(t, tc.expectedDeviation, deviation) + }) + } +} + +func TestCalcSecondaryIPDeviation_MinIPTargetAndWarmIPTargetSet_ExpectedDeviation(t *testing.T) { + createTestCase := func(minIPTarget int, warmIPTarget int, pendingCreate int, expectedDeviation int, warmResources map[string][]Resource, usedResources map[string]Resource) deviationCalcTestCase { + return deviationCalcTestCase{ + warmPoolConfig: &config.WarmPoolConfig{ + DesiredSize: 1, + WarmIPTarget: warmIPTarget, + MinIPTarget: minIPTarget, + }, + isPDPool: false, + pendingCreate: pendingCreate, + warmResources: warmResources, + usedResources: usedResources, + expectedMinIPSize: minIPTarget, + expectedWarmIPSize: warmIPTarget, + expectedDeviation: expectedDeviation, + } + } + + usedResourcesEmpty := map[string]Resource{} + usedResources1 := map[string]Resource{ + pod1: {GroupID: grp1, ResourceID: res1}, + } + usedResources2 := map[string]Resource{ + pod1: {GroupID: grp1, ResourceID: res1}, + pod2: {GroupID: grp2, ResourceID: res2}, + } + warmResourcesEmpty := make(map[string][]Resource) + warmResources1 := make(map[string][]Resource) + warmResources1[res1] = []Resource{{GroupID: res1, ResourceID: res1}} + warmResources2 := make(map[string][]Resource) + warmResources2[res1] = []Resource{{GroupID: res1, ResourceID: res1}, {GroupID: res2, ResourceID: res2}} + warmResources5 := make(map[string][]Resource) + warmResources5[res1] = []Resource{ + {GroupID: res1, ResourceID: res1}, + {GroupID: res2, ResourceID: res2}, + {GroupID: res3, ResourceID: res3}, + {GroupID: res4, ResourceID: res4}, + {GroupID: res5, ResourceID: res5}, + } + + testCases := map[string]deviationCalcTestCase{ + "Scenario1": createTestCase(1, 1, 0, 1, warmResourcesEmpty, usedResourcesEmpty), + "Scenario2": createTestCase(5, 2, 0, 3, warmResources2, usedResourcesEmpty), + "Scenario3": createTestCase(5, 2, 0, 0, warmResources5, usedResourcesEmpty), + "Scenario4": createTestCase(0, 1, 0, -4, warmResources5, usedResourcesEmpty), + "Scenario5": createTestCase(4, 5, 2, 1, warmResources2, usedResources1), + "Scenario6": createTestCase(10, 5, 1, 7, warmResources1, usedResources1), + "Scenario7": createTestCase(12, 12, 5, 5, warmResources2, usedResources2), + "Scenario8": createTestCase(0, 1, 0, 0, warmResources1, usedResources1), + "Scenario9": createTestCase(2, 1, 0, -1, warmResources2, usedResources1), + } + + for name, tc := range testCases { + t.Run(name, func(t *testing.T) { + sIPPool := getMockPool(tc.warmPoolConfig, tc.usedResources, tc.warmResources, tc.capacity, tc.isPDPool) + sIPPool.pendingCreate = tc.pendingCreate + deviation := sIPPool.calculateSecondaryIPDeviation() + assert.Equal(t, tc.expectedWarmIPSize, sIPPool.warmPoolConfig.WarmIPTarget) + assert.Equal(t, tc.expectedMinIPSize, sIPPool.warmPoolConfig.MinIPTarget) + assert.Equal(t, tc.expectedDeviation, deviation) + }) + } +} diff --git a/pkg/provider/branch/provider.go b/pkg/provider/branch/provider.go index f9eb2409..581c1c4a 100644 --- a/pkg/provider/branch/provider.go +++ b/pkg/provider/branch/provider.go @@ -22,6 +22,8 @@ import ( "sync" "time" + "github.com/google/uuid" + "github.com/aws/amazon-vpc-resource-controller-k8s/pkg/api" "github.com/aws/amazon-vpc-resource-controller-k8s/pkg/aws/ec2" "github.com/aws/amazon-vpc-resource-controller-k8s/pkg/aws/vpc" @@ -33,7 +35,6 @@ import ( "github.com/aws/amazon-vpc-resource-controller-k8s/pkg/provider/branch/trunk" "github.com/aws/amazon-vpc-resource-controller-k8s/pkg/utils" "github.com/aws/amazon-vpc-resource-controller-k8s/pkg/worker" - "github.com/google/uuid" "github.com/aws/aws-sdk-go/aws/awserr" "github.com/go-logr/logr" @@ -274,7 +275,8 @@ func (b *branchENIProvider) ReconcileNode(nodeName string) bool { log := b.log.WithValues("node", nodeName) if !isPresent { // return true to set the node next clean up asap since we don't know why trunk is missing - log.Info("no trunk ENI is pointing to the given node", "nodeName", nodeName) + // TODO: Uncomment after dev testing + // log.Info("no trunk ENI is pointing to the given node", "nodeName", nodeName) return true } podList, err := b.apiWrapper.PodAPI.ListPods(nodeName) diff --git a/pkg/provider/ip/provider.go b/pkg/provider/ip/provider.go index f3bba704..ca5c46f2 100644 --- a/pkg/provider/ip/provider.go +++ b/pkg/provider/ip/provider.go @@ -153,9 +153,11 @@ func (p *ipv4Provider) InitResource(instance ec2.EC2Instance) error { // Expected node capacity based on instance type in secondary IP mode nodeCapacity := getCapacity(instance.Type(), instance.Os()) + isPDEnabled := p.conditions.IsWindowsPrefixDelegationEnabled() + p.config = provider.GetWinWarmPoolConfig(p.log, p.apiWrapper, isPDEnabled) + // Set warm pool config to empty config if PD is enabled secondaryIPWPConfig := p.config - isPDEnabled := p.conditions.IsWindowsPrefixDelegationEnabled() if isPDEnabled { secondaryIPWPConfig = &config.WarmPoolConfig{} } else { @@ -239,6 +241,8 @@ func (p *ipv4Provider) UpdateResourceCapacity(instance ec2.EC2Instance) error { resourceProviderAndPool.isPrevPDEnabled = false + p.config = provider.GetWinWarmPoolConfig(p.log, p.apiWrapper, isCurrPDEnabled) + // Set the secondary IP provider pool state to active job := resourceProviderAndPool.resourcePool.SetToActive(p.config) if job.Operations != worker.OperationReconcileNotRequired { diff --git a/pkg/provider/ip/provider_test.go b/pkg/provider/ip/provider_test.go index efe77261..be423b87 100644 --- a/pkg/provider/ip/provider_test.go +++ b/pkg/provider/ip/provider_test.go @@ -16,8 +16,11 @@ package ip import ( "fmt" "reflect" + "strconv" "testing" + v1 "k8s.io/api/core/v1" + mock_ec2 "github.com/aws/amazon-vpc-resource-controller-k8s/mocks/amazon-vcp-resource-controller-k8s/pkg/aws/ec2" mock_condition "github.com/aws/amazon-vpc-resource-controller-k8s/mocks/amazon-vcp-resource-controller-k8s/pkg/condition" mock_k8s "github.com/aws/amazon-vpc-resource-controller-k8s/mocks/amazon-vcp-resource-controller-k8s/pkg/k8s" @@ -47,9 +50,11 @@ var ( nodeCapacity = 14 ipV4WarmPoolConfig = config.WarmPoolConfig{ - DesiredSize: config.IPv4DefaultWPSize, - MaxDeviation: config.IPv4DefaultMaxDev, - ReservedSize: config.IPv4DefaultResSize, + WarmIPTarget: config.IPv4DefaultWinWarmIPTarget, + MinIPTarget: config.IPv4DefaultWinMinIPTarget, + DesiredSize: config.IPv4DefaultWinWarmIPTarget, + MaxDeviation: config.IPv4DefaultWinMaxDev, + ReservedSize: config.IPv4DefaultWinResSize, } ) @@ -327,17 +332,28 @@ func TestIPv4Provider_UpdateResourceCapacity_FromFromPDToIP(t *testing.T) { mockConditions := mock_condition.NewMockConditions(ctrl) mockWorker := mock_worker.NewMockWorker(ctrl) ipV4WarmPoolConfig := config.WarmPoolConfig{ - DesiredSize: config.IPv4DefaultWPSize, - MaxDeviation: config.IPv4DefaultMaxDev, - ReservedSize: config.IPv4DefaultResSize, + WarmIPTarget: config.IPv4DefaultWinWarmIPTarget, + MinIPTarget: config.IPv4DefaultWinMinIPTarget, + DesiredSize: config.IPv4DefaultWinWarmIPTarget, + MaxDeviation: config.IPv4DefaultWinMaxDev, + ReservedSize: config.IPv4DefaultWinResSize, } ipv4Provider := ipv4Provider{apiWrapper: api.Wrapper{K8sAPI: mockK8sWrapper}, workerPool: mockWorker, config: &ipV4WarmPoolConfig, instanceProviderAndPool: map[string]*ResourceProviderAndPool{}, log: zap.New(zap.UseDevMode(true)).WithName("ip provider"), conditions: mockConditions} + expectedVpcCNIConfig := &v1.ConfigMap{ + Data: map[string]string{ + config.EnableWindowsIPAMKey: "true", + config.EnableWindowsPrefixDelegationKey: "false", + config.WarmIPTarget: strconv.Itoa(config.IPv4DefaultWinWarmIPTarget), + config.MinimumIPTarget: strconv.Itoa(config.IPv4DefaultWinMinIPTarget), + }, + } mockPool := mock_pool.NewMockPool(ctrl) mockManager := mock_eni.NewMockENIManager(ctrl) ipv4Provider.putInstanceProviderAndPool(nodeName, mockPool, mockManager, nodeCapacity, true) mockConditions.EXPECT().IsWindowsPrefixDelegationEnabled().Return(false) + mockK8sWrapper.EXPECT().GetConfigMap(config.VpcCniConfigMapName, config.KubeSystemNamespace).Return(expectedVpcCNIConfig, nil) job := &worker.WarmPoolJob{Operations: worker.OperationCreate} mockPool.EXPECT().SetToActive(&ipV4WarmPoolConfig).Return(job) @@ -391,11 +407,20 @@ func TestIPv4Provider_UpdateResourceCapacity_FromFromIPToPD_NonNitro(t *testing. mockWorker := mock_worker.NewMockWorker(ctrl) ipv4Provider := ipv4Provider{apiWrapper: api.Wrapper{K8sAPI: mockK8sWrapper}, workerPool: mockWorker, config: &ipV4WarmPoolConfig, instanceProviderAndPool: map[string]*ResourceProviderAndPool{}, log: zap.New(zap.UseDevMode(true)).WithName("ip provider"), conditions: mockConditions} + expectedVpcCNIConfig := &v1.ConfigMap{ + Data: map[string]string{ + config.EnableWindowsIPAMKey: "true", + config.EnableWindowsPrefixDelegationKey: "false", + config.WarmIPTarget: strconv.Itoa(config.IPv4DefaultWinWarmIPTarget), + config.MinimumIPTarget: strconv.Itoa(config.IPv4DefaultWinMinIPTarget), + }, + } mockPool := mock_pool.NewMockPool(ctrl) mockManager := mock_eni.NewMockENIManager(ctrl) ipv4Provider.putInstanceProviderAndPool(nodeName, mockPool, mockManager, nodeCapacity, false) mockConditions.EXPECT().IsWindowsPrefixDelegationEnabled().Return(true) + mockK8sWrapper.EXPECT().GetConfigMap(config.VpcCniConfigMapName, config.KubeSystemNamespace).Return(expectedVpcCNIConfig, nil) job := &worker.WarmPoolJob{Operations: worker.OperationCreate} mockPool.EXPECT().SetToActive(&ipV4WarmPoolConfig).Return(job) @@ -442,11 +467,20 @@ func TestIPv4Provider_UpdateResourceCapacity_FromIPToIP(t *testing.T) { mockWorker := mock_worker.NewMockWorker(ctrl) ipv4Provider := ipv4Provider{apiWrapper: api.Wrapper{K8sAPI: mockK8sWrapper}, workerPool: mockWorker, config: &ipV4WarmPoolConfig, instanceProviderAndPool: map[string]*ResourceProviderAndPool{}, log: zap.New(zap.UseDevMode(true)).WithName("ip provider"), conditions: mockConditions} + expectedVpcCNIConfig := &v1.ConfigMap{ + Data: map[string]string{ + config.EnableWindowsIPAMKey: "true", + config.EnableWindowsPrefixDelegationKey: "false", + config.WarmIPTarget: strconv.Itoa(config.IPv4DefaultWinWarmIPTarget), + config.MinimumIPTarget: strconv.Itoa(config.IPv4DefaultWinMinIPTarget), + }, + } mockPool := mock_pool.NewMockPool(ctrl) mockManager := mock_eni.NewMockENIManager(ctrl) ipv4Provider.putInstanceProviderAndPool(nodeName, mockPool, mockManager, nodeCapacity, false) mockConditions.EXPECT().IsWindowsPrefixDelegationEnabled().Return(false) + mockK8sWrapper.EXPECT().GetConfigMap(config.VpcCniConfigMapName, config.KubeSystemNamespace).Return(expectedVpcCNIConfig, nil) job := &worker.WarmPoolJob{Operations: worker.OperationCreate} mockPool.EXPECT().SetToActive(&ipV4WarmPoolConfig).Return(job) diff --git a/pkg/provider/prefix/provider.go b/pkg/provider/prefix/provider.go index 3cb22613..5c86383f 100644 --- a/pkg/provider/prefix/provider.go +++ b/pkg/provider/prefix/provider.go @@ -155,11 +155,11 @@ func (p *ipv4PrefixProvider) InitResource(instance ec2.EC2Instance) error { // Expected node capacity based on instance type in PD mode nodeCapacity := getCapacity(instance.Type(), instance.Os()) * pool.NumIPv4AddrPerPrefix - p.config = p.getPDWarmPoolConfig() + isPDEnabled := p.conditions.IsWindowsPrefixDelegationEnabled() + p.config = provider.GetWinWarmPoolConfig(p.log, p.apiWrapper, isPDEnabled) // Set warm pool config to empty if PD is not enabled prefixIPWPConfig := p.config - isPDEnabled := p.conditions.IsWindowsPrefixDelegationEnabled() if !isPDEnabled { prefixIPWPConfig = &config.WarmPoolConfig{} } else { @@ -234,7 +234,7 @@ func (p *ipv4PrefixProvider) UpdateResourceCapacity(instance ec2.EC2Instance) er resourceProviderAndPool.isPrevPDEnabled = true - warmPoolConfig := p.getPDWarmPoolConfig() + warmPoolConfig := provider.GetWinWarmPoolConfig(p.log, p.apiWrapper, isCurrPDEnabled) // Set the secondary IP provider pool state to active job := resourceProviderAndPool.resourcePool.SetToActive(warmPoolConfig) @@ -508,19 +508,6 @@ func getCapacity(instanceType string, instanceOs string) int { return capacity } -// Retrieve dynamic configuration for prefix delegation from config map, else use default warm pool config -func (p *ipv4PrefixProvider) getPDWarmPoolConfig() *config.WarmPoolConfig { - var resourceConfig map[string]config.ResourceConfig - vpcCniConfigMap, err := p.apiWrapper.K8sAPI.GetConfigMap(config.VpcCniConfigMapName, config.KubeSystemNamespace) - if err == nil { - resourceConfig = config.LoadResourceConfigFromConfigMap(p.log, vpcCniConfigMap) - } else { - p.log.Error(err, "failed to read from config map, will use default resource config") - resourceConfig = config.LoadResourceConfig() - } - return resourceConfig[config.ResourceNameIPAddressFromPrefix].WarmPoolConfig -} - func (p *ipv4PrefixProvider) check() healthz.Checker { p.log.Info("IPv4 prefix provider's healthz subpath was added") return func(req *http.Request) error { diff --git a/pkg/provider/prefix/provider_test.go b/pkg/provider/prefix/provider_test.go index 3daea497..d30b95d0 100644 --- a/pkg/provider/prefix/provider_test.go +++ b/pkg/provider/prefix/provider_test.go @@ -19,6 +19,8 @@ import ( "strconv" "testing" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + mock_ec2 "github.com/aws/amazon-vpc-resource-controller-k8s/mocks/amazon-vcp-resource-controller-k8s/pkg/aws/ec2" mock_condition "github.com/aws/amazon-vpc-resource-controller-k8s/mocks/amazon-vcp-resource-controller-k8s/pkg/condition" mock_k8s "github.com/aws/amazon-vpc-resource-controller-k8s/mocks/amazon-vcp-resource-controller-k8s/pkg/k8s" @@ -31,7 +33,6 @@ import ( "github.com/aws/amazon-vpc-resource-controller-k8s/pkg/provider/ip/eni" "github.com/aws/amazon-vpc-resource-controller-k8s/pkg/utils" "github.com/aws/amazon-vpc-resource-controller-k8s/pkg/worker" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "github.com/golang/mock/gomock" "github.com/stretchr/testify/assert" @@ -543,24 +544,6 @@ func getMockIPv4PrefixProvider() ipv4PrefixProvider { log: zap.New(zap.UseDevMode(true)).WithName("prefix provider")} } -func TestGetPDWarmPoolConfig(t *testing.T) { - ctrl := gomock.NewController(t) - defer ctrl.Finish() - - mockK8sWrapper := mock_k8s.NewMockK8sWrapper(ctrl) - mockConditions := mock_condition.NewMockConditions(ctrl) - prefixProvider := ipv4PrefixProvider{apiWrapper: api.Wrapper{K8sAPI: mockK8sWrapper}, - instanceProviderAndPool: map[string]*ResourceProviderAndPool{}, - log: zap.New(zap.UseDevMode(true)).WithName("prefix provider"), conditions: mockConditions} - - for _, c := range []*v1.ConfigMap{vpcCNIConfig, vpcCNIConfigWindows} { - mockK8sWrapper.EXPECT().GetConfigMap(config.VpcCniConfigMapName, config.KubeSystemNamespace).Return(c, nil) - - config := prefixProvider.getPDWarmPoolConfig() - assert.Equal(t, pdWarmPoolConfig, config) - } -} - // TestIsInstanceSupported tests that if the instance type is nitro, return true func TestIsInstanceSupported(t *testing.T) { ctrl := gomock.NewController(t) diff --git a/pkg/provider/provider.go b/pkg/provider/provider.go index c450358f..44a3ecfc 100644 --- a/pkg/provider/provider.go +++ b/pkg/provider/provider.go @@ -14,10 +14,14 @@ package provider import ( - "github.com/aws/amazon-vpc-resource-controller-k8s/pkg/aws/ec2" - "github.com/aws/amazon-vpc-resource-controller-k8s/pkg/pool" + "github.com/go-logr/logr" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/healthz" + + "github.com/aws/amazon-vpc-resource-controller-k8s/pkg/api" + "github.com/aws/amazon-vpc-resource-controller-k8s/pkg/aws/ec2" + "github.com/aws/amazon-vpc-resource-controller-k8s/pkg/config" + "github.com/aws/amazon-vpc-resource-controller-k8s/pkg/pool" ) // ResourceProvider is the provider interface that each resource managed by the controller has to implement @@ -46,3 +50,21 @@ type ResourceProvider interface { IntrospectSummary() interface{} ReconcileNode(nodeName string) bool } + +// GetWinWarmPoolConfig retrieves Windows warmpool configuration from ConfigMap, falls back to using default values on failure +func GetWinWarmPoolConfig(log logr.Logger, w api.Wrapper, isPDEnabled bool) *config.WarmPoolConfig { + var resourceConfig map[string]config.ResourceConfig + vpcCniConfigMap, err := w.K8sAPI.GetConfigMap(config.VpcCniConfigMapName, config.KubeSystemNamespace) + if err == nil { + resourceConfig = config.LoadResourceConfigFromConfigMap(log, vpcCniConfigMap) + } else { + log.Error(err, "failed to read from config map, will use default resource config") + resourceConfig = config.LoadResourceConfig() + } + + if isPDEnabled { + return resourceConfig[config.ResourceNameIPAddressFromPrefix].WarmPoolConfig + } else { + return resourceConfig[config.ResourceNameIPAddress].WarmPoolConfig + } +} diff --git a/pkg/provider/provider_test.go b/pkg/provider/provider_test.go new file mode 100644 index 00000000..ef3331ba --- /dev/null +++ b/pkg/provider/provider_test.go @@ -0,0 +1,112 @@ +// Copyright Amazon.com Inc. or its affiliates. All Rights Reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"). You may +// not use this file except in compliance with the License. A copy of the +// License is located at +// +// http://aws.amazon.com/apache2.0/ +// +// or in the "license" file accompanying this file. This file is distributed +// on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either +// express or implied. See the License for the specific language governing +// permissions and limitations under the License. + +package provider + +import ( + "fmt" + "strconv" + "testing" + + "github.com/golang/mock/gomock" + "github.com/stretchr/testify/assert" + v1 "k8s.io/api/core/v1" + "sigs.k8s.io/controller-runtime/pkg/log/zap" + + mock_k8s "github.com/aws/amazon-vpc-resource-controller-k8s/mocks/amazon-vcp-resource-controller-k8s/pkg/k8s" + "github.com/aws/amazon-vpc-resource-controller-k8s/pkg/api" + "github.com/aws/amazon-vpc-resource-controller-k8s/pkg/config" +) + +func TestGetWinWarmPoolConfig_PDDisabledAndAPICallSuccess_ReturnsConfig(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + log := zap.New(zap.UseDevMode(true)).WithName("provider test") + + configMapToReturn := &v1.ConfigMap{ + Data: map[string]string{ + config.EnableWindowsIPAMKey: "true", + config.EnableWindowsPrefixDelegationKey: "true", + config.WinWarmIPTarget: strconv.Itoa(config.IPv4DefaultWinWarmIPTarget), + config.WinMinimumIPTarget: strconv.Itoa(config.IPv4DefaultWinMinIPTarget), + }, + } + expectedWarmPoolConfig := &config.WarmPoolConfig{ + WarmIPTarget: config.IPv4DefaultWinWarmIPTarget, + MinIPTarget: config.IPv4DefaultWinMinIPTarget, + DesiredSize: config.IPv4DefaultWinWarmIPTarget, + } + + mockK8sWrapper := mock_k8s.NewMockK8sWrapper(ctrl) + mockK8sWrapper.EXPECT().GetConfigMap(config.VpcCniConfigMapName, config.KubeSystemNamespace).Return(configMapToReturn, nil) + apiWrapperMock := api.Wrapper{K8sAPI: mockK8sWrapper} + + actualWarmPoolConfig := GetWinWarmPoolConfig(log, apiWrapperMock, false) + assert.Equal(t, expectedWarmPoolConfig, actualWarmPoolConfig) +} + +func TestGetWinWarmPoolConfig_PDEnabledAndAPICallSuccess_ReturnsConfig(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + log := zap.New(zap.UseDevMode(true)).WithName("provider test") + + configMapToReturn := &v1.ConfigMap{ + Data: map[string]string{ + config.EnableWindowsIPAMKey: "true", + config.EnableWindowsPrefixDelegationKey: "true", + config.WinWarmIPTarget: strconv.Itoa(config.IPv4PDDefaultWarmIPTargetSize), + config.WinWarmPrefixTarget: strconv.Itoa(config.IPv4PDDefaultWarmPrefixTargetSize), + config.WinMinimumIPTarget: strconv.Itoa(config.IPv4PDDefaultMinIPTargetSize), + }, + } + expectedWarmPoolConfig := &config.WarmPoolConfig{ + WarmIPTarget: config.IPv4PDDefaultWarmIPTargetSize, + WarmPrefixTarget: config.IPv4PDDefaultWarmPrefixTargetSize, + MinIPTarget: config.IPv4PDDefaultMinIPTargetSize, + DesiredSize: config.IPv4PDDefaultWarmIPTargetSize, + } + + mockK8sWrapper := mock_k8s.NewMockK8sWrapper(ctrl) + mockK8sWrapper.EXPECT().GetConfigMap(config.VpcCniConfigMapName, config.KubeSystemNamespace).Return(configMapToReturn, nil) + apiWrapperMock := api.Wrapper{K8sAPI: mockK8sWrapper} + + actualWarmPoolConfig := GetWinWarmPoolConfig(log, apiWrapperMock, true) + assert.Equal(t, expectedWarmPoolConfig, actualWarmPoolConfig) +} + +func TestGetWinWarmPoolConfig_PDDisabledAndAPICallFailure_ReturnsDefaultConfig(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + log := zap.New(zap.UseDevMode(true)).WithName("provider test") + + var configMapToReturn *v1.ConfigMap = nil + errorToReturn := fmt.Errorf("Some error occurred while fetching config map") + expectedWarmPoolConfig := &config.WarmPoolConfig{ + WarmIPTarget: config.IPv4DefaultWinWarmIPTarget, + MinIPTarget: config.IPv4DefaultWinMinIPTarget, + DesiredSize: config.IPv4DefaultWinWarmIPTarget, + } + + mockK8sWrapper := mock_k8s.NewMockK8sWrapper(ctrl) + mockK8sWrapper.EXPECT().GetConfigMap( + config.VpcCniConfigMapName, + config.KubeSystemNamespace, + ).Return( + configMapToReturn, + errorToReturn, + ) + apiWrapperMock := api.Wrapper{K8sAPI: mockK8sWrapper} + + actualWarmPoolConfig := GetWinWarmPoolConfig(log, apiWrapperMock, false) + assert.Equal(t, expectedWarmPoolConfig, actualWarmPoolConfig) +}