From 999d76ad381aea935aa54ce1cdd7ea58a2244993 Mon Sep 17 00:00:00 2001 From: atheo89 Date: Fri, 7 Feb 2025 13:54:27 +0100 Subject: [PATCH] Add a new notebook_runtime script where includes the logic to create/watch/update a new ConfigMap the `pipeline-runtime-images` for runtime images and mount it as volume on the notebook when is getting created Move ConfigMapName, mountPath and volumeName as global vars Add DevFlag SET_RUNTIMES_CM to enable/diable the feature Add test cases for the new configMap configuration Remove devflag Revert changes on makefile --- .../controllers/notebook_controller.go | 8 + .../controllers/notebook_controller_test.go | 104 ++++++++ .../controllers/notebook_runtime.go | 248 ++++++++++++++++++ .../controllers/notebook_webhook.go | 6 + 4 files changed, 366 insertions(+) create mode 100644 components/odh-notebook-controller/controllers/notebook_runtime.go diff --git a/components/odh-notebook-controller/controllers/notebook_controller.go b/components/odh-notebook-controller/controllers/notebook_controller.go index e8b77ace91f..e2520ca5862 100644 --- a/components/odh-notebook-controller/controllers/notebook_controller.go +++ b/components/odh-notebook-controller/controllers/notebook_controller.go @@ -40,6 +40,7 @@ import ( "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/wait" + "k8s.io/client-go/rest" "k8s.io/client-go/util/retry" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" @@ -60,6 +61,7 @@ type OpenshiftNotebookReconciler struct { Namespace string Scheme *runtime.Scheme Log logr.Logger + Config *rest.Config } // ClusterRole permissions @@ -190,6 +192,12 @@ func (r *OpenshiftNotebookReconciler) Reconcile(ctx context.Context, req ctrl.Re return ctrl.Result{}, err } + // Create/Watch and Update the pipeline-runtime-image ConfigMap on Notebook's Namspace + err = r.EnsureNotebookConfigMap(notebook, ctx) + if err != nil { + return ctrl.Result{}, err + } + // Call the Rolebinding reconciler if strings.ToLower(strings.TrimSpace(os.Getenv("SET_PIPELINE_RBAC"))) == "true" { err = r.ReconcileRoleBindings(notebook, ctx) diff --git a/components/odh-notebook-controller/controllers/notebook_controller_test.go b/components/odh-notebook-controller/controllers/notebook_controller_test.go index 10ece7619f6..2e33ba848a3 100644 --- a/components/odh-notebook-controller/controllers/notebook_controller_test.go +++ b/components/odh-notebook-controller/controllers/notebook_controller_test.go @@ -87,6 +87,59 @@ var _ = Describe("The Openshift Notebook controller", func() { route := &routev1.Route{} + It("Should create and mount the runtime images ConfigMap in the Notebook's pod", func() { + const ( + Name = "test-notebook-runtime" + Namespace = "default" + configMapName = "pipeline-runtime-images" + mountPath = "/opt/app-root/pipeline-runtimes/" + volumeName = "runtime-images" + ) + + ctx := context.Background() + + By("Creating a ConfigMap before the Notebook") + runtimeImagesCM := createRuntimeImagesConfigMap(configMapName, Namespace, map[string]string{ + "runtime-1.yaml": "content-1", + "runtime-2.yaml": "content-2", + }) + + Expect(cli.Create(ctx, runtimeImagesCM)).Should(Succeed()) + defer func() { + if err := cli.Delete(ctx, runtimeImagesCM); err != nil { + GinkgoT().Logf("Failed to delete ConfigMap: %v", err) + } + }() + + By("Creating a new Notebook with the ConfigMap mounted") + notebook := createNotebook(Name, Namespace) + + notebook.Spec.Template.Spec.Volumes = append(notebook.Spec.Template.Spec.Volumes, corev1.Volume{ + Name: volumeName, + VolumeSource: corev1.VolumeSource{ + ConfigMap: &corev1.ConfigMapVolumeSource{ + LocalObjectReference: corev1.LocalObjectReference{Name: configMapName}, + Optional: pointer.Bool(true), + }, + }, + }) + + notebook.Spec.Template.Spec.Containers[0].VolumeMounts = append( + notebook.Spec.Template.Spec.Containers[0].VolumeMounts, + corev1.VolumeMount{ + Name: volumeName, + MountPath: mountPath, + ReadOnly: true, + }, + ) + + Expect(cli.Create(ctx, notebook)).Should(Succeed()) + time.Sleep(interval) + + By("Checking that the ConfigMap is correctly mounted in the Notebook's pod") + checkConfigMapMount(ctx, Namespace, Name, configMapName, mountPath) + }) + It("Should create a Route to expose the traffic externally", func() { ctx := context.Background() @@ -1147,3 +1200,54 @@ func checkCertConfigMap(ctx context.Context, namespace string, configMapName str } Expect(certificatesFound).Should(Equal(expNumberCerts), "Number of parsed certificates don't match expected one:\n"+certData) } + +func createRuntimeImagesConfigMap(name, namespace string, data map[string]string) *corev1.ConfigMap { + return &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: namespace, + Labels: map[string]string{ + "opendatahub.io/managed-by": "workbenches", + }, + }, + Data: data, + } +} + +func checkConfigMapMount(ctx context.Context, namespace, notebookName, configMapName, mountPath string) { + notebook := &nbv1.Notebook{} + key := types.NamespacedName{Name: notebookName, Namespace: namespace} + Expect(cli.Get(ctx, key, notebook)).Should(Succeed()) + + // Debug: Print all VolumeMounts in the Notebook Spec + for _, container := range notebook.Spec.Template.Spec.Containers { + GinkgoT().Logf("Container: %s, VolumeMounts: %+v", container.Name, container.VolumeMounts) + } + + // Debug: Print all Volumes in the Notebook Spec + GinkgoT().Logf("Notebook Volumes: %+v", notebook.Spec.Template.Spec.Volumes) + + expectedVolumeMount := corev1.VolumeMount{ + Name: "runtime-images", + MountPath: mountPath, + ReadOnly: true, + } + + expectedVolume := corev1.Volume{ + Name: "runtime-images", + VolumeSource: corev1.VolumeSource{ + ConfigMap: &corev1.ConfigMapVolumeSource{ + LocalObjectReference: corev1.LocalObjectReference{Name: configMapName}, + Optional: pointer.Bool(true), + }, + }, + } + + // Check for Volume Mounts + Expect(notebook.Spec.Template.Spec.Containers[0].VolumeMounts).To(ContainElement(expectedVolumeMount), + "Expected volume mount not found!") + + // Check for Volumes + Expect(notebook.Spec.Template.Spec.Volumes).To(ContainElement(expectedVolume), + "Expected volume not found!") +} diff --git a/components/odh-notebook-controller/controllers/notebook_runtime.go b/components/odh-notebook-controller/controllers/notebook_runtime.go new file mode 100644 index 00000000000..78e508c9e58 --- /dev/null +++ b/components/odh-notebook-controller/controllers/notebook_runtime.go @@ -0,0 +1,248 @@ +package controllers + +import ( + "context" + "encoding/json" + "strings" + + "github.com/go-logr/logr" + nbv1 "github.com/kubeflow/kubeflow/components/notebook-controller/api/v1" + corev1 "k8s.io/api/core/v1" + apierrs "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apimachinery/pkg/types" + "k8s.io/client-go/dynamic" + "k8s.io/client-go/rest" +) + +const configMapName = "pipeline-runtime-images" +const mountPath = "/opt/app-root/pipeline-runtimes/" +const volumeName = "runtime-images" + +// checkConfigMapExists verifies if a ConfigMap exists in the namespace. +func (r *OpenshiftNotebookReconciler) checkConfigMapExists(ctx context.Context, configMapName, namespace string) (bool, error) { + configMap := &corev1.ConfigMap{} + err := r.Get(ctx, types.NamespacedName{Name: configMapName, Namespace: namespace}, configMap) + if err != nil { + if apierrs.IsNotFound(err) { + return false, nil // ConfigMap not found + } + return false, err // Some other error occurred + } + //r.Log.Info("ConfigMap found", "ConfigMap.Name", configMapName, "Namespace", namespace) + return true, nil // ConfigMap exists +} + +func (r *OpenshiftNotebookReconciler) syncRuntimeImagesConfigMap(ctx context.Context, notebookNamespace string) error { + log := r.Log.WithValues("namespace", notebookNamespace) + + // Create a dynamic client + config, err := rest.InClusterConfig() + if err != nil { + log.Error(err, "Error creating cluster config") + return err + } + dynamicClient, err := dynamic.NewForConfig(config) + if err != nil { + log.Error(err, "Error creating dynamic client") + return err + } + + // Define GroupVersionResource for ImageStreams + ims := schema.GroupVersionResource{ + Group: "image.openshift.io", + Version: "v1", + Resource: "imagestreams", + } + + // Fetch ImageStreams from "redhat-ods-applications" namespace + imageStreamNamespace := "redhat-ods-applications" + imagestreams, err := dynamicClient.Resource(ims).Namespace(imageStreamNamespace).List(ctx, metav1.ListOptions{}) + if err != nil { + log.Error(err, "Failed to list ImageStreams", "Namespace", imageStreamNamespace) + return err + } + + // Prepare data for ConfigMap + data := make(map[string]string) + for _, item := range imagestreams.Items { + labels := item.GetLabels() + if labels["opendatahub.io/runtime-image"] == "true" { + tags, found, err := unstructured.NestedSlice(item.Object, "spec", "tags") + if err != nil || !found { + log.Error(err, "Failed to extract tags from ImageStream", "ImageStream", item.GetName()) + continue + } + + for _, tag := range tags { + tagMap, ok := tag.(map[string]interface{}) + if !ok { + continue + } + + // Extract metadata annotation + annotations, found, err := unstructured.NestedMap(tagMap, "annotations") + if err != nil || !found { + annotations = map[string]interface{}{} + } + + metadataRaw, ok := annotations["opendatahub.io/runtime-image-metadata"].(string) + if !ok || metadataRaw == "" { + metadataRaw = "[]" + } + + // Parse metadata + metadataParsed := parseRuntimeImageMetadata(metadataRaw) + displayName := extractDisplayName(metadataParsed) + + // Construct the key name + if displayName != "" { + formattedName := formatKeyName(displayName) + data[formattedName] = metadataParsed + } + } + } + } + + // Create or update the ConfigMap in the Notebook's namespace + configMap := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: configMapName, + Namespace: notebookNamespace, + Labels: map[string]string{"opendatahub.io/managed-by": "workbenches"}, + }, + Data: data, + } + + configMapExists, err := r.checkConfigMapExists(ctx, configMapName, notebookNamespace) + if err != nil { + log.Error(err, "Error checking if ConfigMap exists", "ConfigMap.Name", configMapName) + return err + } + + if configMapExists { + existingConfigMap := &corev1.ConfigMap{} + if err := r.Get(ctx, types.NamespacedName{Name: configMapName, Namespace: notebookNamespace}, existingConfigMap); err != nil { + log.Error(err, "Failed to get existing ConfigMap", "ConfigMap.Name", configMapName) + return err + } + + if !jsonEqual(existingConfigMap.Data, data) { + existingConfigMap.Data = data + if err := r.Update(ctx, existingConfigMap); err != nil { + log.Error(err, "Failed to update ConfigMap", "ConfigMap.Name", configMapName) + return err + } + log.Info("Updated existing ConfigMap with new runtime images", "ConfigMap.Name", configMapName) + } + } else { + if err := r.Create(ctx, configMap); err != nil { + log.Error(err, "Failed to create ConfigMap", "ConfigMap.Name", configMapName) + return err + } + log.Info("Created new ConfigMap for runtime images", "ConfigMap.Name", configMapName) + } + + return nil +} + +// jsonEqual compares two JSON-like maps for equality. +func jsonEqual(a, b map[string]string) bool { + aBytes, _ := json.Marshal(a) + bBytes, _ := json.Marshal(b) + return string(aBytes) == string(bBytes) +} + +func extractDisplayName(metadata string) string { + var metadataMap map[string]interface{} + err := json.Unmarshal([]byte(metadata), &metadataMap) + if err != nil { + return "" + } + displayName, ok := metadataMap["display_name"].(string) + if !ok { + return "" + } + return displayName +} + +func formatKeyName(displayName string) string { + replacer := strings.NewReplacer(" ", "-", "(", "", ")", "") + return strings.ToLower(replacer.Replace(displayName)) + ".json" +} + +// parseRuntimeImageMetadata extracts the first object from the JSON array +func parseRuntimeImageMetadata(rawJSON string) string { + var metadataArray []map[string]interface{} + + err := json.Unmarshal([]byte(rawJSON), &metadataArray) + if err != nil || len(metadataArray) == 0 { + return "{}" // Return empty JSON object if parsing fails + } + + // Convert first object back to JSON + metadataJSON, err := json.Marshal(metadataArray[0]) + if err != nil { + return "{}" + } + + return string(metadataJSON) +} + +func (r *OpenshiftNotebookReconciler) EnsureNotebookConfigMap(notebook *nbv1.Notebook, ctx context.Context) error { + return r.syncRuntimeImagesConfigMap(ctx, notebook.Namespace) +} + +func MountPipelineRuntimeImages(notebook *nbv1.Notebook, log logr.Logger) error { + + // Define the volume + configVolume := corev1.Volume{ + Name: volumeName, + VolumeSource: corev1.VolumeSource{ + ConfigMap: &corev1.ConfigMapVolumeSource{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: configMapName, + }, + }, + }, + } + + // Define the volume mount + volumeMount := corev1.VolumeMount{ + Name: volumeName, + MountPath: mountPath, + } + + // Append the volume if it does not already exist + volumes := ¬ebook.Spec.Template.Spec.Volumes + volumeExists := false + for _, v := range *volumes { + if v.Name == volumeName { + volumeExists = true + break + } + } + if !volumeExists { + *volumes = append(*volumes, configVolume) + } + + log.Info("Injecting runtime-images volume into notebook", "notebook", notebook.Name, "namespace", notebook.Namespace) + + // Append the volume mount to all containers + for i, container := range notebook.Spec.Template.Spec.Containers { + mountExists := false + for _, vm := range container.VolumeMounts { + if vm.Name == volumeName { + mountExists = true + break + } + } + if !mountExists { + notebook.Spec.Template.Spec.Containers[i].VolumeMounts = append(container.VolumeMounts, volumeMount) + } + } + + return nil +} diff --git a/components/odh-notebook-controller/controllers/notebook_webhook.go b/components/odh-notebook-controller/controllers/notebook_webhook.go index 5b0db331f81..bd9fc57f9de 100644 --- a/components/odh-notebook-controller/controllers/notebook_webhook.go +++ b/components/odh-notebook-controller/controllers/notebook_webhook.go @@ -251,6 +251,12 @@ func (w *NotebookWebhook) Handle(ctx context.Context, req admission.Request) adm return admission.Errored(http.StatusInternalServerError, err) } + // Mount ConfigMap pipeline-runtime-images as runtime-images + err = MountPipelineRuntimeImages(notebook, log) + if err != nil { + return admission.Errored(http.StatusInternalServerError, err) + } + } // Check Imagestream Info both on create and update operations