Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

🌱 Use internal error reason consistently #11309

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 5 additions & 7 deletions api/v1beta1/machine_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -98,13 +98,12 @@ const (
// MachineWaitingForMinReadySecondsV1Beta2Reason surfaces when a machine is ready for less than MinReadySeconds (and thus not yet available).
MachineWaitingForMinReadySecondsV1Beta2Reason = "WaitingForMinReadySeconds"

// MachineReadyNotYetReportedV1Beta2Reason surfaces when a machine ready is not reported yet.
// Note: this should never happen and it is a signal of some internal error.
MachineReadyNotYetReportedV1Beta2Reason = "ReadyNotYetReported"

// MachineAvailableV1Beta2Reason surfaces when a machine is ready for at least MinReadySeconds.
// Note: MinReadySeconds is assumed 0 until it will be implemented in v1beta2 API.
MachineAvailableV1Beta2Reason = AvailableV1Beta2Reason

// MachineAvailableInternalErrorV1Beta2Reason surfaces unexpected error when computing the Available condition.
MachineAvailableInternalErrorV1Beta2Reason = InternalErrorV1Beta2Reason
)

// Machine's Ready condition and corresponding reasons that will be used in v1Beta2 API version.
Expand All @@ -115,9 +114,8 @@ const (
// these conditions must be true as well.
MachineReadyV1Beta2Condition = ReadyV1Beta2Condition

// MachineErrorComputingReadyV1Beta2Reason surfaces when there was an error computing the ready condition.
// Note: this should never happen and it is a signal of some internal error.
MachineErrorComputingReadyV1Beta2Reason = "ErrorComputingReady"
// MachineReadyInternalErrorV1Beta2Reason surfaces unexpected error when computing the Ready condition.
MachineReadyInternalErrorV1Beta2Reason = InternalErrorV1Beta2Reason
)

// Machine's UpToDate condition and corresponding reasons that will be used in v1Beta2 API version.
Expand Down
16 changes: 11 additions & 5 deletions internal/controllers/machine/machine_controller_status.go
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,7 @@ func setBootstrapReadyCondition(_ context.Context, machine *clusterv1.Machine, b
Status: metav1.ConditionUnknown,
Reason: clusterv1.MachineBootstrapConfigInternalErrorV1Beta2Reason,
Message: "Please check controller logs for errors",
// NOTE: the error is logged by reconcileBootstrap.
})
return
}
Expand Down Expand Up @@ -169,6 +170,7 @@ func setInfrastructureReadyCondition(_ context.Context, machine *clusterv1.Machi
Status: metav1.ConditionUnknown,
Reason: clusterv1.MachineInfrastructureInternalErrorV1Beta2Reason,
Message: "Please check controller logs for errors",
// NOTE: the error is logged by reconcileInfrastructure.
})
return
}
Expand Down Expand Up @@ -276,13 +278,15 @@ func setNodeHealthyAndReadyConditions(ctx context.Context, machine *clusterv1.Ma
Status: metav1.ConditionUnknown,
Reason: clusterv1.MachineNodeInternalErrorV1Beta2Reason,
Message: "Please check controller logs for errors",
// NOTE: the error is logged by reconcileNode.
})

v1beta2conditions.Set(machine, metav1.Condition{
Type: clusterv1.MachineNodeHealthyV1Beta2Condition,
Status: metav1.ConditionUnknown,
Reason: clusterv1.MachineNodeInternalErrorV1Beta2Reason,
Message: "Please check controller logs for errors",
// NOTE: the error is logged by reconcileNode.
})
return
}
Expand Down Expand Up @@ -592,14 +596,14 @@ func setReadyCondition(ctx context.Context, machine *clusterv1.Machine) {
}

readyCondition, err := v1beta2conditions.NewSummaryCondition(machine, clusterv1.MachineReadyV1Beta2Condition, summaryOpts...)
if err != nil || readyCondition == nil {
if err != nil {
// Note, this could only happen if we hit edge cases in computing the summary, which should not happen due to the fact
// that we are passing a non empty list of ForConditionTypes.
log.Error(err, "Failed to set ready condition")
log.Error(err, "Failed to set Ready condition")
readyCondition = &metav1.Condition{
Type: clusterv1.MachineReadyV1Beta2Condition,
Status: metav1.ConditionUnknown,
Reason: clusterv1.MachineErrorComputingReadyV1Beta2Reason,
Reason: clusterv1.MachineReadyInternalErrorV1Beta2Reason,
Message: "Please check controller logs for errors",
}
}
Expand Down Expand Up @@ -650,16 +654,18 @@ func calculateDeletingConditionForSummary(machine *clusterv1.Machine) v1beta2con
}
}

func setAvailableCondition(_ context.Context, machine *clusterv1.Machine) {
func setAvailableCondition(ctx context.Context, machine *clusterv1.Machine) {
log := ctrl.LoggerFrom(ctx)
readyCondition := v1beta2conditions.Get(machine, clusterv1.MachineReadyV1Beta2Condition)

if readyCondition == nil {
// NOTE: this should never happen given that setReadyCondition is called before this method and
// it always add a ready condition.
log.Error(errors.New("Ready condition must be set before setting the available condition"), "Failed to set Available condition")
v1beta2conditions.Set(machine, metav1.Condition{
fabriziopandini marked this conversation as resolved.
Show resolved Hide resolved
Type: clusterv1.MachineAvailableV1Beta2Condition,
Status: metav1.ConditionUnknown,
Reason: clusterv1.MachineReadyNotYetReportedV1Beta2Reason,
Reason: clusterv1.MachineAvailableInternalErrorV1Beta2Reason,
Message: "Please check controller logs for errors",
})
return
Expand Down
2 changes: 1 addition & 1 deletion util/conditions/v1beta2/summary.go
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ func NewSummaryCondition(sourceObj Getter, targetConditionType string, opts ...S
Reason: reason,
Message: message,
// NOTE: LastTransitionTime and ObservedGeneration will be set when this condition is added to an object by calling Set.
}, err
}, nil
sbueringer marked this conversation as resolved.
Show resolved Hide resolved
}

// SetSummaryCondition is a convenience method that calls NewSummaryCondition to create a summary condition from the source object,
Expand Down