From dbce3674e9fc1e89448f88d1605ffae02bd4d698 Mon Sep 17 00:00:00 2001 From: Harshad Reddy Nalla Date: Wed, 22 May 2024 13:30:46 -0400 Subject: [PATCH 1/3] Upgrade go version to 1.20 for notebook-controller Signed-off-by: Harshad Reddy Nalla --- components/notebook-controller/Dockerfile | 2 +- components/notebook-controller/go.mod | 2 +- components/odh-notebook-controller/Dockerfile | 2 +- components/odh-notebook-controller/go.mod | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/components/notebook-controller/Dockerfile b/components/notebook-controller/Dockerfile index 17afac285ea..0c49dc31184 100644 --- a/components/notebook-controller/Dockerfile +++ b/components/notebook-controller/Dockerfile @@ -6,7 +6,7 @@ # # This is necessary because the Jupyter controller now depends on # components/common -ARG GOLANG_VERSION=1.19 +ARG GOLANG_VERSION=1.20 FROM golang:${GOLANG_VERSION} as builder WORKDIR /workspace diff --git a/components/notebook-controller/go.mod b/components/notebook-controller/go.mod index ec81e50989b..8192738566d 100644 --- a/components/notebook-controller/go.mod +++ b/components/notebook-controller/go.mod @@ -1,6 +1,6 @@ module github.com/kubeflow/kubeflow/components/notebook-controller -go 1.19 +go 1.20 require ( github.com/go-logr/logr v1.2.4 diff --git a/components/odh-notebook-controller/Dockerfile b/components/odh-notebook-controller/Dockerfile index 2fca94a1c4c..961c6245fc1 100644 --- a/components/odh-notebook-controller/Dockerfile +++ b/components/odh-notebook-controller/Dockerfile @@ -4,7 +4,7 @@ # # ${PATH_TO_KUBEFLOW/KUBEFLOW repo}/components # -ARG GOLANG_VERSION=1.19 +ARG GOLANG_VERSION=1.20 FROM golang:${GOLANG_VERSION} as builder WORKDIR /workspace diff --git a/components/odh-notebook-controller/go.mod b/components/odh-notebook-controller/go.mod index 4287ee9b41f..bd98e0dc5cf 100644 --- a/components/odh-notebook-controller/go.mod +++ b/components/odh-notebook-controller/go.mod @@ -1,6 +1,6 @@ module github.com/opendatahub-io/kubeflow/components/odh-notebook-controller -go 1.19 +go 1.20 require ( github.com/go-logr/logr v1.2.4 From 0fe63c1ebfec050f3707cc42f135fbc6104fb677 Mon Sep 17 00:00:00 2001 From: Jan Stourac Date: Thu, 23 May 2024 16:44:01 +0200 Subject: [PATCH 2/3] Odd number of arguments passed as key-value pairs for logging Fix the dpanic for log in notebook controller while injecting the trusted CA bundle configuration. https://issues.redhat.com/browse/RHOAIENG-5033 --- .../odh-notebook-controller/controllers/notebook_webhook.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/components/odh-notebook-controller/controllers/notebook_webhook.go b/components/odh-notebook-controller/controllers/notebook_webhook.go index aa6bdf748d8..b8f977b9e78 100644 --- a/components/odh-notebook-controller/controllers/notebook_webhook.go +++ b/components/odh-notebook-controller/controllers/notebook_webhook.go @@ -314,7 +314,7 @@ func CheckAndMountCACertBundle(ctx context.Context, cli client.Client, notebook cm := workbenchConfigMap if cm.Name == workbenchConfigMapName { // Inject the trusted-ca volume and environment variables - log.Info("Injecting trusted-ca volume and environment variables", notebook.Name, "namespace", notebook.Namespace) + log.Info("Injecting trusted-ca volume and environment variables") return InjectCertConfig(notebook, workbenchConfigMapName) } return nil From 38cbf1251701aff4eadc2cf023fa57a2bdcfc244 Mon Sep 17 00:00:00 2001 From: OpenShift Cherrypick Robot Date: Thu, 23 May 2024 19:20:24 +0200 Subject: [PATCH 3/3] [stable] Add logic on nbc-webhook to pickup internal or external registry (#332) * Add logic on nbc-webhook to pickup internal or external registry * create a temporary kubeconfig file for the test setup * Pass kubeconfig as parameter,and handle the scenario of no namespaces that include the imagestream --------- Co-authored-by: atheo89 --- .../controllers/notebook_webhook.go | 115 ++++++++++++++++++ .../controllers/suite_test.go | 2 +- components/odh-notebook-controller/main.go | 1 + 3 files changed, 117 insertions(+), 1 deletion(-) diff --git a/components/odh-notebook-controller/controllers/notebook_webhook.go b/components/odh-notebook-controller/controllers/notebook_webhook.go index aa6bdf748d8..756a9eded2a 100644 --- a/components/odh-notebook-controller/controllers/notebook_webhook.go +++ b/components/odh-notebook-controller/controllers/notebook_webhook.go @@ -20,6 +20,8 @@ import ( "encoding/json" "fmt" "net/http" + "sort" + "strings" "github.com/go-logr/logr" nbv1 "github.com/kubeflow/kubeflow/components/notebook-controller/api/v1" @@ -28,7 +30,10 @@ import ( corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/util/intstr" + "k8s.io/client-go/dynamic" + "k8s.io/client-go/rest" "k8s.io/utils/pointer" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/webhook/admission" @@ -40,6 +45,7 @@ import ( type NotebookWebhook struct { Log logr.Logger Client client.Client + Config *rest.Config Decoder *admission.Decoder OAuthConfig OAuthConfig } @@ -246,6 +252,12 @@ func (w *NotebookWebhook) Handle(ctx context.Context, req admission.Request) adm if err != nil { return admission.Errored(http.StatusInternalServerError, err) } + + // Check Imagestream Info + err = SetContainerImageFromRegistry(ctx, w.Config, notebook, log) + if err != nil { + return admission.Errored(http.StatusInternalServerError, err) + } } // Inject the OAuth proxy if the annotation is present @@ -435,3 +447,106 @@ func InjectCertConfig(notebook *nbv1.Notebook, configMapName string) error { } return nil } + +// SetContainerImageFromRegistry checks if there is an internal registry and takes the corresponding actions to set the container.image value. +// If an internal registry is detected, it uses the default values specified in the Notebook Custom Resource (CR). +// Otherwise, it checks the last-image-selection annotation to find the image stream and fetches the image from status.dockerImageReference, +// assigning it to the container.image value. +func SetContainerImageFromRegistry(ctx context.Context, config *rest.Config, notebook *nbv1.Notebook, log logr.Logger) error { + // Create a dynamic client + dynamicClient, err := dynamic.NewForConfig(config) + if err != nil { + log.Error(err, "Error creating dynamic client") + return err + } + // Specify the GroupVersionResource for imagestreams + ims := schema.GroupVersionResource{ + Group: "image.openshift.io", + Version: "v1", + Resource: "imagestreams", + } + + annotations := notebook.GetAnnotations() + if annotations != nil { + if imageSelection, exists := annotations["notebooks.opendatahub.io/last-image-selection"]; exists { + // Check if the image selection has an internal registry, if so will pickup this. This value constructed on the initialization of the Notebook CR. + if strings.Contains(notebook.Spec.Template.Spec.Containers[0].Image, "image-registry.openshift-image-registry.svc:5000") { + log.Info("Internal registry found. Will pickup the default value from image field.") + return nil + } else { + // Split the imageSelection to imagestream and tag + parts := strings.Split(imageSelection, ":") + if len(parts) != 2 { + log.Error(nil, "Invalid image selection format") + return fmt.Errorf("invalid image selection format") + } + + imagestreamName := parts[0] + tag := parts[1] + + // Specify the namespaces to search in + namespaces := []string{"opendatahub", "redhat-ods-applications"} + + imagestreamFound := false + + for _, namespace := range namespaces { + // List imagestreams in the specified namespace + imagestreams, err := dynamicClient.Resource(ims).Namespace(namespace).List(ctx, metav1.ListOptions{}) + if err != nil { + log.Error(err, "Cannot list imagestreams", "namespace", namespace) + continue + } + + // Iterate through the imagestreams to find matches + for _, item := range imagestreams.Items { + metadata := item.Object["metadata"].(map[string]interface{}) + name := metadata["name"].(string) + + if name == imagestreamName { + status := item.Object["status"].(map[string]interface{}) + + log.Info("No Internal registry found, pick up imageHash from status.tag.dockerImageReference") + + tags := status["tags"].([]interface{}) + for _, t := range tags { + tagMap := t.(map[string]interface{}) + tagName := tagMap["tag"].(string) + if tagName == tag { + items := tagMap["items"].([]interface{}) + if len(items) > 0 { + // Sort items by creationTimestamp to get the most recent one + sort.Slice(items, func(i, j int) bool { + iTime := items[i].(map[string]interface{})["created"].(string) + jTime := items[j].(map[string]interface{})["created"].(string) + return iTime > jTime // Lexicographical comparison of RFC3339 timestamps + }) + imageHash := items[0].(map[string]interface{})["dockerImageReference"].(string) + notebook.Spec.Template.Spec.Containers[0].Image = imageHash + // Update the JUPYTER_IMAGE environment variable + for i, envVar := range notebook.Spec.Template.Spec.Containers[0].Env { + if envVar.Name == "JUPYTER_IMAGE" { + notebook.Spec.Template.Spec.Containers[0].Env[i].Value = imageHash + break + } + } + imagestreamFound = true + break + } + } + } + } + } + if imagestreamFound { + break + } + } + + if !imagestreamFound { + log.Info("Imagestream not found in any of the specified namespaces", "imagestreamName", imagestreamName, "tag", tag) + } + } + } + } + + return nil +} diff --git a/components/odh-notebook-controller/controllers/suite_test.go b/components/odh-notebook-controller/controllers/suite_test.go index 6809cee40f8..a6067c15ae3 100644 --- a/components/odh-notebook-controller/controllers/suite_test.go +++ b/components/odh-notebook-controller/controllers/suite_test.go @@ -1,5 +1,4 @@ /* - Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. You may obtain a copy of the License at @@ -133,6 +132,7 @@ var _ = BeforeSuite(func() { Handler: &NotebookWebhook{ Log: ctrl.Log.WithName("controllers").WithName("notebook-controller"), Client: mgr.GetClient(), + Config: mgr.GetConfig(), OAuthConfig: OAuthConfig{ ProxyImage: OAuthProxyImage, }, diff --git a/components/odh-notebook-controller/main.go b/components/odh-notebook-controller/main.go index f4adc8b0fb6..bef4615c3b0 100644 --- a/components/odh-notebook-controller/main.go +++ b/components/odh-notebook-controller/main.go @@ -116,6 +116,7 @@ func main() { Handler: &controllers.NotebookWebhook{ Log: ctrl.Log.WithName("controllers").WithName("Notebook"), Client: mgr.GetClient(), + Config: mgr.GetConfig(), OAuthConfig: controllers.OAuthConfig{ ProxyImage: oauthProxyImage, },