Skip to content

Commit

Permalink
Merge pull request #3585 from suomiy/kubevirt.vmWizardEnhancements2
Browse files Browse the repository at this point in the history
kubevirt: make VM Wizard errors more explicit
  • Loading branch information
openshift-merge-robot committed Dec 3, 2019
2 parents 38ce98b + 4b7d256 commit d1359cb
Show file tree
Hide file tree
Showing 29 changed files with 272 additions and 96 deletions.
@@ -1,5 +1,4 @@
.kubevirt-create-vm-modal__footer-error {
width: 100%;
margin-bottom: 1em;
align-items: center;
}
Expand Up @@ -12,9 +12,10 @@ import {
import * as _ from 'lodash';
import { Prompt } from 'react-router';
import { useShowErrorToggler } from '../../hooks/use-show-error-toggler';
import { getDialogUIError } from '../../utils/strings';
import { getDialogUIError, getSimpleDialogUIError } from '../../utils/strings';
import { ALL_VM_WIZARD_TABS, VMWizardProps, VMWizardTab } from './types';
import {
getStepError,
hasStepAllRequiredFilled,
isStepLocked,
isStepValid,
Expand Down Expand Up @@ -54,6 +55,8 @@ const CreateVMWizardFooterComponent: React.FC<CreateVMWizardFooterComponentProps
const activeStepID = activeStep.id as VMWizardTab;
const isLocked = _.some(ALL_VM_WIZARD_TABS, (id) => isStepLocked(stepData, id));
const isValid = isStepValid(stepData, activeStepID);
const hasStepAllRequired = hasStepAllRequiredFilled(stepData, activeStepID);
const stepError = getStepError(stepData, activeStepID);
checkValidity(isValid);

const isFirstStep = activeStepID === VMWizardTab.VM_SETTINGS;
Expand All @@ -79,11 +82,17 @@ const CreateVMWizardFooterComponent: React.FC<CreateVMWizardFooterComponentProps
/>
{!isValid && showError && (
<Alert
title={getDialogUIError(hasStepAllRequiredFilled(stepData, activeStepID))}
title={
stepError
? getSimpleDialogUIError(hasStepAllRequired)
: getDialogUIError(hasStepAllRequired)
}
isInline
variant="danger"
className="kubevirt-create-vm-modal__footer-error"
/>
>
{stepError}
</Alert>
)}
{!isLastStep && (
<Button
Expand Down
@@ -1,11 +1,14 @@
import { CloudInitField } from '../../types';
import { InitialStepStateGetter } from './types';

export const getCloudInitInitialState = () => ({
export const getCloudInitInitialState: InitialStepStateGetter = () => ({
value: {
[CloudInitField.IS_FORM]: {
value: true,
},
},
error: null,
isValid: true,
hasAllRequiredFilled: true,
isLocked: false,
});
Expand Up @@ -5,8 +5,9 @@ import { getStorageInitialState } from './storage-tab-initial-state';
import { getResultInitialState } from './result-tab-initial-state';
import { getReviewInitialState } from './review-tab-initial-state';
import { getCloudInitInitialState } from './cloud-init-tab-initial-state';
import { InitialStepStateGetter, StepState } from './types';

const initialStateGetterResolver = {
const initialStateGetterResolver: { [key in VMWizardTab]: InitialStepStateGetter } = {
[VMWizardTab.VM_SETTINGS]: getVmSettingsInitialState,
[VMWizardTab.NETWORKING]: getNetworksInitialState,
[VMWizardTab.STORAGE]: getStorageInitialState,
Expand All @@ -15,13 +16,21 @@ const initialStateGetterResolver = {
[VMWizardTab.RESULT]: getResultInitialState,
};

export const getTabInitialState = (tabKey: VMWizardTab, props: CommonData) => {
export const getTabInitialState = (tabKey: VMWizardTab, data: CommonData): StepState => {
const getter = initialStateGetterResolver[tabKey];

let result;
let result: StepState;
if (getter) {
result = getter(props);
result = getter(data);
}

return result || { value: {}, isValid: false, isLocked: false, hasAllRequiredFilled: false };
return (
result || {
value: {},
error: null,
isValid: false,
isLocked: false,
hasAllRequiredFilled: false,
}
);
};
Expand Up @@ -3,6 +3,7 @@ import { NetworkInterfaceWrapper } from '../../../../k8s/wrapper/vm/network-inte
import { NetworkInterfaceModel, NetworkType } from '../../../../constants/vm/network';
import { NetworkWrapper } from '../../../../k8s/wrapper/vm/network-wrapper';
import { getSequenceName } from '../../../../utils/strings';
import { InitialStepStateGetter } from './types';

export const podNetwork: VMWizardNetwork = {
id: '0',
Expand All @@ -18,8 +19,10 @@ export const podNetwork: VMWizardNetwork = {
}).asResource(),
};

export const getNetworksInitialState = () => ({
export const getNetworksInitialState: InitialStepStateGetter = () => ({
value: [podNetwork],
isValid: true,
error: null,
hasAllRequiredFilled: true,
isValid: true,
isLocked: false,
});
@@ -1,8 +1,9 @@
import { VMImportProvider, VMWareProviderField } from '../../../types';
import { asDisabled, asHidden } from '../../../utils/utils';
import { V2VVMwareStatus } from '../../../../../statuses/v2vvmware';
import { VMwareSettings } from '../types';

export const getVmWareInitialState = () => {
export const getVmWareInitialState = (): VMwareSettings => {
const hiddenByVCenter = asHidden(true, VMWareProviderField.VCENTER);
const fields = {
[VMWareProviderField.VCENTER]: {},
Expand Down
@@ -1,9 +1,14 @@
export const getResultInitialState = () => ({
import { InitialStepStateGetter } from './types';

export const getResultInitialState: InitialStepStateGetter = () => ({
value: {
mainError: null,
errors: [],
requestResults: [],
},
error: null,
isLocked: false,
hasAllRequiredFilled: null,
isValid: null,
isPending: false,
});
@@ -1,5 +1,9 @@
export const getReviewInitialState = () => ({
import { InitialStepStateGetter } from './types';

export const getReviewInitialState: InitialStepStateGetter = () => ({
value: {},
error: null,
isValid: true,
hasAllRequiredFilled: true,
isLocked: false,
});
Expand Up @@ -13,6 +13,7 @@ import { ProvisionSource } from '../../../../constants/vm/provision-source';
import { VM_TEMPLATE_NAME_PARAMETER } from '../../../../constants/vm-templates';
import { BinaryUnit } from '../../../form/size-unit-utils';
import { WINTOOLS_CONTAINER_NAMES } from '../../../modals/cdrom-vm-modal/constants';
import { InitialStepStateGetter } from './types';

const ROOT_DISK_NAME = 'rootdisk';
const WINTOOLS_DISK_NAME = 'windows-guest-tools';
Expand Down Expand Up @@ -80,8 +81,10 @@ export const getProvisionSourceStorage = (provisionSource: ProvisionSource): VMW
return null;
};

export const getStorageInitialState = () => ({
export const getStorageInitialState: InitialStepStateGetter = () => ({
value: [],
isValid: true, // empty Storages are valid
error: null,
hasAllRequiredFilled: true,
isValid: true, // empty Storages are valid
isLocked: false,
});
@@ -0,0 +1,32 @@
import { CommonData, VMImportProvider, VMSettingsField, VMWareProviderField } from '../../types';

export type StepState = {
value: any;
error: string;
isValid: boolean;
isLocked: boolean;
hasAllRequiredFilled: boolean;
};

export type InitialStepStateGetter = (data: CommonData) => StepState;

export type FieldMultiFlag = { [k: string]: boolean };

type SettingsField = {
key?: VMSettingsField | VMWareProviderField;
value?: any;
isHidden?: FieldMultiFlag;
isRequired?: FieldMultiFlag;
skipValidation?: boolean;
};

export type VMwareSettings = { [key in VMWareProviderField]: SettingsField } & {
[VMWareProviderField.V2V_NAME]: string;
[VMWareProviderField.NEW_VCENTER_NAME]: string;
};

export type VMSettings = { [key in VMSettingsField]: SettingsField } & {
[VMSettingsField.PROVIDERS_DATA]: {
[VMImportProvider.VMWARE]?: VMwareSettings;
};
};
Expand Up @@ -3,11 +3,28 @@ import { CommonData, VMSettingsField, VMWizardProps } from '../../types';
import { asHidden, asRequired } from '../../utils/utils';
import { ProvisionSource } from '../../../../constants/vm/provision-source';
import { getProviders } from '../../provider-definitions';
import { InitialStepStateGetter, VMSettings } from './types';

export const getInitialVmSettings = (common: CommonData) => {
export const vmSettingsOrder = {
[VMSettingsField.PROVIDER]: 0,
[VMSettingsField.USER_TEMPLATE]: 1,
[VMSettingsField.PROVISION_SOURCE_TYPE]: 2,
[VMSettingsField.CONTAINER_IMAGE]: 3,
[VMSettingsField.IMAGE_URL]: 4,
[VMSettingsField.OPERATING_SYSTEM]: 5,
[VMSettingsField.FLAVOR]: 6,
[VMSettingsField.MEMORY]: 7,
[VMSettingsField.CPU]: 8,
[VMSettingsField.WORKLOAD_PROFILE]: 9,
[VMSettingsField.NAME]: 10,
[VMSettingsField.DESCRIPTION]: 11,
[VMSettingsField.START_VM]: 12,
};

export const getInitialVmSettings = (data: CommonData): VMSettings => {
const {
data: { isCreateTemplate, isProviderImport },
} = common;
} = data;

const hiddenByProvider = asHidden(isProviderImport, VMWizardProps.isProviderImport);
const hiddenByProviderOrTemplate = isProviderImport
Expand All @@ -23,6 +40,7 @@ export const getInitialVmSettings = (common: CommonData) => {
[VMSettingsField.NAME]: {
isRequired: asRequired(true),
},
[VMSettingsField.HOSTNAME]: {},
[VMSettingsField.DESCRIPTION]: {},
[VMSettingsField.USER_TEMPLATE]: {
isHidden: hiddenByProviderOrTemplate,
Expand Down Expand Up @@ -75,7 +93,10 @@ export const getInitialVmSettings = (common: CommonData) => {
return fields;
};

export const getVmSettingsInitialState = (props) => ({
value: getInitialVmSettings(props),
export const getVmSettingsInitialState: InitialStepStateGetter = (data) => ({
value: getInitialVmSettings(data),
error: null,
hasAllRequiredFilled: false,
isValid: false,
isLocked: false,
});
Expand Up @@ -47,12 +47,14 @@ export const vmWizardInternalActions: VMWizardInternalActions = {
tab: VMWizardTab,
isValid: boolean,
hasAllRequiredFilled: boolean,
error: string,
) => ({
payload: {
id,
tab,
isValid,
hasAllRequiredFilled,
error,
},
type: InternalActionType.SetTabValidity,
}),
Expand Down
Expand Up @@ -184,7 +184,8 @@ export default (state, action: WizardInternalAction) => {
.setIn(
[dialogID, 'tabs', payload.tab, 'hasAllRequiredFilled'],
payload.hasAllRequiredFilled,
);
)
.setIn([dialogID, 'tabs', payload.tab, 'error'], payload.error);
case InternalActionType.SetTabLocked:
return state.setIn([dialogID, 'tabs', payload.tab, 'isLocked'], payload.isLocked);
case InternalActionType.SetVmSettingsFieldValue:
Expand Down
Expand Up @@ -76,6 +76,7 @@ export type WizardInternalAction = {
deviceID?: string;
deviceType?: DeviceType;
bootOrder?: number;
error?: string;
};
};

Expand Down
Expand Up @@ -58,17 +58,22 @@ export const setNetworksTabValidity = (options: UpdateOptions) => {
const { id, dispatch, getState } = options;
const state = getState();
const iNetworks = iGetNetworks(state, id);
let error = null;

let hasAllRequiredFilled = iNetworks.every((iNetwork) =>
iGetIn(iNetwork, ['validation', 'hasAllRequiredFilled']),
);

if (hasAllRequiredFilled && iGetProvisionSource(state, id) === ProvisionSource.PXE) {
hasAllRequiredFilled = !!iNetworks.find(
if (iGetProvisionSource(state, id) === ProvisionSource.PXE) {
const hasBootSource = !!iNetworks.find(
(networkBundle) =>
!iGetIn(networkBundle, ['network', 'pod']) &&
iGetIn(networkBundle, ['networkInterface', 'bootOrder']) === 1,
);
if (!hasBootSource) {
error = 'Please select the boot source.';
hasAllRequiredFilled = false;
}
}

let isValid = hasAllRequiredFilled;
Expand All @@ -77,13 +82,16 @@ export const setNetworksTabValidity = (options: UpdateOptions) => {
isValid = iNetworks.every((iNetwork) => iGetIn(iNetwork, ['validation', 'isValid']));
}

if (checkTabValidityChanged(state, id, VMWizardTab.NETWORKING, isValid, hasAllRequiredFilled)) {
if (
checkTabValidityChanged(state, id, VMWizardTab.NETWORKING, isValid, hasAllRequiredFilled, error)
) {
dispatch(
vmWizardInternalActions[InternalActionType.SetTabValidity](
id,
VMWizardTab.NETWORKING,
isValid,
hasAllRequiredFilled,
error,
),
);
}
Expand Down
Expand Up @@ -69,19 +69,24 @@ export const setStoragesTabValidity = (options: UpdateOptions) => {
const { id, dispatch, getState } = options;
const state = getState();
const iStorages = iGetStorages(state, id);
let error = null;

let hasAllRequiredFilled = iStorages.every((iStorage) =>
iGetIn(iStorage, ['validation', 'hasAllRequiredFilled']),
);

if (hasAllRequiredFilled && iGetProvisionSource(state, id) === ProvisionSource.DISK) {
hasAllRequiredFilled = !!iStorages.find(
if (iGetProvisionSource(state, id) === ProvisionSource.DISK) {
const hasBootSource = !!iStorages.find(
(storageBundle) =>
iGetIn(storageBundle, ['disk', 'bootOrder']) === 1 &&
iGetIn(storageBundle, ['disk', 'disk']) &&
(iGetIn(storageBundle, ['volume', 'dataVolume']) ||
iGetIn(storageBundle, ['volume', 'persistentVolumeClaim'])),
);
if (!hasBootSource) {
error = 'Please select the boot source.';
hasAllRequiredFilled = false;
}
}

let isValid = hasAllRequiredFilled;
Expand All @@ -90,13 +95,16 @@ export const setStoragesTabValidity = (options: UpdateOptions) => {
isValid = iStorages.every((iStorage) => iGetIn(iStorage, ['validation', 'isValid']));
}

if (checkTabValidityChanged(state, id, VMWizardTab.STORAGE, isValid, hasAllRequiredFilled)) {
if (
checkTabValidityChanged(state, id, VMWizardTab.STORAGE, isValid, hasAllRequiredFilled, error)
) {
dispatch(
vmWizardInternalActions[InternalActionType.SetTabValidity](
id,
VMWizardTab.STORAGE,
isValid,
hasAllRequiredFilled,
error,
),
);
}
Expand Down

0 comments on commit d1359cb

Please sign in to comment.