Skip to content

add support for pods supported by IPv4Prefix on ENI #2137

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jul 27, 2021

Conversation

M00nF1sh
Copy link
Collaborator

@M00nF1sh M00nF1sh commented Jul 23, 2021

Description

EC2 recently introduced supports for IPv4Prefix on ENI, which will be supported by EKS VPC CNI soon.

This PR add support for pods supported by IPv4Prefix on ENI.

Previously, the pod ENI resolve logic works by filter ENIs in VPC that contains pod's IPv4Addresses. This approach don't work for IPv4Prefix case.
with this PR, we changed to described the instance on which the pod is running on, and find the ENI supporting the pod's IP address.

Checklist

  • Added tests that cover your change (if possible)
  • Added/modified documentation as required (such as the README.md, or the docs directory)
  • Manually tested
  • Made sure the title of the PR is a good description that can go into the release notes

BONUS POINTS checklist: complete for good vibes and maybe prizes?! 🤯

  • Backfilled missing tests for code in same general area 🎉
  • Refactored something and made the world a better place 🌟

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Jul 23, 2021
@k8s-ci-robot k8s-ci-robot requested a review from kishorj July 23, 2021 22:58
@M00nF1sh M00nF1sh changed the title add PD support add support for pods supported by prefix on ENI Jul 23, 2021
@M00nF1sh M00nF1sh changed the title add support for pods supported by prefix on ENI add support for pods supported by ipv4Prefix on ENI Jul 23, 2021
@M00nF1sh M00nF1sh changed the title add support for pods supported by ipv4Prefix on ENI add support for pods supported by IPv4Prefix on ENI Jul 23, 2021
@codecov-commenter
Copy link

codecov-commenter commented Jul 23, 2021

Codecov Report

Merging #2137 (7ea12b1) into main (4982070) will increase coverage by 0.44%.
The diff coverage is 37.14%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #2137      +/-   ##
==========================================
+ Coverage   49.59%   50.03%   +0.44%     
==========================================
  Files         130      131       +1     
  Lines        7084     7119      +35     
==========================================
+ Hits         3513     3562      +49     
+ Misses       3287     3273      -14     
  Partials      284      284              
Impacted Files Coverage Δ
pkg/networking/node_eni_info_resolver.go 14.28% <0.00%> (+2.52%) ⬆️
pkg/targetgroupbinding/resource_manager.go 18.41% <0.00%> (-0.24%) ⬇️
pkg/networking/pod_eni_info_resolver.go 14.56% <19.44%> (+6.87%) ⬆️
pkg/networking/node_info_provider.go 85.71% <85.71%> (ø)
pkg/k8s/pod_info.go 100.00% <100.00%> (ø)
pkg/service/model_build_load_balancer.go 84.39% <0.00%> (+1.24%) ⬆️
pkg/service/model_builder.go 93.75% <0.00%> (+2.57%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4982070...7ea12b1. Read the comment docs.

@M00nF1sh
Copy link
Collaborator Author

/hold

@k8s-ci-robot k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jul 23, 2021
@kishorj kishorj added the tide/merge-method-squash Denotes a PR that should be squashed by tide when it merges. label Jul 27, 2021
@kishorj
Copy link
Collaborator

kishorj commented Jul 27, 2021

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jul 27, 2021
@M00nF1sh
Copy link
Collaborator Author

/unhold

@k8s-ci-robot k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jul 27, 2021
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: fawadkhaliq, kishorj, M00nF1sh

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 merged commit 8f1c32c into kubernetes-sigs:main Jul 27, 2021
M00nF1sh added a commit to M00nF1sh/aws-load-balancer-controller that referenced this pull request Jul 28, 2021
@stevehipwell
Copy link
Contributor

stevehipwell commented Sep 17, 2021

@M00nF1sh did this make it into v2.2.4? this probably needs it's milestone changing as it looks like it was released in v2.2.2.

@kishorj
Copy link
Collaborator

kishorj commented Sep 17, 2021

@stevehipwell, we backported the fix to the v2.2 release since it was a critical fix.

@stevehipwell
Copy link
Contributor

Thanks @kishorj. I worked this out by checking the released code, which is why I was suggesting that the milestone be changed to make it easier for anyone else to see that.

alebedev87 pushed a commit to alebedev87/aws-load-balancer-controller that referenced this pull request Oct 26, 2023
Timothy-Dougherty pushed a commit to adammw/aws-load-balancer-controller that referenced this pull request Nov 9, 2023
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/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. tide/merge-method-squash Denotes a PR that should be squashed by tide when it merges.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants