[WIP] Add feature for CNI Custom Networking #786
Conversation
| @@ -10,9 +11,9 @@ import ( | |||
| "github.com/weaveworks/eksctl/pkg/vpc" | |||
| ) | |||
|
|
|||
| func (c *ClusterResourceSet) addSubnets(refRT *gfn.Value, topology api.SubnetTopology, subnets map[string]api.Network) { | |||
| func (c *ClusterResourceSet) addSubnets(refRT *gfn.Value, topology api.SubnetTopology, subnets map[string]api.Network, kind string) { | |||
There was a problem hiding this comment.
This function changed a bit (see #776), please rebase :)
| for az, subnet := range subnets { | ||
| alias := string(topology) + strings.ToUpper(strings.Join(strings.Split(az, "-"), "")) | ||
| alias := string(kind) + string(topology) + strings.ToUpper(strings.Join(strings.Split(az, "-"), "")) |
There was a problem hiding this comment.
We will have to maintain names of existing keys as they are, so please only add prefix for new subnets... That is because logical names inside the stack need to remain the same for our append-only stack update code to function as expected, but also in general that is essential in CloudFormation.
| @@ -21,6 +23,8 @@ type ( | |||
| // for additional CIDR associations, e.g. to use with separate CIDR for | |||
| // private subnets or any ad-hoc subnets | |||
| // +optional | |||
| PodSubnets map[string]*CustomSubnets `json:"podSubnets,omitempty"` | |||
There was a problem hiding this comment.
Do you think we could just add this as a new topology under ClusterSubnets?
There was a problem hiding this comment.
@errordeveloper As in PodSubnets under ClusterSubnets?
| @@ -21,6 +23,8 @@ type ( | |||
| // for additional CIDR associations, e.g. to use with separate CIDR for | |||
There was a problem hiding this comment.
this comment belongs to ExtraCIDRs... please replace it with a comment to say that it need to be deprecated.
| @@ -114,3 +116,29 @@ func MustParseCIDR(s string) *IPNet { | |||
| } | |||
| return cidr | |||
| } | |||
|
|
|||
| // SplitIntoN splits the parent IPNet into n subnets | |||
| func SplitIntoN(parent *net.IPNet, n int) ([]*net.IPNet, error) { | |||
There was a problem hiding this comment.
Do you want to migrate to this and copy the unit tests from kops? (I think they had a few there).
There was a problem hiding this comment.
But I'm not sure if needs to be SplitIntoN, SplitInto8 and SplitInto4 is all we really need. As it must be N^2 , and SplitInto2 would be of very little use, and I'm not sure of when SplitInto16 would be applicable...
| Mask: net.CIDRMask(networkLength, 32), | ||
| }) | ||
| } else { | ||
| return nil, fmt.Errorf("Unexpected IP address type: %s", parent) |
There was a problem hiding this comment.
the convention is that error string always begin with lower-case...
|
@tiffanyfay ping :) |
|
@errordeveloper Hey, have a big deadline for next Fri so going to be heads down on that and then will be back on this! I kept thinking I needed to message, but kept forgetting. |
|
@tiffanyfay @errordeveloper is anyone still working on this? I have a need for it soon, if it's not being actively developed I'd like to work on a PR for it. I also think it would be good to have a feature issue outlining exactly how it will work, I don't mind writing that up as well. |
|
@lilley2412 Yeah, I'm going to be working on it next week. We can definitely discuss it! |
|
@errordeveloper @tiffanyfay FYI, opened an issue to propose how the feature might work for discussion #1096 |
|
@tiffanyfay I am closing this due to lack of activity and because we don't have this in our short-term backlog but please feel free to reopen it if/when you'd like to keep working on it! |
Prep for Windows support: Copy pkg/mounter and refactor to use k8s.io/mount-utils
Description
WIP
https://docs.aws.amazon.com/eks/latest/userguide/cni-custom-network.html
Checklist
make build)make test)make integration-test)README.md, andexamplesdirectory)humans.txtfile