Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove followLogs function #3870

Merged
merged 1 commit into from
Jan 8, 2025
Merged

Remove followLogs function #3870

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion packages/components/src/components/PipelineRun/PipelineRun.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ export default /* istanbul ignore next */ function PipelineRun({
taskRun
})
}
fetchLogs={() => fetchLogs(stepName, stepStatus, taskRun)}
fetchLogs={() => fetchLogs({ stepName, stepStatus, taskRun })}
forcePolling={forceLogPolling}
key={`${selectedTaskId}:${selectedStepId}:${selectedRetry}`}
logLevels={logLevels}
Expand Down
6 changes: 4 additions & 2 deletions src/containers/TaskRun/TaskRun.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -182,10 +182,12 @@ export function TaskRunContainer({
: null)}
>
<Log
fetchLogs={() => logsRetriever(stepName, stepStatus, run)}
isLogsMaximized={isLogsMaximized}
enableLogAutoScroll
enableLogScrollButtons
fetchLogs={() =>
logsRetriever({ stepName, stepStatus, taskRun: run })
}
isLogsMaximized={isLogsMaximized}
key={`${stepName}:${currentRetry}`}
logLevels={logLevels}
showLevels={showLogLevels}
Expand Down
36 changes: 13 additions & 23 deletions src/utils/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ export function sortRunsByCreationTime(runs) {
});
}

export async function followLogs(stepName, stepStatus, taskRun) {
export async function fetchLogs({ _stepName, stream, stepStatus, taskRun }) {
const { namespace } = taskRun.metadata;
const { podName } = taskRun.status || {};
let logs;
Expand All @@ -65,22 +65,7 @@ export async function followLogs(stepName, stepStatus, taskRun) {
container,
name: podName,
namespace,
stream: true
});
}
return logs;
}

export async function fetchLogs(stepName, stepStatus, taskRun) {
const { namespace } = taskRun.metadata;
const { podName } = taskRun.status || {};
let logs;
if (podName && stepStatus) {
const { container } = stepStatus;
logs = getPodLog({
container,
name: podName,
namespace
stream
});
}
return logs;
Expand All @@ -91,7 +76,7 @@ export function fetchLogsFallback(externalLogsURL) {
return undefined;
}

return (stepName, stepStatus, taskRun) => {
return ({ _stepName, stepStatus, taskRun }) => {
const { namespace } = taskRun.metadata;
const { podName, startTime, completionTime } = taskRun.status || {};
const { container } = stepStatus;
Expand All @@ -116,18 +101,23 @@ export function getLogsRetriever({
isLogStreamingEnabled,
onFallback
}) {
const logs = isLogStreamingEnabled ? followLogs : fetchLogs;
const fallback = fetchLogsFallback(externalLogsURL);

if (fallback) {
return (stepName, stepStatus, taskRun) =>
logs(stepName, stepStatus, taskRun).catch(() => {
return ({ stepName, stepStatus, taskRun }) =>
fetchLogs({
stepName,
stream: isLogStreamingEnabled,
stepStatus,
taskRun
}).catch(() => {
onFallback(true);
return fallback(stepName, stepStatus, taskRun);
// TODO: logs - support streaming from external logs?
return fallback({ stepName, stepStatus, taskRun });
});
}

return logs;
return fetchLogs;
}

// K8s label documentation comes from here:
Expand Down
118 changes: 60 additions & 58 deletions src/utils/index.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ import * as comms from '../api/comms';
import {
fetchLogs,
fetchLogsFallback,
followLogs,
getLocale,
getLogsRetriever,
getTheme,
Expand Down Expand Up @@ -101,7 +100,7 @@ describe('fetchLogs', () => {
const logs = 'fake logs';
vi.spyOn(API, 'getPodLog').mockImplementation(() => logs);

const returnedLogs = fetchLogs(stepName, stepStatus, taskRun);
const returnedLogs = fetchLogs({ stepName, stepStatus, taskRun });
expect(API.getPodLog).toHaveBeenCalledWith(
expect.objectContaining({
container: stepStatus.container,
Expand All @@ -120,7 +119,60 @@ describe('fetchLogs', () => {
const taskRun = { metadata: { namespace: 'default' } };
vi.spyOn(API, 'getPodLog');

fetchLogs(stepName, stepStatus, taskRun);
fetchLogs({ stepName, stepStatus, taskRun });
expect(API.getPodLog).not.toHaveBeenCalled();
});

it('should return the streamed pod logs', () => {
const namespace = 'default';
const podName = 'pipeline-run-123456';
const stepName = 'kubectl-apply';
const stepStatus = { container: 'step-kubectl-apply' };
const taskRun = {
metadata: { namespace },
status: { podName }
};

const logs = {
getReader() {
return {
read() {
return Promise.resolve({
done: true,
value: new TextEncoder().encode('fake logs')
});
}
};
}
};
vi.spyOn(API, 'getPodLog').mockImplementation(() => logs);

const returnedLogs = fetchLogs({
stepName,
stream: true,
stepStatus,
taskRun
});
expect(API.getPodLog).toHaveBeenCalledWith(
expect.objectContaining({
container: stepStatus.container,
name: podName,
namespace,
stream: true
})
);
returnedLogs.then(data => {
expect(data).toBe(logs);
});
});

it('should not call the API when the pod is not specified', () => {
const stepName = 'kubectl-apply';
const stepStatus = { container: 'step-kubectl-apply' };
const taskRun = { metadata: { namespace: 'default' } };
vi.spyOn(API, 'getPodLog');

fetchLogs({ stepName, stream: true, stepStatus, taskRun });
expect(API.getPodLog).not.toHaveBeenCalled();
});
});
Expand All @@ -146,7 +198,7 @@ describe('fetchLogsFallback', () => {
vi.spyOn(comms, 'get').mockImplementation(() => {});

const fallback = fetchLogsFallback(externalLogsURL);
fallback(stepName, stepStatus, taskRun);
fallback({ stepName, stepStatus, taskRun });
expect(comms.get).toHaveBeenCalledWith(
`http://localhost:3000${externalLogsURL}/${namespace}/${podName}/${container}?startTime=${startTime.replaceAll(
':',
Expand All @@ -167,64 +219,14 @@ describe('fetchLogsFallback', () => {
vi.spyOn(comms, 'get').mockImplementation(() => {});

const fallback = fetchLogsFallback(externalLogsURL);
fallback(stepName, stepStatus, taskRun);
fallback({ stepName, stepStatus, taskRun });
expect(comms.get).toHaveBeenCalledWith(
`http://localhost:3000${externalLogsURL}/${namespace}/${podName}/${container}`,
{ Accept: 'text/plain' }
);
});
});

describe('followLogs', () => {
it('should return the pod logs', () => {
const namespace = 'default';
const podName = 'pipeline-run-123456';
const stepName = 'kubectl-apply';
const stepStatus = { container: 'step-kubectl-apply' };
const taskRun = {
metadata: { namespace },
status: { podName }
};

const logs = {
getReader() {
return {
read() {
return Promise.resolve({
done: true,
value: new TextEncoder().encode('fake logs')
});
}
};
}
};
vi.spyOn(API, 'getPodLog').mockImplementation(() => logs);

const returnedLogs = followLogs(stepName, stepStatus, taskRun);
expect(API.getPodLog).toHaveBeenCalledWith(
expect.objectContaining({
container: stepStatus.container,
name: podName,
namespace,
stream: true
})
);
returnedLogs.then(data => {
expect(data).toBe(logs);
});
});

it('should not call the API when the pod is not specified', () => {
const stepName = 'kubectl-apply';
const stepStatus = { container: 'step-kubectl-apply' };
const taskRun = { metadata: { namespace: 'default' } };
vi.spyOn(API, 'getPodLog');

followLogs(stepName, stepStatus, taskRun);
expect(API.getPodLog).not.toHaveBeenCalled();
});
});

describe('getLogsRetriever', () => {
const namespace = 'fake_namespace';
const podName = 'fake_podName';
Expand All @@ -236,7 +238,7 @@ describe('getLogsRetriever', () => {
vi.spyOn(API, 'getPodLog').mockImplementation(() => {});
const logsRetriever = getLogsRetriever({});
expect(logsRetriever).toBeDefined();
logsRetriever(stepName, stepStatus, taskRun);
logsRetriever({ stepName, stepStatus, taskRun });
expect(API.getPodLog).toHaveBeenCalledWith({
container: stepName,
name: podName,
Expand All @@ -250,7 +252,7 @@ describe('getLogsRetriever', () => {
const onFallback = vi.fn();
const logsRetriever = getLogsRetriever({ externalLogsURL, onFallback });
expect(logsRetriever).toBeDefined();
await logsRetriever(stepName, stepStatus, taskRun);
await logsRetriever({ stepName, stepStatus, taskRun });
expect(API.getPodLog).toHaveBeenCalledWith({
container: stepName,
name: podName,
Expand All @@ -269,7 +271,7 @@ describe('getLogsRetriever', () => {
const onFallback = vi.fn();
const logsRetriever = getLogsRetriever({ externalLogsURL, onFallback });
expect(logsRetriever).toBeDefined();
await logsRetriever(stepName, stepStatus, taskRun);
await logsRetriever({ stepName, stepStatus, taskRun });
expect(API.getPodLog).toHaveBeenCalledWith({
container: stepName,
name: podName,
Expand Down
Loading