Introduce annotation to tweak scalingConfig.desiredSize control (#85)

Introduce Annotation for Dynamic Nodegroup `spec.ScalingConfig.desiredSize`
Control

This patch introduces a new annotation that allows flexible management of the
desired size for a nodegroup scaling configuration. The annotation,
`eks.services.k8s.aws/desired-size-managed-by`, can be set on a nodegroup custom
resource and specifies whether the desired size is managed externally by an
autoscaler or by the ACK controller.

The possible values for the annotation include 'external-autoscaler' and
'ack-eks-controller'. If the annotation is not set or has an invalid value, the
controller defaults to managing the desired size.

Adjustments are made to delta comparison logic, custom pre and post-compare
hooks, and the update configuration logic to ensure proper handling of the
desired size based on the annotation.

This enhancement provides flexibility in managing the desired size of a
nodegroup, allowing external entities or the ACK controller to control this
aspect.

Signed-off-by: Amine Hilaly <[email protected]>

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

Author: a-hilaly

README.md

@@ -8,6 +8,38 @@ Kubernetes Github project.
 
 [ack-issues]: https://github.com/aws-controllers-k8s/community/issues
 
+## Getting started
+
+To install the `eks-controller` on your cluster, follow the the
+[installation][ack-install] instructions.
+
+Currently, the `eks-controller` is GA and supports the following resources:
+- `Cluster`
+- `Nodegroup`
+- `FargateProfile`
+- `Addon`
+- `PodIdentityAssociation`
+
+A detailed list of the resources supported specifications can be found in the
+[references][ack-references] section.
+
+## Annotations
+
+For some resources, the `eks-controller` supports annotations to customize the
+behavior of the controller. The following annotations are supported:
+
+- **Nodegroup**
+    - `eks.service.k8s.aws/desired-size-managed-by`: used to control whether the
+      controller should manage the desiredSize of the Nodegroup `spec.ScalingConfig.DesiredSize`.
+      It supports the following values:
+        - `ack-eks-controller`: If set the controller will be responsible for
+          managing the desired size of the nodegroup.
+        - `external-autoscaler`: If set will ignore any changes to the
+          `spec.ScalingConfig.DesiredSize` and will not manage the desired size
+          of the nodegroup.
+
+      If not set, the controller will default to `ack-eks-controller`.
+
 ## Contributing
 
 We welcome community contributions and pull requests.
@@ -24,3 +56,6 @@ You can also learn more about our [Governance](/GOVERNANCE.md) structure.
 ## License
 
 This project is [licensed](/LICENSE) under the Apache-2.0 License.
+
+[ack-references]:https://aws-controllers-k8s.github.io/community/reference
+[ack-install]:https://aws-controllers-k8s.github.io/community/docs/user-docs/install/

apis/v1alpha1/ack-generate-metadata.yaml

@@ -1,13 +1,13 @@
 ack_generate_info:
-  build_date: "2023-12-14T21:31:34Z"
-  build_hash: 1f16813c807af6889060b4ce7ded2a69dc027d8c
+  build_date: "2024-01-02T04:11:25Z"
+  build_hash: 00e081fb541587f33970ad80c99f2ac02e9c2525
   go_version: go1.21.5
-  version: v0.28.0
-api_directory_checksum: c243e9ab23c6b9038f6f6aebba5f0b34efafdf6a
+  version: v0.28.0-8-g00e081f
+api_directory_checksum: e3ab17e79de95c932895a4372c0b01e976aec4cb
 api_version: v1alpha1
 aws_sdk_go_version: v1.49.0
 generator_config_info:
-  file_checksum: 9b2af7d38552d1e11a320576c802f375dcba3a06
+  file_checksum: 95f5a8da41f547e5d9c7f207d5405b64ed866196
   original_file_name: generator.yaml
 last_modification:
   reason: API generation

apis/v1alpha1/annotation.go

@@ -0,0 +1,48 @@
+// Copyright Amazon.com Inc. or its affiliates. All Rights Reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License"). You may
+// not use this file except in compliance with the License. A copy of the
+// License is located at
+//
+//     http://aws.amazon.com/apache2.0/
+//
+// or in the "license" file accompanying this file. This file is distributed
+// on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either
+// express or implied. See the License for the specific language governing
+// permissions and limitations under the License.
+
+package v1alpha1
+
+import "fmt"
+
+var (
+	// DesiredSizeManagedByAnnotation is the annotation key used to set the management style for the
+	// desired size of a nodegroup scaling configuration. This annotation can only be set on a
+	// nodegroup custom resource.
+	//
+	// The value of this annotation must be one of the following:
+	//
+	// - 'external-autoscaler': The desired size is managed by an external entity. Causing the
+	//                          controller to completly ignore the `scalingConfig.desiredSize` field
+	// 						    and not reconcile the desired size of a nodegroup.
+	//
+	// - 'ack-eks-controller':  The desired size is managed by the ACK controller. Causing the
+	//                          controller to reconcile the desired size of the nodegroup with the
+	//                          value of the `spec.scalingConfig.desiredSize` field.
+	//
+	// By default the desired size is managed by the controller. If the annotation is not set, or
+	// the value is not one of the above, the controller will default to managing the desired size
+	// as if the annotation was set to "controller".
+	DesiredSizeManagedByAnnotation = fmt.Sprintf("%s/desired-size-managed-by", GroupVersion.Group)
+)
+
+const (
+	// DesiredSizeManagedByExternalAutoscaler is the value of the DesiredSizeManagedByAnnotation
+	// annotation that indicates that the desired size of a nodegroup is managed by an external
+	// autoscaler.
+	DesiredSizeManagedByExternalAutoscaler = "external-autoscaler"
+	// DesiredSizeManagedByACKController is the value of the DesiredSizeManagedByAnnotation
+	// annotation that indicates that the desired size of a nodegroup is managed by the ACK
+	// controller.
+	DesiredSizeManagedByACKController = "ack-eks-controller"
+)

apis/v1alpha1/generator.yaml

@@ -202,6 +202,8 @@ resources:
     hooks:
       delta_pre_compare:
         code: customPreCompare(delta, a, b)
+      delta_post_compare:
+        code: customPostCompare(delta, a, b)
       sdk_create_post_set_output:
         template_path: hooks/nodegroup/sdk_create_post_set_output.go.tpl
       sdk_read_one_post_set_output:

generator.yaml

@@ -202,6 +202,8 @@ resources:
     hooks:
       delta_pre_compare:
         code: customPreCompare(delta, a, b)
+      delta_post_compare:
+        code: customPostCompare(delta, a, b)
       sdk_create_post_set_output:
         template_path: hooks/nodegroup/sdk_create_post_set_output.go.tpl
       sdk_read_one_post_set_output:

helm/crds/services.k8s.aws_adoptedresources.yaml

@@ -161,10 +161,10 @@ spec:
                               description: 'Kind of the referent. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds'
                               type: string
                             name:
-                              description: 'Name of the referent. More info: http://kubernetes.io/docs/user-guide/identifiers#names'
+                              description: 'Name of the referent. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names#names'
                               type: string
                             uid:
-                              description: 'UID of the referent. More info: http://kubernetes.io/docs/user-guide/identifiers#uids'
+                              description: 'UID of the referent. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names#uids'
                               type: string
                           required:
                           - apiVersion

pkg/resource/nodegroup/delta.go

@@ -102,6 +102,48 @@ func customPreCompare(
 	}
 }
 
+func getDesiredSizeManagedByAnnotation(nodegroup *svcapitypes.Nodegroup) (string, bool) {
+	if len(nodegroup.Annotations) == 0 {
+		return "", false
+	}
+	managedBy, ok := nodegroup.Annotations[svcapitypes.DesiredSizeManagedByAnnotation]
+	return managedBy, ok
+}
+
+func isManagedByExternalAutoscaler(nodegroup *svcapitypes.Nodegroup) bool {
+	managedBy, ok := getDesiredSizeManagedByAnnotation(nodegroup)
+	if !ok {
+		return false
+	}
+	return managedBy == svcapitypes.DesiredSizeManagedByExternalAutoscaler
+}
+
+func customPostCompare(
+	delta *ackcompare.Delta,
+	a *resource,
+	b *resource,
+) {
+	// We only want to compare the desiredSize field if and only if the
+	// desiredSize is managed by the controller, meaning that in the case
+	// where the desiredSize is managed by an external entity, we do not
+	// want to compare the desiredSize field.
+	// When managed by an external entity, an annotation is set on the
+	// nodegroup resource to indicate that the desiredSize is managed
+	// externally.
+	if isManagedByExternalAutoscaler(a.ko) && delta.DifferentAt("Spec.ScalingConfig.DesiredSize") {
+		// We need to unset the desiredSize field in the delta so that the
+		// controller does not attempt to reconcile the desiredSize of the
+		// nodegroup.
+		newDiffs := make([]*ackcompare.Difference, 0)
+		for _, d := range delta.Differences {
+			if !d.Path.Contains("Spec.ScalingConfig.DesiredSize") {
+				newDiffs = append(newDiffs, d)
+			}
+		}
+		delta.Differences = newDiffs
+	}
+}
+
 // requeueWaitUntilCanModify returns a `ackrequeue.RequeueNeededAfter` struct
 // explaining the nodegroup cannot be modified until it reaches an active
 // status.
@@ -218,7 +260,7 @@ func (rm *resourceManager) customUpdate(
 
 	if delta.DifferentAt("Spec.Labels") || delta.DifferentAt("Spec.Taints") ||
 		delta.DifferentAt("Spec.ScalingConfig") || delta.DifferentAt("Spec.UpdateConfig") {
-		if err := rm.updateConfig(ctx, desired, latest); err != nil {
+		if err := rm.updateConfig(ctx, delta, desired, latest); err != nil {
 			return nil, err
 		}
 		return returnNodegroupUpdating(updatedRes)
@@ -342,8 +384,21 @@ func (rm *resourceManager) updateVersion(
 	return nil
 }
 
+func (rm *resourceManager) newUpdateScalingConfigPayload(
+	desired, latest *resource,
+) *svcsdk.NodegroupScalingConfig {
+	sc := rm.newNodegroupScalingConfig(desired)
+	// We need to default the desiredSize to the current observed
+	// value in the case where the desiredSize is managed externally.
+	if isManagedByExternalAutoscaler(desired.ko) && latest.ko.Spec.ScalingConfig != nil {
+		sc.DesiredSize = latest.ko.Spec.ScalingConfig.DesiredSize
+	}
+	return sc
+}
+
 func (rm *resourceManager) updateConfig(
 	ctx context.Context,
+	delta *ackcompare.Delta,
 	desired *resource,
 	latest *resource,
 ) (err error) {
@@ -359,7 +414,7 @@ func (rm *resourceManager) updateConfig(
 	}
 
 	if desired.ko.Spec.ScalingConfig != nil {
-		input.SetScalingConfig(rm.newNodegroupScalingConfig(desired))
+		input.SetScalingConfig(rm.newUpdateScalingConfigPayload(desired, latest))
 	}
 
 	if desired.ko.Spec.UpdateConfig != nil {