Refactor the main API struct

This will accommodate for

- multiple nodegroups
- public and private subnets
- use taskmanager tasks for nodegroup(s) deletion
- cleanup nodegroup terminology

README.md

@@ -158,7 +158,7 @@ eksctl delete cluster --name=<name> [--region=<region>]
 
 ### Scaling nodegroup
 
-The default nodegroup can be scaled by using the `eksctl scale nodegroup` command. For example, to scale to 5 nodes:
+The initial nodegroup can be scaled by using the `eksctl scale nodegroup` command. For example, to scale to 5 nodes:
 
 ```
 eksctl scale nodegroup --name=<name> --nodes=5

integration/integration_test.go

@@ -9,9 +9,6 @@ import (
 
 	"github.com/weaveworks/eksctl/pkg/eks/api"
 	"github.com/weaveworks/eksctl/pkg/testutils"
-
-	. "github.com/onsi/ginkgo"
-	. "github.com/onsi/gomega"
 )
 
 const (

pkg/cfn/builder/api_test.go

@@ -3,6 +3,7 @@ package builder_test
 import (
 	"encoding/base64"
 	"encoding/json"
+	"net"
 	"path/filepath"
 	"strings"
 
@@ -90,33 +91,84 @@ var _ = Describe("CloudFormation template builder API", func() {
 
 	testAZs := []string{"us-west-2b", "us-west-2a", "us-west-2c"}
 
+	newClusterConfig := func() *api.ClusterConfig {
+		cfg := api.NewClusterConfig()
+		ng := cfg.NewNodeGroup()
+
+		cfg.Region = "us-west-2"
+		cfg.ClusterName = clusterName
+		cfg.AvailabilityZones = testAZs
+		ng.InstanceType = "t2.medium"
+
+		return cfg
+	}
+
 	Describe("GetAllOutputsFromClusterStack", func() {
 		caCertData, err := base64.StdEncoding.DecodeString(caCert)
 		It("should not error", func() { Expect(err).ShouldNot(HaveOccurred()) })
 
 		expected := &api.ClusterConfig{
+			Region:                   "us-west-2",
 			ClusterName:              clusterName,
-			SecurityGroup:            "sg-0b44c48bcba5b7362",
-			Subnets:                  []string{"subnet-0f98135715dfcf55f", "subnet-0ade11bad78dced9e", "subnet-0e2e63ff1712bf6ef"},
-			VPC:                      "vpc-0e265ad953062b94b",
 			Endpoint:                 endpoint,
 			CertificateAuthorityData: caCertData,
 			ARN:                      arn,
-			NodeInstanceRoleARN:      "",
 			AvailabilityZones:        testAZs,
-		}
 
-		initial := &api.ClusterConfig{
-			ClusterName:       clusterName,
-			AvailabilityZones: testAZs,
+			VPC: api.ClusterVPC{
+				Network: api.Network{
+					ID: "vpc-0e265ad953062b94b",
+					CIDR: &net.IPNet{
+						IP:   []byte{192, 168, 0, 0},
+						Mask: []byte{255, 255, 0, 0},
+					},
+				},
+				SecurityGroup: "sg-0b44c48bcba5b7362",
+				Subnets: map[api.SubnetTopology]map[string]api.Network{
+					"Public": map[string]api.Network{
+						"us-west-2b": {
+							ID: "subnet-0f98135715dfcf55f",
+							CIDR: &net.IPNet{
+								IP:   []byte{192, 168, 64, 0},
+								Mask: []byte{255, 255, 192, 0},
+							},
+						},
+						"us-west-2a": {
+							ID: "subnet-0ade11bad78dced9e",
+							CIDR: &net.IPNet{
+								IP:   []byte{192, 168, 128, 0},
+								Mask: []byte{255, 255, 192, 0},
+							},
+						},
+						"us-west-2c": {
+							ID: "subnet-0e2e63ff1712bf6ef",
+							CIDR: &net.IPNet{
+								IP:   []byte{192, 168, 192, 0},
+								Mask: []byte{255, 255, 192, 0},
+							},
+						},
+					},
+				},
+			},
+			NodeGroups: []*api.NodeGroup{
+				{
+					AMI:            "",
+					InstanceType:   "t2.medium",
+					SubnetTopology: "Public",
+				},
+			},
 		}
 
+		initial := newClusterConfig()
+
+		initial.SetSubnets()
+
 		rs := NewClusterResourceSet(initial)
 		rs.AddAllResources()
 
 		sampleStack := newStackWithOutputs(map[string]string{
 			"SecurityGroup":            "sg-0b44c48bcba5b7362",
-			"Subnets":                  "subnet-0f98135715dfcf55f,subnet-0ade11bad78dced9e,subnet-0e2e63ff1712bf6ef",
+			"SubnetsPublic":            "subnet-0f98135715dfcf55f,subnet-0ade11bad78dced9e,subnet-0e2e63ff1712bf6ef",
 			"VPC":                      "vpc-0e265ad953062b94b",
 			"Endpoint":                 endpoint,
 			"CertificateAuthorityData": caCert,
@@ -130,17 +182,14 @@ var _ = Describe("CloudFormation template builder API", func() {
 		})
 
 		It("should be equal", func() {
-			Expect(initial).To(Equal(expected))
+			Expect(*initial).To(Equal(*expected))
 		})
 	})
 
 	Describe("AutoNameTag", func() {
-		rs := NewNodeGroupResourceSet(&api.ClusterConfig{
-			ClusterName:       clusterName,
-			AvailabilityZones: testAZs,
-			NodeType:          "t2.medium",
-			Region:            "us-west-2",
-		}, "eksctl-test-123-cluster", 0)
+		cfg := newClusterConfig()
+
+		rs := NewNodeGroupResourceSet(cfg, "eksctl-test-123-cluster", 0)
 
 		err := rs.AddAllResources()
 		It("should add all resources without errors", func() {
@@ -179,12 +228,15 @@ var _ = Describe("CloudFormation template builder API", func() {
 	})
 
 	Describe("NodeGroupTags", func() {
-		rs := NewNodeGroupResourceSet(&api.ClusterConfig{
-			ClusterName:       clusterName,
-			AvailabilityZones: testAZs,
-			NodeType:          "t2.medium",
-			Region:            "us-west-2",
-		}, "eksctl-test-123-cluster", 0)
+		cfg := api.NewClusterConfig()
+		ng := cfg.NewNodeGroup()
+
+		cfg.Region = "us-west-2"
+		cfg.ClusterName = clusterName
+		cfg.AvailabilityZones = testAZs
+		ng.InstanceType = "t2.medium"
+
+		rs := NewNodeGroupResourceSet(cfg, "eksctl-test-123-cluster", 0)
 		rs.AddAllResources()
 
 		template, err := rs.RenderJSON()
@@ -210,17 +262,15 @@ var _ = Describe("CloudFormation template builder API", func() {
 	})
 
 	Describe("NodeGroupAutoScaling", func() {
-		rs := NewNodeGroupResourceSet(&api.ClusterConfig{
-			ClusterName:       clusterName,
-			AvailabilityZones: testAZs,
-			NodeType:          "t2.medium",
-			Region:            "us-west-2",
-			Addons: api.ClusterAddons{
-				WithIAM: api.AddonIAM{
-					PolicyAutoScaling: true,
-				},
+		cfg := newClusterConfig()
+
+		cfg.Addons = api.ClusterAddons{
+			WithIAM: api.AddonIAM{
+				PolicyAutoScaling: true,
 			},
-		}, "eksctl-test-123-cluster", 0)
+		}
+
+		rs := NewNodeGroupResourceSet(cfg, "eksctl-test-123-cluster", 0)
 		rs.AddAllResources()
 
 		template, err := rs.RenderJSON()
@@ -251,20 +301,18 @@ var _ = Describe("CloudFormation template builder API", func() {
 	})
 
 	Describe("UserData", func() {
+		cfg := newClusterConfig()
 
 		var c *cloudconfig.CloudConfig
 
 		caCertData, err := base64.StdEncoding.DecodeString(caCert)
 		It("should not error", func() { Expect(err).ShouldNot(HaveOccurred()) })
 
-		rs := NewNodeGroupResourceSet(&api.ClusterConfig{
-			ClusterName:              clusterName,
-			AvailabilityZones:        testAZs,
-			NodeType:                 "m5.large",
-			Region:                   "us-west-2",
-			Endpoint:                 endpoint,
-			CertificateAuthorityData: caCertData,
-		}, "eksctl-test-123-cluster", 0)
+		cfg.Endpoint = endpoint
+		cfg.CertificateAuthorityData = caCertData
+		cfg.NodeGroups[0].InstanceType = "m5.large"
+
+		rs := NewNodeGroupResourceSet(cfg, "eksctl-test-123-cluster", 0)
 		rs.AddAllResources()
 
 		template, err := rs.RenderJSON()

pkg/cfn/builder/cluster.go

@@ -1,35 +1,28 @@
 package builder
 
 import (
-	"net"
-
 	cfn "github.com/aws/aws-sdk-go/service/cloudformation"
 	gfn "github.com/awslabs/goformation/cloudformation"
 
 	"github.com/weaveworks/eksctl/pkg/eks/api"
 )
 
-const (
-	cfnOutputClusterCertificateAuthorityData = "CertificateAuthorityData"
-	cfnOutputClusterEndpoint                 = "Endpoint"
-	cfnOutputClusterARN                      = "ARN"
-	cfnOutputClusterStackName                = "ClusterStackName"
-)
-
 // ClusterResourceSet stores the resource information of the cluster
 type ClusterResourceSet struct {
 	rs             *resourceSet
 	spec           *api.ClusterConfig
 	vpc            *gfn.Value
-	subnets        []*gfn.Value
+	subnets        map[api.SubnetTopology][]*gfn.Value
 	securityGroups []*gfn.Value
+	outputs        *ClusterStackOutputs
 }
 
 // NewClusterResourceSet returns a resource set for the new cluster
 func NewClusterResourceSet(spec *api.ClusterConfig) *ClusterResourceSet {
 	return &ClusterResourceSet{
-		rs:   newResourceSet(),
-		spec: spec,
+		rs:      newResourceSet(),
+		spec:    spec,
+		outputs: &ClusterStackOutputs{},
 	}
 }
 
@@ -38,18 +31,11 @@ func (c *ClusterResourceSet) AddAllResources() error {
 
 	templateDescriptionFeatures := clusterTemplateDescriptionDefaultFeatures
 
-	if c.spec.VPC != "" && len(c.spec.Subnets) >= 3 {
+	if c.spec.VPC.ID != "" && c.spec.VPC.HasSufficientPublicSubnets() {
 		c.importResourcesForVPC()
 		templateDescriptionFeatures = " (with shared VPC and dedicated IAM role) "
 	} else {
-		_, globalCIDR, _ := net.ParseCIDR("192.168.0.0/16")
-
-		subnets := map[string]*net.IPNet{}
-		_, subnets[c.spec.AvailabilityZones[0]], _ = net.ParseCIDR("192.168.64.0/18")
-		_, subnets[c.spec.AvailabilityZones[1]], _ = net.ParseCIDR("192.168.128.0/18")
-		_, subnets[c.spec.AvailabilityZones[2]], _ = net.ParseCIDR("192.168.192.0/18")
-
-		c.addResourcesForVPC(globalCIDR, subnets)
+		c.addResourcesForVPC()
 	}
 	c.addOutputsForVPC()
 
@@ -86,7 +72,7 @@ func (c *ClusterResourceSet) addResourcesForControlPlane(version string) {
 		RoleArn: gfn.MakeFnGetAttString("ServiceRole.Arn"),
 		Version: gfn.NewString(version),
 		ResourcesVpcConfig: &gfn.AWSEKSCluster_ResourcesVpcConfig{
-			SubnetIds:        c.subnets,
+			SubnetIds:        c.subnets[api.SubnetTopologyPublic],
 			SecurityGroupIds: c.securityGroups,
 		},
 	})
@@ -98,5 +84,26 @@ func (c *ClusterResourceSet) addResourcesForControlPlane(version string) {
 
 // GetAllOutputs collects all outputs of the cluster
 func (c *ClusterResourceSet) GetAllOutputs(stack cfn.Stack) error {
-	return c.rs.GetAllOutputs(stack, c.spec)
+	if err := c.rs.GetAllOutputs(stack, c.outputs); err != nil {
+		return err
+	}
+
+	c.spec.VPC.ID = c.outputs.VPC
+	c.spec.VPC.SecurityGroup = c.outputs.SecurityGroup
+
+	// 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)
+	}
+
+	for i, subnet := range c.outputs.SubnetsPublic {
+		c.spec.VPC.ImportSubnet(api.SubnetTopologyPublic, c.spec.AvailabilityZones[i], subnet)
+	}
+
+	c.spec.ClusterStackName = c.outputs.ClusterStackName
+	c.spec.Endpoint = c.outputs.Endpoint
+	c.spec.CertificateAuthorityData = c.outputs.CertificateAuthorityData
+	c.spec.ARN = c.outputs.ARN
+
+	return nil
 }

pkg/cfn/builder/iam.go

@@ -5,8 +5,6 @@ import (
 )
 
 const (
-	cfnOutputNodeInstanceRoleARN = "NodeInstanceRoleARN"
-
 	iamPolicyAmazonEKSServicePolicyARN = "arn:aws:iam::aws:policy/AmazonEKSServicePolicy"
 	iamPolicyAmazonEKSClusterPolicyARN = "arn:aws:iam::aws:policy/AmazonEKSClusterPolicy"
 
@@ -84,19 +82,19 @@ func (n *NodeGroupResourceSet) WithIAM() bool {
 func (n *NodeGroupResourceSet) addResourcesForIAM() {
 	n.rs.withIAM = true
 
-	if len(n.spec.NodePolicyARNs) == 0 {
-		n.spec.NodePolicyARNs = iamDefaultNodePolicyARNs
+	if len(n.spec.PolicyARNs) == 0 {
+		n.spec.PolicyARNs = iamDefaultNodePolicyARNs
 	}
-	if n.spec.Addons.WithIAM.PolicyAmazonEC2ContainerRegistryPowerUser {
-		n.spec.NodePolicyARNs = append(n.spec.NodePolicyARNs, iamPolicyAmazonEC2ContainerRegistryPowerUserARN)
+	if n.clusterSpec.Addons.WithIAM.PolicyAmazonEC2ContainerRegistryPowerUser {
+		n.spec.PolicyARNs = append(n.spec.PolicyARNs, iamPolicyAmazonEC2ContainerRegistryPowerUserARN)
 	} else {
-		n.spec.NodePolicyARNs = append(n.spec.NodePolicyARNs, iamPolicyAmazonEC2ContainerRegistryReadOnlyARN)
+		n.spec.PolicyARNs = append(n.spec.PolicyARNs, iamPolicyAmazonEC2ContainerRegistryReadOnlyARN)
 	}
 
 	refIR := n.newResource("NodeInstanceRole", &gfn.AWSIAMRole{
 		Path:                     gfn.NewString("/"),
 		AssumeRolePolicyDocument: makeAssumeRolePolicyDocument("ec2.amazonaws.com"),
-		ManagedPolicyArns:        makeStringSlice(n.spec.NodePolicyARNs...),
+		ManagedPolicyArns:        makeStringSlice(n.spec.PolicyARNs...),
 	})
 
 	n.instanceProfile = n.newResource("NodeInstanceProfile", &gfn.AWSIAMInstanceProfile{
@@ -132,7 +130,7 @@ func (n *NodeGroupResourceSet) addResourcesForIAM() {
 		},
 	)
 
-	if n.spec.Addons.WithIAM.PolicyAutoScaling {
+	if n.clusterSpec.Addons.WithIAM.PolicyAutoScaling {
 		n.rs.attachAllowPolicy("PolicyAutoScaling", refIR, "*",
 			[]string{
 				"autoscaling:DescribeAutoScalingGroups",
@@ -145,5 +143,5 @@ func (n *NodeGroupResourceSet) addResourcesForIAM() {
 		)
 	}
 
-	n.rs.newOutputFromAtt(cfnOutputNodeInstanceRoleARN, "NodeInstanceRole.Arn", true)
+	n.rs.newOutputFromAtt(cfnOutputInstanceRoleARN, "NodeInstanceRole.Arn", true)
 }