Skip to content
This repository has been archived by the owner on Oct 12, 2023. It is now read-only.

Commit

Permalink
Improve controller error handling and logging.
Browse files Browse the repository at this point in the history
Controller was ignoring errors creating/updating/deleting Applications
when it should be returning that error so the object is requeued with
backoff.

Fixed some inaccurate or messy logging.
  • Loading branch information
dgoodwin committed Sep 4, 2020
1 parent 47c3908 commit 77d522b
Showing 1 changed file with 18 additions and 7 deletions.
25 changes: 18 additions & 7 deletions pkg/controllers/applicationset_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,9 @@ func (r *ApplicationSetReconciler) Reconcile(req ctrl.Request) (ctrl.Result, err

var applicationSetInfo argoprojiov1alpha1.ApplicationSet
if err := r.Get(ctx, req.NamespacedName, &applicationSetInfo); err != nil {
log.Infof("Unable to fetch applicationSetInfo %e", err)
if client.IgnoreNotFound(err) != nil {
log.WithField("request", req).WithError(err).Infof("unable to get ApplicationSet")
}
return ctrl.Result{}, client.IgnoreNotFound(err)
}

Expand All @@ -74,13 +76,20 @@ func (r *ApplicationSetReconciler) Reconcile(req ctrl.Request) (ctrl.Result, err
log.Infof("apps from generator: %+v", apps)
if err != nil {
log.WithError(err).Error("error generating applications")
return ctrl.Result{}, err
}
desiredApplications = append(desiredApplications, apps...)

}

r.createOrUpdateInCluster(ctx, applicationSetInfo, desiredApplications)
r.deleteInCluster(ctx, applicationSetInfo, desiredApplications)
err := r.createOrUpdateInCluster(ctx, applicationSetInfo, desiredApplications)
if err != nil {
return ctrl.Result{}, err
}
err = r.deleteInCluster(ctx, applicationSetInfo, desiredApplications)
if err != nil {
return ctrl.Result{}, err
}

return ctrl.Result{}, nil
}
Expand Down Expand Up @@ -121,7 +130,7 @@ func (r *ApplicationSetReconciler) SetupWithManager(mgr ctrl.Manager) error {
// For new application it will call create
// For application that need to update it will call update
// The function also adds owner reference to all applications, and uses it for delete them.
func (r *ApplicationSetReconciler) createOrUpdateInCluster(ctx context.Context, applicationSet argoprojiov1alpha1.ApplicationSet, desiredApplications []argov1alpha1.Application) {
func (r *ApplicationSetReconciler) createOrUpdateInCluster(ctx context.Context, applicationSet argoprojiov1alpha1.ApplicationSet, desiredApplications []argov1alpha1.Application) error {

//create or updates the application in appList
for _, app := range desiredApplications {
Expand All @@ -135,18 +144,19 @@ func (r *ApplicationSetReconciler) createOrUpdateInCluster(ctx context.Context,
})

if err != nil {
appLog.WithError(err).Errorf("failed to %s Application", action)
appLog.WithError(err).WithField("action", action).Error("failed to create/update Application")
continue
}

r.Recorder.Eventf(&applicationSet, core.EventTypeNormal, fmt.Sprint(action), "%s Application %q", action, app.Name)
appLog.Logf(log.InfoLevel, "%s Application", action)
}
return nil
}

// deleteInCluster will delete application that are current in the cluster but not in appList.
// The function must be called after all generators had been called and generated applications
func (r *ApplicationSetReconciler) deleteInCluster(ctx context.Context, applicationSet argoprojiov1alpha1.ApplicationSet, desiredApplications []argov1alpha1.Application) {
func (r *ApplicationSetReconciler) deleteInCluster(ctx context.Context, applicationSet argoprojiov1alpha1.ApplicationSet, desiredApplications []argov1alpha1.Application) error {

// Save current applications to be able to delete the ones that are not in appList
var current argov1alpha1.ApplicationList
Expand All @@ -167,12 +177,13 @@ func (r *ApplicationSetReconciler) deleteInCluster(ctx context.Context, applicat
err := r.Client.Delete(ctx, &app)
if err != nil {
appLog.WithError(err).Error("failed to delete Application")
continue
return err
}
r.Recorder.Eventf(&applicationSet, core.EventTypeNormal, "Deleted", "Deleted Application %q", app.Name)
appLog.Log(log.InfoLevel, "Deleted application")
}
}
return nil
}

var _ handler.EventHandler = &clusterSecretEventHandler{}

0 comments on commit 77d522b

Please sign in to comment.