Skip to content

Commit

Permalink
Add a fast Create Plan page - step 7
Browse files Browse the repository at this point in the history 
Changes:
1. improve layout by wrapping title and info alert with a drawer. Now
   the only component pushing the VM table down is the namespace bar.
2. disable all edits when create action is triggered
   a) for description items - hide pencil icon
   b) for form components - propagate isDisabled flag
3. remove 'Create and start' button
4. rename 'Create and edit' to 'Create'
5. re-generate storage map name in case of naming conflict
6. add advanced validations:
   a) NICs with empty network profile (oVirt only)
   b) VM has multiple NICs on the same network
   c) VM has multiple NICs mapped to Pod Networking
   d) VM has unmmaped networks
   e) VM has unmapped storages
7. display loading indicator(dots) until main queries are done
8. move/add responsibility to useFetchEffect() hook
   a) handle API errors from data queries
   b) handle loading state (skip dispatch)
   c) block queries when editing is done
9. move responsibility from action handlers to generic handler
   a) block processing actions when editing is done
   b) log actions with payload
   c) track if actions were dispatched (initial loading)

Signed-off-by: Radoslaw Szwajkowski <rszwajko@redhat.com>
  • Loading branch information
@rszwajko
rszwajko committed Feb 13, 2024
1 parent d3c6789 commit 3b70f66
Show file tree
Hide file tree
Showing 17 changed files with 675 additions and 354 deletions.
1 change: 1 addition & 0 deletions packages/eslint-plugin/cspell.wordlist.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ fullname
cncf
omitempty
nics
NICS
pnic
virtio
SSHA
Expand Down
15 changes: 12 additions & 3 deletions packages/forklift-console-plugin/locales/en/plugin__forklift-console-plugin.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@
"Add source and target providers for the migration.": "Add source and target providers for the migration.",
"All discovered networks have been mapped to the default network.": "All discovered networks have been mapped to the default network.",
"All discovered storages have been mapped to the default storage.": "All discovered storages have been mapped to the default storage.",
"All networks detected on the selected VMs require a mapping.": "All networks detected on the selected VMs require a mapping.",
"All storages detected on the selected VMs require a mapping.": "All storages detected on the selected VMs require a mapping.",
"API Error": "API Error",
"Application credential ID": "Application credential ID",
"Application credential name": "Application credential name",
Expand Down Expand Up @@ -64,9 +66,8 @@
"Controls the interval at which a new snapshot is requested prior to initiating a warm migration. The default value is 60 minutes.": "Controls the interval at which a new snapshot is requested prior to initiating a warm migration. The default value is 60 minutes.",
"Copied": "Copied",
"Copy": "Copy",
"Create": "Create",
"Create a migration plan and select VMs from the source provider for migration.": "Create a migration plan and select VMs from the source provider for migration.",
"Create and edit": "Create and edit",
"Create and start": "Create and start",
"Create NetworkMap": "Create NetworkMap",
"Create new provider": "Create new provider",
"Create plan": "Create plan",
Expand Down Expand Up @@ -160,6 +161,7 @@
"If true, the provider's CA certificate won't be validated.": "If true, the provider's CA certificate won't be validated.",
"If true, the provider's TLS certificate won't be validated.": "If true, the provider's TLS certificate won't be validated.",
"Image": "Image",
"Incomplete mapping": "Incomplete mapping",
"Information concerns": "Information concerns",
"Invalid password": "Invalid password",
"Invalid username": "Invalid username",
Expand Down Expand Up @@ -194,6 +196,8 @@
"Migrations (last 7 days)": "Migrations (last 7 days)",
"Migrations for virtualization": "Migrations for virtualization",
"MTU": "MTU",
"Multiple NICs mapped to Pod Networking ": "Multiple NICs mapped to Pod Networking ",
"Multiple NICs on the same network": "Multiple NICs on the same network",
"Must gather cleanup after (hours)": "Must gather cleanup after (hours)",
"Name": "Name",
"Name is primarily intended for creation idempotence and configuration definition. Cannot be updated.": "Name is primarily intended for creation idempotence and configuration definition. Cannot be updated.",
Expand All @@ -212,6 +216,8 @@
"Networks": "Networks",
"Networks used by the selected VMs": "Networks used by the selected VMs",
"New name was generated for the Network Map due to naming conflict.": "New name was generated for the Network Map due to naming conflict.",
"New name was generated for the Storage Map due to naming conflict.": "New name was generated for the Storage Map due to naming conflict.",
"NICs with empty NIC profile": "NICs with empty NIC profile",
"No credentials found.": "No credentials found.",
"No inventory data available.": "No inventory data available.",
"No NetworkMaps found.": "No NetworkMaps found.",
Expand All @@ -227,7 +233,6 @@
"No secret.": "No secret.",
"No StorageMaps found.": "No StorageMaps found.",
"No storages in this category": "No storages in this category",
"No target provider exists ": "No target provider exists ",
"Not Ready": "Not Ready",
"NUMA": "NUMA",
"Number of cluster in provider": "Number of cluster in provider",
Expand Down Expand Up @@ -344,6 +349,7 @@
"Storage": "Storage",
"Storage classes": "Storage classes",
"Storage domains": "Storage domains",
"Storage Map name re-generated": "Storage Map name re-generated",
"Storage map:": "Storage map:",
"Storage mappings have been re-generated": "Storage mappings have been re-generated",
"StorageMaps": "StorageMaps",
Expand Down Expand Up @@ -413,6 +419,9 @@
"Virtual Machine Migrations (last 7 days)": "Virtual Machine Migrations (last 7 days)",
"Virtual machines": "Virtual machines",
"Virtual Machines": "Virtual Machines",
"VM(s) with more than one interface mapped to Pod Networking were detected.": "VM(s) with more than one interface mapped to Pod Networking were detected.",
"VM(s) with multiple NICs on the same network were detected.": "VM(s) with multiple NICs on the same network were detected.",
"VM(s) with NICs that are not linked with a NIC profile were detected.": "VM(s) with NICs that are not linked with a NIC profile were detected.",
"VMs": "VMs",
"VMware only: vSphere product name.": "VMware only: vSphere product name.",
"VMware Virtual Disk Development Kit (VDDK) image, for example: quay.io/kubev2v/vddk:latest .": "VMware Virtual Disk Development Kit (VDDK) image, for example: quay.io/kubev2v/vddk:latest .",
Expand Down
67 changes: 27 additions & 40 deletions ...ift-console-plugin/src/modules/Providers/views/migrate/ProvidersCreateVmMigrationPage.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,22 +4,16 @@ import SectionHeading from 'src/components/headers/SectionHeading';
import { PlanCreateProgress } from 'src/modules/Plans/views/create';
import { ForkliftTrans, useForkliftTranslation } from 'src/utils/i18n';

import { Alert, AlertVariant, Button, Flex, FlexItem, PageSection } from '@patternfly/react-core';
import { LoadingDots } from '@kubev2v/common';
import { Alert, Button, Flex, FlexItem, PageSection } from '@patternfly/react-core';
import BellIcon from '@patternfly/react-icons/dist/esm/icons/bell-icon';

import { PlansCreateForm } from './components/PlansCreateForm';
import { StateAlerts } from './components/StateAlerts';
import { startCreate } from './reducer/actions';
import { GeneralAlerts } from './types';
import { isDone } from './reducer/helpers';
import { useFetchEffects } from './useFetchEffects';
import { useSaveEffect } from './useSaveEffect';

const generalMessages = (
t: (key: string) => string,
): { [key in GeneralAlerts]: { title: string; body: string } } => ({
NEXT_VALID_PROVIDER_SELECTED: { title: t('Error'), body: t('No target provider exists ') },
});

const ProvidersCreateVmMigrationPage: FC = () => {
const { t } = useForkliftTranslation();
const history = useHistory();
Expand All @@ -35,25 +29,29 @@ const ProvidersCreateVmMigrationPage: FC = () => {
return <></>;
}

if (!isDone(state.flow.initialLoading) && !state.flow.apiError) {
return <LoadingDots />;
}

return (
<PageSection variant="light">
<Alert
className="co-alert forklift--create-vm-migration-plan--alert"
customIcon={<BellIcon />}
variant="info"
title={t('How to create a migration plan')}
>
<ForkliftTrans>
To migrate virtual machines select target provider, namespace, mappings and click the{' '}
<strong>Create</strong> button to crete the plan.
</ForkliftTrans>
</Alert>

<PlanCreateProgress step="migrate" />
<PlansCreateForm state={state} dispatch={dispatch}>
<Alert
className="co-alert forklift--create-vm-migration-plan--alert"
customIcon={<BellIcon />}
variant="info"
title={t('How to create a migration plan')}
>
<ForkliftTrans>
To migrate virtual machines select target provider, namespace, mappings and click the{' '}
<strong>Create</strong> button to crete the plan.
</ForkliftTrans>
</Alert>

<SectionHeading text={t('Migrate')} />
<PlanCreateProgress step="migrate" />

<PlansCreateForm state={state} dispatch={dispatch} />
<SectionHeading text={t('Migrate')} />
</PlansCreateForm>
{state.flow.apiError && (
<Alert
className="co-alert co-alert--margin-top"
Expand All @@ -64,29 +62,18 @@ const ProvidersCreateVmMigrationPage: FC = () => {
{state.flow.apiError.message || state.flow.apiError.toString()}
</Alert>
)}
<StateAlerts
variant={AlertVariant.danger}
messages={Array.from(state.alerts.general.errors).map((key) => ({
key,
...generalMessages(t)[key],
}))}
/>
<Flex>
<FlexItem>
<Button
variant="primary"
isDisabled={Object.values(state.validation).some(
(validation) => validation === 'error',
)}
isDisabled={
!!state.flow.apiError ||
Object.values(state.validation).some((validation) => validation === 'error')
}
isLoading={isLoading}
onClick={() => dispatch(startCreate())}
>
{t('Create and edit')}
</Button>
</FlexItem>
<FlexItem>
<Button variant="secondary" isDisabled={true} isLoading={isLoading}>
{t('Create and start')}
{t('Create')}
</Button>
</FlexItem>
<FlexItem>
Expand Down
9 changes: 5 additions & 4 deletions ...console-plugin/src/modules/Providers/views/migrate/components/EditableDescriptionItem.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,8 @@ export const EditableDescriptionItem: FC<{
content: ReactNode;
ariaEditLabel: string;
onEdit: () => void;
}> = ({ title, content, ariaEditLabel = 'Edit', onEdit }) => (
isDisabled?: boolean;
}> = ({ title, content, ariaEditLabel = 'Edit', onEdit, isDisabled = false }) => (
<DescriptionListGroup>
<DescriptionListTerm>{title}</DescriptionListTerm>
<DescriptionListDescription>
Expand All @@ -25,9 +26,9 @@ export const EditableDescriptionItem: FC<{
className="forklift-page-editable-description-item-button"
style={{ paddingTop: 0 }}
variant="plain"
icon={<PencilAltIcon />}
aria-label={ariaEditLabel}
onClick={onEdit}
{...(isDisabled
? {}
: { icon: <PencilAltIcon />, onClick: onEdit, 'aria-Label': ariaEditLabel })}
/>
</div>
</DescriptionListDescription>
Expand Down
57 changes: 55 additions & 2 deletions ...orklift-console-plugin/src/modules/Providers/views/migrate/components/PlansCreateForm.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ import { StateAlerts } from './StateAlerts';

const buildNetworkMessages = (
t: (key: string) => string,
): { [key in NetworkAlerts]: { title: string; body: string } } => ({
): { [key in NetworkAlerts]: { title: string; body: string; blocker?: boolean } } => ({
NET_MAP_NAME_REGENERATED: {
title: t('Network Map name re-generated'),
body: t('New name was generated for the Network Map due to naming conflict.'),
Expand All @@ -63,17 +63,46 @@ const buildNetworkMessages = (
title: t('Network mappings have been re-generated'),
body: t('All discovered networks have been mapped to the default network.'),
},
MULTIPLE_NICS_ON_THE_SAME_NETWORK: {
title: t('Multiple NICs on the same network'),
body: t('VM(s) with multiple NICs on the same network were detected.'),
},
OVIRT_NICS_WITH_EMPTY_PROFILE: {
title: t('NICs with empty NIC profile'),
body: t('VM(s) with NICs that are not linked with a NIC profile were detected.'),
blocker: true,
},
UNMAPPED_NETWORKS: {
title: t('Incomplete mapping'),
body: t('All networks detected on the selected VMs require a mapping.'),
blocker: true,
},
MULTIPLE_NICS_MAPPED_TO_POD_NETWORKING: {
title: t('Multiple NICs mapped to Pod Networking '),
body: t('VM(s) with more than one interface mapped to Pod Networking were detected.'),
blocker: true,
},
});
const buildStorageMessages = (
t: (key: string) => string,
): { [key in StorageAlerts]: { title: string; body: string } } => ({
): { [key in StorageAlerts]: { title: string; body: string; blocker?: boolean } } => ({
STORAGE_MAPPING_REGENERATED: {
title: t('Storage mappings have been re-generated'),
body: t('All discovered storages have been mapped to the default storage.'),
},
STORAGE_MAP_NAME_REGENERATED: {
title: t('Storage Map name re-generated'),
body: t('New name was generated for the Storage Map due to naming conflict.'),
},
UNMAPPED_STORAGES: {
title: t('Incomplete mapping'),
body: t('All storages detected on the selected VMs require a mapping.'),
blocker: true,
},
});

export const PlansCreateForm = ({
children,
state: {
underConstruction: { plan, netMap, storageMap },
validation,
Expand All @@ -99,6 +128,7 @@ export const PlansCreateForm = ({
},
dispatch,
}: {
children?;
state: CreateVmMigrationPageState;
dispatch: (action: PageAction<unknown, unknown>) => void;
}) => {
Expand Down Expand Up @@ -137,6 +167,7 @@ export const PlansCreateForm = ({
}
>
<DrawerContentBody>
{children}
<DescriptionList
className="forklift-create-provider-edit-section"
columnModifier={{
Expand All @@ -160,6 +191,7 @@ export const PlansCreateForm = ({
id="planName"
value={plan.metadata.name}
validated={validation.planName}
isDisabled={flow.editingDone}
onChange={(value) => dispatch(setPlanName(value?.trim() ?? ''))}
/>
</FormGroup>
Expand All @@ -170,6 +202,7 @@ export const PlansCreateForm = ({
content={plan.metadata.name}
ariaEditLabel={t('Edit plan name')}
onEdit={() => setIsNameEdited(true)}
isDisabled={flow.editingDone}
/>
)}
<DetailsItem
Expand Down Expand Up @@ -211,6 +244,7 @@ export const PlansCreateForm = ({
onChange={(value) => dispatch(setPlanTargetProvider(value))}
validated={validation.targetProvider}
id="targetProvider"
isDisabled={flow.editingDone}
>
{[
<FormSelectOption
Expand Down Expand Up @@ -247,6 +281,7 @@ export const PlansCreateForm = ({
}
ariaEditLabel={t('Edit target provider')}
onEdit={() => setIsTargetProviderEdited(true)}
isDisabled={flow.editingDone}
/>
)}
{isTargetNamespaceEdited ||
Expand All @@ -264,6 +299,7 @@ export const PlansCreateForm = ({
onChange={(value) => dispatch(setPlanTargetNamespace(value))}
validated={validation.targetNamespace}
id="targetNamespace"
isDisabled={flow.editingDone}
>
{[
<FormSelectOption
Expand Down Expand Up @@ -302,6 +338,7 @@ export const PlansCreateForm = ({
}
ariaEditLabel={t('Edit target namespace')}
onEdit={() => setIsTargetNamespaceEdited(true)}
isDisabled={flow.editingDone}
/>
)}
<DescriptionListGroup>
Expand All @@ -318,6 +355,14 @@ export const PlansCreateForm = ({
</span>
</DescriptionListTerm>
<DescriptionListDescription className="forklift-page-mapping-list">
<StateAlerts
variant={AlertVariant.danger}
messages={Array.from(alerts.networkMappings.errors).map((key) => ({
key,
...networkMessages[key],
}))}
onClose={(key) => dispatch(removeAlert(key as NetworkAlerts))}
/>
<StateAlerts
variant={AlertVariant.warning}
messages={Array.from(alerts.networkMappings.warnings).map((key) => ({
Expand Down Expand Up @@ -356,6 +401,14 @@ export const PlansCreateForm = ({
</span>
</DescriptionListTerm>
<DescriptionListDescription className="forklift-page-mapping-list">
<StateAlerts
variant={AlertVariant.danger}
messages={Array.from(alerts.storageMappings.errors).map((key) => ({
key,
...storageMessages[key],
}))}
onClose={(key) => dispatch(removeAlert(key as StorageAlerts))}
/>
<StateAlerts
variant={AlertVariant.warning}
messages={Array.from(alerts.storageMappings.warnings).map((key) => ({
Expand Down
8 changes: 5 additions & 3 deletions ...es/forklift-console-plugin/src/modules/Providers/views/migrate/components/StateAlerts.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,17 @@ import { Alert, AlertActionCloseButton, AlertVariant } from '@patternfly/react-c

export const StateAlerts: FC<{
variant: AlertVariant;
messages: { key: string; title: string; body: string }[];
messages: { key: string; title: string; body: string; blocker?: boolean }[];
onClose?: (key: string) => void;
}> = ({ variant, messages, onClose }) => (
<>
{messages.map(({ key, title, body }) => (
{messages.map(({ key, title, body, blocker }) => (
<Alert
key={key}
isInline
actionClose={onClose ? <AlertActionCloseButton onClose={() => onClose(key)} /> : undefined}
actionClose={
onClose && !blocker ? <AlertActionCloseButton onClose={() => onClose(key)} /> : undefined
}
variant={variant}
title={title}
>
Expand Down
Loading

0 comments on commit 3b70f66

Please sign in to comment.