diff --git a/controllers/core/configmap_controller.go b/controllers/core/configmap_controller.go index e56e3847..1f5b251f 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 + // Check if Windows IP target configurations in ConfigMap have changed + var isWinIPConfigsUpdated bool + + 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 + isWinIPConfigsUpdated = true + } + if isPDEnabled && winPDWarmPrefixTargetUpdated { r.curWinPDWarmPrefixTarget = warmPrefixTarget - logger.Info("updated PD configs from configmap", config.WarmIPTarget, r.curWinPDWarmIPTarget, - config.MinimumIPTarget, r.curWinPDMinIPTarget, config.WarmPrefixTarget, r.curWinPDWarmPrefixTarget) - - isPDConfigUpdated = true + 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..03123d73 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, @@ -127,7 +141,7 @@ func Test_Reconcile_ConfigMap_PD_Disabled_If_IPAM_Disabled(t *testing.T) { ctrl := gomock.NewController(t) defer ctrl.Finish() - mock := NewConfigMapMock(ctrl, mockConfigMapPD) + mock := NewConfigMapMock(ctrl, mockConfigMap) mock.MockCondition.EXPECT().IsWindowsIPAMEnabled().Return(false) mock.MockCondition.EXPECT().IsWindowsPrefixDelegationEnabled().Return(false) mock.MockK8sAPI.EXPECT().GetConfigMap(config.VpcCniConfigMapName, config.KubeSystemNamespace).Return(createCoolDownMockCM("30"), nil).AnyTimes() diff --git a/docs/windows/secondary_ip_mode_config_options.md b/docs/windows/secondary_ip_mode_config_options.md new file mode 100644 index 00000000..2f552d76 --- /dev/null +++ b/docs/windows/secondary_ip_mode_config_options.md @@ -0,0 +1,63 @@ +# Configuration options when using secondary IP addresses Windows + +We provide multiple configuration options that allow you to fine-tune the IP address allocation behavior on Windows +nodes using the secondary IP address mode. These configuration options can be set in the `amazon-vpc-cni` ConfigMap in +the `kube-system` namespace. + +- `windows-warm-ip-target` → The total number of IP addresses that should be allocated to each Windows node in excess of + the current need at any given time. The excess IPs can be used by newly launched pods, which aids in faster pod + startup times since there is no wait time for additional IP addresses to be allocated. The VPC Resource Controller + will attempt to ensure that this excess desired threshold is always met. + + Defaults to 3 if unspecified or invalid. Must be greater than or equal to 1. + + For example, if no pods were running on a given Windows node, and if you set `windows-warm-ip-target` to 5, the VPC + Resource Controller will aim to ensure that each Windows node always has at least 5 IP addresses in excess, ready for + use, allocated to its ENI. If 2 pods are scheduled on the node, the controller will allocate 2 additional IP addresses + to the ENI, maintaining the 5 warm IP address target. + +- `windows-minimum-ip-target` → Defaults to 3 if unspecified or invalid. The minimum number of IP addresses, both in use + by running pods and available as warm IPs, that should be allocated to each Windows node at any given time. The + controller will attempt to ensure that this minimum threshold is always met. + + Defaults to 3 if unspecified or invalid. Must be greater than or equal to 0. + + For example, if no pods were running on a given Windows node, and if you set `windows-minimum-ip-target` to 10, the + VPC Resource Controller will aim to ensure that the total number of IP addresses on the Windows node should be at + least 10. Therefore, before pods are scheduled, there should be at least 10 IP addresses available. If 5 pods are + scheduled on a given node, they will consume 5 of the 10 available IPs. The VPC Resource Controller will keep 5 the + remaining available IPs available in addition to the 5 already in use to meet the target of 10. + +### Considerations while using the above configuration options + +- These configuration options only apply when the VPC Resource Controller is operating in the secondary IP mode. They do + not affect the prefix delegation mode. More explicitly, if `enable-windows-prefix-delegation` is set to false, or is + not specified, then the VPC Resource Controller operates in secondary IP mode. +- Setting either `windows-warm-ip-target` or `windows-minimum-ip-target` to a negative value will result in the + respective default value being used. +- If the values of `windows-warm-ip-target` or `windows-minimum-ip-target` are set such that the maximum node IP + capacity would be exceeded, the controller will limit the allocation to the maximum capacity possible. +- The `warm-prefix-target` configuration option will be ignored when using the secondary IP mode, as it only applies to + the prefix delegation mode. +- If `windows-warm-ip-target` is set to 0, the system will implicitly set `windows-warm-ip-target` to 1. This is + because on-demand IP allocation whereby an IP is allocated on the Windows node as the pods are scheduled is currently + not supported. Implicitly Setting `windows-warm-ip-target` to 1 ensures the minimum acceptable non-zero value is set + since the `windows-warm-ip-target` should always be at least 1. +- The configuration options `warm-ip-target` and `minimum-ip-target` are deprecated in favor of the new + options `windows-warm-ip-target` and `windows-minimum-ip-target`. + +### Examples + +| `windows-warm-ip-target` | `windows-minimum-ip-target` | Running Pods | Total Allocated IPs | Warm IPs | +|--------------------------|-----------------------------|--------------|---------------------|----------| +| 1 | 0 | 0 | 1 | 1 | +| 1 | 0 | 5 | 6 | 1 | +| 5 | 0 | 0 | 5 | 5 | +| 1 | 1 | 0 | 1 | 1 | +| 1 | 1 | 1 | 2 | 1 | +| 1 | 3 | 3 | 4 | 1 | +| 1 | 3 | 5 | 6 | 1 | +| 5 | 10 | 0 | 10 | 10 | +| 10 | 10 | 0 | 10 | 10 | +| 10 | 10 | 10 | 20 | 10 | +| 15 | 10 | 10 | 25 | 15 | diff --git a/pkg/config/loader.go b/pkg/config/loader.go index 0a40ab02..b08ca64b 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 = 3 + IPv4DefaultWinMinIPTarget = 3 + IPv4DefaultWinMaxDev = 0 + IPv4DefaultWinResSize = 0 + + // Default Windows Configuration for IPv4 prefix resource type IPv4PDDefaultWorker = 2 IPv4PDDefaultWPSize = 1 IPv4PDDefaultMaxDev = 0 @@ -70,26 +71,45 @@ 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) { - warmIPTarget, minIPTarget, warmPrefixTarget = 0, 0, 0 - +// ParseWinIPTargetConfigs parses Windows IP target configuration parameters in the amazon-vpc-cni ConfigMap +// If all three config parameter values (warm-ip-target, min-ip-target, warm-prefix-target) are 0 or unset, or config map does not exist, +// then default values for warm-ip-target and min-ip-target will be set. +func ParseWinIPTargetConfigs(log logr.Logger, vpcCniConfigMap *v1.ConfigMap) (warmIPTarget int, minIPTarget int, warmPrefixTarget int, isPDEnabled bool) { if vpcCniConfigMap.Data == nil { - return warmIPTarget, minIPTarget, warmPrefixTarget + warmIPTarget = IPv4DefaultWinWarmIPTarget + minIPTarget = IPv4DefaultWinMinIPTarget + log.V(1).Info( + "No ConfigMap data found, falling back to using default values", + "minIPTarget", minIPTarget, + "warmIPTarget", warmIPTarget, + ) + return warmIPTarget, minIPTarget, 0, false + } + + isPDEnabled, err := strconv.ParseBool(vpcCniConfigMap.Data[EnableWindowsPrefixDelegationKey]) + if err != nil { + log.V(1).Info("Could not parse prefix delegation flag from ConfigMap, falling back to using secondary IP mode") + isPDEnabled = false } warmIPTargetStr, foundWarmIP := vpcCniConfigMap.Data[WarmIPTarget] @@ -105,36 +125,74 @@ 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 - } - + // If warm IP target config value is not found, or there is an error parsing it, the value will be set to zero if foundWarmIP { warmIPTargetInt, err := strconv.Atoi(warmIPTargetStr) if err != nil { - log.Error(err, "failed to parse warm ip target", "warm ip target", warmIPTargetStr) + log.V(1).Info("could not parse warm ip target, defaulting to zero", "warm ip target", warmIPTargetStr) + warmIPTarget = 0 } else { warmIPTarget = warmIPTargetInt + + // Handle secondary IP mode scenario where WarmIPTarget is explicitly configured to zero + // In such a case there must always be 1 warm IP to ensure that the warmpool is never empty + if !isPDEnabled && warmIPTarget == 0 { + log.V(1).Info("Explicitly setting WarmIPTarget zero value not supported in secondary IP mode, will override with 1") + warmIPTarget = 1 + } } + } else { + log.V(1).Info("could not find warm ip target in ConfigMap, defaulting to zero") + warmIPTarget = 0 } + + // If min IP target config value is not found, or there is an error parsing it, the value will be set to zero if foundMinIP { minIPTargetInt, err := strconv.Atoi(minIPTargetStr) if err != nil { - log.Error(err, "failed to parse minimum ip target", "minimum ip target", minIPTargetStr) + log.V(1).Info("could not parse minimum ip target, defaulting to zero", "minimum ip target", minIPTargetStr) + minIPTarget = 0 } else { minIPTarget = minIPTargetInt } + } else { + log.V(1).Info("could not find minimum ip target in ConfigMap, defaulting to zero") + minIPTarget = 0 } - if foundWarmPrefix { + + // If PD is enabled and warm prefix target config value is not found, or there is an error parsing it, the value will be set to zero + if !isPDEnabled && foundWarmPrefix { + log.V(1).Info("warm prefix configuration not supported in secondary IP mode, will ignore warm prefix configuration") + warmPrefixTarget = 0 + } 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("could not find warm prefix target in ConfigMap, defaulting to zero") + warmPrefixTarget = 0 } - return warmIPTarget, minIPTarget, warmPrefixTarget + + if warmIPTarget == 0 && minIPTarget == 0 { + if isPDEnabled && warmPrefixTarget == 0 { + minIPTarget = IPv4PDDefaultMinIPTargetSize + warmIPTarget = IPv4PDDefaultWarmIPTargetSize + } else if !isPDEnabled { + minIPTarget = IPv4DefaultWinMinIPTarget + warmIPTarget = IPv4DefaultWinWarmIPTarget + } + log.V(1).Info( + "No valid configuration values for warm-ip-target, min-ip-target and warm-prefix-target found in ConfigMap, falling back to using default values", + "minIPTarget", minIPTarget, + "warmIPTarget", warmIPTarget, + ) + } + + return warmIPTarget, minIPTarget, warmPrefixTarget, isPDEnabled } // getDefaultResourceConfig returns the default Resource Configuration. @@ -153,13 +211,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..e246eb75 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] @@ -61,8 +62,8 @@ func TestLoadResourceConfig(t *testing.T) { assert.Equal(t, IPv4PDDefaultWarmPrefixTargetSize, prefixIPv4WPConfig.WarmPrefixTarget) } -// TestParseWinPDTargets parses prefix delegation configurations from a vpc cni config map -func TestParseWinPDTargets(t *testing.T) { +// TestParseWinIPTargetConfigs_PDEnabledWithDefaultTargets parses prefix delegation configurations from a vpc cni config map +func TestParseWinIPTargetConfigs_PDEnabledWithDefaultTargets(t *testing.T) { log := zap.New(zap.UseDevMode(true)).WithName("loader test") vpcCNIConfig := &v1.ConfigMap{ @@ -74,14 +75,109 @@ func TestParseWinPDTargets(t *testing.T) { WarmPrefixTarget: strconv.Itoa(IPv4PDDefaultWarmPrefixTargetSize), }, } - warmIPTarget, minIPTarget, warmPrefixTarget := ParseWinPDTargets(log, vpcCNIConfig) + warmIPTarget, minIPTarget, warmPrefixTarget, isPDEnabled := ParseWinIPTargetConfigs(log, vpcCNIConfig) assert.Equal(t, IPv4PDDefaultWarmIPTargetSize, warmIPTarget) assert.Equal(t, IPv4PDDefaultMinIPTargetSize, minIPTarget) assert.Equal(t, IPv4PDDefaultWarmPrefixTargetSize, warmPrefixTarget) + assert.Equal(t, true, isPDEnabled) +} + +func TestParseWinIPTargetConfigs_PDDisabledWithAllZeroTargets(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(0), + MinimumIPTarget: strconv.Itoa(0), + }, + } + warmIPTarget, minIPTarget, _, isPDEnabled := ParseWinIPTargetConfigs(log, vpcCNIConfig) + assert.Equal(t, 1, warmIPTarget) + assert.Equal(t, 0, minIPTarget) + assert.Equal(t, false, isPDEnabled) +} + +func TestParseWinIPTargetConfigs_PDEnabledWithAllZeroTargets(t *testing.T) { + log := zap.New(zap.UseDevMode(true)).WithName("loader test") + + vpcCNIConfig := &v1.ConfigMap{ + Data: map[string]string{ + EnableWindowsIPAMKey: "true", + EnableWindowsPrefixDelegationKey: "true", + WarmIPTarget: strconv.Itoa(0), + MinimumIPTarget: strconv.Itoa(0), + WarmPrefixTarget: strconv.Itoa(0), + }, + } + warmIPTarget, minIPTarget, warmPrefixTarget, isPDEnabled := ParseWinIPTargetConfigs(log, vpcCNIConfig) + assert.Equal(t, IPv4PDDefaultWarmIPTargetSize, warmIPTarget) + assert.Equal(t, IPv4PDDefaultMinIPTargetSize, minIPTarget) + assert.Equal(t, IPv4PDDefaultWarmPrefixTargetSize, warmPrefixTarget) + assert.Equal(t, true, isPDEnabled) +} + +func TestParseWinIPTargetConfigs_PDDisabledWithDefaultTargets(t *testing.T) { + log := zap.New(zap.UseDevMode(true)).WithName("loader test") + + expectedWarmIPTarget := IPv4DefaultWinWarmIPTarget + expectedMinIPTarget := IPv4DefaultWinMinIPTarget + expectedWarmPrefixTarget := 0 + vpcCNIConfig := &v1.ConfigMap{ + Data: map[string]string{ + EnableWindowsIPAMKey: "true", + EnableWindowsPrefixDelegationKey: "false", + WarmIPTarget: strconv.Itoa(expectedWarmIPTarget), + MinimumIPTarget: strconv.Itoa(expectedMinIPTarget), + }, + } + warmIPTarget, minIPTarget, warmPrefixTarget, isPDEnabled := ParseWinIPTargetConfigs(log, vpcCNIConfig) + assert.Equal(t, expectedWarmIPTarget, warmIPTarget) + assert.Equal(t, expectedMinIPTarget, minIPTarget) + assert.Equal(t, expectedWarmPrefixTarget, warmPrefixTarget) + assert.Equal(t, false, isPDEnabled) +} + +func TestParseWinIPTargetConfigs_PDDisabledAndInvalidConfig_ReturnsDefault(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, IPv4DefaultWinWarmIPTarget, warmIPTarget) + assert.Equal(t, IPv4DefaultWinMinIPTarget, 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 +189,60 @@ func TestParseWinPDTargets_Negative(t *testing.T) { WarmPrefixTarget: strconv.Itoa(0), }, } - warmIPTarget, minIPTarget, warmPrefixTarget := ParseWinPDTargets(log, vpcCNIConfig) + warmIPTarget, minIPTarget, warmPrefixTarget, isPDEnabled := 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) + assert.Equal(t, true, isPDEnabled) +} + +func TestParseWinIPTargetConfigs_OnlyWindowsIPAM_EffectsDefaults(t *testing.T) { + log := zap.New(zap.UseDevMode(true)).WithName("loader test") + + vpcCNIConfig := &v1.ConfigMap{ + Data: map[string]string{ + EnableWindowsIPAMKey: "true", + }, + } + + warmIPTarget, minIPTarget, warmPrefixTarget, isPDEnabled := ParseWinIPTargetConfigs(log, vpcCNIConfig) + assert.Equal(t, IPv4DefaultWinWarmIPTarget, warmIPTarget) + assert.Equal(t, IPv4DefaultWinMinIPTarget, minIPTarget) + assert.Equal(t, 0, warmPrefixTarget) + assert.Equal(t, false, isPDEnabled) } -// TestParseWinPDTargets_Invalid parses prefix delegation configurations with invalid values and returns 0s as targets -func TestParseWinPDTargets_Invalid(t *testing.T) { +func TestParseWinIPTargetConfigs_PartiallyEmptyTargets_EffectsSpecified(t *testing.T) { + log := zap.New(zap.UseDevMode(true)).WithName("loader test") + + vpcCNIConfig := &v1.ConfigMap{ + Data: map[string]string{ + WinWarmIPTarget: "1", + }, + } + + warmIPTarget, minIPTarget, warmPrefixTarget, isPDEnabled := ParseWinIPTargetConfigs(log, vpcCNIConfig) + assert.Equal(t, 1, warmIPTarget) + assert.Equal(t, 0, minIPTarget) + assert.Equal(t, 0, warmPrefixTarget) + assert.Equal(t, false, isPDEnabled) +} + +// TestParseWinIPTargetConfigs_EmptyConfigMap_ReturnsDefaultsWithSecondaryIP parses configurations with empty configmap +func TestParseWinIPTargetConfigs_EmptyConfigMap_ReturnsDefaultsWithSecondaryIP(t *testing.T) { + log := zap.New(zap.UseDevMode(true)).WithName("loader test") + + vpcCNIConfig := &v1.ConfigMap{} + warmIPTarget, minIPTarget, warmPrefixTarget, isPDEnabled := ParseWinIPTargetConfigs(log, vpcCNIConfig) + assert.Equal(t, IPv4DefaultWinWarmIPTarget, warmIPTarget) + assert.Equal(t, IPv4DefaultWinMinIPTarget, minIPTarget) + assert.Equal(t, 0, warmPrefixTarget) + assert.Equal(t, false, isPDEnabled) +} + +// 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,10 +254,88 @@ func TestParseWinPDTargets_Invalid(t *testing.T) { WarmPrefixTarget: "can't parse", }, } - warmIPTarget, minIPTarget, warmPrefixTarget := ParseWinPDTargets(log, vpcCNIConfig) + warmIPTarget, minIPTarget, warmPrefixTarget, isPDEnabled := ParseWinIPTargetConfigs(log, vpcCNIConfig) + assert.Equal(t, IPv4PDDefaultWarmIPTargetSize, warmIPTarget) + assert.Equal(t, IPv4PDDefaultMinIPTargetSize, minIPTarget) + assert.Equal(t, 0, warmPrefixTarget) + assert.Equal(t, true, isPDEnabled) +} + +// TestParseWinIPTargetConfigs_PDEnabledAndWarmPrefixInvalid parses prefix delegation configurations with warm prefix being invalid +func TestParseWinIPTargetConfigs_PDEnabledAndWarmPrefixInvalid(t *testing.T) { + log := zap.New(zap.UseDevMode(true)).WithName("loader test") + + vpcCNIConfig := &v1.ConfigMap{ + Data: map[string]string{ + EnableWindowsIPAMKey: "true", + EnableWindowsPrefixDelegationKey: "true", + WarmIPTarget: "2", + MinimumIPTarget: "2", + WarmPrefixTarget: "invalid value", + }, + } + warmIPTarget, minIPTarget, warmPrefixTarget, isPDEnabled := ParseWinIPTargetConfigs(log, vpcCNIConfig) + assert.Equal(t, 2, warmIPTarget) + assert.Equal(t, 2, minIPTarget) + assert.Equal(t, 0, warmPrefixTarget) + assert.Equal(t, true, isPDEnabled) +} + +// TestParseWinIPTargetConfigs_PDEnabledAndWarmAndMinimumIPInvalid parses prefix delegation configurations with only warm prefix being valid +func TestParseWinIPTargetConfigs_PDEnabledAndWarmAndMinimumIPInvalid(t *testing.T) { + log := zap.New(zap.UseDevMode(true)).WithName("loader test") + + vpcCNIConfig := &v1.ConfigMap{ + Data: map[string]string{ + EnableWindowsIPAMKey: "true", + EnableWindowsPrefixDelegationKey: "true", + WarmIPTarget: "invalid value", + MinimumIPTarget: "invalid value", + WarmPrefixTarget: "1", + }, + } + warmIPTarget, minIPTarget, warmPrefixTarget, isPDEnabled := ParseWinIPTargetConfigs(log, vpcCNIConfig) assert.Equal(t, 0, warmIPTarget) assert.Equal(t, 0, minIPTarget) + assert.Equal(t, 1, warmPrefixTarget) + assert.Equal(t, true, isPDEnabled) +} + +// TestParseWinIPTargetConfigs_PDEnabledAndWarmAndOnlyWarmPrefixSet parses prefix delegation configurations with only warm prefix set +func TestParseWinIPTargetConfigs_PDEnabledAndWarmAndOnlyWarmPrefixSet(t *testing.T) { + log := zap.New(zap.UseDevMode(true)).WithName("loader test") + + vpcCNIConfig := &v1.ConfigMap{ + Data: map[string]string{ + EnableWindowsIPAMKey: "true", + EnableWindowsPrefixDelegationKey: "true", + WarmPrefixTarget: "1", + }, + } + warmIPTarget, minIPTarget, warmPrefixTarget, isPDEnabled := ParseWinIPTargetConfigs(log, vpcCNIConfig) + assert.Equal(t, 0, warmIPTarget) + assert.Equal(t, 0, minIPTarget) + assert.Equal(t, 1, warmPrefixTarget) + assert.Equal(t, true, isPDEnabled) +} + +// TestParseWinIPTargetConfigs_PDEnabledAndWarmAndOnlyWarmPrefixNotSet parses prefix delegation configurations with only warm prefix not set +func TestParseWinIPTargetConfigs_PDEnabledAndWarmAndOnlyWarmPrefixNotSet(t *testing.T) { + log := zap.New(zap.UseDevMode(true)).WithName("loader test") + + vpcCNIConfig := &v1.ConfigMap{ + Data: map[string]string{ + EnableWindowsIPAMKey: "true", + EnableWindowsPrefixDelegationKey: "true", + WarmIPTarget: "2", + MinimumIPTarget: "2", + }, + } + warmIPTarget, minIPTarget, warmPrefixTarget, isPDEnabled := ParseWinIPTargetConfigs(log, vpcCNIConfig) + assert.Equal(t, 2, warmIPTarget) + assert.Equal(t, 2, minIPTarget) assert.Equal(t, 0, warmPrefixTarget) + assert.Equal(t, true, isPDEnabled) } // TestLoadResourceConfigFromConfigMap tests the custom configuration for PD is loaded correctly from config map @@ -148,14 +367,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..8f5fe26b 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,21 @@ 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", + "isPDPool", p.isPDPool, + "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 +458,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 +736,73 @@ 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 + } + + 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.V(1).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..ef15acc3 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) + // Both 2 warm secondary IP addresses need to be deleted, warm pool should drain to zero + assert.Equal(t, worker.OperationDeleted, job.Operations) + expectedDeletedCount := 2 + 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,259 @@ func TestNumResourcesFromMap(t *testing.T) { count = numResourcesFromMap(map[string][]Resource{grp5: {}}) assert.Equal(t, 0, count) } + +// TestCalcSecondaryIPDeviation_PDDisabledAndInvalidZeroTargets tests the pool calculation when prefix delegation is disabled, and secondary IP mode is active +// Zero values for minIPTarget and warmIPTarget are permitted because the user should have the flexibility to configure as little as zero minimum and warm IP targets +// Note that at current time, implementation in 'loader.go' prevents these zero values from being propagated, and instead overrides warm-ip-target with a default value of 1 so warmpool always has an available IP +func TestCalcSecondaryIPDeviation_PDDisabledAndInvalidZeroTargets(t *testing.T) { + poolConfig.MinIPTarget = 0 + poolConfig.WarmIPTarget = 0 + pdPool := getMockPool(poolConfig, nil, nil, 7, false) + + deviation := pdPool.calculateSecondaryIPDeviation() + assert.Equal(t, 0, pdPool.warmPoolConfig.WarmIPTarget) + assert.Equal(t, 0, pdPool.warmPoolConfig.MinIPTarget) + assert.Equal(t, 0, deviation) +} + +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..e27fff9f 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" diff --git a/pkg/provider/ip/provider.go b/pkg/provider/ip/provider.go index f3bba704..bd4b5d2e 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 && isNitroInstance) + // 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..e05f42e5 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,21 @@ 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), + config.WarmPrefixTarget: strconv.Itoa(0), + }, + } 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 +468,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) +} diff --git a/scripts/test/run-integration-tests.sh b/scripts/test/run-integration-tests.sh index 043723a2..16e52144 100755 --- a/scripts/test/run-integration-tests.sh +++ b/scripts/test/run-integration-tests.sh @@ -42,7 +42,7 @@ function run_integration_tests(){ TEST_RESULT=success (cd $INTEGRATION_TEST_DIR/perpodsg && CGO_ENABLED=0 ginkgo --skip=LOCAL $EXTRA_GINKGO_FLAGS -v -timeout=35m -- -cluster-kubeconfig=$KUBE_CONFIG_PATH -cluster-name=$CLUSTER_NAME --aws-region=$REGION --aws-vpc-id $VPC_ID) || TEST_RESULT=fail if [[ -z "${SKIP_WINDOWS_TEST}" ]]; then - (cd $INTEGRATION_TEST_DIR/windows && CGO_ENABLED=0 ginkgo --skip=LOCAL $EXTRA_GINKGO_FLAGS -v -timeout=120m -- -cluster-kubeconfig=$KUBE_CONFIG_PATH -cluster-name=$CLUSTER_NAME --aws-region=$REGION --aws-vpc-id $VPC_ID) || TEST_RESULT=fail + (cd $INTEGRATION_TEST_DIR/windows && CGO_ENABLED=0 ginkgo --skip=LOCAL $EXTRA_GINKGO_FLAGS -v -timeout=180m -- -cluster-kubeconfig=$KUBE_CONFIG_PATH -cluster-name=$CLUSTER_NAME --aws-region=$REGION --aws-vpc-id $VPC_ID) || TEST_RESULT=fail else echo "skipping Windows tests" fi diff --git a/test/README.md b/test/README.md index bb9aa28c..da942178 100644 --- a/test/README.md +++ b/test/README.md @@ -63,7 +63,7 @@ The Integration test suite provides the following focuses. echo "Running Security Group for Pods Integration Tests" (cd perpodsg && CGO_ENABLED=0 GOOS=$OS ginkgo -v --timeout 40m -- --cluster-kubeconfig=$KUBE_CONFIG_PATH --cluster-name=$CLUSTER_NAME --aws-region=$AWS_REGION --aws-vpc-id=$VPC_ID) echo "Running Windows Integration Tests" - (cd windows && CGO_ENABLED=0 GOOS=$OS ginkgo -v --timeout 40m -- --cluster-kubeconfig=$KUBE_CONFIG_PATH --cluster-name=$CLUSTER_NAME --aws-region=$AWS_REGION --aws-vpc-id=$VPC_ID) + (cd windows && CGO_ENABLED=0 GOOS=$OS ginkgo -v --timeout 180m -- --cluster-kubeconfig=$KUBE_CONFIG_PATH --cluster-name=$CLUSTER_NAME --aws-region=$AWS_REGION --aws-vpc-id=$VPC_ID) ``` #### Running Integration tests on Controller running on EKS Control Plane diff --git a/test/integration/windows/windows_test.go b/test/integration/windows/windows_test.go index 14a83489..c2d27de6 100644 --- a/test/integration/windows/windows_test.go +++ b/test/integration/windows/windows_test.go @@ -59,6 +59,12 @@ var _ = Describe("Windows Integration Test", func() { testConfigMap v1.ConfigMap ) + var sleepInfinityContainerCommands = []string{ + GetCommandToTestHostConnectivity( + "www.amazon.com", 80, 2, true, + ), + } + BeforeEach(func() { namespace = "windows-test" jobParallelism = 1 @@ -203,10 +209,240 @@ var _ = Describe("Windows Integration Test", func() { }) }) + Describe("configMap secondary IP mode tests", Label("windows-prefix-delegation-disabled"), func() { + // Test workflow when windows prefix delegation is disabled and secondary IP mode is active + // In secondary IP mode, pods must have secondary IPs assigned + var testPod, testPod2, testPod3 *v1.Pod + var instanceID string + var nodeName string + var bufferForCoolDown = config.CoolDownPeriod + (time.Second * 5) + var poolReconciliationWaitTime = time.Second * 5 + container := manifest.NewWindowsContainerBuilder().Args(sleepInfinityContainerCommands).Build() + + data = map[string]string{ + config.EnableWindowsIPAMKey: "true", + config.EnableWindowsPrefixDelegationKey: "false", + } + testerContainerCommands = []string{ + GetCommandToTestHostConnectivity("www.amazon.com", 80, 2, false), + } + + JustBeforeEach(func() { + windowsNodeList = node.GetNodeAndWaitTillCapacityPresent(frameWork.NodeManager, "windows", + config.ResourceNameIPAddress) + instanceID = manager.GetNodeInstanceID(&windowsNodeList.Items[0]) + nodeName = windowsNodeList.Items[0].Name + + testPod = generateTestPodSpec(1, container, nodeName) + testPod2 = generateTestPodSpec(2, container, nodeName) + testPod3 = generateTestPodSpec(3, container, nodeName) + + data[config.EnableWindowsIPAMKey] = "true" + data[config.EnableWindowsPrefixDelegationKey] = "false" + updateConfigMap(data, poolReconciliationWaitTime) + + GinkgoWriter.Printf("Waiting %d seconds for cooldown period...\n", int(bufferForCoolDown.Seconds())) + time.Sleep(bufferForCoolDown) + }) + + AfterEach(func() { + data = map[string]string{ + config.EnableWindowsIPAMKey: "true", + config.EnableWindowsPrefixDelegationKey: "false", + } + updateConfigMap(data, poolReconciliationWaitTime) + }) + + Context("when prefix delegation is disabled and secondary IP mode is active", func() { + Context("When windows-warm-prefix-target is set to non zero value", Label("windows-warm-prefix-target"), func() { + BeforeEach(func() { + data[config.WinWarmPrefixTarget] = "2" + }) + It("if windows-warm-prefix-target is 2 the value should be ignored and no prefixes should be assigned", func() { + By(fmt.Sprintf("creating 1 Windows pod and waiting until in ready status with timeout of %d seconds", int(utils.WindowsPodsCreationTimeout.Seconds()))) + createdPod1, err := frameWork.PodManager.CreateAndWaitTillPodIsRunning(ctx, testPod, utils.WindowsPodsCreationTimeout) + Expect(err).ToNot(HaveOccurred()) + verify.WindowsPodHaveIPv4Address(createdPod1) + + GinkgoWriter.Printf("Waiting %d seconds for warmpool to reconciliate after creating new pod(s)...\n", int(poolReconciliationWaitTime.Seconds())) + time.Sleep(poolReconciliationWaitTime) + + ipv4AddressCount, prefixCount, err := frameWork.EC2Manager.GetPrivateIPv4AddressAndPrefix(instanceID) + Expect(err).ToNot(HaveOccurred()) + Expect(len(ipv4AddressCount)).To(BeNumerically(">=", 1)) + Expect(len(prefixCount)).To(Equal(0)) + + err = frameWork.PodManager.DeleteAndWaitTillPodIsDeleted(ctx, testPod) + Expect(err).ToNot(HaveOccurred()) + }) + }) + + Context("When windows-warm-ip-target is set to non zero value", Label("windows-warm-ip-target"), func() { + BeforeEach(func() { + data[config.WinWarmIPTarget] = "1" + data[config.WinMinimumIPTarget] = "0" + }) + It("if windows-warm-ip-target is 1 should have 1 warm IPs available when 0 pods were running", func() { + ipv4AddressCount, prefixCount, err := frameWork.EC2Manager.GetPrivateIPv4AddressAndPrefix(instanceID) + Expect(err).ToNot(HaveOccurred()) + + Expect(len(ipv4AddressCount)).To(Equal(1)) + Expect(len(prefixCount)).To(Equal(0)) + }) + }) + + Context("When windows-warm-ip-target is 5", Label("windows-warm-ip-target"), func() { + BeforeEach(func() { + data[config.WinWarmIPTarget] = "5" + }) + It("should have 7 warm IPs available when 2 pods were running", func() { + By(fmt.Sprintf("creating 2 Windows pods and waiting until in ready status with timeout of %d seconds", int(utils.WindowsPodsCreationTimeout.Seconds()))) + createdPod1, err := frameWork.PodManager.CreateAndWaitTillPodIsRunning(ctx, testPod, utils.WindowsPodsCreationTimeout) + Expect(err).ToNot(HaveOccurred()) + createdPod2, err := frameWork.PodManager.CreateAndWaitTillPodIsRunning(ctx, testPod2, utils.WindowsPodsCreationTimeout) + Expect(err).ToNot(HaveOccurred()) + + verify.WindowsPodHaveIPv4Address(createdPod1) + verify.WindowsPodHaveIPv4Address(createdPod2) + + GinkgoWriter.Printf("Waiting %d seconds for warmpool to reconciliate after creating new pod(s)...\n", int(poolReconciliationWaitTime.Seconds())) + time.Sleep(poolReconciliationWaitTime) + + ipv4AddressCount, prefixCount, err := frameWork.EC2Manager.GetPrivateIPv4AddressAndPrefix(instanceID) + Expect(err).ToNot(HaveOccurred()) + Expect(len(ipv4AddressCount)).To(BeNumerically("==", 7)) + Expect(len(prefixCount)).To(Equal(0)) + + err = frameWork.PodManager.DeleteAndWaitTillPodIsDeleted(ctx, testPod) + Expect(err).ToNot(HaveOccurred()) + err = frameWork.PodManager.DeleteAndWaitTillPodIsDeleted(ctx, testPod2) + Expect(err).ToNot(HaveOccurred()) + }) + }) + + Context("When windows-minimum-ip-target is set to non zero value", Label("windows-minimum-ip-target"), func() { + BeforeEach(func() { + data[config.WinMinimumIPTarget] = "6" + }) + It("if windows-minimum-ip-target is 6 should have 6 warm IPs available when 0 pods were running", func() { + ipv4AddressCount, prefixCount, err := frameWork.EC2Manager.GetPrivateIPv4AddressAndPrefix(instanceID) + Expect(err).ToNot(HaveOccurred()) + Expect(len(ipv4AddressCount)).To(Equal(6)) + Expect(len(prefixCount)).To(Equal(0)) + }) + }) + + Context("When windows-minimum-ip-target is set to 6 with 3 pods running", Label("windows-minimum-ip-target"), func() { + BeforeEach(func() { + data[config.WinMinimumIPTarget] = "6" + }) + It("if windows-minimum-ip-target is 6 should have 6 IPs assigned to the node when 3 pods were running", func() { + By(fmt.Sprintf("creating 3 Windows pods and waiting until in ready status with timeout of %d seconds", int(utils.WindowsPodsCreationTimeout.Seconds()))) + createdPod1, err := frameWork.PodManager.CreateAndWaitTillPodIsRunning(ctx, testPod, utils.WindowsPodsCreationTimeout) + Expect(err).ToNot(HaveOccurred()) + createdPod2, err := frameWork.PodManager.CreateAndWaitTillPodIsRunning(ctx, testPod2, utils.WindowsPodsCreationTimeout) + Expect(err).ToNot(HaveOccurred()) + createdPod3, err := frameWork.PodManager.CreateAndWaitTillPodIsRunning(ctx, testPod3, utils.WindowsPodsCreationTimeout) + Expect(err).ToNot(HaveOccurred()) + + verify.WindowsPodHaveIPv4Address(createdPod1) + verify.WindowsPodHaveIPv4Address(createdPod2) + verify.WindowsPodHaveIPv4Address(createdPod3) + + GinkgoWriter.Printf("Waiting %d seconds for warmpool to reconciliate after creating new pod(s)...\n", int(poolReconciliationWaitTime.Seconds())) + time.Sleep(poolReconciliationWaitTime) + + ipv4AddressCount, prefixCount, err := frameWork.EC2Manager.GetPrivateIPv4AddressAndPrefix(instanceID) + Expect(err).ToNot(HaveOccurred()) + Expect(len(ipv4AddressCount)).To(Equal(6)) + Expect(len(prefixCount)).To(Equal(0)) + + err = frameWork.PodManager.DeleteAndWaitTillPodIsDeleted(ctx, testPod) + Expect(err).ToNot(HaveOccurred()) + err = frameWork.PodManager.DeleteAndWaitTillPodIsDeleted(ctx, testPod2) + Expect(err).ToNot(HaveOccurred()) + err = frameWork.PodManager.DeleteAndWaitTillPodIsDeleted(ctx, testPod3) + Expect(err).ToNot(HaveOccurred()) + }) + }) + + Context("When windows-warm-ip-target and windows-minimum-ip-target set to non zero values", Label("windows-minimum-ip-target"), func() { + Context("windows-minimum-ip-target=3 and windows-warm-ip-target=6", func() { + BeforeEach(func() { + data[config.WinMinimumIPTarget] = "3" + data[config.WinWarmIPTarget] = "6" + }) + It("if should have 6 IPs assigned and 6 warm IPs available when 0 pods were running", func() { + ipv4AddressCount, prefixCount, err := frameWork.EC2Manager.GetPrivateIPv4AddressAndPrefix(instanceID) + Expect(err).ToNot(HaveOccurred()) + Expect(len(ipv4AddressCount)).To(Equal(6)) + Expect(len(prefixCount)).To(Equal(0)) + }) + }) + + Context("windows-minimum-ip-target=8 and windows-warm-ip-target=4", func() { + BeforeEach(func() { + data[config.WinMinimumIPTarget] = "8" + data[config.WinWarmIPTarget] = "4" + }) + It("should have 8 IPs assigned and 8 warm IPs available when 0 pods were running", func() { + ipv4AddressCount, prefixCount, err := frameWork.EC2Manager.GetPrivateIPv4AddressAndPrefix(instanceID) + Expect(err).ToNot(HaveOccurred()) + Expect(len(ipv4AddressCount)).To(Equal(8)) + Expect(len(prefixCount)).To(Equal(0)) + }) + }) + Context("windows-minimum-ip-target=2 and windows-warm-ip-target=4", func() { + BeforeEach(func() { + data[config.WinMinimumIPTarget] = "2" + data[config.WarmIPTarget] = "4" + }) + It("should have 6 IPs assigned and 4 warm IPs available when 2 pods were running", func() { + By(fmt.Sprintf("creating 2 Windows pods and waiting until in ready status with timeout of %d seconds", int(utils.WindowsPodsCreationTimeout.Seconds()))) + createdPod1, err := frameWork.PodManager.CreateAndWaitTillPodIsRunning(ctx, testPod, utils.WindowsPodsCreationTimeout) + Expect(err).ToNot(HaveOccurred()) + createdPod2, err := frameWork.PodManager.CreateAndWaitTillPodIsRunning(ctx, testPod2, utils.WindowsPodsCreationTimeout) + Expect(err).ToNot(HaveOccurred()) + + verify.WindowsPodHaveIPv4Address(createdPod1) + verify.WindowsPodHaveIPv4Address(createdPod2) + + GinkgoWriter.Printf("Waiting %d seconds for warmpool to reconciliate after creating new pod(s)...\n", int(poolReconciliationWaitTime.Seconds())) + time.Sleep(poolReconciliationWaitTime) + + ipv4AddressCount, prefixCount, err := frameWork.EC2Manager.GetPrivateIPv4AddressAndPrefix(instanceID) + Expect(err).ToNot(HaveOccurred()) + Expect(len(ipv4AddressCount)).To(Equal(6)) + Expect(len(prefixCount)).To(Equal(0)) + + err = frameWork.PodManager.DeleteAndWaitTillPodIsDeleted(ctx, testPod) + Expect(err).ToNot(HaveOccurred()) + err = frameWork.PodManager.DeleteAndWaitTillPodIsDeleted(ctx, testPod2) + Expect(err).ToNot(HaveOccurred()) + }) + }) + }) + + Context("When windows-warm-ip-target and windows-min-ip-target set to 0", Label("windows-warm-ip-target"), func() { + BeforeEach(func() { + data[config.WinMinimumIPTarget] = "0" + data[config.WinWarmIPTarget] = "0" + }) + + It("should result in warm-ip-target=1 and min-ip-target=0", func() { + ipv4AddressCount, prefixCount, err := frameWork.EC2Manager.GetPrivateIPv4AddressAndPrefix(instanceID) + Expect(err).ToNot(HaveOccurred()) + Expect(len(ipv4AddressCount)).To(Equal(1)) + Expect(len(prefixCount)).To(Equal(0)) + }) + }) + }) + }) + Describe("configMap enable-windows-prefix-delegation tests", Label("windows-prefix-delegation"), func() { // Test windows prefix delegation feature enable/disable. When feature enabled, pod must have // prefix ips assigned. Otherwise, pod must have secondary ip assigned. - var testPod, testPod2 *v1.Pod + var testPod, testPod2, testPodLongLiving *v1.Pod var createdPod *v1.Pod var instanceID string var nodeName string @@ -225,12 +461,13 @@ var _ = Describe("Windows Integration Test", func() { nodeName = windowsNodeList.Items[0].Name testerContainerCommands = []string{ - GetCommandToTestHostConnectivity("www.amazon.com", 80, 2), + GetCommandToTestHostConnectivity("www.amazon.com", 80, 2, false), } testerContainer = manifest.NewWindowsContainerBuilder(). Args(testerContainerCommands). Build() + testContainerLongLiving := manifest.NewWindowsContainerBuilder().Args(sleepInfinityContainerCommands).Build() testPod, err = manifest.NewWindowsPodBuilder(). Namespace("windows-test"). @@ -253,6 +490,17 @@ var _ = Describe("Windows Integration Test", func() { NodeName(nodeName). Build() Expect(err).ToNot(HaveOccurred()) + + testPodLongLiving, err = manifest.NewWindowsPodBuilder(). + Namespace("windows-test"). + Name("windows-pod-long-living"). + Container(testContainerLongLiving). + OS("windows"). + TerminationGracePeriod(0). + RestartPolicy(v1.RestartPolicyNever). + NodeName(nodeName). + Build() + Expect(err).ToNot(HaveOccurred()) }) Context("when prefix delegation is enabled", func() { @@ -335,7 +583,7 @@ var _ = Describe("Windows Integration Test", func() { By("creating 1 pod and waiting for ready should not create new prefix") // verify if ip assigned is coming from a prefix - createdPod, err = frameWork.PodManager.CreateAndWaitTillPodIsRunning(ctx, testPod, utils.WindowsPodsCreationTimeout) + createdPod, err = frameWork.PodManager.CreateAndWaitTillPodIsRunning(ctx, testPodLongLiving, utils.WindowsPodsCreationTimeout) Expect(err).ToNot(HaveOccurred()) _, prefixesAfterPod1, err := frameWork.EC2Manager.GetPrivateIPv4AddressAndPrefix(instanceID) @@ -357,7 +605,7 @@ var _ = Describe("Windows Integration Test", func() { Expect(len(privateIPsBefore)).To(Equal(len(privateIPsAfter))) verify.WindowsPodHaveIPv4AddressFromPrefixes(createdPod, prefixesAfterPod2) - err = frameWork.PodManager.DeleteAndWaitTillPodIsDeleted(ctx, testPod) + err = frameWork.PodManager.DeleteAndWaitTillPodIsDeleted(ctx, testPodLongLiving) Expect(err).ToNot(HaveOccurred()) err = frameWork.PodManager.DeleteAndWaitTillPodIsDeleted(ctx, testPod2) Expect(err).ToNot(HaveOccurred()) @@ -467,7 +715,7 @@ var _ = Describe("Windows Integration Test", func() { By("creating 1 pod and waiting for ready should not create new prefix") // verify if ip assigned is coming from a prefix - createdPod, err = frameWork.PodManager.CreateAndWaitTillPodIsRunning(ctx, testPod, utils.WindowsPodsCreationTimeout) + createdPod, err = frameWork.PodManager.CreateAndWaitTillPodIsRunning(ctx, testPodLongLiving, utils.WindowsPodsCreationTimeout) Expect(err).ToNot(HaveOccurred()) _, prefixesAfterPod1, err := frameWork.EC2Manager.GetPrivateIPv4AddressAndPrefix(instanceID) @@ -489,7 +737,7 @@ var _ = Describe("Windows Integration Test", func() { Expect(len(privateIPsBefore)).To(Equal(len(privateIPsAfter))) verify.WindowsPodHaveIPv4AddressFromPrefixes(createdPod, prefixesAfterPod2) - err = frameWork.PodManager.DeleteAndWaitTillPodIsDeleted(ctx, testPod) + err = frameWork.PodManager.DeleteAndWaitTillPodIsDeleted(ctx, testPodLongLiving) Expect(err).ToNot(HaveOccurred()) err = frameWork.PodManager.DeleteAndWaitTillPodIsDeleted(ctx, testPod2) Expect(err).ToNot(HaveOccurred()) @@ -631,7 +879,7 @@ var _ = Describe("Windows Integration Test", func() { CreateAndWaitUntilDeploymentReady(ctx, oldControllerDeployment) Expect(err).ToNot(HaveOccurred()) - By("creating windows pod and waiting for it to timout") + By("creating windows pod and waiting for it to timeout") createdPod, err := frameWork.PodManager. CreateAndWaitTillPodIsRunning(ctx, testPod, utils.WindowsPodsCreationTimeout) Expect(err).To(HaveOccurred()) @@ -685,7 +933,7 @@ var _ = Describe("Windows Integration Test", func() { BeforeEach(func() { jobParallelism = 30 testerContainerCommands = []string{ - GetCommandToTestHostConnectivity(service.Spec.ClusterIP, service.Spec.Ports[0].Port, 10), + GetCommandToTestHostConnectivity(service.Spec.ClusterIP, service.Spec.Ports[0].Port, 10, false), } }) @@ -702,7 +950,7 @@ var _ = Describe("Windows Integration Test", func() { BeforeEach(func() { jobParallelism = 1 testerContainerCommands = []string{ - GetCommandToTestHostConnectivity(service.Spec.ClusterIP, 1, 1), + GetCommandToTestHostConnectivity(service.Spec.ClusterIP, 1, 1, false), } }) @@ -714,7 +962,7 @@ var _ = Describe("Windows Integration Test", func() { Context("when connecting to internet", func() { BeforeEach(func() { testerContainerCommands = []string{ - GetCommandToTestHostConnectivity("www.amazon.com", 80, 2), + GetCommandToTestHostConnectivity("www.amazon.com", 80, 2, false), } }) @@ -727,7 +975,7 @@ var _ = Describe("Windows Integration Test", func() { Context("when connecting to invalid url", func() { BeforeEach(func() { testerContainerCommands = []string{ - GetCommandToTestHostConnectivity("www.amazon.zzz", 80, 1), + GetCommandToTestHostConnectivity("www.amazon.zzz", 80, 1, false), } }) @@ -778,7 +1026,7 @@ var _ = Describe("Windows Integration Test", func() { testerContainer = manifest.NewWindowsContainerBuilder(). Args([]string{ - GetCommandToTestHostConnectivity(service.Spec.ClusterIP, service.Spec.Ports[0].Port, 10)}). + GetCommandToTestHostConnectivity(service.Spec.ClusterIP, service.Spec.Ports[0].Port, 10, false)}). Build() testerJob = manifest.NewWindowsJob(). @@ -835,7 +1083,7 @@ var _ = Describe("Windows Integration Test", func() { Describe("when creating pod with same namespace and name", func() { BeforeEach(func() { testerContainerCommands = []string{ - GetCommandToTestHostConnectivity("www.amazon.com", 80, 2), + GetCommandToTestHostConnectivity("www.amazon.com", 80, 2, false), } }) @@ -883,19 +1131,45 @@ var _ = Describe("Windows Integration Test", func() { }) }) -// GetCommandToTestHostConnectivity tests the DNS Resolution and the tcp connection to the -// host -func GetCommandToTestHostConnectivity(host string, port int32, retries int) string { +func generateTestPodSpec(index int, testerContainer v1.Container, nodeName string) *v1.Pod { + testPod, err := manifest.NewWindowsPodBuilder(). + Namespace("windows-test"). + Name(fmt.Sprintf("windows-secondary-ip-pod-%d", index)). + Container(testerContainer). + OS("windows"). + TerminationGracePeriod(0). + RestartPolicy(v1.RestartPolicyNever). + NodeName(nodeName). + Build() + Expect(err).ToNot(HaveOccurred()) + return testPod +} + +func updateConfigMap(data map[string]string, waitTime time.Duration) { + By("updating the configmap") + builtConfigMap := *manifest.NewConfigMapBuilder().Data(data).Build() + configMapWrapper.UpdateConfigMap(frameWork.ConfigMapManager, ctx, &builtConfigMap) + GinkgoWriter.Printf("Updated amazon-vpc-cni config map data: %v\n", data) + GinkgoWriter.Printf("Waiting %d seconds for pool reconciliation...\n", int(waitTime.Seconds())) + time.Sleep(waitTime) +} + +func GetCommandToTestHostConnectivity(host string, port int32, retries int, sleepForever bool) string { return fmt.Sprintf(` $Server = "%s" $Port = %d $Retries = %d $RetryInterval = 1 + $SleepForever = $%t # If true, sleep forever after the test is complete While (-Not (Test-NetConnection -ComputerName $Server -Port $Port).TcpTestSucceeded) { if ($Retries -le 0) { Write-Warning "maximum number of connection attempts reached, exiting" - exit 1 + if (!$SleepForever) { + exit 1 + } else { + break + } } Write-Warning "failed to connect to server $Server, will retry" Start-Sleep -s $RetryInterval @@ -903,7 +1177,14 @@ func GetCommandToTestHostConnectivity(host string, port int32, retries int) stri # Limit RetryInterval to 20 seconds after it exceeds certain value $RetryInterval = if ($RetryInterval -lt 20) {$RetryInterval*2} else {20} } - Write-Output "connection from $env:COMPUTERNAME to $Server succeeded"`, host, port, retries) + Write-Output "connection from $env:COMPUTERNAME to $Server succeeded" + if ($SleepForever) { + while ($true) { + $SleepSeconds = 3600 + Write-Output "Sleeping forver, will sleep for $SleepSeconds seconds at a time..." + Start-Sleep -Seconds $SleepSeconds; + } + }`, host, port, retries, sleepForever) } // Install and start the dot net web server, it's light weight so starts pretty quick @@ -921,5 +1202,5 @@ func GetCommandToContinuouslyTestHostConnectivity(host string, tries int, interv Start-Sleep -s %d # Sleep for specified interval before testing connection %s # The test connection command $val++ - }`, tries, interval, GetCommandToTestHostConnectivity(host, 80, 10)) + }`, tries, interval, GetCommandToTestHostConnectivity(host, 80, 10, false)) }