Error out when there is ambiguity in cluster.yaml. Which configuratio…

…n did you want, single AZ or multi AZ?

ref coreos#439 (comment)

multi-node/aws/pkg/config/config.go

@@ -32,7 +32,6 @@ func newDefaultCluster() *Cluster {
 		ClusterName:              "kubernetes",
 		ReleaseChannel:           "alpha",
 		VPCCIDR:                  "10.0.0.0/16",
-		InstanceCIDR:             "10.0.0.0/24",
 		ControllerIP:             "10.0.0.50",
 		PodCIDR:                  "10.2.0.0/16",
 		ServiceCIDR:              "10.3.0.0/24",
@@ -75,18 +74,27 @@ func ClusterFromBytes(data []byte) (*Cluster, error) {
 	// as it will with RecordSets
 	c.HostedZone = WithTrailingDot(c.HostedZone)
 
+	// If the user specified no subnets, we assume that a single AZ configuration with the default instanceCIDR is demanded
+	if len(c.Subnets) == 0 && c.InstanceCIDR == "" {
+		c.InstanceCIDR = "10.0.0.0/24"
+	}
+
+	if err := c.valid(); err != nil {
+		return nil, fmt.Errorf("invalid cluster: %v", err)
+	}
+
+	// For backward-compatibility
 	if len(c.Subnets) == 0 {
+		if c.InstanceCIDR == "" {
+			c.InstanceCIDR = "10.0.0.0/24"
+		}
 		c.Subnets = []Subnet{
 			{
 				AvailabilityZone: c.AvailabilityZone,
 				InstanceCIDR:     c.InstanceCIDR,
 			},
 		}
 	}
-
-	if err := c.valid(); err != nil {
-		return nil, fmt.Errorf("invalid cluster: %v", err)
-	}
 	return c, nil
 }
 
@@ -403,35 +411,71 @@ func (cfg Cluster) valid() error {
 		return fmt.Errorf("invalid controllerIP: %s", cfg.ControllerIP)
 	}
 
-	var instanceCIDRs = make([]*net.IPNet, 0)
-	for i, subnet := range cfg.Subnets {
-		if subnet.AvailabilityZone == "" {
-			return fmt.Errorf("availabilityZone must be set for subnet #%d", i)
+	if len(cfg.Subnets) == 0 {
+		if cfg.AvailabilityZone == "" {
+			return fmt.Errorf("availabilityZone must be set")
 		}
-		_, instanceCIDR, err := net.ParseCIDR(subnet.InstanceCIDR)
+		_, instanceCIDR, err := net.ParseCIDR(cfg.InstanceCIDR)
 		if err != nil {
-			return fmt.Errorf("invalid instanceCIDR for subnet #%d: %v", i, err)
+			return fmt.Errorf("invalid instanceCIDR: %v", err)
 		}
-		instanceCIDRs = append(instanceCIDRs, instanceCIDR)
 		if !vpcNet.Contains(instanceCIDR.IP) {
-			return fmt.Errorf("vpcCIDR (%s) does not contain instanceCIDR (%s) for subnet #%d",
+			return fmt.Errorf("vpcCIDR (%s) does not contain instanceCIDR (%s)",
 				cfg.VPCCIDR,
 				cfg.InstanceCIDR,
-				i,
 			)
 		}
-		if i == 0 && !instanceCIDR.Contains(controllerIPAddr) {
+		if !instanceCIDR.Contains(controllerIPAddr) {
 			return fmt.Errorf("instanceCIDR (%s) does not contain controllerIP (%s)",
-				subnet.InstanceCIDR,
+				cfg.InstanceCIDR,
+				cfg.ControllerIP,
+			)
+		}
+	} else {
+		if cfg.InstanceCIDR != "" {
+			return fmt.Errorf("The top-level instanceCIDR(%s) must be empty when subnets are specified", cfg.InstanceCIDR)
+		}
+		if cfg.AvailabilityZone != "" {
+			return fmt.Errorf("The top-level availabilityZone(%s) must be empty when subnets are specified", cfg.AvailabilityZone)
+		}
+
+		var instanceCIDRs = make([]*net.IPNet, 0)
+		for i, subnet := range cfg.Subnets {
+			if subnet.AvailabilityZone == "" {
+				return fmt.Errorf("availabilityZone must be set for subnet #%d", i)
+			}
+			_, instanceCIDR, err := net.ParseCIDR(subnet.InstanceCIDR)
+			if err != nil {
+				return fmt.Errorf("invalid instanceCIDR for subnet #%d: %v", i, err)
+			}
+			instanceCIDRs = append(instanceCIDRs, instanceCIDR)
+			if !vpcNet.Contains(instanceCIDR.IP) {
+				return fmt.Errorf("vpcCIDR (%s) does not contain instanceCIDR (%s) for subnet #%d",
+					cfg.VPCCIDR,
+					cfg.InstanceCIDR,
+					i,
+				)
+			}
+		}
+
+		controllerInstanceCidrExists := false
+		for _, a := range instanceCIDRs {
+			if a.Contains(controllerIPAddr) {
+				controllerInstanceCidrExists = true
+			}
+		}
+		if !controllerInstanceCidrExists {
+			return fmt.Errorf("No instanceCIDRs in Subnets (%v) contain controllerIP (%s)",
+				instanceCIDRs,
 				cfg.ControllerIP,
 			)
 		}
-	}
 
-	for i, a := range instanceCIDRs {
-		for j, b := range instanceCIDRs[i+1:] {
-			if i > 0 && cidrOverlap(a, b) {
-				return fmt.Errorf("CIDR of subnet %d (%s) overlaps with CIDR of subnet %d (%s)", i, a, j, b)
+		for i, a := range instanceCIDRs {
+			for j, b := range instanceCIDRs[i+1:] {
+				if i > 0 && cidrOverlap(a, b) {
+					return fmt.Errorf("CIDR of subnet %d (%s) overlaps with CIDR of subnet %d (%s)", i, a, j, b)
+				}
 			}
 		}
 	}

multi-node/aws/pkg/config/config_test.go

@@ -9,11 +9,16 @@ import (
 const minimalConfigYaml = `externalDNSName: test.staging.core-os.net
 keyName: test-key-name
 region: us-west-1
-availabilityZone: us-west-1c
 clusterName: test-cluster-name
 kmsKeyArn: "arn:aws:kms:us-west-1:xxxxxxxxx:key/xxxxxxxxxxxxxxxxxxx"
 `
 
+const availabilityZoneConfig = `
+availabilityZone: us-west-1c
+`
+
+const singleAzConfigYaml = minimalConfigYaml + availabilityZoneConfig
+
 var goodNetworkingConfigs = []string{
 	``, //Tests validity of default network config values
 	`
@@ -116,14 +121,14 @@ hostedZone: "whatever.com"
 func TestNetworkValidation(t *testing.T) {
 
 	for _, networkConfig := range goodNetworkingConfigs {
-		configBody := minimalConfigYaml + networkConfig
+		configBody := singleAzConfigYaml + networkConfig
 		if _, err := ClusterFromBytes([]byte(configBody)); err != nil {
 			t.Errorf("Correct config tested invalid: %s\n%s", err, networkConfig)
 		}
 	}
 
 	for _, networkConfig := range incorrectNetworkingConfigs {
-		configBody := minimalConfigYaml + networkConfig
+		configBody := singleAzConfigYaml + networkConfig
 		if _, err := ClusterFromBytes([]byte(configBody)); err == nil {
 			t.Errorf("Incorrect config tested valid, expected error:\n%s", networkConfig)
 		}
@@ -170,7 +175,7 @@ dnsServiceIP: 10.6.142.100
 	}
 
 	for _, testConfig := range testConfigs {
-		configBody := minimalConfigYaml + testConfig.NetworkConfig
+		configBody := singleAzConfigYaml + testConfig.NetworkConfig
 		cluster, err := ClusterFromBytes([]byte(configBody))
 		if err != nil {
 			t.Errorf("Unexpected error parsing config: %v\n %s", err, configBody)
@@ -286,7 +291,7 @@ releaseChannel: non-existant #this release channel will never exist
 	}
 
 	for _, conf := range validConfigs {
-		confBody := minimalConfigYaml + conf.conf
+		confBody := singleAzConfigYaml + conf.conf
 		c, err := ClusterFromBytes([]byte(confBody))
 		if err != nil {
 			t.Errorf("failed to parse config %s: %v", confBody, err)
@@ -302,7 +307,7 @@ releaseChannel: non-existant #this release channel will never exist
 	}
 
 	for _, conf := range invalidConfigs {
-		confBody := minimalConfigYaml + conf
+		confBody := singleAzConfigYaml + conf
 		_, err := ClusterFromBytes([]byte(confBody))
 		if err == nil {
 			t.Errorf("expected error parsing invalid config: %s", confBody)
@@ -399,7 +404,22 @@ availabilityZone: "ap-northeast-1a"
 
 	invalidConfigs := []string{
 		`
+# You can't specify both the top-level availability zone and subnets
+# (It doesn't make sense. Which configuration did you want, single or multi AZ one?)
 availabilityZone: "ap-northeast-1a"
+subnets:
+  - availabilityZone: "ap-northeast-1b"
+    instanceCIDR: "10.0.0.0/24"
+`,
+		`
+# You can't specify both the top-level instanceCIDR and subnets
+# (It doesn't make sense. Which configuration did you want, single or multi AZ one?)
+instanceCIDR: "10.0.0.0/24"
+subnets:
+- availabilityZone: "ap-northeast-1b"
+  instanceCIDR: "10.0.1.0/24"
+`,
+		`
 subnets:
 # Missing AZ like this
 # - availabilityZone: "ap-northeast-1a"

multi-node/aws/pkg/config/tls_config_test.go

@@ -9,7 +9,7 @@ import (
 )
 
 func genTLSAssets(t *testing.T) *RawTLSAssets {
-	cluster, err := ClusterFromBytes([]byte(minimalConfigYaml))
+	cluster, err := ClusterFromBytes([]byte(singleAzConfigYaml))
 	if err != nil {
 		t.Fatalf("failed generating config: %v", err)
 	}

multi-node/aws/pkg/config/user_data_config_test.go

@@ -19,7 +19,7 @@ func (d *dummyEncryptService) Encrypt(input *kms.EncryptInput) (*kms.EncryptOutp
 }
 
 func TestCloudConfigTemplating(t *testing.T) {
-	cluster, err := ClusterFromBytes([]byte(minimalConfigYaml))
+	cluster, err := ClusterFromBytes([]byte(singleAzConfigYaml))
 	if err != nil {
 		t.Fatalf("Unable to load cluster config: %v", err)
 	}