Skip to content

Commit

Permalink
one more attempt
Browse files Browse the repository at this point in the history 
Signed-off-by: reggie-k <regina.voloshin@codefresh.io>
  • Loading branch information
@reggie-k
reggie-k committed Jan 20, 2025
1 parent 735ba6e commit 81f8975
Show file tree
Hide file tree
Showing 2 changed files with 135 additions and 281 deletions.
122 changes: 36 additions & 86 deletions applicationset/controllers/applicationset_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -529,6 +529,33 @@ func ignoreNotAllowedNamespaces(namespaces []string) predicate.Predicate {
})
}

// ignoreWhenAnnotationApplicationSetRefreshIsRemoved returns a predicate that ignores updates to ApplicationSet resources
// when the ApplicationSetRefresh annotation is removed
// First reconciliation is triggered when the annotation is added by [webhook.go#refreshApplicationSet]
// Using this predicate we avoid a second reconciliation triggered by the controller himself when the annotation is removed.
func ignoreWhenAnnotationApplicationSetRefreshIsRemoved() predicate.Predicate {
return predicate.Funcs{
UpdateFunc: func(e event.UpdateEvent) bool {
oldAppset, isAppSet := e.ObjectOld.(*argov1alpha1.ApplicationSet)
if !isAppSet {
return false
}
newAppset, isAppSet := e.ObjectNew.(*argov1alpha1.ApplicationSet)
if !isAppSet {
return false
}

_, oldHasRefreshAnnotation := oldAppset.Annotations[common.AnnotationApplicationSetRefresh]
_, newHasRefreshAnnotation := newAppset.Annotations[common.AnnotationApplicationSetRefresh]

if oldHasRefreshAnnotation && !newHasRefreshAnnotation {
return false
}
return true
},
}
}

func appControllerIndexer(rawObj client.Object) []string {
// grab the job object, extract the owner...
app := rawObj.(*argov1alpha1.Application)
Expand All @@ -550,20 +577,20 @@ func (r *ApplicationSetReconciler) SetupWithManager(mgr ctrl.Manager, enableProg
return fmt.Errorf("error setting up with manager: %w", err)
}

appOwnsHandler := getApplicationOwnsHandler(enableProgressiveSyncs)
appSetOwnsHandler := getApplicationSetOwnsHandler()
ownsHandler := getOwnsHandlerPredicates(enableProgressiveSyncs)

return ctrl.NewControllerManagedBy(mgr).WithOptions(controller.Options{
MaxConcurrentReconciles: maxConcurrentReconciliations,
}).For(&argov1alpha1.ApplicationSet{}, builder.WithPredicates(appSetOwnsHandler)).
Owns(&argov1alpha1.Application{}, builder.WithPredicates(appOwnsHandler)).
}).For(&argov1alpha1.ApplicationSet{}, builder.WithPredicates(ignoreWhenAnnotationApplicationSetRefreshIsRemoved())).
Owns(&argov1alpha1.Application{}, builder.WithPredicates(ownsHandler)).
WithEventFilter(ignoreNotAllowedNamespaces(r.ApplicationSetNamespaces)).
Watches(
&corev1.Secret{},
&clusterSecretEventHandler{
Client: mgr.GetClient(),
Log: log.WithField("type", "createSecretEventHandler"),
}).
// TODO: also watch Applications and respond on changes if we own them.
Complete(r)
}

Expand Down Expand Up @@ -1457,7 +1484,7 @@ func syncApplication(application argov1alpha1.Application, prune bool) argov1alp
return application
}

func getApplicationOwnsHandler(enableProgressiveSyncs bool) predicate.Funcs {
func getOwnsHandlerPredicates(enableProgressiveSyncs bool) predicate.Funcs {
return predicate.Funcs{
CreateFunc: func(e event.CreateEvent) bool {
// if we are the owner and there is a create event, we most likely created it and do not need to
Expand Down Expand Up @@ -1494,8 +1521,8 @@ func getApplicationOwnsHandler(enableProgressiveSyncs bool) predicate.Funcs {
if !isApp {
return false
}
requeue := shouldRequeueForApplication(appOld, appNew, enableProgressiveSyncs)
logCtx.WithField("requeue", requeue).Debugf("requeue caused by application %s", appNew.Name)
requeue := shouldRequeueApplicationSet(appOld, appNew, enableProgressiveSyncs)
logCtx.WithField("requeue", requeue).Debugf("requeue: %t caused by application %s", requeue, appNew.Name)
return requeue
},
GenericFunc: func(e event.GenericEvent) bool {
Expand All @@ -1512,13 +1539,13 @@ func getApplicationOwnsHandler(enableProgressiveSyncs bool) predicate.Funcs {
}
}

// shouldRequeueForApplication determines when we want to requeue an ApplicationSet for reconciling based on an owned
// shouldRequeueApplicationSet determines when we want to requeue an ApplicationSet for reconciling based on an owned
// application change
// The applicationset controller owns a subset of the Application CR.
// We do not need to re-reconcile if parts of the application change outside the applicationset's control.
// An example being, Application.ApplicationStatus.ReconciledAt which gets updated by the application controller.
// Additionally, Application.ObjectMeta.ResourceVersion and Application.ObjectMeta.Generation which are set by K8s.
func shouldRequeueForApplication(appOld *argov1alpha1.Application, appNew *argov1alpha1.Application, enableProgressiveSyncs bool) bool {
func shouldRequeueApplicationSet(appOld *argov1alpha1.Application, appNew *argov1alpha1.Application, enableProgressiveSyncs bool) bool {
if appOld == nil || appNew == nil {
return false
}
Expand Down Expand Up @@ -1551,81 +1578,4 @@ func shouldRequeueForApplication(appOld *argov1alpha1.Application, appNew *argov
return false
}

func getApplicationSetOwnsHandler() predicate.Funcs {
return predicate.Funcs{
CreateFunc: func(e event.CreateEvent) bool {
appSet, isApp := e.Object.(*argov1alpha1.ApplicationSet)
if !isApp {
return false
}
log.WithField("applicationset", appSet.QualifiedName()).Debugln("received create event")
// Always queue a new applicationset
return true
},
DeleteFunc: func(e event.DeleteEvent) bool {
appSet, isApp := e.Object.(*argov1alpha1.ApplicationSet)
if !isApp {
return false
}
log.WithField("applicationset", appSet.QualifiedName()).Debugln("received delete event")
// Always queue for the deletion of an applicationset
return true
},
UpdateFunc: func(e event.UpdateEvent) bool {
appSetOld, isAppSet := e.ObjectOld.(*argov1alpha1.ApplicationSet)
if !isAppSet {
return false
}
appSetNew, isAppSet := e.ObjectNew.(*argov1alpha1.ApplicationSet)
if !isAppSet {
return false
}
requeue := shouldRequeueForApplicationSet(appSetOld, appSetNew)
log.WithField("applicationset", appSetNew.QualifiedName()).
WithField("requeue", requeue).Debugln("received update event")
return requeue
},
GenericFunc: func(e event.GenericEvent) bool {
appSet, isApp := e.Object.(*argov1alpha1.ApplicationSet)
if !isApp {
return false
}
log.WithField("applicationset", appSet.QualifiedName()).Debugln("received generic event")
// Always queue for the generic of an applicationset
return true
},
}
}

// shouldRequeueForApplicationSet determines when we need to requeue an applicationset
func shouldRequeueForApplicationSet(appSetOld, appSetNew *argov1alpha1.ApplicationSet) bool {
if appSetOld == nil || appSetNew == nil {
return false
}
// only compare the applicationset spec, annotations, labels and finalizers, specifically avoiding
// the status field. status is owned by the applicationset controller,
// and we do not need to requeue when it does bookkeeping
// NB: the ApplicationDestination comes from the ApplicationSpec being embedded
// in the ApplicationSetTemplate from the generators
if !cmp.Equal(appSetOld.Spec, appSetNew.Spec, cmpopts.EquateEmpty(), cmpopts.EquateComparable(argov1alpha1.ApplicationDestination{})) ||
!cmp.Equal(appSetOld.ObjectMeta.GetLabels(), appSetNew.ObjectMeta.GetLabels(), cmpopts.EquateEmpty()) ||
!cmp.Equal(appSetOld.ObjectMeta.GetFinalizers(), appSetNew.ObjectMeta.GetFinalizers(), cmpopts.EquateEmpty()) {
return true
}

// Requeue only when the refresh annotation is newly added to the ApplicationSet.
// Changes to other annotations made simultaneously might be missed, but such cases are rare.
if !cmp.Equal(appSetOld.ObjectMeta.GetAnnotations(), appSetNew.ObjectMeta.GetAnnotations(), cmpopts.EquateEmpty()) {
_, oldHasRefreshAnnotation := appSetOld.Annotations[common.AnnotationApplicationSetRefresh]
_, newHasRefreshAnnotation := appSetNew.Annotations[common.AnnotationApplicationSetRefresh]

if oldHasRefreshAnnotation && !newHasRefreshAnnotation {
return false
}
return true
}

return false
}

var _ handler.EventHandler = &clusterSecretEventHandler{}
Loading

0 comments on commit 81f8975

Please sign in to comment.