Allow more control over ASG definition

config/config.go

@@ -17,6 +17,7 @@ import (
 	"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"
 	"github.com/coreos/kube-aws/netutil"
 	yaml "gopkg.in/yaml.v2"
 )
@@ -229,6 +230,7 @@ type DeploymentSettings struct {
 
 // Part of configuration which is specific to worker nodes
 type WorkerSettings struct {
+	model.Worker           `yaml:"worker,omitempty"`
 	WorkerCount            int      `yaml:"workerCount,omitempty"`
 	WorkerCreateTimeout    string   `yaml:"workerCreateTimeout,omitempty"`
 	WorkerInstanceType     string   `yaml:"workerInstanceType,omitempty"`
@@ -241,6 +243,7 @@ type WorkerSettings struct {
 
 // Part of configuration which is specific to controller nodes
 type ControllerSettings struct {
+	model.Controller         `yaml:"controller,omitempty"`
 	ControllerCount          int    `yaml:"controllerCount,omitempty"`
 	ControllerCreateTimeout  string `yaml:"controllerCreateTimeout,omitempty"`
 	ControllerInstanceType   string `yaml:"controllerInstanceType,omitempty"`
@@ -281,7 +284,6 @@ type Cluster struct {
 	TLSCertDurationDays    int    `yaml:"tlsCertDurationDays,omitempty"`
 	HostedZone             string `yaml:"hostedZone,omitempty"`
 	HostedZoneID           string `yaml:"hostedZoneId,omitempty"`
-	Worker                 Worker
 	providedEncryptService EncryptService
 }
 
@@ -291,17 +293,6 @@ type Subnet struct {
 	lastAllocatedAddr *net.IP
 }
 
-// Just a place-holder to keep compatibility of cloud-config-worker between main cluster and node pool
-// Without this, {{if .Worker.SpotFleet.Enabled}} in the cloud-config-worker template fails with an obvious error like
-// "executing "CloudConfigWorker" at <.Worker>: can't evaluate field Worker in type *config.Config"
-type Worker struct {
-	SpotFleet SpotFleet
-}
-
-type SpotFleet struct {
-	Enabled bool
-}
-
 type Experimental struct {
 	AuditLog              AuditLog              `yaml:"auditLog"`
 	AwsEnvironment        AwsEnvironment        `yaml:"awsEnvironment"`
@@ -379,11 +370,45 @@ var supportedReleaseChannels = map[string]bool{
 }
 
 func (c WorkerSettings) MinWorkerCount() int {
-	return c.WorkerCount - 1
+	if c.Worker.AutoScalingGroup.MinSize == 0 {
+		return c.WorkerCount
+	}
+	return c.Worker.AutoScalingGroup.MinSize
 }
 
 func (c WorkerSettings) MaxWorkerCount() int {
-	return c.WorkerCount + 1
+	if c.Worker.AutoScalingGroup.MaxSize == 0 {
+		return c.WorkerCount
+	}
+	return c.Worker.AutoScalingGroup.MaxSize
+}
+
+func (c WorkerSettings) WorkerRollingUpdateMinInstancesInService() int {
+	if c.AutoScalingGroup.RollingUpdateMinInstancesInService == 0 {
+		return c.MaxWorkerCount() - 1
+	}
+	return c.AutoScalingGroup.RollingUpdateMinInstancesInService
+}
+
+func (c ControllerSettings) MinControllerCount() int {
+	if c.Controller.AutoScalingGroup.MinSize == 0 {
+		return c.ControllerCount
+	}
+	return c.Controller.AutoScalingGroup.MinSize
+}
+
+func (c ControllerSettings) MaxControllerCount() int {
+	if c.Controller.AutoScalingGroup.MaxSize == 0 {
+		return c.ControllerCount
+	}
+	return c.Controller.AutoScalingGroup.MaxSize
+}
+
+func (c ControllerSettings) ControllerRollingUpdateMinInstancesInService() int {
+	if c.AutoScalingGroup.RollingUpdateMinInstancesInService == 0 {
+		return c.MaxControllerCount() - 1
+	}
+	return c.AutoScalingGroup.RollingUpdateMinInstancesInService
 }
 
 // Required by kubelet to locate the apiserver
@@ -399,9 +424,6 @@ func (c KubeClusterSettings) K8sNetworkPlugin() string {
 func (c Cluster) Config() (*Config, error) {
 	config := Config{Cluster: c}
 
-	config.MinControllerCount = config.ControllerCount - 1
-	config.MaxControllerCount = config.ControllerCount + 1
-
 	// Check if we are running CoreOS 1151.0.0 or greater when using rkt as
 	// runtime. Proceed regardless if running alpha. TODO(pb) delete when rkt
 	// works well with stable.
@@ -630,9 +652,6 @@ type etcdInstance struct {
 type Config struct {
 	Cluster
 
-	MinControllerCount int
-	MaxControllerCount int
-
 	EtcdEndpoints      string
 	EtcdInitialCluster string
 	EtcdInstances      []etcdInstance
@@ -856,6 +875,17 @@ func (c WorkerSettings) Valid() error {
 		}
 	}
 
+	if c.WorkerCount < 0 {
+		return fmt.Errorf("`workerCount` must be zero or greater if specified")
+	}
+	// one is the default WorkerCount
+	if c.WorkerCount != 1 && (c.AutoScalingGroup.MinSize != 0 || c.AutoScalingGroup.MaxSize != 0) {
+		return fmt.Errorf("`worker.autoScalingGroup.minSize` and `worker.autoScalingGroup.maxSize` can only be specified without `workerCount`")
+	}
+	if err := c.AutoScalingGroup.Valid(); err != nil {
+		return err
+	}
+
 	return nil
 }
 
@@ -874,6 +904,17 @@ func (c ControllerSettings) Valid() error {
 		}
 	}
 
+	if c.ControllerCount < 0 {
+		return fmt.Errorf("`controllerCount` must be zero or greater if specified")
+	}
+	// one is the default ControllerCount
+	if c.ControllerCount != 1 && (c.AutoScalingGroup.MinSize != 0 || c.AutoScalingGroup.MaxSize != 0) {
+		return fmt.Errorf("`controller.autoScalingGroup.minSize` and `controller.autoScalingGroup.maxSize` can only be specified without `controllerCount`")
+	}
+	if err := c.AutoScalingGroup.Valid(); err != nil {
+		return err
+	}
+
 	return nil
 }
 

config/config_cluster_size_test.go

@@ -0,0 +1,188 @@
+package config
+
+import (
+	"fmt"
+	"strings"
+	"testing"
+)
+
+const zeroOrGreaterError = "must be zero or greater"
+const disjointConfigError = "can only be specified without"
+const lessThanOrEqualError = "must be less than or equal to"
+
+func TestASGsAllDefaults(t *testing.T) {
+	checkControllerASG(nil, nil, nil, nil, 1, 1, 0, "", t)
+	checkWorkerASG(nil, nil, nil, nil, 1, 1, 0, "", t)
+}
+
+func TestASGsDefaultToMainCount(t *testing.T) {
+	configuredCount := 6
+	checkControllerASG(&configuredCount, nil, nil, nil, 6, 6, 5, "", t)
+	checkWorkerASG(&configuredCount, nil, nil, nil, 6, 6, 5, "", t)
+}
+
+func TestASGsInvalidMainCount(t *testing.T) {
+	configuredCount := -1
+	checkControllerASG(&configuredCount, nil, nil, nil, 0, 0, 0, zeroOrGreaterError, t)
+	checkWorkerASG(&configuredCount, nil, nil, nil, 0, 0, 0, zeroOrGreaterError, t)
+}
+
+func TestASGsOnlyMinConfigured(t *testing.T) {
+	configuredMin := 4
+	// we expect min cannot be configured without a max
+	checkControllerASG(nil, &configuredMin, nil, nil, 0, 0, 0, lessThanOrEqualError, t)
+	checkWorkerASG(nil, &configuredMin, nil, nil, 0, 0, 0, lessThanOrEqualError, t)
+}
+
+func TestASGsOnlyMaxConfigured(t *testing.T) {
+	configuredMax := 3
+	// we expect min to be equal to main count if only max specified
+	checkControllerASG(nil, nil, &configuredMax, nil, 1, 3, 2, "", t)
+	checkWorkerASG(nil, nil, &configuredMax, nil, 1, 3, 2, "", t)
+}
+
+func TestASGsMinMaxConfigured(t *testing.T) {
+	configuredMin := 2
+	configuredMax := 5
+	checkControllerASG(nil, &configuredMin, &configuredMax, nil, 2, 5, 4, "", t)
+	checkWorkerASG(nil, &configuredMin, &configuredMax, nil, 2, 5, 4, "", t)
+}
+
+func TestASGsInvalidMin(t *testing.T) {
+	configuredMin := -1
+	configuredMax := 5
+	checkControllerASG(nil, &configuredMin, &configuredMax, nil, 0, 0, 0, zeroOrGreaterError, t)
+	checkWorkerASG(nil, &configuredMin, &configuredMax, nil, 0, 0, 0, zeroOrGreaterError, t)
+}
+
+func TestASGsInvalidMax(t *testing.T) {
+	configuredMin := 1
+	configuredMax := -1
+	checkControllerASG(nil, &configuredMin, &configuredMax, nil, 0, 0, 0, zeroOrGreaterError, t)
+	checkWorkerASG(nil, &configuredMin, &configuredMax, nil, 0, 0, 0, zeroOrGreaterError, t)
+}
+
+func TestASGsMinConfiguredWithMainCount(t *testing.T) {
+	configuredCount := 2
+	configuredMin := 4
+	checkControllerASG(&configuredCount, &configuredMin, nil, nil, 0, 0, 0, disjointConfigError, t)
+	checkWorkerASG(&configuredCount, &configuredMin, nil, nil, 0, 0, 0, disjointConfigError, t)
+}
+
+func TestASGsMaxConfiguredWithMainCount(t *testing.T) {
+	configuredCount := 2
+	configuredMax := 4
+	checkControllerASG(&configuredCount, nil, &configuredMax, nil, 0, 0, 0, disjointConfigError, t)
+	checkWorkerASG(&configuredCount, nil, &configuredMax, nil, 0, 0, 0, disjointConfigError, t)
+}
+
+func TestASGsMinMaxConfiguredWithMainCount(t *testing.T) {
+	configuredCount := 2
+	configuredMin := 3
+	configuredMax := 4
+	checkControllerASG(&configuredCount, &configuredMin, &configuredMax, nil, 0, 0, 0, disjointConfigError, t)
+	checkWorkerASG(&configuredCount, &configuredMin, &configuredMax, nil, 0, 0, 0, disjointConfigError, t)
+}
+
+func TestASGsMinInServiceConfigured(t *testing.T) {
+	configuredMin := 5
+	configuredMax := 10
+	configuredMinInService := 7
+	checkControllerASG(nil, &configuredMin, &configuredMax, &configuredMinInService, 5, 10, 7, "", t)
+	checkWorkerASG(nil, &configuredMin, &configuredMax, &configuredMinInService, 5, 10, 7, "", t)
+}
+
+const testConfig = minimalConfigYaml + `
+subnets:
+  - availabilityZone: ap-northeast-1a
+    instanceCIDR: 10.0.1.0/24
+  - availabilityZone: ap-northeast-1c
+    instanceCIDR: 10.0.2.0/24
+`
+
+func checkControllerASG(configuredCount *int, configuredMin *int, configuredMax *int, configuredMinInstances *int,
+	expectedMin int, expectedMax int, expectedMinInstances int, expectedError string, t *testing.T) {
+	config := testConfig
+	if configuredCount != nil {
+		config += fmt.Sprintf("controllerCount: %d\n", *configuredCount)
+	}
+	config += "controller:\n" + buildASGConfig(configuredMin, configuredMax, configuredMinInstances)
+
+	cluster, err := ClusterFromBytes([]byte(config))
+	if err != nil {
+		if expectedError == "" || !strings.Contains(err.Error(), expectedError) {
+			t.Errorf("Failed to validate cluster with controller config: %v", err)
+		}
+	} else {
+		config, err := cluster.Config()
+		if err != nil {
+			t.Errorf("Failed to create cluster config: %v", err)
+		} else {
+			if config.MinControllerCount() != expectedMin {
+				t.Errorf("Controller ASG min count did not match the expected value: actual value of %d != expected value of %d",
+					config.MinControllerCount(), expectedMin)
+			}
+			if config.MaxControllerCount() != expectedMax {
+				t.Errorf("Controller ASG max count did not match the expected value: actual value of %d != expected value of %d",
+					config.MaxControllerCount(), expectedMax)
+			}
+			if config.ControllerRollingUpdateMinInstancesInService() != expectedMinInstances {
+				t.Errorf("Controller ASG rolling update min instances count did not match the expected value: actual value of %d != expected value of %d",
+					config.ControllerRollingUpdateMinInstancesInService(), expectedMinInstances)
+			}
+		}
+	}
+}
+
+func checkWorkerASG(configuredCount *int, configuredMin *int, configuredMax *int, configuredMinInstances *int,
+	expectedMin int, expectedMax int, expectedMinInstances int, expectedError string, t *testing.T) {
+	config := testConfig
+	if configuredCount != nil {
+		config += fmt.Sprintf("workerCount: %d\n", *configuredCount)
+	}
+	config += "worker:\n" + buildASGConfig(configuredMin, configuredMax, configuredMinInstances)
+
+	cluster, err := ClusterFromBytes([]byte(config))
+	if err != nil {
+		if expectedError == "" || !strings.Contains(err.Error(), expectedError) {
+			t.Errorf("Failed to validate cluster with worker config: %v", err)
+		}
+	} else {
+		config, err := cluster.Config()
+		if err != nil {
+			if expectedError == "" || !strings.Contains(err.Error(), expectedError) {
+				t.Errorf("Failed to create cluster config: %v", err)
+			}
+		} else {
+			if config.MinWorkerCount() != expectedMin {
+				t.Errorf("Worker ASG min count did not match the expected value: actual value of %d != expected value of %d",
+					config.MinWorkerCount(), expectedMin)
+			}
+			if config.MaxWorkerCount() != expectedMax {
+				t.Errorf("Worker ASG max count did not match the expected value: actual value of %d != expected value of %d",
+					config.MaxWorkerCount(), expectedMax)
+			}
+			if config.WorkerRollingUpdateMinInstancesInService() != expectedMinInstances {
+				t.Errorf("Worker ASG rolling update min instances count did not match the expected value: actual value of %d != expected value of %d",
+					config.WorkerRollingUpdateMinInstancesInService(), expectedMinInstances)
+			}
+		}
+	}
+}
+
+func buildASGConfig(configuredMin *int, configuredMax *int, configuredMinInstances *int) string {
+	asg := ""
+	if configuredMin != nil {
+		asg += fmt.Sprintf("    minSize: %d\n", *configuredMin)
+	}
+	if configuredMax != nil {
+		asg += fmt.Sprintf("    maxSize: %d\n", *configuredMax)
+	}
+	if configuredMinInstances != nil {
+		asg += fmt.Sprintf("    rollingUpdateMinInstancesInService: %d\n", *configuredMinInstances)
+	}
+	if asg != "" {
+		return "  autoScalingGroup:\n" + asg
+	}
+	return ""
+}

config/templates/cluster.yaml

@@ -50,9 +50,16 @@ availabilityZone: {{.AvailabilityZone}}
 # ARN of the KMS key used to encrypt TLS assets.
 kmsKeyArn: "{{.KMSKeyARN}}"
 
-# Number of controller nodes to create
+# Number of controller nodes to create, for more control use `controller.autoScalingGroup` and do not use this setting
 #controllerCount: 1
 
+controller:
+  # Auto Scaling Group definition for controllers. If only `controllerCount` is specified, min and max will be the set to that value and `rollingUpdateMinInstancesInService` will be one less.
+  # autoScalingGroup:
+  #   minSize: 1
+  #   maxSize: 3
+  #   rollingUpdateMinInstancesInService: 2
+
 # Maximum time to wait for controller creation
 #controlerCreateTimeout: PT15M
 
@@ -68,9 +75,16 @@ kmsKeyArn: "{{.KMSKeyARN}}"
 # Number of I/O operations per second (IOPS) that the controller node disk supports. Leave blank if controllerRootVolumeType is not io1
 #controllerRootVolumeIOPS: 0
 
-# Number of worker nodes to create
+# Number of worker nodes to create, for more control use `worker.autoScalingGroup` and do not use this setting
 #workerCount: 1
 
+worker:
+  # Auto Scaling Group definition for workers. If only `workerCount` is specified, min and max will be the set to that value and `rollingUpdateMinInstancesInService` will be one less.
+  # autoScalingGroup:
+  #   minSize: 1
+  #   maxSize: 3
+  #   rollingUpdateMinInstancesInService: 2
+
 # Maximum time to wait for worker creation
 #workerCreateTimeout: PT15M