Skip to content
This repository has been archived by the owner on Jan 12, 2023. It is now read-only.

Commit

Permalink
[Backport] Pipeline summary: handle special case where the VM has an …
Browse files Browse the repository at this point in the history
…error but the current step does not (#391) (#392)

* Add mock data to reproduce bug case

* Handle yellow steps (no error, but in progress when VM errored)

* Stop the elapsed time when there is a VM-level error, and show the error in the stalled step

* Make the yellow icon red
  • Loading branch information
mturley committed Jan 19, 2021
1 parent afd5642 commit 91e265d
Show file tree
Hide file tree
Showing 5 changed files with 99 additions and 47 deletions.
6 changes: 4 additions & 2 deletions src/app/Plans/components/Step.tsx
Expand Up @@ -12,13 +12,15 @@ import {
} from '@patternfly/react-tokens';
import spacing from '@patternfly/react-styles/css/utilities/Spacing/spacing';
import { StepType } from '@app/common/constants';
import { IVMStatus } from '@app/queries/types';

interface IStepProps {
vmStatus: IVMStatus;
type: StepType;
error?: boolean;
}

const Step: React.FunctionComponent<IStepProps> = ({ type, error }: IStepProps) => {
const Step: React.FunctionComponent<IStepProps> = ({ vmStatus, type, error }: IStepProps) => {
let step: React.ReactElement | null = null;
if (type === StepType.Full) {
step = (
Expand All @@ -36,7 +38,7 @@ const Step: React.FunctionComponent<IStepProps> = ({ type, error }: IStepProps)
className={spacing.mlSm}
height="1em"
width="1em"
color={error ? dangerColor.value : infoColor.value}
color={error || vmStatus.error ? dangerColor.value : infoColor.value}
/>
);
}
Expand Down
87 changes: 51 additions & 36 deletions src/app/Plans/components/VMStatusTable.tsx
Expand Up @@ -13,7 +13,7 @@ import {
import Step from './Step';
import { IVMStatus, IStep } from '@app/queries/types';
import TickingElapsedTime from '@app/common/components/TickingElapsedTime';
import { getStepType, isStepOnError } from '@app/common/helpers';
import { findCurrentStep, getStepType, isStepOnError } from '@app/common/helpers';

interface IVMStatusTableProps {
status: IVMStatus;
Expand All @@ -31,41 +31,56 @@ const VMStatusTable: React.FunctionComponent<IVMStatusTableProps> = ({
{ title: 'State', cellTransforms: [truncate] },
];

const rows: IRow[] = status.pipeline.map((step: IStep, index) => ({
meta: { step },
cells: [
{
title: (
<Flex
spaceItems={{ default: 'spaceItemsSm' }}
alignContent={{ default: 'alignContentFlexStart' }}
flexWrap={{ default: 'nowrap' }}
>
<FlexItem>
<Step type={getStepType(status, index)} error={isStepOnError(status, index)} />
</FlexItem>
<FlexItem>
<Text>{step.description ? step.description.replace(/\.$/, '') : ''}</Text>
</FlexItem>
</Flex>
),
},
{
title: <TickingElapsedTime start={step.started} end={step.completed} />,
},
{
title: step.error ? (
<Popover headerContent={step.phase} bodyContent={step.error?.reasons}>
<Button variant="link" isInline>
{step.phase}
</Button>
</Popover>
) : (
step.phase
),
},
],
}));
const { currentStepIndex } = findCurrentStep(status.pipeline);

const rows: IRow[] = status.pipeline.map((step: IStep, index) => {
const isCurrentStep = currentStepIndex === index;
const error = step.error || (isCurrentStep && status.error);
return {
meta: { step },
cells: [
{
title: (
<Flex
spaceItems={{ default: 'spaceItemsSm' }}
alignContent={{ default: 'alignContentFlexStart' }}
flexWrap={{ default: 'nowrap' }}
>
<FlexItem>
<Step
vmStatus={status}
type={getStepType(status, index)}
error={isStepOnError(status, index)}
/>
</FlexItem>
<FlexItem>
<Text>{step.description ? step.description.replace(/\.$/, '') : ''}</Text>
</FlexItem>
</Flex>
),
},
{
title: (
<TickingElapsedTime
start={step.started}
end={step.completed || (status.error ? status.completed : undefined)}
/>
),
},
{
title: error ? (
<Popover headerContent={error.phase} bodyContent={error?.reasons}>
<Button variant="link" isInline>
{step.phase || error?.reasons}
</Button>
</Popover>
) : (
step.phase
),
},
],
};
});

return (
<>
Expand Down
5 changes: 3 additions & 2 deletions src/app/common/components/PipelineSummary.tsx
Expand Up @@ -52,6 +52,7 @@ const GetStepTypeIcon: React.FunctionComponent<IGetStepTypeIcon> = ({
index,
}: IGetStepTypeIcon) => {
const res = getStepType(status, index);
const { currentStepIndex } = findCurrentStep(status.pipeline);
let icon: React.ReactNode;
if (res === StepType.Full) {
icon = (
Expand All @@ -62,7 +63,7 @@ const GetStepTypeIcon: React.FunctionComponent<IGetStepTypeIcon> = ({
} else if (res === StepType.Half) {
icon = (
<ResourcesAlmostFullIcon
color={isStepOnError(status, index) ? dangerColor.value : infoColor.value}
color={isStepOnError(status, index) || status.error ? dangerColor.value : infoColor.value}
/>
);
} else {
Expand All @@ -71,7 +72,7 @@ const GetStepTypeIcon: React.FunctionComponent<IGetStepTypeIcon> = ({
return (
<>
{icon}
{index < status.pipeline.length - 1 ? <Dash isReached={true} /> : null}
{index < status.pipeline.length - 1 ? <Dash isReached={index < currentStepIndex} /> : null}
</>
);
};
Expand Down
7 changes: 2 additions & 5 deletions src/app/common/helpers.ts
Expand Up @@ -88,11 +88,8 @@ export const formatDuration = (
export const getStepType = (status: IVMStatus, index: number): StepType => {
const { currentStepIndex } = findCurrentStep(status.pipeline);
const step = status.pipeline[index];
if (status.completed || step?.completed || index < currentStepIndex) return StepType.Full;
if (status.started && index === currentStepIndex) {
if (status.error) return StepType.Full;
return StepType.Half;
}
if (step?.completed || index < currentStepIndex) return StepType.Full;
if (status.started && index === currentStepIndex) return StepType.Half;
return StepType.Empty;
};

Expand Down
41 changes: 39 additions & 2 deletions src/app/queries/mocks/plans.mock.ts
Expand Up @@ -91,7 +91,7 @@ if (process.env.NODE_ENV === 'test' || process.env.DATA_SOURCE === 'mock') {
},
{
name: 'PostHook',
description: 'Pre hook',
description: 'Post hook',
progress: { total: 1, completed: 0 },
phase: 'Mock Step Phase',
},
Expand Down Expand Up @@ -185,6 +185,43 @@ if (process.env.NODE_ENV === 'test' || process.env.DATA_SOURCE === 'mock') {
},
};

const vmStatusWithYellow: IVMStatus = {
id: vm2.id,
pipeline: [
{
name: 'PreHook',
description: 'Pre hook',
progress: { total: 1, completed: 1 },
phase: 'Mock Step Phase',
started: '2020-10-10T14:04:10Z',
completed: '2020-10-10T14:21:10Z',
},
{
name: 'DiskTransfer',
description: 'Transfer disks.',
progress: { total: 1024 * 64, completed: 0 },
annotations: { unit: 'MB' },
started: '2020-10-10T14:21:10Z',
},
{
name: 'ImageConversion',
description: 'Convert image to kubevirt.',
progress: { total: 1, completed: 0 },
phase: 'Mock Step Phase',
},
{
name: 'PostHook',
description: 'Post hook',
progress: { total: 1, completed: 0 },
phase: 'Mock Step Phase',
},
],
phase: 'Mock VM Phase',
started: '2020-10-10T14:04:10Z',
completed: '2020-10-11T14:04:10Z',
error: { phase: 'ImportCreated', reasons: ['Import CR not found.'] },
};

const plan1: IPlan = {
apiVersion: CLUSTER_API_VERSION,
kind: 'Plan',
Expand Down Expand Up @@ -329,7 +366,7 @@ if (process.env.NODE_ENV === 'test' || process.env.DATA_SOURCE === 'mock') {
migration: {
active: '',
started: '2020-10-10T14:04:10Z',
vms: [vmStatus1, vmStatus2, vmStatus3, vmStatus4],
vms: [vmStatus1, vmStatusWithYellow, vmStatus3, vmStatus4],
},
},
};
Expand Down

0 comments on commit 91e265d

Please sign in to comment.