Add --vpc-cidr flag

- automate splitting of subnets
- create private subnets as well as public

Makefile

@@ -24,9 +24,13 @@ build: ## Build eksctl
 test: generate ## Run unit tests
 	@git diff --exit-code pkg/nodebootstrap/assets.go > /dev/null || (git --no-pager diff; exit 1)
 	@git diff --exit-code ./pkg/eks/mocks > /dev/null || (git --no-pager diff; exit 1)
-	@CGO_ENABLED=0 go test -v -covermode=count -coverprofile=coverage.out ./pkg/... ./cmd/...
+	@$(MAKE) unit-test
 	@test -z $(COVERALLS_TOKEN) || $(GOPATH)/bin/goveralls -coverprofile=coverage.out -service=circle-ci
 
+.PHONY: unit-test
+unit-test:
+	@CGO_ENABLED=0 go test -v -covermode=count -coverprofile=coverage.out ./pkg/... ./cmd/...
+
 LINTER ?= gometalinter ./...
 .PHONY: lint
 lint: ## Run linter over the codebase

pkg/cfn/builder/api_test.go

@@ -13,6 +13,7 @@ import (
 	. "github.com/onsi/gomega"
 	. "github.com/weaveworks/eksctl/pkg/cfn/builder"
 	"github.com/weaveworks/eksctl/pkg/cloudconfig"
+	"github.com/weaveworks/eksctl/pkg/eks"
 	"github.com/weaveworks/eksctl/pkg/eks/api"
 	"github.com/weaveworks/eksctl/pkg/nodebootstrap"
 )
@@ -100,6 +101,8 @@ var _ = Describe("CloudFormation template builder API", func() {
 		cfg.AvailabilityZones = testAZs
 		ng.InstanceType = "t2.medium"
 
+		*cfg.VPC.CIDR = api.DefaultCIDR()
+
 		return cfg
 	}
 
@@ -115,7 +118,7 @@ var _ = Describe("CloudFormation template builder API", func() {
 			ARN:                      arn,
 			AvailabilityZones:        testAZs,
 
-			VPC: api.ClusterVPC{
+			VPC: &api.ClusterVPC{
 				Network: api.Network{
 					ID: "vpc-0e265ad953062b94b",
 					CIDR: &net.IPNet{
@@ -129,22 +132,45 @@ var _ = Describe("CloudFormation template builder API", func() {
 						"us-west-2b": {
 							ID: "subnet-0f98135715dfcf55f",
 							CIDR: &net.IPNet{
-								IP:   []byte{192, 168, 64, 0},
-								Mask: []byte{255, 255, 192, 0},
+								IP:   []byte{192, 168, 0, 0},
+								Mask: []byte{255, 255, 224, 0},
 							},
 						},
 						"us-west-2a": {
 							ID: "subnet-0ade11bad78dced9e",
 							CIDR: &net.IPNet{
-								IP:   []byte{192, 168, 128, 0},
-								Mask: []byte{255, 255, 192, 0},
+								IP:   []byte{192, 168, 32, 0},
+								Mask: []byte{255, 255, 224, 0},
 							},
 						},
 						"us-west-2c": {
 							ID: "subnet-0e2e63ff1712bf6ef",
 							CIDR: &net.IPNet{
-								IP:   []byte{192, 168, 192, 0},
-								Mask: []byte{255, 255, 192, 0},
+								IP:   []byte{192, 168, 64, 0},
+								Mask: []byte{255, 255, 224, 0},
+							},
+						},
+					},
+					"Private": map[string]api.Network{
+						"us-west-2b": {
+							ID: "subnet-0f98135715dfcf55a",
+							CIDR: &net.IPNet{
+								IP:   []byte{192, 168, 96, 0},
+								Mask: []byte{255, 255, 224, 0},
+							},
+						},
+						"us-west-2a": {
+							ID: "subnet-0ade11bad78dced9f",
+							CIDR: &net.IPNet{
+								IP:   []byte{192, 168, 128, 0},
+								Mask: []byte{255, 255, 224, 0},
+							},
+						},
+						"us-west-2c": {
+							ID: "subnet-0e2e63ff1712bf6ea",
+							CIDR: &net.IPNet{
+								IP:   []byte{192, 168, 160, 0},
+								Mask: []byte{255, 255, 224, 0},
 							},
 						},
 					},
@@ -159,30 +185,48 @@ var _ = Describe("CloudFormation template builder API", func() {
 			},
 		}
 
-		initial := newClusterConfig()
+		cfg := newClusterConfig()
+		ctl := eks.New(cfg)
 
-		initial.SetSubnets()
+		It("should not error when calling SetSubnets", func() {
+			err := ctl.SetSubnets()
+			Expect(err).ShouldNot(HaveOccurred())
+		})
 
-		rs := NewClusterResourceSet(initial)
-		rs.AddAllResources()
+		It("should have public and private subnets", func() {
+			Expect(len(cfg.VPC.Subnets)).To(Equal(2))
+			for _, k := range []api.SubnetTopology{"Public", "Private"} {
+				Expect(cfg.VPC.Subnets).To(HaveKey(k))
+				Expect(len(cfg.VPC.Subnets[k])).To(Equal(3))
+			}
+		})
+
+		rs := NewClusterResourceSet(cfg)
+		It("should add all resources without error", func() {
+			err := rs.AddAllResources()
+			Expect(err).ShouldNot(HaveOccurred())
+		})
 
-		sampleStack := newStackWithOutputs(map[string]string{
+		sampleOutputs := map[string]string{
 			"SecurityGroup":            "sg-0b44c48bcba5b7362",
 			"SubnetsPublic":            "subnet-0f98135715dfcf55f,subnet-0ade11bad78dced9e,subnet-0e2e63ff1712bf6ef",
+			"SubnetsPrivate":           "subnet-0f98135715dfcf55a,subnet-0ade11bad78dced9f,subnet-0e2e63ff1712bf6ea",
 			"VPC":                      "vpc-0e265ad953062b94b",
 			"Endpoint":                 endpoint,
 			"CertificateAuthorityData": caCert,
 			"ARN":                      arn,
 			"ClusterStackName":         "",
-		})
+		}
+
+		sampleStack := newStackWithOutputs(sampleOutputs)
 
 		It("should not error", func() {
 			err := rs.GetAllOutputs(sampleStack)
 			Expect(err).ShouldNot(HaveOccurred())
 		})
 
 		It("should be equal", func() {
-			Expect(*initial).To(Equal(*expected))
+			Expect(*cfg).To(Equal(*expected))
 		})
 	})
 

pkg/cfn/builder/cluster.go

@@ -1,6 +1,8 @@
 package builder
 
 import (
+	"fmt"
+
 	cfn "github.com/aws/aws-sdk-go/service/cloudformation"
 	gfn "github.com/awslabs/goformation/cloudformation"
 
@@ -31,10 +33,19 @@ func (c *ClusterResourceSet) AddAllResources() error {
 
 	templateDescriptionFeatures := clusterTemplateDescriptionDefaultFeatures
 
-	if c.spec.VPC.ID != "" && c.spec.VPC.HasSufficientPublicSubnets() {
+	if c.spec.VPC.ID != "" && c.spec.HasSufficientPublicSubnets() {
 		c.importResourcesForVPC()
 		templateDescriptionFeatures = " (with shared VPC and dedicated IAM role) "
 	} else {
+		topologies := len(c.spec.VPC.Subnets)
+		switch {
+		case topologies < 1:
+			return fmt.Errorf("too few subnet topologies: %v", c.spec.VPC.Subnets)
+		case topologies == 1 && !c.spec.HasSufficientPublicSubnets():
+			return fmt.Errorf("too few public subnets: %v", c.spec.VPC.Subnets["Public"])
+		case topologies == 2 && !c.spec.HasSufficientPrivateSubnets():
+			return fmt.Errorf("too few private subnets: %v", c.spec.VPC.Subnets["Private"])
+		}
 		c.addResourcesForVPC()
 	}
 	c.addOutputsForVPC()
@@ -93,11 +104,11 @@ func (c *ClusterResourceSet) GetAllOutputs(stack cfn.Stack) error {
 
 	// TODO: shouldn't assume the order is the same, can probably do an API lookup
 	for i, subnet := range c.outputs.SubnetsPrivate {
-		c.spec.VPC.ImportSubnet(api.SubnetTopologyPrivate, c.spec.AvailabilityZones[i], subnet)
+		c.spec.ImportSubnet(api.SubnetTopologyPrivate, c.spec.AvailabilityZones[i], subnet)
 	}
 
 	for i, subnet := range c.outputs.SubnetsPublic {
-		c.spec.VPC.ImportSubnet(api.SubnetTopologyPublic, c.spec.AvailabilityZones[i], subnet)
+		c.spec.ImportSubnet(api.SubnetTopologyPublic, c.spec.AvailabilityZones[i], subnet)
 	}
 
 	c.spec.ClusterStackName = c.outputs.ClusterStackName

pkg/cfn/builder/nodegroup.go

@@ -111,7 +111,7 @@ func (n *NodeGroupResourceSet) addResourcesForNodeGroup() {
 	// and tags don't have `PropagateAtLaunch` field, so we have a custom method here until this gets resolved
 	var vpcZoneIdentifier interface{}
 	if len(n.spec.AvailabilityZones) > 0 {
-		vpcZoneIdentifier = n.clusterSpec.VPC.SubnetIDs(api.SubnetTopologyPublic)
+		vpcZoneIdentifier = n.clusterSpec.SubnetIDs(api.SubnetTopologyPublic)
 	} else {
 		vpcZoneIdentifier = map[string][]interface{}{
 			gfn.FnSplit: []interface{}{

pkg/cfn/builder/vpc.go

@@ -8,7 +8,6 @@ import (
 )
 
 func (c *ClusterResourceSet) addSubnets(refRT *gfn.Value, topology api.SubnetTopology) {
-	c.subnets = make(map[api.SubnetTopology][]*gfn.Value)
 	for az, subnet := range c.spec.VPC.Subnets[topology] {
 		alias := strings.ToUpper(strings.Join(strings.Split(az, "-"), ""))
 		refSubnet := c.newResource("Subnet"+string(topology)+alias, &gfn.AWSEC2Subnet{
@@ -32,6 +31,8 @@ func (c *ClusterResourceSet) addResourcesForVPC() {
 		EnableDnsHostnames: gfn.True(),
 	})
 
+	c.subnets = make(map[api.SubnetTopology][]*gfn.Value)
+
 	refIG := c.newResource("InternetGateway", &gfn.AWSEC2InternetGateway{})
 	c.newResource("VPCGatewayAttachment", &gfn.AWSEC2VPCGatewayAttachment{
 		InternetGatewayId: refIG,
@@ -59,16 +60,16 @@ func (c *ClusterResourceSet) addResourcesForVPC() {
 
 func (c *ClusterResourceSet) importResourcesForVPC() {
 	c.vpc = gfn.NewString(c.spec.VPC.ID)
-	for _, topology := range c.spec.VPC.SubnetTopologies() {
-		for _, subnet := range c.spec.VPC.SubnetIDs(topology) {
+	for topology := range c.spec.VPC.Subnets {
+		for _, subnet := range c.spec.SubnetIDs(topology) {
 			c.subnets[topology] = append(c.subnets[topology], gfn.NewString(subnet))
 		}
 	}
 }
 
 func (c *ClusterResourceSet) addOutputsForVPC() {
 	c.rs.newOutput(cfnOutputClusterVPC, c.vpc, true)
-	for _, topology := range c.spec.VPC.SubnetTopologies() {
+	for topology := range c.spec.VPC.Subnets {
 		c.rs.newJoinedOutput(cfnOutputClusterSubnets+string(topology), c.subnets[topology], true)
 	}
 }

pkg/ctl/create/cluster.go

@@ -90,6 +90,8 @@ func createClusterCmd() *cobra.Command {
 
 	fs.StringVar(&kopsClusterNameForVPC, "vpc-from-kops-cluster", "", "re-use VPC from a given kops cluster")
 
+	fs.IPNetVar(cfg.VPC.CIDR, "vpc-cidr", api.DefaultCIDR(), "global CIDR to use for VPC")
+
 	return cmd
 }
 
@@ -132,13 +134,15 @@ func doCreateCluster(cfg *api.ClusterConfig, ng *api.NodeGroup, name string) err
 		if err := kw.UseVPC(cfg); err != nil {
 			return err
 		}
-		logger.Success("using VPC (%s) and subnets (%v) from kops cluster %q", cfg.VPC.ID, cfg.VPC.SubnetIDs(api.SubnetTopologyPublic), kopsClusterNameForVPC)
+		logger.Success("using VPC (%s) and subnets (%v) from kops cluster %q", cfg.VPC.ID, cfg.SubnetIDs(api.SubnetTopologyPublic), kopsClusterNameForVPC)
 	} else {
 		// kw.UseVPC() sets AZs based on subenets used
 		if err := ctl.SetAvailabilityZones(availabilityZones); err != nil {
 			return err
 		}
-		cfg.SetSubnets()
+		if err := ctl.SetSubnets(); err != nil {
+			return err
+		}
 	}
 
 	if err := ctl.EnsureAMI(ng); err != nil {

pkg/eks/api/api.go

@@ -38,7 +38,7 @@ var SupportedRegions = []string{
 var DefaultWaitTimeout = 20 * time.Minute
 
 // DefaultNodeCount defines the default number of nodes to be created
-var DefaultNodeCount = 2
+const DefaultNodeCount = 2
 
 // ClusterProvider provides an interface with the needed AWS APIs
 type ClusterProvider interface {
@@ -57,7 +57,7 @@ type ClusterConfig struct {
 
 	WaitTimeout time.Duration
 
-	VPC ClusterVPC
+	VPC *ClusterVPC
 
 	NodeGroups []*NodeGroup
 
@@ -76,29 +76,13 @@ type ClusterConfig struct {
 // it doesn't include initial nodegroup, so user must
 // call NewNodeGroup to create one
 func NewClusterConfig() *ClusterConfig {
-	return &ClusterConfig{}
-}
-
-// SetSubnets defines CIDRs for each of the subnets,
-// it must be called after SetAvailabilityZones
-func (c *ClusterConfig) SetSubnets() {
-	_, c.VPC.CIDR, _ = net.ParseCIDR("192.168.0.0/16")
-
-	c.VPC.Subnets = map[SubnetTopology]map[string]Network{
-		SubnetTopologyPublic: map[string]Network{},
+	cfg := &ClusterConfig{
+		VPC: &ClusterVPC{},
 	}
 
-	zoneCIDRs := []string{
-		"192.168.64.0/18",
-		"192.168.128.0/18",
-		"192.168.192.0/18",
-	}
-	for i, zone := range c.AvailabilityZones {
-		_, zoneCIDR, _ := net.ParseCIDR(zoneCIDRs[i])
-		c.VPC.Subnets[SubnetTopologyPublic][zone] = Network{
-			CIDR: zoneCIDR,
-		}
-	}
+	cfg.VPC.CIDR = &net.IPNet{}
+
+	return cfg
 }
 
 // NewNodeGroup crears new nodegroup inside cluster config,
@@ -142,72 +126,6 @@ type NodeGroup struct {
 	SSHPublicKeyName string
 }
 
-type (
-	// ClusterVPC holds global subnet and all child public/private subnet
-	ClusterVPC struct {
-		Network              // global CIRD and VPC ID
-		SecurityGroup string // cluster SG
-		// subnets are either public or private for use with separate nodegroups
-		// these are keyed by AZ for conveninece
-		Subnets map[SubnetTopology]map[string]Network
-		// for additional CIRD associations, e.g. to use with separate CIDR for
-		// private subnets or any ad-hoc subnets
-		ExtraCIDRs []*net.IPNet
-	}
-	// SubnetTopology can be SubnetTopologyPrivate or SubnetTopologyPublic
-	SubnetTopology string
-	// Network holds ID and CIDR
-	Network struct {
-		ID   string
-		CIDR *net.IPNet
-	}
-)
-
-const (
-	// SubnetTopologyPrivate repesents privately-routed subnets
-	SubnetTopologyPrivate SubnetTopology = "Private"
-	// SubnetTopologyPublic repesents publicly-routed subnets
-	SubnetTopologyPublic SubnetTopology = "Public"
-)
-
-// SubnetIDs returns list of subnets
-func (c ClusterVPC) SubnetIDs(topology SubnetTopology) []string {
-	subnets := []string{}
-	for _, s := range c.Subnets[topology] {
-		subnets = append(subnets, s.ID)
-	}
-	return subnets
-}
-
-// SubnetTopologies returns list of topologies supported
-// by a given cluster config
-func (c ClusterVPC) SubnetTopologies() []SubnetTopology {
-	topologies := []SubnetTopology{}
-	for topology := range c.Subnets {
-		topologies = append(topologies, topology)
-	}
-	return topologies
-}
-
-// ImportSubnet loads a given subnet into cluster config
-func (c ClusterVPC) ImportSubnet(topology SubnetTopology, az, subnetID string) {
-	if _, ok := c.Subnets[topology]; !ok {
-		c.Subnets[topology] = map[string]Network{}
-	}
-	if network, ok := c.Subnets[topology][az]; !ok {
-		c.Subnets[topology][az] = Network{ID: subnetID}
-	} else {
-		network.ID = subnetID
-		c.Subnets[topology][az] = network
-	}
-}
-
-// HasSufficientPublicSubnets validates if there is a suffiecent
-// number of subnets available to create a cluster
-func (c ClusterVPC) HasSufficientPublicSubnets() bool {
-	return len(c.SubnetIDs(SubnetTopologyPublic)) >= 3
-}
-
 type (
 	// ClusterAddons provides addons for the created EKS cluster
 	ClusterAddons struct {