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.

Sign up for GitHub

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

Support for wrapped RequeueAfterError/interface #1020

Merged
merged 1 commit into from
Jun 18, 2019
Merged

Support for wrapped RequeueAfterError/interface #1020

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
14 changes: 7 additions & 7 deletions pkg/controller/cluster/cluster_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,8 @@ package cluster
import (
"context"

"k8s.io/apimachinery/pkg/api/errors"
"github.com/pkg/errors"
apierrors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/klog"
Expand Down Expand Up @@ -77,7 +78,7 @@ func (r *ReconcileCluster) Reconcile(request reconcile.Request) (reconcile.Resul
cluster := &clusterv1alpha1.Cluster{}
err := r.Get(context.Background(), request.NamespacedName, cluster)
if err != nil {
if errors.IsNotFound(err) {
if apierrors.IsNotFound(err) {
// Object not found, return. Created objects are automatically garbage collected.
// For additional cleanup logic use finalizers.
return reconcile.Result{}, nil
Expand Down Expand Up @@ -137,11 +138,10 @@ func (r *ReconcileCluster) Reconcile(request reconcile.Request) (reconcile.Resul
}

klog.Infof("reconciling cluster object %v triggers idempotent reconcile.", name)
err = r.actuator.Reconcile(cluster)
if err != nil {
if requeueErr, ok := err.(*controllerError.RequeueAfterError); ok {
klog.Infof("Actuator returned requeue after error: %v", requeueErr)
return reconcile.Result{Requeue: true, RequeueAfter: requeueErr.RequeueAfter}, nil
if err := r.actuator.Reconcile(cluster); err != nil {
if requeueErr, ok := errors.Cause(err).(controllerError.HasRequeueAfterError); ok {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For others that might not know: from the docs of errors.Cause

If the error does not implement Cause, the original error will be returned. If the error is nil, nil will be returned without further investigation.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @vincepri,

Thank you. I actually had to go look up Cause to see what it did and found the same. The fact that it handles nil and even returns the original error made this very efficient to implement.

Copy link
Contributor Author

@akutz akutz Jun 12, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Technically lines 141 and 142 (and the others like them) could be combined, but I think the way it is is more readable.

klog.Infof("Actuator returned requeue-after error: %v", requeueErr)
return reconcile.Result{Requeue: true, RequeueAfter: requeueErr.GetRequeueAfter()}, nil
}
klog.Errorf("Error reconciling cluster object %v; %v", name, err)
return reconcile.Result{}, err
Expand Down
15 changes: 15 additions & 0 deletions pkg/controller/error/requeue_error.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,15 @@ import (
"time"
)

// HasRequeueAfterError represents that an actuator managed object should
// be requeued for further processing after the given RequeueAfter time has
// passed.
type HasRequeueAfterError interface {
// GetRequeueAfter gets the duration to wait until the managed object is
// requeued for further processing.
GetRequeueAfter() time.Duration
}

// RequeueAfterError represents that an actuator managed object should be
// requeued for further processing after the given RequeueAfter time has
// passed.
Expand All @@ -32,3 +41,9 @@ type RequeueAfterError struct {
func (e *RequeueAfterError) Error() string {
return fmt.Sprintf("requeue in: %s", e.RequeueAfter)
}

// GetRequeueAfter gets the duration to wait until the managed object is
// requeued for further processing.
func (e *RequeueAfterError) GetRequeueAfter() time.Duration {
return e.RequeueAfter
}
13 changes: 7 additions & 6 deletions pkg/controller/machine/machine_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"context"
"os"

"github.com/pkg/errors"
corev1 "k8s.io/api/core/v1"
apierrors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
Expand Down Expand Up @@ -169,9 +170,9 @@ func (r *ReconcileMachine) Reconcile(request reconcile.Request) (reconcile.Resul

klog.Infof("Reconciling machine %q triggers delete", name)
if err := r.actuator.Delete(ctx, cluster, m); err != nil {
if requeueErr, ok := err.(*controllerError.RequeueAfterError); ok {
if requeueErr, ok := errors.Cause(err).(controllerError.HasRequeueAfterError); ok {
klog.Infof("Actuator returned requeue-after error: %v", requeueErr)
return reconcile.Result{Requeue: true, RequeueAfter: requeueErr.RequeueAfter}, nil
return reconcile.Result{Requeue: true, RequeueAfter: requeueErr.GetRequeueAfter()}, nil
}

klog.Errorf("Failed to delete machine %q: %v", name, err)
Expand Down Expand Up @@ -206,9 +207,9 @@ func (r *ReconcileMachine) Reconcile(request reconcile.Request) (reconcile.Resul
if exist {
klog.Infof("Reconciling machine %q triggers idempotent update", name)
if err := r.actuator.Update(ctx, cluster, m); err != nil {
if requeueErr, ok := err.(*controllerError.RequeueAfterError); ok {
if requeueErr, ok := errors.Cause(err).(controllerError.HasRequeueAfterError); ok {
klog.Infof("Actuator returned requeue-after error: %v", requeueErr)
return reconcile.Result{Requeue: true, RequeueAfter: requeueErr.RequeueAfter}, nil
return reconcile.Result{Requeue: true, RequeueAfter: requeueErr.GetRequeueAfter()}, nil
}

klog.Errorf(`Error updating machine "%s/%s": %v`, m.Namespace, name, err)
Expand All @@ -221,9 +222,9 @@ func (r *ReconcileMachine) Reconcile(request reconcile.Request) (reconcile.Resul
// Machine resource created. Machine does not yet exist.
klog.Infof("Reconciling machine object %v triggers idempotent create.", m.ObjectMeta.Name)
if err := r.actuator.Create(ctx, cluster, m); err != nil {
if requeueErr, ok := err.(*controllerError.RequeueAfterError); ok {
if requeueErr, ok := errors.Cause(err).(controllerError.HasRequeueAfterError); ok {
klog.Infof("Actuator returned requeue-after error: %v", requeueErr)
return reconcile.Result{Requeue: true, RequeueAfter: requeueErr.RequeueAfter}, nil
return reconcile.Result{Requeue: true, RequeueAfter: requeueErr.GetRequeueAfter()}, nil
}

klog.Warningf("Failed to create machine %q: %v", name, err)
Expand Down