From 755cb1b7b6cc9ce5939de2617edff165f91bb7bb Mon Sep 17 00:00:00 2001 From: Michael McCune Date: Mon, 8 Nov 2021 14:41:01 -0500 Subject: [PATCH] expand CAPI_GROUP usage to cover other capi group variables This change updates the logic for the clusterapi autoscaler provider so that the `CAPI_GROUP` environment variable will also affect the annotations keys for minimum and maximum node group size, the machine annotation, machine deletion, and the cluster name label. It also addes unit tests and an update to the readme. --- .../cloudprovider/clusterapi/README.md | 9 ++ .../clusterapi/clusterapi_nodegroup.go | 2 - .../clusterapi/clusterapi_utils.go | 66 +++++++++++++- .../clusterapi/clusterapi_utils_test.go | 88 +++++++++++++++++++ 4 files changed, 160 insertions(+), 5 deletions(-) diff --git a/cluster-autoscaler/cloudprovider/clusterapi/README.md b/cluster-autoscaler/cloudprovider/clusterapi/README.md index 3e434648b0e1..9684bc09bc2e 100644 --- a/cluster-autoscaler/cloudprovider/clusterapi/README.md +++ b/cluster-autoscaler/cloudprovider/clusterapi/README.md @@ -121,6 +121,15 @@ some situations, such as testing or prototyping, you may wish to change this group variable. For these situations you may use the environment variable `CAPI_GROUP` to change the group that the provider will use. +Please note that setting the `CAPI_GROUP` environment variable will also cause the +annotations for minimum and maximum size to change. +This behavior will also affect the machine annotation on nodes, the machine deletion annotation, +and the cluster name label. For example, if `CAPI_GROUP=test.k8s.io` +then the minimum size annotation key will be `test.k8s.io/cluster-api-autoscaler-node-group-min-size`, +the machine annotation on nodes will be `test.8s.io/machine`, the machine deletion +annotation will be `test.k8s.io/delete-machine`, and the cluster name label will be +`test.k8s.io/cluster-name`. + ## Sample manifest A sample manifest that will create a deployment running the autoscaler is diff --git a/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_nodegroup.go b/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_nodegroup.go index db422d04d4b2..c8f10b5ad13d 100644 --- a/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_nodegroup.go +++ b/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_nodegroup.go @@ -32,8 +32,6 @@ const ( deprecatedMachineDeleteAnnotationKey = "cluster.k8s.io/delete-machine" // TODO: determine what currently relies on deprecatedMachineAnnotationKey to determine when it can be removed deprecatedMachineAnnotationKey = "cluster.k8s.io/machine" - machineDeleteAnnotationKey = "cluster.x-k8s.io/delete-machine" - machineAnnotationKey = "cluster.x-k8s.io/machine" debugFormat = "%s (min: %d, max: %d, replicas: %d)" ) diff --git a/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_utils.go b/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_utils.go index f564efc0c8f8..231ffc98bebe 100644 --- a/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_utils.go +++ b/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_utils.go @@ -17,6 +17,7 @@ limitations under the License. package clusterapi import ( + "fmt" "strconv" "strings" @@ -28,13 +29,15 @@ import ( const ( deprecatedNodeGroupMinSizeAnnotationKey = "cluster.k8s.io/cluster-api-autoscaler-node-group-min-size" deprecatedNodeGroupMaxSizeAnnotationKey = "cluster.k8s.io/cluster-api-autoscaler-node-group-max-size" - nodeGroupMinSizeAnnotationKey = "cluster.x-k8s.io/cluster-api-autoscaler-node-group-min-size" - nodeGroupMaxSizeAnnotationKey = "cluster.x-k8s.io/cluster-api-autoscaler-node-group-max-size" - clusterNameLabel = "cluster.x-k8s.io/cluster-name" deprecatedClusterNameLabel = "cluster.k8s.io/cluster-name" ) var ( + // clusterNameLabel is the label applied to objects(Machine, MachineSet, MachineDeployment) + // to identify which cluster they are owned by. Because the label can be + // affected by the CAPI_GROUP environment variable, it is initialized here. + clusterNameLabel = getClusterNameLabel() + // errMissingMinAnnotation is the error returned when a // machine set does not have an annotation keyed by // nodeGroupMinSizeAnnotationKey. @@ -52,6 +55,23 @@ var ( // errInvalidMaxAnnotationValue is the error returned when a // machine set has a non-integral max annotation value. errInvalidMaxAnnotation = errors.New("invalid max annotation") + + // machineDeleteAnnotationKey is the annotation used by cluster-api to indicate + // that a machine should be deleted. Because this key can be affected by the + // CAPI_GROUP env variable, it is initialized here. + machineDeleteAnnotationKey = getMachineDeleteAnnotationKey() + + // machineAnnotationKey is the annotation used by the cluster-api on Node objects + // to specify the name of the related Machine object. Because this can be affected + // by the CAPI_GROUP env variable, it is initialized here. + machineAnnotationKey = getMachineAnnotationKey() + + // nodeGroupMinSizeAnnotationKey and nodeGroupMaxSizeAnnotationKey are the keys + // used in MachineSet and MachineDeployment annotations to specify the limits + // for the node group. Because the keys can be affected by the CAPI_GROUP env + // variable, they are initialized here. + nodeGroupMinSizeAnnotationKey = getNodeGroupMinSizeAnnotationKey() + nodeGroupMaxSizeAnnotationKey = getNodeGroupMaxSizeAnnotationKey() ) type normalizedProviderID string @@ -180,3 +200,43 @@ func clusterNameFromResource(r *unstructured.Unstructured) string { return "" } + +// getNodeGroupMinSizeAnnotationKey returns the key that is used for the +// node group minimum size annotation. This function is needed because the user can +// change the default group name by using the CAPI_GROUP environment variable. +func getNodeGroupMinSizeAnnotationKey() string { + key := fmt.Sprintf("%s/cluster-api-autoscaler-node-group-min-size", getCAPIGroup()) + return key +} + +// getNodeGroupMaxSizeAnnotationKey returns the key that is used for the +// node group maximum size annotation. This function is needed because the user can +// change the default group name by using the CAPI_GROUP environment variable. +func getNodeGroupMaxSizeAnnotationKey() string { + key := fmt.Sprintf("%s/cluster-api-autoscaler-node-group-max-size", getCAPIGroup()) + return key +} + +// getMachineDeleteAnnotationKey returns the key that is used by cluster-api for marking +// machines to be deleted. This function is needed because the user can change the default +// group name by using the CAPI_GROUP environment variable. +func getMachineDeleteAnnotationKey() string { + key := fmt.Sprintf("%s/delete-machine", getCAPIGroup()) + return key +} + +// getMachineAnnotationKey returns the key that is used by cluster-api for annotating +// nodes with their related machine objects. This function is needed because the user can change +// the default group name by using the CAPI_GROUP environment variable. +func getMachineAnnotationKey() string { + key := fmt.Sprintf("%s/machine", getCAPIGroup()) + return key +} + +// getClusterNameLabel returns the key that is used by cluster-api for labeling +// which cluster an object belongs to. This function is needed because the user can change +// the default group name by using the CAPI_GROUP environment variable. +func getClusterNameLabel() string { + key := fmt.Sprintf("%s/cluster-name", getCAPIGroup()) + return key +} diff --git a/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_utils_test.go b/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_utils_test.go index 65ee11fe9ccb..ea8244703bfc 100644 --- a/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_utils_test.go +++ b/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_utils_test.go @@ -17,6 +17,8 @@ limitations under the License. package clusterapi import ( + "fmt" + "os" "reflect" "strings" "testing" @@ -689,4 +691,90 @@ func Test_clusterNameFromResource(t *testing.T) { } }) } + +} + +func Test_getKeyHelpers(t *testing.T) { + for _, tc := range []struct { + name string + expected string + testfunc func() string + }{ + { + name: "default group, min size annotation key", + expected: fmt.Sprintf("%s/cluster-api-autoscaler-node-group-min-size", defaultCAPIGroup), + testfunc: getNodeGroupMinSizeAnnotationKey, + }, + { + name: "default group, max size annotation key", + expected: fmt.Sprintf("%s/cluster-api-autoscaler-node-group-max-size", defaultCAPIGroup), + testfunc: getNodeGroupMaxSizeAnnotationKey, + }, + { + name: "default group, machine delete annotation key", + expected: fmt.Sprintf("%s/delete-machine", defaultCAPIGroup), + testfunc: getMachineDeleteAnnotationKey, + }, + { + name: "default group, machine annotation key", + expected: fmt.Sprintf("%s/machine", defaultCAPIGroup), + testfunc: getMachineAnnotationKey, + }, + { + name: "default group, cluster name label", + expected: fmt.Sprintf("%s/cluster-name", defaultCAPIGroup), + testfunc: getClusterNameLabel, + }, + } { + t.Run(tc.name, func(t *testing.T) { + observed := tc.testfunc() + if observed != tc.expected { + t.Errorf("%s, mismatch, expected=%s, observed=%s", tc.name, observed, tc.expected) + } + }) + } + + testgroup := "test.k8s.io" + if err := os.Setenv(CAPIGroupEnvVar, testgroup); err != nil { + t.Fatalf("unexpected error: %v", err) + } + + for _, tc := range []struct { + name string + expected string + testfunc func() string + }{ + { + name: "test group, min size annotation key", + expected: fmt.Sprintf("%s/cluster-api-autoscaler-node-group-min-size", testgroup), + testfunc: getNodeGroupMinSizeAnnotationKey, + }, + { + name: "test group, max size annotation key", + expected: fmt.Sprintf("%s/cluster-api-autoscaler-node-group-max-size", testgroup), + testfunc: getNodeGroupMaxSizeAnnotationKey, + }, + { + name: "test group, machine delete annotation key", + expected: fmt.Sprintf("%s/delete-machine", testgroup), + testfunc: getMachineDeleteAnnotationKey, + }, + { + name: "test group, machine annotation key", + expected: fmt.Sprintf("%s/machine", testgroup), + testfunc: getMachineAnnotationKey, + }, + { + name: "test group, cluster name label", + expected: fmt.Sprintf("%s/cluster-name", testgroup), + testfunc: getClusterNameLabel, + }, + } { + t.Run(tc.name, func(t *testing.T) { + observed := tc.testfunc() + if observed != tc.expected { + t.Errorf("%s, mismatch, expected=%s, observed=%s", tc.name, observed, tc.expected) + } + }) + } }