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

kubevirt: make VM Wizard errors more explicit #3585

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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
@@ -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