fix: improved UX for attaching to running jobs

Instead of showing nothing for an extended period of time, we now open a window immediately and show some (for now textual) progress info.

This PR also fixes some issues that could result in bogus output (`cannot read property runtime_env of undefined`) in job description.

Author: starpit

plugins/plugin-codeflare/src/controller/attach.ts

@@ -14,6 +14,7 @@
  * limitations under the License.
  */
 
+import Debug from "debug"
 import { MadWizardOptions } from "madwizard"
 import { Arguments, ParsedOptions } from "@kui-shell/core"
 
@@ -31,7 +32,14 @@ type Options = ParsedOptions &
  * @return the path to the captured log directory and a cleanup
  * handler to be invoked when you are done consuming the output
  */
-export async function attach(profile: string, jobId: boolean | string, opts: MadWizardOptions = {}) {
+export async function attach(
+  profile: string,
+  jobId: boolean | string,
+  opts: MadWizardOptions = {},
+  stdout: (line: string) => void
+) {
+  const debug = Debug("plugin-codeflare/attach")
+
   const deployGuidebook = "ml/ray/aggregator/in-cluster/client-side/deploy"
   const startGuidebook =
     typeof jobId === "string"
@@ -56,14 +64,23 @@ export async function attach(profile: string, jobId: boolean | string, opts: Mad
     opts
   )
 
+  const { cli } = await import("madwizard/dist/fe/cli")
+
   // *deploy* the aggregator pod, then *start* an aggregator instance
   // for the given jobId
-  const { cli } = await import("madwizard/dist/fe/cli")
-  await cli(
-    ["codeflare", "guide", deployGuidebook],
-    undefined,
-    Object.assign({}, options, { name: "log-aggregator-deploy" })
-  )
+  try {
+    const deployOptions = Object.assign({}, options, { name: "log-aggregator-deploy" })
+    debug("Deploying log aggregator", deployGuidebook, deployOptions)
+    stdout("Deploying log aggregator...\n")
+    await cli(["codeflare", "guide", deployGuidebook], undefined, deployOptions)
+    debug("deploying log aggregator: done")
+  } catch (err) {
+    console.error("Error attaching", err)
+    throw err
+  }
+
+  debug("attaching to", jobId)
+  stdout("Attaching to job...\n")
   const resp = await cli(
     ["codeflare", "guide", startGuidebook],
     undefined,
@@ -72,6 +89,9 @@ export async function attach(profile: string, jobId: boolean | string, opts: Mad
 
   // the logdir that we captured
   const logdir = resp && resp.env ? resp.env.LOGDIR_STAGE : undefined
+  if (logdir) {
+    debug("successfully attached to", jobId, logdir)
+  }
 
   return {
     logdir,
@@ -89,12 +109,16 @@ export default async function attachCommand(args: Arguments<Options>) {
   const jobId = args.parsedOptions.a
   const profile = args.parsedOptions.p || (await Profiles.lastUsed())
 
-  const resp = await attach(profile, jobId, { verbose: args.parsedOptions.V })
+  const stdout = await args.createOutputStream()
+  const { logdir, cleanExit } = await attach(profile, jobId, { verbose: args.parsedOptions.V }, stdout)
+
+  if (logdir) {
+    if (cleanExit) {
+      window.addEventListener("beforeunload", () => cleanExit())
+    }
 
-  if (resp.logdir) {
-    process.env.LOGDIR = resp.logdir
-    await args.REPL.qexec(`codeflare dashboardui -f ${encodeComponent(resp.logdir)}`)
-    // TODO utilize resp.cleanExit?
+    stdout("Opening Dashboard\n")
+    return args.REPL.qexec(`codeflare dashboardui -f ${encodeComponent(logdir)}`)
   } else {
     throw new Error("Could not attach, due to missing logging directory")
   }

plugins/plugin-codeflare/src/controller/description.ts

@@ -16,19 +16,24 @@
 
 import { join } from "path"
 import { Arguments, Registrar, encodeComponent } from "@kui-shell/core"
+
+import Status from "./status"
 import { expand } from "../lib/util"
 
 type Item = { label: string; value: string }
 
 export type Summary = Item[]
 
-export type SummaryResponse = {
+type SummaryResponse = {
   jobid: string
+  status: Status
+  start_time: number
+  end_time: number
   cmdline: {
     appPart: string
     systemPart: string
   }
-  runtimeEnv: {
+  runtime_env: {
     env_vars: {
       KUBE_CONTEXT: string
       KUBE_NS: string
@@ -48,11 +53,29 @@ function jobDefinition(filepath: string) {
   return encodeComponent(expand(join(filepath.replace(/'/g, ""), "ray-job-definition.json")))
 }
 
-export async function getJobDefinition(runDir: string, REPL: Arguments["REPL"]) {
+export async function getJobDefinition(runDir: string, REPL: Arguments["REPL"], inRetry = 0): Promise<SummaryResponse> {
   try {
-    return JSON.parse(await REPL.qexec<string>(`vfs fslice ${encodeComponent(jobDefinition(runDir))} 0`))
+    const data = await REPL.qexec<string>(`vfs fslice ${jobDefinition(runDir)} 0`)
+    try {
+      return JSON.parse(data)
+    } catch (err) {
+      if (inRetry < 10 && typeof data === "string" && data.length === 0) {
+        // some race condition; the vfs might not be ready; FIXME!!! hack here:
+        await new Promise((resolve) => setTimeout(resolve, 100))
+        return getJobDefinition(runDir, REPL, inRetry + 1)
+      } else {
+        console.error(
+          "Error parsing job definition",
+          jobDefinition(runDir),
+          typeof data === "string" ? data.length : -1,
+          data
+        )
+        throw err
+      }
+    }
   } catch (err) {
-    console.error(runDir, err)
+    console.error("Error loading job definition", runDir, err)
+    throw err
   }
 }
 
@@ -63,6 +86,10 @@ async function app(args: Arguments) {
   }
 
   const jobInfo = await getJobDefinition(filepath, args.REPL)
+  if (!jobInfo) {
+    return "Error fetching job information"
+  }
+
   const RAY_IMAGE = jobInfo.runtime_env.env_vars ? jobInfo.runtime_env.env_vars.RAY_IMAGE : "Unknown"
 
   const status = jobInfo.status.toLowerCase()

plugins/plugin-codeflare/src/controller/run/get.ts

@@ -23,6 +23,7 @@ import { setTabReadonly } from "@kui-shell/plugin-madwizard"
 
 import { productName } from "@kui-shell/client/config.d/name.json"
 
+import Status from "../status"
 import { width, height } from "../dashboard"
 import { getJobDefinition } from "../description"
 
@@ -48,8 +49,6 @@ function capitalize(str: string) {
   return str[0].toUpperCase() + str.slice(1)
 }
 
-/** TODO: these are specific to Ray */
-type Status = "SUCCEEDED" | "STOPPED" | "RUNNING" | "PENDING" | "ERROR"
 function statusColor(status: Status) {
   switch (status) {
     case "SUCCEEDED":

plugins/plugin-codeflare/src/controller/status.ts

@@ -0,0 +1,20 @@
+/*
+ * Copyright 2022 The Kubernetes Authors
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+/** TODO: these are specific to Ray */
+type Status = "SUCCEEDED" | "STOPPED" | "RUNNING" | "PENDING" | "ERROR"
+
+export default Status

plugins/plugin-codeflare/src/tray/menus/profiles/active-runs.ts

@@ -14,9 +14,9 @@
  * limitations under the License.
  */
 
-import { basename } from "path"
 import { MenuItemConstructorOptions } from "electron"
 import { CreateWindowFunction } from "@kui-shell/core"
+import { productName } from "@kui-shell/client/config.d/name.json"
 
 import { rayIcon } from "../../icons"
 import UpdateFunction from "../../update"
@@ -27,10 +27,10 @@ import { runMenuItems } from "./runs"
 /** active run watcher per profile */
 const watchers: Record<string, ProfileActiveRunWatcher> = {}
 
-/** This is the utility function that will open a CodeFlare Dashboard window, pointing to the given local filesystem `logdir` */
-async function openDashboard(createWindow: CreateWindowFunction, logdir: string, follow = true) {
-  await createWindow(["codeflare", "dashboard", follow ? "-f" : "", logdir], {
-    title: "CodeFlare Dashboard - " + basename(logdir),
+/** This is the utility function that will open a CodeFlare Dashboard window and attaches to the given job */
+async function openDashboard(createWindow: CreateWindowFunction, profile: string, runId: string, follow = true) {
+  await createWindow(["codeflare", "dashboard", follow ? "-f" : "", "-a", runId, "-p", profile], {
+    title: productName + " Dashboard - " + runId,
   })
 }
 
@@ -53,7 +53,7 @@ async function openMenuItem(this: CreateWindowFunction, profile: string, runId:
   // ^^^ disabled... due to the FIXME
 
   // otherwise, we will need to start of a log aggregator
-  process.env.NO_WAIT = "true" // don't wait for job termination
+  /* process.env.NO_WAIT = "true" // don't wait for job termination
   process.env.QUIET_CONSOLE = "true" // don't tee logs to the console
   const { attach } = await import("../../../controller/attach")
   const { logdir, cleanExit } = await attach(profile, runId, { verbose: true })
@@ -72,7 +72,8 @@ async function openMenuItem(this: CreateWindowFunction, profile: string, runId:
     if (typeof cleanExit === "function") {
       cleanExit()
     }
-  }
+    } */
+  await openDashboard(this, profile, runId)
 }
 
 /** @return menu items that allow attaching to an active run in the given `profileName` */

plugins/plugin-codeflare/src/tray/watchers/profile/active-runs.ts

@@ -38,7 +38,7 @@ export default class ProfileActiveRunWatcher {
     private readonly updateFn: UpdateFunction,
     private readonly profile: string,
     public readonly rayAddress = ProfileActiveRunWatcher.initWatcher(profile),
-    private readonly timer = setInterval(() => this.updateActiveJobs(), 5000)
+    private readonly timer = setInterval(() => this.updateActiveJobs(), 3000)
   ) {
     onRun(() => this.updateActiveJobs())
   }