Skip to content

Commit 545e639

Browse files
Merge main into v1.10-branch with known resolved conflicts
2 parents 825518f + 0b1dcd8 commit 545e639

File tree

4 files changed

+38
-24
lines changed

4 files changed

+38
-24
lines changed

.coderabbit.yaml

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
---
2+
language: en-US
3+
early_access: false
4+
enable_free_tier: true
5+
reviews:
6+
poem: false

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

Lines changed: 26 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,9 @@ var getWebhookTracer func() trace.Tracer = sync.OnceValue(func() trace.Tracer {
7373
const (
7474
IMAGE_STREAM_NOT_FOUND_EVENT = "imagestream-not-found"
7575
IMAGE_STREAM_TAG_NOT_FOUND_EVENT = "imagestream-tag-not-found"
76+
77+
WorkbenchImageNamespaceAnnotation = "opendatahub.io/workbench-image-namespace"
78+
LastImageSelectionAnnotation = "notebooks.opendatahub.io/last-image-selection"
7679
)
7780

7881
// InjectReconciliationLock injects the kubeflow notebook controller culling
@@ -716,7 +719,7 @@ func SetContainerImageFromRegistry(ctx context.Context, cli client.Client, noteb
716719

717720
annotations := notebook.GetAnnotations()
718721
if annotations != nil {
719-
if imageSelection, exists := annotations["notebooks.opendatahub.io/last-image-selection"]; exists {
722+
if imageSelection, exists := annotations[LastImageSelectionAnnotation]; exists {
720723

721724
containerFound := false
722725
// Iterate over containers to find the one matching the notebook name
@@ -743,35 +746,37 @@ func SetContainerImageFromRegistry(ctx context.Context, cli client.Client, noteb
743746
imagestreamName := imageSelected[0]
744747
imgSelection := &imagev1.ImageStream{}
745748

746-
// Search for the ImageStream in the controller namespace first
747-
// As default, the ImageStream is created in the controller namespace
748-
// if not found, search in the notebook namespace
749-
// Note: This is in this order, so users should not overwrite the ImageStream
750-
err := cli.Get(ctx, types.NamespacedName{Name: imagestreamName, Namespace: controllerNamespace}, imgSelection)
749+
// in user-created Notebook CRs, the annotation may be missing
750+
// Dashboard creates it with an empty value (null in TypeScript) when the controllerNamespace is intended
751+
// https://github.com/opendatahub-io/odh-dashboard/blob/2692224c3157f00a6fe93a2ca5bd267e3ff964ca/frontend/src/api/k8s/notebooks.ts#L215-L216
752+
imageNamespace, nsExists := annotations[WorkbenchImageNamespaceAnnotation]
753+
if !nsExists || strings.TrimSpace(imageNamespace) == "" {
754+
log.Info("Unable to find the namespace annotation in the Notebook CR, or the value of it is an empty string. Will search in controller namespace",
755+
"annotationName", WorkbenchImageNamespaceAnnotation,
756+
"controllerNamespace", controllerNamespace)
757+
imageNamespace = controllerNamespace
758+
}
759+
760+
// Search for the ImageStream in the specified namespace
761+
// As default when no namespace specified, the ImageStream is searched for in the controller namespace
762+
err := cli.Get(ctx, types.NamespacedName{Name: imagestreamName, Namespace: imageNamespace}, imgSelection)
751763
if apierrs.IsNotFound(err) {
752-
log.Info("Unable to find the ImageStream in controller namespace, try finding in notebook namespace", "imagestream", imagestreamName, "controllerNamespace", controllerNamespace)
753-
// Check if the ImageStream is present in the notebook namespace
754-
err = cli.Get(ctx, types.NamespacedName{Name: imagestreamName, Namespace: notebook.Namespace}, imgSelection)
755-
if err != nil {
756-
log.Error(err, "Error getting ImageStream", "imagestream", imagestreamName, "controllerNamespace", controllerNamespace)
757-
span.AddEvent(IMAGE_STREAM_NOT_FOUND_EVENT)
758-
} else {
759-
// ImageStream found in the notebook namespace
760-
imagestreamFound = true
761-
log.Info("ImageStream found in notebook namespace", "imagestream", imagestreamName, "namespace", notebook.Namespace)
762-
}
764+
span.AddEvent(IMAGE_STREAM_NOT_FOUND_EVENT)
765+
log.Info("Unable to find the ImageStream in the expected namespace",
766+
"imagestream", imagestreamName,
767+
"imageNamespace", imageNamespace)
763768
} else if err != nil {
764-
log.Error(err, "Error getting ImageStream", "imagestream", imagestreamName, "controllerNamespace", controllerNamespace)
769+
log.Error(err, "Error getting ImageStream", "imagestream", imagestreamName, "imageNamespace", imageNamespace)
765770
} else {
766-
// ImageStream found in the controller namespace
771+
// ImageStream found
767772
imagestreamFound = true
768-
log.Info("ImageStream found in controller namespace", "imagestream", imagestreamName, "controllerNamespace", controllerNamespace)
773+
log.Info("ImageStream found", "imagestream", imagestreamName, "namespace", imageNamespace)
769774
}
770775

771776
if imagestreamFound {
772777
// Check if the ImageStream has a status and tags
773778
if imgSelection.Status.Tags == nil {
774-
log.Error(nil, "ImageStream has no status or tags", "name", imagestreamName, "namespace", controllerNamespace)
779+
log.Error(nil, "ImageStream has no status or tags", "name", imagestreamName, "namespace", imageNamespace)
775780
span.AddEvent(IMAGE_STREAM_TAG_NOT_FOUND_EVENT)
776781
return fmt.Errorf("ImageStream has no status or tags")
777782
}

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,8 @@ var _ = Describe("The Openshift Notebook webhook", func() {
9999
Namespace: Namespace,
100100
Annotations: map[string]string{
101101
"notebooks.opendatahub.io/last-image-selection": "some-image:some-tag",
102+
// dashboard gives an empty string here to mean the image is from the operator's namespace
103+
"opendatahub.io/workbench-image-namespace": "",
102104
},
103105
},
104106
Spec: nbv1.NotebookSpec{
@@ -205,6 +207,7 @@ var _ = Describe("The Openshift Notebook webhook", func() {
205207
Namespace: Namespace,
206208
Annotations: map[string]string{
207209
"notebooks.opendatahub.io/last-image-selection": "some-image:some-tag",
210+
"opendatahub.io/workbench-image-namespace": Namespace,
208211
},
209212
},
210213
Spec: nbv1.NotebookSpec{

components/odh-notebook-controller/main.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -140,20 +140,20 @@ func main() {
140140
setupLog.Info("Controller is running in namespace", "namespace", namespace)
141141
if err = (&controllers.OpenshiftNotebookReconciler{
142142
Client: mgr.GetClient(),
143-
Log: ctrl.Log.WithName("controllers").WithName("Notebook"),
143+
Log: ctrl.Log.WithName("controllers").WithName("odh-notebook-controller"),
144144
Namespace: namespace,
145145
Scheme: mgr.GetScheme(),
146146
Config: mgr.GetConfig(),
147147
}).SetupWithManager(mgr); err != nil {
148-
setupLog.Error(err, "unable to create controller", "controller", "Notebook")
148+
setupLog.Error(err, "unable to create controller", "controller", "odh-notebook-controller")
149149
os.Exit(1)
150150
}
151151

152152
// Setup notebook mutating webhook
153153
hookServer := mgr.GetWebhookServer()
154154
notebookWebhook := &webhook.Admission{
155155
Handler: &controllers.NotebookWebhook{
156-
Log: ctrl.Log.WithName("controllers").WithName("Notebook"),
156+
Log: ctrl.Log.WithName("controllers").WithName("odh-notebook-webhook"),
157157
Client: mgr.GetClient(),
158158
Config: mgr.GetConfig(),
159159
Namespace: namespace,

0 commit comments

Comments
 (0)