Add Windows secondary IP mode configurable options for managing IP address allocation

[tzifudzi]

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

Issue

Resolves #428

Description of changes

This change introduces changes to provide configurable options for managing IP address allocation on Windows nodes when using secondary IP mode. The VPC Resource Controller will now allow configuring of the windows-minimum-ip-target and windows-warm-ip-target parameters in the amazon-vpc-cni ConfigMap. These parameters will control the minimum number of IP addresses to be allocated on each Windows node, and the number of 'warm' IP addresses to be pre-allocated respectively. The VPC RC will always ensure that based on the configurations, there will always be enough warm IPs to satisfy the configurations.

Previously, the VPC Resource Controller utilized a fixed and hardcoded number of 3 warm secondary IP addresses on each Windows node. This approach, while beneficial for enabling faster pod startup times, wasn't ideal for the following example scenarios.

1. When there is a need to configure fewer warm IPs to improve IP address utilization in the subnet. The fixed warm IP target could lead to inefficient IP address utilization, particularly in subnets with limited IPv4 address space.
2. When there is a need to configure a higher target for warm IPs to ensure faster pod startup times. The fixed warm IP target may not provide enough pre-allocated IP addresses to meet the requirements for faster pod launches in a case where a warmpool target greater than 3 is desired.
3. When there is a need to configure a minimum IP target after the node is set up. The lack of a configurable minimum IP target could result in insufficient IP addresses being allocated to the node, potentially impacting pod startup times in cases where high availability is desired.

Scope

• This change only affects Windows nodes when enable-windows-ipam is set to true
• This change only affects Windows modes when running in the Secondary IP mode, in contrast to running in prefix delegation mode

Notes for reviewers

• A warmpool target of 1 is enforced even if a zero value is set because the current code design workflow relies on the warmpool always being populated with at least 1 warm IP. In future, a follow up change can be applied to make it both minimum IP and warm IP targets configurable to zero such that zero IPs are kept warm for use cases where no warm IPs are desired.

Todo

• Integration tests will be added later to further validate the proposed changes. To start with unit tests have been added and manual e2e tests have been performed. After reaching consensus on the core design and implementation of the initial changes, I can follow up with integration tests either in this PR or a different one.
• Documentation will be updated to reflect the new configuration options and their intended usage once the core design and default behavior have been finalized and agreed upon. This will ensure that the documentation accurately represents the final implementation.

Integration Test Output

Output as of Tue Jul 23 11:52AM PDT

[tzifudzi:~/GolandProjects/amazon-vpc-resource-controller-k8s/test/integration/windows] feature/secondary-ip-configs(+11/-15,1)+* 3h0m40s 130 ± ginkgo --skip=Stress -v --timeout 180m -- --cluster-kubeconfig='/Users/tzifudzi/.kube/config' --cluster-name=$CLUSTER_NAME --aws-region=$AWS_REGION --aws-vpc-id=$VPC_ID --ginkgo.skip="stress"
Running Suite: Windows Integration Test Suite - /Users/tzifudzi/GolandProjects/amazon-vpc-resource-controller-k8s/test/integration/windows
...
Ran 34 of 40 Specs in 6833.774 seconds
SUCCESS! -- 34 Passed | 0 Failed | 0 Pending | 6 Skipped
PASS
...

Integration test runs

• July 22 integration-test-output.txt
• Aug 29 output.txt

[tzifudzi]

cc @jiechen0826 Please join the review

[tzifudzi]

@haouc @jiechen0826 I considered creating a validating webhook as part of this change to validate the configuration values and prevent invalid values from being set. I chose to leave it out to keep the change minimal and do a follow up change separately. Any thoughts on this?

[jiechen0826]

I'm halfway through the changes, but just posting these comments first

[jiechen0826]

Reviewed all files. Ready to approve once the above comments are addressed.

[jiechen0826]

Just one more comment. Btw, there seems to be a unit test failure.

[jiechen0826]

LGTM. Thanks for implementing the feature and addressing my comments!

[tzifudzi]

@jiechen0826 I have added documentation and integration tests. Please take another look? The logic you reviewed has not changed.

Note: New integration tests for secondary IP mode are all passing, currently investigating a few failures in PD tests. Once all integration tests are passing I will drop a note on the test output. In the meantime please proceed to review.

[jiechen0826]

LGTM, thanks for adding the integration tests and doc update!

[tzifudzi]

@haouc I have responded to your latest feedback and pushed new code changes in commit e466622. On merge I will squash changes into one commit. Please re-review.

[haouc]

lgtm, thanks.