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

[WIP] Add feature for CNI Custom Networking #786

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions pkg/apis/eksctl.io/v1alpha5/vpc.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ type (
// +optional
Network `json:",inline"` // global CIDR and VPC ID
// +optional
PodCIDRs []*ipnet.IPNet `json:"podCIDRs,omitempty"`
// +optional
SecurityGroup string `json:"securityGroup,omitempty"` // cluster SG
// subnets are either public or private for use with separate nodegroups
// these are keyed by AZ for convenience
Expand All @@ -21,6 +23,8 @@ type (
// for additional CIDR associations, e.g. to use with separate CIDR for
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this comment belongs to ExtraCIDRs... please replace it with a comment to say that it need to be deprecated.

// private subnets or any ad-hoc subnets
// +optional
PodSubnets map[string]*CustomSubnets `json:"podSubnets,omitempty"`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you think we could just add this as a new topology under ClusterSubnets?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@errordeveloper As in PodSubnets under ClusterSubnets?

// +optional
ExtraCIDRs []*ipnet.IPNet `json:"extraCIDRs,omitempty"`
// for pre-defined shared node SG
SharedNodeSecurityGroup string `json:"sharedNodeSecurityGroup,omitempty"`
Expand All @@ -39,6 +43,12 @@ type (
// +optional
CIDR *ipnet.IPNet `json:"cidr,omitempty"`
}

// PodSubnets holds the pod VPC CIDR and subnets
CustomSubnets struct {
CIDR *ipnet.IPNet `json:"cidr,omitempty"`
Subnets *ClusterSubnets `json:"subnets,omitempty"`
}
)

const (
Expand Down
50 changes: 50 additions & 0 deletions pkg/apis/eksctl.io/v1alpha5/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

45 changes: 37 additions & 8 deletions pkg/cfn/builder/vpc.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package builder

import (
"fmt"
"strings"

gfn "github.com/awslabs/goformation/cloudformation"
Expand All @@ -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) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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, "-"), ""))
Copy link
Contributor

@errordeveloper errordeveloper May 13, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

subnet := &gfn.AWSEC2Subnet{
AvailabilityZone: gfn.NewString(az),
CidrBlock: gfn.NewString(subnet.CIDR.String()),
Expand All @@ -31,10 +32,21 @@ func (c *ClusterResourceSet) addSubnets(refRT *gfn.Value, topology api.SubnetTop
}}
}
refSubnet := c.newResource("Subnet"+alias, subnet)
c.newResource("RouteTableAssociation"+alias, &gfn.AWSEC2SubnetRouteTableAssociation{
SubnetId: refSubnet,
RouteTableId: refRT,
})
if alias == "" {
c.newResource("RouteTableAssociation"+alias, &gfn.AWSEC2SubnetRouteTableAssociation{
SubnetId: refSubnet,
RouteTableId: refRT,
})
} else {
c.newResource("RouteTableAssociation"+alias, &awsCloudFormationResource{
Type: "AWS::EC2::SubnetRouteTableAssociation",
Properties: map[string]interface{}{
"SubnetId": refSubnet,
"RouteTableId": refRT,
},
DependsOn: []string{alias},
})
}
c.subnets[topology] = append(c.subnets[topology], refSubnet)
}
}
Expand All @@ -49,6 +61,17 @@ func (c *ClusterResourceSet) addResourcesForVPC() {
EnableDnsHostnames: gfn.True(),
})

for i, podCIDR := range c.spec.VPC.PodCIDRs {
c.newResource(fmt.Sprintf("PodCIDR%d", i), &awsCloudFormationResource{
Type: "AWS::EC2::VPCCidrBlock",
Properties: map[string]interface{}{
"CidrBlock": podCIDR.String(),
"VpcId": c.vpc,
},
DependsOn: []string{"VPC"},
})
}

c.subnets = make(map[api.SubnetTopology][]*gfn.Value)

refIG := c.newResource("InternetGateway", &gfn.AWSEC2InternetGateway{})
Expand All @@ -67,7 +90,7 @@ func (c *ClusterResourceSet) addResourcesForVPC() {
GatewayId: refIG,
})

c.addSubnets(refPublicRT, api.SubnetTopologyPublic, c.spec.VPC.Subnets.Public)
c.addSubnets(refPublicRT, api.SubnetTopologyPublic, c.spec.VPC.Subnets.Public, "")

c.newResource("NATIP", &gfn.AWSEC2EIP{
Domain: gfn.NewString("vpc"),
Expand All @@ -89,7 +112,13 @@ func (c *ClusterResourceSet) addResourcesForVPC() {
NatGatewayId: refNG,
})

c.addSubnets(refPrivateRT, api.SubnetTopologyPrivate, c.spec.VPC.Subnets.Private)
c.addSubnets(refPrivateRT, api.SubnetTopologyPrivate, c.spec.VPC.Subnets.Private, "")

// TODO add specific name for pod subnets
for i := range c.spec.VPC.PodCIDRs {

c.addSubnets(refPrivateRT, api.SubnetTopologyPrivate, c.spec.VPC.PodSubnets[fmt.Sprintf("eksctlGroup%d", i)].Subnets.Private, fmt.Sprintf("PodCIDR%d", i))
}
}

func (c *ClusterResourceSet) importResourcesForVPC() {
Expand Down
28 changes: 28 additions & 0 deletions pkg/utils/ipnet/ipnet.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,9 @@ package ipnet
// TODO: this is not ideal, we should move this out or do something else about it.

import (
"encoding/binary"
"encoding/json"
"fmt"
"net"
"reflect"

Expand Down Expand Up @@ -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) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you want to migrate to this and copy the unit tests from kops? (I think they had a few there).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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...

networkLength, _ := parent.Mask.Size()
networkLength += 3

var subnets []*net.IPNet
for i := 0; i < n; i++ {
ip4 := parent.IP.To4()
if ip4 != nil {
n := binary.BigEndian.Uint32(ip4)
n += uint32(i) << uint(32-networkLength)
subnetIP := make(net.IP, len(ip4))
binary.BigEndian.PutUint32(subnetIP, n)

subnets = append(subnets, &net.IPNet{
IP: subnetIP,
Mask: net.CIDRMask(networkLength, 32),
})
} else {
return nil, fmt.Errorf("Unexpected IP address type: %s", parent)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the convention is that error string always begin with lower-case...

}
}

return subnets, nil
}
45 changes: 40 additions & 5 deletions pkg/vpc/vpc.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ func SetSubnets(spec *api.ClusterConfig) error {
var err error

vpc := spec.VPC
vpc.PodSubnets = make(map[string]*api.CustomSubnets)
vpc.Subnets = &api.ClusterSubnets{
Private: map[string]api.Network{},
Public: map[string]api.Network{},
Expand All @@ -31,10 +32,16 @@ func SetSubnets(spec *api.ClusterConfig) error {
cidr := api.DefaultCIDR()
vpc.CIDR = &cidr
}
prefix, _ := spec.VPC.CIDR.Mask.Size()
if (prefix < 16) || (prefix > 24) {
return fmt.Errorf("VPC CIDR prefix must be betwee /16 and /24")

cidrs := append(spec.VPC.PodCIDRs, vpc.CIDR)
for _, cidr := range cidrs {
prefix, _ := cidr.Mask.Size()
if (prefix < 16) || (prefix > 28) {
return fmt.Errorf("VPC CIDR (%v) prefix must be between /16 and /28", cidr)
}
}

// Create subnets for VPC CIDR
zoneCIDRs, err := subnet.SplitInto8(&spec.VPC.CIDR.IPNet)
if err != nil {
return err
Expand All @@ -59,6 +66,34 @@ func SetSubnets(spec *api.ClusterConfig) error {
logger.Info("subnets for %s - public:%s private:%s", zone, public.String(), private.String())
}

// Create subnets for pod CIDRs
for c, podCIDR := range spec.VPC.PodCIDRs {
logger.Info("pod CIDR %s", podCIDR.String()) // TODO delete
zoneCIDRs, err := ipnet.SplitIntoN(&podCIDR.IPNet, 4)
if err != nil {
return err
}

if zonesTotal > len(zoneCIDRs) {
return fmt.Errorf("insufficient number of subnets (have %d, but need %d) for %d availability zones", len(zoneCIDRs), 2*zonesTotal, zonesTotal)
}

podSubnets := &api.CustomSubnets{
Subnets: &api.ClusterSubnets{
Private: make(map[string]api.Network),
},
}
vpc.PodSubnets[fmt.Sprintf("eksctlGroup%d", c)] = podSubnets

for i, zone := range spec.AvailabilityZones {
private := zoneCIDRs[i]
vpc.PodSubnets[fmt.Sprintf("eksctlGroup%d", c)].Subnets.Private[zone] = api.Network{
CIDR: &ipnet.IPNet{IPNet: *private},
}
logger.Info("pod subnet for %s - private:%s", zone, private.String())
}
}

return nil
}

Expand All @@ -73,11 +108,11 @@ func describeSubnets(provider api.ClusterProvider, subnetIDs ...string) ([]*ec2.
return output.Subnets, nil
}

func describe(povider api.ClusterProvider, vpcID string) (*ec2.Vpc, error) {
func describe(provider api.ClusterProvider, vpcID string) (*ec2.Vpc, error) {
input := &ec2.DescribeVpcsInput{
VpcIds: []*string{aws.String(vpcID)},
}
output, err := povider.EC2().DescribeVpcs(input)
output, err := provider.EC2().DescribeVpcs(input)
if err != nil {
return nil, err
}
Expand Down