cluster-autoscaler: support additional Brightbox server groups

cluster-autoscaler/cloudprovider/brightbox

Allows autoscaled servers to be members of further server groups
in addition to the default and the scaling group.

cluster-autoscaler/cloudprovider/brightbox/README.md

@@ -8,26 +8,29 @@ Builder](https://github.com/brightbox/kubernetes-cluster)
 
 # How Autoscaler works on Brightbox Cloud
 
-The autoscaler looks for [Server
-Groups](https://www.brightbox.com/docs/guides/cli/server-groups/) named
-after the cluster-name option passed to the autoscaler (--cluster-name).
-
-A group named with a suffix of the cluster-name
-(e.g. k8s-worker.k8s-test.cluster.local) is a candidate to be a scaling
-group. The autoscaler will then check the description to see if it is
-a pair of integers separated by a colon (e.g. 1:4). If it finds those
-numbers then they will become the minimum and maximum server size for
-that group, and autoscaler will attempt to scale the group between those sizes.
-
-The type of server, the image used  and the target zone will be
-dynamically determined from the existing members. If these differ, or
-there are no existing servers, autoscaler will log an error and will not
-scale that group.
-
-A group named precisely the same as the cluster-name
-(e.g. k8s-test.cluster.local) is considered to be the default cluster
-group and all autoscaled servers created are placed within it as well
-as the scaling group.
+The autoscaler looks for the first [Config
+Map](https://api.gb1.brightbox.com/1.0/index.html) with a name that has
+a suffix the same as the `cluster-name` option passed to the autoscaler
+(`--cluster-name`).
+
+The config map data consist of a colon separated key-value pairs. The
+key and the value are treated as strings.
+
+```
+server_group: grp-sda44
+min: 1
+max: 4
+default_group: grp-y6cai
+additional_groups: grp-abcde,grp-testy,grp-winga
+image: img-testy
+zone: zon-testy
+user_data: <base64 encoded userdata>
+```
+
+The `server_group`, `min` and `max` items are required. All the rest
+are optional. Additional Groups should be comma separated without spaces.
+
+The names of the autocreated servers are derived from the name of the config map.
 
 The Brightbox Cloud provider only supports auto-discovery mode using
 this pattern. `node-group-auto-discovery` and `nodes` options are
@@ -94,7 +97,8 @@ to use.
 
 ## Checking the environment
 
-You can check the brightbox-credentials secret by running the `check-env` job from the examples directory.
+You can check the brightbox-credentials secret by running the `check-env`
+job from the examples directory.
 
 ```
 $ kubectl apply -f examples/check-env.yaml
@@ -134,7 +138,8 @@ the master nodes. This avoids it accidentally killing itself.
 
 ## Viewing the cluster-autoscaler options
 
-Cluster autoscaler has many options that can be adjusted to better fit the needs of your application. To view them run
+Cluster autoscaler has many options that can be adjusted to better fit
+the needs of your application. To view them run
 
 ```
 $ kubectl create job ca-options --image=brightbox/cluster-autoscaler-brightbox:dev -- ./cluster-autoscaler -h
@@ -154,9 +159,10 @@ Extract the repository to a machine running docker and then run the make command
 $ make build
 ```
 
-This builds an autoscaler containing only the Brightbox Cloud provider, tagged as `brightbox/cluster-autoscaler-brightbox:dev`. To build any other version add a TAG variable
+This builds an autoscaler containing only the Brightbox Cloud provider,
+tagged as `brightbox/cluster-autoscaler-brightbox:dev`. To build any
+other version add a TAG variable
 
 ```
 make build TAG=1.1x
 ```
-

cluster-autoscaler/cloudprovider/brightbox/brightbox_cloud_provider.go

@@ -133,13 +133,16 @@ func (b *brightboxCloudProvider) Refresh() error {
 			continue
 		}
 		klog.V(4).Infof("Group %q: Node defaults found in %q. Adding to node group list", configMap.Data["server_group"], configMap.Id)
-		newNodeGroup := makeNodeGroupFromAPIDetails(
+		newNodeGroup, err := makeNodeGroupFromAPIDetails(
 			defaultServerName(configMap.Name),
 			mapData,
 			minSize,
 			maxSize,
 			b.Cloud,
 		)
+		if err != nil {
+			return err
+		}
 		group, err := b.GetServerGroup(newNodeGroup.Id())
 		if err != nil {
 			return err

cluster-autoscaler/cloudprovider/brightbox/brightbox_node_group.go

@@ -354,16 +354,19 @@ func makeNodeGroupFromAPIDetails(
 	minSize int,
 	maxSize int,
 	cloudclient *k8ssdk.Cloud,
-) *brightboxNodeGroup {
+) (*brightboxNodeGroup, error) {
 	klog.V(4).Info("makeNodeGroupFromApiDetails")
+	if mapData["server_group"] == "" {
+		return nil, cloudprovider.ErrIllegalConfiguration
+	}
 	userData := mapData["user_data"]
 	options := &brightbox.ServerOptions{
 		Image:        mapData["image"],
 		Name:         &name,
 		ServerType:   mapData["type"],
 		Zone:         mapData["zone"],
 		UserData:     &userData,
-		ServerGroups: []string{mapData["default_group"], mapData["server_group"]},
+		ServerGroups: mergeServerGroups(mapData),
 	}
 	result := brightboxNodeGroup{
 		id:            mapData["server_group"],
@@ -373,7 +376,27 @@ func makeNodeGroupFromAPIDetails(
 		Cloud:         cloudclient,
 	}
 	klog.V(4).Info(result.Debug())
-	return &result
+	return &result, nil
+}
+
+func mergeServerGroups(data map[string]string) []string {
+	uniqueMap := map[string]bool{}
+	addFromSplit(uniqueMap, data["server_group"])
+	addFromSplit(uniqueMap, data["default_group"])
+	addFromSplit(uniqueMap, data["additional_groups"])
+	result := make([]string, 0, len(uniqueMap))
+	for key := range uniqueMap {
+		result = append(result, key)
+	}
+	return result
+}
+
+func addFromSplit(uniqueMap map[string]bool, source string) {
+	for _, element := range strings.Split(source, ",") {
+		if element != "" {
+			uniqueMap[element] = true
+		}
+	}
 }
 
 func (ng *brightboxNodeGroup) createServers(amount int) error {

cluster-autoscaler/cloudprovider/brightbox/brightbox_node_group_test.go

@@ -19,6 +19,7 @@ package brightbox
 import (
 	"errors"
 	"strconv"
+	"strings"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
@@ -58,8 +59,13 @@ var (
 		"zone":          fakeNodeGroupZoneID,
 		"user_data":     fakeNodeGroupUserData,
 	}
-	ErrFake       = errors.New("fake API Error")
-	fakeInstances = []cloudprovider.Instance{
+	fakeMainGroupList           = []string{fakeNodeGroupID, fakeNodeGroupMainGroupID}
+	fakeAdditionalGroupList     = []string{"grp-abcde", "grp-testy", "grp-winga"}
+	fakeFullGroupList           = append(fakeMainGroupList, fakeAdditionalGroupList...)
+	fakeAdditionalGroups        = strings.Join(fakeAdditionalGroupList, ",")
+	fakeDefaultAdditionalGroups = fakeNodeGroupMainGroupID + "," + fakeAdditionalGroups
+	ErrFake                     = errors.New("fake API Error")
+	fakeInstances               = []cloudprovider.Instance{
 		{
 			Id: "brightbox://srv-rp897",
 			Status: &cloudprovider.InstanceStatus{
@@ -112,17 +118,17 @@ var (
 )
 
 func TestMaxSize(t *testing.T) {
-	assert.Equal(t, makeFakeNodeGroup(nil).MaxSize(), fakeMaxSize)
+	assert.Equal(t, makeFakeNodeGroup(t, nil).MaxSize(), fakeMaxSize)
 }
 
 func TestMinSize(t *testing.T) {
-	assert.Equal(t, makeFakeNodeGroup(nil).MinSize(), fakeMinSize)
+	assert.Equal(t, makeFakeNodeGroup(t, nil).MinSize(), fakeMinSize)
 }
 
 func TestSize(t *testing.T) {
 	mockclient := new(mocks.CloudAccess)
 	testclient := k8ssdk.MakeTestClient(mockclient, nil)
-	nodeGroup := makeFakeNodeGroup(testclient)
+	nodeGroup := makeFakeNodeGroup(t, testclient)
 	fakeServerGroup := &fakeGroups()[0]
 	t.Run("TargetSize", func(t *testing.T) {
 		mockclient.On("ServerGroup", fakeNodeGroupID).
@@ -168,7 +174,7 @@ func TestSize(t *testing.T) {
 func TestIncreaseSize(t *testing.T) {
 	mockclient := new(mocks.CloudAccess)
 	testclient := k8ssdk.MakeTestClient(mockclient, nil)
-	nodeGroup := makeFakeNodeGroup(testclient)
+	nodeGroup := makeFakeNodeGroup(t, testclient)
 	t.Run("Creating details set properly", func(t *testing.T) {
 		assert.Equal(t, fakeNodeGroupID, nodeGroup.id)
 		assert.Equal(t, fakeNodeGroupName, *nodeGroup.serverOptions.Name)
@@ -212,7 +218,7 @@ func TestIncreaseSize(t *testing.T) {
 func TestDeleteNodes(t *testing.T) {
 	mockclient := new(mocks.CloudAccess)
 	testclient := k8ssdk.MakeTestClient(mockclient, nil)
-	nodeGroup := makeFakeNodeGroup(testclient)
+	nodeGroup := makeFakeNodeGroup(t, testclient)
 	fakeServerGroup := &fakeGroups()[0]
 	mockclient.On("ServerGroup", fakeNodeGroupID).
 		Return(fakeServerGroup, nil).
@@ -258,7 +264,7 @@ func TestDeleteNodes(t *testing.T) {
 func TestExist(t *testing.T) {
 	mockclient := new(mocks.CloudAccess)
 	testclient := k8ssdk.MakeTestClient(mockclient, nil)
-	nodeGroup := makeFakeNodeGroup(testclient)
+	nodeGroup := makeFakeNodeGroup(t, testclient)
 	fakeServerGroup := &fakeGroups()[0]
 	t.Run("Find Group", func(t *testing.T) {
 		mockclient.On("ServerGroup", nodeGroup.Id()).
@@ -276,7 +282,7 @@ func TestExist(t *testing.T) {
 func TestNodes(t *testing.T) {
 	mockclient := new(mocks.CloudAccess)
 	testclient := k8ssdk.MakeTestClient(mockclient, nil)
-	nodeGroup := makeFakeNodeGroup(testclient)
+	nodeGroup := makeFakeNodeGroup(t, testclient)
 	fakeServerGroup := &fakeGroups()[0]
 	mockclient.On("ServerGroup", fakeNodeGroupID).
 		Return(fakeServerGroup, nil)
@@ -308,23 +314,117 @@ func TestTemplateNodeInfo(t *testing.T) {
 	testclient := k8ssdk.MakeTestClient(mockclient, nil)
 	mockclient.On("ServerType", fakeNodeGroupServerTypeID).
 		Return(fakeServerTypezx45f(), nil)
-	obj, err := makeFakeNodeGroup(testclient).TemplateNodeInfo()
+	obj, err := makeFakeNodeGroup(t, testclient).TemplateNodeInfo()
 	require.NoError(t, err)
 	assert.Equal(t, fakeResource(), obj.Allocatable)
 }
 
+func TestNodeGroupErrors(t *testing.T) {
+	mockclient := new(mocks.CloudAccess)
+	testclient := k8ssdk.MakeTestClient(mockclient, nil)
+	emptyMapData := map[string]string{}
+	obj, err := makeNodeGroupFromAPIDetails(
+		fakeNodeGroupName,
+		emptyMapData,
+		fakeMinSize,
+		fakeMaxSize,
+		testclient,
+	)
+	assert.Equal(t, cloudprovider.ErrIllegalConfiguration, err)
+	assert.Nil(t, obj)
+}
+
+func TestMultipleGroups(t *testing.T) {
+	mockclient := new(mocks.CloudAccess)
+	testclient := k8ssdk.MakeTestClient(mockclient, nil)
+	t.Run("server group and multi default group", func(t *testing.T) {
+		multigroupData := map[string]string{
+			"min":           strconv.Itoa(fakeMinSize),
+			"max":           strconv.Itoa(fakeMaxSize),
+			"image":         fakeNodeGroupImageID,
+			"type":          fakeNodeGroupServerTypeID,
+			"zone":          fakeNodeGroupZoneID,
+			"user_data":     fakeNodeGroupUserData,
+			"server_group":  fakeNodeGroupID,
+			"default_group": fakeDefaultAdditionalGroups,
+		}
+		obj, err := makeFakeNodeGroupFromMap(
+			testclient,
+			multigroupData,
+		)
+		require.NoError(t, err)
+		assert.ElementsMatch(t, fakeFullGroupList, obj.serverOptions.ServerGroups)
+	})
+	t.Run("server group and multi additional group", func(t *testing.T) {
+		multigroupData := map[string]string{
+			"min":               strconv.Itoa(fakeMinSize),
+			"max":               strconv.Itoa(fakeMaxSize),
+			"image":             fakeNodeGroupImageID,
+			"type":              fakeNodeGroupServerTypeID,
+			"zone":              fakeNodeGroupZoneID,
+			"user_data":         fakeNodeGroupUserData,
+			"server_group":      fakeNodeGroupID,
+			"additional_groups": fakeDefaultAdditionalGroups,
+		}
+		obj, err := makeFakeNodeGroupFromMap(
+			testclient,
+			multigroupData,
+		)
+		require.NoError(t, err)
+		assert.ElementsMatch(t, fakeFullGroupList, obj.serverOptions.ServerGroups)
+	})
+	t.Run("server group, default group and multi additional group", func(t *testing.T) {
+		multigroupData := map[string]string{
+			"min":               strconv.Itoa(fakeMinSize),
+			"max":               strconv.Itoa(fakeMaxSize),
+			"image":             fakeNodeGroupImageID,
+			"type":              fakeNodeGroupServerTypeID,
+			"zone":              fakeNodeGroupZoneID,
+			"user_data":         fakeNodeGroupUserData,
+			"server_group":      fakeNodeGroupID,
+			"default_group":     fakeNodeGroupMainGroupID,
+			"additional_groups": fakeAdditionalGroups,
+		}
+		obj, err := makeFakeNodeGroupFromMap(
+			testclient,
+			multigroupData,
+		)
+		require.NoError(t, err)
+		assert.ElementsMatch(t, fakeFullGroupList, obj.serverOptions.ServerGroups)
+	})
+	t.Run("server group, default group and multi additional group with duplicates", func(t *testing.T) {
+		multigroupData := map[string]string{
+			"min":               strconv.Itoa(fakeMinSize),
+			"max":               strconv.Itoa(fakeMaxSize),
+			"image":             fakeNodeGroupImageID,
+			"type":              fakeNodeGroupServerTypeID,
+			"zone":              fakeNodeGroupZoneID,
+			"user_data":         fakeNodeGroupUserData,
+			"server_group":      fakeNodeGroupID,
+			"default_group":     fakeNodeGroupMainGroupID,
+			"additional_groups": fakeDefaultAdditionalGroups,
+		}
+		obj, err := makeFakeNodeGroupFromMap(
+			testclient,
+			multigroupData,
+		)
+		require.NoError(t, err)
+		assert.ElementsMatch(t, fakeFullGroupList, obj.serverOptions.ServerGroups)
+	})
+}
+
 func TestCreate(t *testing.T) {
-	obj, err := makeFakeNodeGroup(nil).Create()
+	obj, err := makeFakeNodeGroup(t, nil).Create()
 	assert.Equal(t, cloudprovider.ErrNotImplemented, err)
 	assert.Nil(t, obj)
 }
 
 func TestDelete(t *testing.T) {
-	assert.Equal(t, cloudprovider.ErrNotImplemented, makeFakeNodeGroup(nil).Delete())
+	assert.Equal(t, cloudprovider.ErrNotImplemented, makeFakeNodeGroup(t, nil).Delete())
 }
 
 func TestAutoprovisioned(t *testing.T) {
-	assert.False(t, makeFakeNodeGroup(nil).Autoprovisioned())
+	assert.False(t, makeFakeNodeGroup(t, nil).Autoprovisioned())
 }
 
 func fakeResource() *schedulerframework.Resource {
@@ -336,10 +436,19 @@ func fakeResource() *schedulerframework.Resource {
 	}
 }
 
-func makeFakeNodeGroup(brightboxCloudClient *k8ssdk.Cloud) *brightboxNodeGroup {
+func makeFakeNodeGroup(t *testing.T, brightboxCloudClient *k8ssdk.Cloud) *brightboxNodeGroup {
+	obj, err := makeFakeNodeGroupFromMap(
+		brightboxCloudClient,
+		fakeMapData,
+	)
+	require.NoError(t, err)
+	return obj
+}
+
+func makeFakeNodeGroupFromMap(brightboxCloudClient *k8ssdk.Cloud, mapData map[string]string) (*brightboxNodeGroup, error) {
 	return makeNodeGroupFromAPIDetails(
 		fakeNodeGroupName,
-		fakeMapData,
+		mapData,
 		fakeMinSize,
 		fakeMaxSize,
 		brightboxCloudClient,