[UI] Pass namespace to APIs (kubeflow#2676)

* Format other frontend code using prettier * Regenerate frontend api clients * Pass namespace to CreateRun api request * Fetch logs from pod namespace * Fix log handler cannot work with pod name only query bug * Fix and refactor existing tests * Unit test adding namespace to create run api call. * Remove unneeded snapshot * Fix lint errors * Consistently use resource reference id for namespace
Jeffwan · Dec 9, 2020 · a3a2564 · a3a2564
1 parent 8e4b715
commit a3a2564
Show file tree

Hide file tree

Showing 16 changed files with 314 additions and 1,344 deletions.
diff --git a/frontend/server/k8s-helper.ts b/frontend/server/k8s-helper.ts
@@ -158,11 +158,11 @@ export function waitForTensorboardInstance(logdir: string, timeout: number): Pro
   });
 }
 
-export function getPodLogs(podName: string): Promise<string> {
+export function getPodLogs(podName: string, podNamespace?: string): Promise<string> {
   if (!k8sV1Client) {
     throw new Error('Cannot access kubernetes API');
   }
-  return (k8sV1Client.readNamespacedPodLog(podName, namespace, 'main') as any).then(
+  return (k8sV1Client.readNamespacedPodLog(podName, podNamespace || namespace, 'main') as any).then(
     (response: any) => (response && response.body ? response.body.toString() : ''),
     (error: any) => {
       throw new Error(JSON.stringify(error.body));

diff --git a/frontend/server/server.ts b/frontend/server/server.ts
@@ -333,14 +333,18 @@ const logsHandler = async (req, res) => {
     return;
   }
 
-  const podName = decodeURIComponent(req.query.podname);
-  if (!podName) {
+  if (!req.query.podname) {
     res.status(404).send('podname argument is required');
     return;
   }
+  const podName = decodeURIComponent(req.query.podname);
+
+  // 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 podLogsHandler.getPodLogs(podName);
+    const stream = await podLogsHandler.getPodLogs(podName, podNamespace);
     stream.on('error', err => res.status(500).send('Could not get main container logs: ' + err));
     stream.on('end', () => res.end());
     stream.pipe(res);

diff --git a/frontend/server/workflow-helper.ts b/frontend/server/workflow-helper.ts
@@ -90,19 +90,20 @@ function workflowNameFromPodName(podName: string) {
 export class PodLogsHandler {
   fromConfig?: (podName: string) => Promise<IMinioRequestConfig>;
 
-  async getPodLogs(podName: string) {
+  async getPodLogs(podName: string, podNamespace?: string) {
     try {
       // retrieve from k8s
       const stream = new PassThrough();
-      stream.end(await getPodLogs(podName));
-      console.log(`Getting logs for pod:${podName}.`);
+      stream.end(await getPodLogs(podName, podNamespace));
+      console.log(`Getting logs for pod:${podName} in namespace ${podNamespace}.`);
       return stream;
     } catch (k8sError) {
-      console.error(`Unable to get logs for pod:${podName}: ${k8sError}`);
+      console.error(`Unable to get logs for pod:${podName}:`, k8sError);
       return this.getPodLogsFromArchive(podName);
     }
   }
 
+  // TODO: support pod in a certain namespace
   async getPodLogsFromArchive(podName: string) {
     try {
       // try argo workflow crd status
@@ -118,14 +119,13 @@ export class PodLogsHandler {
           console.log(`Getting logs for pod:${podName} from ${request.bucket}/${request.key}.`);
           return stream;
         } catch (configError) {
-          console.error(`Unable to get logs for pod:${podName}: ${configError}`);
+          console.error(`Unable to get logs for pod:${podName}:`, configError);
           throw new Error(
             `Unable to retrieve logs from ${podName}: ${workflowError}, ${configError}`,
           );
         }
       }
-      console.error(`Unable to get logs for pod:${podName}: ${workflowError}`);
-      console.error(workflowError);
+      console.error(`Unable to get logs for pod:${podName}:`, workflowError);
       throw new Error(`Unable to retrieve logs from ${podName}: ${workflowError}`);
     }
   }

diff --git a/frontend/src/apis/job/api.ts b/frontend/src/apis/job/api.ts
@@ -371,6 +371,7 @@ export enum ApiResourceType {
   JOB = <any>'JOB',
   PIPELINE = <any>'PIPELINE',
   PIPELINEVERSION = <any>'PIPELINE_VERSION',
+  NAMESPACE = <any>'NAMESPACE',
 }
 
 /**
@@ -705,7 +706,7 @@ export const JobServiceApiFetchParamCreator = function(configuration?: Configura
      * @param {string} [page_token]
      * @param {number} [page_size]
      * @param {string} [sort_by] Can be format of \&quot;field_name\&quot;, \&quot;field_name asc\&quot; or \&quot;field_name des\&quot;. Ascending by default.
-     * @param {'UNKNOWN_RESOURCE_TYPE' | 'EXPERIMENT' | 'JOB' | 'PIPELINE' | 'PIPELINE_VERSION'} [resource_reference_key_type] The type of the resource that referred to.
+     * @param {'UNKNOWN_RESOURCE_TYPE' | 'EXPERIMENT' | 'JOB' | 'PIPELINE' | 'PIPELINE_VERSION' | 'NAMESPACE'} [resource_reference_key_type] The type of the resource that referred to.
      * @param {string} [resource_reference_key_id] The ID of the resource that referred to.
      * @param {string} [filter] A url-encoded, JSON-serialized Filter protocol buffer (see [filter.proto](https://github.com/kubeflow/pipelines/ blob/master/backend/api/filter.proto)).
      * @param {*} [options] Override http request option.
@@ -720,7 +721,8 @@ export const JobServiceApiFetchParamCreator = function(configuration?: Configura
         | 'EXPERIMENT'
         | 'JOB'
         | 'PIPELINE'
-        | 'PIPELINE_VERSION',
+        | 'PIPELINE_VERSION'
+        | 'NAMESPACE',
       resource_reference_key_id?: string,
       filter?: string,
       options: any = {},
@@ -904,7 +906,7 @@ export const JobServiceApiFp = function(configuration?: Configuration) {
      * @param {string} [page_token]
      * @param {number} [page_size]
      * @param {string} [sort_by] Can be format of \&quot;field_name\&quot;, \&quot;field_name asc\&quot; or \&quot;field_name des\&quot;. Ascending by default.
-     * @param {'UNKNOWN_RESOURCE_TYPE' | 'EXPERIMENT' | 'JOB' | 'PIPELINE' | 'PIPELINE_VERSION'} [resource_reference_key_type] The type of the resource that referred to.
+     * @param {'UNKNOWN_RESOURCE_TYPE' | 'EXPERIMENT' | 'JOB' | 'PIPELINE' | 'PIPELINE_VERSION' | 'NAMESPACE'} [resource_reference_key_type] The type of the resource that referred to.
      * @param {string} [resource_reference_key_id] The ID of the resource that referred to.
      * @param {string} [filter] A url-encoded, JSON-serialized Filter protocol buffer (see [filter.proto](https://github.com/kubeflow/pipelines/ blob/master/backend/api/filter.proto)).
      * @param {*} [options] Override http request option.
@@ -919,7 +921,8 @@ export const JobServiceApiFp = function(configuration?: Configuration) {
         | 'EXPERIMENT'
         | 'JOB'
         | 'PIPELINE'
-        | 'PIPELINE_VERSION',
+        | 'PIPELINE_VERSION'
+        | 'NAMESPACE',
       resource_reference_key_id?: string,
       filter?: string,
       options?: any,
@@ -1012,7 +1015,7 @@ export const JobServiceApiFactory = function(
      * @param {string} [page_token]
      * @param {number} [page_size]
      * @param {string} [sort_by] Can be format of \&quot;field_name\&quot;, \&quot;field_name asc\&quot; or \&quot;field_name des\&quot;. Ascending by default.
-     * @param {'UNKNOWN_RESOURCE_TYPE' | 'EXPERIMENT' | 'JOB' | 'PIPELINE' | 'PIPELINE_VERSION'} [resource_reference_key_type] The type of the resource that referred to.
+     * @param {'UNKNOWN_RESOURCE_TYPE' | 'EXPERIMENT' | 'JOB' | 'PIPELINE' | 'PIPELINE_VERSION' | 'NAMESPACE'} [resource_reference_key_type] The type of the resource that referred to.
      * @param {string} [resource_reference_key_id] The ID of the resource that referred to.
      * @param {string} [filter] A url-encoded, JSON-serialized Filter protocol buffer (see [filter.proto](https://github.com/kubeflow/pipelines/ blob/master/backend/api/filter.proto)).
      * @param {*} [options] Override http request option.
@@ -1027,7 +1030,8 @@ export const JobServiceApiFactory = function(
         | 'EXPERIMENT'
         | 'JOB'
         | 'PIPELINE'
-        | 'PIPELINE_VERSION',
+        | 'PIPELINE_VERSION'
+        | 'NAMESPACE',
       resource_reference_key_id?: string,
       filter?: string,
       options?: any,
@@ -1118,7 +1122,7 @@ export class JobServiceApi extends BaseAPI {
    * @param {string} [page_token]
    * @param {number} [page_size]
    * @param {string} [sort_by] Can be format of \&quot;field_name\&quot;, \&quot;field_name asc\&quot; or \&quot;field_name des\&quot;. Ascending by default.
-   * @param {'UNKNOWN_RESOURCE_TYPE' | 'EXPERIMENT' | 'JOB' | 'PIPELINE' | 'PIPELINE_VERSION'} [resource_reference_key_type] The type of the resource that referred to.
+   * @param {'UNKNOWN_RESOURCE_TYPE' | 'EXPERIMENT' | 'JOB' | 'PIPELINE' | 'PIPELINE_VERSION' | 'NAMESPACE'} [resource_reference_key_type] The type of the resource that referred to.
    * @param {string} [resource_reference_key_id] The ID of the resource that referred to.
    * @param {string} [filter] A url-encoded, JSON-serialized Filter protocol buffer (see [filter.proto](https://github.com/kubeflow/pipelines/ blob/master/backend/api/filter.proto)).
    * @param {*} [options] Override http request option.
@@ -1134,7 +1138,8 @@ export class JobServiceApi extends BaseAPI {
       | 'EXPERIMENT'
       | 'JOB'
       | 'PIPELINE'
-      | 'PIPELINE_VERSION',
+      | 'PIPELINE_VERSION'
+      | 'NAMESPACE',
     resource_reference_key_id?: string,
     filter?: string,
     options?: any,

diff --git a/frontend/src/apis/pipeline/api.ts b/frontend/src/apis/pipeline/api.ts
@@ -341,6 +341,7 @@ export enum ApiResourceType {
   JOB = <any>'JOB',
   PIPELINE = <any>'PIPELINE',
   PIPELINEVERSION = <any>'PIPELINE_VERSION',
+  NAMESPACE = <any>'NAMESPACE',
 }
 
 /**
@@ -797,7 +798,7 @@ export const PipelineServiceApiFetchParamCreator = function(configuration?: Conf
     },
     /**
      *
-     * @param {'UNKNOWN_RESOURCE_TYPE' | 'EXPERIMENT' | 'JOB' | 'PIPELINE' | 'PIPELINE_VERSION'} [resource_key_type] The type of the resource that referred to.
+     * @param {'UNKNOWN_RESOURCE_TYPE' | 'EXPERIMENT' | 'JOB' | 'PIPELINE' | 'PIPELINE_VERSION' | 'NAMESPACE'} [resource_key_type] The type of the resource that referred to.
      * @param {string} [resource_key_id] The ID of the resource that referred to.
      * @param {number} [page_size]
      * @param {string} [page_token]
@@ -812,7 +813,8 @@ export const PipelineServiceApiFetchParamCreator = function(configuration?: Conf
         | 'EXPERIMENT'
         | 'JOB'
         | 'PIPELINE'
-        | 'PIPELINE_VERSION',
+        | 'PIPELINE_VERSION'
+        | 'NAMESPACE',
       resource_key_id?: string,
       page_size?: number,
       page_token?: string,
@@ -1140,7 +1142,7 @@ export const PipelineServiceApiFp = function(configuration?: Configuration) {
     },
     /**
      *
-     * @param {'UNKNOWN_RESOURCE_TYPE' | 'EXPERIMENT' | 'JOB' | 'PIPELINE' | 'PIPELINE_VERSION'} [resource_key_type] The type of the resource that referred to.
+     * @param {'UNKNOWN_RESOURCE_TYPE' | 'EXPERIMENT' | 'JOB' | 'PIPELINE' | 'PIPELINE_VERSION' | 'NAMESPACE'} [resource_key_type] The type of the resource that referred to.
      * @param {string} [resource_key_id] The ID of the resource that referred to.
      * @param {number} [page_size]
      * @param {string} [page_token]
@@ -1155,7 +1157,8 @@ export const PipelineServiceApiFp = function(configuration?: Configuration) {
         | 'EXPERIMENT'
         | 'JOB'
         | 'PIPELINE'
-        | 'PIPELINE_VERSION',
+        | 'PIPELINE_VERSION'
+        | 'NAMESPACE',
       resource_key_id?: string,
       page_size?: number,
       page_token?: string,
@@ -1321,7 +1324,7 @@ export const PipelineServiceApiFactory = function(
     },
     /**
      *
-     * @param {'UNKNOWN_RESOURCE_TYPE' | 'EXPERIMENT' | 'JOB' | 'PIPELINE' | 'PIPELINE_VERSION'} [resource_key_type] The type of the resource that referred to.
+     * @param {'UNKNOWN_RESOURCE_TYPE' | 'EXPERIMENT' | 'JOB' | 'PIPELINE' | 'PIPELINE_VERSION' | 'NAMESPACE'} [resource_key_type] The type of the resource that referred to.
      * @param {string} [resource_key_id] The ID of the resource that referred to.
      * @param {number} [page_size]
      * @param {string} [page_token]
@@ -1336,7 +1339,8 @@ export const PipelineServiceApiFactory = function(
         | 'EXPERIMENT'
         | 'JOB'
         | 'PIPELINE'
-        | 'PIPELINE_VERSION',
+        | 'PIPELINE_VERSION'
+        | 'NAMESPACE',
       resource_key_id?: string,
       page_size?: number,
       page_token?: string,
@@ -1507,7 +1511,7 @@ export class PipelineServiceApi extends BaseAPI {
 
   /**
    *
-   * @param {'UNKNOWN_RESOURCE_TYPE' | 'EXPERIMENT' | 'JOB' | 'PIPELINE' | 'PIPELINE_VERSION'} [resource_key_type] The type of the resource that referred to.
+   * @param {'UNKNOWN_RESOURCE_TYPE' | 'EXPERIMENT' | 'JOB' | 'PIPELINE' | 'PIPELINE_VERSION' | 'NAMESPACE'} [resource_key_type] The type of the resource that referred to.
    * @param {string} [resource_key_id] The ID of the resource that referred to.
    * @param {number} [page_size]
    * @param {string} [page_token]
@@ -1523,7 +1527,8 @@ export class PipelineServiceApi extends BaseAPI {
       | 'EXPERIMENT'
       | 'JOB'
       | 'PIPELINE'
-      | 'PIPELINE_VERSION',
+      | 'PIPELINE_VERSION'
+      | 'NAMESPACE',
     resource_key_id?: string,
     page_size?: number,
     page_token?: string,

diff --git a/frontend/src/apis/run/api.ts b/frontend/src/apis/run/api.ts
@@ -301,6 +301,7 @@ export enum ApiResourceType {
   JOB = <any>'JOB',
   PIPELINE = <any>'PIPELINE',
   PIPELINEVERSION = <any>'PIPELINE_VERSION',
+  NAMESPACE = <any>'NAMESPACE',
 }
 
 /**
@@ -754,7 +755,7 @@ export const RunServiceApiFetchParamCreator = function(configuration?: Configura
      * @param {string} [page_token]
      * @param {number} [page_size]
      * @param {string} [sort_by] Can be format of \&quot;field_name\&quot;, \&quot;field_name asc\&quot; or \&quot;field_name des\&quot; (Example, \&quot;name asc\&quot; or \&quot;id des\&quot;). Ascending by default.
-     * @param {'UNKNOWN_RESOURCE_TYPE' | 'EXPERIMENT' | 'JOB' | 'PIPELINE' | 'PIPELINE_VERSION'} [resource_reference_key_type] The type of the resource that referred to.
+     * @param {'UNKNOWN_RESOURCE_TYPE' | 'EXPERIMENT' | 'JOB' | 'PIPELINE' | 'PIPELINE_VERSION' | 'NAMESPACE'} [resource_reference_key_type] The type of the resource that referred to.
      * @param {string} [resource_reference_key_id] The ID of the resource that referred to.
      * @param {string} [filter] A url-encoded, JSON-serialized Filter protocol buffer (see [filter.proto](https://github.com/kubeflow/pipelines/ blob/master/backend/api/filter.proto)).
      * @param {*} [options] Override http request option.
@@ -769,7 +770,8 @@ export const RunServiceApiFetchParamCreator = function(configuration?: Configura
         | 'EXPERIMENT'
         | 'JOB'
         | 'PIPELINE'
-        | 'PIPELINE_VERSION',
+        | 'PIPELINE_VERSION'
+        | 'NAMESPACE',
       resource_reference_key_id?: string,
       filter?: string,
       options: any = {},
@@ -1216,7 +1218,7 @@ export const RunServiceApiFp = function(configuration?: Configuration) {
      * @param {string} [page_token]
      * @param {number} [page_size]
      * @param {string} [sort_by] Can be format of \&quot;field_name\&quot;, \&quot;field_name asc\&quot; or \&quot;field_name des\&quot; (Example, \&quot;name asc\&quot; or \&quot;id des\&quot;). Ascending by default.
-     * @param {'UNKNOWN_RESOURCE_TYPE' | 'EXPERIMENT' | 'JOB' | 'PIPELINE' | 'PIPELINE_VERSION'} [resource_reference_key_type] The type of the resource that referred to.
+     * @param {'UNKNOWN_RESOURCE_TYPE' | 'EXPERIMENT' | 'JOB' | 'PIPELINE' | 'PIPELINE_VERSION' | 'NAMESPACE'} [resource_reference_key_type] The type of the resource that referred to.
      * @param {string} [resource_reference_key_id] The ID of the resource that referred to.
      * @param {string} [filter] A url-encoded, JSON-serialized Filter protocol buffer (see [filter.proto](https://github.com/kubeflow/pipelines/ blob/master/backend/api/filter.proto)).
      * @param {*} [options] Override http request option.
@@ -1231,7 +1233,8 @@ export const RunServiceApiFp = function(configuration?: Configuration) {
         | 'EXPERIMENT'
         | 'JOB'
         | 'PIPELINE'
-        | 'PIPELINE_VERSION',
+        | 'PIPELINE_VERSION'
+        | 'NAMESPACE',
       resource_reference_key_id?: string,
       filter?: string,
       options?: any,
@@ -1442,7 +1445,7 @@ export const RunServiceApiFactory = function(
      * @param {string} [page_token]
      * @param {number} [page_size]
      * @param {string} [sort_by] Can be format of \&quot;field_name\&quot;, \&quot;field_name asc\&quot; or \&quot;field_name des\&quot; (Example, \&quot;name asc\&quot; or \&quot;id des\&quot;). Ascending by default.
-     * @param {'UNKNOWN_RESOURCE_TYPE' | 'EXPERIMENT' | 'JOB' | 'PIPELINE' | 'PIPELINE_VERSION'} [resource_reference_key_type] The type of the resource that referred to.
+     * @param {'UNKNOWN_RESOURCE_TYPE' | 'EXPERIMENT' | 'JOB' | 'PIPELINE' | 'PIPELINE_VERSION' | 'NAMESPACE'} [resource_reference_key_type] The type of the resource that referred to.
      * @param {string} [resource_reference_key_id] The ID of the resource that referred to.
      * @param {string} [filter] A url-encoded, JSON-serialized Filter protocol buffer (see [filter.proto](https://github.com/kubeflow/pipelines/ blob/master/backend/api/filter.proto)).
      * @param {*} [options] Override http request option.
@@ -1457,7 +1460,8 @@ export const RunServiceApiFactory = function(
         | 'EXPERIMENT'
         | 'JOB'
         | 'PIPELINE'
-        | 'PIPELINE_VERSION',
+        | 'PIPELINE_VERSION'
+        | 'NAMESPACE',
       resource_reference_key_id?: string,
       filter?: string,
       options?: any,
@@ -1595,7 +1599,7 @@ export class RunServiceApi extends BaseAPI {
    * @param {string} [page_token]
    * @param {number} [page_size]
    * @param {string} [sort_by] Can be format of \&quot;field_name\&quot;, \&quot;field_name asc\&quot; or \&quot;field_name des\&quot; (Example, \&quot;name asc\&quot; or \&quot;id des\&quot;). Ascending by default.
-   * @param {'UNKNOWN_RESOURCE_TYPE' | 'EXPERIMENT' | 'JOB' | 'PIPELINE' | 'PIPELINE_VERSION'} [resource_reference_key_type] The type of the resource that referred to.
+   * @param {'UNKNOWN_RESOURCE_TYPE' | 'EXPERIMENT' | 'JOB' | 'PIPELINE' | 'PIPELINE_VERSION' | 'NAMESPACE'} [resource_reference_key_type] The type of the resource that referred to.
    * @param {string} [resource_reference_key_id] The ID of the resource that referred to.
    * @param {string} [filter] A url-encoded, JSON-serialized Filter protocol buffer (see [filter.proto](https://github.com/kubeflow/pipelines/ blob/master/backend/api/filter.proto)).
    * @param {*} [options] Override http request option.
@@ -1611,7 +1615,8 @@ export class RunServiceApi extends BaseAPI {
       | 'EXPERIMENT'
       | 'JOB'
       | 'PIPELINE'
-      | 'PIPELINE_VERSION',
+      | 'PIPELINE_VERSION'
+      | 'NAMESPACE',
     resource_reference_key_id?: string,
     filter?: string,
     options?: any,

diff --git a/frontend/src/lib/Apis.test.ts b/frontend/src/lib/Apis.test.ts
@@ -55,6 +55,19 @@ describe('Apis', () => {
     });
   });
 
+  it('getPodLogs in a specific namespace', async () => {
+    const spy = fetchSpy('http://some/address');
+    expect(await Apis.getPodLogs('some-pod-name', 'some-namespace-name')).toEqual(
+      'http://some/address',
+    );
+    expect(spy).toHaveBeenCalledWith(
+      'k8s/pod/logs?podname=some-pod-name&podnamespace=some-namespace-name',
+      {
+        credentials: 'same-origin',
+      },
+    );
+  });
+
   it('getPodLogs error', async () => {
     jest.spyOn(console, 'error').mockImplementation(() => null);
     window.fetch = jest.fn(() =>
@@ -64,6 +77,9 @@ describe('Apis', () => {
       }),
     );
     expect(Apis.getPodLogs('some-pod-name')).rejects.toThrowError('bad response');
+    expect(Apis.getPodLogs('some-pod-name', 'some-namespace-name')).rejects.toThrowError(
+      'bad response',
+    );
   });
 
   it('getBuildInfo returns build information', async () => {

diff --git a/frontend/src/lib/Apis.ts b/frontend/src/lib/Apis.ts
@@ -165,8 +165,12 @@ export class Apis {
   /**
    * Get pod logs
    */
-  public static getPodLogs(podName: string): Promise<string> {
-    return this._fetch(`k8s/pod/logs?podname=${encodeURIComponent(podName)}`);
+  public static getPodLogs(podName: string, podNamespace?: string): Promise<string> {
+    let query = `k8s/pod/logs?podname=${encodeURIComponent(podName)}`;
+    if (podNamespace) {
+      query += `&podnamespace=${encodeURIComponent(podNamespace)}`;
+    }
+    return this._fetch(query);
   }
 
   public static get basePath(): string {

diff --git a/frontend/src/lib/KubeflowClient.tsx b/frontend/src/lib/KubeflowClient.tsx
@@ -35,8 +35,7 @@ export function init(): void {
   }
 }
 
-const NamespaceContext = React.createContext<string | undefined>(undefined);
-export const NamespaceContextConsumer = NamespaceContext.Consumer;
+export const NamespaceContext = React.createContext<string | undefined>(undefined);
 export class NamespaceContextProvider extends React.Component {
   state = {
     namespace,