Improve event filtering to avoid scary messages (#563)

* Improve event filtering to avoid scary messages

* Improve warnings not to show normal events in them

* Cleanup of code to make more readable

frontend/src/pages/notebookController/screens/server/SpawnerPage.tsx

@@ -76,15 +76,16 @@ const SpawnerPage: React.FC = () => {
   const { notebookNamespace: projectName } = useNamespaces();
   const currentUserState = useNotebookUserState();
   const username = currentUserState.user;
-  const { startShown, hideStartShown, refreshNotebookForStart } = useSpawnerNotebookModalState();
+  const [createInProgress, setCreateInProgress] = React.useState<boolean>(false);
+  const { startShown, hideStartShown, refreshNotebookForStart } =
+    useSpawnerNotebookModalState(createInProgress);
   const [selectedImageTag, setSelectedImageTag] = React.useState<ImageTag>({
     image: undefined,
     tag: undefined,
   });
   const { selectedSize, setSelectedSize, sizes } = usePreferredNotebookSize();
   const [selectedGpu, setSelectedGpu] = React.useState<string>('0');
   const [variableRows, setVariableRows] = React.useState<VariableRow[]>([]);
-  const [createInProgress, setCreateInProgress] = React.useState<boolean>(false);
   const [submitError, setSubmitError] = React.useState<Error | null>(null);
 
   React.useEffect(() => {

frontend/src/pages/notebookController/screens/server/StartServerModal.tsx

@@ -49,6 +49,7 @@ const StartServerModal: React.FC<StartServerModalProps> = ({ open, spawnInProgre
   const notebookStatus = useDeepCompareMemoize(unstableNotebookStatus);
   const getNotebookLink = useNotebookRedirectLink();
   const history = useHistory();
+  const spawnFailed = spawnStatus?.status === AlertVariant.danger;
 
   React.useEffect(() => {
     if (!open) {
@@ -59,8 +60,6 @@ const StartServerModal: React.FC<StartServerModalProps> = ({ open, spawnInProgre
     }
   }, [open]);
 
-  const spawnFailed = spawnStatus?.status === AlertVariant.danger;
-
   const navigateToNotebook = React.useCallback(
     (useCurrentTab: boolean): void => {
       getNotebookLink()
@@ -86,7 +85,7 @@ const StartServerModal: React.FC<StartServerModalProps> = ({ open, spawnInProgre
 
   React.useEffect(() => {
     let timer;
-    if (isNotebookRunning) {
+    if (isNotebookRunning && open) {
       setSpawnPercentile(100);
       setSpawnStatus({
         status: AlertVariant.success,
@@ -102,10 +101,10 @@ const StartServerModal: React.FC<StartServerModalProps> = ({ open, spawnInProgre
     return () => {
       clearTimeout(timer);
     };
-  }, [isNotebookRunning, navigateToNotebook, isUsingCurrentTab]);
+  }, [isNotebookRunning, open, isUsingCurrentTab, navigateToNotebook]);
 
   React.useEffect(() => {
-    if (spawnInProgress && !isNotebookRunning) {
+    if (spawnInProgress && !isNotebookRunning && open) {
       if (!notebookStatus) {
         return;
       }
@@ -132,7 +131,7 @@ const StartServerModal: React.FC<StartServerModalProps> = ({ open, spawnInProgre
         });
       }
     }
-  }, [notebookStatus, spawnInProgress, isNotebookRunning]);
+  }, [notebookStatus, spawnInProgress, isNotebookRunning, open]);
 
   const renderProgress = () => {
     let variant: ProgressVariant | undefined;
@@ -181,7 +180,7 @@ const StartServerModal: React.FC<StartServerModalProps> = ({ open, spawnInProgre
         onClick={onClose}
         isDisabled={!open}
       >
-        {spawnFailed ? 'Close' : 'Cancel'}
+        Cancel
       </Button>
     ) : isUsingCurrentTab ? null : (
       <ActionList>

frontend/src/pages/notebookController/screens/server/useSpawnerNotebookModalState.ts

@@ -3,6 +3,8 @@ import { useHistory } from 'react-router-dom';
 import { NotebookControllerContext } from '../../NotebookControllerContext';
 import { FAST_POLL_INTERVAL } from '../../../../utilities/const';
 import { NotebookControllerContextProps } from '../../notebookControllerContextTypes';
+import { stopNotebook } from '../../../../services/notebookService';
+import useNamespaces from '../../useNamespaces';
 
 const useRefreshNotebookAndCleanup = (startShown: boolean) => {
   const { requestNotebookRefresh } = React.useContext(NotebookControllerContext);
@@ -24,13 +26,16 @@ const useRefreshNotebookAndCleanup = (startShown: boolean) => {
   }, []);
 };
 
-const useSpawnerNotebookModalState = (): {
+const useSpawnerNotebookModalState = (
+  createInProgress: boolean,
+): {
   startShown: boolean;
   hideStartShown: () => void;
   refreshNotebookForStart: () => void;
 } => {
   const { currentUserNotebook: notebook, currentUserNotebookIsRunning: isNotebookRunning } =
     React.useContext(NotebookControllerContext);
+  const { notebookNamespace } = useNamespaces();
   const history = useHistory();
   const [startShown, setStartShown] = React.useState<boolean>(false);
 
@@ -41,7 +46,14 @@ const useSpawnerNotebookModalState = (): {
         const notStopped = !notebook.metadata.annotations?.['kubeflow-resource-stopped'];
         if (notStopped) {
           // Not stopped means we are spawning (as it is not running)
-          setStartShown(true);
+          if (!createInProgress) {
+            // We are not creating, make sure the Notebook is stopped
+            stopNotebook(notebookNamespace, notebook.metadata.name).catch(() => {
+              console.error('Failed to stop notebook on refresh');
+            });
+          } else {
+            setStartShown(true);
+          }
         } else {
           // Stopped, no need for a modal (if it is open)
           setStartShown(false);
@@ -52,7 +64,7 @@ const useSpawnerNotebookModalState = (): {
         history.replace('/notebookController');
       }
     }
-  }, [notebook, history, startShown, isNotebookRunning]);
+  }, [notebook, history, startShown, isNotebookRunning, createInProgress, notebookNamespace]);
 
   const refreshNotebookForStart = useRefreshNotebookAndCleanup(startShown);
   const hideStartShown = React.useCallback(() => setStartShown(false), []);

frontend/src/types.ts

@@ -614,7 +614,7 @@ export type K8sEvent = {
   lastTimestamp: string | null; // if it never starts, the value is null
   message: string;
   reason: string;
-  type: string;
+  type: 'Warning' | 'Normal';
 };
 
 export type NotebookStatus = {

frontend/src/utilities/notebookControllerUtils.ts

@@ -266,9 +266,6 @@ export const validateNotebookNamespaceRoleBinding = async (
   );
 };
 
-export const getEventTimestamp = (event: K8sEvent): string =>
-  event.lastTimestamp || event.eventTime;
-
 export const useNotebookRedirectLink = (): (() => Promise<string>) => {
   const { currentUserNotebook } = React.useContext(NotebookControllerContext);
   const { notebookNamespace } = useNamespaces();
@@ -322,6 +319,59 @@ export const useNotebookRedirectLink = (): (() => Promise<string>) => {
   }, [backupRoute, notebookNamespace, routeName]);
 };
 
+export const getEventTimestamp = (event: K8sEvent): string =>
+  event.lastTimestamp || event.eventTime;
+
+const filterEvents = (
+  allEvents: K8sEvent[],
+  lastActivity: Date,
+): [filterEvents: K8sEvent[], thisInstanceEvents: K8sEvent[], gracePeroid: boolean] => {
+  const thisInstanceEvents = allEvents.filter(
+    (event) => new Date(getEventTimestamp(event)) >= lastActivity,
+  );
+  if (thisInstanceEvents.length === 0) {
+    // Filtered out all of the events, exit early
+    return [[], [], false];
+  }
+
+  let filteredEvents = thisInstanceEvents;
+
+  const now = Date.now();
+  let gracePeriod = false;
+
+  // Ignore the rest of the filter logic if we pass 20 minutes
+  const maxCap = new Date(lastActivity).setMinutes(lastActivity.getMinutes() + 20);
+  if (now <= maxCap) {
+    // Haven't hit the cap yet, filter events for accepted scenarios
+    const infoEvents = filteredEvents.filter((event) => event.type === 'Normal');
+    const idleTime = new Date(lastActivity).setSeconds(lastActivity.getSeconds() + 30);
+    gracePeriod = idleTime - now > 0;
+
+    // Suppress the warnings when we are idling
+    if (gracePeriod) {
+      filteredEvents = infoEvents;
+    }
+
+    // If we are scaling up, we want to focus on that
+    const hasScaleUp = infoEvents.some((event) => event.reason === 'TriggeredScaleUp');
+    if (hasScaleUp) {
+      // Scaling up event is present -- look for issues with it
+      const hasScaleUpIssueIndex = thisInstanceEvents.findIndex(
+        (event) => event.reason === 'NotTriggerScaleUp',
+      );
+      if (hasScaleUpIssueIndex >= 0) {
+        // Has scale up issue, show that
+        filteredEvents = [thisInstanceEvents[hasScaleUpIssueIndex]];
+      } else {
+        // Haven't hit a failure in scale up, show just infos
+        filteredEvents = infoEvents;
+      }
+    }
+  }
+
+  return [filteredEvents, thisInstanceEvents, gracePeriod];
+};
+
 export const useNotebookStatus = (
   spawnInProgress: boolean,
 ): [status: NotebookStatus | null, events: K8sEvent[]] => {
@@ -337,18 +387,16 @@ export const useNotebookStatus = (
   const annotationTime = notebook?.metadata.annotations?.['notebooks.kubeflow.org/last-activity'];
   const lastActivity = annotationTime ? new Date(annotationTime) : null;
   if (!lastActivity) {
-    // Modal is closed, we don't have a filter time, ignore
+    // Notebook not started, we don't have a filter time, ignore
     return [null, []];
   }
 
-  const filteredEvents = events.filter(
-    (event) => new Date(getEventTimestamp(event)) >= lastActivity,
-  );
+  const [filteredEvents, thisInstanceEvents, gracePeriod] = filterEvents(events, lastActivity);
   if (filteredEvents.length === 0) {
-    // We filter out all the events, nothing to show
-    return [null, []];
+    return [null, thisInstanceEvents];
   }
 
+  // Parse the last event
   let percentile = 0;
   let status: EventStatus = EventStatus.IN_PROGRESS;
   const lastItem = filteredEvents[filteredEvents.length - 1];
@@ -381,8 +429,10 @@ export const useNotebookStatus = (
         break;
       }
       default: {
-        currentEvent = 'Error creating oauth proxy container';
-        status = EventStatus.ERROR;
+        if (lastItem.type === 'Warning') {
+          currentEvent = 'Issue creating oauth proxy container';
+          status = EventStatus.WARNING;
+        }
       }
     }
   } else {
@@ -427,14 +477,23 @@ export const useNotebookStatus = (
         percentile = 64;
         break;
       }
+      case 'NotTriggerScaleUp':
+        currentEvent = 'Failed to scale-up';
+        status = EventStatus.ERROR;
+        break;
       case 'TriggeredScaleUp': {
         currentEvent = 'Pod triggered scale-up';
         status = EventStatus.INFO;
         break;
       }
       default: {
-        currentEvent = 'Error creating notebook container';
-        status = EventStatus.ERROR;
+        if (!gracePeriod && lastItem.reason === 'FailedScheduling') {
+          currentEvent = 'Insufficient resources to start';
+          status = EventStatus.ERROR;
+        } else if (lastItem.type === 'Warning') {
+          currentEvent = 'Issue creating notebook container';
+          status = EventStatus.WARNING;
+        }
       }
     }
   }
@@ -447,7 +506,7 @@ export const useNotebookStatus = (
       currentEventDescription: lastItem.message,
       currentStatus: status,
     },
-    filteredEvents,
+    thisInstanceEvents,
   ];
 };
 

frontend/src/utilities/useWatchNotebookEvents.tsx

@@ -2,11 +2,12 @@ import * as React from 'react';
 import { getNotebookEvents } from '../services/notebookEventsService';
 import useNotification from './useNotification';
 import { K8sEvent } from '../types';
+import { FAST_POLL_INTERVAL } from './const';
 
 export const useWatchNotebookEvents = (
   projectName: string,
   notebookName: string,
-  watch: boolean,
+  activeFetch: boolean,
 ): K8sEvent[] => {
   const [notebookEvents, setNoteBookEvents] = React.useState<K8sEvent[]>([]);
   const notification = useNotification();
@@ -17,31 +18,31 @@ export const useWatchNotebookEvents = (
 
     const clear = () => {
       cancelled = true;
-      if (watchHandle) {
-        clearTimeout(watchHandle);
-      }
+      clearTimeout(watchHandle);
     };
 
-    const watchNotebookEvents = () => {
-      if (projectName && notebookName && watch) {
-        getNotebookEvents(projectName, notebookName)
-          .then((data: K8sEvent[]) => {
-            if (cancelled) {
-              return;
-            }
-            setNoteBookEvents(data);
-          })
-          .catch((e) => {
-            notification.error('Error fetching notebook events', e.response.data.message);
-            clear();
-          });
-        watchHandle = setTimeout(watchNotebookEvents, 1000);
-      }
-    };
+    if (activeFetch) {
+      const watchNotebookEvents = () => {
+        if (projectName && notebookName) {
+          getNotebookEvents(projectName, notebookName)
+            .then((data: K8sEvent[]) => {
+              if (cancelled) {
+                return;
+              }
+              setNoteBookEvents(data);
+            })
+            .catch((e) => {
+              notification.error('Error fetching notebook events', e.response.data.message);
+              clear();
+            });
+          watchHandle = setTimeout(watchNotebookEvents, FAST_POLL_INTERVAL);
+        }
+      };
 
-    watchNotebookEvents();
+      watchNotebookEvents();
+    }
     return clear;
-  }, [projectName, notebookName, notification, watch]);
+  }, [projectName, notebookName, notification, activeFetch]);
 
   return notebookEvents;
 };