Drop subnets[].natGateway.preconfigured and subnets[].internetGateway…

….preconfigured completely and induce these from other settings like before
HotelsDotCom · Jan 31, 2017 · 18e4422 · 18e4422
1 parent 5bc0395
commit 18e4422
Show file tree

Hide file tree

Showing 9 changed files with 139 additions and 202 deletions.
diff --git a/config/config.go b/config/config.go
@@ -195,7 +195,6 @@ func (c *Cluster) SetDefaults() {
 		// This implies a deployment to an existing VPC with a route table with a preconfigured Internet Gateway
 		// and all the subnets created by kube-aws are public
 		if publicTopologyImplied {
-			c.Subnets[i].InternetGateway.Preconfigured = true
 			c.Subnets[i].RouteTable.ID = c.RouteTableID
 			if s.Private {
 				panic(fmt.Sprintf("mapPublicIPs(=%v) and subnets[%d].private(=%v) conflicts: %+v", c.MapPublicIPs, i, s.Private, s))
@@ -209,7 +208,6 @@ func (c *Cluster) SetDefaults() {
 		// This implies a deployment to an existing VPC with a route table with a preconfigured NAT Gateway
 		// and all the subnets created by kube-aws are private
 		if privateTopologyImplied {
-			c.Subnets[i].NATGateway.Preconfigured = true
 			c.Subnets[i].RouteTable.ID = c.RouteTableID
 			if s.Private {
 				panic(fmt.Sprintf("mapPublicIPs(=%v) and subnets[%d].private(=%v) conflicts. You don't need to set true to both of them. If you want to make all the subnets private, make mapPublicIPs false. If you want to make only part of subnets private, make subnets[].private true accordingly: %+v", c.MapPublicIPs, i, s.Private, s))
@@ -585,16 +583,16 @@ func (c Cluster) Config() (*Config, error) {
 
 		var instance model.EtcdInstance
 
-		if subnet.Private {
+		if subnet.ManageNATGateway() {
 			ngw, err := c.FindNATGatewayForPrivateSubnet(subnet)
 
 			if err != nil {
-				return nil, fmt.Errorf("failed getting the NAT gateway for the subnet %s in %v: %v", subnet.LogicalName(), c.NATGateways(), err)
+				return nil, fmt.Errorf("failed getting a NAT gateway for the subnet %s in %v: %v", subnet.LogicalName(), c.NATGateways(), err)
 			}
 
-			instance = model.NewPrivateEtcdInstance(subnet, *ngw)
+			instance = model.NewEtcdInstanceDependsOnNewlyCreatedNGW(subnet, *ngw)
 		} else {
-			instance = model.NewPublicEtcdInstance(subnet)
+			instance = model.NewEtcdInstance(subnet)
 		}
 
 		config.EtcdInstances[etcdIndex] = instance
@@ -1016,7 +1014,7 @@ func (c DeploymentSettings) NATGateways() []model.NATGateway {
 	for _, privateSubnet := range c.PrivateSubnets() {
 		var publicSubnet model.Subnet
 		ngwConfig := privateSubnet.NATGateway
-		if !ngwConfig.Preconfigured {
+		if privateSubnet.ManageNATGateway() {
 			found := false
 			for _, s := range c.PublicSubnets() {
 				if s.AvailabilityZone == privateSubnet.AvailabilityZone {
@@ -1027,9 +1025,9 @@ func (c DeploymentSettings) NATGateways() []model.NATGateway {
 			if !found {
 				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)
+			ngws = append(ngws, ngw)
 		}
-		ngw := model.NewNATGateway(ngwConfig, privateSubnet, publicSubnet)
-		ngws = append(ngws, ngw)
 	}
 	return ngws
 }

diff --git a/config/templates/stack-template.json b/config/templates/stack-template.json
@@ -571,7 +571,7 @@
         "Tenancy": "{{$.EtcdTenancy}}",
         "UserData": { "Fn::FindInMap" : [ "EtcdInstanceParams", "UserData", "cloudconfig"] }
       },
-      {{if $etcdInstance.DependencyRef}}
+      {{if $etcdInstance.DependencyExists}}
       "DependsOn": [{{$etcdInstance.DependencyRef}}],
       {{end}}
       "Type": "AWS::EC2::Instance"
@@ -1148,7 +1148,7 @@
     {{end}}
 
     {{range $index, $subnet := .Subnets}}
-    {{if not $subnet.HasIdentifier }}
+    {{if $subnet.ManageSubnet }}
     ,
     "{{$subnet.LogicalName}}": {
       "Properties": {
@@ -1169,18 +1169,6 @@
       },
       "Type": "AWS::EC2::Subnet"
     }
-    {{end}}
-    {{if $.ElasticFileSystemID}}
-    ,
-    "{{$subnet.LogicalName}}MountTarget": {
-      "Properties" : {
-        "FileSystemId": "{{$.ElasticFileSystemID}}",
-        "SubnetId": {{$subnet.Ref}},
-        "SecurityGroups": [ { "Ref": "SecurityGroupMountTarget" } ]
-      },
-      "Type" : "AWS::EFS::MountTarget"
-    }
-    {{end}}
     ,
     "{{$subnet.LogicalName}}RouteTableAssociation": {
       "Properties": {
@@ -1189,6 +1177,7 @@
       },
       "Type": "AWS::EC2::SubnetRouteTableAssociation"
     }
+    {{if $subnet.ManageRouteTable}}
     ,
     "{{$subnet.RouteTableName}}": {
       "Properties": {
@@ -1206,6 +1195,19 @@
       },
       "Type": "AWS::EC2::RouteTable"
     }
+    {{end}}
+    {{end}}
+    {{if $.ElasticFileSystemID}}
+    ,
+    "{{$subnet.LogicalName}}MountTarget": {
+      "Properties" : {
+        "FileSystemId": "{{$.ElasticFileSystemID}}",
+        "SubnetId": {{$subnet.Ref}},
+        "SecurityGroups": [ { "Ref": "SecurityGroupMountTarget" } ]
+      },
+      "Type" : "AWS::EFS::MountTarget"
+    }
+    {{end}}
     {{if $subnet.ManageRouteToInternet}}
     ,
     "{{$subnet.RouteTableName}}ToInternet": {
@@ -1311,12 +1313,14 @@
     },
     {{end}}
     {{range $index, $subnet := .Subnets}}
+    {{if $subnet.ManageRouteTable}}
     "{{$subnet.RouteTableName}}" : {
       "Description" : "The route table assigned to the subnet {{$subnet.LogicalName}}",
       "Value" :  {{$subnet.RouteTableRef}},
       "Export" : { "Name" : {"Fn::Sub": "${AWS::StackName}-{{$subnet.RouteTableName}}" }}
     },
     {{end}}
+    {{end}}
     "WorkerSecurityGroup" : {
       "Description" : "The security group assigned to worker nodes",
       "Value" :  { "Ref" : "SecurityGroupWorker" },

diff --git a/e2e/run b/e2e/run
@@ -107,6 +107,8 @@ configure() {
 
   ${KUBE_AWS_CMD} render
 
+  ${KUBE_AWS_CMD} up --export --s3-uri ${KUBE_AWS_S3_URI} --pretty-print
+
   ${KUBE_AWS_CMD} validate --s3-uri ${KUBE_AWS_S3_URI}
 
   echo Generated configuration files in ${WORK_DIR}:

diff --git a/model/etcd.go b/model/etcd.go
@@ -8,22 +8,23 @@ type Etcd struct {
 
 type EtcdInstance interface {
 	SubnetRef() string
-	DependencyRef() string
+	DependencyExists() bool
+	DependencyRef() (string, error)
 }
 
 type etcdInstanceImpl struct {
 	subnet     Subnet
 	natGateway NATGateway
 }
 
-func NewPrivateEtcdInstance(s Subnet, ngw NATGateway) EtcdInstance {
+func NewEtcdInstanceDependsOnNewlyCreatedNGW(s Subnet, ngw NATGateway) EtcdInstance {
 	return etcdInstanceImpl{
 		subnet:     s,
 		natGateway: ngw,
 	}
 }
 
-func NewPublicEtcdInstance(s Subnet) EtcdInstance {
+func NewEtcdInstance(s Subnet) EtcdInstance {
 	return etcdInstanceImpl{
 		subnet: s,
 	}
@@ -33,10 +34,18 @@ func (i etcdInstanceImpl) SubnetRef() string {
 	return i.subnet.Ref()
 }
 
-func (i etcdInstanceImpl) DependencyRef() string {
+func (i etcdInstanceImpl) DependencyExists() bool {
+	return i.subnet.Private && i.natGateway != nil && i.natGateway.ManageRoute()
+}
+
+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.subnet.Private && i.natGateway.ManageRoute() {
-		return fmt.Sprintf(`"%s"`, i.natGateway.NATGatewayRouteName())
+	if i.DependencyExists() {
+		name, err := i.natGateway.NATGatewayRouteName()
+		if err != nil {
+			return "", err
+		}
+		return fmt.Sprintf(`"%s"`, name), nil
 	}
-	return ""
+	return "", nil
 }
diff --git a/model/identifier.go b/model/identifier.go
@@ -22,3 +22,17 @@ func (i Identifier) Ref(logicalName string) string {
 		return fmt.Sprintf(`{ "Ref" : %q }`, logicalName)
 	}
 }
+
+func (i Identifier) IdOrRef(refProvider func() (string, error)) (string, error) {
+	if i.IDFromStackOutput != "" {
+		return fmt.Sprintf(`{ "ImportValue" : %q }`, i.IDFromStackOutput), nil
+	} else if i.ID != "" {
+		return fmt.Sprintf(`"%s"`, i.ID), nil
+	} else {
+		logicalName, err := refProvider()
+		if err != nil {
+			return "", fmt.Errorf("failed to get id or ref: %v", err)
+		}
+		return fmt.Sprintf(`{ "Ref" : %q }`, logicalName), nil
+	}
+}
diff --git a/model/internet_gateway.go b/model/internet_gateway.go
@@ -1,8 +1,7 @@
 package model
 
 type InternetGateway struct {
-	Identifier    `yaml:",inline"`
-	Preconfigured bool `yaml:"preconfigured,omitempty"`
+	Identifier `yaml:",inline"`
 }
 
 func (g InternetGateway) ManageInternetGateway() bool {

diff --git a/model/nat_gateway.go b/model/nat_gateway.go
@@ -6,7 +6,6 @@ import (
 
 type NATGatewayConfig struct {
 	Identifier      `yaml:",inline"`
-	Preconfigured   bool   `yaml:"preconfigured,omitempty"`
 	EIPAllocationID string `yaml:"eipAllocationId,omitempty"`
 }
 
@@ -18,9 +17,9 @@ type NATGateway interface {
 	ManageEIP() bool
 	ManageNATGateway() bool
 	ManageRoute() bool
-	NATGatewayRouteName() string
+	NATGatewayRouteName() (string, error)
 	Ref() string
-	PrivateSubnetRouteTableRef() string
+	PrivateSubnetRouteTableRef() (string, error)
 	PublicSubnetRef() string
 	Validate() error
 }
@@ -44,15 +43,15 @@ func (g natGatewayImpl) LogicalName() string {
 }
 
 func (g natGatewayImpl) ManageNATGateway() bool {
-	return !g.HasIdentifier() && !g.Preconfigured
+	return g.privateSubnet.ManageNATGateway()
 }
 
 func (g natGatewayImpl) ManageEIP() bool {
 	return g.EIPAllocationID == ""
 }
 
 func (g natGatewayImpl) ManageRoute() bool {
-	return !g.Preconfigured
+	return g.privateSubnet.ManageRouteToNATGateway()
 }
 
 func (g natGatewayImpl) EIPLogicalName() string {
@@ -78,34 +77,38 @@ func (g natGatewayImpl) PublicSubnetRef() string {
 	return g.publicSubnet.Ref()
 }
 
-func (g natGatewayImpl) PrivateSubnetRouteTableRef() string {
-	return g.privateSubnet.RouteTableRef()
+func (g natGatewayImpl) PrivateSubnetRouteTableRef() (string, error) {
+	ref, err := g.privateSubnet.RouteTableRef()
+	if err != nil {
+		return "", err
+	}
+	return ref, nil
 }
 
-func (g natGatewayImpl) NATGatewayRouteName() string {
-	return g.privateSubnet.NATGatewayRouteName()
+func (g natGatewayImpl) NATGatewayRouteName() (string, error) {
+	return fmt.Sprintf("%sRouteToNatGateway", g.privateSubnet.ReferenceName()), nil
 }
 
 func (g natGatewayImpl) Validate() error {
-	if g.Preconfigured {
+	if !g.ManageNATGateway() {
 		if !g.privateSubnet.HasIdentifier() {
-			return fmt.Errorf("an NGW with preconfigured=true must be associated to an existing private subnet: %+v", g)
+			return fmt.Errorf("a preconfigured NGW must be associated to an existing private subnet: %+v", g)
 		}
 
 		if g.publicSubnet.Provided() {
-			return fmt.Errorf("an NGW with preconfigured=true must not be associated to an existing public subnet: %+v", g)
+			return fmt.Errorf("a preconfigured NGW must not be associated to an existing public subnet: %+v", g)
 		}
 
 		if !g.privateSubnet.RouteTable.HasIdentifier() {
-			return fmt.Errorf("an NGW with preconfigured=true must have an existing route table provided via routeTable.id or routeTable.idFromStackOutput: %+v", g)
+			return fmt.Errorf("a preconfigured NGW must have an existing route table provided via routeTable.id or routeTable.idFromStackOutput: %+v", g)
 		}
 
 		if g.HasIdentifier() {
-			return fmt.Errorf("an NGW with preconcfigured=true must not have id or idFromStackOutput: %+v", g)
+			return fmt.Errorf("a preconfigured NGW must not have id or idFromStackOutput: %+v", g)
 		}
 
 		if g.EIPAllocationID != "" {
-			return fmt.Errorf("an NGW with preconcfigured=true must not have an eipAllocactionID: %+v", g)
+			return fmt.Errorf("a preconfigured NGW must not have an eipAllocactionID: %+v", g)
 		}
 	}
 	return nil