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

Signed-off-by: Radoslaw Szwajkowski <[email protected]>
  • Loading branch information
@rszwajko
rszwajko committed Feb 12, 2024
1 parent 34ea621 commit 640e540
Show file tree
Hide file tree
Showing 15 changed files with 531 additions and 201 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
14 changes: 12 additions & 2 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 Down Expand Up @@ -344,6 +350,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 @@ -410,6 +417,9 @@
"Virtual Machine Migrations": "Virtual Machine Migrations",
"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
44 changes: 20 additions & 24 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 @@ -37,23 +37,23 @@ const ProvidersCreateVmMigrationPage: FC = () => {

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 @@ -75,18 +75,14 @@ const ProvidersCreateVmMigrationPage: FC = () => {
<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
46 changes: 45 additions & 1 deletion ...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,6 +63,25 @@ 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,
Expand All @@ -71,9 +90,18 @@ const buildStorageMessages = (
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.'),
},
});

export const PlansCreateForm = ({
children,
state: {
underConstruction: { plan, netMap, storageMap },
validation,
Expand All @@ -99,6 +127,7 @@ export const PlansCreateForm = ({
},
dispatch,
}: {
children?;
state: CreateVmMigrationPageState;
dispatch: (action: PageAction<unknown, unknown>) => void;
}) => {
Expand Down Expand Up @@ -137,6 +166,7 @@ export const PlansCreateForm = ({
}
>
<DrawerContentBody>
{children}
<DescriptionList
className="forklift-create-provider-edit-section"
columnModifier={{
Expand All @@ -160,6 +190,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 +201,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 +243,7 @@ export const PlansCreateForm = ({
onChange={(value) => dispatch(setPlanTargetProvider(value))}
validated={validation.targetProvider}
id="targetProvider"
isDisabled={flow.editingDone}
>
{[
<FormSelectOption
Expand Down Expand Up @@ -247,6 +280,7 @@ export const PlansCreateForm = ({
}
ariaEditLabel={t('Edit target provider')}
onEdit={() => setIsTargetProviderEdited(true)}
isDisabled={flow.editingDone}
/>
)}
{isTargetNamespaceEdited ||
Expand All @@ -264,6 +298,7 @@ export const PlansCreateForm = ({
onChange={(value) => dispatch(setPlanTargetNamespace(value))}
validated={validation.targetNamespace}
id="targetNamespace"
isDisabled={flow.editingDone}
>
{[
<FormSelectOption
Expand Down Expand Up @@ -302,6 +337,7 @@ export const PlansCreateForm = ({
}
ariaEditLabel={t('Edit target namespace')}
onEdit={() => setIsTargetNamespaceEdited(true)}
isDisabled={flow.editingDone}
/>
)}
<DescriptionListGroup>
Expand All @@ -318,6 +354,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
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
10 changes: 5 additions & 5 deletions packages/forklift-console-plugin/src/modules/Providers/views/migrate/reducer/actions.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -147,8 +147,8 @@ export interface PlanAvailableSourceStorages {
error?: Error;
}

export interface PlanNickProfiles {
nickProfiles: OVirtNicProfile[];
export interface PlanNicProfiles {
nicProfiles: OVirtNicProfile[];
loading: boolean;
error?: Error;
}
Expand Down Expand Up @@ -346,12 +346,12 @@ export const setAvailableTargetStorages = (
});

export const setNicProfiles = (
nickProfiles: OVirtNicProfile[],
nicProfiles: OVirtNicProfile[],
nicProfilesLoading: boolean,
nicProfilesError: Error,
): PageAction<CreateVmMigration, PlanNickProfiles> => ({
): PageAction<CreateVmMigration, PlanNicProfiles> => ({
type: 'SET_NICK_PROFILES',
payload: { nickProfiles, loading: nicProfilesLoading, error: nicProfilesError },
payload: { nicProfiles: nicProfiles, loading: nicProfilesLoading, error: nicProfilesError },
});

export const setDisks = (
Expand Down
Loading

0 comments on commit 640e540

Please sign in to comment.