MachinePool: always patch owned conditions, fix GetTypedPhase, doc fixes

Signed-off-by: Stefan Büringer buringerst@vmware.com
sbueringer · Jun 20, 2023 · 436a197 · 436a197
1 parent 0fea6f3
commit 436a197
Show file tree

Hide file tree

Showing 4 changed files with 18 additions and 22 deletions.
diff --git a/docs/book/src/tasks/experimental-features/machine-pools.md b/docs/book/src/tasks/experimental-features/machine-pools.md
@@ -28,12 +28,12 @@ For developer docs on the MachinePool controller, see [here](./../../developer/a
 
 ## MachinePools vs MachineDeployments
 
-Although MachinePools provide a similar feature to MachineDeployments, MachinePools do so by leveraging an InfraMachinePool which corresponds 1:1 with a resource like VMSS on Azure or Autoscaling Groups on AWS which we treat as a black box. When a MachinePool is scaled up, the InfraMachinePool scales itself up and populates its provider ID list based on the response from the infrastructure provider. On the other hand, a when a MachineDeployment is scaled up, new Machines are created which then create an individual InfraMachine, which corresponds to a VM in any infrastructure provider.
+Although MachinePools provide a similar feature to MachineDeployments, MachinePools do so by leveraging an InfraMachinePool which corresponds 1:1 with a resource like VMSS on Azure or Autoscaling Groups on AWS which we treat as a black box. When a MachinePool is scaled up, the InfraMachinePool scales itself up and populates its provider ID list based on the response from the infrastructure provider. On the other hand, when a MachineDeployment is scaled up, new Machines are created which then create an individual InfraMachine, which corresponds to a VM in any infrastructure provider.
 
 | MachinePools                                                                                                                                                        | MachineDeployments                                                                                                                     |
 | ------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -------------------------------------------------------------------------------------------------------------------------------------- |
 | Creates new instances through a single infrastructure resource like VMSS in Azure or Autoscaling Groups in AWS.                                                     | Creates new instances by creating new Machines, which create individual VM instances on the infra provider.                            |
 | Set of instances is orchestrated by the infrastructure provider.                                                                                                    | Set of instances is orchestrated by Cluster API using a MachineSet.                                                                    |
 | Each MachinePool corresponds 1:1 with an associated InfraMachinePool.                                                                                               | Each MachineDeployment includes a MachineSet, and for each replica, it creates a Machine and InfraMachine.                             |
 | Each MachinePool requires only a single BootstrapConfig.                                                                                                            | Each MachineDeployment uses an InfraMachineTemplate and a BootstrapConfigTemplate, and each Machine requires a unique BootstrapConfig. |
-| Maintains a list of instances in the `providerIDList` field in the MachinePool spec. This list is populated based on the response from the infrastructure provider. | Maintains a list of instances through the Machine resources owned by the MachineSet.                                                   |
+| Maintains a list of instances in the `providerIDList` field in the MachinePool spec. This list is populated based on the response from the infrastructure provider. | Maintains a list of instances through the Machine resources owned by the MachineSet.                                                   |
diff --git a/docs/proposals/20190919-machinepool-api.md b/docs/proposals/20190919-machinepool-api.md
@@ -176,9 +176,6 @@ type MachinePoolSpec struct
     - Type: `MachineTemplateSpec`
     - Description: Machine Template that describes the configuration of each machine instance in a
       machine pool.
-  - **Strategy [optional]**
-    - Type: `*MachineDeploymentStrategy`
-    - Description: Strategy to employ in replacing existing machine instances in a machine pool.
   - **MinReadySeconds [optional]**
     - Type: `*int32`
     - Description: Minimum number of seconds for which a newly created machine should be ready.
@@ -246,13 +243,13 @@ MachinePoolPhasePending = MachinePoolPhase("pending")
 
 ###### Expectations
 
-- When MachinePool.Spec.Template.Spec.Bootstrap.Data is:
-  - <nil>, expect the field to be set by an external controller.
+- When MachinePool.Spec.Template.Spec.Bootstrap.DataSecretName is:
+  - \<nil\>, expect the field to be set by an external controller.
   - “” (empty string), expect the bootstrap step to be ignored.
   - “...” (populated by user or from the bootstrap provider), expect the contents to be used by a
     bootstrap or infra provider.
 - When MachinePool.Spec.Template.Spec.InfrastructureRef is:
-  - <nil> or not found, expect InfrastructureRef will be set/found during subsequent requeue.
+  - \<nil\> or not found, expect InfrastructureRef will be set/found during subsequent requeue.
   - “...” (populated by user) and found, expect the infrastructure provider is waiting for bootstrap
     data to be ready.
   - Found, expect InfrastructureRef to reference an object such as GoogleManagedInstanceGroup,
@@ -269,7 +266,7 @@ MachinePoolPhaseProvisioning = MachinePoolPhase("provisioning")
 ###### Transition Conditions
 
 - MachinePool.Spec.Template.Spec.Bootstrap.ConfigRef -> Status.Ready is true
-- MachinePool.Spec.Template.Spec.Bootstrap.Data is not <nil>
+- MachinePool.Spec.Template.Spec.Bootstrap.DataSecretName is not \<nil\>
 
 ###### Expectations
 
@@ -302,7 +299,7 @@ MachinePoolPhaseRunning = MachinePoolPhase("running")
 
 ###### Transition Conditions
 
-- Number of Kubernetes Nodes in a Ready state matching MachinePool.Spec.Selector equal MachinePool.Status.Replicas.
+- Number of Kubernetes Nodes matching MachinePool.Spec.ProviderIDList in a Ready state equal to MachinePool.Spec.Replicas.
 
 ###### Expectations
 
@@ -319,7 +316,7 @@ MachinePoolPhaseDeleting = MachinePoolPhase("deleting")
 
 ###### Transition Conditions
 
-- MachinePool.ObjectMeta.DeletionTimestamp is not <nil>
+- MachinePool.ObjectMeta.DeletionTimestamp is not \<nil\>
 
 ###### Expectations
 

diff --git a/exp/api/v1beta1/machinepool_types.go b/exp/api/v1beta1/machinepool_types.go
@@ -197,6 +197,7 @@ func (m *MachinePoolStatus) GetTypedPhase() MachinePoolPhase {
 		MachinePoolPhaseRunning,
 		MachinePoolPhaseScalingUp,
 		MachinePoolPhaseScalingDown,
+		MachinePoolPhaseScaling,
 		MachinePoolPhaseDeleting,
 		MachinePoolPhaseFailed:
 		return phase

diff --git a/exp/internal/controllers/machinepool_controller.go b/exp/internal/controllers/machinepool_controller.go
@@ -157,18 +157,16 @@ func (r *MachinePoolReconciler) Reconcile(ctx context.Context, req ctrl.Request)
 
 		// Always attempt to patch the object and status after each reconciliation.
 		// Patch ObservedGeneration only if the reconciliation completed successfully
-		patchOpts := []patch.Option{}
+		patchOpts := []patch.Option{
+			patch.WithOwnedConditions{Conditions: []clusterv1.ConditionType{
+				clusterv1.ReadyCondition,
+				clusterv1.BootstrapReadyCondition,
+				clusterv1.InfrastructureReadyCondition,
+				expv1.ReplicasReadyCondition,
+			}},
+		}
 		if reterr == nil {
-			patchOpts = append(
-				patchOpts,
-				patch.WithStatusObservedGeneration{},
-				patch.WithOwnedConditions{Conditions: []clusterv1.ConditionType{
-					clusterv1.ReadyCondition,
-					clusterv1.BootstrapReadyCondition,
-					clusterv1.InfrastructureReadyCondition,
-					expv1.ReplicasReadyCondition,
-				}},
-			)
+			patchOpts = append(patchOpts, patch.WithStatusObservedGeneration{})
 		}
 		if err := patchHelper.Patch(ctx, mp, patchOpts...); err != nil {
 			reterr = kerrors.NewAggregate([]error{reterr, err})