feat(ui) Update UI to get archived logs if available (#305)

* Update UI to get archived logs if available * Fix formatting * Add note in admin guide * Add note in admin guide * Fix typo from #299 and lint format * Add bracket
red-hat-data-services · Sep 17, 2020 · 55a9085 · 55a9085
1 parent b77a1ea
commit 55a9085
Show file tree

Hide file tree

Showing 7 changed files with 85 additions and 31 deletions.
diff --git a/frontend/server/app.ts b/frontend/server/app.ts
@@ -138,25 +138,26 @@ function createUIServer(options: UIConfigs) {
   registerHandler(app.delete, '/apps/tensorboard', tensorboardDeleteHandler);
   registerHandler(app.post, '/apps/tensorboard', tensorboardCreateHandler);
 
-    /** Pod logs - conditionally stream through API server, otherwise directly from k8s and archive */
+  /** Pod logs - conditionally stream through API server, otherwise directly from k8s and archive */
   if (options.artifacts.streamLogsFromServerApi) {
     app.all(
-        '/k8s/pod/logs',
-        proxy({
-          changeOrigin: true,
-          onProxyReq: proxyReq => {
-            console.log('Proxied log request: ', proxyReq.path);
-          },
-          pathRewrite: (pathStr: string, req: any) => {
-            const nodeId = req.query.podname;
-            const runId = req.query.runid;
-            return `/${apiVersionPrefix}/runs/${runId}/nodes/${nodeId}/log`;
-          },
-          target: apiServerAddress,
-        }),
+      '/k8s/pod/logs',
+      proxy({
+        changeOrigin: true,
+        onProxyReq: proxyReq => {
+          console.log('Proxied log request: ', proxyReq.path);
+        },
+        pathRewrite: (pathStr: string, req: any) => {
+          const nodeId = req.query.podname;
+          const runId = req.query.runid;
+          return `/${apiVersionPrefix}/runs/${runId}/nodes/${nodeId}/log`;
+        },
+        target: apiServerAddress,
+      }),
     );
   } else {
-  registerHandler(app.get, '/k8s/pod/logs', getPodLogsHandler(options.argo, options.artifacts));
+    registerHandler(app.get, '/k8s/pod/logs', getPodLogsHandler(options.argo, options.artifacts));
+  }
   /** Pod info */
   registerHandler(app.get, '/k8s/pod', podInfoHandler);
   registerHandler(app.get, '/k8s/pod/events', podEventsHandler);

diff --git a/frontend/server/configs.ts b/frontend/server/configs.ts
@@ -85,7 +85,7 @@ export function loadConfigs(argv: string[], env: ProcessEnv): UIConfigs {
     /** Bucket to retrive logs from */
     ARGO_ARCHIVE_BUCKETNAME = 'mlpipeline',
     /** Prefix to logs. */
-    ARGO_ARCHIVE_PREFIX = 'logs',
+    ARGO_ARCHIVE_PREFIX = 'artifacts',
     /** Should use server API for log streaming? */
     STREAM_LOGS_FROM_SERVER_API = 'false',
     /** Disables GKE metadata endpoint. */

diff --git a/frontend/server/handlers/pod-logs.ts b/frontend/server/handlers/pod-logs.ts
@@ -66,13 +66,14 @@ export function getPodLogsHandler(
       return;
     }
     const podName = decodeURIComponent(req.query.podname);
+    const taskName = decodeURIComponent(req.query.taskname) || undefined;
 
     // This is optional.
     // Note decodeURIComponent(undefined) === 'undefined', so I cannot pass the argument directly.
     const podNamespace = decodeURIComponent(req.query.podnamespace || '') || undefined;
 
     try {
-      const stream = await getPodLogsStream(podName, podNamespace);
+      const stream = await getPodLogsStream(podName, podNamespace, taskName);
       stream.on('error', err => {
         if (
           err?.message &&

diff --git a/frontend/server/workflow-helper.ts b/frontend/server/workflow-helper.ts
@@ -11,7 +11,7 @@
 // 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.
-import path from 'path';
+import path, { join } from 'path';
 import { PassThrough, Stream } from 'stream';
 import { ClientOptions as MinioClientOptions } from 'minio';
 import { getK8sSecret, getArgoWorkflow, getPodLogs } from './k8s-helper';
@@ -61,15 +61,15 @@ export interface SecretSelector {
  * fails.
  */
 export function composePodLogsStreamHandler<T = Stream>(
-  handler: (podName: string, namespace?: string) => Promise<T>,
-  fallback?: (podName: string, namespace?: string) => Promise<T>,
+  handler: (podName: string, namespace?: string, taskName?: string) => Promise<T>,
+  fallback?: (podName: string, namespace?: string, taskName?: string) => Promise<T>,
 ) {
-  return async (podName: string, namespace?: string) => {
+  return async (podName: string, namespace?: string, taskName?: string) => {
     try {
-      return await handler(podName, namespace);
+      return await handler(podName, namespace, taskName);
     } catch (err) {
       if (fallback) {
-        return await fallback(podName, namespace);
+        return await fallback(podName, namespace, taskName);
       }
       console.warn(err);
       throw err;
@@ -81,8 +81,13 @@ export function composePodLogsStreamHandler<T = Stream>(
  * Returns a stream containing the pod logs using kubernetes api.
  * @param podName name of the pod.
  * @param namespace namespace of the pod (uses the same namespace as the server if not provided).
+ * @param taskName name of the task.
  */
-export async function getPodLogsStreamFromK8s(podName: string, namespace?: string) {
+export async function getPodLogsStreamFromK8s(
+  podName: string,
+  namespace?: string,
+  taskName?: string,
+) {
   const stream = new PassThrough();
   stream.end(await getPodLogs(podName, namespace));
   console.log(`Getting logs for pod:${podName} in namespace ${namespace}.`);
@@ -94,6 +99,7 @@ export async function getPodLogsStreamFromK8s(podName: string, namespace?: strin
  * workflow status (uses k8s api to retrieve the workflow and secrets).
  * @param podName name of the pod.
  * @param namespace namespace of the pod (uses the same namespace as the server if not provided).
+ * @param taskName name of the task.
  */
 export const getPodLogsStreamFromWorkflow = toGetPodLogsStream(
   getPodLogsMinioRequestConfigfromWorkflow,
@@ -107,10 +113,14 @@ export const getPodLogsStreamFromWorkflow = toGetPodLogsStream(
  * on the provided pod name and namespace (optional).
  */
 export function toGetPodLogsStream(
-  getMinioRequestConfig: (podName: string, namespace?: string) => Promise<MinioRequestConfig>,
+  getMinioRequestConfig: (
+    podName: string,
+    namespace?: string,
+    taskName?: string,
+  ) => Promise<MinioRequestConfig>,
 ) {
-  return async (podName: string, namespace?: string) => {
-    const request = await getMinioRequestConfig(podName, namespace);
+  return async (podName: string, namespace?: string, taskName?: string) => {
+    const request = await getMinioRequestConfig(podName, namespace, taskName);
     console.log(`Getting logs for pod:${podName} from ${request.bucket}/${request.key}.`);
     return await getObjectStream(request);
   };
@@ -131,9 +141,24 @@ export function createPodLogsMinioRequestConfig(
 ) {
   // TODO: support pod log artifacts for diff namespace.
   // different bucket/prefix for diff namespace?
-  return async (podName: string, _namespace?: string): Promise<MinioRequestConfig> => {
+  return async (
+    podName: string,
+    _namespace?: string,
+    _taskName?: string,
+  ): Promise<MinioRequestConfig> => {
     // create a new client each time to ensure session token has not expired
     const client = await createMinioClient(minioOptions);
+
+    if (_taskName) {
+      const taskNameIndex = podName.indexOf(_taskName);
+      const workflowName = podName.substring(0, taskNameIndex - 1);
+      return {
+        bucket,
+        client,
+        key: path.join(prefix, workflowName, _taskName, 'main-log.tgz'),
+      };
+    }
+
     const workflowName = workflowNameFromPodName(podName);
     return {
       bucket,

diff --git a/frontend/src/lib/Apis.ts b/frontend/src/lib/Apis.ts
@@ -98,8 +98,15 @@ export class Apis {
   /**
    * Get pod logs
    */
-  public static getPodLogs(runId: string, podName: string, podNamespace: string): Promise<string> {
-    let query = `k8s/pod/logs?podname=${encodeURIComponent(podName)}&runid=${encodeURIComponent(runId)}`;
+  public static getPodLogs(
+    runId: string,
+    podName: string,
+    podNamespace: string,
+    taskName?: string,
+  ): Promise<string> {
+    let query = `k8s/pod/logs?podname=${encodeURIComponent(podName)}&runid=${encodeURIComponent(
+      runId,
+    )}${taskName ? '&taskname=' + encodeURIComponent(taskName) : ''}`;
     if (podNamespace) {
       query += `&podnamespace=${encodeURIComponent(podNamespace)}`;
     }

diff --git a/frontend/src/pages/RunDetails.tsx b/frontend/src/pages/RunDetails.tsx
@@ -933,7 +933,21 @@ class RunDetails extends Page<RunDetailsInternalProps, RunDetailsState> {
     let logsBannerMode = '' as Mode;
 
     try {
-      selectedNodeDetails.logs = await Apis.getPodLogs(runId, selectedNodeDetails.id, namespace);
+      const taskPodId = selectedNodeDetails.id;
+      let taskName = '';
+      for (const taskRunId of Object.getOwnPropertyNames(this.state.workflow.status.taskRuns)) {
+        const taskRun = this.state.workflow.status.taskRuns[taskRunId];
+        if (taskRun.status && taskRun.status.podName === taskPodId) {
+          taskName = taskRun.pipelineTaskName;
+        }
+      }
+
+      selectedNodeDetails.logs = await Apis.getPodLogs(
+        runId,
+        selectedNodeDetails.id,
+        namespace,
+        taskName,
+      );
     } catch (err) {
       let errMsg = await errorToMessage(err);
       logsBannerMessage = 'Failed to retrieve pod logs.';

diff --git a/kfp-admin-guide.md b/kfp-admin-guide.md
@@ -29,6 +29,12 @@ kubectl rollout restart deploy/ml-pipeline -n kubeflow
 kubectl rollout restart deploy/metadata-writer -n kubeflow
 ```
 
+Archived logs are disabled in the KFP-Tekton UI by default. To enable this feature, run the following command:
+
+```shell
+kubectl set env -n kubeflow deploy/ml-pipeline-ui ARGO_ARCHIVE_LOGS=true
+```
+
 ## Enable Auto Strip for End of File newlines
 
 Tekton by design are passing parameter outputs as it including unintentional End of File (EOF) newlines. Tekton are expecting users to know this behavior when designing their components. Therefore, the kfp-tekton team designed an experimental feature to auto strip the EOF newlines for a better user experience. This feature is disabled by default and only works for files that are not depended on EOF newlines. To enable this feature, run the following commands: