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.

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

Display matrix TaskRuns on the PipelineRun details page #3081

Merged
merged 1 commit into from
Aug 29, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
55 changes: 46 additions & 9 deletions packages/components/src/components/PipelineRun/PipelineRun.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand All @@ -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,
Expand All @@ -146,6 +169,7 @@ export /* istanbul ignore next */ class PipelineRunContainer extends Component {
selectedRetry,
selectedStepId,
selectedTaskId,
selectedTaskRunName,
triggerHeader,
view
} = this.props;
Expand Down Expand Up @@ -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 = {
Expand Down Expand Up @@ -287,11 +322,13 @@ export /* istanbul ignore next */ class PipelineRunContainer extends Component {
{taskRuns.length > 0 && (
<div className="tkn--tasks">
<TaskTree
isSelectedTaskMatrix={!!pipelineTask?.matrix}
onRetryChange={onRetryChange}
onSelect={handleTaskSelected}
onSelect={this.onTaskSelected}
selectedRetry={selectedRetry}
selectedStepId={selectedStepId}
selectedTaskId={selectedTaskId}
selectedTaskRunName={selectedTaskRunName}
taskRuns={taskRuns}
/>
{(selectedStepId && (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,10 @@ export const Default = () => {
return (
<PipelineRun
fetchLogs={() => 'sample log output'}
handleTaskSelected={(taskId, stepId) => {
handleTaskSelected={({
selectedStepId: stepId,
selectedTaskId: taskId
}) => {
setSelectedStepId(stepId);
setSelectedTaskId(taskId);
}}
Expand All @@ -193,7 +196,10 @@ export const WithMinimalStatus = () => {
return (
<PipelineRun
fetchLogs={() => 'sample log output'}
handleTaskSelected={(taskId, stepId) => {
handleTaskSelected={({
selectedStepId: stepId,
selectedTaskId: taskId
}) => {
setSelectedStepId(stepId);
setSelectedTaskId(taskId);
}}
Expand All @@ -212,7 +218,10 @@ export const WithPodDetails = () => {
return (
<PipelineRun
fetchLogs={() => 'sample log output'}
handleTaskSelected={(taskId, stepId) => {
handleTaskSelected={({
selectedStepId: stepId,
selectedTaskId: taskId
}) => {
setSelectedStepId(stepId);
setSelectedTaskId(taskId);
}}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 });
};

Expand Down
10 changes: 7 additions & 3 deletions packages/components/src/components/Task/Task.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ import {
} from '@carbon/icons-react';
import {
getStepStatusReason,
getTranslateWithId,
updateUnexecutedSteps
} from '@tektoncd/dashboard-utils';

Expand Down Expand Up @@ -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 => {
Expand Down
2 changes: 1 addition & 1 deletion packages/components/src/components/Task/Task.stories.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ export const Expanded = args => {
<Task
{...args}
expanded
onSelect={(_, stepId) => setSelectedStepId(stepId)}
onSelect={({ selectedStepId: stepId }) => setSelectedStepId(stepId)}
reason="Running"
selectedStepId={selectedStepId}
steps={[
Expand Down
49 changes: 42 additions & 7 deletions packages/components/src/components/Task/Task.test.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand All @@ -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', () => {
Expand All @@ -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', () => {
Expand Down Expand Up @@ -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', () => {
Expand All @@ -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', () => {
Expand All @@ -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', () => {
Expand All @@ -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
});
});
});
33 changes: 28 additions & 5 deletions packages/components/src/components/TaskTree/TaskTree.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,13 @@ const defaults = {
};

const TaskTree = ({
isSelectedTaskMatrix,
onRetryChange,
onSelect,
selectedRetry,
selectedStepId,
selectedTaskId,
selectedTaskRunName,
taskRuns = defaults.taskRuns
}) => {
if (!taskRuns) {
Expand All @@ -35,6 +37,8 @@ const TaskTree = ({
.filter(Boolean)
.find(taskRun => getStatus(taskRun).status === 'False');

let hasExpandedTask = false;

return (
<ol className="tkn--task-tree">
{taskRuns.map((taskRun, index) => {
Expand All @@ -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]
Expand All @@ -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 &&
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Found this value tough to deconstruct. Perhaps worth adding a comment or two, or splitting up into more variables with explanatory names?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah there's a lot going on there, added comments for each branch. Let me know if that helps

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, thanks

// 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 (
<Task
displayName={displayName || pipelineTaskName || name}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ export const Default = {
return (
<TaskTree
{...args}
onSelect={(taskId, stepId) => {
onSelect={({ selectedStepId: stepId, selectedTaskId: taskId }) => {
setSelectedStepId(stepId);
setSelectedTaskId(taskId);
}}
Expand Down
2 changes: 2 additions & 0 deletions packages/utils/src/utils/constants.js
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand All @@ -35,6 +36,7 @@ export const queryParams = {
RETRY: 'retry',
STEP: 'step',
TASK_RUN_DETAILS: 'taskRunDetails',
TASK_RUN_NAME: 'taskRunName',
VIEW: 'view'
};

Expand Down
19 changes: 12 additions & 7 deletions packages/utils/src/utils/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Loading