Handle deleted container size in workbenches

opendatahub-io · Jul 3, 2024 · 3704712 · 3704712
1 parent e687eb8
commit 3704712
Show file tree

Hide file tree

Showing 5 changed files with 146 additions and 92 deletions.
diff --git a/frontend/src/__mocks__/mockDashboardConfig.ts b/frontend/src/__mocks__/mockDashboardConfig.ts
@@ -1,4 +1,5 @@
 import { DashboardConfigKind, KnownLabels } from '~/k8sTypes';
+import { NotebookSize } from '~/types';
 
 type MockDashboardConfigType = {
   disableInfo?: boolean;
@@ -26,6 +27,7 @@ type MockDashboardConfigType = {
   disableDistributedWorkloads?: boolean;
   disableModelRegistry?: boolean;
   disableNotebookController?: boolean;
+  notebookSizes?: NotebookSize[];
 };
 
 export const mockDashboardConfig = ({
@@ -54,6 +56,73 @@ export const mockDashboardConfig = ({
   disableDistributedWorkloads = false,
   disableModelRegistry = true,
   disableNotebookController = false,
+  notebookSizes = [
+    {
+      name: 'XSmall',
+      resources: {
+        limits: {
+          cpu: '0.5',
+          memory: '500Mi',
+        },
+        requests: {
+          cpu: '0.1',
+          memory: '100Mi',
+        },
+      },
+    },
+    {
+      name: 'Small',
+      resources: {
+        limits: {
+          cpu: '2',
+          memory: '8Gi',
+        },
+        requests: {
+          cpu: '1',
+          memory: '8Gi',
+        },
+      },
+    },
+    {
+      name: 'Medium',
+      resources: {
+        limits: {
+          cpu: '6',
+          memory: '24Gi',
+        },
+        requests: {
+          cpu: '3',
+          memory: '24Gi',
+        },
+      },
+    },
+    {
+      name: 'Large',
+      resources: {
+        limits: {
+          cpu: '14',
+          memory: '56Gi',
+        },
+        requests: {
+          cpu: '7',
+          memory: '56Gi',
+        },
+      },
+    },
+    {
+      name: 'X Large',
+      resources: {
+        limits: {
+          cpu: '30',
+          memory: '120Gi',
+        },
+        requests: {
+          cpu: '15',
+          memory: '120Gi',
+        },
+      },
+    },
+  ],
 }: MockDashboardConfigType): DashboardConfigKind => ({
   apiVersion: 'opendatahub.io/v1alpha',
   kind: 'OdhDashboardConfig',
@@ -147,73 +216,7 @@ export const mockDashboardConfig = ({
         },
       },
     ],
-    notebookSizes: [
-      {
-        name: 'XSmall',
-        resources: {
-          limits: {
-            cpu: '0.5',
-            memory: '500Mi',
-          },
-          requests: {
-            cpu: '0.1',
-            memory: '100Mi',
-          },
-        },
-      },
-      {
-        name: 'Small',
-        resources: {
-          limits: {
-            cpu: '2',
-            memory: '8Gi',
-          },
-          requests: {
-            cpu: '1',
-            memory: '8Gi',
-          },
-        },
-      },
-      {
-        name: 'Medium',
-        resources: {
-          limits: {
-            cpu: '6',
-            memory: '24Gi',
-          },
-          requests: {
-            cpu: '3',
-            memory: '24Gi',
-          },
-        },
-      },
-      {
-        name: 'Large',
-        resources: {
-          limits: {
-            cpu: '14',
-            memory: '56Gi',
-          },
-          requests: {
-            cpu: '7',
-            memory: '56Gi',
-          },
-        },
-      },
-      {
-        name: 'X Large',
-        resources: {
-          limits: {
-            cpu: '30',
-            memory: '120Gi',
-          },
-          requests: {
-            cpu: '15',
-            memory: '120Gi',
-          },
-        },
-      },
-    ],
+    notebookSizes,
     templateOrder: ['test-model'],
     templateDisablement: ['test-model'],
   },

diff --git a/frontend/src/__tests__/cypress/cypress/tests/mocked/projects/workbench.cy.ts b/frontend/src/__tests__/cypress/cypress/tests/mocked/projects/workbench.cy.ts
@@ -1,4 +1,5 @@
 import {
+  mockDashboardConfig,
   mockDscStatus,
   mockK8sResourceList,
   mockNotebookK8sResource,
@@ -399,10 +400,66 @@ describe('Workbench page', () => {
     });
 
     cy.get('@editWorkbench.all').then((interceptions) => {
-      expect(interceptions).to.have.length(2); // 1 dry run request and 1 actaul request
+      expect(interceptions).to.have.length(2); // 1 dry run request and 1 actual request
     });
   });
 
+  it('Handle deleted notebook sizes in workbenches table', () => {
+    initIntercepts({});
+    cy.interceptOdh(
+      'GET /api/config',
+      mockDashboardConfig({
+        notebookSizes: [
+          {
+            name: 'Medium',
+            resources: {
+              limits: {
+                cpu: '6',
+                memory: '24Gi',
+              },
+              requests: {
+                cpu: '3',
+                memory: '24Gi',
+              },
+            },
+          },
+          {
+            name: 'Large',
+            resources: {
+              limits: {
+                cpu: '14',
+                memory: '56Gi',
+              },
+              requests: {
+                cpu: '7',
+                memory: '56Gi',
+              },
+            },
+          },
+          {
+            name: 'X Large',
+            resources: {
+              limits: {
+                cpu: '30',
+                memory: '120Gi',
+              },
+              requests: {
+                cpu: '15',
+                memory: '120Gi',
+              },
+            },
+          },
+        ],
+      }),
+    );
+    workbenchPage.visit('test-project');
+    const notebookRow = workbenchPage.getNotebookRow('Test Notebook');
+    notebookRow.shouldHaveNotebookImageName('Test Image');
+    notebookRow.shouldHaveContainerSize('Custom');
+    notebookRow.findKebabAction('Edit workbench').click();
+    editSpawnerPage.shouldHaveContainerSizeInput('Custom');
+  });
+
   it('Validate that updating invalid workbench will navigate to the new page with an error message', () => {
     initIntercepts({});
     notFoundSpawnerPage.visit('updated-notebook');

diff --git a/frontend/src/pages/projects/screens/detail/notebooks/NotebookTableRow.tsx b/frontend/src/pages/projects/screens/detail/notebooks/NotebookTableRow.tsx
@@ -1,17 +1,8 @@
 import * as React from 'react';
 import { ActionsColumn, ExpandableRowContent, Tbody, Td, Tr } from '@patternfly/react-table';
-import {
-  Button,
-  Flex,
-  FlexItem,
-  Icon,
-  Popover,
-  Split,
-  SplitItem,
-  Tooltip,
-} from '@patternfly/react-core';
+import { Button, Flex, FlexItem, Icon, Popover, Split, SplitItem } from '@patternfly/react-core';
 import { useNavigate } from 'react-router-dom';
-import { ExclamationCircleIcon, InfoCircleIcon } from '@patternfly/react-icons';
+import { InfoCircleIcon } from '@patternfly/react-icons';
 import { NotebookState } from '~/pages/projects/notebook/types';
 import NotebookRouteLink from '~/pages/projects/notebook/NotebookRouteLink';
 import NotebookStatusToggle from '~/pages/projects/notebook/NotebookStatusToggle';
@@ -51,7 +42,7 @@ const NotebookTableRow: React.FC<NotebookTableRowProps> = ({
   const { currentProject } = React.useContext(ProjectDetailsContext);
   const navigate = useNavigate();
   const [isExpanded, setExpanded] = React.useState(false);
-  const { size: notebookSize, error: sizeError } = useNotebookDeploymentSize(obj.notebook);
+  const { size: notebookSize } = useNotebookDeploymentSize(obj.notebook);
   const [notebookImage, loaded, loadError] = useNotebookImage(obj.notebook);
 
   return (
@@ -124,7 +115,7 @@ const NotebookTableRow: React.FC<NotebookTableRowProps> = ({
                   }
                 >
                   <DashboardPopupIconButton
-                    aria-label="Notebook image has out of date Elrya version"
+                    aria-label="Notebook image has out of date Elyra version"
                     data-testid="outdated-elyra-info"
                     icon={
                       <Icon status="info">
@@ -143,14 +134,7 @@ const NotebookTableRow: React.FC<NotebookTableRowProps> = ({
               spaceItems={{ default: 'spaceItemsXs' }}
               alignItems={{ default: 'alignItemsCenter' }}
             >
-              <FlexItem>{notebookSize?.name ?? 'Unknown'}</FlexItem>
-              {sizeError && (
-                <Tooltip content={sizeError}>
-                  <Icon aria-label="error icon" role="button" status="danger" tabIndex={0}>
-                    <ExclamationCircleIcon />
-                  </Icon>
-                </Tooltip>
-              )}
+              <FlexItem>{notebookSize?.name ?? 'Custom'}</FlexItem>
             </Flex>
           </Td>
         ) : null}

diff --git a/frontend/src/pages/projects/screens/spawner/SpawnerPage.tsx b/frontend/src/pages/projects/screens/spawner/SpawnerPage.tsx
@@ -11,7 +11,7 @@ import {
   StackItem,
 } from '@patternfly/react-core';
 import ApplicationsPage from '~/pages/ApplicationsPage';
-import { ImageStreamAndVersion } from '~/types';
+import { ImageStreamAndVersion, NotebookSize } from '~/types';
 import GenericSidebar from '~/components/GenericSidebar';
 import NameDescriptionField from '~/concepts/k8s/NameDescriptionField';
 import { ProjectDetailsContext } from '~/pages/projects/ProjectDetailsContext';
@@ -97,10 +97,16 @@ const SpawnerPage: React.FC<SpawnerPageProps> = ({ existingNotebook }) => {
 
   const { size: notebookSize } = useNotebookDeploymentSize(existingNotebook);
   React.useEffect(() => {
+    const customSize: NotebookSize = { name: 'Custom', resources: {} };
     if (notebookSize) {
       setSelectedSize(notebookSize.name);
+    } else {
+      if (existingNotebook && !sizes.some((size) => size.name === 'Custom')) {
+        sizes.unshift(customSize);
+      }
+      setSelectedSize(customSize.name);
     }
-  }, [notebookSize, setSelectedSize]);
+  }, [notebookSize, setSelectedSize, sizes, existingNotebook]);
 
   const [notebookAcceleratorProfileState, setNotebookAcceleratorProfileState] =
     useNotebookAcceleratorProfile(existingNotebook);

diff --git a/frontend/src/pages/projects/screens/spawner/deploymentSize/ContainerSizeSelector.tsx b/frontend/src/pages/projects/screens/spawner/deploymentSize/ContainerSizeSelector.tsx
@@ -35,7 +35,11 @@ const ContainerSizeSelector: React.FC<ContainerSizeSelectorProps> = ({
         {sizes.map((size) => {
           const { name } = size;
           const desc = getSizeDescription(size);
-          return <SelectOption key={name} value={name} description={desc} />;
+          return name === 'Custom' ? (
+            <SelectOption key={name} value={name} />
+          ) : (
+            <SelectOption key={name} value={name} description={desc} />
+          );
         })}
       </Select>