Skip to content
This repository has been archived by the owner on Sep 30, 2020. It is now read-only.

Commit

Permalink
Allow more control over ASG definition (#142)
Browse files Browse the repository at this point in the history
* Allow more control over ASG definition

Min/max size and min instances in service can now be directly defined.
Defaults take from the older `controllerCount` and `workerCount`

* Start to unify worker/controller configuration

- Start model package
- Fix up nodepool ASGs as they only inherit static default values
- Move validation into Valid() methods
- Move ASG config into `worker` and `controller`
- Maintain backwards compatibility for now on
MinWorkerCount/MaxWorkerCount
- Increase validation for setting count along with min/max
- Remove desired capacity, it currently do not do much
  • Loading branch information
c-knowles authored and mumoshu committed Dec 14, 2016
1 parent 56f08d5 commit 0cc915e
Show file tree
Hide file tree
Showing 11 changed files with 322 additions and 42 deletions.
81 changes: 61 additions & 20 deletions config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand Down Expand Up @@ -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"`
Expand All @@ -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"`
Expand Down Expand Up @@ -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
}

Expand All @@ -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"`
Expand Down Expand Up @@ -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
Expand All @@ -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.
Expand Down Expand Up @@ -630,9 +652,6 @@ type etcdInstance struct {
type Config struct {
Cluster

MinControllerCount int
MaxControllerCount int

EtcdEndpoints string
EtcdInitialCluster string
EtcdInstances []etcdInstance
Expand Down Expand Up @@ -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
}

Expand All @@ -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
}

Expand Down
188 changes: 188 additions & 0 deletions config/config_cluster_size_test.go
Original file line number Diff line number Diff line change
@@ -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 ""
}
18 changes: 16 additions & 2 deletions config/templates/cluster.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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

Expand Down
Loading

0 comments on commit 0cc915e

Please sign in to comment.