Skip to content
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

configure network interfaces #2026

Closed
universam1 opened this issue Jun 30, 2022 · 21 comments · Fixed by #5437
Closed

configure network interfaces #2026

universam1 opened this issue Jun 30, 2022 · 21 comments · Fixed by #5437
Labels
api Issues that require API changes feature New feature or request needs-design Design required

Comments

@universam1
Copy link

universam1 commented Jun 30, 2022

Tell us about your request
configure the network interfaces. In particular enable AssociatePublicIpAddress

Tell us about the problem you're trying to solve. What are you trying to do, and why is it hard?
Our particular requirement is to associate public IPs on some node's interfaces for UDP ingress where there is no LB support avail.
We do not want to enable public IPs an all nodes, that's why we cannot use a public subnet.

Are you currently working around this issue?
None, docs even state "Do not configure network instances in the launch template"

Community Note

  • Please vote on this issue by adding a 👍 reaction to the original issue to help the community and maintainers prioritize this request
  • Please do not leave "+1" or "me too" comments, they generate extra noise for issue followers and do not help prioritize the request
  • If you are interested in working on this issue or have submitted a pull request, please leave a comment
@universam1 universam1 added the feature New feature or request label Jun 30, 2022
@bwagner5
Copy link
Contributor

bwagner5 commented Jul 5, 2022

Thanks for raising this issue! The only way to do this today would be to create a public subnet w/ auto-assign IPs and use a subnet selector in an isolated provisioner and then have the pods select that provisioner.

We could build this option into the Cloud Provider as an option, but you'd still need to use a separate Provisioner that pods would need to select. I know you said it's not possible to use a public subnet, is that due to IP space utilization or some other issue with that approach?

@universam1
Copy link
Author

Thank you for your response and checking back!

In fact after realizing that there is not going to be a low hanging solution available, we are investigating the option of public subnet with auto-assign public IPs again. The motivation for per node configuration was to be explicit, limit the scope of exposed nodes, and support other subnet members (ex. EC2 instances) w/o public exposure.

As you mentioned we can also solve this problem by logical segregation, requires infra changes. From our side it looks like we can solve it this way so it is not a hard requirement any more, but it would be a great feature anyway.
Thanks!

@devopsmash
Copy link

It would be amazing to have this feature of AssociatePublicIpAddress.
We have a use case that for some of our public servers we need Elastic IPs (static IPs), and for the rest of our public servers, we need the regular public IPs.

@ellistarn
Copy link
Contributor

I'm supportive of exposing https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-properties-ec2-launchtemplate-networkinterface.html through the AWSNodeTemplate. Anyone interested in picking this up?

@ellistarn ellistarn added the good-first-issue Good for newcomers label Jan 10, 2023
@nami-su
Copy link

nami-su commented Jan 28, 2023

@ellistarn I can give it a try

@ellistarn
Copy link
Contributor

Looking into this a little bit. Is there a reason you can't configure this on your subnet? https://docs.aws.amazon.com/vpc/latest/userguide/working-with-subnets.html#subnet-public-ip.

This parameter is a field on launchtemplatedata.networkInterfaces. Instead of placing this field at the top level, is there a way to logically map to the underlying LT. Should we expose a networkinterfaces struct? @bwagner5

@bwagner5
Copy link
Contributor

The NetworkInterfaces struct is probably the best way to expose this and solves other problems with EFA and potentially reducing CNI interface allocations at run-time.

#3369
#3127

@stevehipwell
Copy link
Contributor

Would it be possible to configure ENIs based on inputs (IDs or tags) so we could specify explicit subnets to work across AZs? I have two scenarios where the network interface needs customising.

  • For Cilium in custom networking mode an ENI from the subnet needs to exist on the node
  • For an IPv6 EKS clusters adding an explicit IPv6 IP to the ENI removes the requirement for the documented invasive VPC changes

@stevehipwell
Copy link
Contributor

I'm just re-reading the docs and I'm not 100% sure this is essential for Cilium as the docs aren't completely clear on this; I'm just about to test it though. I do think that in all secondary networking scenarios explicitly adding the ENIs at instance creation time should have a positive impact on node start-up.

@myaser
Copy link
Contributor

myaser commented Apr 24, 2023

@ellistarn & @bwagner5

I can work on the networkinterfaces struct

@lorrrrrrrenzo
Copy link
Contributor

I just created this Issue, #3815, and wrote a PoC that might be useful here. Of course, feel free to reference it:

main...lorenzadia:karpenter:lorenzo.explictly-disable-public-ipaddr-in-launch-template

@jonathan-innis jonathan-innis added needs-design Design required api Issues that require API changes and removed good-first-issue Good for newcomers labels Apr 28, 2023
@devopsmash
Copy link

Thanks for raising this issue! The only way to do this today would be to create a public subnet w/ auto-assign IPs and use a subnet selector in an isolated provisioner and then have the pods select that provisioner.

We could build this option into the Cloud Provider as an option, but you'd still need to use a separate Provisioner that pods would need to select. I know you said it's not possible to use a public subnet, is that due to IP space utilization or some other issue with that approach?

It seems that "automatically assign public IP addresses in subnet" is severity MEDIUM in AWS Security Hub

image

@vara-bonthu
Copy link

vara-bonthu commented Aug 14, 2023

Hey @myaser, it's great to see the pull request addressing this feature. 👍

@ellistarn @bwagner5 @jonathan-innis The ability to configure network interfaces is becoming increasingly important, especially for Machine Learning workloads.

Currently, we're achieving this by utilizing Managed Node Groups with Launch Templates. However, considering the significance of this functionality, it would be highly beneficial for users if we prioritize its implementation. This will provide users the capability to configure multiple network interfaces with EFA enabled in AWS Node templates.

Looking forward to the progress on this feature!

@billrayburn billrayburn added the v1 Issues requiring resolution by the v1 milestone label Sep 27, 2023
@billrayburn billrayburn removed the v1 Issues requiring resolution by the v1 milestone label Nov 8, 2023
@garvinp-stripe
Copy link
Contributor

Currently, we run nodes that uses our own CNI. This CNI expects a secondary ENI (and doesn't dynamically add new ENIs) and utilize that ENI for pod networking. Its a pretty unique setup so I don't expect too many other users run into this. Because of this we need to be able to define ENIs in the launch template (EC2NodeClass).

@jmdeal
Copy link
Contributor

jmdeal commented Feb 13, 2024

Reopenning since the PR does not address the full scope of this issue.

@jmdeal jmdeal reopened this Feb 13, 2024
@jonathan-innis
Copy link
Contributor

jonathan-innis commented Feb 14, 2024

We can check with @universam1, but I think this may resolve the entire feature request. From reading through the description, it sounds like the original ask was that we just support AssociatePublicIpAddress. We are doing that now, so it seems reasonable to me that we could probably go ahead and close this issue.

@universam1 Thoughts?

@jmdeal
Copy link
Contributor

jmdeal commented Feb 14, 2024

I think it does address the entire feature as originally requested but some additional discussion increased the scope to more general configuration. I could see either continuing that here or opening a new issue to track.

@arditti
Copy link

arditti commented Mar 19, 2024

There was already a wider-scope PR 3314 that was closed as it was suggested to be handled over this issue

@jonathan-innis
Copy link
Contributor

There was already a wider-scope PR #3314 that was closed as it was suggested to be handled over this issue

@arditti We did merge that PR which now allows users to directly set associatePublicIPAddress in their EC2NodeClass

@jonathan-innis
Copy link
Contributor

I could see either continuing that here or opening a new issue to track

I'm more in favor of a new issue to track, I'd rather capture specific use-cases rather than capture an entire implementation detail. If we leave this issue open in this state, I suspect this may be open for a while, specifically because we may not ever support all of the Network Interfaces struct.

I'm going to close out this issue.

@garvinp-stripe @vara-bonthu @stevehipwell Since y'all had specific use-cases that were outside of the "associatePublicIPAddress" case, if y'all still need your use-cases addressed, please open separate issues that describe your use-case.

@stevehipwell
Copy link
Contributor

@jonathan-innis I'll open new issues if required when we revisit Cilium and IPv6.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api Issues that require API changes feature New feature or request needs-design Design required
Projects
None yet