Skip to content

Commit b1d31f9

Browse files
authored
RHOAIENG-25707: feat(webhook): read namespace annotation on Notebook CR to locate ImageStream (#625)
* RHOAIENG-25707: feat(webhook): read namespace annotation on Notebook CR to locate ImageStream https://issues.redhat.com/browse/RHOAIENG-25707 Read `opendatahub.io/workbench-image-namespace` and `notebooks.opendatahub.io/last-image-selection` annotations to identify the correct ImageStream from which the imageHash needs to be used. * fixup, interpret a "" image namespace as meaning the default controller namespace, because that's how dashboard does it
1 parent cb4d4af commit b1d31f9

File tree

2 files changed

+29
-21
lines changed

2 files changed

+29
-21
lines changed

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{

0 commit comments

Comments
 (0)