Skip to content

Commit

Permalink
Merge pull request #121 from appuio/reconcile-fixes
Browse files Browse the repository at this point in the history
Reconcile even if no `EmailSent` condition can be found
  • Loading branch information
bastjan authored Mar 8, 2023
2 parents 081c78c + a9584ef commit 9587b52
Show file tree
Hide file tree
Showing 3 changed files with 66 additions and 63 deletions.
16 changes: 8 additions & 8 deletions controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ func ControllerCommand() *cobra.Command {
invEmailBackend := cmd.Flags().String("email-backend", "stdout", "Backend to use for sending invitation mails (one of stdout, mailgun)")
invEmailSender := cmd.Flags().String("email-sender", "[email protected]", "Sender address for invitation mails")
invEmailSubject := cmd.Flags().String("email-subject", "You have been invited to APPUiO Cloud", "Subject for invitation mails")
invEmailRetryInterval := cmd.Flags().Duration("email-retry-interval", 5*time.Minute, "Retry interval for sending e-mail messages")
invEmailBaseRetryDelay := cmd.Flags().Duration("email-base-retry-interval", 15*time.Second, "Retry interval for sending e-mail messages. There is also an exponential back-off applied by the controller.")

invEmailMailgunToken := cmd.Flags().String("mailgun-token", "CHANGEME", "Token used to access Mailgun API")
invEmailMailgunDomain := cmd.Flags().String("mailgun-domain", "example.com", "Mailgun Domain to use")
Expand Down Expand Up @@ -108,7 +108,7 @@ func ControllerCommand() *cobra.Command {
*beRefreshJitter,
*invTokenValidFor,
*redeemedInvitationTTL,
*invEmailRetryInterval,
*invEmailBaseRetryDelay,
mailSender,
ctrl.Options{
Scheme: scheme,
Expand Down Expand Up @@ -142,7 +142,7 @@ func setupManager(
beRefreshJitter,
invTokenValidFor time.Duration,
redeemedInvitationTTL time.Duration,
invEmailRetryInterval time.Duration,
invEmailBaseRetryDelay time.Duration,
mailSender mailsenders.MailSender,
opt ctrl.Options,
) (ctrl.Manager, error) {
Expand Down Expand Up @@ -219,11 +219,11 @@ func setupManager(
return nil, err
}
invmail := &controllers.InvitationEmailReconciler{
Client: mgr.GetClient(),
Scheme: mgr.GetScheme(),
Recorder: mgr.GetEventRecorderFor("invitation-cleanup-controller"),
RetryInterval: invEmailRetryInterval,
MailSender: mailSender,
Client: mgr.GetClient(),
Scheme: mgr.GetScheme(),
Recorder: mgr.GetEventRecorderFor("invitation-email-controller"),
BaseRetryDelay: invEmailBaseRetryDelay,
MailSender: mailSender,
}
if err = invmail.SetupWithManager(mgr); err != nil {
return nil, err
Expand Down
47 changes: 28 additions & 19 deletions controllers/invitation_email_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,28 +5,34 @@ import (
"fmt"
"time"

"go.uber.org/multierr"
"golang.org/x/time/rate"
apimeta "k8s.io/apimachinery/pkg/api/meta"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/client-go/tools/record"
"k8s.io/client-go/util/workqueue"
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/controller"
"sigs.k8s.io/controller-runtime/pkg/log"

"github.com/appuio/control-api/mailsenders"

userv1 "github.com/appuio/control-api/apis/user/v1"
)

const ReasonSendFailed = "SendFailed"

// InvitationEmailReconciler reconciles invitations and sends invitation emails if appropriate
type InvitationEmailReconciler struct {
client.Client

Recorder record.EventRecorder
Scheme *runtime.Scheme

MailSender mailsenders.MailSender
RetryInterval time.Duration
MailSender mailsenders.MailSender
BaseRetryDelay time.Duration
}

//+kubebuilder:rbac:groups="rbac.appuio.io",resources=invitations,verbs=get;list;watch
Expand All @@ -48,35 +54,26 @@ func (r *InvitationEmailReconciler) Reconcile(ctx context.Context, req ctrl.Requ
return ctrl.Result{}, nil
}

if inv.Status.Token == "" {
if inv.Status.Token == "" || inv.Spec.Email == "" {
return ctrl.Result{}, nil
}

if apimeta.IsStatusConditionTrue(inv.Status.Conditions, userv1.ConditionEmailSent) {
return ctrl.Result{}, nil
}
condition := apimeta.FindStatusCondition(inv.Status.Conditions, userv1.ConditionEmailSent)

if condition == nil {
return ctrl.Result{}, nil
}

email := inv.Spec.Email
id, err := r.MailSender.Send(ctx, email, inv.Name, inv.Status.Token)

if err != nil {
log.V(0).Error(err, "Error in e-mail backend")

// Only update status if the error changes
if condition.Reason != err.Error() {
apimeta.SetStatusCondition(&inv.Status.Conditions, metav1.Condition{
Type: userv1.ConditionEmailSent,
Status: metav1.ConditionFalse,
Reason: err.Error(),
})
return ctrl.Result{}, r.Client.Status().Update(ctx, &inv)
}
return ctrl.Result{RequeueAfter: r.RetryInterval}, nil
apimeta.SetStatusCondition(&inv.Status.Conditions, metav1.Condition{
Type: userv1.ConditionEmailSent,
Status: metav1.ConditionFalse,
Reason: ReasonSendFailed,
Message: err.Error(),
})
return ctrl.Result{}, multierr.Append(err, r.Client.Status().Update(ctx, &inv))
}

var message string
Expand All @@ -96,5 +93,17 @@ func (r *InvitationEmailReconciler) Reconcile(ctx context.Context, req ctrl.Requ
func (r *InvitationEmailReconciler) SetupWithManager(mgr ctrl.Manager) error {
return ctrl.NewControllerManagedBy(mgr).
For(&userv1.Invitation{}).
WithOptions(controller.Options{
RateLimiter: r.rateLimiter(),
}).
Complete(r)
}

func (r *InvitationEmailReconciler) rateLimiter() workqueue.RateLimiter {
// This is the default rate limiter for controllers with higher baseDelay if the reconciliation fails.
return workqueue.NewMaxOfRateLimiter(
workqueue.NewItemExponentialFailureRateLimiter(r.BaseRetryDelay, 5*time.Minute),
// 10 qps, 100 bucket size. This is only for retry speed and its only the overall factor (not per item)
&workqueue.BucketRateLimiter{Limiter: rate.NewLimiter(rate.Limit(10), 100)},
)
}
66 changes: 30 additions & 36 deletions controllers/invitation_email_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,14 +30,7 @@ func (s *SenderWithConstantId) Send(context.Context, string, string, string) (st
func Test_InvitationEmailReconciler_Reconcile_Success(t *testing.T) {
ctx := context.Background()

subject := &userv1.Invitation{
ObjectMeta: metav1.ObjectMeta{
Name: "subject",
},
Status: userv1.InvitationStatus{
Token: "abc",
},
}
subject := baseInvitation()
apimeta.SetStatusCondition(&subject.Status.Conditions, metav1.Condition{
Type: userv1.ConditionEmailSent,
Status: metav1.ConditionFalse,
Expand All @@ -58,14 +51,7 @@ func Test_InvitationEmailReconciler_Reconcile_Success(t *testing.T) {
func Test_InvitationEmailReconciler_Reconcile_WithSendingFailure_Success(t *testing.T) {
ctx := context.Background()

subject := &userv1.Invitation{
ObjectMeta: metav1.ObjectMeta{
Name: "subject",
},
Status: userv1.InvitationStatus{
Token: "abc",
},
}
subject := baseInvitation()
apimeta.SetStatusCondition(&subject.Status.Conditions, metav1.Condition{
Type: userv1.ConditionEmailSent,
Status: metav1.ConditionFalse,
Expand All @@ -75,26 +61,20 @@ func Test_InvitationEmailReconciler_Reconcile_WithSendingFailure_Success(t *test

r := invitationEmailReconcilerWithFailingSender(c)
_, err := r.Reconcile(ctx, requestFor(subject))
require.NoError(t, err)
require.Error(t, err)

require.NoError(t, c.Get(ctx, client.ObjectKeyFromObject(subject), subject))
require.False(t, apimeta.IsStatusConditionTrue(subject.Status.Conditions, userv1.ConditionEmailSent))
condition := apimeta.FindStatusCondition(subject.Status.Conditions, userv1.ConditionEmailSent)
require.NotNil(t, condition)
require.Equal(t, "Err0r", condition.Reason)
require.Equal(t, ReasonSendFailed, condition.Reason)
}

func Test_InvitationEmailReconciler_Reconcile_NoEmail_Success(t *testing.T) {
ctx := context.Background()

subject := &userv1.Invitation{
ObjectMeta: metav1.ObjectMeta{
Name: "subject",
},
Status: userv1.InvitationStatus{
Token: "abc",
},
}
subject := baseInvitation()
subject.Spec.Email = ""

c := prepareTest(t, subject)

Expand All @@ -108,20 +88,34 @@ func Test_InvitationEmailReconciler_Reconcile_NoEmail_Success(t *testing.T) {

func invitationEmailReconciler(c client.WithWatch) *InvitationEmailReconciler {
return &InvitationEmailReconciler{
Client: c,
Scheme: c.Scheme(),
Recorder: record.NewFakeRecorder(3),
MailSender: &SenderWithConstantId{},
RetryInterval: time.Minute,
Client: c,
Scheme: c.Scheme(),
Recorder: record.NewFakeRecorder(3),
MailSender: &SenderWithConstantId{},
BaseRetryDelay: time.Minute,
}
}

func invitationEmailReconcilerWithFailingSender(c client.WithWatch) *InvitationEmailReconciler {
return &InvitationEmailReconciler{
Client: c,
Scheme: c.Scheme(),
Recorder: record.NewFakeRecorder(3),
MailSender: &FailingSender{},
RetryInterval: time.Minute,
Client: c,
Scheme: c.Scheme(),
Recorder: record.NewFakeRecorder(3),
MailSender: &FailingSender{},
BaseRetryDelay: time.Minute,
}
}

func baseInvitation() *userv1.Invitation {
return &userv1.Invitation{
ObjectMeta: metav1.ObjectMeta{
Name: "subject",
},
Spec: userv1.InvitationSpec{
Email: "[email protected]",
},
Status: userv1.InvitationStatus{
Token: "abc",
},
}
}

0 comments on commit 9587b52

Please sign in to comment.