Use existing subnets when creating/updating cluster (#227)

This commit introduces new keys named `subnetId` and `subnetLogiaclName` into items under the top-level `subnets` in `cluster.yaml` to allow deployments to existing subnets while customizing logical names of subnets. Notes on the current implementation: * `subnetId` will be changed to just `id` in the future. See #227 (comment) * Route table associations and EFS mount targets may fail while being created in in some cases. See #227 (comment) and #227 (comment) Notes from original commits before squashing: * Launch in existing subnets * Validate update and export * Don't try to validate config during update * Move r53 schecks to create, update nodepool stack template
kubernetes-retired · Jan 24, 2017 · 68d88b9 · 68d88b9
1 parent c538e61
commit 68d88b9
Show file tree

Hide file tree

Showing 8 changed files with 96 additions and 90 deletions.
diff --git a/cluster/cluster.go b/cluster/cluster.go
@@ -151,12 +151,7 @@ func (c *Cluster) stackProvisioner() *cfnstack.Provisioner {
 	return cfnstack.NewProvisioner(c.StackName(), c.WorkerDeploymentSettings().StackTags(), stackPolicyBody, c.session)
 }
 
-func (c *Cluster) Create(stackBody string, s3URI string) error {
-	r53Svc := route53.New(c.session)
-	if err := c.validateDNSConfig(r53Svc); err != nil {
-		return err
-	}
-
+func (c *Cluster) Validate(stackBody string, s3URI string) error {
 	ec2Svc := ec2.New(c.session)
 	if c.KeyName != "" {
 		if err := c.validateKeyPair(ec2Svc); err != nil {
@@ -176,6 +171,19 @@ func (c *Cluster) Create(stackBody string, s3URI string) error {
 		return err
 	}
 
+	return nil
+}
+
+func (c *Cluster) Create(stackBody string, s3URI string) error {
+	r53Svc := route53.New(c.session)
+	if err := c.validateDNSConfig(r53Svc); err != nil {
+		return err
+	}
+
+	if err := c.Validate(stackBody, s3URI); err != nil {
+		return err
+	}
+
 	cfSvc := cloudformation.New(c.session)
 	s3Svc := s3.New(c.session)
 

diff --git a/cmd/up.go b/cmd/up.go
@@ -3,10 +3,11 @@ package cmd
 import (
 	"fmt"
 
+	"io/ioutil"
+
 	"github.com/coreos/kube-aws/cluster"
 	"github.com/coreos/kube-aws/config"
 	"github.com/spf13/cobra"
-	"io/ioutil"
 )
 
 var (
@@ -47,6 +48,12 @@ func runCmdUp(cmd *cobra.Command, args []string) error {
 		return fmt.Errorf("Failed to render stack template: %v", err)
 	}
 
+	cluster := cluster.New(conf, upOpts.awsDebug)
+
+	if err := cluster.Validate(string(data), upOpts.s3URI); err != nil {
+		return fmt.Errorf("Error validating cluster: %v", err)
+	}
+
 	if upOpts.export {
 		templatePath := fmt.Sprintf("%s.stack-template.json", conf.ClusterName)
 		fmt.Printf("Exporting %s\n", templatePath)
@@ -59,7 +66,6 @@ func runCmdUp(cmd *cobra.Command, args []string) error {
 		return nil
 	}
 
-	cluster := cluster.New(conf, upOpts.awsDebug)
 	fmt.Printf("Creating AWS resources. This should take around 5 minutes.\n")
 	if err := cluster.Create(string(data), upOpts.s3URI); err != nil {
 		return fmt.Errorf("Error creating cluster: %v", err)

diff --git a/config/config.go b/config/config.go
@@ -303,6 +303,8 @@ type Subnet struct {
 	AvailabilityZone  string `yaml:"availabilityZone,omitempty"`
 	InstanceCIDR      string `yaml:"instanceCIDR,omitempty"`
 	lastAllocatedAddr *net.IP
+	SubnetId          string `yaml:"subnetId,omitempty"`
+	SubnetLogicalName string
 }
 
 type Experimental struct {
@@ -812,7 +814,11 @@ func (c DeploymentSettings) Valid() (*DeploymentValidationResult, error) {
 		}
 
 		var instanceCIDRs = make([]*net.IPNet, 0)
+
 		for i, subnet := range c.Subnets {
+			if subnet.SubnetId != "" {
+				continue
+			}
 			if subnet.AvailabilityZone == "" {
 				return nil, fmt.Errorf("availabilityZone must be set for subnet #%d", i)
 			}
@@ -935,7 +941,6 @@ Validates the an existing VPC and it's existing subnets do not conflict with thi
 cluster configuration
 */
 func (c *Cluster) ValidateExistingVPC(existingVPCCIDR string, existingSubnetCIDRS []string) error {
-
 	_, existingVPC, err := net.ParseCIDR(existingVPCCIDR)
 	if err != nil {
 		return fmt.Errorf("error parsing existing vpc cidr %s : %v", existingVPCCIDR, err)
@@ -970,19 +975,23 @@ func (c *Cluster) ValidateExistingVPC(existingVPCCIDR string, existingSubnetCIDR
 	// Loop through all subnets
 	// Note: legacy instanceCIDR/availabilityZone stuff has already been marshalled into this format
 	for _, subnet := range c.Subnets {
-		_, instanceNet, err := net.ParseCIDR(subnet.InstanceCIDR)
-		if err != nil {
-			return fmt.Errorf("error parsing instances cidr %s : %v", c.InstanceCIDR, err)
-		}
+		if subnet.SubnetId != "" {
+			continue
+		} else {
+			_, instanceNet, err := net.ParseCIDR(subnet.InstanceCIDR)
+			if err != nil {
+				return fmt.Errorf("error parsing instances cidr %s : %v", c.InstanceCIDR, err)
+			}
 
-		//Loop through all existing subnets in the VPC and look for conflicting CIDRS
-		for _, existingSubnet := range existingSubnets {
-			if netutil.CidrOverlap(instanceNet, existingSubnet) {
-				return fmt.Errorf(
-					"instance cidr (%s) conflicts with existing subnet cidr=%s",
-					instanceNet,
-					existingSubnet,
-				)
+			//Loop through all existing subnets in the VPC and look for conflicting CIDRS
+			for _, existingSubnet := range existingSubnets {
+				if netutil.CidrOverlap(instanceNet, existingSubnet) {
+					return fmt.Errorf(
+						"instance cidr (%s) conflicts with existing subnet cidr=%s",
+						instanceNet,
+						existingSubnet,
+					)
+				}
 			}
 		}
 	}
@@ -1075,3 +1084,11 @@ func withHostedZoneIDPrefix(id string) string {
 	}
 	return id
 }
+
+// Ref returns SubnetId or ref to newly created resource
+func (s Subnet) Ref() string {
+	if s.SubnetId != "" {
+		return fmt.Sprintf("%q", s.SubnetId)
+	}
+	return fmt.Sprintf(`{"Ref" : "%s"}`, s.SubnetLogicalName)
+}
diff --git a/config/config_test.go b/config/config_test.go
@@ -3,14 +3,15 @@ package config
 import (
 	"bytes"
 	"fmt"
-	"github.com/coreos/kube-aws/netutil"
-	"github.com/coreos/kube-aws/test/helper"
-	"gopkg.in/yaml.v2"
 	"net"
 	"reflect"
 	"strings"
 	"testing"
 	"text/template"
+
+	"github.com/coreos/kube-aws/netutil"
+	"github.com/coreos/kube-aws/test/helper"
+	"gopkg.in/yaml.v2"
 )
 
 const minimalConfigYaml = `externalDNSName: test.staging.core-os.net
@@ -874,8 +875,8 @@ func TestValidateExistingVPC(t *testing.T) {
 
 	cluster.VPCCIDR = "10.0.0.0/16"
 	cluster.Subnets = []*Subnet{
-		{"ap-northeast-1a", "10.0.1.0/24", nil},
-		{"ap-northeast-1a", "10.0.2.0/24", nil},
+		{"ap-northeast-1a", "10.0.1.0/24", nil, "", ""},
+		{"ap-northeast-1a", "10.0.2.0/24", nil, "", ""},
 	}
 
 	for _, testCase := range validCases {
@@ -900,8 +901,8 @@ func TestValidateUserData(t *testing.T) {
 
 	cluster.Region = "us-west-1"
 	cluster.Subnets = []*Subnet{
-		{"us-west-1a", "10.0.1.0/16", nil},
-		{"us-west-1b", "10.0.2.0/16", nil},
+		{"us-west-1a", "10.0.1.0/16", nil, "", ""},
+		{"us-west-1b", "10.0.2.0/16", nil, "", ""},
 	}
 
 	helper.WithDummyCredentials(func(dir string) {
@@ -924,8 +925,8 @@ func TestRenderStackTemplate(t *testing.T) {
 
 	cluster.Region = "us-west-1"
 	cluster.Subnets = []*Subnet{
-		{"us-west-1a", "10.0.1.0/16", nil},
-		{"us-west-1b", "10.0.2.0/16", nil},
+		{"us-west-1a", "10.0.1.0/16", nil, "", ""},
+		{"us-west-1b", "10.0.2.0/16", nil, "", ""},
 	}
 
 	helper.WithDummyCredentials(func(dir string) {

diff --git a/config/templates/cluster.yaml b/config/templates/cluster.yaml
@@ -171,8 +171,10 @@ controller:
 # subnets:
 #   - availabilityZone: us-west-1a
 #     instanceCIDR: "10.0.0.0/24"
+#     subnetId: "subnet-xxxxxxxx" #optional
 #   - availabilityZone: us-west-1b
 #     instanceCIDR: "10.0.1.0/24"
+#     subnetId: "subnet-xxxxxxxx" #optional
 
 # CIDR for all service IP addresses
 # serviceCIDR: "10.3.0.0/24"

diff --git a/config/templates/stack-template.json b/config/templates/stack-template.json
@@ -51,12 +51,8 @@
         {{end}}
         "VPCZoneIdentifier": [
           {{range $index, $subnet := .Subnets}}
-          {{with $subnetLogicalName := printf "Subnet%d" $index}}
           {{if gt $index 0}},{{end}}
-          {
-            "Ref": "{{$subnetLogicalName}}"
-          }
-          {{end}}
+          {{$subnet.Ref}}
           {{end}}
         ]
       },
@@ -159,12 +155,8 @@
         ],
         "VPCZoneIdentifier": [
           {{range $index, $subnet := .Subnets}}
-          {{with $subnetLogicalName := printf "Subnet%d" $index}}
           {{if gt $index 0}},{{end}}
-          {
-            "Ref": "{{$subnetLogicalName}}"
-          }
-          {{end}}
+          {{$subnet.Ref}}
           {{end}}
         ],
         "LoadBalancerNames" : [
@@ -660,7 +652,7 @@
 	"Subnets" : [
           {{range $index, $subnet := .Subnets}}
           {{if gt $index 0}},{{end}}
-          { "Ref" : "Subnet{{$index}}" }
+          {{$subnet.Ref}}
           {{end}}
 	],
 	"Listeners" : [
@@ -1084,17 +1076,17 @@
     }
     {{end}}
     {{range $index, $subnet := .Subnets}}
-    {{with $subnetLogicalName := printf "Subnet%d" $index}}
+    {{if not $subnet.SubnetId }}
     ,
-    "{{$subnetLogicalName}}": {
+    "{{$subnet.SubnetLogicalName}}": {
       "Properties": {
         "AvailabilityZone": "{{$subnet.AvailabilityZone}}",
         "CidrBlock": "{{$subnet.InstanceCIDR}}",
         "MapPublicIpOnLaunch": {{$.MapPublicIPs}},
         "Tags": [
           {
             "Key": "Name",
-            "Value": "{{$.ClusterName}}-{{$subnetLogicalName}}"
+            "Value": "{{$.ClusterName}}-{{$subnet.SubnetLogicalName}}"
           },
           {
             "Key": "KubernetesCluster",
@@ -1105,19 +1097,19 @@
       },
       "Type": "AWS::EC2::Subnet"
     }
+    {{end}}
     {{if $.ElasticFileSystemID}}
     ,
-    "{{$subnetLogicalName}}MountTarget": {
+    "{{$subnet.SubnetLogicalName}}MountTarget": {
       "Properties" : {
         "FileSystemId": "{{$.ElasticFileSystemID}}",
-        "SubnetId": { "Ref": "{{$subnetLogicalName}}" },
+        "SubnetId": {{$subnet.Ref}},
         "SecurityGroups": [ { "Ref": "SecurityGroupMountTarget" } ]
       },
       "Type" : "AWS::EFS::MountTarget"
     }
     {{end}}
     {{end}}
-    {{end}}
     {{if not .VPCID}}
     ,
     "{{.VPCLogicalName}}": {
@@ -1190,14 +1182,12 @@
       "Type": "AWS::EC2::VPCGatewayAttachment"
     }
     {{range $index, $subnet := .Subnets}}
-    {{with $subnetLogicalName := printf "Subnet%d" $index}}
+    {{if not $subnet.SubnetId }}
     ,
-    "{{$subnetLogicalName}}RouteTableAssociation": {
+    "{{$subnet.SubnetLogicalName}}RouteTableAssociation": {
       "Properties": {
         "RouteTableId": { "Ref" : "RouteTable"},
-        "SubnetId": {
-          "Ref": "{{$subnetLogicalName}}"
-        }
+        "SubnetId": {{$subnet.Ref}}
       },
       "Type": "AWS::EC2::SubnetRouteTableAssociation"
     }
@@ -1206,14 +1196,12 @@
     {{else}}
     {{if .RouteTableID}}
     {{range $index, $subnet := .Subnets}}
-    {{with $subnetLogicalName := printf "Subnet%d" $index}}
+    {{if not $subnet.SubnetId }}
     ,
-    "{{$subnetLogicalName}}RouteTableAssociation": {
+    "{{$subnet.SubnetLogicalName}}RouteTableAssociation": {
       "Properties": {
         "RouteTableId": "{{$.RouteTableID}}",
-        "SubnetId": {
-          "Ref": "{{$subnetLogicalName}}"
-        }
+        "SubnetId": {{$subnet.Ref}}
       },
       "Type": "AWS::EC2::SubnetRouteTableAssociation"
     }

diff --git a/nodepool/config/config.go b/nodepool/config/config.go
@@ -7,14 +7,15 @@ import (
 	"fmt"
 	"io/ioutil"
 
+	"path/filepath"
+
 	cfg "github.com/coreos/kube-aws/config"
 	"github.com/coreos/kube-aws/coreos/amiregistry"
 	"github.com/coreos/kube-aws/coreos/userdatavalidation"
 	"github.com/coreos/kube-aws/filereader/jsontemplate"
 	"github.com/coreos/kube-aws/filereader/userdatatemplate"
 	model "github.com/coreos/kube-aws/model"
 	"gopkg.in/yaml.v2"
-	"path/filepath"
 )
 
 type Ref struct {