From 29765cfa7c37bb32aa5a641d01f325b4bd3369fd Mon Sep 17 00:00:00 2001 From: Yehudit Kerido Date: Wed, 4 Dec 2024 14:58:37 +0200 Subject: [PATCH 1/2] Redesign structs for workspacekind resource payloads Signed-off-by: Yehudit Kerido --- .vscode/settings.json | 3 + .../backend/internal/models/helper_test.go | 46 ++++++++++ .../backend/internal/models/pod_models.go | 58 +++---------- .../backend/internal/models/workspacekinds.go | 85 ++++--------------- 4 files changed, 78 insertions(+), 114 deletions(-) create mode 100644 .vscode/settings.json create mode 100644 workspaces/backend/internal/models/helper_test.go diff --git a/.vscode/settings.json b/.vscode/settings.json new file mode 100644 index 00000000..ad92582b --- /dev/null +++ b/.vscode/settings.json @@ -0,0 +1,3 @@ +{ + "editor.formatOnSave": true +} diff --git a/workspaces/backend/internal/models/helper_test.go b/workspaces/backend/internal/models/helper_test.go new file mode 100644 index 00000000..a074d3e0 --- /dev/null +++ b/workspaces/backend/internal/models/helper_test.go @@ -0,0 +1,46 @@ +package models + +import ( + "testing" +) + +func TestGetOrDefaultWithRecovery(t *testing.T) { + // Test case 1: Value is non-nil, should return the value + nonNilValue := 42 + result := GetOrDefaultWithRecovery(&nonNilValue, 0) + if result != 42 { + t.Errorf("Expected 42, got %d", result) + } + + // Test case 2: Value is nil, should return the default value + result = GetOrDefaultWithRecovery[int](nil, 99) + if result != 99 { + t.Errorf("Expected 99, got %d", result) + } + + // Test case 3: Simulate a panic and ensure recovery works + defer func() { + if r := recover(); r != nil { + t.Errorf("Function did not handle panic as expected") + } + }() + result = GetOrDefaultWithRecovery[int](nil, 123) // Should not panic + if result != 123 { + t.Errorf("Expected 123, got %d", result) + } + + // Test case 4-5: Value is one of models structs. + dummy_image_config1 := ImageConfigValue{ + Id: "jupyterlab_scipy_180", + DisplayName: "JupyterLab, with SciPy Packages", + Labels: map[string]string{"python_version": "3.11"}, + } + hidden_result := GetOrDefaultWithRecovery(dummy_image_config1.Hidden, false) + if hidden_result { + t.Errorf("Expected false, got %v", hidden_result) + } + id_result := GetOrDefaultWithRecovery(&dummy_image_config1.Id, "dummy_id") + if id_result != "jupyterlab_scipy_180" { + t.Errorf("Expected 'jupyterlab_scipy_180', got %v", id_result) + } +} diff --git a/workspaces/backend/internal/models/pod_models.go b/workspaces/backend/internal/models/pod_models.go index 54e72d7c..33fa7feb 100644 --- a/workspaces/backend/internal/models/pod_models.go +++ b/workspaces/backend/internal/models/pod_models.go @@ -1,7 +1,5 @@ package models -import v1 "k8s.io/api/core/v1" - type WorkspaceKindPodMetadata struct { Labels map[string]string `json:"labels"` Annotations map[string]string `json:"annotations"` @@ -13,34 +11,11 @@ type ImageConfig struct { } type ImageConfigValue struct { - Id string `json:"id"` - Spawner OptionSpawnerInfo `json:"spawner"` - Redirect *OptionRedirect `json:"redirect,omitempty"` - Spec ImageConfigSpec `json:"spec"` -} -type OptionSpawnerInfo struct { - DisplayName string `json:"displayName"` - Description *string `json:"description,omitempty"` - Labels []OptionSpawnerLabel `json:"labels,omitempty"` - Hidden *bool `json:"hidden,omitempty"` -} - -type OptionSpawnerLabel struct { - Key string `json:"key"` - Value string `json:"value"` -} - -type ImageConfigSpec struct { - Image string `json:"image"` - ImagePullPolicy string `json:"imagePullPolicy"` - Ports []ImagePort `json:"ports"` -} - -type ImagePort struct { - Id string `json:"id"` - Port int32 `json:"port"` - DisplayName string `json:"displayName"` - Protocol string `json:"protocol"` + Id string `json:"id"` + DisplayName string `json:"displayName"` + Labels map[string]string `json:"labels,omitempty"` + Hidden *bool `json:"hidden,omitempty"` + Redirect *OptionRedirect `json:"redirect,omitempty"` } type PodConfig struct { @@ -49,27 +24,20 @@ type PodConfig struct { } type PodConfigValue struct { - Id string `json:"id"` - Spawner OptionSpawnerInfo `json:"spawner"` - Redirect *OptionRedirect `json:"redirect,omitempty"` - Spec PodConfigSpec `json:"spec"` + Id string `json:"id"` + DisplayName string `json:"displayName"` + Description string `json:"description"` + Labels map[string]string `json:"labels,omitempty"` } type OptionRedirect struct { - To string `json:"to"` - Message *RedirectMessage `json:"message,omitempty"` + To string `json:"to"` + Message *Message `json:"message,omitempty"` } -type RedirectMessage struct { - Level string `json:"level"` +type Message struct { Text string `json:"text"` -} - -type PodConfigSpec struct { - Affinity *v1.Affinity `json:"affinity,omitempty"` - NodeSelector map[string]string `json:"nodeSelector,omitempty"` - Tolerations []v1.Toleration `json:"tolerations,omitempty"` - Resources *v1.ResourceRequirements `json:"resources,omitempty"` + Level string `json:"level"` } type WorkspaceKindPodOptions struct { diff --git a/workspaces/backend/internal/models/workspacekinds.go b/workspaces/backend/internal/models/workspacekinds.go index 87a2069f..18d06f51 100644 --- a/workspaces/backend/internal/models/workspacekinds.go +++ b/workspaces/backend/internal/models/workspacekinds.go @@ -20,7 +20,7 @@ package models import ( kubefloworgv1beta1 "github.com/kubeflow/notebooks/workspaces/controller/api/v1beta1" - v1 "k8s.io/api/core/v1" + // v1 "k8s.io/api/core/v1" ) type WorkspaceKindModel struct { @@ -39,49 +39,28 @@ func BuildImageConfigValues(item *kubefloworgv1beta1.WorkspaceKind) []ImageConfi imageConfigValues := []ImageConfigValue{} if item.Spec.PodTemplate.Options.ImageConfig.Values != nil { for _, item := range item.Spec.PodTemplate.Options.ImageConfig.Values { - labels := []OptionSpawnerLabel{} + labels := map[string]string{} for _, label := range item.Spawner.Labels { - labels = append(labels, OptionSpawnerLabel{ - Key: label.Key, - Value: label.Value, - }) - } - - ports := []ImagePort{} - for _, port := range item.Spec.Ports { - ports = append(ports, ImagePort{ - Id: port.Id, - DisplayName: port.DisplayName, - Port: port.Port, - Protocol: string(port.Protocol), - }) + labels[label.Key] = label.Value } var redirect *OptionRedirect if item.Redirect != nil { redirect = &OptionRedirect{ To: item.Redirect.To, - Message: &RedirectMessage{ - Level: string(item.Redirect.Message.Level), + Message: &Message{ Text: item.Redirect.Message.Text, + Level: string(item.Redirect.Message.Level), }, } } imageConfigValues = append(imageConfigValues, ImageConfigValue{ - Id: item.Id, - Spawner: OptionSpawnerInfo{ - DisplayName: item.Spawner.DisplayName, - Description: item.Spawner.Description, - Labels: labels, - Hidden: item.Spawner.Hidden, - }, - Redirect: redirect, - Spec: ImageConfigSpec{ - Image: item.Spec.Image, - ImagePullPolicy: string(*item.Spec.ImagePullPolicy), - Ports: ports, - }, + Id: item.Id, + DisplayName: item.Spawner.DisplayName, + Labels: labels, + Hidden: item.Spawner.Hidden, + Redirect: redirect, }) } } @@ -92,48 +71,16 @@ func BuildPodConfigValues(item *kubefloworgv1beta1.WorkspaceKind) []PodConfigVal podConfigValues := []PodConfigValue{} if item.Spec.PodTemplate.Options.PodConfig.Values != nil { for _, item := range item.Spec.PodTemplate.Options.PodConfig.Values { - labels := []OptionSpawnerLabel{} + labels := map[string]string{} for _, label := range item.Spawner.Labels { - labels = append(labels, OptionSpawnerLabel{ - Key: label.Key, - Value: label.Value, - }) - } - - var redirect *OptionRedirect - if item.Redirect != nil { - redirect = &OptionRedirect{ - To: item.Redirect.To, - Message: &RedirectMessage{ - Level: string(item.Redirect.Message.Level), - Text: item.Redirect.Message.Text, - }, - } - } - - tolerations := []v1.Toleration{} - for _, toleration := range item.Spec.Tolerations { - tolerations = append(tolerations, v1.Toleration{ - Key: toleration.Key, - Operator: toleration.Operator, - Effect: toleration.Effect, - }) + labels[label.Key] = label.Value } podConfigValues = append(podConfigValues, PodConfigValue{ - Id: item.Id, - Spawner: OptionSpawnerInfo{ - DisplayName: item.Spawner.DisplayName, - Description: item.Spawner.Description, - Labels: labels, - Hidden: item.Spawner.Hidden, - }, - Redirect: redirect, - Spec: PodConfigSpec{ - Affinity: item.Spec.Affinity, - NodeSelector: item.Spec.NodeSelector, - Tolerations: tolerations, - }, + Id: item.Id, + DisplayName: item.Spawner.DisplayName, + Description: *item.Spawner.Description, + Labels: labels, }) } } From e4156bb932d7cf88442aef9ab37ca4916d09d2cc Mon Sep 17 00:00:00 2001 From: yehudit1987 <34643974+yehudit1987@users.noreply.github.com> Date: Wed, 4 Dec 2024 15:02:13 +0200 Subject: [PATCH 2/2] Redesign structs for workspacekind resource payloads Signed-off-by: yehudit1987 <34643974+yehudit1987@users.noreply.github.com> --- .vscode/settings.json | 3 --- 1 file changed, 3 deletions(-) delete mode 100644 .vscode/settings.json diff --git a/.vscode/settings.json b/.vscode/settings.json deleted file mode 100644 index ad92582b..00000000 --- a/.vscode/settings.json +++ /dev/null @@ -1,3 +0,0 @@ -{ - "editor.formatOnSave": true -}