Skip to content

Commit

Permalink
[UI] Pass namespace to APIs (#2676)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
Bobgy authored and k8s-ci-robot committed Dec 17, 2019
1 parent 3a84418 commit e86e409
Show file tree
Hide file tree
Showing 16 changed files with 314 additions and 1,344 deletions.
4 changes: 2 additions & 2 deletions frontend/server/k8s-helper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand Down
10 changes: 7 additions & 3 deletions frontend/server/server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
14 changes: 7 additions & 7 deletions frontend/server/workflow-helper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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}`);
}
}
Expand Down
21 changes: 13 additions & 8 deletions frontend/src/apis/job/api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -371,6 +371,7 @@ export enum ApiResourceType {
JOB = <any>'JOB',
PIPELINE = <any>'PIPELINE',
PIPELINEVERSION = <any>'PIPELINE_VERSION',
NAMESPACE = <any>'NAMESPACE',
}

/**
Expand Down Expand Up @@ -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.
Expand All @@ -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 = {},
Expand Down Expand Up @@ -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.
Expand All @@ -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,
Expand Down Expand Up @@ -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.
Expand All @@ -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,
Expand Down Expand Up @@ -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.
Expand All @@ -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,
Expand Down
21 changes: 13 additions & 8 deletions frontend/src/apis/pipeline/api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -341,6 +341,7 @@ export enum ApiResourceType {
JOB = <any>'JOB',
PIPELINE = <any>'PIPELINE',
PIPELINEVERSION = <any>'PIPELINE_VERSION',
NAMESPACE = <any>'NAMESPACE',
}

/**
Expand Down Expand Up @@ -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]
Expand All @@ -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,
Expand Down Expand Up @@ -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]
Expand All @@ -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,
Expand Down Expand Up @@ -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]
Expand All @@ -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,
Expand Down Expand Up @@ -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]
Expand All @@ -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,
Expand Down
21 changes: 13 additions & 8 deletions frontend/src/apis/run/api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -301,6 +301,7 @@ export enum ApiResourceType {
JOB = <any>'JOB',
PIPELINE = <any>'PIPELINE',
PIPELINEVERSION = <any>'PIPELINE_VERSION',
NAMESPACE = <any>'NAMESPACE',
}

/**
Expand Down Expand Up @@ -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.
Expand All @@ -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 = {},
Expand Down Expand Up @@ -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.
Expand All @@ -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,
Expand Down Expand Up @@ -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.
Expand All @@ -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,
Expand Down Expand Up @@ -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.
Expand All @@ -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,
Expand Down
16 changes: 16 additions & 0 deletions frontend/src/lib/Apis.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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(() =>
Expand All @@ -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 () => {
Expand Down
8 changes: 6 additions & 2 deletions frontend/src/lib/Apis.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
3 changes: 1 addition & 2 deletions frontend/src/lib/KubeflowClient.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
Loading

0 comments on commit e86e409

Please sign in to comment.