Skip to content

Commit

Permalink
Merge pull request #57 from red-hat-data-services/master
Browse files Browse the repository at this point in the history 
[rhoai-2.10] sync rhoai-2.10 branch with master branch
  • Loading branch information
@harshad16
harshad16 authored May 31, 2024
2 parents 4d4a27a + 4b30e42 commit c1f19c4
Show file tree
Hide file tree
Showing 6 changed files with 296 additions and 102 deletions.
2 changes: 1 addition & 1 deletion components/odh-notebook-controller/Makefile
View file
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@ undeploy-dev: setup undeploy-kf ## Undeploy controller from the Openshift cluste

.PHONY: e2e-test
e2e-test: ## Run e2e tests for the controller
go test ./e2e/ -run ^TestE2ENotebookController -v --nb-namespace=${K8S_NAMESPACE} ${E2E_TEST_FLAGS}
go test ./e2e/ -run ^TestE2ENotebookController -v --nb-namespace=${K8S_NAMESPACE} --deploymentMode=oauth ${E2E_TEST_FLAGS}

.PHONY: run-ci-e2e-tests
run-ci-e2e-tests:
Expand Down
53 changes: 52 additions & 1 deletion components/odh-notebook-controller/controllers/notebook_controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -253,7 +253,40 @@ var _ = Describe("The Openshift Notebook controller", func() {

})

Context("When creating a Notebook, test Networkpolicies", func() {
// New test case for notebook update
When("Updating a Notebook", func() {
const (
Name = "test-notebook-update"
Namespace = "default"
)

notebook := createNotebook(Name, Namespace)

It("Should update the Notebook specification", func() {
ctx := context.Background()

By("By creating a new Notebook")
Expect(cli.Create(ctx, notebook)).Should(Succeed())
time.Sleep(interval)

By("By updating the Notebook's image")
key := types.NamespacedName{Name: Name, Namespace: Namespace}
Expect(cli.Get(ctx, key, notebook)).Should(Succeed())

updatedImage := "registry.redhat.io/ubi8/ubi:updated"
notebook.Spec.Template.Spec.Containers[0].Image = updatedImage
Expect(cli.Update(ctx, notebook)).Should(Succeed())
time.Sleep(interval)

By("By checking that the Notebook's image is updated")
Eventually(func() string {
Expect(cli.Get(ctx, key, notebook)).Should(Succeed())
return notebook.Spec.Template.Spec.Containers[0].Image
}, timeout, interval).Should(Equal(updatedImage))
})
})

When("Creating a Notebook, test Networkpolicies", func() {
const (
Name = "test-notebook-np"
Namespace = "default"
Expand Down Expand Up @@ -869,3 +902,21 @@ var _ = Describe("The Openshift Notebook controller", func() {
})
})
})

// Setting func async to the upstream branch v1.7-branch,
// as servicemesh changes have not been moved stable branch
func createNotebook(name, namespace string) *nbv1.Notebook {
return &nbv1.Notebook{
ObjectMeta: metav1.ObjectMeta{
Name: name,
Namespace: namespace,
},
Spec: nbv1.NotebookSpec{
Template: nbv1.NotebookTemplateSpec{
Spec: corev1.PodSpec{Containers: []corev1.Container{{
Name: name,
Image: "registry.redhat.io/ubi8/ubi:latest",
}}}},
},
}
}
204 changes: 108 additions & 96 deletions components/odh-notebook-controller/controllers/notebook_webhook.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -252,7 +252,10 @@ func (w *NotebookWebhook) Handle(ctx context.Context, req admission.Request) adm
if err != nil {
return admission.Errored(http.StatusInternalServerError, err)
}
}

// Check Imagestream Info both on create and update operations
if req.Operation == admissionv1.Create || req.Operation == admissionv1.Update {
// Check Imagestream Info
err = SetContainerImageFromRegistry(ctx, w.Config, notebook, log)
if err != nil {
Expand Down Expand Up @@ -453,100 +456,109 @@ func InjectCertConfig(notebook *nbv1.Notebook, configMapName string) error {
// 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
// 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 {

containerFound := false
// Iterate over containers to find the one matching the notebook name
for i, container := range notebook.Spec.Template.Spec.Containers {
if container.Name == notebook.Name {
containerFound = true

// Check if the container.Image value has an internal registry, if so will pickup this without extra checks.
// This value constructed on the initialization of the Notebook CR.
if strings.Contains(container.Image, "image-registry.openshift-image-registry.svc:5000") {
log.Info("Internal registry found. Will pick up the default value from image field.")
return nil
} else {
log.Info("No internal registry found, let's pick up image reference from relevant ImageStream 'status.tags[].tag.dockerImageReference'")

// Split the imageSelection to imagestream and tag
imageSelected := strings.Split(imageSelection, ":")
if len(imageSelected) != 2 {
log.Error(nil, "Invalid image selection format")
return fmt.Errorf("invalid image selection format")
}

// 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)

// Match with the ImageStream name
if name == imageSelected[0] {
status := item.Object["status"].(map[string]interface{})

// Match to the corresponding tag of the image
tags := status["tags"].([]interface{})
for _, t := range tags {
tagMap := t.(map[string]interface{})
tagName := tagMap["tag"].(string)
if tagName == imageSelected[1] {
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)
// Update the Containers[i].Image value
notebook.Spec.Template.Spec.Containers[i].Image = imageHash
// Update the JUPYTER_IMAGE environment variable with the image selection for example "jupyter-datascience-notebook:2023.2"
for i, envVar := range container.Env {
if envVar.Name == "JUPYTER_IMAGE" {
container.Env[i].Value = imageSelection
break
}
}
imagestreamFound = true
break
}
}
}
}
}
if imagestreamFound {
break
}
}
if !imagestreamFound {
log.Error(nil, "Imagestream not found in any of the specified namespaces", "imageSelected", imageSelected[0], "tag", imageSelected[1])
}
}
}
}
if !containerFound {
log.Error(nil, "No container found matching the notebook name", "notebookName", notebook.Name)
}
}
}
return nil
}
16 changes: 15 additions & 1 deletion components/odh-notebook-controller/e2e/helper.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,12 @@ package e2e
import (
"crypto/tls"
"fmt"
netv1 "k8s.io/api/networking/v1"
"log"
"net/http"
"time"

netv1 "k8s.io/api/networking/v1"

nbv1 "github.com/kubeflow/kubeflow/components/notebook-controller/api/v1"
routev1 "github.com/openshift/api/route/v1"
appsv1 "k8s.io/api/apps/v1"
Expand Down Expand Up @@ -228,3 +229,16 @@ func setupThothMinimalOAuthNotebook() notebookContext {
}
return thothMinimalOAuthNbContext
}

// Setting func async to the upstream branch v1.7-branch,
// as servicemesh changes have not been moved stable branch
func notebooksForScenario(notebooks []notebookContext, mode DeploymentMode) []notebookContext {
var filtered []notebookContext
for _, notebook := range notebooks {
if notebook.deploymentMode == mode {
filtered = append(filtered, notebook)
}
}

return filtered
}
Loading

0 comments on commit c1f19c4

Please sign in to comment.