From 5113f801f130f2d8f0a1b90403b7d868c696c1b8 Mon Sep 17 00:00:00 2001 From: fabriziopandini Date: Mon, 8 Jun 2020 20:44:20 +0200 Subject: [PATCH] Add conditions to the Machine object --- api/v1alpha2/conversion.go | 1 + api/v1alpha2/zz_generated.conversion.go | 1 + api/v1alpha3/condition_consts.go | 31 +++++++++++++ api/v1alpha3/machine_types.go | 12 +++++ api/v1alpha3/zz_generated.deepcopy.go | 7 +++ .../crd/bases/cluster.x-k8s.io_machines.yaml | 44 +++++++++++++++++++ controllers/machine_controller.go | 8 ++++ controllers/machine_controller_phases.go | 34 +++++++++++--- controllers/machine_controller_test.go | 20 ++++----- 9 files changed, 141 insertions(+), 17 deletions(-) diff --git a/api/v1alpha2/conversion.go b/api/v1alpha2/conversion.go index 98677ce2be9d..00974460d07e 100644 --- a/api/v1alpha2/conversion.go +++ b/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 } diff --git a/api/v1alpha2/zz_generated.conversion.go b/api/v1alpha2/zz_generated.conversion.go index 188ed97bc4fd..c096b057d71d 100644 --- a/api/v1alpha2/zz_generated.conversion.go +++ b/api/v1alpha2/zz_generated.conversion.go @@ -918,6 +918,7 @@ func autoConvert_v1alpha3_MachineStatus_To_v1alpha2_MachineStatus(in *v1alpha3.M out.BootstrapReady = in.BootstrapReady out.InfrastructureReady = in.InfrastructureReady // WARNING: in.ObservedGeneration requires manual conversion: does not exist in peer-type + // WARNING: in.Conditions requires manual conversion: does not exist in peer-type return nil } diff --git a/api/v1alpha3/condition_consts.go b/api/v1alpha3/condition_consts.go index 4c216d2be996..e8fdf7da7930 100644 --- a/api/v1alpha3/condition_consts.go +++ b/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" +) diff --git a/api/v1alpha3/machine_types.go b/api/v1alpha3/machine_types.go index a164f5a2368f..32fe0dc431df 100644 --- a/api/v1alpha3/machine_types.go +++ b/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 diff --git a/api/v1alpha3/zz_generated.deepcopy.go b/api/v1alpha3/zz_generated.deepcopy.go index 71b8377fa46f..a6efd2d9c6d7 100644 --- a/api/v1alpha3/zz_generated.deepcopy.go +++ b/api/v1alpha3/zz_generated.deepcopy.go @@ -841,6 +841,13 @@ func (in *MachineStatus) DeepCopyInto(out *MachineStatus) { *out = make(MachineAddresses, len(*in)) copy(*out, *in) } + if in.Conditions != nil { + in, out := &in.Conditions, &out.Conditions + *out = make(Conditions, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new MachineStatus. diff --git a/config/crd/bases/cluster.x-k8s.io_machines.yaml b/config/crd/bases/cluster.x-k8s.io_machines.yaml index 501eb79eb84a..9480df7aa308 100644 --- a/config/crd/bases/cluster.x-k8s.io_machines.yaml +++ b/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 diff --git a/controllers/machine_controller.go b/controllers/machine_controller.go index ab7feca5d3c1..c686570c2174 100644 --- a/controllers/machine_controller.go +++ b/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) diff --git a/controllers/machine_controller_phases.go b/controllers/machine_controller_phases.go index 73cf303a9804..1fc2732bd8a6 100644 --- a/controllers/machine_controller_phases.go +++ b/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, diff --git a/controllers/machine_controller_test.go b/controllers/machine_controller_test.go index ee4d1574ba68..e33efb44b2ba 100644 --- a/controllers/machine_controller_test.go +++ b/controllers/machine_controller_test.go @@ -31,9 +31,9 @@ import ( "k8s.io/utils/pointer" clusterv1 "sigs.k8s.io/cluster-api/api/v1alpha3" "sigs.k8s.io/cluster-api/controllers/external" + "sigs.k8s.io/cluster-api/test/helpers" "sigs.k8s.io/cluster-api/util" "sigs.k8s.io/controller-runtime/pkg/client" - "sigs.k8s.io/controller-runtime/pkg/client/fake" "sigs.k8s.io/controller-runtime/pkg/handler" "sigs.k8s.io/controller-runtime/pkg/log" "sigs.k8s.io/controller-runtime/pkg/metrics" @@ -105,7 +105,7 @@ func TestMachineFinalizer(t *testing.T) { g := NewWithT(t) mr := &MachineReconciler{ - Client: fake.NewFakeClientWithScheme( + Client: helpers.NewFakeClientWithScheme( scheme.Scheme, clusterCorrectMeta, machineValidCluster, @@ -266,7 +266,7 @@ func TestMachineOwnerReference(t *testing.T) { g := NewWithT(t) mr := &MachineReconciler{ - Client: fake.NewFakeClientWithScheme( + Client: helpers.NewFakeClientWithScheme( scheme.Scheme, testCluster, machineInvalidCluster, @@ -419,11 +419,11 @@ func TestReconcileRequest(t *testing.T) { t.Run("machine should be "+tc.machine.Name, func(t *testing.T) { g := NewWithT(t) - clientFake := fake.NewFakeClientWithScheme( + clientFake := helpers.NewFakeClientWithScheme( scheme.Scheme, &testCluster, &tc.machine, - external.TestGenericInfrastructureCRD, + external.TestGenericInfrastructureCRD.DeepCopy(), &infraConfig, ) @@ -546,7 +546,7 @@ func TestReconcileDeleteExternal(t *testing.T) { } r := &MachineReconciler{ - Client: fake.NewFakeClientWithScheme(scheme.Scheme, objs...), + Client: helpers.NewFakeClientWithScheme(scheme.Scheme, objs...), Log: log.Log, scheme: scheme.Scheme, } @@ -590,7 +590,7 @@ func TestRemoveMachineFinalizerAfterDeleteReconcile(t *testing.T) { } key := client.ObjectKey{Namespace: m.Namespace, Name: m.Name} mr := &MachineReconciler{ - Client: fake.NewFakeClientWithScheme(scheme.Scheme, testCluster, m), + Client: helpers.NewFakeClientWithScheme(scheme.Scheme, testCluster, m), Log: log.Log, scheme: scheme.Scheme, } @@ -667,7 +667,7 @@ func TestReconcileMetrics(t *testing.T) { objs = append(objs, machine) r := &MachineReconciler{ - Client: fake.NewFakeClientWithScheme(scheme.Scheme, objs...), + Client: helpers.NewFakeClientWithScheme(scheme.Scheme, objs...), Log: log.Log, scheme: scheme.Scheme, } @@ -778,7 +778,7 @@ func Test_clusterToActiveMachines(t *testing.T) { objs = append(objs, m2) r := &MachineReconciler{ - Client: fake.NewFakeClientWithScheme(scheme.Scheme, objs...), + Client: helpers.NewFakeClientWithScheme(scheme.Scheme, objs...), Log: log.Log, scheme: scheme.Scheme, } @@ -952,7 +952,7 @@ func TestIsDeleteNodeAllowed(t *testing.T) { } mr := &MachineReconciler{ - Client: fake.NewFakeClientWithScheme( + Client: helpers.NewFakeClientWithScheme( scheme.Scheme, tc.cluster, tc.machine,