Merge pull request #328 from opendatahub-io/v2.22.0-fixes

V2.22.0 fixes
red-hat-data-services · Apr 29, 2024 · 709fbd3 · 709fbd3
2 parents 883c004 + bc33d07
commit 709fbd3
Show file tree

Hide file tree

Showing 10 changed files with 267 additions and 130 deletions.
diff --git a/backend/src/routes/api/notebooks/utils.ts b/backend/src/routes/api/notebooks/utils.ts
@@ -73,8 +73,8 @@ export const enableNotebook = async (
   const url = request.headers.origin;
 
   try {
-    await getNotebook(fastify, notebookNamespace, name);
-    return await updateNotebook(fastify, username, url, notebookData);
+    const notebook = await getNotebook(fastify, notebookNamespace, name);
+    return await updateNotebook(fastify, username, url, notebookData, notebook);
   } catch (e) {
     if (e.response?.statusCode === 404) {
       return await createNotebook(fastify, username, url, notebookData);

diff --git a/backend/src/utils/notebookUtils.ts b/backend/src/utils/notebookUtils.ts
@@ -1,4 +1,5 @@
 import { getDashboardConfig } from './resourceUtils';
+import { merge } from 'lodash';
 import {
   ContainerResourceAttributes,
   EnvironmentVariable,
@@ -510,6 +511,7 @@ export const updateNotebook = async (
   username: string,
   url: string,
   notebookData: NotebookData,
+  oldNotebook: Notebook,
 ): Promise<Notebook> => {
   if (!notebookData) {
     const error = createCustomError(
@@ -521,7 +523,29 @@ export const updateNotebook = async (
     throw error;
   }
   try {
-    const notebookAssembled = await generateNotebookResources(fastify, username, url, notebookData);
+    const serverNotebook = await generateNotebookResources(fastify, username, url, notebookData);
+
+    // Fix for Workbench Certs that get overridden
+    // We are intentionally applying on some details as to avoid implementing logic to properly
+    // manage the notebook the same way as workbench
+    const importantOldNotebookDetails: RecursivePartial<Notebook> = {
+      spec: {
+        template: {
+          spec: {
+            containers: [
+              {
+                env: oldNotebook.spec.template.spec.containers[0].env,
+                volumeMounts: oldNotebook.spec.template.spec.containers[0].volumeMounts,
+              },
+            ],
+            volumes: oldNotebook.spec.template.spec.volumes,
+          },
+        },
+      },
+    };
+
+    const notebookAssembled = merge({}, importantOldNotebookDetails, serverNotebook);
+
     const response = await fastify.kube.customObjectsApi.patchNamespacedCustomObject(
       'kubeflow.org',
       'v1',

diff --git a/frontend/src/__mocks__/mockDWUsageByOwnerPrometheusResponse.ts b/frontend/src/__mocks__/mockDWUsageByOwnerPrometheusResponse.ts
@@ -0,0 +1,16 @@
+import { WorkloadMetricIndexedByOwner, WorkloadMetricPromQueryResponse } from '~/api';
+import { WorkloadOwnerType } from '~/k8sTypes';
+import { mockPrometheusQueryVectorResponse } from './mockPrometheusQueryVectorResponse';
+
+export const mockDWUsageByOwnerPrometheusResponse = (
+  usageByOwner: WorkloadMetricIndexedByOwner,
+): WorkloadMetricPromQueryResponse =>
+  mockPrometheusQueryVectorResponse({
+    result: Object.values(WorkloadOwnerType).flatMap((ownerKind) =>
+      Object.keys(usageByOwner[ownerKind]).map((ownerName) => ({
+        // eslint-disable-next-line camelcase
+        metric: { owner_kind: ownerKind, owner_name: ownerName },
+        value: [0, String(usageByOwner[ownerKind][ownerName])],
+      })),
+    ),
+  });
diff --git a/frontend/src/__mocks__/mockWorkloadK8sResource.ts b/frontend/src/__mocks__/mockWorkloadK8sResource.ts
@@ -1,6 +1,6 @@
 import { genUID } from '~/__mocks__/mockUtils';
 import { WorkloadStatusType } from '~/concepts/distributedWorkloads/utils';
-import { WorkloadCondition, WorkloadKind, WorkloadPodSet } from '~/k8sTypes';
+import { WorkloadCondition, WorkloadKind, WorkloadOwnerType, WorkloadPodSet } from '~/k8sTypes';
 
 const mockWorkloadStatusConditions: Record<WorkloadStatusType, WorkloadCondition[]> = {
   Pending: [
@@ -127,14 +127,16 @@ const mockWorkloadStatusConditions: Record<WorkloadStatusType, WorkloadCondition
 type MockResourceConfigType = {
   k8sName?: string;
   namespace?: string;
-  ownerJobName?: string;
+  ownerKind?: WorkloadOwnerType;
+  ownerName?: string;
   mockStatus?: WorkloadStatusType | null;
   podSets?: WorkloadPodSet[];
 };
 export const mockWorkloadK8sResource = ({
   k8sName = 'test-workload',
   namespace = 'test-project',
-  ownerJobName,
+  ownerKind = WorkloadOwnerType.Job,
+  ownerName,
   mockStatus = WorkloadStatusType.Succeeded,
   podSets = [],
 }: MockResourceConfigType): WorkloadKind => ({
@@ -150,16 +152,16 @@ export const mockWorkloadK8sResource = ({
     namespace,
     resourceVersion: '9279356',
     uid: genUID('workload'),
-    ...(ownerJobName
+    ...(ownerName
       ? {
           ownerReferences: [
             {
               apiVersion: 'batch/v1',
               blockOwnerDeletion: true,
               controller: true,
-              kind: 'Job',
-              name: ownerJobName,
-              uid: genUID('job'),
+              kind: ownerKind,
+              name: ownerName,
+              uid: genUID(ownerKind.toLowerCase()),
             },
           ],
         }

diff --git a/...d/src/__tests__/cypress/cypress/e2e/distributedWorkloads/GlobalDistributedWorkloads.cy.ts b/...d/src/__tests__/cypress/cypress/e2e/distributedWorkloads/GlobalDistributedWorkloads.cy.ts
@@ -6,7 +6,7 @@ import { mockK8sResourceList } from '~/__mocks__/mockK8sResourceList';
 import { mockProjectK8sResource } from '~/__mocks__/mockProjectK8sResource';
 import { mockPrometheusQueryVectorResponse } from '~/__mocks__/mockPrometheusQueryVectorResponse';
 import { mockWorkloadK8sResource } from '~/__mocks__/mockWorkloadK8sResource';
-import { ClusterQueueKind, LocalQueueKind, WorkloadKind } from '~/k8sTypes';
+import { ClusterQueueKind, LocalQueueKind, WorkloadKind, WorkloadOwnerType } from '~/k8sTypes';
 import { WorkloadStatusType } from '~/concepts/distributedWorkloads/utils';
 import { mockClusterQueueK8sResource } from '~/__mocks__/mockClusterQueueK8sResource';
 import { mockLocalQueueK8sResource } from '~/__mocks__/mockLocalQueueK8sResource';
@@ -35,9 +35,16 @@ const initIntercepts = ({
     mockLocalQueueK8sResource({ name: 'test-local-queue', namespace: 'test-project' }),
   ],
   workloads = [
-    mockWorkloadK8sResource({ k8sName: 'test-workload', mockStatus: WorkloadStatusType.Succeeded }),
+    mockWorkloadK8sResource({
+      k8sName: 'test-workload',
+      ownerKind: WorkloadOwnerType.Job,
+      ownerName: 'test-workload-job',
+      mockStatus: WorkloadStatusType.Succeeded,
+    }),
     mockWorkloadK8sResource({
       k8sName: 'test-workload-2',
+      ownerKind: WorkloadOwnerType.RayCluster,
+      ownerName: 'test-workload-2-rc',
       mockStatus: WorkloadStatusType.Succeeded,
     }),
   ],