Consolidate result conversion and computation

Consolidate BuildRuntimeResult() into summarizeAndPatch() to simplify
where the results are computed, summarized and patched.

Move the event recording and logging of context specific errors into
RecordContextualError() and call it in summarizeAndPatch().

Introduce Waiting error for wait and requeue scenarios. Update
ComputeReconcileResult() and RecordContextualError() to consider Waiting
error.

Signed-off-by: Sunny <darkowlzz@protonmail.com>
This commit is contained in:
Sunny 2022-01-27 14:20:30 +05:30 committed by Hidde Beydals
parent 474658a076
commit 78882b3b36
8 changed files with 159 additions and 92 deletions

View File

@ -139,20 +139,19 @@ func (r *BucketReconciler) Reconcile(ctx context.Context, req ctrl.Request) (res
return ctrl.Result{}, nil
}
// Initialize the patch helper
// Initialize the patch helper with the current version of the object.
patchHelper, err := patch.NewHelper(obj, r.Client)
if err != nil {
return ctrl.Result{}, err
}
// recResult stores the abstracted reconcile result.
var recResult sreconcile.Result
// Always attempt to patch the object and status after each reconciliation
// NOTE: This deferred block only modifies the named return error. The
// result from the reconciliation remains the same. Any requeue attributes
// set in the result will continue to be effective.
// NOTE: The final runtime result and error are set in this block.
defer func() {
retErr = r.summarizeAndPatch(ctx, obj, patchHelper, recResult, retErr)
result, retErr = r.summarizeAndPatch(ctx, obj, patchHelper, recResult, retErr)
// Always record readiness and duration metrics
r.Metrics.RecordReadiness(ctx, obj)
@ -163,13 +162,13 @@ func (r *BucketReconciler) Reconcile(ctx context.Context, req ctrl.Request) (res
if !controllerutil.ContainsFinalizer(obj, sourcev1.SourceFinalizer) {
controllerutil.AddFinalizer(obj, sourcev1.SourceFinalizer)
recResult = sreconcile.ResultRequeue
return ctrl.Result{Requeue: true}, nil
return
}
// Examine if the object is under deletion
if !obj.ObjectMeta.DeletionTimestamp.IsZero() {
res, err := r.reconcileDelete(ctx, obj)
return sreconcile.BuildRuntimeResult(ctx, r.EventRecorder, obj, res, err)
recResult, retErr = r.reconcileDelete(ctx, obj)
return
}
// Reconcile actual object
@ -178,13 +177,21 @@ func (r *BucketReconciler) Reconcile(ctx context.Context, req ctrl.Request) (res
r.reconcileSource,
r.reconcileArtifact,
}
recResult, err = r.reconcile(ctx, obj, reconcilers)
return sreconcile.BuildRuntimeResult(ctx, r.EventRecorder, obj, recResult, err)
recResult, retErr = r.reconcile(ctx, obj, reconcilers)
return
}
// summarizeAndPatch analyzes the object conditions to create a summary of the
// status conditions and patches the object with the calculated summary.
func (r *BucketReconciler) summarizeAndPatch(ctx context.Context, obj *sourcev1.Bucket, patchHelper *patch.Helper, res sreconcile.Result, recErr error) error {
// status conditions, computes runtime results and patches the object in the K8s
// API server.
func (r *BucketReconciler) summarizeAndPatch(
ctx context.Context,
obj *sourcev1.Bucket,
patchHelper *patch.Helper,
res sreconcile.Result,
recErr error) (ctrl.Result, error) {
sreconcile.RecordContextualError(ctx, r.EventRecorder, obj, recErr)
// Record the value of the reconciliation request if any.
if v, ok := meta.ReconcileAnnotationValue(obj.GetAnnotations()); ok {
obj.Status.SetLastHandledReconcileRequest(v)
@ -192,7 +199,8 @@ func (r *BucketReconciler) summarizeAndPatch(ctx context.Context, obj *sourcev1.
// Compute the reconcile results, obtain patch options and reconcile error.
var patchOpts []patch.Option
patchOpts, recErr = sreconcile.ComputeReconcileResult(obj, res, recErr, bucketOwnedConditions)
var result ctrl.Result
patchOpts, result, recErr = sreconcile.ComputeReconcileResult(obj, obj.GetRequeueAfter(), res, recErr, bucketOwnedConditions)
// Summarize the Ready condition based on abnormalities that may have been observed.
conditions.SetSummary(obj,
@ -214,7 +222,7 @@ func (r *BucketReconciler) summarizeAndPatch(ctx context.Context, obj *sourcev1.
recErr = kerrors.NewAggregate([]error{recErr, err})
}
return recErr
return result, recErr
}
// reconcile steps iterates through the actual reconciliation tasks for objec,

View File

@ -145,20 +145,19 @@ func (r *GitRepositoryReconciler) Reconcile(ctx context.Context, req ctrl.Reques
return ctrl.Result{}, nil
}
// Initialize the patch helper
// Initialize the patch helper with the current version of the object.
patchHelper, err := patch.NewHelper(obj, r.Client)
if err != nil {
return ctrl.Result{}, err
}
// recResult stores the abstracted reconcile result.
var recResult sreconcile.Result
// Always attempt to patch the object and status after each reconciliation
// NOTE: This deferred block only modifies the named return error. The
// result from the reconciliation remains the same. Any requeue attributes
// set in the result will continue to be effective.
// NOTE: The final runtime result and error are set in this block.
defer func() {
retErr = r.summarizeAndPatch(ctx, obj, patchHelper, recResult, retErr)
result, retErr = r.summarizeAndPatch(ctx, obj, patchHelper, recResult, retErr)
// Always record readiness and duration metrics
r.Metrics.RecordReadiness(ctx, obj)
@ -170,13 +169,13 @@ func (r *GitRepositoryReconciler) Reconcile(ctx context.Context, req ctrl.Reques
if !controllerutil.ContainsFinalizer(obj, sourcev1.SourceFinalizer) {
controllerutil.AddFinalizer(obj, sourcev1.SourceFinalizer)
recResult = sreconcile.ResultRequeue
return ctrl.Result{Requeue: true}, nil
return
}
// Examine if the object is under deletion
if !obj.ObjectMeta.DeletionTimestamp.IsZero() {
res, err := r.reconcileDelete(ctx, obj)
return sreconcile.BuildRuntimeResult(ctx, r.EventRecorder, obj, res, err)
recResult, retErr = r.reconcileDelete(ctx, obj)
return
}
// Reconcile actual object
@ -186,13 +185,21 @@ func (r *GitRepositoryReconciler) Reconcile(ctx context.Context, req ctrl.Reques
r.reconcileInclude,
r.reconcileArtifact,
}
recResult, err = r.reconcile(ctx, obj, reconcilers)
return sreconcile.BuildRuntimeResult(ctx, r.EventRecorder, obj, recResult, err)
recResult, retErr = r.reconcile(ctx, obj, reconcilers)
return
}
// summarizeAndPatch analyzes the object conditions to create a summary of the
// status conditions and patches the object with the calculated summary.
func (r *GitRepositoryReconciler) summarizeAndPatch(ctx context.Context, obj *sourcev1.GitRepository, patchHelper *patch.Helper, res sreconcile.Result, recErr error) error {
// status conditions, computes runtime results and patches the object in the K8s
// API server.
func (r *GitRepositoryReconciler) summarizeAndPatch(
ctx context.Context,
obj *sourcev1.GitRepository,
patchHelper *patch.Helper,
res sreconcile.Result,
recErr error) (ctrl.Result, error) {
sreconcile.RecordContextualError(ctx, r.EventRecorder, obj, recErr)
// Record the value of the reconciliation request if any.
if v, ok := meta.ReconcileAnnotationValue(obj.GetAnnotations()); ok {
obj.Status.SetLastHandledReconcileRequest(v)
@ -200,7 +207,8 @@ func (r *GitRepositoryReconciler) summarizeAndPatch(ctx context.Context, obj *so
// Compute the reconcile results, obtain patch options and reconcile error.
var patchOpts []patch.Option
patchOpts, recErr = sreconcile.ComputeReconcileResult(obj, res, recErr, gitRepoOwnedConditions)
var result ctrl.Result
patchOpts, result, recErr = sreconcile.ComputeReconcileResult(obj, obj.GetRequeueAfter(), res, recErr, gitRepoOwnedConditions)
// Summarize the Ready condition based on abnormalities that may have been observed.
conditions.SetSummary(obj,
@ -222,7 +230,7 @@ func (r *GitRepositoryReconciler) summarizeAndPatch(ctx context.Context, obj *so
recErr = kerrors.NewAggregate([]error{recErr, err})
}
return recErr
return result, recErr
}
// reconcile steps iterates through the actual reconciliation tasks for objec,

View File

@ -169,21 +169,19 @@ func (r *HelmChartReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
return ctrl.Result{}, nil
}
// Initialize the patch helper
// Initialize the patch helper with the current version of the object.
patchHelper, err := patch.NewHelper(obj, r.Client)
if err != nil {
return ctrl.Result{}, err
}
// Result of the sub-reconciliation
// recResult stores the abstracted reconcile result.
var recResult sreconcile.Result
// Always attempt to patch the object after each reconciliation.
// NOTE: This deferred block only modifies the named return error. The
// result from the reconciliation remains the same. Any requeue attributes
// set in the result will continue to be effective.
// NOTE: The final runtime result and error are set in this block.
defer func() {
retErr = r.summarizeAndPatch(ctx, obj, patchHelper, recResult, retErr)
result, retErr = r.summarizeAndPatch(ctx, obj, patchHelper, recResult, retErr)
// Always record readiness and duration metrics
r.Metrics.RecordReadiness(ctx, obj)
@ -195,13 +193,13 @@ func (r *HelmChartReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
if !controllerutil.ContainsFinalizer(obj, sourcev1.SourceFinalizer) {
controllerutil.AddFinalizer(obj, sourcev1.SourceFinalizer)
recResult = sreconcile.ResultRequeue
return ctrl.Result{Requeue: true}, nil
return
}
// Examine if the object is under deletion
if !obj.ObjectMeta.DeletionTimestamp.IsZero() {
res, err := r.reconcileDelete(ctx, obj)
return sreconcile.BuildRuntimeResult(ctx, r.EventRecorder, obj, res, err)
recResult, retErr = r.reconcileDelete(ctx, obj)
return
}
// Reconcile actual object
@ -210,15 +208,21 @@ func (r *HelmChartReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
r.reconcileSource,
r.reconcileArtifact,
}
recResult, err = r.reconcile(ctx, obj, reconcilers)
return sreconcile.BuildRuntimeResult(ctx, r.EventRecorder, obj, recResult, err)
recResult, retErr = r.reconcile(ctx, obj, reconcilers)
return
}
// summarizeAndPatch analyzes the object conditions to create a summary of the
// status conditions and patches the object with the calculated summary. The
// reconciler error type is also used to determine the conditions and the
// returned error.
func (r *HelmChartReconciler) summarizeAndPatch(ctx context.Context, obj *sourcev1.HelmChart, patchHelper *patch.Helper, res sreconcile.Result, recErr error) error {
// status conditions, computes runtime results and patches the object in the K8s
// API server.
func (r *HelmChartReconciler) summarizeAndPatch(
ctx context.Context,
obj *sourcev1.HelmChart,
patchHelper *patch.Helper,
res sreconcile.Result,
recErr error) (ctrl.Result, error) {
sreconcile.RecordContextualError(ctx, r.EventRecorder, obj, recErr)
// Record the value of the reconciliation request, if any
if v, ok := meta.ReconcileAnnotationValue(obj.GetAnnotations()); ok {
obj.Status.SetLastHandledReconcileRequest(v)
@ -226,7 +230,8 @@ func (r *HelmChartReconciler) summarizeAndPatch(ctx context.Context, obj *source
// Compute the reconcile results, obtain patch options and reconcile error.
var patchOpts []patch.Option
patchOpts, recErr = sreconcile.ComputeReconcileResult(obj, res, recErr, helmChartOwnedConditions)
var result ctrl.Result
patchOpts, result, recErr = sreconcile.ComputeReconcileResult(obj, obj.GetRequeueAfter(), res, recErr, helmChartOwnedConditions)
// Summarize Ready condition
conditions.SetSummary(obj,
@ -247,7 +252,7 @@ func (r *HelmChartReconciler) summarizeAndPatch(ctx context.Context, obj *source
}
recErr = kerrors.NewAggregate([]error{recErr, err})
}
return recErr
return result, recErr
}
// reconcile steps through the actual reconciliation tasks for the object, it returns early on the first step that

View File

@ -1372,7 +1372,8 @@ func TestHelmChartReconciler_summarizeAndPatch(t *testing.T) {
builder := fake.NewClientBuilder().WithScheme(testEnv.GetScheme())
r := &HelmChartReconciler{
Client: builder.Build(),
Client: builder.Build(),
EventRecorder: record.NewFakeRecorder(32),
}
obj := &sourcev1.HelmChart{
ObjectMeta: metav1.ObjectMeta{
@ -1393,7 +1394,7 @@ func TestHelmChartReconciler_summarizeAndPatch(t *testing.T) {
patchHelper, err := patch.NewHelper(obj, r.Client)
g.Expect(err).ToNot(HaveOccurred())
gotErr := r.summarizeAndPatch(ctx, obj, patchHelper, tt.result, tt.reconcileErr)
_, gotErr := r.summarizeAndPatch(ctx, obj, patchHelper, tt.result, tt.reconcileErr)
g.Expect(gotErr != nil).To(Equal(tt.wantErr))
g.Expect(obj.Status.Conditions).To(conditions.MatchConditions(tt.assertConditions))

View File

@ -132,21 +132,19 @@ func (r *HelmRepositoryReconciler) Reconcile(ctx context.Context, req ctrl.Reque
return ctrl.Result{}, nil
}
// Initialize the patch helper
// Initialize the patch helper with the current version of the object.
patchHelper, err := patch.NewHelper(obj, r.Client)
if err != nil {
return ctrl.Result{}, err
}
// Result of the sub-reconciliation.
// recResult stores the abstracted reconcile result.
var recResult sreconcile.Result
// Always attempt to patch the object after each reconciliation.
// NOTE: This deferred block only modifies the named return error. The
// result from the reconciliation remains the same. Any requeue attributes
// set in the result will continue to be effective.
// NOTE: The final runtime result and error are set in this block.
defer func() {
retErr = r.summarizeAndPatch(ctx, obj, patchHelper, recResult, retErr)
result, retErr = r.summarizeAndPatch(ctx, obj, patchHelper, recResult, retErr)
// Always record readiness and duration metrics
r.Metrics.RecordReadiness(ctx, obj)
@ -158,13 +156,13 @@ func (r *HelmRepositoryReconciler) Reconcile(ctx context.Context, req ctrl.Reque
if !controllerutil.ContainsFinalizer(obj, sourcev1.SourceFinalizer) {
controllerutil.AddFinalizer(obj, sourcev1.SourceFinalizer)
recResult = sreconcile.ResultRequeue
return ctrl.Result{Requeue: true}, nil
return
}
// Examine if the object is under deletion
if !obj.ObjectMeta.DeletionTimestamp.IsZero() {
res, err := r.reconcileDelete(ctx, obj)
return sreconcile.BuildRuntimeResult(ctx, r.EventRecorder, obj, res, err)
recResult, retErr = r.reconcileDelete(ctx, obj)
return
}
// Reconcile actual object
@ -173,15 +171,21 @@ func (r *HelmRepositoryReconciler) Reconcile(ctx context.Context, req ctrl.Reque
r.reconcileSource,
r.reconcileArtifact,
}
recResult, err = r.reconcile(ctx, obj, reconcilers)
return sreconcile.BuildRuntimeResult(ctx, r.EventRecorder, obj, recResult, err)
recResult, retErr = r.reconcile(ctx, obj, reconcilers)
return
}
// summarizeAndPatch analyzes the object conditions to create a summary of the
// status conditions and patches the object with the calculated summary. The
// reconciler error type is also used to determine the conditions and the
// returned error.
func (r *HelmRepositoryReconciler) summarizeAndPatch(ctx context.Context, obj *sourcev1.HelmRepository, patchHelper *patch.Helper, res sreconcile.Result, recErr error) error {
// status conditions, computes runtime results and patches the object in the K8s
// API server.
func (r *HelmRepositoryReconciler) summarizeAndPatch(
ctx context.Context,
obj *sourcev1.HelmRepository,
patchHelper *patch.Helper,
res sreconcile.Result,
recErr error) (ctrl.Result, error) {
sreconcile.RecordContextualError(ctx, r.EventRecorder, obj, recErr)
// Record the value of the reconciliation request, if any.
if v, ok := meta.ReconcileAnnotationValue(obj.GetAnnotations()); ok {
obj.Status.SetLastHandledReconcileRequest(v)
@ -189,7 +193,8 @@ func (r *HelmRepositoryReconciler) summarizeAndPatch(ctx context.Context, obj *s
// Compute the reconcile results, obtain patch options and reconcile error.
var patchOpts []patch.Option
patchOpts, recErr = sreconcile.ComputeReconcileResult(obj, res, recErr, helmRepoOwnedConditions)
var result ctrl.Result
patchOpts, result, recErr = sreconcile.ComputeReconcileResult(obj, obj.GetRequeueAfter(), res, recErr, helmRepoOwnedConditions)
// Summarize Ready condition.
conditions.SetSummary(obj,
@ -211,7 +216,7 @@ func (r *HelmRepositoryReconciler) summarizeAndPatch(ctx context.Context, obj *s
recErr = kerrors.NewAggregate([]error{recErr, err})
}
return recErr
return result, recErr
}
// reconcile iterates through the sub-reconcilers and processes the source

View File

@ -752,7 +752,8 @@ func TestHelmRepositoryReconciler_summarizeAndPatch(t *testing.T) {
builder := fakeclient.NewClientBuilder().WithScheme(testEnv.GetScheme())
r := &HelmRepositoryReconciler{
Client: builder.Build(),
Client: builder.Build(),
EventRecorder: record.NewFakeRecorder(32),
}
obj := &sourcev1.HelmRepository{
ObjectMeta: metav1.ObjectMeta{
@ -773,7 +774,7 @@ func TestHelmRepositoryReconciler_summarizeAndPatch(t *testing.T) {
patchHelper, err := patch.NewHelper(obj, r.Client)
g.Expect(err).ToNot(HaveOccurred())
gotErr := r.summarizeAndPatch(ctx, obj, patchHelper, tt.result, tt.reconcileErr)
_, gotErr := r.summarizeAndPatch(ctx, obj, patchHelper, tt.result, tt.reconcileErr)
g.Expect(gotErr != nil).To(Equal(tt.wantErr))
g.Expect(obj.Status.Conditions).To(conditions.MatchConditions(tt.assertConditions))

View File

@ -16,6 +16,8 @@ limitations under the License.
package error
import "time"
// Stalling is the reconciliation stalled state error. It contains an error
// and a reason for the stalled condition.
type Stalling struct {
@ -54,3 +56,24 @@ func (ee *Event) Error() string {
func (ee *Event) Unwrap() error {
return ee.Err
}
// Waiting is the reconciliation wait state error. It contains an error, wait
// duration and a reason for the wait.
type Waiting struct {
// RequeueAfter is the wait duration after which to requeue.
RequeueAfter time.Duration
// Reason is the reason for the wait.
Reason string
// Err is the error that caused the wait.
Err error
}
// Error implement error interface.
func (we *Waiting) Error() string {
return we.Err.Error()
}
// Unwrap returns the underlying error.
func (we *Waiting) Unwrap() error {
return we.Err
}

View File

@ -18,8 +18,10 @@ package reconcile
import (
"context"
"time"
corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/runtime"
kuberecorder "k8s.io/client-go/tools/record"
ctrl "sigs.k8s.io/controller-runtime"
@ -27,7 +29,6 @@ import (
"github.com/fluxcd/pkg/runtime/conditions"
"github.com/fluxcd/pkg/runtime/patch"
sourcev1 "github.com/fluxcd/source-controller/api/v1beta2"
serror "github.com/fluxcd/source-controller/internal/error"
)
@ -48,42 +49,51 @@ const (
// BuildRuntimeResult converts a given Result and error into the
// return values of a controller's Reconcile function.
func BuildRuntimeResult(ctx context.Context, recorder kuberecorder.EventRecorder, obj sourcev1.Source, rr Result, err error) (ctrl.Result, error) {
// NOTE: The return values can be modified based on the error type.
// For example, if an error signifies a short requeue period that's
// not equal to the requeue period of the object, the error can be checked
// and an appropriate result with the period can be returned.
//
// Example:
// if e, ok := err.(*waitError); ok {
// return ctrl.Result{RequeueAfter: e.RequeueAfter}, err
// }
// func BuildRuntimeResult(ctx context.Context, recorder kuberecorder.EventRecorder, obj sourcev1.Source, rr Result, err error) (ctrl.Result, error) {
func BuildRuntimeResult(successInterval time.Duration, rr Result, err error) ctrl.Result {
// Handle special errors that contribute to expressing the result.
if e, ok := err.(*serror.Waiting); ok {
return ctrl.Result{RequeueAfter: e.RequeueAfter}
}
// Log and record event based on the error.
switch rr {
case ResultRequeue:
return ctrl.Result{Requeue: true}
case ResultSuccess:
return ctrl.Result{RequeueAfter: successInterval}
default:
return ctrl.Result{}
}
}
// RecordContextualError records the contextual errors based on their types.
// An event is recorded for the errors that are returned to the runtime. The
// runtime handles the logging of the error.
// An event is recorded and an error is logged for errors that are known to be
// swallowed, not returned to the runtime.
func RecordContextualError(ctx context.Context, recorder kuberecorder.EventRecorder, obj runtime.Object, err error) {
switch e := err.(type) {
case *serror.Event:
recorder.Eventf(obj, corev1.EventTypeWarning, e.Reason, e.Error())
case *serror.Waiting:
// Waiting errors are not returned to the runtime. Log it explicitly.
ctrl.LoggerFrom(ctx).Info("reconciliation waiting", "reason", e.Err, "duration", e.RequeueAfter)
recorder.Event(obj, corev1.EventTypeNormal, e.Reason, e.Error())
case *serror.Stalling:
// Stalling errors are not returned to the runtime. Log it explicitly.
ctrl.LoggerFrom(ctx).Error(e, "reconciliation stalled")
recorder.Eventf(obj, corev1.EventTypeWarning, e.Reason, e.Error())
}
switch rr {
case ResultRequeue:
return ctrl.Result{Requeue: true}, err
case ResultSuccess:
return ctrl.Result{RequeueAfter: obj.GetRequeueAfter()}, err
default:
return ctrl.Result{}, err
}
}
// ComputeReconcileResult analyzes the reconcile results (result + error),
// updates the status conditions of the object with any corrections and returns
// result patch configuration and any error to the caller. The caller is
// responsible for using the patch option to patch the object in the API server.
func ComputeReconcileResult(obj conditions.Setter, res Result, recErr error, ownedConditions []string) ([]patch.Option, error) {
// object patch configuration, runtime result and runtime error. The caller is
// responsible for using the patch configuration to patch the object in the API
// server.
func ComputeReconcileResult(obj conditions.Setter, successInterval time.Duration, res Result, recErr error, ownedConditions []string) ([]patch.Option, ctrl.Result, error) {
result := BuildRuntimeResult(successInterval, res, recErr)
// Remove reconciling condition on successful reconciliation.
if recErr == nil && res == ResultSuccess {
conditions.Delete(obj, meta.ReconcilingCondition)
@ -105,10 +115,16 @@ func ComputeReconcileResult(obj conditions.Setter, res Result, recErr error, own
// requeuing.
pOpts = append(pOpts, patch.WithStatusObservedGeneration{})
conditions.MarkStalled(obj, t.Reason, t.Error())
return pOpts, nil
return pOpts, result, nil
}
// NOTE: Non-empty result with stalling error indicates that the
// returned result is incorrect.
case *serror.Waiting:
// The reconcile resulted in waiting error, remove stalled condition if
// present.
conditions.Delete(obj, meta.StalledCondition)
// The reconciler needs to wait and retry. Return no error.
return pOpts, result, nil
case nil:
// The reconcile didn't result in any error, we are not in stalled
// state. If a requeue is requested, the current generation has not been
@ -123,7 +139,7 @@ func ComputeReconcileResult(obj conditions.Setter, res Result, recErr error, own
conditions.Delete(obj, meta.StalledCondition)
}
return pOpts, recErr
return pOpts, result, recErr
}
// LowestRequeuingResult returns the ReconcileResult with the lowest requeue