Skip to content

Commit

Permalink
Update frontend unit tests
Browse files Browse the repository at this point in the history
Signed-off-by: droctothorpe <[email protected]>
  • Loading branch information
droctothorpe committed Aug 13, 2024
1 parent b65348c commit b7e8141
Show file tree
Hide file tree
Showing 5 changed files with 92 additions and 43 deletions.
53 changes: 36 additions & 17 deletions frontend/server/workflow-helper.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,40 +39,49 @@ describe('workflow-helper', () => {
describe('composePodLogsStreamHandler', () => {
it('returns the stream from the default handler if there is no errors.', async () => {
const defaultStream = new PassThrough();
const defaultHandler = jest.fn((_podName: string, _namespace?: string) =>
const defaultHandler = jest.fn((_podName: string, _createdAt: string, _namespace?: string) =>
Promise.resolve(defaultStream),
);
const stream = await composePodLogsStreamHandler(defaultHandler)('podName', 'namespace');
expect(defaultHandler).toBeCalledWith('podName', 'namespace');
const stream = await composePodLogsStreamHandler(defaultHandler)(
'podName',
'2024-08-13',
'namespace',
);
expect(defaultHandler).toBeCalledWith('podName', '2024-08-13', 'namespace');
expect(stream).toBe(defaultStream);
});

it('returns the stream from the fallback handler if there is any error.', async () => {
const fallbackStream = new PassThrough();
const defaultHandler = jest.fn((_podName: string, _namespace?: string) =>
const defaultHandler = jest.fn((_podName: string, _createdAt: string, _namespace?: string) =>
Promise.reject('unknown error'),
);
const fallbackHandler = jest.fn((_podName: string, _namespace?: string) =>
const fallbackHandler = jest.fn((_podName: string, _createdAt: string, _namespace?: string) =>
Promise.resolve(fallbackStream),
);
const stream = await composePodLogsStreamHandler(defaultHandler, fallbackHandler)(
'podName',
'2024-08-13',
'namespace',
);
expect(defaultHandler).toBeCalledWith('podName', 'namespace');
expect(fallbackHandler).toBeCalledWith('podName', 'namespace');
expect(defaultHandler).toBeCalledWith('podName', '2024-08-13', 'namespace');
expect(fallbackHandler).toBeCalledWith('podName', '2024-08-13', 'namespace');
expect(stream).toBe(fallbackStream);
});

it('throws error if both handler and fallback fails.', async () => {
const defaultHandler = jest.fn((_podName: string, _namespace?: string) =>
const defaultHandler = jest.fn((_podName: string, _createdAt: string, _namespace?: string) =>
Promise.reject('unknown error for default'),
);
const fallbackHandler = jest.fn((_podName: string, _namespace?: string) =>
const fallbackHandler = jest.fn((_podName: string, _createdAt: string, _namespace?: string) =>
Promise.reject('unknown error for fallback'),
);
await expect(
composePodLogsStreamHandler(defaultHandler, fallbackHandler)('podName', 'namespace'),
composePodLogsStreamHandler(defaultHandler, fallbackHandler)(
'podName',
'2024-08-13',
'namespace',
),
).rejects.toEqual('unknown error for fallback');
});
});
Expand All @@ -82,7 +91,7 @@ describe('workflow-helper', () => {
const mockedGetPodLogs: jest.Mock = getPodLogs as any;
mockedGetPodLogs.mockResolvedValueOnce('pod logs');

const stream = await getPodLogsStreamFromK8s('podName', 'namespace');
const stream = await getPodLogsStreamFromK8s('podName', '', 'namespace');
expect(mockedGetPodLogs).toBeCalledWith('podName', 'namespace', 'main');
expect(stream.read().toString()).toBe('pod logs');
});
Expand All @@ -101,24 +110,34 @@ describe('workflow-helper', () => {
client,
key: 'folder/key',
};
const createRequest = jest.fn((_podName: string, _namespace?: string) =>
const createRequest = jest.fn((_podName: string, _createdAt: string, _namespace?: string) =>
Promise.resolve(configs),
);
const stream = await toGetPodLogsStream(createRequest)('podName', 'namespace');
const stream = await toGetPodLogsStream(createRequest)('podName', '2024-08-13', 'namespace');
expect(mockedClientGetObject).toBeCalledWith('bucket', 'folder/key');
});
});

describe('createPodLogsMinioRequestConfig', () => {
it('returns a MinioRequestConfig factory with the provided minioClientOptions, bucket, and prefix.', async () => {
const mockedClient: jest.Mock = MinioClient as any;
const requestFunc = await createPodLogsMinioRequestConfig(minioConfig, 'bucket', 'prefix');
const request = await requestFunc('workflow-name-abc', 'namespace');
const requestFunc = await createPodLogsMinioRequestConfig(
minioConfig,
'bucket',
'artifacts/{{workflow.name}}/{{workflow.creationTimestamp.Y}}/{{workflow.creationTimestamp.m}}/{{workflow.creationTimestamp.d}}/{{pod.name}}',
);
const request = await requestFunc(
'workflow-name-system-container-impl-foo',
'2024-08-13',
'namespace',
);

expect(mockedClient).toBeCalledWith(minioConfig);
expect(request.client).toBeInstanceOf(MinioClient);
expect(request.bucket).toBe('bucket');
expect(request.key).toBe('prefix/workflow-name/workflow-name-abc/main.log');
expect(request.key).toBe(
'artifacts/workflow-name/2024/08/13/workflow-name-system-container-impl-foo/main.log',
);
});
});

Expand Down Expand Up @@ -174,7 +193,7 @@ describe('workflow-helper', () => {
mockedClientGetObject.mockResolvedValueOnce(objStream);
objStream.end('some fake logs.');

const stream = await getPodLogsStreamFromWorkflow('workflow-name-abc', "2024-07-09");
const stream = await getPodLogsStreamFromWorkflow('workflow-name-abc', '2024-07-09');

expect(mockedGetArgoWorkflow).toBeCalledWith('workflow-name');

Expand Down
50 changes: 29 additions & 21 deletions frontend/server/workflow-helper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,11 @@ export const getPodLogsStreamFromWorkflow = toGetPodLogsStream(
* on the provided pod name and namespace (optional).
*/
export function toGetPodLogsStream(
getMinioRequestConfig: (podName: string, createdAt: string, namespace?: string) => Promise<MinioRequestConfig>,
getMinioRequestConfig: (
podName: string,
createdAt: string,
namespace?: string,
) => Promise<MinioRequestConfig>,
) {
return async (podName: string, createdAt: string, namespace?: string) => {
const request = await getMinioRequestConfig(podName, createdAt, namespace);
Expand All @@ -128,50 +132,54 @@ export function toGetPodLogsStream(
* client).
* @param minioOptions Minio options to create a minio client.
* @param bucket bucket containing the pod logs artifacts.
* @param prefix prefix for pod logs artifacts stored in the bucket.
* @param keyFormat the keyFormat for pod logs artifacts stored in the bucket.
*/
export function createPodLogsMinioRequestConfig(
minioOptions: MinioClientOptions,
bucket: string,
keyFormat: string,
) {
return async (podName: string, createdAt: string, namespace: string = ''): Promise<MinioRequestConfig> => {
return async (
podName: string,
createdAt: string,
namespace: string = '',
): Promise<MinioRequestConfig> => {
// create a new client each time to ensure session token has not expired
const client = await createMinioClient(minioOptions, 's3');
const createdAtArray = createdAt.split("-")
const createdAtArray = createdAt.split('-');

let key: string = keyFormat
.replace(/\s+/g, '') // Remove all whitespace.
.replace("{{workflow.name}}", podName.replace(/-system-container-impl-.*/, ''))
.replace("{{workflow.creationTimestamp.Y}}", createdAtArray[0])
.replace("{{workflow.creationTimestamp.m}}", createdAtArray[1])
.replace("{{workflow.creationTimestamp.d}}", createdAtArray[2])
.replace("{{pod.name}}", podName)
.replace("{{workflow.namespace}}", namespace)
.replace(/\s+/g, '') // Remove all whitespace.
.replace('{{workflow.name}}', podName.replace(/-system-container-impl-.*/, ''))
.replace('{{workflow.creationTimestamp.Y}}', createdAtArray[0])
.replace('{{workflow.creationTimestamp.m}}', createdAtArray[1])
.replace('{{workflow.creationTimestamp.d}}', createdAtArray[2])
.replace('{{pod.name}}', podName)
.replace('{{workflow.namespace}}', namespace);

if (!key.endsWith("/")) {
key = key + "/"
if (!key.endsWith('/')) {
key = key + '/';
}
key = key + "main.log"
key = key + 'main.log';

console.log("keyFormat: ", keyFormat)
console.log("key: ", key)
console.log('keyFormat: ', keyFormat);
console.log('key: ', key);

// If there are unresolved template tags in the keyFormat, throw an error
// that surfaces in the frontend's console log.
if (key.includes("{") || key.includes("}")) {
if (key.includes('{') || key.includes('}')) {
throw new Error(
`keyFormat, which is defined in config.ts or through the ARGO_KEYFORMAT env var, appears to include template tags that are not supported. ` +
`The resulting log key, ${key}, includes unresolved template tags and is therefore invalid.`
)
`The resulting log key, ${key}, includes unresolved template tags and is therefore invalid.`,
);
}

const regex = /^[a-zA-Z0-9\-._/]+$/; // Allow letters, numbers, -, ., _, /
if (!regex.test(key)) {
throw new Error(
`The log key, ${key}, which is derived from keyFormat in config.ts or through the ARGO_KEYFORMAT env var, is an invalid path. ` +
`Supported characters include: letters, numbers, -, ., _, and /.`
)
`Supported characters include: letters, numbers, -, ., _, and /.`,
);
}

return { bucket, client, key };
Expand Down
2 changes: 1 addition & 1 deletion frontend/src/components/tabs/RuntimeNodeDetailsV2.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -306,7 +306,7 @@ async function getLogsInfo(execution: Execution, runId?: string): Promise<Map<st
let logsBannerMessage = '';
let logsBannerAdditionalInfo = '';
const customPropertiesMap = execution.getCustomPropertiesMap();
const createdAt = new Date(execution.getCreateTimeSinceEpoch()).toISOString().split('T')[0]
const createdAt = new Date(execution.getCreateTimeSinceEpoch()).toISOString().split('T')[0];

if (execution) {
podName = customPropertiesMap.get(KfpExecutionProperties.POD_NAME)?.getStringValue() || '';
Expand Down
19 changes: 17 additions & 2 deletions frontend/src/lib/Apis.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,9 @@ describe('Apis', () => {

it('getPodLogs', async () => {
const spy = fetchSpy('http://some/address');
expect(await Apis.getPodLogs('a-run-id', 'some-pod-name', 'ns')).toEqual('http://some/address');
expect(await Apis.getPodLogs('a-run-id', 'some-pod-name', 'ns', '')).toEqual(
'http://some/address',
);
expect(spy).toHaveBeenCalledWith(
'k8s/pod/logs?podname=some-pod-name&runid=a-run-id&podnamespace=ns',
{
Expand All @@ -71,7 +73,7 @@ describe('Apis', () => {

it('getPodLogs in a specific namespace', async () => {
const spy = fetchSpy('http://some/address');
expect(await Apis.getPodLogs('a-run-id', 'some-pod-name', 'some-namespace-name')).toEqual(
expect(await Apis.getPodLogs('a-run-id', 'some-pod-name', 'some-namespace-name', '')).toEqual(
'http://some/address',
);
expect(spy).toHaveBeenCalledWith(
Expand All @@ -82,6 +84,19 @@ describe('Apis', () => {
);
});

it('getPodLogs with createdat specified', async () => {
const spy = fetchSpy('http://some/address');
expect(await Apis.getPodLogs('a-run-id', 'some-pod-name', 'ns', '2024-08-13')).toEqual(
'http://some/address',
);
expect(spy).toHaveBeenCalledWith(
'k8s/pod/logs?podname=some-pod-name&runid=a-run-id&podnamespace=ns&createdat=2024-08-13',
{
credentials: 'same-origin',
},
);
});

it('getPodLogs error', async () => {
jest.spyOn(console, 'error').mockImplementation(() => null);
window.fetch = jest.fn(() =>
Expand Down
11 changes: 9 additions & 2 deletions frontend/src/lib/Apis.ts
Original file line number Diff line number Diff line change
Expand Up @@ -108,14 +108,21 @@ export class Apis {
/**
* Get pod logs
*/
public static getPodLogs(runId: string, podName: string, podNamespace: string, createdAt: string): Promise<string> {
public static getPodLogs(
runId: string,
podName: string,
podNamespace: string,
createdAt: string,
): Promise<string> {
let query = `k8s/pod/logs?podname=${encodeURIComponent(podName)}&runid=${encodeURIComponent(
runId,
)}`;
if (podNamespace) {
query += `&podnamespace=${encodeURIComponent(podNamespace)}`;
}
query += `&createdat=${encodeURIComponent(createdAt)}`;
if (createdAt) {
query += `&createdat=${encodeURIComponent(createdAt)}`;
}
return this._fetch(query);
}

Expand Down

0 comments on commit b7e8141

Please sign in to comment.