✨ Support BYO Public IPv4 Pool when provision infrastructure #4905

mtulio · 2024-04-03T19:13:55Z

What type of PR is this?

/kind feature
/kind api-change
/kind documentation

What this PR does / why we need it:

Introducing support of PublicIpv4Pool to provision base cluster infrastructure consuming public IPv4 (EIP) from a custom Public IPv4 pool brought to AWS.

A subset of changes of this PR is isolated into those PRs:

Which issue(s) this PR fixes

Fixes #4887

Special notes for your reviewer:

The changes proposes to create Elastic IPs for each resource and zone claiming public IP address when creating the core infrastructure components and Machines in public subnets. The components supported for core infrastructure are Nat Gateways and Network Load Balancer for API server.

Checklist:

Release note:

Introduce the support of provisioning public IPv4 address consuming from a custom Public IPv4 Pool that is brought to AWS. When the field `PublicIpv4Pool` is set with the pool ID, all the network resources which claims public IPv4, such as Network Load Balancers and NAT Gateways, will be created consuming from the custom pool.

k8s-ci-robot · 2024-04-03T19:14:04Z

Hi @mtulio. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

mtulio

Temporary changes to test locally as it has been addressed in #4892 . It will be removed once merged/rebased;

pkg/cloud/services/ec2/instances.go

api/v1beta2/types.go

mtulio · 2024-04-29T22:21:46Z

prevent WIP notifications
/uncc Ankitasw dlipovetsky

mtulio · 2024-04-30T05:44:29Z

/test pull-cluster-api-provider-aws-e2e

k8s-ci-robot · 2024-04-30T05:44:44Z

@mtulio: Cannot trigger testing until a trusted user reviews the PR and leaves an /ok-to-test message.

In response to this:

/test pull-cluster-api-provider-aws-e2e

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

mtulio · 2024-04-30T05:46:31Z

HI @damdo @nrb - can I have labels to test in this PR? Thanks

damdo · 2024-04-30T07:22:23Z

/ok-to-test

mtulio · 2024-05-15T13:10:59Z

Hey @mtulio thanks a lot for this PR! I've left some comments, mostly nitpicks, catching typos, and some questions, nothing really wrong with it! :)

Hello @damdo , thanks a lot for your review, good suggestions. All applied. I am leaving only two comments "unresolved" to further discussion. Let me know your thoughts.

vr4manta · 2024-05-15T13:17:31Z

/lgtm

k8s-ci-robot · 2024-05-15T13:17:35Z

@vr4manta: changing LGTM is restricted to collaborators

In response to this:

/lgtm

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

mtulio · 2024-05-16T17:10:14Z

Hi CAPA maintainers, I will be OoO in the next three weeks, please also ping @rvanderp3 to align/address items in your review in this PR. Thanks.

damdo

Thanks for addressing feedback @mtulio
Changes
/lgtm

damdo · 2024-05-23T17:03:53Z

Let's get an approver review.
Could any of you PTAL
/assign @richardcase @Ankitasw @dlipovetsky @vincepri @nrb

Thanks!

nrb · 2024-05-28T17:11:30Z

api/v1beta2/awsmachine_webhook.go

+ if !strings.HasPrefix(*eipp.PublicIpv4Pool, awsPublicIpv4PoolPrefix) {
+ allErrs = append(allErrs, field.Invalid(field.NewPath("spec.elasticIpPool.publicIpv4Pool"), r.Spec.ElasticIPPool, fmt.Sprintf("publicIpv4Pool must start with %s.", awsPublicIpv4PoolPrefix)))
+ }
+ }


Do we need the eipp.PublicIpv4Pool == nil && eipp.PublicIpv4PoolFallBackOrder != nil case here?

nrb · 2024-05-28T17:13:13Z

api/v1beta2/network_types.go

@@ -477,6 +483,25 @@ func (v *VPCSpec) IsIPv6Enabled() bool {
 return v.IPv6 != nil
 }

+// GetElasticIPPool returns the custom Elastic IP Pool configuration when present.
+func (v *VPCSpec) GetElasticIPPool() *ElasticIPPool {
+ if v.ElasticIPPool == nil {


Could we just return v.ElasticIPPool here, and eliminate the if statement?

nrb · 2024-05-28T17:15:11Z

api/v1beta2/network_types.go

+ // resource ID starts with 'ipv4pool-ec2'.
+ //
+ // +kubebuilder:validation:MaxLength=30
+ // +kubebuilder:validation:XValidation:rule="self.startsWith('ipv4pool-ec2-')",message="publicIpv4Pool must start with 'ipv4pool-ec2-'"


This validation is present as an XValidation rule and in the webhook; do we need both?

nrb · 2024-05-28T17:18:01Z

docs/book/src/topics/bring-your-own-aws-infrastructure.md

+Bringing your own Public IPv4 Pool (BYOIPv4) can be used as an alternative to buying Public IPs from AWS, also considering the changes in charging for this since February 2024[2].
+
+Supported resources to BYO Public IPv4 Pool (`BYO Public IPv4`):
+- Nat Gateways


Suggested change

- Nat Gateways

- NAT Gateways

nrb · 2024-05-28T17:20:40Z

pkg/cloud/services/ec2/instances_test.go

@@ -2141,7 +2141,7 @@ func TestCreateInstance(t *testing.T) {
 AvailabilityZone: aws.String("us-east-1b"),
 MapPublicIpOnLaunch: aws.Bool(true),
 }},
- }, nil)
+ }, nil).MaxTimes(2)


Why set this max? To fail earlier?

I believe the mock was failing due to it being called more than once. I believe the intent here was address that since it is supposed to be called twice.

k8s-ci-robot · 2024-06-03T19:29:11Z

New changes are detected. LGTM label has been removed.

linux-foundation-easycla · 2024-06-03T19:29:12Z

The committers listed above are authorized under a signed CLA.

✅ login: mtulio / name: Marco Braga (c27ddf6)
✅ login: jcpowermac / name: Joseph Callen (a62b90d)

k8s-ci-robot · 2024-06-03T19:29:17Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please ask for approval from ankitasw. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Introducing support of BYO Public IPv4 Pool to allow CAPA allocate IPv4 Elastic IPs from user-provided IPv4 pools that was brought to AWS when provisioning base cluster infrastructure. This change introduce the API fields: - AWSCLuster NetworkSpec.ElasticIPPool: allowing the controllers to consume from user-provided public pools when provisioning core components from the infrastructure, like Nat Gateways and public Network Load Balancer (API server only) - AWSMachine ElasticIPPool: allowing the machine to consume from BYO Public IPv4 pool when the instance is deployed in the public subnets. The ElasticIPPool structure defines a custom IPv4 Pool (previously created in the AWS Account) to teach controllers to set the pool when creating public ip addresses (Elastic IPs) for components which requires it, such as Nat Gateways and NLBs.

jcpowermac · 2024-06-04T12:10:11Z

/test pull-cluster-api-provider-aws-test

k8s-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Apr 3, 2024

k8s-ci-robot requested review from Ankitasw and dlipovetsky April 3, 2024 19:14

mtulio mentioned this pull request Apr 3, 2024

SPLAT-1437: CAPI/AWS/BYOIP: supporting Public IPv4 Pool openshift/installer#8175

Draft

mtulio commented Apr 3, 2024

View reviewed changes

pkg/cloud/services/ec2/instances.go Outdated Show resolved Hide resolved

pkg/cloud/services/ec2/instances.go Outdated Show resolved Hide resolved

api/v1beta2/types.go Outdated Show resolved Hide resolved

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 4, 2024

mtulio force-pushed the SPLAT-1437-capa-byoip branch from dad9f3c to 994196a Compare April 29, 2024 22:08

k8s-ci-robot removed request for dlipovetsky and Ankitasw April 29, 2024 22:21

k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 30, 2024

mtulio force-pushed the SPLAT-1437-capa-byoip branch from 994196a to fa1f162 Compare April 30, 2024 05:36

k8s-ci-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Apr 30, 2024

mtulio force-pushed the SPLAT-1437-capa-byoip branch from fa1f162 to 4c41955 Compare April 30, 2024 05:46

k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Apr 30, 2024

mtulio force-pushed the SPLAT-1437-capa-byoip branch from d1af290 to 73a2e51 Compare May 15, 2024 13:08

damdo reviewed May 23, 2024

View reviewed changes

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 23, 2024

k8s-ci-robot assigned Ankitasw, dlipovetsky, richardcase and vincepri May 23, 2024

nrb reviewed May 28, 2024

View reviewed changes

k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jun 3, 2024

k8s-ci-robot added cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. and removed cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Jun 3, 2024

jcpowermac force-pushed the SPLAT-1437-capa-byoip branch from 4bba41a to 3648331 Compare June 3, 2024 19:46

mtulio and others added 2 commits June 3, 2024 15:59

Resolve issues from comments

a62b90d

k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. and removed cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. labels Jun 4, 2024

jcpowermac force-pushed the SPLAT-1437-capa-byoip branch from 3648331 to a62b90d Compare June 4, 2024 14:03

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

✨ Support BYO Public IPv4 Pool when provision infrastructure #4905

✨ Support BYO Public IPv4 Pool when provision infrastructure #4905

mtulio commented Apr 3, 2024 •

edited

k8s-ci-robot commented Apr 3, 2024

mtulio left a comment

mtulio commented Apr 29, 2024

mtulio commented Apr 30, 2024

k8s-ci-robot commented Apr 30, 2024

mtulio commented Apr 30, 2024 •

edited

damdo commented Apr 30, 2024

mtulio commented May 15, 2024

vr4manta commented May 15, 2024

k8s-ci-robot commented May 15, 2024

mtulio commented May 16, 2024

damdo left a comment

damdo commented May 23, 2024

nrb May 28, 2024

nrb May 28, 2024

nrb May 28, 2024

nrb May 28, 2024

nrb May 28, 2024

rvanderp3 Jun 3, 2024

k8s-ci-robot commented Jun 3, 2024

linux-foundation-easycla bot commented Jun 3, 2024 •

edited

k8s-ci-robot commented Jun 3, 2024

jcpowermac commented Jun 4, 2024

✨ Support BYO Public IPv4 Pool when provision infrastructure #4905

Are you sure you want to change the base?

✨ Support BYO Public IPv4 Pool when provision infrastructure #4905

Conversation

mtulio commented Apr 3, 2024 • edited

k8s-ci-robot commented Apr 3, 2024

mtulio left a comment

Choose a reason for hiding this comment

mtulio commented Apr 29, 2024

mtulio commented Apr 30, 2024

k8s-ci-robot commented Apr 30, 2024

mtulio commented Apr 30, 2024 • edited

damdo commented Apr 30, 2024

mtulio commented May 15, 2024

vr4manta commented May 15, 2024

k8s-ci-robot commented May 15, 2024

mtulio commented May 16, 2024

damdo left a comment

Choose a reason for hiding this comment

damdo commented May 23, 2024

nrb May 28, 2024

Choose a reason for hiding this comment

nrb May 28, 2024

Choose a reason for hiding this comment

nrb May 28, 2024

Choose a reason for hiding this comment

nrb May 28, 2024

Choose a reason for hiding this comment

nrb May 28, 2024

Choose a reason for hiding this comment

rvanderp3 Jun 3, 2024

Choose a reason for hiding this comment

k8s-ci-robot commented Jun 3, 2024

linux-foundation-easycla bot commented Jun 3, 2024 • edited

k8s-ci-robot commented Jun 3, 2024

jcpowermac commented Jun 4, 2024

mtulio commented Apr 3, 2024 •

edited

mtulio commented Apr 30, 2024 •

edited

linux-foundation-easycla bot commented Jun 3, 2024 •

edited