cluster-api-provider-aws
cluster-api-provider-aws copied to clipboard
kubernetes-sigs
✨ Support BYO Public IPv4 Pool when provision infrastructure
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:
- [x] https://github.com/kubernetes-sigs/cluster-api-provider-aws/pull/4899
- [x] https://github.com/kubernetes-sigs/cluster-api-provider-aws/pull/4900
Which issue(s) this PR fixes
Fixes #4887
Special notes for your reviewer:
Checklist:
- [ ] squashed commits
- [ ] includes documentation (WIP)
- [ ] includes emojis
- [ ] adds unit tests
- [ ] adds or updates e2e tests (NA)
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.
[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 assign justinsb for approval. For more information see the Kubernetes Code Review Process.
The full list of commands accepted by this bot can be found here.
Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment
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: 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: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:
| Test name | Commit | Details | Required | Rerun command |
|---|---|---|---|---|
| pull-cluster-api-provider-aws-test | 4c41955afcb2e2bce07af1754e4c34226c07debe | link | true | /test pull-cluster-api-provider-aws-test |
| pull-cluster-api-provider-aws-verify | 4c41955afcb2e2bce07af1754e4c34226c07debe | link | true | /test pull-cluster-api-provider-aws-verify |
Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.
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. I understand the commands that are listed here.
Downstream OpenShift e2e is passing. The downstream is setting PublicIpv4Pool both for infrastructure (VPCSpec) and Machine consuming from an Public IPv4 Pool brought to CI account. Starting tests for regular e2e:
/test pull-cluster-api-provider-aws-e2e /test pull-cluster-api-provider-aws-e2e-eks
e2e is mostly passing (eks seems to be flake), added docs and fixed the verify:
/test pull-cluster-api-provider-aws-e2e /test pull-cluster-api-provider-aws-e2e-eks
This PR is ready for review. We have some spots to improve the unit tests, but I would like to know if this approach is feasile by maitainers to cover the request #4887 .
The e2e had timeouts, e2e-eks pass, trying again with fix in verify.
/test pull-cluster-api-provider-aws-e2e /test pull-cluster-api-provider-aws-e2e-eks
Please take a look? /cc @damdo @nrb @r4f4
/test pull-cluster-api-provider-aws-e2e /test pull-cluster-api-provider-aws-e2e-eks
The commits/changes I am performing is improving/adding unit tests, there is not block for review.
@damdo @nrb Would you mind adding it to the review queue, please?
FWIW I just ran a chaos scenario by fulfilling the custom CIDR pool and run the installation with the fallback to Amazon-pool. I found one better approach to check the fallback, just fixed in the last commit. The details of the tests and results are available in this comment: https://github.com/openshift/installer/pull/8175#issuecomment-2111229833
/test pull-cluster-api-provider-aws-e2e /test pull-cluster-api-provider-aws-e2e-eks
It looks like the e2e failed on teardown:
/test pull-cluster-api-provider-aws-e2e
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: 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.
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.
Let's get an approver review. Could any of you PTAL /assign @richardcase @Ankitasw @dlipovetsky @vincepri @nrb
Thanks!
The committers listed above are authorized under a signed CLA.
- :white_check_mark: login: mtulio / name: Marco Braga (2f230b38348af971c2bb4f6214d620f8774f1e1a)
![linux-foundation-easycla[bot] avatar](https://avatars.githubusercontent.com/in/17893?v=4 "Published by a pub.dev verified publisher"){kind=small}
/test pull-cluster-api-provider-aws-e2e-eks /test pull-cluster-api-provider-aws-e2e /test pull-cluster-api-provider-aws-e2e-clusterclass /test pull-cluster-api-provider-aws-e2e-conformance
Thanks @jcpowermac and @rvanderp3 for taking care of this PR addressing the review while I was out.
/test pull-cluster-api-provider-aws-e2e-eks /test pull-cluster-api-provider-aws-e2e /test pull-cluster-api-provider-aws-e2e-clusterclass /test pull-cluster-api-provider-aws-e2e-conformance
Hey @damdo - apologies I killed the jobs while addressing this comment. Let me trigger again:
/test pull-cluster-api-provider-aws-e2e-eks /test pull-cluster-api-provider-aws-e2e /test pull-cluster-api-provider-aws-e2e-clusterclass /test pull-cluster-api-provider-aws-e2e-conformance
/test pull-cluster-api-provider-aws-e2e-eks
Unrelated issue:
STEP: Event details for AWSIAMUserBootstrapper : Resource: AWS::IAM::User, Status: CREATE_FAILED, Reason: Resource handler returned message: "Resource of type 'AWS::IAM::User' with identifier 'bootstrapper.cluster-api-provider-aws.sigs.k8s.io' already exists." (RequestToken: fb13356d-5ac6-1c5f-1b83-6ea7aad87fb4, HandlerErrorCode: AlreadyExists) @ 06/07/24 23:02:31.865
STEP: Event details for cluster-api-provider-aws-sigs-k8s-io : Resource: AWS::CloudFormation::Stack, Status: ROLLBACK_IN_PROGRESS, Reason: The following resource(s) failed to create: [AWSIAMManagedPolicyControllers, AWSIAMInstanceProfileControllers, AWSIAMInstanceProfileNodes, AWSIAMInstanceProfileControlPlane, AWSIAMManagedPolicyControllersEKS, AWSIAMUserBootstrapper]. Rollback requested by user. @ 06/07/24 23:02:31.865
Looking the IAM resource name "Resource of type 'AWS::IAM::User' with identifier 'bootstrapper.cluster-api-provider-aws.sigs.k8s.io' already exists it seems not to be specific for this job run, it makes me wonder if that job supports parallel executions. cc @damdo @nrb
I will trigger again with a, hopefully, low traffic period (Friday night =] ).
/test pull-cluster-api-provider-aws-e2e-eks
[APPROVALNOTIFIER] This PR is APPROVED
This pull-request has been approved by: nrb
The full list of commands accepted by this bot can be found here.
The pull request process is described here
- ~~OWNERS~~ [nrb]
Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment