diff --git a/cluster-autoscaler/cloudprovider/clusterapi/README.md b/cluster-autoscaler/cloudprovider/clusterapi/README.md index ef0277f00228..50d686aa681b 100644 --- a/cluster-autoscaler/cloudprovider/clusterapi/README.md +++ b/cluster-autoscaler/cloudprovider/clusterapi/README.md @@ -101,12 +101,12 @@ resources depending on the type of cluster-api mechanism that you are using. There are two annotations that control how a cluster resource should be scaled: -* `cluster.k8s.io/cluster-api-autoscaler-node-group-min-size` - This specifies +* `cluster.x-k8s.io/cluster-api-autoscaler-node-group-min-size` - This specifies the minimum number of nodes for the associated resource group. The autoscaler will not scale the group below this number. Please note that currently the cluster-api provider will not scale down to zero nodes. -* `cluster.k8s.io/cluster-api-autoscaler-node-group-max-size` - This specifies +* `cluster.x-k8s.io/cluster-api-autoscaler-node-group-max-size` - This specifies the maximum number of nodes for the associated resource group. The autoscaler will not scale the group above this number. diff --git a/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_controller.go b/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_controller.go index 459217a58a0e..fb56d7f80d56 100644 --- a/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_controller.go +++ b/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_controller.go @@ -258,7 +258,12 @@ func (c *machineController) findMachineByProviderID(providerID normalizedProvide if node == nil { return nil, nil } - return c.findMachine(node.Annotations[machineAnnotationKey]) + + machineID, ok := node.Annotations[machineAnnotationKey] + if !ok { + machineID = node.Annotations[deprecatedMachineAnnotationKey] + } + return c.findMachine(machineID) } func isFailedMachineProviderID(providerID normalizedProviderID) bool { diff --git a/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_controller_test.go b/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_controller_test.go index bad77f565f0f..d6c414c63b22 100644 --- a/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_controller_test.go +++ b/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_controller_test.go @@ -486,24 +486,32 @@ func deleteTestConfigs(t *testing.T, controller *machineController, testConfigs func TestControllerFindMachine(t *testing.T) { type testCase struct { - description string - name string - namespace string - lookupSucceeds bool + description string + name string + namespace string + useDeprecatedAnnotation bool + lookupSucceeds bool } var testCases = []testCase{{ - description: "lookup fails", - lookupSucceeds: false, - name: "machine-does-not-exist", - namespace: "namespace-does-not-exist", + description: "lookup fails", + lookupSucceeds: false, + useDeprecatedAnnotation: false, + name: "machine-does-not-exist", + namespace: "namespace-does-not-exist", }, { - description: "lookup fails in valid namespace", - lookupSucceeds: false, - name: "machine-does-not-exist-in-existing-namespace", + description: "lookup fails in valid namespace", + lookupSucceeds: false, + useDeprecatedAnnotation: false, + name: "machine-does-not-exist-in-existing-namespace", }, { - description: "lookup succeeds", - lookupSucceeds: true, + description: "lookup succeeds", + lookupSucceeds: true, + useDeprecatedAnnotation: false, + }, { + description: "lookup succeeds with deprecated annotation", + lookupSucceeds: true, + useDeprecatedAnnotation: true, }} test := func(t *testing.T, tc testCase, testConfig *testConfig) { @@ -541,6 +549,19 @@ func TestControllerFindMachine(t *testing.T) { if tc.namespace == "" { tc.namespace = testConfig.machines[0].GetNamespace() } + if tc.useDeprecatedAnnotation { + for i := range testConfig.machines { + n := testConfig.nodes[i] + annotations := n.GetAnnotations() + val, ok := annotations[machineAnnotationKey] + if !ok { + t.Fatal("node did not contain machineAnnotationKey") + } + delete(annotations, machineAnnotationKey) + annotations[deprecatedMachineAnnotationKey] = val + n.SetAnnotations(annotations) + } + } test(t, tc, testConfig) }) } diff --git a/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_nodegroup.go b/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_nodegroup.go index c0ba8e77d605..8d04f62e33cc 100644 --- a/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_nodegroup.go +++ b/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_nodegroup.go @@ -27,9 +27,13 @@ import ( ) const ( - machineDeleteAnnotationKey = "cluster.k8s.io/delete-machine" - machineAnnotationKey = "cluster.k8s.io/machine" - debugFormat = "%s (min: %d, max: %d, replicas: %d)" + // deprecatedMachineDeleteAnnotationKey should not be removed until minimum cluster-api support is v1alpha3 + 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)" ) type nodegroup struct { diff --git a/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_nodegroup_test.go b/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_nodegroup_test.go index 11bf4d48096d..9c7e22453af1 100644 --- a/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_nodegroup_test.go +++ b/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_nodegroup_test.go @@ -649,6 +649,9 @@ func TestNodeGroupDeleteNodes(t *testing.T) { if _, found := machine.GetAnnotations()[machineDeleteAnnotationKey]; !found { t.Errorf("expected annotation %q on machine %s", machineDeleteAnnotationKey, machine.GetName()) } + if _, found := machine.GetAnnotations()[deprecatedMachineDeleteAnnotationKey]; !found { + t.Errorf("expected annotation %q on machine %s", deprecatedMachineDeleteAnnotationKey, machine.GetName()) + } } gvr, err := ng.scalableResource.GroupVersionResource() diff --git a/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_unstructured.go b/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_unstructured.go index d5bf3e387339..ac6991bebbe9 100644 --- a/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_unstructured.go +++ b/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_unstructured.go @@ -129,15 +129,12 @@ func (r unstructuredScalableResource) UnmarkMachineForDeletion(machine *unstruct } annotations := u.GetAnnotations() - if _, ok := annotations[machineDeleteAnnotationKey]; ok { - delete(annotations, machineDeleteAnnotationKey) - u.SetAnnotations(annotations) - _, updateErr := r.controller.managementClient.Resource(r.controller.machineResource).Namespace(u.GetNamespace()).Update(context.TODO(), u, metav1.UpdateOptions{}) - - return updateErr - } + delete(annotations, machineDeleteAnnotationKey) + delete(annotations, deprecatedMachineDeleteAnnotationKey) + u.SetAnnotations(annotations) + _, updateErr := r.controller.managementClient.Resource(r.controller.machineResource).Namespace(u.GetNamespace()).Update(context.TODO(), u, metav1.UpdateOptions{}) - return nil + return updateErr } func (r unstructuredScalableResource) MarkMachineForDeletion(machine *unstructured.Unstructured) error { @@ -154,6 +151,7 @@ func (r unstructuredScalableResource) MarkMachineForDeletion(machine *unstructur } annotations[machineDeleteAnnotationKey] = time.Now().String() + annotations[deprecatedMachineDeleteAnnotationKey] = time.Now().String() u.SetAnnotations(annotations) _, updateErr := r.controller.managementClient.Resource(r.controller.machineResource).Namespace(u.GetNamespace()).Update(context.TODO(), u, metav1.UpdateOptions{}) diff --git a/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_utils.go b/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_utils.go index 104bffecbe69..f564efc0c8f8 100644 --- a/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_utils.go +++ b/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_utils.go @@ -26,10 +26,12 @@ import ( ) const ( - nodeGroupMinSizeAnnotationKey = "cluster.k8s.io/cluster-api-autoscaler-node-group-min-size" - nodeGroupMaxSizeAnnotationKey = "cluster.k8s.io/cluster-api-autoscaler-node-group-max-size" - clusterNameLabel = "cluster.x-k8s.io/cluster-name" - deprecatedClusterNameLabel = "cluster.k8s.io/cluster-name" + 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 ( @@ -60,6 +62,9 @@ type normalizedProviderID string // value is not of type int. func minSize(annotations map[string]string) (int, error) { val, found := annotations[nodeGroupMinSizeAnnotationKey] + if !found { + val, found = annotations[deprecatedNodeGroupMinSizeAnnotationKey] + } if !found { return 0, errMissingMinAnnotation } @@ -76,6 +81,9 @@ func minSize(annotations map[string]string) (int, error) { // value is not of type int. func maxSize(annotations map[string]string) (int, error) { val, found := annotations[nodeGroupMaxSizeAnnotationKey] + if !found { + val, found = annotations[deprecatedNodeGroupMaxSizeAnnotationKey] + } if !found { return 0, errMissingMaxAnnotation } diff --git a/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_utils_test.go b/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_utils_test.go index 9a9cada7734c..65ee11fe9ccb 100644 --- a/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_utils_test.go +++ b/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_utils_test.go @@ -112,6 +112,24 @@ func TestUtilParseScalingBounds(t *testing.T) { }, min: 0, max: 1, + }, { + description: "deprecated min/max annotations still work, result is min 0, max 1", + annotations: map[string]string{ + deprecatedNodeGroupMinSizeAnnotationKey: "0", + deprecatedNodeGroupMaxSizeAnnotationKey: "1", + }, + min: 0, + max: 1, + }, { + description: "deprecated min/max annotations do not take precedence over non-deprecated annotations, result is min 1, max 2", + annotations: map[string]string{ + deprecatedNodeGroupMinSizeAnnotationKey: "0", + deprecatedNodeGroupMaxSizeAnnotationKey: "1", + nodeGroupMinSizeAnnotationKey: "1", + nodeGroupMaxSizeAnnotationKey: "2", + }, + min: 1, + max: 2, }} { t.Run(tc.description, func(t *testing.T) { machineSet := unstructured.Unstructured{