diff --git a/packages/components/src/components/PipelineRun/PipelineRun.jsx b/packages/components/src/components/PipelineRun/PipelineRun.jsx index 5da3f3314..2afb1c82f 100644 --- a/packages/components/src/components/PipelineRun/PipelineRun.jsx +++ b/packages/components/src/components/PipelineRun/PipelineRun.jsx @@ -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} diff --git a/src/containers/TaskRun/TaskRun.jsx b/src/containers/TaskRun/TaskRun.jsx index 0a52d5e52..c67d1b8cf 100644 --- a/src/containers/TaskRun/TaskRun.jsx +++ b/src/containers/TaskRun/TaskRun.jsx @@ -182,10 +182,12 @@ export function TaskRunContainer({ : null)} > logsRetriever(stepName, stepStatus, run)} - isLogsMaximized={isLogsMaximized} enableLogAutoScroll enableLogScrollButtons + fetchLogs={() => + logsRetriever({ stepName, stepStatus, taskRun: run }) + } + isLogsMaximized={isLogsMaximized} key={`${stepName}:${currentRetry}`} logLevels={logLevels} showLevels={showLogLevels} diff --git a/src/utils/index.js b/src/utils/index.js index f8f084542..f1a44861a 100644 --- a/src/utils/index.js +++ b/src/utils/index.js @@ -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; @@ -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; @@ -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; @@ -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: diff --git a/src/utils/index.test.js b/src/utils/index.test.js index d438d0876..9cebe99db 100644 --- a/src/utils/index.test.js +++ b/src/utils/index.test.js @@ -16,7 +16,6 @@ import * as comms from '../api/comms'; import { fetchLogs, fetchLogsFallback, - followLogs, getLocale, getLogsRetriever, getTheme, @@ -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, @@ -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(); }); }); @@ -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( ':', @@ -167,7 +219,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}`, { Accept: 'text/plain' } @@ -175,56 +227,6 @@ describe('fetchLogsFallback', () => { }); }); -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'; @@ -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, @@ -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, @@ -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,