Fix types in accelerator spawner submit (#3422)

* Fix types in accelerator spawner submit * make types more strict * update test * update backend types * notebookUtils type fix * fix test * lint
opendatahub-io · Nov 6, 2024 · 9b98188 · 9b98188
1 parent 9242b81
commit 9b98188
Show file tree

Hide file tree

Showing 6 changed files with 68 additions and 17 deletions.
diff --git a/backend/src/types.ts b/backend/src/types.ts
@@ -834,15 +834,15 @@ export type NotebookData = {
   notebookSizeName: string;
   imageName: string;
   imageTagName: string;
-  acceleratorProfile: AcceleratorProfileState;
+  acceleratorProfile?: AcceleratorProfileState;
   envVars: EnvVarReducedTypeKeyValues;
   state: NotebookState;
   username?: string;
   storageClassName?: string;
 };
 
 export type AcceleratorProfileState = {
-  acceleratorProfile?: AcceleratorProfileKind;
+  acceleratorProfile: AcceleratorProfileKind;
   count: number;
 };
 

diff --git a/backend/src/utils/notebookUtils.ts b/backend/src/utils/notebookUtils.ts
@@ -160,7 +160,7 @@ export const assembleNotebook = async (
   envName: string,
   tolerationSettings: NotebookTolerationSettings,
 ): Promise<Notebook> => {
-  const { notebookSizeName, imageName, imageTagName, acceleratorProfile, envVars } = data;
+  const { notebookSizeName, imageName, imageTagName, envVars } = data;
 
   const notebookSize = getNotebookSize(notebookSizeName);
 
@@ -196,17 +196,17 @@ export const assembleNotebook = async (
   const tolerations: Toleration[] = [];
 
   const affinity: NotebookAffinity = {};
-  if (acceleratorProfile.count > 0 && acceleratorProfile.acceleratorProfile) {
+  if (data.acceleratorProfile?.count > 0) {
     if (!resources.limits) {
       resources.limits = {};
     }
     if (!resources.requests) {
       resources.requests = {};
     }
-    resources.limits[acceleratorProfile.acceleratorProfile.spec.identifier] =
-      acceleratorProfile.count;
-    resources.requests[acceleratorProfile.acceleratorProfile.spec.identifier] =
-      acceleratorProfile.count;
+    resources.limits[data.acceleratorProfile.acceleratorProfile.spec.identifier] =
+      data.acceleratorProfile.count;
+    resources.requests[data.acceleratorProfile.acceleratorProfile.spec.identifier] =
+      data.acceleratorProfile.count;
   } else {
     // step type down to string to avoid type errors
     const containerResourceKeys: string[] = Object.values(ContainerResourceAttributes);
@@ -224,8 +224,8 @@ export const assembleNotebook = async (
     });
   }
 
-  if (acceleratorProfile.acceleratorProfile?.spec.tolerations) {
-    tolerations.push(...acceleratorProfile.acceleratorProfile.spec.tolerations);
+  if (data.acceleratorProfile?.acceleratorProfile?.spec.tolerations) {
+    tolerations.push(...data.acceleratorProfile.acceleratorProfile.spec.tolerations);
   }
 
   if (tolerationSettings?.enabled) {
@@ -274,7 +274,7 @@ export const assembleNotebook = async (
         'opendatahub.io/username': username,
         'kubeflow-resource-stopped': null,
         'opendatahub.io/accelerator-name':
-          acceleratorProfile.acceleratorProfile?.metadata.name || '',
+          data.acceleratorProfile?.acceleratorProfile.metadata.name || '',
       },
       name: name,
       namespace: namespace,

diff --git a/frontend/src/__tests__/cypress/cypress/pages/notebookServer.ts b/frontend/src/__tests__/cypress/cypress/pages/notebookServer.ts
@@ -40,6 +40,10 @@ class NotebookServer {
   findStopNotebookServerButton() {
     return cy.findByTestId('stop-nb-server-button');
   }
+
+  findAcceleratorProfileSelect() {
+    return cy.findByTestId('accelerator-profile-select');
+  }
 }
 
 export const notebookServer = new NotebookServer();
diff --git a/frontend/src/__tests__/cypress/cypress/tests/mocked/applications/notebookServer.cy.ts b/frontend/src/__tests__/cypress/cypress/tests/mocked/applications/notebookServer.cy.ts
@@ -16,7 +16,11 @@ import {
   stopNotebookModal,
 } from '~/__tests__/cypress/cypress/pages/administration';
 import { homePage } from '~/__tests__/cypress/cypress/pages/home/home';
-import { StorageClassModel } from '~/__tests__/cypress/cypress/utils/models';
+import {
+  AcceleratorProfileModel,
+  StorageClassModel,
+} from '~/__tests__/cypress/cypress/utils/models';
+import { mockAcceleratorProfile } from '~/__mocks__/mockAcceleratorProfile';
 
 const groupSubjects: RoleBindingSubject[] = [
   {
@@ -45,7 +49,17 @@ const initIntercepts = () => {
       disableStorageClasses: false,
     }),
   );
-
+  cy.interceptK8sList(
+    AcceleratorProfileModel,
+    mockK8sResourceList([
+      mockAcceleratorProfile({
+        name: 'test-gpu',
+        displayName: 'Test GPU',
+        namespace: 'opendatahub',
+        uid: 'uid',
+      }),
+    ]),
+  );
   cy.interceptK8sList(StorageClassModel, mockStorageClassList());
 };
 
@@ -75,7 +89,35 @@ describe('NotebookServer', () => {
         notebookSizeName: 'XSmall',
         imageName: 'code-server-notebook',
         imageTagName: '2023.2',
-        acceleratorProfile: { count: 0, useExistingSettings: false },
+        envVars: { configMap: {}, secrets: {} },
+        state: 'started',
+        storageClassName: 'openshift-default-sc',
+      });
+    });
+  });
+
+  it('should start notebook server with accelerator profile', () => {
+    notebookServer.visit();
+    notebookServer.findAcceleratorProfileSelect().click();
+    notebookServer.findAcceleratorProfileSelect().findSelectOption('Test GPU').click();
+    notebookServer.findAcceleratorProfileSelect().should('contain', 'Test GPU');
+    notebookServer.findStartServerButton().should('be.visible');
+    notebookServer.findStartServerButton().click();
+
+    cy.wait('@startNotebookServer').then((interception) => {
+      expect(interception.request.body).to.eql({
+        notebookSizeName: 'XSmall',
+        imageName: 'code-server-notebook',
+        imageTagName: '2023.2',
+        acceleratorProfile: {
+          count: 1,
+          acceleratorProfile: mockAcceleratorProfile({
+            name: 'test-gpu',
+            displayName: 'Test GPU',
+            namespace: 'opendatahub',
+            uid: 'uid',
+          }),
+        },
         envVars: { configMap: {}, secrets: {} },
         state: 'started',
         storageClassName: 'openshift-default-sc',

diff --git a/frontend/src/pages/notebookController/screens/server/SpawnerPage.tsx b/frontend/src/pages/notebookController/screens/server/SpawnerPage.tsx
@@ -273,7 +273,12 @@ const SpawnerPage: React.FC = () => {
       notebookSizeName: selectedSize.name,
       imageName: selectedImageTag.image?.name || '',
       imageTagName: selectedImageTag.tag?.name || '',
-      acceleratorProfile: acceleratorProfileFormData,
+      acceleratorProfile: acceleratorProfileFormData.profile
+        ? {
+            acceleratorProfile: acceleratorProfileFormData.profile,
+            count: acceleratorProfileFormData.count,
+          }
+        : undefined,
       envVars,
       state: NotebookState.Started,
       username: impersonatedUsername || undefined,

diff --git a/frontend/src/types.ts b/frontend/src/types.ts
@@ -577,8 +577,8 @@ export type NotebookData = {
   notebookSizeName: string;
   imageName: string;
   imageTagName: string;
-  acceleratorProfile: {
-    acceleratorProfile?: AcceleratorProfileKind;
+  acceleratorProfile?: {
+    acceleratorProfile: AcceleratorProfileKind;
     count: number;
   };
   envVars: EnvVarReducedTypeKeyValues;