fix: Top should allow pageup/pagedown to cycle through clusters

starpit · starpit · commit a67b42429485 · 2023-04-19T14:23:14.000-04:00
diff --git a/plugins/plugin-codeflare-dashboard/src/components/Top/Header.tsx b/plugins/plugin-codeflare-dashboard/src/components/Top/Header.tsx
@@ -30,7 +30,7 @@ export default class Header extends React.PureComponent<Props> {
             <Text color="blue" bold>
               {"Cluster   " /* Cheapo alignment with "Namespace" */}
             </Text>
-            {this.props.cluster}
+            {this.props.cluster.replace(/:\d+$/, "")}
           </Text>
 
           <Spacer />
diff --git a/plugins/plugin-codeflare-dashboard/src/components/Top/index.tsx b/plugins/plugin-codeflare-dashboard/src/components/Top/index.tsx
@@ -22,18 +22,19 @@ import { Box, Text, render } from "ink"
 import type Group from "./Group.js"
 import type {
   Context,
-  ChangeContextRequest,
   ChangeContextRequestHandler,
   WatcherInitializer,
+  UpdateError,
   UpdatePayload,
+  UpdatePayloadOrError,
   ResourceSpec,
 } from "./types.js"
 
+import Header from "./Header.js"
 import JobBox from "./JobBox.js"
+import { isError } from "./types.js"
 import defaultValueFor from "./defaults.js"
 
-import Header from "./Header.js"
-
 type UI = {
   /** Force a refresh */
   refreshCycle?: number
@@ -55,6 +56,9 @@ type State = UI & {
   /** Model from controller */
   rawModel: UpdatePayload
 
+  /** Error in updating model? */
+  updateError: null | UpdateError
+
   /** Our grouping of `rawModel` */
   groups: Group[]
 
@@ -83,19 +87,19 @@ class Top extends React.PureComponent<Props, State> {
     return this.state?.selectedGroupIdx >= 0 && this.state?.selectedGroupIdx < this.state.groups.length
   }
 
+  private clearCurrentJobSelection() {
+    this.setState({ selectedGroupIdx: -1 })
+  }
+
   /** Current cluster context */
   private get currentContext() {
     return {
+      context: this.state?.rawModel?.context || this.props.context,
       cluster: this.state?.rawModel?.cluster || this.props.cluster,
       namespace: this.state?.rawModel?.namespace || this.props.namespace,
     }
   }
 
-  /** Updated cluster context */
-  private updatedContext({ which }: Pick<ChangeContextRequest, "which">, next: string) {
-    return Object.assign(this.currentContext, which === "namespace" ? { namespace: next } : { cluster: next })
-  }
-
   public async componentDidMount() {
     this.setState({ watcher: await this.props.initWatcher(this.currentContext, this.onData) })
 
@@ -131,6 +135,28 @@ class Top extends React.PureComponent<Props, State> {
     }
   }
 
+  private async cycleThroughContexts(which: "namespace" | "cluster", dir: "up" | "down") {
+    if (this.currentContext) {
+      const updatedContext = await this.props.changeContext({ which, context: this.currentContext, dir })
+
+      if (updatedContext) {
+        this.reinit(updatedContext)
+      }
+    }
+  }
+
+  private cycleThroughJobs(dir: "left" | "right") {
+    if (this.state.groups) {
+      const incr = dir === "left" ? -1 : 1
+      this.setState((curState) => ({
+        selectedGroupIdx:
+          curState?.selectedGroupIdx === undefined
+            ? 0
+            : this.mod(curState.selectedGroupIdx + incr, curState.groups.length + 1),
+      }))
+    }
+  }
+
   /** Handle keyboard events from the user */
   private initKeyboardEvents() {
     if (!process.stdin.isTTY) {
@@ -149,46 +175,23 @@ class Top extends React.PureComponent<Props, State> {
       } else {
         switch (key.name) {
           case "escape":
-            this.setState({ selectedGroupIdx: -1 })
+            this.clearCurrentJobSelection()
             break
+
           case "up":
           case "down":
-            /** Change context selection */
-            if (this.state?.rawModel.namespace) {
-              this.props
-                .changeContext({ which: "namespace", from: this.state.rawModel.namespace, dir: key.name })
-                .then((next) => {
-                  if (next) {
-                    this.reinit(this.updatedContext({ which: "namespace" }, next))
-                  }
-                })
-            }
+            this.cycleThroughContexts("namespace", key.name)
+            break
+
+          case "pageup":
+          case "pagedown":
+            this.cycleThroughContexts("cluster", key.name === "pageup" ? "up" : "down")
             break
 
           case "left":
           case "right":
-            /** Change job selection */
-            if (this.state.groups) {
-              const incr = key.name === "left" ? -1 : 1
-              this.setState((curState) => ({
-                selectedGroupIdx:
-                  curState?.selectedGroupIdx === undefined
-                    ? 0
-                    : this.mod(curState.selectedGroupIdx + incr, curState.groups.length + 1),
-              }))
-            }
+            this.cycleThroughJobs(key.name)
             break
-          /*case "i":
-            this.setState((curState) => ({ blockCells: !this.useBlocks(curState) }))
-            break*/
-          /*case "g":
-            this.setState((curState) => ({
-              groupHosts: !this.groupHosts(curState),
-              groups: !curState?.rawModel
-                ? curState?.groups
-                : this.groupBy(curState.rawModel, !this.groupHosts(curState)),
-            }))
-            break */
         }
       }
     })
@@ -198,28 +201,38 @@ class Top extends React.PureComponent<Props, State> {
     return { min: { cpu: 0, mem: 0, gpu: 0 }, tot: {} }
   }
 
-  private reinit(context: Context) {
+  private async reinit(context: Context) {
     if (this.state?.watcher) {
       this.state?.watcher.kill()
     }
-    this.setState({ groups: [], rawModel: Object.assign({ hosts: [], stats: this.emptyStats }, context) })
-    this.props.initWatcher(context, this.onData)
+    this.setState({
+      groups: [],
+      updateError: null,
+      watcher: await this.props.initWatcher(context, this.onData),
+      rawModel: Object.assign({ hosts: [], stats: this.emptyStats }, context),
+    })
   }
 
   /** We have received data from the controller */
-  private readonly onData = (rawModel: UpdatePayload) => {
+  private readonly onData = (rawModel: UpdatePayloadOrError) => {
     if (rawModel.cluster !== this.currentContext.cluster || rawModel.namespace !== this.currentContext.namespace) {
       // this is straggler data from the prior context
       return
-    }
-
-    this.setState((curState) => {
-      if (JSON.stringify(curState?.rawModel) === JSON.stringify(rawModel)) {
-        return null
-      } else {
-        return { rawModel, groups: this.groupBy(rawModel) }
+    } else if (isError(rawModel)) {
+      // update error
+      if (!this.state?.updateError || JSON.stringify(rawModel) !== JSON.stringify(this.state.updateError)) {
+        this.setState({ updateError: rawModel })
       }
-    })
+    } else {
+      // good update from current context
+      this.setState((curState) => {
+        if (JSON.stringify(curState?.rawModel) === JSON.stringify(rawModel)) {
+          return null
+        } else {
+          return { rawModel, groups: this.groupBy(rawModel) }
+        }
+      })
+    }
   }
 
   private groupBy(model: UpdatePayload): State["groups"] {
@@ -272,7 +285,9 @@ class Top extends React.PureComponent<Props, State> {
   }
 
   private body() {
-    if (this.state.groups.length === 0) {
+    if (this.state?.updateError) {
+      return <Text color="red">{this.state.updateError.message}</Text>
+    } else if (this.state.groups.length === 0) {
       return <Text>No active jobs</Text>
     } else {
       return (
@@ -291,13 +306,13 @@ class Top extends React.PureComponent<Props, State> {
   }
 
   public render() {
-    if (!this.state?.groups) {
+    if (!this.state?.updateError && !this.state?.groups) {
       // TODO spinner? this means we haven't received the first data set, yet
       return <React.Fragment />
     } else {
       return (
         <Box flexDirection="column">
-          <Header cluster={this.state.rawModel.cluster} namespace={this.state.rawModel.namespace} />
+          <Header {...this.currentContext} />
           <Box marginTop={1}>{this.body()}</Box>
         </Box>
       )
diff --git a/plugins/plugin-codeflare-dashboard/src/components/Top/types.ts b/plugins/plugin-codeflare-dashboard/src/components/Top/types.ts
@@ -54,20 +54,33 @@ type JobsByHost = {
 
 /** The cluster focus of the model */
 export type Context = {
+  /** Kubernetes context name */
+  context: string
+
   /** Kubernetes cluster name */
   cluster: string
 
   /** Kubernetes namespace */
   namespace: string
 }
 
+/** Oops, something bad happened while fetching a model update */
+export type UpdateError = Context & { message: string }
+
 /** Updated model */
 export type UpdatePayload = Context & JobsByHost
 
-export type OnData = (payload: UpdatePayload) => void
+/** Updated model or error in doing so */
+export type UpdatePayloadOrError = (Context & JobsByHost) | UpdateError
+
+export function isError(payload: UpdatePayloadOrError): payload is UpdateError {
+  return typeof payload === "object" && typeof (payload as UpdateError).message === "string"
+}
+
+export type OnData = (payload: UpdatePayloadOrError) => void
 
 export type WatcherInitializer = (context: Context, cb: OnData) => Promise<{ kill(): void }>
 
-export type ChangeContextRequest = { which: "context" | "namespace"; from: string; dir: "down" | "up" }
+export type ChangeContextRequest = { which: "cluster" | "namespace"; context: Context; dir: "down" | "up" }
 
-export type ChangeContextRequestHandler = (req: ChangeContextRequest) => Promise<string | undefined>
+export type ChangeContextRequestHandler = (req: ChangeContextRequest) => Promise<Context | undefined>
diff --git a/plugins/plugin-codeflare-dashboard/src/controller/dashboard/top/index.ts b/plugins/plugin-codeflare-dashboard/src/controller/dashboard/top/index.ts
@@ -20,7 +20,7 @@ import type { Arguments } from "@kui-shell/core"
 import type TopOptions from "./options.js"
 
 import { enterAltBufferMode } from "../term.js"
-import { getCurrentCluster, getCurrentNamespace, changeContext } from "../../kubernetes.js"
+import { getCurrentContext, getCurrentCluster, getCurrentNamespace, changeContext } from "../../kubernetes.js"
 
 import initWatcher from "./watcher.js"
 
@@ -36,7 +36,12 @@ export default async function jobsController(args: Arguments<TopOptions>) {
   }
 
   // these will be the initial values of cluster and namespace focus
-  const [cluster, ns] = await Promise.all([getCurrentCluster(), getNamespaceFromArgsOrCurrent(args)])
+  const [context, cluster, ns] = await Promise.all([
+    getCurrentContext(),
+    getCurrentCluster(),
+    getNamespaceFromArgsOrCurrent(args),
+  ])
+  debug("context", context)
   debug("cluster", cluster)
   debug("namespace", ns || "using namespace from user current context")
 
@@ -45,6 +50,7 @@ export default async function jobsController(args: Arguments<TopOptions>) {
 
   debug("rendering")
   await render({
+    context,
     cluster,
     namespace: ns,
     initWatcher: initWatcher.bind(args.parsedOptions),
diff --git a/plugins/plugin-codeflare-dashboard/src/controller/dashboard/top/watcher.ts b/plugins/plugin-codeflare-dashboard/src/controller/dashboard/top/watcher.ts
@@ -27,9 +27,9 @@ import defaultValueFor from "../../../components/Top/defaults.js"
 type Model = Record<string, Record<string, Record<string, PodRec>>>
 //                  host            job            name
 
-export default async function initWatcher(this: TopOptions, { cluster, namespace: ns }: Context, cb: OnData) {
+export default async function initWatcher(this: TopOptions, { context, cluster, namespace: ns }: Context, cb: OnData) {
   const debug = Debug("plugin-codeflare-dashboard/controller/top")
-  debug("init watcher callbacks", cluster, ns)
+  debug("init watcher callbacks", context, cluster, ns)
 
   // To help us parse out one "record's" worth of output from kubectl
   const recordSeparator = "-----------"
@@ -76,15 +76,36 @@ export default async function initWatcher(this: TopOptions, { cluster, namespace
     "bash",
     [
       "-c",
-      `"while true; do kubectl get pod -n ${ns} --no-headers -o=custom-columns=NAME:.metadata.name,JOB:'.metadata.labels.app\\.kubernetes\\.io/instance',HOST:.status.hostIP,CPU:'.spec.containers[0].resources.requests.cpu',CPUL:'.spec.containers[0].resources.limits.cpu',MEM:'.spec.containers[0].resources.requests.memory',MEML:'.spec.containers[0].resources.limits.memory',GPU:.spec.containers[0].resources.requests.'nvidia\\.com/gpu',GPUL:.spec.containers[0].resources.limits.'nvidia\\.com/gpu',JOB2:'.metadata.labels.appwrapper\\.mcad\\.ibm\\.com',CTIME:.metadata.creationTimestamp,USER:'.metadata.labels.app\\.kubernetes\\.io/owner'; echo '${recordSeparator}'; sleep 2; done"`,
+      `"while true; do kubectl get pod --context ${context} -n ${ns} --no-headers -o=custom-columns=NAME:.metadata.name,JOB:'.metadata.labels.app\\.kubernetes\\.io/instance',HOST:.status.hostIP,CPU:'.spec.containers[0].resources.requests.cpu',CPUL:'.spec.containers[0].resources.limits.cpu',MEM:'.spec.containers[0].resources.requests.memory',MEML:'.spec.containers[0].resources.limits.memory',GPU:.spec.containers[0].resources.requests.'nvidia\\.com/gpu',GPUL:.spec.containers[0].resources.limits.'nvidia\\.com/gpu',JOB2:'.metadata.labels.appwrapper\\.mcad\\.ibm\\.com',CTIME:.metadata.creationTimestamp,USER:'.metadata.labels.app\\.kubernetes\\.io/owner'; echo '${recordSeparator}'; sleep 2; done"`,
     ],
-    { shell: "/bin/bash", stdio: ["ignore", "pipe", "inherit"] }
+    { shell: "/bin/bash", stdio: ["ignore", "pipe", "pipe"] }
   )
   debug("spawned watcher")
-  process.on("exit", () => child.kill())
 
-  child.on("error", (err) => console.error(err))
-  child.on("exit", (code) => debug("watcher subprocess exiting", code))
+  const killit = () => child.kill()
+  process.once("exit", killit)
+
+  let message = ""
+  child.stderr.on("data", (data) => {
+    const msg = data.toString()
+    if (message !== msg) {
+      message += msg
+    }
+  })
+
+  child.once("error", (err) => {
+    console.error(err)
+    process.off("exit", killit)
+  })
+
+  child.once("exit", (code) => {
+    debug("watcher subprocess exiting", code)
+    process.off("exit", killit)
+
+    if (code !== 0 && message.length > 0) {
+      cb({ context, cluster, namespace: ns, message })
+    }
+  })
 
   let leftover = ""
   child.stdout.on("data", (data) => {
@@ -163,7 +184,7 @@ export default async function initWatcher(this: TopOptions, { cluster, namespace
         })),
       }))
 
-      cb(Object.assign({ cluster, namespace: ns }, stats(hosts)))
+      cb(Object.assign({ context, cluster, namespace: ns }, stats(hosts)))
     }
   })
 
diff --git a/plugins/plugin-codeflare-dashboard/src/controller/kubernetes.ts b/plugins/plugin-codeflare-dashboard/src/controller/kubernetes.ts
