Skip to content

Conversation

@aaroniscode
Copy link
Contributor

@aaroniscode aaroniscode commented Jul 9, 2020

What this PR does / why we need it:
Until the code is refactored in #1792 , this PR fixes the NAT gateway issue where creation fails because it assigned an EIP that is already bound to a different NAT gateway. The PR tries to keep the exact same workflow and steps but just batch creates the EIPs before creating the NAT gateways.

Changes/Additions:

Created a createNatGateways function that batch creates all the NAT gateways while keeping the createNatGateway function intact as much as possible
Changed getOrAllocateAddress to getOrAllocateAddresses to batch find/create all the EIPs before the NAT gateways are created

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #1752

Note I need to test this tomorrow because I had to resolve a merge conflict with the recent NAT gateway status condition code change.

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Jul 9, 2020
@k8s-ci-robot k8s-ci-robot requested review from detiber and vincepri July 9, 2020 03:03
@k8s-ci-robot k8s-ci-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Jul 9, 2020
@aaroniscode
Copy link
Contributor Author

so looks like I broke something when rebasing my branch and fixing a merge conflict. I'll fix the test errors tomorrow. Let's see if there is consensus on the approach.

@randomvariable
Copy link
Member

/lgtm
/approve

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jul 9, 2020
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: randomvariable

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

The pull request process is described here

Needs approval from an approver in each of these files:

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

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 9, 2020
@randomvariable
Copy link
Member

Thanks @aaroniscode !

@randomvariable randomvariable added this to the v0.5.5 milestone Jul 9, 2020
@k8s-ci-robot k8s-ci-robot merged commit 5ddb728 into kubernetes-sigs:master Jul 9, 2020
@aaroniscode aaroniscode deleted the eip_fix_batch branch July 10, 2020 19:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Fails to create one of three NAT gateways

3 participants