Skip to content

Commit ba7e765

Browse files
committed
Fix linter issues and code review suggestions
1 parent c64b93d commit ba7e765

File tree

2 files changed

+20
-5
lines changed

2 files changed

+20
-5
lines changed

components/odh-notebook-controller/controllers/notebook_controller_test.go

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1061,7 +1061,10 @@ var _ = Describe("The Openshift Notebook controller", func() {
10611061
By("Ensuring finalizer exists first")
10621062
Eventually(func() []string {
10631063
var updated nbv1.Notebook
1064-
cli.Get(ctx, notebookKey, &updated)
1064+
err := cli.Get(ctx, notebookKey, &updated)
1065+
if err != nil {
1066+
return nil
1067+
}
10651068
return updated.Finalizers
10661069
}, duration, interval).Should(ContainElement(Finalizer))
10671070

@@ -1083,7 +1086,10 @@ var _ = Describe("The Openshift Notebook controller", func() {
10831086
By("Ensuring prerequisite resources exist")
10841087
Eventually(func() []string {
10851088
var updated nbv1.Notebook
1086-
cli.Get(ctx, notebookKey, &updated)
1089+
err := cli.Get(ctx, notebookKey, &updated)
1090+
if err != nil {
1091+
return nil
1092+
}
10871093
return updated.Finalizers
10881094
}, duration, interval).Should(ContainElement(Finalizer))
10891095

@@ -1100,8 +1106,10 @@ var _ = Describe("The Openshift Notebook controller", func() {
11001106
By("Verifying notebook is marked for deletion but persists due to finalizer")
11011107
Eventually(func() bool {
11021108
var updated nbv1.Notebook
1103-
err := cli.Get(ctx, notebookKey, &updated)
1104-
return err == nil && updated.DeletionTimestamp != nil
1109+
if err := cli.Get(ctx, notebookKey, &updated); err != nil {
1110+
return false
1111+
}
1112+
return updated.DeletionTimestamp != nil
11051113
}, duration, interval).Should(BeTrue(),
11061114
"Notebook should be marked for deletion but persist due to finalizer")
11071115

@@ -1120,7 +1128,7 @@ var _ = Describe("The Openshift Notebook controller", func() {
11201128
Eventually(func() error {
11211129
var oauth oauthv1.OAuthClient
11221130
return cli.Get(ctx, oauthClientKey, &oauth)
1123-
}, time.Second*30).Should(MatchError(ContainSubstring("not found")))
1131+
}, duration, interval).Should(MatchError(ContainSubstring("not found")))
11241132

11251133
By("Verifying finalizer is removed and notebook is deleted")
11261134
Eventually(func() error {

components/odh-notebook-controller/controllers/notebook_oauth.go

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -327,10 +327,13 @@ func (r *OpenshiftNotebookReconciler) ReconcileOAuthClient(notebook *nbv1.Notebo
327327

328328
func (r *OpenshiftNotebookReconciler) HandleOAuthClientFinalizer(notebook *nbv1.Notebook, ctx context.Context) (bool, error) {
329329
finalizerName := "notebooks.kubeflow.org/oauthclient"
330+
log := logf.FromContext(ctx)
330331

331332
// Check if object is being deleted
332333
if notebook.GetDeletionTimestamp() != nil {
333334
if controllerutil.ContainsFinalizer(notebook, finalizerName) {
335+
log.Info("Notebook deletion detected, cleaning up OAuthClient", "notebook", notebook.Name)
336+
334337
if err := r.cleanupOAuthClient(notebook, ctx); err != nil {
335338
return true, err
336339
}
@@ -339,13 +342,17 @@ func (r *OpenshiftNotebookReconciler) HandleOAuthClientFinalizer(notebook *nbv1.
339342
if err := r.Update(ctx, notebook); err != nil {
340343
return true, err
341344
}
345+
346+
log.Info("OAuthClient cleanup completed", "notebook", notebook.Name)
342347
}
343348
// Return true to indicate finalization is in progress
344349
return true, nil
345350
}
346351

347352
// Add finalizer if it doesn't exist yet
348353
if !controllerutil.ContainsFinalizer(notebook, finalizerName) {
354+
log.Info("Adding finalizer to notebook", "notebook", notebook.Name)
355+
349356
controllerutil.AddFinalizer(notebook, finalizerName)
350357
if err := r.Update(ctx, notebook); err != nil {
351358
return true, err

0 commit comments

Comments
 (0)