Generalize subnet names so that we can have mutiple private or public…

… subnets per AZ

config/config.go

@@ -185,8 +185,8 @@ func (c *Cluster) SetDefaults() {
 	publicTopologyImplied := c.RouteTableID != "" && c.MapPublicIPs
 
 	for i, s := range c.Subnets {
-		if s.CustomName == "" {
-			c.Subnets[i].CustomName = fmt.Sprintf("Subnet%d", i)
+		if s.Name == "" {
+			c.Subnets[i].Name = fmt.Sprintf("Subnet%d", i)
 		}
 
 		// DEPRECATED AND REMOVED IN THE FUTURE
@@ -969,7 +969,7 @@ func (s DeploymentSettings) AllSubnets() []model.Subnet {
 
 func (c DeploymentSettings) FindSubnetMatching(condition model.Subnet) model.Subnet {
 	for _, s := range c.Subnets {
-		if s.CustomName == condition.CustomName {
+		if s.Name == condition.Name {
 			return s
 		}
 	}
@@ -1015,14 +1015,15 @@ func (c DeploymentSettings) NATGateways() []model.NATGateway {
 		var publicSubnet model.Subnet
 		ngwConfig := privateSubnet.NATGateway
 		if privateSubnet.ManageNATGateway() {
-			found := false
+			publicSubnetFound := false
 			for _, s := range c.PublicSubnets() {
 				if s.AvailabilityZone == privateSubnet.AvailabilityZone {
 					publicSubnet = s
-					found = true
+					publicSubnetFound = true
+					break
 				}
 			}
-			if !found {
+			if !publicSubnetFound {
 				panic(fmt.Sprintf("No appropriate public subnet found for a non-preconfigured NAT gateway associated to private subnet %s", privateSubnet.LogicalName()))
 			}
 			ngw := model.NewNATGateway(ngwConfig, privateSubnet, publicSubnet)

config/config_test.go

@@ -332,12 +332,12 @@ subnets:
 				{
 					InstanceCIDR:     "10.4.3.0/24",
 					AvailabilityZone: "ap-northeast-1a",
-					CustomName:       "Subnet0",
+					Name:             "Subnet0",
 				},
 				{
 					InstanceCIDR:     "10.4.4.0/24",
 					AvailabilityZone: "ap-northeast-1c",
-					CustomName:       "Subnet1",
+					Name:             "Subnet1",
 				},
 			},
 		},
@@ -353,7 +353,7 @@ instanceCIDR: 10.4.3.0/24
 				{
 					AvailabilityZone: "ap-northeast-1a",
 					InstanceCIDR:     "10.4.3.0/24",
-					CustomName:       "Subnet0",
+					Name:             "Subnet0",
 				},
 			},
 		},
@@ -370,7 +370,7 @@ subnets: []
 				{
 					AvailabilityZone: "ap-northeast-1a",
 					InstanceCIDR:     "10.4.3.0/24",
-					CustomName:       "Subnet0",
+					Name:             "Subnet0",
 				},
 			},
 		},
@@ -384,7 +384,7 @@ subnets: []
 				{
 					AvailabilityZone: "ap-northeast-1a",
 					InstanceCIDR:     "10.0.0.0/24",
-					CustomName:       "Subnet0",
+					Name:             "Subnet0",
 				},
 			},
 		},
@@ -397,7 +397,7 @@ availabilityZone: "ap-northeast-1a"
 				{
 					AvailabilityZone: "ap-northeast-1a",
 					InstanceCIDR:     "10.0.0.0/24",
-					CustomName:       "Subnet0",
+					Name:             "Subnet0",
 				},
 			},
 		},

config/templates/stack-template.json

@@ -1148,7 +1148,7 @@
     {{end}}
 
     {{range $index, $subnet := .Subnets}}
-    {{if $subnet.ManageSubnet }}
+    {{if $subnet.ManageSubnet}}
     ,
     "{{$subnet.LogicalName}}": {
       "Properties": {
@@ -1179,12 +1179,12 @@
     }
     {{if $subnet.ManageRouteTable}}
     ,
-    "{{$subnet.RouteTableName}}": {
+    "{{$subnet.RouteTableLogicalName}}": {
       "Properties": {
         "Tags": [
           {
             "Key": "Name",
-            "Value": "{{$.ClusterName}}-{{$subnet.RouteTableName}}"
+            "Value": "{{$.ClusterName}}-{{$subnet.RouteTableLogicalName}}"
           },
           {
             "Key": "KubernetesCluster",
@@ -1196,7 +1196,6 @@
       "Type": "AWS::EC2::RouteTable"
     }
     {{end}}
-    {{end}}
     {{if $.ElasticFileSystemID}}
     ,
     "{{$subnet.LogicalName}}MountTarget": {
@@ -1210,7 +1209,7 @@
     {{end}}
     {{if $subnet.ManageRouteToInternet}}
     ,
-    "{{$subnet.RouteTableName}}ToInternet": {
+    "{{$subnet.InternetGatewayRouteLogicalName}}": {
       "Properties": {
         "DestinationCidrBlock": "0.0.0.0/0",
         "GatewayId": {{$.InternetGatewayRef}},
@@ -1220,6 +1219,7 @@
     }
     {{end}}
     {{end}}
+    {{end}}
 
     {{range $i, $ngw := .NATGateways}}
     {{if $ngw.ManageEIP}}
@@ -1241,18 +1241,20 @@
       "Type": "AWS::EC2::NatGateway"
     }
     {{end}}
-    {{if $ngw.ManageRoute}}
+    {{range $_, $s := $ngw.PrivateSubnets}}
+    {{if $s.ManageRouteToNATGateway}}
     ,
-    "{{$ngw.NATGatewayRouteName}}": {
+    "{{$s.NATGatewayRouteLogicalName}}": {
       "Properties": {
         "DestinationCidrBlock": "0.0.0.0/0",
         "NatGatewayId": {{$ngw.Ref}},
-        "RouteTableId": {{$ngw.PrivateSubnetRouteTableRef}}
+        "RouteTableId": {{$s.RouteTableRef}}
       },
       "Type": "AWS::EC2::Route"
     }
     {{end}}
     {{end}}
+    {{end}}
 
     {{if not .InternetGatewayID}}
     ,
@@ -1314,10 +1316,17 @@
     {{end}}
     {{range $index, $subnet := .Subnets}}
     {{if $subnet.ManageRouteTable}}
-    "{{$subnet.RouteTableName}}" : {
+    "{{$subnet.RouteTableLogicalName}}" : {
       "Description" : "The route table assigned to the subnet {{$subnet.LogicalName}}",
       "Value" :  {{$subnet.RouteTableRef}},
-      "Export" : { "Name" : {"Fn::Sub": "${AWS::StackName}-{{$subnet.RouteTableName}}" }}
+      "Export" : { "Name" : {"Fn::Sub": "${AWS::StackName}-{{$subnet.RouteTableLogicalName}}" }}
+    },
+    {{end}}
+    {{if $subnet.ManageSubnet}}
+    "{{$subnet.LogicalName}}" : {
+      "Description" : "The subnet id of {{$subnet.LogicalName}}",
+      "Value" :  {{$subnet.Ref}},
+      "Export" : { "Name" : {"Fn::Sub": "${AWS::StackName}-{{$subnet.LogicalName}}" }}
     },
     {{end}}
     {{end}}

e2e/run

@@ -488,6 +488,12 @@ nodepools_destroy() {
   KUBE_AWS_NODE_POOL_INDEX=2 nodepool_destroy
 }
 
+nodepools_rerun() {
+  nodepools_destroy
+  build
+  nodepools
+}
+
 all_destroy() {
   nodepools_destroy
   main_destroy

model/etcd.go

@@ -35,16 +35,13 @@ func (i etcdInstanceImpl) SubnetRef() string {
 }
 
 func (i etcdInstanceImpl) DependencyExists() bool {
-	return i.subnet.Private && i.natGateway != nil && i.natGateway.ManageRoute()
+	return i.subnet.Private && i.subnet.ManageRouteToNATGateway()
 }
 
 func (i etcdInstanceImpl) DependencyRef() (string, error) {
 	// We have to wait until the route to the NAT gateway if it doesn't exist yet(hence ManageRoute=true) or the etcd node fails due to inability to connect internet
 	if i.DependencyExists() {
-		name, err := i.natGateway.NATGatewayRouteName()
-		if err != nil {
-			return "", err
-		}
+		name := i.subnet.NATGatewayRouteLogicalName()
 		return fmt.Sprintf(`"%s"`, name), nil
 	}
 	return "", nil

model/identifier.go

@@ -13,23 +13,25 @@ func (i Identifier) HasIdentifier() bool {
 	return i.ID != "" || i.IDFromStackOutput != ""
 }
 
-func (i Identifier) Ref(logicalName string) string {
+func (i Identifier) Ref(logicalNameProvider func() string) string {
 	if i.IDFromStackOutput != "" {
-		return fmt.Sprintf(`{ "ImportValue" : %q }`, i.IDFromStackOutput)
+		return fmt.Sprintf(`{ "Fn::ImportValue" : %q }`, i.IDFromStackOutput)
 	} else if i.ID != "" {
 		return fmt.Sprintf(`"%s"`, i.ID)
 	} else {
-		return fmt.Sprintf(`{ "Ref" : %q }`, logicalName)
+		return fmt.Sprintf(`{ "Ref" : %q }`, logicalNameProvider())
 	}
 }
 
-func (i Identifier) IdOrRef(refProvider func() (string, error)) (string, error) {
+// RefOrError should be used instead of Ref where possible so that kube-aws can print a more useful error message with
+// the line number for the stack-template.json when there's an error.
+func (i Identifier) RefOrError(logicalNameProvider func() (string, error)) (string, error) {
 	if i.IDFromStackOutput != "" {
-		return fmt.Sprintf(`{ "ImportValue" : %q }`, i.IDFromStackOutput), nil
+		return fmt.Sprintf(`{ "Fn::ImportValue" : %q }`, i.IDFromStackOutput), nil
 	} else if i.ID != "" {
 		return fmt.Sprintf(`"%s"`, i.ID), nil
 	} else {
-		logicalName, err := refProvider()
+		logicalName, err := logicalNameProvider()
 		if err != nil {
 			return "", fmt.Errorf("failed to get id or ref: %v", err)
 		}