Skip to content

Commit

Permalink
fix potential infinite loop
Browse files Browse the repository at this point in the history
  • Loading branch information
sbryzak committed May 2, 2024
1 parent c7df220 commit 087cdcb
Show file tree
Hide file tree
Showing 2 changed files with 72 additions and 9 deletions.
20 changes: 12 additions & 8 deletions controllers/deactivation/deactivation_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,8 @@ func (r *Reconciler) SetupWithManager(mgr manager.Manager) error {
Named("deactivation").
For(&toolchainv1alpha1.MasterUserRecord{}, builder.WithPredicates(CreateAndUpdateOnlyPredicate{})).
Watches(&source.Kind{Type: &toolchainv1alpha1.UserSignup{}},
handler.EnqueueRequestsFromMapFunc(MapUserSignupToMasterUserRecord())).
handler.EnqueueRequestsFromMapFunc(MapUserSignupToMasterUserRecord()),
builder.WithPredicates(GenerationOrConditionsChangedPredicate{})).
Complete(r)
}

Expand Down Expand Up @@ -227,14 +228,17 @@ func (r *Reconciler) Reconcile(ctx context.Context, request ctrl.Request) (ctrl.
deactivatingCondition.Reason != toolchainv1alpha1.UserSignupDeactivatingNotificationCRCreatedReason {
// If the UserSignup has been marked as deactivating, however the deactivating notification hasn't been
// created yet, then set the deactivation timestamp to a temporary value, calculated as being the current
// timestamp plus the number of pre-deactivation days configured in the settings

// timestamp plus the number of pre-deactivation days configured in the settings, BUT only if it is currently nil
// OR the calculated value is more than 24 hours later than the current value
deactivationDueTime := time.Now().Add(time.Duration(deactivatingNotificationDays) * 24 * time.Hour)
ts := v1.NewTime(deactivationDueTime)
usersignup.Status.ScheduledDeactivationTimestamp = &ts
if err := r.Client.Status().Update(ctx, usersignup); err != nil {
logger.Error(err, "failed to update usersignup status")
return reconcile.Result{}, err
if usersignup.Status.ScheduledDeactivationTimestamp == nil ||
usersignup.Status.ScheduledDeactivationTimestamp.Time.Before(deactivationDueTime.Add(-24*time.Hour)) {
ts := v1.NewTime(deactivationDueTime)
usersignup.Status.ScheduledDeactivationTimestamp = &ts
if err := r.Client.Status().Update(ctx, usersignup); err != nil {
logger.Error(err, "failed to update usersignup status")
return reconcile.Result{}, err
}
}

return reconcile.Result{}, nil
Expand Down
61 changes: 60 additions & 1 deletion controllers/deactivation/predicate.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
package deactivation

import "sigs.k8s.io/controller-runtime/pkg/event"
import (
toolchainv1alpha1 "github.com/codeready-toolchain/api/api/v1alpha1"
"sigs.k8s.io/controller-runtime/pkg/event"
"sigs.k8s.io/controller-runtime/pkg/predicate"
)

// CreateAndUpdateOnlyPredicate will filter out all events except Create and Update
type CreateAndUpdateOnlyPredicate struct {
Expand All @@ -25,3 +29,58 @@ func (CreateAndUpdateOnlyPredicate) Delete(_ event.DeleteEvent) bool {
func (CreateAndUpdateOnlyPredicate) Generic(_ event.GenericEvent) bool {
return false
}

type GenerationOrConditionsChangedPredicate struct {
predicate.GenerationChangedPredicate
}

// Update implements default UpdateEvent filter for validating no generation change
func (GenerationOrConditionsChangedPredicate) Update(e event.UpdateEvent) bool {
if e.ObjectOld == nil || e.ObjectNew == nil {
return false
}

if e.ObjectNew.GetGeneration() != e.ObjectOld.GetGeneration() {
return true
}

switch objNew := e.ObjectNew.(type) {
case *toolchainv1alpha1.UserSignup:
switch objOld := e.ObjectOld.(type) {
case *toolchainv1alpha1.UserSignup:
if !ConditionsMatch(objOld.Status.Conditions, objNew.Status.Conditions) {
return true
}
}
}

return false
}

func ConditionsMatch(first, second []toolchainv1alpha1.Condition) bool {
if len(first) != len(second) {
return false
}
for _, c := range first {
if !ContainsCondition(second, c) {
return false
}
}
for _, c := range second {
if !ContainsCondition(first, c) {
return false
}
}
return true
}

// ContainsCondition returns true if the specified list of conditions contains the specified condition.
// LastTransitionTime is ignored.
func ContainsCondition(conditions []toolchainv1alpha1.Condition, contains toolchainv1alpha1.Condition) bool {
for _, c := range conditions {
if c.Type == contains.Type {
return contains.Status == c.Status && contains.Reason == c.Reason && contains.Message == c.Message
}
}
return false
}

0 comments on commit 087cdcb

Please sign in to comment.