cluster-api-provider-aws: AWSMachinePool does not work in combination with cluster-autoscaler

@AverageMarcus I propose we leave this issue open until the required annotation PRs are merged. I have closed my other PR: kubernetes-sigs/cluster-api#5685

That was the original intent, but the code doesn’t work as intended because replicas will never be nil (also incidentally why I originally opened this issue as a bug). It will be defaulted to a value of 1 if omitted. It also occurred to me that using replicas to indicate “don’t reconcile this value” is a pretty fragile indicator. If cluster-autoscaler has scaled up the pool to say, 100 instances, and someone comes back and misunderstands how it works and sets replicas to 120, AWSMachinePool will suddenly begin reconciling replicas and scale up to 120 instances. Then cluster-autoscaler will try to scale down, and we are back into a thrashing scenario. I think based on this feedback I am going to start on a PR to implement a proper ignoreReplicas spec field.

@benmoss This bug is filed against CAPA, not cluster-autoscaler.

@elmiko I am OK with changing it to a CAPA feature request instead of a CAPA bug if that is everyone’s preference based on the context provided. I only listed it as bug because it did not behave as we initially designed it.

not sure if this will satisfy this issue, but i did see that @mboersma put up this wip pr to the autoscaler, https://github.com/kubernetes/autoscaler/pull/4676

i’m excited to see the progress here =)

/remove-lifecycle stale

In the implementation in our fork of CAPI/CAPA, we have used the externallyManagedReplicaCount field on the MachinePool to reverse the flow of reconciliation. The infrastructure provider updates the spec.replicas and relevant status fields on the MachinePool every reconciliation loop. So the replicas are not nil, in fact if I recall correctly it isn’t possible to have replicas be nil. If it is set to nil, it effectively defaults to 1.

I think you are correct that having the node pool scaler drive replicas through CAPI is more appealing for certain use cases, once the implementation is available and production-ready. It is worth noting that the externallyManagedReplicaCount feature is just an alternate (optional) configuration and permits additional flexibility with regard to node pool scalers, leaving this preference up to the cluster operator. The externallyManagedReplicaCount MachinePool feature does not prevent the features being planned in the MachinePoolMachines proposal from proceeding. The features are actually complementary, because having MachinePoolMachines will still be useful in the externallyManagedReplicaCount case for things like MachineHealthChecks.

Sounds good to me. Just not 100% sure on the naming. replicas makes me think about kubernetes resources rather than ASG instances. How about something like ignoreDesiredSize ?