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

Jump to bottom

re-adjust finalizer/status update call order #692

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

re-adjust finalizer/status update call order #692

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
22 changes: 12 additions & 10 deletions controllers/dspipeline_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -188,8 +188,6 @@ func (r *DSPAReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.

dspaStatus := dspastatus.NewDSPAStatus(dspa)

defer r.updateStatus(ctx, dspa, dspaStatus, log, req)

// FixMe: Hack for stubbing gvk during tests as these are not populated by test suite
// https://github.com/opendatahub-io/data-science-pipelines-operator/pull/7#discussion_r1102887037
// In production we expect these to be populated
Expand All @@ -199,25 +197,27 @@ func (r *DSPAReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.
dspa.APIVersion, dspa.Kind = gvk.Version, gvk.Kind
}

updateFinalizer := func() {
if err1 := r.Update(ctx, dspa); err1 != nil {
log.Info(fmt.Sprintf("Encountered error when updating finalizer %s, error: %v", finalizerName, err1))
}
}

// Ensure that empty values do not overwrite desired state
if dspa.ObjectMeta.DeletionTimestamp.IsZero() {
if !controllerutil.ContainsFinalizer(dspa, finalizerName) {
controllerutil.AddFinalizer(dspa, finalizerName)
if err := r.Update(ctx, dspa); err != nil {
return ctrl.Result{}, err
}
defer updateFinalizer()
}
} else {
if controllerutil.ContainsFinalizer(dspa, finalizerName) {
params.Name = dspa.Name
params.Namespace = dspa.Namespace
if err := r.cleanUpResources(params); err != nil {
return ctrl.Result{}, err
if err1 := r.cleanUpResources(params); err1 != nil {
return ctrl.Result{}, err1
}
controllerutil.RemoveFinalizer(dspa, finalizerName)
if err := r.Update(ctx, dspa); err != nil {
return ctrl.Result{}, err
}
defer updateFinalizer()
}

// Stop reconciliation as the item is being deleted
Expand Down Expand Up @@ -341,6 +341,8 @@ func (r *DSPAReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.
return ctrl.Result{Requeue: true, RequeueAfter: requeueTime}, nil
}

r.updateStatus(ctx, dspa, dspaStatus, log, req)

return ctrl.Result{}, nil
}

Expand Down
Loading