From 5bc2a17b671e742c06dca6966b337f102450a728 Mon Sep 17 00:00:00 2001 From: mohamedch7 <121046693+mohamedch7@users.noreply.github.com> Date: Wed, 15 Jan 2025 21:35:39 +0100 Subject: [PATCH] fix(ws): Move NewWorkspaceModelFromWorkspace from models to repositories Signed-off-by: mohamedch7 <121046693+mohamedch7@users.noreply.github.com> --- .../backend/api/workspaces_handler_test.go | 10 +- .../backend/internal/models/workspaces.go | 92 ------------------ .../internal/repositories/workspaces.go | 96 ++++++++++++++++++- 3 files changed, 97 insertions(+), 101 deletions(-) diff --git a/workspaces/backend/api/workspaces_handler_test.go b/workspaces/backend/api/workspaces_handler_test.go index 70a911f7..37830883 100644 --- a/workspaces/backend/api/workspaces_handler_test.go +++ b/workspaces/backend/api/workspaces_handler_test.go @@ -205,9 +205,9 @@ var _ = Describe("Workspaces Handler", func() { Expect(k8sClient.Get(ctx, types.NamespacedName{Name: workspaceKindName}, workspaceKind)).To(Succeed()) expectedWorkspaces := []models.WorkspaceModel{ - models.NewWorkspaceModelFromWorkspace(workspace1, workspaceKind), - models.NewWorkspaceModelFromWorkspace(workspace2, workspaceKind), - models.NewWorkspaceModelFromWorkspace(workspace3, workspaceKind), + repositories.NewWorkspaceModelFromWorkspace(workspace1, workspaceKind), + repositories.NewWorkspaceModelFromWorkspace(workspace2, workspaceKind), + repositories.NewWorkspaceModelFromWorkspace(workspace3, workspaceKind), } Expect(workspaces).To(ConsistOf(expectedWorkspaces)) @@ -262,8 +262,8 @@ var _ = Describe("Workspaces Handler", func() { Expect(k8sClient.Get(ctx, types.NamespacedName{Name: workspaceKindName}, workspaceKind)).To(Succeed()) expectedWorkspaces := []models.WorkspaceModel{ - models.NewWorkspaceModelFromWorkspace(workspace1, workspaceKind), - models.NewWorkspaceModelFromWorkspace(workspace2, workspaceKind), + repositories.NewWorkspaceModelFromWorkspace(workspace1, workspaceKind), + repositories.NewWorkspaceModelFromWorkspace(workspace2, workspaceKind), } Expect(workspaces).To(ConsistOf(expectedWorkspaces)) diff --git a/workspaces/backend/internal/models/workspaces.go b/workspaces/backend/internal/models/workspaces.go index 220bd1b4..2ac0cbf2 100644 --- a/workspaces/backend/internal/models/workspaces.go +++ b/workspaces/backend/internal/models/workspaces.go @@ -16,10 +16,6 @@ limitations under the License. package models -import ( - kubefloworgv1beta1 "github.com/kubeflow/notebooks/workspaces/controller/api/v1beta1" -) - type WorkspaceModel struct { Name string `json:"name"` Namespace string `json:"namespace"` @@ -88,91 +84,3 @@ type DataVolumeModel struct { MountPath string `json:"mount_path"` ReadOnly bool `json:"read_only"` } - -func NewWorkspaceModelFromWorkspace(item *kubefloworgv1beta1.Workspace, wsk *kubefloworgv1beta1.WorkspaceKind) WorkspaceModel { - dataVolumes := make([]DataVolumeModel, len(item.Spec.PodTemplate.Volumes.Data)) - for i, volume := range item.Spec.PodTemplate.Volumes.Data { - dataVolumes[i] = DataVolumeModel{ - PvcName: volume.PVCName, - MountPath: volume.MountPath, - ReadOnly: *volume.ReadOnly, - } - } - - imageConfigRedirectChain := make([]*RedirectChain, len(item.Status.PodTemplateOptions.ImageConfig.RedirectChain)) - for i, chain := range item.Status.PodTemplateOptions.ImageConfig.RedirectChain { - imageConfigRedirectChain[i] = &RedirectChain{ - Source: chain.Source, - Target: chain.Target, - } - } - - podConfigRedirectChain := make([]*RedirectChain, len(item.Status.PodTemplateOptions.PodConfig.RedirectChain)) - - for i, chain := range item.Status.PodTemplateOptions.PodConfig.RedirectChain { - podConfigRedirectChain[i] = &RedirectChain{ - Source: chain.Source, - Target: chain.Target, - } - } - - podMetadataLabels := item.Spec.PodTemplate.PodMetadata.Labels - if podMetadataLabels == nil { - podMetadataLabels = map[string]string{} - } - - podMetadataAnnotations := item.Spec.PodTemplate.PodMetadata.Annotations - if podMetadataAnnotations == nil { - podMetadataAnnotations = map[string]string{} - } - - workspaceModel := WorkspaceModel{ - Name: item.ObjectMeta.Name, - Namespace: item.Namespace, - WorkspaceKind: WorkspaceKind{ - Name: item.Spec.Kind, - Type: "POD_TEMPLATE", - }, - DeferUpdates: *item.Spec.DeferUpdates, - Paused: *item.Spec.Paused, - PausedTime: item.Status.PauseTime, - State: string(item.Status.State), - StateMessage: item.Status.StateMessage, - PodTemplate: PodTemplate{ - PodMetadata: &PodMetadata{ - Labels: podMetadataLabels, - Annotations: podMetadataAnnotations, - }, - Volumes: &Volumes{ - Home: &DataVolumeModel{ - PvcName: *item.Spec.PodTemplate.Volumes.Home, - MountPath: wsk.Spec.PodTemplate.VolumeMounts.Home, - ReadOnly: false, // From where to get this value? - }, - Data: dataVolumes, - }, - ImageConfig: &ImageConfig{ - Current: item.Spec.PodTemplate.Options.ImageConfig, - Desired: item.Status.PodTemplateOptions.ImageConfig.Desired, - RedirectChain: imageConfigRedirectChain, - }, - PodConfig: &PodConfig{ - Current: item.Spec.PodTemplate.Options.PodConfig, - Desired: item.Spec.PodTemplate.Options.PodConfig, - RedirectChain: podConfigRedirectChain, - }, - }, - Activity: Activity{ - LastActivity: item.Status.Activity.LastActivity, - LastUpdate: item.Status.Activity.LastUpdate, - // TODO: update these fields when the last probe is implemented - LastProbe: &Probe{ - StartTimeMs: 0, - EndTimeMs: 0, - Result: "default_result", - Message: "default_message", - }, - }, - } - return workspaceModel -} diff --git a/workspaces/backend/internal/repositories/workspaces.go b/workspaces/backend/internal/repositories/workspaces.go index 4d187c23..8bf25b75 100644 --- a/workspaces/backend/internal/repositories/workspaces.go +++ b/workspaces/backend/internal/repositories/workspaces.go @@ -54,7 +54,7 @@ func (r *WorkspaceRepository) GetWorkspace(ctx context.Context, namespace string return models.WorkspaceModel{}, err } - return models.NewWorkspaceModelFromWorkspace(workspace, kind), nil + return NewWorkspaceModelFromWorkspace(workspace, kind), nil } func (r *WorkspaceRepository) GetWorkspaces(ctx context.Context, namespace string) ([]models.WorkspaceModel, error) { @@ -73,7 +73,7 @@ func (r *WorkspaceRepository) GetWorkspaces(ctx context.Context, namespace strin if err := r.client.Get(ctx, client.ObjectKey{Name: item.Spec.Kind}, kind); err != nil { return nil, err } - workspacesModels[i] = models.NewWorkspaceModelFromWorkspace(&item, kind) + workspacesModels[i] = NewWorkspaceModelFromWorkspace(&item, kind) } return workspacesModels, nil @@ -91,7 +91,7 @@ func (r *WorkspaceRepository) GetAllWorkspaces(ctx context.Context) ([]models.Wo if err := r.client.Get(ctx, client.ObjectKey{Name: item.Spec.Kind}, kind); err != nil { return nil, err } - workspacesModels[i] = models.NewWorkspaceModelFromWorkspace(&item, kind) + workspacesModels[i] = NewWorkspaceModelFromWorkspace(&item, kind) } return workspacesModels, nil @@ -149,7 +149,7 @@ func (r *WorkspaceRepository) CreateWorkspace(ctx context.Context, workspaceMode return models.WorkspaceModel{}, err } - return models.NewWorkspaceModelFromWorkspace(workspace, kind), nil + return NewWorkspaceModelFromWorkspace(workspace, kind), nil } func (r *WorkspaceRepository) DeleteWorkspace(ctx context.Context, namespace, workspaceName string) error { @@ -169,3 +169,91 @@ func (r *WorkspaceRepository) DeleteWorkspace(ctx context.Context, namespace, wo return nil } + +func NewWorkspaceModelFromWorkspace(item *kubefloworgv1beta1.Workspace, wsk *kubefloworgv1beta1.WorkspaceKind) models.WorkspaceModel { + dataVolumes := make([]models.DataVolumeModel, len(item.Spec.PodTemplate.Volumes.Data)) + for i, volume := range item.Spec.PodTemplate.Volumes.Data { + dataVolumes[i] = models.DataVolumeModel{ + PvcName: volume.PVCName, + MountPath: volume.MountPath, + ReadOnly: *volume.ReadOnly, + } + } + + imageConfigRedirectChain := make([]*models.RedirectChain, len(item.Status.PodTemplateOptions.ImageConfig.RedirectChain)) + for i, chain := range item.Status.PodTemplateOptions.ImageConfig.RedirectChain { + imageConfigRedirectChain[i] = &models.RedirectChain{ + Source: chain.Source, + Target: chain.Target, + } + } + + podConfigRedirectChain := make([]*models.RedirectChain, len(item.Status.PodTemplateOptions.PodConfig.RedirectChain)) + + for i, chain := range item.Status.PodTemplateOptions.PodConfig.RedirectChain { + podConfigRedirectChain[i] = &models.RedirectChain{ + Source: chain.Source, + Target: chain.Target, + } + } + + podMetadataLabels := item.Spec.PodTemplate.PodMetadata.Labels + if podMetadataLabels == nil { + podMetadataLabels = map[string]string{} + } + + podMetadataAnnotations := item.Spec.PodTemplate.PodMetadata.Annotations + if podMetadataAnnotations == nil { + podMetadataAnnotations = map[string]string{} + } + + workspaceModel := models.WorkspaceModel{ + Name: item.ObjectMeta.Name, + Namespace: item.Namespace, + WorkspaceKind: models.WorkspaceKind{ + Name: item.Spec.Kind, + Type: "POD_TEMPLATE", + }, + DeferUpdates: *item.Spec.DeferUpdates, + Paused: *item.Spec.Paused, + PausedTime: item.Status.PauseTime, + State: string(item.Status.State), + StateMessage: item.Status.StateMessage, + PodTemplate: models.PodTemplate{ + PodMetadata: &models.PodMetadata{ + Labels: podMetadataLabels, + Annotations: podMetadataAnnotations, + }, + Volumes: &models.Volumes{ + Home: &models.DataVolumeModel{ + PvcName: *item.Spec.PodTemplate.Volumes.Home, + MountPath: wsk.Spec.PodTemplate.VolumeMounts.Home, + ReadOnly: false, // From where to get this value? + }, + Data: dataVolumes, + }, + ImageConfig: &models.ImageConfig{ + Current: item.Spec.PodTemplate.Options.ImageConfig, + Desired: item.Status.PodTemplateOptions.ImageConfig.Desired, + RedirectChain: imageConfigRedirectChain, + }, + PodConfig: &models.PodConfig{ + Current: item.Spec.PodTemplate.Options.PodConfig, + Desired: item.Spec.PodTemplate.Options.PodConfig, + RedirectChain: podConfigRedirectChain, + }, + }, + Activity: models.Activity{ + LastActivity: item.Status.Activity.LastActivity, + LastUpdate: item.Status.Activity.LastUpdate, + // TODO: update these fields when the last probe is implemented + LastProbe: &models.Probe{ + StartTimeMs: 0, + EndTimeMs: 0, + Result: "default_result", + Message: "default_message", + }, + }, + } + return workspaceModel +}