Skip to content

Commit ec8c311

Browse files
shivamerlashivamerla
committed
Fix to handle CR cleanup with foreground propagation
* Handle custom cleanup irrespective of finalizer is set or not * Remove custom finalizer if present * Avoid further reconciliation when the object is being deleted Signed-off-by: Shiva Krishna, Merla <smerla@nvidia.com>
1 parent 828145b commit ec8c311

8 files changed

+65
-60
lines changed

‎internal/controller/nemo_datastore_controller.go

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -132,23 +132,23 @@ func (r *NemoDatastoreReconciler) Reconcile(ctx context.Context, req ctrl.Reques
132132
}
133133
} else {
134134
// The instance is being deleted
135+
// Perform platform specific cleanup of resources
136+
if err := r.cleanupNemoDatastore(ctx, NemoDatastore); err != nil {
137+
r.GetEventRecorder().Eventf(NemoDatastore, corev1.EventTypeNormal, "Delete",
138+
"NemoDatastore %s is being deleted", NemoDatastore.Name)
139+
return ctrl.Result{}, err
140+
}
135141
if controllerutil.ContainsFinalizer(NemoDatastore, NemoDatastoreFinalizer) {
136-
// Perform platform specific cleanup of resources
137-
if err := r.cleanupNemoDatastore(ctx, NemoDatastore); err != nil {
138-
r.GetEventRecorder().Eventf(NemoDatastore, corev1.EventTypeNormal, "Delete",
139-
"NemoDatastore %s in deleted", NemoDatastore.Name)
140-
return ctrl.Result{}, err
141-
}
142-
143142
// Remove finalizer to allow for deletion
144143
controllerutil.RemoveFinalizer(NemoDatastore, NemoDatastoreFinalizer)
145144
if err := r.Update(ctx, NemoDatastore); err != nil {
146145
r.GetEventRecorder().Eventf(NemoDatastore, corev1.EventTypeNormal, "Delete",
147146
"NemoDatastore %s finalizer removed", NemoDatastore.Name)
148147
return ctrl.Result{}, err
149148
}
150-
return ctrl.Result{}, nil
151149
}
150+
// return as the cr is being deleted and GC will cleanup owned objects
151+
return ctrl.Result{}, nil
152152
}
153153

154154
// Fetch container orchestrator type

‎internal/controller/nemo_entitystore_controller.go

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -131,23 +131,23 @@ func (r *NemoEntitystoreReconciler) Reconcile(ctx context.Context, req ctrl.Requ
131131
}
132132
} else {
133133
// The instance is being deleted
134+
// Perform platform specific cleanup of resources
135+
if err := r.cleanupNemoEntitystore(ctx, NemoEntitystore); err != nil {
136+
r.GetEventRecorder().Eventf(NemoEntitystore, corev1.EventTypeNormal, "Delete",
137+
"NemoEntitystore %s is being deleted", NemoEntitystore.Name)
138+
return ctrl.Result{}, err
139+
}
134140
if controllerutil.ContainsFinalizer(NemoEntitystore, NemoEntitystoreFinalizer) {
135-
// Perform platform specific cleanup of resources
136-
if err := r.cleanupNemoEntitystore(ctx, NemoEntitystore); err != nil {
137-
r.GetEventRecorder().Eventf(NemoEntitystore, corev1.EventTypeNormal, "Delete",
138-
"NemoEntitystore %s in deleted", NemoEntitystore.Name)
139-
return ctrl.Result{}, err
140-
}
141-
142141
// Remove finalizer to allow for deletion
143142
controllerutil.RemoveFinalizer(NemoEntitystore, NemoEntitystoreFinalizer)
144143
if err := r.Update(ctx, NemoEntitystore); err != nil {
145144
r.GetEventRecorder().Eventf(NemoEntitystore, corev1.EventTypeNormal, "Delete",
146145
"NemoEntitystore %s finalizer removed", NemoEntitystore.Name)
147146
return ctrl.Result{}, err
148147
}
149-
return ctrl.Result{}, nil
150148
}
149+
// return as the cr is being deleted and GC will cleanup owned objects
150+
return ctrl.Result{}, nil
151151
}
152152

153153
// Fetch container orchestrator type

‎internal/controller/nemo_evaluator_controller.go

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -132,23 +132,23 @@ func (r *NemoEvaluatorReconciler) Reconcile(ctx context.Context, req ctrl.Reques
132132
}
133133
} else {
134134
// The instance is being deleted
135+
// Perform platform specific cleanup of resources
136+
if err := r.cleanupNemoEvaluator(ctx, NemoEvaluator); err != nil {
137+
r.GetEventRecorder().Eventf(NemoEvaluator, corev1.EventTypeNormal, "Delete",
138+
"NemoEvaluator %s is being deleted", NemoEvaluator.Name)
139+
return ctrl.Result{}, err
140+
}
135141
if controllerutil.ContainsFinalizer(NemoEvaluator, NemoEvaluatorFinalizer) {
136-
// Perform platform specific cleanup of resources
137-
if err := r.cleanupNemoEvaluator(ctx, NemoEvaluator); err != nil {
138-
r.GetEventRecorder().Eventf(NemoEvaluator, corev1.EventTypeNormal, "Delete",
139-
"NemoEvaluator %s in deleted", NemoEvaluator.Name)
140-
return ctrl.Result{}, err
141-
}
142-
143142
// Remove finalizer to allow for deletion
144143
controllerutil.RemoveFinalizer(NemoEvaluator, NemoEvaluatorFinalizer)
145144
if err := r.Update(ctx, NemoEvaluator); err != nil {
146145
r.GetEventRecorder().Eventf(NemoEvaluator, corev1.EventTypeNormal, "Delete",
147146
"NemoEvaluator %s finalizer removed", NemoEvaluator.Name)
148147
return ctrl.Result{}, err
149148
}
150-
return ctrl.Result{}, nil
151149
}
150+
// return as the cr is being deleted and GC will cleanup owned objects
151+
return ctrl.Result{}, nil
152152
}
153153

154154
// Fetch container orchestrator type

‎internal/controller/nemo_guardrail_controller.go

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -132,23 +132,24 @@ func (r *NemoGuardrailReconciler) Reconcile(ctx context.Context, req ctrl.Reques
132132
}
133133
} else {
134134
// The instance is being deleted
135-
if controllerutil.ContainsFinalizer(NemoGuardrail, NemoGuardrailFinalizer) {
136-
// Perform platform specific cleanup of resources
137-
if err := r.cleanupNemoGuardrail(ctx, NemoGuardrail); err != nil {
138-
r.GetEventRecorder().Eventf(NemoGuardrail, corev1.EventTypeNormal, "Delete",
139-
"NemoGuardrail %s in deleted", NemoGuardrail.Name)
140-
return ctrl.Result{}, err
141-
}
135+
// Perform platform specific cleanup of resources
136+
if err := r.cleanupNemoGuardrail(ctx, NemoGuardrail); err != nil {
137+
r.GetEventRecorder().Eventf(NemoGuardrail, corev1.EventTypeNormal, "Delete",
138+
"NemoGuardrail %s is being deleted", NemoGuardrail.Name)
139+
return ctrl.Result{}, err
140+
}
142141

142+
if controllerutil.ContainsFinalizer(NemoGuardrail, NemoGuardrailFinalizer) {
143143
// Remove finalizer to allow for deletion
144144
controllerutil.RemoveFinalizer(NemoGuardrail, NemoGuardrailFinalizer)
145145
if err := r.Update(ctx, NemoGuardrail); err != nil {
146146
r.GetEventRecorder().Eventf(NemoGuardrail, corev1.EventTypeNormal, "Delete",
147147
"NemoGuardrail %s finalizer removed", NemoGuardrail.Name)
148148
return ctrl.Result{}, err
149149
}
150-
return ctrl.Result{}, nil
151150
}
151+
// return as the cr is being deleted and GC will cleanup owned objects
152+
return ctrl.Result{}, nil
152153
}
153154

154155
// Fetch container orchestrator type

‎internal/controller/nemocustomizer_controller.go

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -146,23 +146,24 @@ func (r *NemoCustomizerReconciler) Reconcile(ctx context.Context, req ctrl.Reque
146146
}
147147
} else {
148148
// The instance is being deleted
149-
if controllerutil.ContainsFinalizer(NemoCustomizer, NemoCustomizerFinalizer) {
150-
// Perform platform specific cleanup of resources
151-
if err := r.cleanupNemoCustomizer(ctx, NemoCustomizer); err != nil {
152-
r.GetEventRecorder().Eventf(NemoCustomizer, corev1.EventTypeNormal, "Delete",
153-
"NemoCustomizer %s in deleted", NemoCustomizer.Name)
154-
return ctrl.Result{}, err
155-
}
149+
// Perform platform specific cleanup of resources
150+
if err := r.cleanupNemoCustomizer(ctx, NemoCustomizer); err != nil {
151+
r.GetEventRecorder().Eventf(NemoCustomizer, corev1.EventTypeNormal, "Delete",
152+
"NemoCustomizer %s in being deleted", NemoCustomizer.Name)
153+
return ctrl.Result{}, err
154+
}
156155

156+
if controllerutil.ContainsFinalizer(NemoCustomizer, NemoCustomizerFinalizer) {
157157
// Remove finalizer to allow for deletion
158158
controllerutil.RemoveFinalizer(NemoCustomizer, NemoCustomizerFinalizer)
159159
if err := r.Update(ctx, NemoCustomizer); err != nil {
160160
r.GetEventRecorder().Eventf(NemoCustomizer, corev1.EventTypeNormal, "Delete",
161161
"NemoCustomizer %s finalizer removed", NemoCustomizer.Name)
162162
return ctrl.Result{}, err
163163
}
164-
return ctrl.Result{}, nil
165164
}
165+
// return as the cr is being deleted and GC will cleanup owned objects
166+
return ctrl.Result{}, nil
166167
}
167168

168169
// Fetch container orchestrator type

‎internal/controller/nimcache_controller.go

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -165,19 +165,20 @@ func (r *NIMCacheReconciler) Reconcile(ctx context.Context, req ctrl.Request) (c
165165
}
166166
} else {
167167
// The instance is being deleted
168-
if controllerutil.ContainsFinalizer(nimCache, NIMCacheFinalizer) {
169-
// Perform cleanup of resources
170-
if err = r.cleanupNIMCache(ctx, nimCache); err != nil {
171-
return ctrl.Result{}, err
172-
}
168+
// Perform cleanup of resources
169+
if err = r.cleanupNIMCache(ctx, nimCache); err != nil {
170+
return ctrl.Result{}, err
171+
}
173172

173+
if controllerutil.ContainsFinalizer(nimCache, NIMCacheFinalizer) {
174174
// Remove finalizer to allow for deletion
175175
controllerutil.RemoveFinalizer(nimCache, NIMCacheFinalizer)
176176
if err := r.Update(ctx, nimCache); err != nil {
177177
return ctrl.Result{}, err
178178
}
179-
return ctrl.Result{}, nil
180179
}
180+
// return as the cr is being deleted and GC will cleanup owned objects
181+
return ctrl.Result{}, nil
181182
}
182183

183184
// Fetch container orchestrator type

‎internal/controller/nimpipeline_controller.go

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -89,19 +89,20 @@ func (r *NIMPipelineReconciler) Reconcile(ctx context.Context, req ctrl.Request)
8989
}
9090
} else {
9191
// The instance is being deleted
92-
if controllerutil.ContainsFinalizer(nimPipeline, NIMPipelineFinalizer) {
93-
// Perform cleanup of resources
94-
if err := r.cleanupNIMPipeline(ctx, nimPipeline); err != nil {
95-
return ctrl.Result{}, err
96-
}
92+
// Perform cleanup of resources
93+
if err := r.cleanupNIMPipeline(ctx, nimPipeline); err != nil {
94+
return ctrl.Result{}, err
95+
}
9796

97+
if controllerutil.ContainsFinalizer(nimPipeline, NIMPipelineFinalizer) {
9898
// Remove finalizer to allow for deletion
9999
controllerutil.RemoveFinalizer(nimPipeline, NIMPipelineFinalizer)
100100
if err := r.Update(ctx, nimPipeline); err != nil {
101101
return ctrl.Result{}, err
102102
}
103-
return ctrl.Result{}, nil
104103
}
104+
// return as the cr is being deleted and GC will cleanup owned objects
105+
return ctrl.Result{}, nil
105106
}
106107

107108
// Handle nim-cache reconciliation

‎internal/controller/nimservice_controller.go

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -133,23 +133,24 @@ func (r *NIMServiceReconciler) Reconcile(ctx context.Context, req ctrl.Request)
133133
}
134134
} else {
135135
// The instance is being deleted
136-
if controllerutil.ContainsFinalizer(nimService, NIMServiceFinalizer) {
137-
// Perform platform specific cleanup of resources
138-
if err := r.Platform.Delete(ctx, r, nimService); err != nil {
139-
r.GetEventRecorder().Eventf(nimService, corev1.EventTypeNormal, "Delete",
140-
"NIMService %s in deleted", nimService.Name)
141-
return ctrl.Result{}, err
142-
}
136+
// Perform platform specific cleanup of resources
137+
if err := r.Platform.Delete(ctx, r, nimService); err != nil {
138+
r.GetEventRecorder().Eventf(nimService, corev1.EventTypeNormal, "Delete",
139+
"NIMService %s is being deleted", nimService.Name)
140+
return ctrl.Result{}, err
141+
}
143142

143+
if controllerutil.ContainsFinalizer(nimService, NIMServiceFinalizer) {
144144
// Remove finalizer to allow for deletion
145145
controllerutil.RemoveFinalizer(nimService, NIMServiceFinalizer)
146146
if err := r.Update(ctx, nimService); err != nil {
147147
r.GetEventRecorder().Eventf(nimService, corev1.EventTypeNormal, "Delete",
148148
"NIMService %s finalizer removed", nimService.Name)
149149
return ctrl.Result{}, err
150150
}
151-
return ctrl.Result{}, nil
152151
}
152+
// return as the cr is being deleted and GC will cleanup owned objects
153+
return ctrl.Result{}, nil
153154
}
154155

155156
// Fetch container orchestrator type

0 commit comments

Comments
 (0)