Merge pull request #3136 from fabriziopandini/add-conditions-to-machine

🌱 Add conditions to the Machine object

api/v1alpha2/conversion.go

@@ -118,6 +118,7 @@ func (src *Machine) ConvertTo(dstRaw conversion.Hub) error {
 	}
 	restoreMachineSpec(&restored.Spec, &dst.Spec)
 	dst.Status.ObservedGeneration = restored.Status.ObservedGeneration
+	dst.Status.Conditions = restored.Status.Conditions
 
 	return nil
 }

api/v1alpha3/condition_consts.go

@@ -25,3 +25,34 @@ const (
 )
 
 // ANCHOR_END: CommonConditions
+
+// Conditions and condition Reasons for the Machine object
+
+// MachineSummaryConditionsCount defines the total number of conditions on the Machine object.
+const MachineSummaryConditionsCount = 2
+
+const (
+	// BootstrapReadyCondition reports a summary of current status of the bootstrap object defined for this machine.
+	// This condition is mirrored from the Ready condition in the bootstrap ref object, and
+	// the absence of this condition might signal problems in the reconcile external loops or the fact that
+	// the bootstrap provider does not not implements the Ready condition yet.
+	BootstrapReadyCondition ConditionType = "BootstrapReady"
+
+	// WaitingForDataSecretFallbackReason (Severity=Info) documents a machine waiting for the bootstrap data secret
+	// to be available.
+	// NOTE: This reason is used only as a fallback when the bootstrap object is not reporting its own ready condition.
+	WaitingForDataSecretFallbackReason = "WaitingForDataSecret"
+)
+
+const (
+	// InfrastructureReadyCondition reports a summary of current status of the infrastructure object defined for this machine.
+	// This condition is mirrored from the Ready condition in the infrastructure ref object, and
+	// the absence of this condition might signal problems in the reconcile external loops or the fact that
+	// the infrastructure provider does not not implements the Ready condition yet.
+	InfrastructureReadyCondition ConditionType = "InfrastructureReady"
+
+	// WaitingForInfrastructureFallbackReason (Severity=Info) documents a machine waiting for the machine infrastructure
+	// to be available.
+	// NOTE: This reason is used only as a fallback when the infrastructure object is not reporting its own ready condition.
+	WaitingForInfrastructureFallbackReason = "WaitingForInfrastructure"
+)

api/v1alpha3/machine_types.go

@@ -159,6 +159,10 @@ type MachineStatus struct {
 	// ObservedGeneration is the latest generation observed by the controller.
 	// +optional
 	ObservedGeneration int64 `json:"observedGeneration,omitempty"`
+
+	// Conditions defines current service state of the Machine.
+	// +optional
+	Conditions Conditions `json:"conditions,omitempty"`
 }
 
 // ANCHOR_END: MachineStatus
@@ -231,6 +235,14 @@ type Machine struct {
 	Status MachineStatus `json:"status,omitempty"`
 }
 
+func (m *Machine) GetConditions() Conditions {
+	return m.Status.Conditions
+}
+
+func (m *Machine) SetConditions(conditions Conditions) {
+	m.Status.Conditions = conditions
+}
+
 // +kubebuilder:object:root=true
 
 // MachineList contains a list of Machine

config/crd/bases/cluster.x-k8s.io_machines.yaml

@@ -560,6 +560,50 @@ spec:
               bootstrapReady:
                 description: BootstrapReady is the state of the bootstrap provider.
                 type: boolean
+              conditions:
+                description: Conditions defines current service state of the Machine.
+                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:
                 description: "FailureMessage will be set in the event that there is
                   a terminal problem reconciling the Machine and will contain a more

controllers/cluster_controller_phases_test.go

@@ -130,9 +130,9 @@ func TestClusterReconcilePhases(t *testing.T) {
 				var c client.Client
 				if tt.infraRef != nil {
 					infraConfig := &unstructured.Unstructured{Object: tt.infraRef}
-					c = fake.NewFakeClientWithScheme(scheme.Scheme, external.TestGenericInfrastructureCRD, tt.cluster, infraConfig)
+					c = fake.NewFakeClientWithScheme(scheme.Scheme, external.TestGenericInfrastructureCRD.DeepCopy(), tt.cluster, infraConfig)
 				} else {
-					c = fake.NewFakeClientWithScheme(scheme.Scheme, external.TestGenericInfrastructureCRD, tt.cluster)
+					c = fake.NewFakeClientWithScheme(scheme.Scheme, external.TestGenericInfrastructureCRD.DeepCopy(), tt.cluster)
 				}
 				r := &ClusterReconciler{
 					Client: c,

controllers/machine_controller.go

@@ -43,6 +43,7 @@ import (
 	kubedrain "sigs.k8s.io/cluster-api/third_party/kubernetes-drain"
 	"sigs.k8s.io/cluster-api/util"
 	"sigs.k8s.io/cluster-api/util/annotations"
+	"sigs.k8s.io/cluster-api/util/conditions"
 	"sigs.k8s.io/cluster-api/util/patch"
 	"sigs.k8s.io/cluster-api/util/predicates"
 	ctrl "sigs.k8s.io/controller-runtime"
@@ -166,6 +167,13 @@ func (r *MachineReconciler) Reconcile(req ctrl.Request) (_ ctrl.Result, reterr e
 	}
 
 	defer func() {
+		// always update the readyCondition with the summary of the machine conditions.
+		conditions.SetSummary(m,
+			// we want to surface infrastructure problems first, then the others.
+			conditions.WithConditionOrder(clusterv1.InfrastructureReadyCondition),
+			conditions.WithStepCounter(clusterv1.MachineSummaryConditionsCount),
+		)
+
 		r.reconcilePhase(ctx, m)
 		r.reconcileMetrics(ctx, m)
 

controllers/machine_controller_phases.go

@@ -29,6 +29,7 @@ import (
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/utils/pointer"
 	"sigs.k8s.io/cluster-api/util/annotations"
+	"sigs.k8s.io/cluster-api/util/conditions"
 	utilconversion "sigs.k8s.io/cluster-api/util/conversion"
 	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
 	"sigs.k8s.io/controller-runtime/pkg/handler"
@@ -158,6 +159,14 @@ func (r *MachineReconciler) reconcileExternal(ctx context.Context, cluster *clus
 
 // reconcileBootstrap reconciles the Spec.Bootstrap.ConfigRef object on a Machine.
 func (r *MachineReconciler) reconcileBootstrap(ctx context.Context, cluster *clusterv1.Cluster, m *clusterv1.Machine) error {
+	// If the bootstrap data is populated, set ready and return.
+	if m.Spec.Bootstrap.DataSecretName != nil {
+		m.Status.BootstrapReady = true
+		conditions.MarkTrue(m, clusterv1.BootstrapReadyCondition)
+		return nil
+	}
+
+	// If the Boostrap ref is nil (and so the machine should use user generated data secret), return.
 	if m.Spec.Bootstrap.ConfigRef == nil {
 		return nil
 	}
@@ -172,12 +181,6 @@ func (r *MachineReconciler) reconcileBootstrap(ctx context.Context, cluster *clu
 	}
 	bootstrapConfig := externalResult.Result
 
-	// If the bootstrap data is populated, set ready and return.
-	if m.Spec.Bootstrap.DataSecretName != nil {
-		m.Status.BootstrapReady = true
-		return nil
-	}
-
 	// If the bootstrap config is being deleted, return early.
 	if !bootstrapConfig.GetDeletionTimestamp().IsZero() {
 		return nil
@@ -187,7 +190,16 @@ func (r *MachineReconciler) reconcileBootstrap(ctx context.Context, cluster *clu
 	ready, err := external.IsReady(bootstrapConfig)
 	if err != nil {
 		return err
-	} else if !ready {
+	}
+
+	// Report a summary of current status of the bootstrap object defined for this machine.
+	conditions.SetMirror(m, clusterv1.BootstrapReadyCondition,
+		conditions.UnstructuredGetter(bootstrapConfig),
+		conditions.WithFallbackValue(ready, clusterv1.WaitingForDataSecretFallbackReason, clusterv1.ConditionSeverityInfo, ""),
+	)
+
+	// If the bootstrap provider is not ready, requeue.
+	if !ready {
 		return errors.Wrapf(&capierrors.RequeueAfterError{RequeueAfter: externalReadyWait},
 			"Bootstrap provider for Machine %q in namespace %q is not ready, requeuing", m.Name, m.Namespace)
 	}
@@ -236,6 +248,14 @@ func (r *MachineReconciler) reconcileInfrastructure(ctx context.Context, cluster
 		return err
 	}
 	m.Status.InfrastructureReady = ready
+
+	// Report a summary of current status of the infrastructure object defined for this machine.
+	conditions.SetMirror(m, clusterv1.InfrastructureReadyCondition,
+		conditions.UnstructuredGetter(infraConfig),
+		conditions.WithFallbackValue(ready, clusterv1.WaitingForInfrastructureFallbackReason, clusterv1.ConditionSeverityInfo, ""),
+	)
+
+	// If the infrastructure provider is not ready, return early.
 	if !ready {
 		return errors.Wrapf(&capierrors.RequeueAfterError{RequeueAfter: externalReadyWait},
 			"Infrastructure provider for Machine %q in namespace %q is not ready, requeuing", m.Name, m.Namespace,

controllers/machine_controller_phases_test.go

@@ -118,8 +118,8 @@ var _ = Describe("Reconcile Machine Phases", func() {
 				defaultCluster,
 				defaultKubeconfigSecret,
 				machine,
-				external.TestGenericBootstrapCRD,
-				external.TestGenericInfrastructureCRD,
+				external.TestGenericBootstrapCRD.DeepCopy(),
+				external.TestGenericInfrastructureCRD.DeepCopy(),
 				bootstrapConfig,
 				infraConfig,
 			),
@@ -154,8 +154,8 @@ var _ = Describe("Reconcile Machine Phases", func() {
 				defaultCluster,
 				defaultKubeconfigSecret,
 				machine,
-				external.TestGenericBootstrapCRD,
-				external.TestGenericInfrastructureCRD,
+				external.TestGenericBootstrapCRD.DeepCopy(),
+				external.TestGenericInfrastructureCRD.DeepCopy(),
 				bootstrapConfig,
 				infraConfig,
 			),
@@ -195,8 +195,8 @@ var _ = Describe("Reconcile Machine Phases", func() {
 				defaultCluster,
 				defaultKubeconfigSecret,
 				machine,
-				external.TestGenericBootstrapCRD,
-				external.TestGenericInfrastructureCRD,
+				external.TestGenericBootstrapCRD.DeepCopy(),
+				external.TestGenericInfrastructureCRD.DeepCopy(),
 				bootstrapConfig,
 				infraConfig,
 			),
@@ -262,8 +262,8 @@ var _ = Describe("Reconcile Machine Phases", func() {
 				defaultCluster,
 				defaultKubeconfigSecret,
 				machine,
-				external.TestGenericBootstrapCRD,
-				external.TestGenericInfrastructureCRD,
+				external.TestGenericBootstrapCRD.DeepCopy(),
+				external.TestGenericInfrastructureCRD.DeepCopy(),
 				bootstrapConfig,
 				infraConfig,
 			),
@@ -316,8 +316,8 @@ var _ = Describe("Reconcile Machine Phases", func() {
 				defaultCluster,
 				defaultKubeconfigSecret,
 				machine,
-				external.TestGenericBootstrapCRD,
-				external.TestGenericInfrastructureCRD,
+				external.TestGenericBootstrapCRD.DeepCopy(),
+				external.TestGenericInfrastructureCRD.DeepCopy(),
 				bootstrapConfig,
 				infraConfig,
 			),
@@ -381,8 +381,8 @@ var _ = Describe("Reconcile Machine Phases", func() {
 				defaultCluster,
 				defaultKubeconfigSecret,
 				machine,
-				external.TestGenericBootstrapCRD,
-				external.TestGenericInfrastructureCRD,
+				external.TestGenericBootstrapCRD.DeepCopy(),
+				external.TestGenericInfrastructureCRD.DeepCopy(),
 				bootstrapConfig,
 				infraConfig,
 			),
@@ -426,8 +426,8 @@ var _ = Describe("Reconcile Machine Phases", func() {
 				defaultCluster,
 				defaultKubeconfigSecret,
 				machine,
-				external.TestGenericBootstrapCRD,
-				external.TestGenericInfrastructureCRD,
+				external.TestGenericBootstrapCRD.DeepCopy(),
+				external.TestGenericInfrastructureCRD.DeepCopy(),
 				bootstrapConfig,
 				infraConfig,
 			),
@@ -493,8 +493,8 @@ var _ = Describe("Reconcile Machine Phases", func() {
 				defaultCluster,
 				defaultKubeconfigSecret,
 				machine,
-				external.TestGenericBootstrapCRD,
-				external.TestGenericInfrastructureCRD,
+				external.TestGenericBootstrapCRD.DeepCopy(),
+				external.TestGenericInfrastructureCRD.DeepCopy(),
 				bootstrapConfig,
 				infraConfig,
 			),
@@ -730,8 +730,8 @@ func TestReconcileBootstrap(t *testing.T) {
 			r := &MachineReconciler{
 				Client: fake.NewFakeClientWithScheme(scheme.Scheme,
 					tc.machine,
-					external.TestGenericBootstrapCRD,
-					external.TestGenericInfrastructureCRD,
+					external.TestGenericBootstrapCRD.DeepCopy(),
+					external.TestGenericInfrastructureCRD.DeepCopy(),
 					bootstrapConfig,
 				),
 				Log:    log.Log,
@@ -931,8 +931,8 @@ func TestReconcileInfrastructure(t *testing.T) {
 			r := &MachineReconciler{
 				Client: fake.NewFakeClientWithScheme(scheme.Scheme,
 					tc.machine,
-					external.TestGenericBootstrapCRD,
-					external.TestGenericInfrastructureCRD,
+					external.TestGenericBootstrapCRD.DeepCopy(),
+					external.TestGenericInfrastructureCRD.DeepCopy(),
 					infraConfig,
 				),
 				Log:    log.Log,