Refresh notebook status and fetch all status periodically (#697)

* refresh notebook status and fetch all status periodically * small fix for listening notebook * some rename and clean up * add input group for mount path and change placeholder text * refactor useRelatedNotebooks
opendatahub-io · Oct 26, 2022 · 1433957 · 1433957
1 parent 1822613
commit 1433957
Show file tree

Hide file tree

Showing 20 changed files with 181 additions and 165 deletions.
diff --git a/frontend/src/pages/projects/ProjectDetailsContext.tsx b/frontend/src/pages/projects/ProjectDetailsContext.tsx
@@ -1,5 +1,5 @@
 import * as React from 'react';
-import { PersistentVolumeClaimKind, ProjectKind, SecretKind } from '../../k8sTypes';
+import { PersistentVolumeClaimKind, ProjectKind } from '../../k8sTypes';
 import { Outlet, useParams } from 'react-router-dom';
 import {
   Bullseye,
@@ -18,6 +18,7 @@ import useProjectPvcs from './screens/detail/storage/useProjectPvcs';
 import useDataConnections from './screens/detail/data-connections/useDataConnections';
 import { DataConnection } from './types';
 import { NotebookState } from './notebook/types';
+import { POLL_INTERVAL } from '../../utilities/const';
 
 type ContextResourceData<T> = {
   data: T[];
@@ -42,6 +43,10 @@ const DEFAULT_DATA: ContextResourceData<never> = {
 
 const useContextResourceData = <T,>(resourceData): ContextResourceData<T> => {
   const [values, loaded, error, refresh] = resourceData;
+  React.useEffect(() => {
+    const timer = setInterval(() => refresh(), POLL_INTERVAL);
+    return () => clearInterval(timer);
+  }, [refresh]);
   return React.useMemo(
     () => ({
       data: values,

diff --git a/frontend/src/pages/projects/dataConnections/AWSField.tsx b/frontend/src/pages/projects/dataConnections/AWSField.tsx
@@ -19,6 +19,7 @@ const AWSField: React.FC<AWSFieldProps> = ({ values, onUpdate }) => {
       {Object.values(AWS_KEYS).map((value: AWS_KEYS) => (
         <StackItem key={value}>
           <AWSInputField
+            isPassword={value === AWS_KEYS.SECRET_ACCESS_KEY}
             isRequired={AWS_REQUIRED_KEYS.includes(value)}
             onChange={update}
             type={value}

diff --git a/frontend/src/pages/projects/notebook/ListNotebookState.tsx b/frontend/src/pages/projects/notebook/ListNotebookState.tsx
@@ -37,7 +37,7 @@ const ListNotebookState: React.FC<ListNotebookStateProps> = ({
       case 'notebook':
         return <NotebookRouteLink notebook={state.notebook} isRunning={state.isRunning} />;
       case 'status':
-        return <NotebookStatusToggle notebookState={state} />;
+        return <NotebookStatusToggle notebookState={state} doListen={true} />;
       default:
         console.error('Unknown show type', show);
         return <>-</>;

diff --git a/frontend/src/pages/projects/notebook/NotebookStatusToggle.tsx b/frontend/src/pages/projects/notebook/NotebookStatusToggle.tsx
@@ -9,14 +9,15 @@ import NotebookStatusPopover from './NotebookStatusPopover';
 
 type NotebookStatusToggleProps = {
   notebookState: NotebookState;
+  doListen: boolean;
 };
 
-const NotebookStatusToggle: React.FC<NotebookStatusToggleProps> = ({ notebookState }) => {
+const NotebookStatusToggle: React.FC<NotebookStatusToggleProps> = ({ notebookState, doListen }) => {
   const { notebook, isStarting, isRunning, refresh } = notebookState;
   const [isOpenConfirm, setOpenConfirm] = React.useState(false);
   const [inProgress, setInProgress] = React.useState(false);
   const [isPopoverVisible, setPopoverVisible] = React.useState(false);
-  const listenToNotebookStart = useRefreshNotebookUntilStart(notebookState);
+  const listenToNotebookStart = useRefreshNotebookUntilStart(notebookState, doListen);
   const [dontShowModalValue] = useStopNotebookModalAvailability();
   const notebookName = notebook.metadata.name;
   const notebookNamespace = notebook.metadata.namespace;

diff --git a/frontend/src/pages/projects/notebook/useRefreshNotebookUntilStart.ts b/frontend/src/pages/projects/notebook/useRefreshNotebookUntilStart.ts
@@ -4,14 +4,15 @@ import { NotebookState } from './types';
 
 const useRefreshNotebookUntilStart = (
   notebookState: NotebookState,
+  doListen: boolean,
 ): ((listen: boolean) => void) => {
   const [watchingForNotebook, setWatchingForNotebook] = React.useState(false);
   const lastNotebookState = React.useRef<NotebookState>(notebookState);
   lastNotebookState.current = notebookState;
 
   React.useEffect(() => {
     let interval;
-    if (watchingForNotebook) {
+    if (watchingForNotebook && doListen) {
       interval = setInterval(() => {
         const { isRunning, refresh } = lastNotebookState.current;
         if (!isRunning) {
@@ -28,7 +29,7 @@ const useRefreshNotebookUntilStart = (
     return () => {
       clearInterval(interval);
     };
-  }, [watchingForNotebook]);
+  }, [watchingForNotebook, doListen]);
 
   return React.useCallback((listen: boolean) => {
     setWatchingForNotebook(listen);

diff --git a/frontend/src/pages/projects/notebook/useRelatedNotebooks.ts b/frontend/src/pages/projects/notebook/useRelatedNotebooks.ts
@@ -16,35 +16,40 @@ const useRelatedNotebooks = (
     notebooks: { data, loaded, error },
   } = React.useContext(ProjectDetailsContext);
 
-  if (!resourceName) {
-    return { connectedNotebooks: [], loaded: false, error };
-  }
-
-  let connectedNotebooks: NotebookKind[];
-  switch (context) {
-    case ConnectedNotebookContext.PVC:
-      connectedNotebooks = data.reduce<NotebookKind[]>((acc, { notebook }) => {
-        const relatedPVCNames = getNotebookPVCNames(notebook);
-        if (!relatedPVCNames.includes(resourceName)) {
-          return acc;
-        }
-
-        return [...acc, notebook];
-      }, []);
-      break;
-    case ConnectedNotebookContext.DATA_CONNECTION:
-      connectedNotebooks = data.reduce<NotebookKind[]>((acc, { notebook }) => {
-        const relatedEnvs = getNotebookSecretNames(notebook);
-        if (!relatedEnvs.includes(resourceName)) {
-          return acc;
-        }
-
-        return [...acc, notebook];
-      }, []);
-      break;
-    default:
-      connectedNotebooks = [];
-  }
+  const [connectedNotebooks, setConnectedNotebooks] = React.useState<NotebookKind[]>([]);
+
+  React.useEffect(() => {
+    if (resourceName) {
+      switch (context) {
+        case ConnectedNotebookContext.PVC:
+          setConnectedNotebooks(
+            data.reduce<NotebookKind[]>((acc, { notebook }) => {
+              const relatedPVCNames = getNotebookPVCNames(notebook);
+              if (!relatedPVCNames.includes(resourceName)) {
+                return acc;
+              }
+
+              return [...acc, notebook];
+            }, []),
+          );
+          break;
+        case ConnectedNotebookContext.DATA_CONNECTION:
+          setConnectedNotebooks(
+            data.reduce<NotebookKind[]>((acc, { notebook }) => {
+              const relatedEnvs = getNotebookSecretNames(notebook);
+              if (!relatedEnvs.includes(resourceName)) {
+                return acc;
+              }
+
+              return [...acc, notebook];
+            }, []),
+          );
+          break;
+        default:
+          setConnectedNotebooks([]);
+      }
+    }
+  }, [context, resourceName, data]);
 
   return {
     connectedNotebooks,

diff --git a/frontend/src/pages/projects/notebook/utils.ts b/frontend/src/pages/projects/notebook/utils.ts
@@ -3,7 +3,10 @@ import { EventStatus, NotebookStatus } from '../../../types';
 import { useWatchNotebookEvents } from './useWatchNotebookEvents';
 
 export const hasStopAnnotation = (notebook: NotebookKind): boolean => {
-  return !!notebook.metadata.annotations?.['kubeflow-resource-stopped'];
+  return !!(
+    notebook.metadata.annotations?.['kubeflow-resource-stopped'] &&
+    notebook.metadata.annotations['kubeflow-resource-stopped'] !== 'odh-notebook-controller-lock'
+  );
 };
 
 export const getNotebookMountPaths = (notebook?: NotebookKind): string[] => {

diff --git a/frontend/src/pages/projects/pvc/MountPathField.tsx b/frontend/src/pages/projects/pvc/MountPathField.tsx
@@ -1,5 +1,5 @@
 import * as React from 'react';
-import { FormGroup, TextInput } from '@patternfly/react-core';
+import { FormGroup, InputGroup, InputGroupText, TextInput } from '@patternfly/react-core';
 import { MountPath } from '../types';
 
 type MountPathFieldProps = {
@@ -21,24 +21,27 @@ const MountPathField: React.FC<MountPathFieldProps> = ({
       label="Mount folder"
       validated={mountPath.error ? 'error' : 'success'}
     >
-      <TextInput
-        isRequired
-        aria-label="mount-path-folder-value"
-        type="text"
-        value={mountPath.value}
-        placeholder="data"
-        onChange={(value) => {
-          let error = '';
-          if (value.length === 0) {
-            error = 'Required';
-          } else if (!/^[a-z-]+$/.test(value)) {
-            error = 'Must only consist of lower case letters and dashes';
-          } else if (inUseMountPaths.includes(value)) {
-            error = 'Mount folder is already in use for this workbench';
-          }
-          setMountPath({ value, error });
-        }}
-      />
+      <InputGroup>
+        <InputGroupText variant="plain">/</InputGroupText>
+        <TextInput
+          isRequired
+          aria-label="mount-path-folder-value"
+          type="text"
+          value={mountPath.value}
+          placeholder="eg. data"
+          onChange={(value) => {
+            let error = '';
+            if (value.length === 0) {
+              error = 'Required';
+            } else if (!/^[a-z-]+$/.test(value)) {
+              error = 'Must only consist of lower case letters and dashes';
+            } else if (inUseMountPaths.includes(value)) {
+              error = 'Mount folder is already in use for this workbench';
+            }
+            setMountPath({ value, error });
+          }}
+        />
+      </InputGroup>
     </FormGroup>
   );
 };

diff --git a/frontend/src/pages/projects/screens/detail/ProjectDetails.tsx b/frontend/src/pages/projects/screens/detail/ProjectDetails.tsx
@@ -7,7 +7,7 @@ import DataConnectionsList from './data-connections/DataConnectionsList';
 import GenericSidebar from '../../components/GenericSidebar';
 import StorageList from './storage/StorageList';
 import { ProjectSectionID } from './types';
-import NotebooksList from './notebooks/NotebooksList';
+import NotebookList from './notebooks/NotebookList';
 import { ProjectDetailsContext } from '../../ProjectDetailsContext';
 import { getProjectDescription, getProjectDisplayName } from '../../utils';
 
@@ -23,8 +23,8 @@ const ProjectDetails: React.FC = () => {
 
   const scrollableSelectorID = 'project-details-list';
   const sections: SectionType[] = [
-    { id: ProjectSectionID.WORKBENCHES, component: <NotebooksList /> },
-    { id: ProjectSectionID.STORAGES, component: <StorageList /> },
+    { id: ProjectSectionID.WORKBENCHES, component: <NotebookList /> },
+    { id: ProjectSectionID.CLUSTER_STORAGES, component: <StorageList /> },
     { id: ProjectSectionID.DATA_CONNECTIONS, component: <DataConnectionsList /> },
   ];
 

diff --git a/frontend/src/pages/projects/screens/detail/const.ts b/frontend/src/pages/projects/screens/detail/const.ts
@@ -2,6 +2,6 @@ import { ProjectSectionID, ProjectSectionTitlesType } from './types';
 
 export const ProjectSectionTitles: ProjectSectionTitlesType = {
   [ProjectSectionID.WORKBENCHES]: 'Workbenches',
-  [ProjectSectionID.STORAGES]: 'Storages',
+  [ProjectSectionID.CLUSTER_STORAGES]: 'Cluster storages',
   [ProjectSectionID.DATA_CONNECTIONS]: 'Data connections',
 };
diff --git a/...creens/detail/notebooks/NotebooksList.tsx → ...screens/detail/notebooks/NotebookList.tsx b/...creens/detail/notebooks/NotebooksList.tsx → ...screens/detail/notebooks/NotebookList.tsx
@@ -7,15 +7,24 @@ import { ProjectSectionTitles } from '../const';
 import { ProjectDetailsContext } from '../../../ProjectDetailsContext';
 import { useNavigate } from 'react-router-dom';
 import NotebookTable from './NotebookTable';
+import { FAST_POLL_INTERVAL } from '../../../../../utilities/const';
 
-const NotebooksList: React.FC = () => {
+const NotebookList: React.FC = () => {
   const {
     currentProject,
     notebooks: { data: notebookStates, loaded, error: loadError, refresh: refreshNotebooks },
   } = React.useContext(ProjectDetailsContext);
   const navigate = useNavigate();
   const projectName = currentProject.metadata.name;
 
+  React.useEffect(() => {
+    let interval;
+    if (notebookStates.some((notebookState) => notebookState.isStarting)) {
+      interval = setInterval(() => refreshNotebooks(), FAST_POLL_INTERVAL);
+    }
+    return () => clearInterval(interval);
+  }, [notebookStates, refreshNotebooks]);
+
   return (
     <DetailsSection
       id={ProjectSectionID.WORKBENCHES}
@@ -45,4 +54,4 @@ const NotebooksList: React.FC = () => {
   );
 };
 
-export default NotebooksList;
+export default NotebookList;
diff --git a/frontend/src/pages/projects/screens/detail/notebooks/NotebookTableRow.tsx b/frontend/src/pages/projects/screens/detail/notebooks/NotebookTableRow.tsx
@@ -47,7 +47,7 @@ const NotebookTableRow: React.FC<NotebookTableRowProps> = ({
         </Td>
         <Td>{notebookSize?.name ?? 'Unknown'}</Td>
         <Td>
-          <NotebookStatusToggle notebookState={obj} />
+          <NotebookStatusToggle notebookState={obj} doListen={false} />
         </Td>
         <Td>
           <NotebookRouteLink label="Open" notebook={obj.notebook} isRunning={obj.isRunning} />

diff --git a/frontend/src/pages/projects/screens/detail/storage/ExistingConnectedNotebooks.tsx b/frontend/src/pages/projects/screens/detail/storage/ExistingConnectedNotebooks.tsx
@@ -1,7 +1,5 @@
 import * as React from 'react';
-import { NotebookKind } from '../../../../../k8sTypes';
 import { Alert, Chip, ChipGroup, FormGroup, Spinner, Text } from '@patternfly/react-core';
-import { getNotebook } from '../../../../../api';
 import { ProjectDetailsContext } from '../../../ProjectDetailsContext';
 import { getNotebookDisplayName } from '../../../utils';
 
@@ -14,30 +12,9 @@ const ExistingConnectedNotebooks: React.FC<ExistingConnectedNotebooksProps> = ({
   existingNotebooks,
   setExistingNotebooks,
 }) => {
-  const [notebookMap, setNotebookMap] = React.useState<{ [name: string]: NotebookKind }>({});
-  const [loaded, setLoaded] = React.useState(false);
-  const [error, setError] = React.useState<Error | undefined>();
-
-  const { currentProject } = React.useContext(ProjectDetailsContext);
-  const namespace = currentProject.metadata.name;
-
-  React.useEffect(() => {
-    if (existingNotebooks.length > 0) {
-      Promise.all(existingNotebooks.map((name) => getNotebook(name, namespace)))
-        .then((fetchedNotebooks) => {
-          setNotebookMap(
-            fetchedNotebooks.reduce(
-              (acc, notebook) => ({ ...acc, [notebook.metadata.name]: notebook }),
-              {},
-            ),
-          );
-          setLoaded(true);
-        })
-        .catch((e) => {
-          setError(e);
-        });
-    }
-  }, [existingNotebooks, namespace]);
+  const {
+    notebooks: { data: allNotebooks, loaded, error },
+  } = React.useContext(ProjectDetailsContext);
 
   let content: React.ReactNode;
   if (error) {
@@ -54,7 +31,13 @@ const ExistingConnectedNotebooks: React.FC<ExistingConnectedNotebooksProps> = ({
     content = (
       <ChipGroup>
         {existingNotebooks.map((notebookName) => {
-          const notebookDisplayName = getNotebookDisplayName(notebookMap[notebookName]);
+          const foundNotebook = allNotebooks.find(
+            (notebook) => notebook.notebook.metadata.name === notebookName,
+          );
+          if (!foundNotebook) {
+            return null;
+          }
+          const notebookDisplayName = getNotebookDisplayName(foundNotebook.notebook);
           return (
             <Chip
               key={notebookDisplayName}

diff --git a/frontend/src/pages/projects/screens/detail/storage/StorageList.tsx b/frontend/src/pages/projects/screens/detail/storage/StorageList.tsx
@@ -17,15 +17,15 @@ const StorageList: React.FC = () => {
   return (
     <>
       <DetailsSection
-        id={ProjectSectionID.STORAGES}
-        title={ProjectSectionTitles[ProjectSectionID.STORAGES]}
+        id={ProjectSectionID.CLUSTER_STORAGES}
+        title={ProjectSectionTitles[ProjectSectionID.CLUSTER_STORAGES]}
         actions={[
           <Button
             onClick={() => setOpen(true)}
-            key={`action-${ProjectSectionID.STORAGES}`}
+            key={`action-${ProjectSectionID.CLUSTER_STORAGES}`}
             variant="secondary"
           >
-            Add storage
+            Add cluster storage
           </Button>,
         ]}
         isLoading={!loaded}

diff --git a/frontend/src/pages/projects/screens/detail/types.ts b/frontend/src/pages/projects/screens/detail/types.ts
@@ -1,6 +1,6 @@
 export enum ProjectSectionID {
   WORKBENCHES = 'workbenches',
-  STORAGES = 'storages',
+  CLUSTER_STORAGES = 'cluster-storages',
   DATA_CONNECTIONS = 'data-connections',
 }