Merge pull request kubernetes-sigs#5056 from Nordix/ms-avail-condition

✨ Add Conditions to MachineSet for Machine Create and Ready

Author: k8s-ci-robot

api/v1alpha3/conversion.go

@@ -130,13 +130,35 @@ func (dst *MachineList) ConvertFrom(srcRaw conversion.Hub) error {
 func (src *MachineSet) ConvertTo(dstRaw conversion.Hub) error {
 	dst := dstRaw.(*v1alpha4.MachineSet)
 
-	return Convert_v1alpha3_MachineSet_To_v1alpha4_MachineSet(src, dst, nil)
+	if err := Convert_v1alpha3_MachineSet_To_v1alpha4_MachineSet(src, dst, nil); err != nil {
+		return err
+	}
+	// Manually restore data.
+	restored := &v1alpha4.MachineSet{}
+	if ok, err := utilconversion.UnmarshalData(src, restored); err != nil || !ok {
+		return err
+	}
+	dst.Status.Conditions = restored.Status.Conditions
+	return nil
 }
 
 func (dst *MachineSet) ConvertFrom(srcRaw conversion.Hub) error {
 	src := srcRaw.(*v1alpha4.MachineSet)
 
-	return Convert_v1alpha4_MachineSet_To_v1alpha3_MachineSet(src, dst, nil)
+	if err := Convert_v1alpha4_MachineSet_To_v1alpha3_MachineSet(src, dst, nil); err != nil {
+		return err
+	}
+
+	// Preserve Hub data on down-conversion except for metadata
+	if err := utilconversion.MarshalData(src, dst); err != nil {
+		return err
+	}
+	return nil
+}
+
+// Status.Conditions was introduced in v1alpha4, thus requiring a custom conversion function; the values is going to be preserved in an annotation thus allowing roundtrip without loosing informations
+func Convert_v1alpha4_MachineSetStatus_To_v1alpha3_MachineSetStatus(in *v1alpha4.MachineSetStatus, out *MachineSetStatus, s apiconversion.Scope) error {
+	return autoConvert_v1alpha4_MachineSetStatus_To_v1alpha3_MachineSetStatus(in, out, s)
 }
 
 func (src *MachineSetList) ConvertTo(dstRaw conversion.Hub) error {

api/v1alpha3/zz_generated.conversion.go

@@ -218,3 +218,36 @@ const (
 	// WaitingForAvailableMachinesReason (Severity=Warning) reflects the fact that the required minimum number of machines for a machinedeployment are not available.
 	WaitingForAvailableMachinesReason = "WaitingForAvailableMachines"
 )
+
+// Conditions and condition Reasons for  MachineSets
+
+const (
+	// MachinesCreatedCondition documents that the machines controlled by the MachineSet are created.
+	// When this condition is false, it indicates that there was an error when cloning the infrastructure/bootstrap template or
+	// when generating the machine object.
+	MachinesCreatedCondition ConditionType = "MachinesCreated"
+
+	// MachinesReadyCondition reports an aggregate of current status of the machines controlled by the MachineSet.
+	MachinesReadyCondition ConditionType = "MachinesReady"
+
+	// BootstrapTemplateCloningFailedReason (Severity=Error) documents a MachineSet failing to
+	// clone the bootstrap template.
+	BootstrapTemplateCloningFailedReason = "BootstrapTemplateCloningFailed"
+
+	// InfrastructureTemplateCloningFailedReason (Severity=Error) documents a MachineSet failing to
+	// clone the infrastructure template.
+	InfrastructureTemplateCloningFailedReason = "InfrastructureTemplateCloningFailed"
+
+	// MachineCreationFailedReason (Severity=Error) documents a MachineSet failing to
+	// generate a machine object.
+	MachineCreationFailedReason = "MachineCreationFailed"
+
+	// ResizedCondition documents a MachineSet is resizing the set of controlled machines.
+	ResizedCondition ConditionType = "Resized"
+
+	// ScalingUpReason (Severity=Info) documents a MachineSet is increasing the number of replicas.
+	ScalingUpReason = "ScalingUp"
+
+	// ScalingDownReason (Severity=Info) documents a MachineSet is decreasing the number of replicas.
+	ScalingDownReason = "ScalingDown"
+)

api/v1alpha4/condition_consts.go

@@ -163,6 +163,9 @@ type MachineSetStatus struct {
 	FailureReason *capierrors.MachineSetStatusError `json:"failureReason,omitempty"`
 	// +optional
 	FailureMessage *string `json:"failureMessage,omitempty"`
+	// Conditions defines current service state of the MachineSet.
+	// +optional
+	Conditions Conditions `json:"conditions,omitempty"`
 }
 
 // ANCHOR_END: MachineSetStatus
@@ -222,3 +225,13 @@ type MachineSetList struct {
 func init() {
 	SchemeBuilder.Register(&MachineSet{}, &MachineSetList{})
 }
+
+// GetConditions returns the set of conditions for the MachineSet.
+func (m *MachineSet) GetConditions() Conditions {
+	return m.Status.Conditions
+}
+
+// SetConditions updates the set of conditions on the MachineSet.
+func (m *MachineSet) SetConditions(conditions Conditions) {
+	m.Status.Conditions = conditions
+}

api/v1alpha4/machineset_types.go

@@ -743,6 +743,50 @@ spec:
                   minReadySeconds) for this MachineSet.
                 format: int32
                 type: integer
+              conditions:
+                description: Conditions defines current service state of the MachineSet.
+                items:
+                  description: Condition defines an observation of a Cluster API resource
+                    operational state.
+                  properties:
+                    lastTransitionTime:
+                      description: Last time the condition transitioned from one status
+                        to another. This should be when the underlying condition changed.
+                        If that is not known, then using the time when the API field
+                        changed is acceptable.
+                      format: date-time
+                      type: string
+                    message:
+                      description: A human readable message indicating details about
+                        the transition. This field may be empty.
+                      type: string
+                    reason:
+                      description: The reason for the condition's last transition
+                        in CamelCase. The specific API may choose whether or not this
+                        field is considered a guaranteed API. This field may not be
+                        empty.
+                      type: string
+                    severity:
+                      description: Severity provides an explicit classification of
+                        Reason code, so the users or machines can immediately understand
+                        the current situation and act accordingly. The Severity field
+                        MUST be set only when Status=False.
+                      type: string
+                    status:
+                      description: Status of the condition, one of True, False, Unknown.
+                      type: string
+                    type:
+                      description: Type of condition in CamelCase or in foo.example.com/CamelCase.
+                        Many .condition.type values are consistent across resources
+                        like Available, but because arbitrary conditions can be useful
+                        (see .node.status.conditions), the ability to deconflict is
+                        important.
+                      type: string
+                  required:
+                  - status
+                  - type
+                  type: object
+                type: array
               failureMessage:
                 type: string
               failureReason:

api/v1alpha4/zz_generated.deepcopy.go

@@ -35,6 +35,7 @@ import (
 	"sigs.k8s.io/cluster-api/controllers/remote"
 	"sigs.k8s.io/cluster-api/util"
 	"sigs.k8s.io/cluster-api/util/annotations"
+	"sigs.k8s.io/cluster-api/util/collections"
 	"sigs.k8s.io/cluster-api/util/conditions"
 	utilconversion "sigs.k8s.io/cluster-api/util/conversion"
 	"sigs.k8s.io/cluster-api/util/patch"
@@ -140,7 +141,7 @@ func (r *MachineSetReconciler) Reconcile(ctx context.Context, req ctrl.Request)
 
 	defer func() {
 		// Always attempt to patch the object and status after each reconciliation.
-		if err := patchHelper.Patch(ctx, machineSet); err != nil {
+		if err := patchMachineSet(ctx, patchHelper, machineSet); err != nil {
 			reterr = kerrors.NewAggregate([]error{reterr, err})
 		}
 	}()
@@ -159,6 +160,28 @@ func (r *MachineSetReconciler) Reconcile(ctx context.Context, req ctrl.Request)
 	return result, err
 }
 
+func patchMachineSet(ctx context.Context, patchHelper *patch.Helper, machineSet *clusterv1.MachineSet, options ...patch.Option) error {
+	// Always update the readyCondition by summarizing the state of other conditions.
+	conditions.SetSummary(machineSet,
+		conditions.WithConditions(
+			clusterv1.MachinesCreatedCondition,
+			clusterv1.ResizedCondition,
+			clusterv1.MachinesReadyCondition,
+		),
+	)
+
+	// Patch the object, ignoring conflicts on the conditions owned by this controller.
+	options = append(options,
+		patch.WithOwnedConditions{Conditions: []clusterv1.ConditionType{
+			clusterv1.ReadyCondition,
+			clusterv1.MachinesCreatedCondition,
+			clusterv1.ResizedCondition,
+			clusterv1.MachinesReadyCondition,
+		}},
+	)
+	return patchHelper.Patch(ctx, machineSet, options...)
+}
+
 func (r *MachineSetReconciler) reconcile(ctx context.Context, cluster *clusterv1.Cluster, machineSet *clusterv1.MachineSet) (ctrl.Result, error) {
 	log := ctrl.LoggerFrom(ctx)
 	log.V(4).Info("Reconcile MachineSet")
@@ -347,6 +370,7 @@ func (r *MachineSetReconciler) syncReplicas(ctx context.Context, ms *clusterv1.M
 					Labels:      machine.Labels,
 				})
 				if err != nil {
+					conditions.MarkFalse(ms, clusterv1.MachinesCreatedCondition, clusterv1.BootstrapTemplateCloningFailedReason, clusterv1.ConditionSeverityError, err.Error())
 					return errors.Wrapf(err, "failed to clone bootstrap configuration for MachineSet %q in namespace %q", ms.Name, ms.Namespace)
 				}
 				machine.Spec.Bootstrap.ConfigRef = bootstrapRef
@@ -361,6 +385,7 @@ func (r *MachineSetReconciler) syncReplicas(ctx context.Context, ms *clusterv1.M
 				Annotations: machine.Annotations,
 			})
 			if err != nil {
+				conditions.MarkFalse(ms, clusterv1.MachinesCreatedCondition, clusterv1.InfrastructureTemplateCloningFailedReason, clusterv1.ConditionSeverityError, err.Error())
 				return errors.Wrapf(err, "failed to clone infrastructure configuration for MachineSet %q in namespace %q", ms.Name, ms.Namespace)
 			}
 			machine.Spec.InfrastructureRef = *infraRef
@@ -369,6 +394,8 @@ func (r *MachineSetReconciler) syncReplicas(ctx context.Context, ms *clusterv1.M
 				log.Error(err, "Unable to create Machine", "machine", machine.Name)
 				r.recorder.Eventf(ms, corev1.EventTypeWarning, "FailedCreate", "Failed to create machine %q: %v", machine.Name, err)
 				errs = append(errs, err)
+				conditions.MarkFalse(ms, clusterv1.MachinesCreatedCondition, clusterv1.MachineCreationFailedReason,
+					clusterv1.ConditionSeverityError, err.Error())
 
 				// Try to cleanup the external objects if the Machine creation failed.
 				if err := r.Client.Delete(ctx, util.ObjectReferenceToUnstructured(*infraRef)); !apierrors.IsNotFound(err) {
@@ -598,6 +625,7 @@ func (r *MachineSetReconciler) updateStatus(ctx context.Context, cluster *cluste
 	fullyLabeledReplicasCount := 0
 	readyReplicasCount := 0
 	availableReplicasCount := 0
+	desiredReplicas := *ms.Spec.Replicas
 	templateLabel := labels.Set(ms.Spec.Template.Labels).AsSelectorPreValidated()
 
 	for _, machine := range filteredMachines {
@@ -641,12 +669,35 @@ func (r *MachineSetReconciler) updateStatus(ctx context.Context, cluster *cluste
 		newStatus.DeepCopyInto(&ms.Status)
 
 		log.V(4).Info(fmt.Sprintf("Updating status for %v: %s/%s, ", ms.Kind, ms.Namespace, ms.Name) +
-			fmt.Sprintf("replicas %d->%d (need %d), ", ms.Status.Replicas, newStatus.Replicas, *ms.Spec.Replicas) +
+			fmt.Sprintf("replicas %d->%d (need %d), ", ms.Status.Replicas, newStatus.Replicas, desiredReplicas) +
 			fmt.Sprintf("fullyLabeledReplicas %d->%d, ", ms.Status.FullyLabeledReplicas, newStatus.FullyLabeledReplicas) +
 			fmt.Sprintf("readyReplicas %d->%d, ", ms.Status.ReadyReplicas, newStatus.ReadyReplicas) +
 			fmt.Sprintf("availableReplicas %d->%d, ", ms.Status.AvailableReplicas, newStatus.AvailableReplicas) +
 			fmt.Sprintf("sequence No: %v->%v", ms.Status.ObservedGeneration, newStatus.ObservedGeneration))
 	}
+	switch {
+	// We are scaling up
+	case newStatus.Replicas < desiredReplicas:
+		conditions.MarkFalse(ms, clusterv1.ResizedCondition, clusterv1.ScalingUpReason, clusterv1.ConditionSeverityWarning, "Scaling up MachineSet to %d replicas (actual %d)", desiredReplicas, newStatus.Replicas)
+	// We are scaling down
+	case newStatus.Replicas > desiredReplicas:
+		conditions.MarkFalse(ms, clusterv1.ResizedCondition, clusterv1.ScalingDownReason, clusterv1.ConditionSeverityWarning, "Scaling down MachineSet to %d replicas (actual %d)", desiredReplicas, newStatus.Replicas)
+		// This means that there was no error in generating the desired number of machine objects
+		conditions.MarkTrue(ms, clusterv1.MachinesCreatedCondition)
+	default:
+		// Make sure last resize operation is marked as completed.
+		// NOTE: we are checking the number of machines ready so we report resize completed only when the machines
+		// are actually provisioned (vs reporting completed immediately after the last machine object is created). This convention is also used by KCP.
+		if newStatus.ReadyReplicas == newStatus.Replicas {
+			conditions.MarkTrue(ms, clusterv1.ResizedCondition)
+		}
+		// This means that there was no error in generating the desired number of machine objects
+		conditions.MarkTrue(ms, clusterv1.MachinesCreatedCondition)
+	}
+
+	// Aggregate the operational state of all the machines; while aggregating we are adding the
+	// source ref (reason@machine/name) so the problem can be easily tracked down to its source machine.
+	conditions.SetAggregate(ms, clusterv1.MachinesReadyCondition, collections.FromMachines(filteredMachines...).ConditionGetters(), conditions.AddSourceRef(), conditions.WithStepCounterIf(false))
 
 	return nil
 }