From a2a2bba801e093b529313bcd68eb85037e0ab40d Mon Sep 17 00:00:00 2001
From: Stefan Bueringer <buringerst@vmware.com>
Date: Wed, 3 Jul 2024 20:57:08 +0200
Subject: [PATCH] Improve godoc of the autoscaler replica field defaulting
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Signed-off-by: Stefan Büringer buringerst@vmware.com
---
 internal/webhooks/machinedeployment.go | 17 ++++++-----------
 internal/webhooks/machineset.go        | 17 ++++++-----------
 2 files changed, 12 insertions(+), 22 deletions(-)

diff --git a/internal/webhooks/machinedeployment.go b/internal/webhooks/machinedeployment.go
index 7f1441bc74a4..80dae949086b 100644
--- a/internal/webhooks/machinedeployment.go
+++ b/internal/webhooks/machinedeployment.go
@@ -317,19 +317,14 @@ func (webhook *MachineDeployment) validate(oldMD, newMD *clusterv1.MachineDeploy
 //
 // We are supporting the following use cases:
 // * A new MD is created and replicas should be managed by the autoscaler
-//   - Either via the default annotation or via the min size and max size annotations the replicas field
-//     is defaulted to a value which is within the (min size, max size) range so the autoscaler can take control.
+//   - If the min size and max size annotations are set, the replicas field is defaulted to the value of the min size
+//     annotation so the autoscaler can take control.
 //
 // * An existing MD which initially wasn't controlled by the autoscaler should be later controlled by the autoscaler
-//   - To adopt an existing MD users can use the default, min size and max size annotations to enable the autoscaler
-//     and to ensure the replicas field is within the (min size, max size) range. Without the annotations handing over
-//     control to the autoscaler by unsetting the replicas field would lead to the field being set to 1. This is very
-//     disruptive for existing Machines and if 1 is outside the (min size, max size) range the autoscaler won't take
-//     control.
-//
-// Notes:
-//   - While the min size and max size annotations of the autoscaler provide the best UX, other autoscalers can use the
-//     DefaultReplicasAnnotation if they have similar use cases.
+//   - To adopt an existing MD users can use the min size and max size annotations to enable the autoscaler
+//     and to ensure the replicas field is within the (min size, max size) range. Without defaulting based on the annotations, handing over
+//     control to the autoscaler by unsetting the replicas field would lead to the field being set to 1. This could be
+//     very disruptive if the previous value of the replica field is greater than 1.
 func calculateMachineDeploymentReplicas(ctx context.Context, oldMD *clusterv1.MachineDeployment, newMD *clusterv1.MachineDeployment, dryRun bool) (int32, error) {
 	// If replicas is already set => Keep the current value.
 	if newMD.Spec.Replicas != nil {
diff --git a/internal/webhooks/machineset.go b/internal/webhooks/machineset.go
index 6fe5d90ccde5..fbdfeb0eee22 100644
--- a/internal/webhooks/machineset.go
+++ b/internal/webhooks/machineset.go
@@ -270,19 +270,14 @@ func validateSkippedMachineSetPreflightChecks(o client.Object) *field.Error {
 //
 // We are supporting the following use cases:
 // * A new MS is created and replicas should be managed by the autoscaler
-//   - Either via the default annotation or via the min size and max size annotations the replicas field
-//     is defaulted to a value which is within the (min size, max size) range so the autoscaler can take control.
+//   - If the min size and max size annotations are set, the replicas field is defaulted to the value of the min size
+//     annotation so the autoscaler can take control.
 //
 // * An existing MS which initially wasn't controlled by the autoscaler should be later controlled by the autoscaler
-//   - To adopt an existing MS users can use the default, min size and max size annotations to enable the autoscaler
-//     and to ensure the replicas field is within the (min size, max size) range. Without the annotations handing over
-//     control to the autoscaler by unsetting the replicas field would lead to the field being set to 1. This is very
-//     disruptive for existing Machines and if 1 is outside the (min size, max size) range the autoscaler won't take
-//     control.
-//
-// Notes:
-//   - While the min size and max size annotations of the autoscaler provide the best UX, other autoscalers can use the
-//     DefaultReplicasAnnotation if they have similar use cases.
+//   - To adopt an existing MS users can use the min size and max size annotations to enable the autoscaler
+//     and to ensure the replicas field is within the (min size, max size) range. Without defaulting based on the annotations, handing over
+//     control to the autoscaler by unsetting the replicas field would lead to the field being set to 1. This could be
+//     very disruptive if the previous value of the replica field is greater than 1.
 func calculateMachineSetReplicas(ctx context.Context, oldMS *clusterv1.MachineSet, newMS *clusterv1.MachineSet, dryRun bool) (int32, error) {
 	// If replicas is already set => Keep the current value.
 	if newMS.Spec.Replicas != nil {