From 60e47e098ea43c7dea27511e112292e25f128c76 Mon Sep 17 00:00:00 2001 From: Alan Greene Date: Tue, 22 Aug 2023 01:06:34 +0100 Subject: [PATCH] Display matrix TaskRuns on the PipelineRun details page Add support for displaying all TaskRuns produced by a matrix task on the PipelineRun details page. Future updates will improve the display of the matrix information making it easier to differentiate between them on the left task/step nav, and making it clearer which params are provided from the matrix. --- .../components/PipelineRun/PipelineRun.jsx | 55 ++++++++++++++++--- .../PipelineRun/PipelineRun.stories.jsx | 15 ++++- .../PipelineRun/PipelineRun.test.jsx | 2 +- .../components/src/components/Task/Task.jsx | 10 +++- .../src/components/Task/Task.stories.jsx | 2 +- .../src/components/Task/Task.test.jsx | 49 ++++++++++++++--- .../src/components/TaskTree/TaskTree.jsx | 33 +++++++++-- .../components/TaskTree/TaskTree.stories.jsx | 2 +- packages/utils/src/utils/constants.js | 2 + packages/utils/src/utils/index.js | 19 ++++--- src/containers/PipelineRun/PipelineRun.jsx | 37 ++++++++++--- src/containers/TaskRun/TaskRun.jsx | 2 +- 12 files changed, 183 insertions(+), 45 deletions(-) diff --git a/packages/components/src/components/PipelineRun/PipelineRun.jsx b/packages/components/src/components/PipelineRun/PipelineRun.jsx index a068a9fc2..ed0102640 100644 --- a/packages/components/src/components/PipelineRun/PipelineRun.jsx +++ b/packages/components/src/components/PipelineRun/PipelineRun.jsx @@ -112,6 +112,16 @@ export /* istanbul ignore next */ class PipelineRunContainer extends Component { })); }; + getPipelineTask = ({ pipelineRun, selectedTaskId, taskRun }) => { + const memberOf = taskRun?.metadata?.labels?.[labelConstants.MEMBER_OF]; + const pipelineTask = ( + pipelineRun.spec?.pipelineSpec?.[memberOf] || + pipelineRun.status?.pipelineSpec?.[memberOf] + )?.find(task => task.name === selectedTaskId); + + return pipelineTask; + }; + loadTaskRuns = () => { const { pipelineRun } = this.props; if ( @@ -122,19 +132,32 @@ export /* istanbul ignore next */ class PipelineRunContainer extends Component { } let { taskRuns } = this.props; - - if (!taskRuns) { - return []; - } - - return taskRuns; + return taskRuns || []; }; + onTaskSelected = ({ + selectedRetry: retry, selectedStepId, selectedTaskId, taskRunName + }) => { + const { handleTaskSelected, pipelineRun } = this.props; + const taskRuns = this.loadTaskRuns(); + let taskRun = taskRuns.find( + ({ metadata }) => + metadata.labels?.[labelConstants.PIPELINE_TASK] === selectedTaskId + ) || {}; + + const pipelineTask = this.getPipelineTask({ pipelineRun, selectedTaskId, taskRun }); + handleTaskSelected({ + selectedRetry: retry, + selectedStepId, + selectedTaskId, + taskRunName: pipelineTask?.matrix ? taskRunName : undefined + }); + } + render() { const { customNotification, error, - handleTaskSelected, icon, intl, loading, @@ -146,6 +169,7 @@ export /* istanbul ignore next */ class PipelineRunContainer extends Component { selectedRetry, selectedStepId, selectedTaskId, + selectedTaskRunName, triggerHeader, view } = this.props; @@ -236,7 +260,18 @@ export /* istanbul ignore next */ class PipelineRunContainer extends Component { } const taskRuns = this.loadTaskRuns(); - let taskRun = taskRuns.find(({ metadata }) => metadata.labels?.[labelConstants.PIPELINE_TASK] === selectedTaskId) || {}; + let taskRun = taskRuns.find( + ({ metadata }) => + metadata.labels?.[labelConstants.PIPELINE_TASK] === selectedTaskId + ) || {}; + + const pipelineTask = this.getPipelineTask({ pipelineRun, selectedTaskId, taskRun }); + if (pipelineTask?.matrix && selectedTaskRunName) { + taskRun = taskRuns.find( + ({ metadata }) => + metadata.name === selectedTaskRunName + ); + } if (taskRun.status?.retriesStatus && selectedRetry) { taskRun = { @@ -287,11 +322,13 @@ export /* istanbul ignore next */ class PipelineRunContainer extends Component { {taskRuns.length > 0 && (
{(selectedStepId && ( diff --git a/packages/components/src/components/PipelineRun/PipelineRun.stories.jsx b/packages/components/src/components/PipelineRun/PipelineRun.stories.jsx index 9d5ed96c1..fa567d466 100644 --- a/packages/components/src/components/PipelineRun/PipelineRun.stories.jsx +++ b/packages/components/src/components/PipelineRun/PipelineRun.stories.jsx @@ -174,7 +174,10 @@ export const Default = () => { return ( 'sample log output'} - handleTaskSelected={(taskId, stepId) => { + handleTaskSelected={({ + selectedStepId: stepId, + selectedTaskId: taskId + }) => { setSelectedStepId(stepId); setSelectedTaskId(taskId); }} @@ -193,7 +196,10 @@ export const WithMinimalStatus = () => { return ( 'sample log output'} - handleTaskSelected={(taskId, stepId) => { + handleTaskSelected={({ + selectedStepId: stepId, + selectedTaskId: taskId + }) => { setSelectedStepId(stepId); setSelectedTaskId(taskId); }} @@ -212,7 +218,10 @@ export const WithPodDetails = () => { return ( 'sample log output'} - handleTaskSelected={(taskId, stepId) => { + handleTaskSelected={({ + selectedStepId: stepId, + selectedTaskId: taskId + }) => { setSelectedStepId(stepId); setSelectedTaskId(taskId); }} diff --git a/packages/components/src/components/PipelineRun/PipelineRun.test.jsx b/packages/components/src/components/PipelineRun/PipelineRun.test.jsx index f0c313584..d674bfc26 100644 --- a/packages/components/src/components/PipelineRun/PipelineRun.test.jsx +++ b/packages/components/src/components/PipelineRun/PipelineRun.test.jsx @@ -135,7 +135,7 @@ it('PipelineRunContainer handles init step failures for retry', async () => { selectedTaskId: null }; - handleTaskSelected = (selectedTaskId, selectedStepId) => { + handleTaskSelected = ({ selectedStepId, selectedTaskId }) => { this.setState({ selectedStepId, selectedTaskId }); }; diff --git a/packages/components/src/components/Task/Task.jsx b/packages/components/src/components/Task/Task.jsx index 315c5457d..1c1e2ac68 100644 --- a/packages/components/src/components/Task/Task.jsx +++ b/packages/components/src/components/Task/Task.jsx @@ -20,7 +20,6 @@ import { } from '@carbon/icons-react'; import { getStepStatusReason, - getTranslateWithId, updateUnexecutedSteps } from '@tektoncd/dashboard-utils'; @@ -74,9 +73,14 @@ class Task extends Component { } handleClick = () => { - const { id, selectedRetry } = this.props; + const { id, selectedRetry, taskRun } = this.props; const { selectedStepId } = this.state; - this.props.onSelect(id, selectedStepId, selectedRetry); + this.props.onSelect({ + selectedRetry, + selectedStepId, + selectedTaskId: id, + taskRunName: taskRun.metadata?.name + }); }; handleStepSelected = selectedStepId => { diff --git a/packages/components/src/components/Task/Task.stories.jsx b/packages/components/src/components/Task/Task.stories.jsx index e5a15bfbb..8c587e8c8 100644 --- a/packages/components/src/components/Task/Task.stories.jsx +++ b/packages/components/src/components/Task/Task.stories.jsx @@ -56,7 +56,7 @@ export const Expanded = args => { setSelectedStepId(stepId)} + onSelect={({ selectedStepId: stepId }) => setSelectedStepId(stepId)} reason="Running" selectedStepId={selectedStepId} steps={[ diff --git a/packages/components/src/components/Task/Task.test.jsx b/packages/components/src/components/Task/Task.test.jsx index fd30c408b..b1b015764 100644 --- a/packages/components/src/components/Task/Task.test.jsx +++ b/packages/components/src/components/Task/Task.test.jsx @@ -60,7 +60,12 @@ describe('Task', () => { /> ); - expect(onSelect).toHaveBeenCalledWith(props.id, firstStepName, undefined); + expect(onSelect).toHaveBeenCalledWith({ + selectedRetry: undefined, + selectedStepId: firstStepName, + selectedTaskId: props.id, + taskRunName: undefined + }); }); it('automatically selects first error step in expanded Task', () => { @@ -82,7 +87,12 @@ describe('Task', () => { /> ); - expect(onSelect).toHaveBeenCalledWith(props.id, errorStepName, undefined); + expect(onSelect).toHaveBeenCalledWith({ + selectedRetry: undefined, + selectedStepId: errorStepName, + selectedTaskId: props.id, + taskRunName: undefined + }); }); it('handles no steps', () => { @@ -98,7 +108,12 @@ describe('Task', () => { /> ); - expect(onSelect).toHaveBeenCalledWith(props.id, undefined, undefined); + expect(onSelect).toHaveBeenCalledWith({ + selectedRetry: undefined, + selectedStepId: undefined, + selectedTaskId: props.id, + taskRunName: undefined + }); }); it('renders completed steps in expanded state', () => { @@ -167,7 +182,12 @@ describe('Task', () => { expect(onSelect).not.toHaveBeenCalled(); fireEvent.click(getByText(props.displayName)); expect(onSelect).toHaveBeenCalledTimes(1); - expect(onSelect).toHaveBeenCalledWith(props.id, null, undefined); + expect(onSelect).toHaveBeenCalledWith({ + selectedRetry: undefined, + selectedStepId: null, + selectedTaskId: props.id, + taskRunName: undefined + }); }); it('handles click event with retries', () => { @@ -179,7 +199,12 @@ describe('Task', () => { expect(onSelect).not.toHaveBeenCalled(); fireEvent.click(getByText(`${props.displayName} (retry ${selectedRetry})`)); expect(onSelect).toHaveBeenCalledTimes(1); - expect(onSelect).toHaveBeenCalledWith(props.id, null, selectedRetry); + expect(onSelect).toHaveBeenCalledWith({ + selectedRetry, + selectedStepId: null, + selectedTaskId: props.id, + taskRunName: undefined + }); }); it('handles click event on Step', () => { @@ -192,7 +217,12 @@ describe('Task', () => { expect(onSelect).not.toHaveBeenCalled(); fireEvent.click(getByText(stepName)); expect(onSelect).toHaveBeenCalledTimes(1); - expect(onSelect).toHaveBeenCalledWith(props.id, stepName, undefined); + expect(onSelect).toHaveBeenCalledWith({ + selectedRetry: undefined, + selectedStepId: stepName, + selectedTaskId: props.id, + taskRunName: undefined + }); }); it('handles click event on Step with retries', () => { @@ -212,6 +242,11 @@ describe('Task', () => { expect(onSelect).not.toHaveBeenCalled(); fireEvent.click(getByText(stepName)); expect(onSelect).toHaveBeenCalledTimes(1); - expect(onSelect).toHaveBeenCalledWith(props.id, stepName, selectedRetry); + expect(onSelect).toHaveBeenCalledWith({ + selectedRetry, + selectedStepId: stepName, + selectedTaskId: props.id, + taskRunName: undefined + }); }); }); diff --git a/packages/components/src/components/TaskTree/TaskTree.jsx b/packages/components/src/components/TaskTree/TaskTree.jsx index 646d03fe2..e5a2d2caa 100644 --- a/packages/components/src/components/TaskTree/TaskTree.jsx +++ b/packages/components/src/components/TaskTree/TaskTree.jsx @@ -20,11 +20,13 @@ const defaults = { }; const TaskTree = ({ + isSelectedTaskMatrix, onRetryChange, onSelect, selectedRetry, selectedStepId, selectedTaskId, + selectedTaskRunName, taskRuns = defaults.taskRuns }) => { if (!taskRuns) { @@ -35,6 +37,8 @@ const TaskTree = ({ .filter(Boolean) .find(taskRun => getStatus(taskRun).status === 'False'); + let hasExpandedTask = false; + return (
    {taskRuns.map((taskRun, index) => { @@ -48,7 +52,11 @@ const TaskTree = ({ } = labels; let taskRunToUse = taskRun; - if (selectedRetry && selectedTaskId === pipelineTaskName) { + if ( + selectedRetry && + selectedTaskId === pipelineTaskName && + (selectedTaskRunName ? name === selectedTaskRunName : true) + ) { taskRunToUse = { ...taskRunToUse, status: taskRunToUse.status?.retriesStatus?.[selectedRetry] @@ -58,10 +66,25 @@ const TaskTree = ({ const { reason, status } = getStatus(taskRunToUse); const { steps } = taskRunToUse.status || {}; const expanded = - (!selectedTaskId && erroredTask?.metadata.uid === uid) || - selectedTaskId === pipelineTaskName || - (!erroredTask && !selectedTaskId && index === 0); - const selectDefaultStep = !selectedTaskId; + // should only have 1 expanded task at a time (may change in a future design) + // we expand the first task matching the rules below + !hasExpandedTask && + // no explicitly selected task and current task failed, expand this one + ((!selectedTaskId && erroredTask?.metadata.uid === uid) || + // or this task is the selected one + (selectedTaskId === pipelineTaskName && + // if it's a matrixed TaskRun only expand if it matches the specified taskRunName + // if it's not a matrixed TaskRun, or no taskRunName specified, this is the first match so expand it + (selectedTaskRunName ? name === selectedTaskRunName : true)) || + // otherwise there's no error and no explicit selection, expand the first task by default + (!erroredTask && !selectedTaskId && index === 0)); + + if (!hasExpandedTask && expanded) { + hasExpandedTask = true; + } + + const selectDefaultStep = + !selectedTaskId || (isSelectedTaskMatrix && !selectedTaskRunName); return ( { + onSelect={({ selectedStepId: stepId, selectedTaskId: taskId }) => { setSelectedStepId(stepId); setSelectedTaskId(taskId); }} diff --git a/packages/utils/src/utils/constants.js b/packages/utils/src/utils/constants.js index ea31d9e49..2820678f3 100644 --- a/packages/utils/src/utils/constants.js +++ b/packages/utils/src/utils/constants.js @@ -16,6 +16,7 @@ export const labels = { DASHBOARD_DESCRIPTION: 'dashboard.tekton.dev/description', DASHBOARD_DISPLAY_NAME: 'dashboard.tekton.dev/displayName', DASHBOARD_IMPORT: 'dashboard.tekton.dev/import', + MEMBER_OF: 'tekton.dev/memberOf', PIPELINE: 'tekton.dev/pipeline', PIPELINE_RUN: 'tekton.dev/pipelineRun', PIPELINE_TASK: 'tekton.dev/pipelineTask', @@ -35,6 +36,7 @@ export const queryParams = { RETRY: 'retry', STEP: 'step', TASK_RUN_DETAILS: 'taskRunDetails', + TASK_RUN_NAME: 'taskRunName', VIEW: 'view' }; diff --git a/packages/utils/src/utils/index.js b/packages/utils/src/utils/index.js index 6a059a96d..b8d8071d7 100644 --- a/packages/utils/src/utils/index.js +++ b/packages/utils/src/utils/index.js @@ -494,19 +494,24 @@ export function getTaskRunsWithPlaceholders({ const taskRunsToDisplay = []; pipelineTasks.forEach(pipelineTask => { - const realTaskRun = taskRuns.find( + const realTaskRuns = taskRuns.filter( taskRun => taskRun.metadata.labels?.[labelConstants.PIPELINE_TASK] === pipelineTask.name ); - const taskRunToDisplay = - realTaskRun || - getPlaceholderTaskRun({ clusterTasks, pipelineTask, tasks }); + realTaskRuns.forEach(taskRun => { + taskRunsToDisplay.push(addDashboardLabels({ pipelineTask, taskRun })); + }); - taskRunsToDisplay.push( - addDashboardLabels({ pipelineTask, taskRun: taskRunToDisplay }) - ); + if (!realTaskRuns.length) { + taskRunsToDisplay.push( + addDashboardLabels({ + pipelineTask, + taskRun: getPlaceholderTaskRun({ clusterTasks, pipelineTask, tasks }) + }) + ); + } }); return taskRunsToDisplay; diff --git a/src/containers/PipelineRun/PipelineRun.jsx b/src/containers/PipelineRun/PipelineRun.jsx index 292ffb7e1..526d20d7d 100644 --- a/src/containers/PipelineRun/PipelineRun.jsx +++ b/src/containers/PipelineRun/PipelineRun.jsx @@ -60,7 +60,7 @@ import { } from '../../utils'; import NotFound from '../NotFound'; -const { PIPELINE_TASK, RETRY, STEP, VIEW } = queryParamConstants; +const { PIPELINE_TASK, RETRY, STEP, TASK_RUN_NAME, VIEW } = queryParamConstants; export /* istanbul ignore next */ function PipelineRunContainer() { const intl = useIntl(); @@ -72,6 +72,7 @@ export /* istanbul ignore next */ function PipelineRunContainer() { const queryParams = new URLSearchParams(location.search); const currentPipelineTaskName = queryParams.get(PIPELINE_TASK); + const currentTaskRunName = queryParams.get(TASK_RUN_NAME); let currentRetry = queryParams.get(RETRY); if (!currentRetry || !/^[0-9]+$/.test(currentRetry)) { @@ -151,10 +152,15 @@ export /* istanbul ignore next */ function PipelineRunContainer() { tasks }); - function getSelectedTaskRun({ selectedRetry, selectedTaskId }) { - const taskRun = taskRuns.find( - ({ metadata }) => - metadata.labels?.[labelConstants.PIPELINE_TASK] === selectedTaskId + function getSelectedTaskRun({ + selectedRetry, + selectedTaskId, + selectedTaskRunName + }) { + const taskRun = taskRuns.find(({ metadata }) => + selectedTaskRunName + ? metadata.name === selectedTaskRunName + : metadata.labels?.[labelConstants.PIPELINE_TASK] === selectedTaskId ); if (!taskRun) { @@ -168,7 +174,12 @@ export /* istanbul ignore next */ function PipelineRunContainer() { return { podName: taskRun.status?.retriesStatus?.[selectedRetry]?.podName }; } - function handleTaskSelected(selectedTaskId, selectedStepId, retry) { + function handleTaskSelected({ + selectedRetry: retry, + selectedStepId, + selectedTaskId, + taskRunName + }) { queryParams.set(PIPELINE_TASK, selectedTaskId); if (selectedStepId) { queryParams.set(STEP, selectedStepId); @@ -182,6 +193,13 @@ export /* istanbul ignore next */ function PipelineRunContainer() { queryParams.delete(RETRY); } + if (taskRunName) { + // matrix run + queryParams.set(TASK_RUN_NAME, taskRunName); + } else { + queryParams.delete(TASK_RUN_NAME); + } + if ( selectedStepId !== currentSelectedStepId || selectedTaskId !== currentPipelineTaskName @@ -438,7 +456,11 @@ export /* istanbul ignore next */ function PipelineRunContainer() { const selectedTaskId = currentPipelineTaskName; const { podName } = - getSelectedTaskRun({ selectedRetry: currentRetry, selectedTaskId }) || {}; + getSelectedTaskRun({ + selectedRetry: currentRetry, + selectedTaskId, + selectedTaskRunName: currentTaskRunName + }) || {}; let { data: pod } = usePod( { name: podName, namespace }, { enabled: !!podName && view === 'pod' } @@ -550,6 +572,7 @@ export /* istanbul ignore next */ function PipelineRunContainer() { selectedRetry={currentRetry} selectedStepId={currentSelectedStepId} selectedTaskId={selectedTaskId} + selectedTaskRunName={currentTaskRunName} taskRuns={taskRuns} tasks={tasks.concat(clusterTasks)} view={view} diff --git a/src/containers/TaskRun/TaskRun.jsx b/src/containers/TaskRun/TaskRun.jsx index b7ce65dc9..ec893022b 100644 --- a/src/containers/TaskRun/TaskRun.jsx +++ b/src/containers/TaskRun/TaskRun.jsx @@ -191,7 +191,7 @@ export function TaskRunContainer() { navigate(browserURL); } - function handleTaskSelected(_, newSelectedStepId, retry) { + function handleTaskSelected({ selectedStepId: newSelectedStepId }) { if (newSelectedStepId) { queryParams.set(STEP, newSelectedStepId); queryParams.delete(TASK_RUN_DETAILS);