Skip to content

Commit

Permalink
kubevirt: deprecate using StorageClassConfigMap in inappropriate plac…
Browse files Browse the repository at this point in the history
…ess + SC/volume-access modes fixes

- introduce useStorageClassConfigMap hook
- move  volume/access modes resolution into the places where dv/pvc gets created
  - from modal
  - from import
- prefer visible volume/access mode
- deprecate assertDefaultModes
- add storageClassConfigMap to common data in VMWizard
- fix disk modal
    - allow empty modes
    - allow empty or no storage class
    - fix enums
- show  volume/access modes in ReviewTab
- remove async and get requests from modal patches
- show CDs in disks tab to allow editing of volume/access modes
- allow editing storages until the DV gets created
- simplify create/import VM request flow
  • Loading branch information
suomiy committed Apr 1, 2020
1 parent bb53fec commit 04a8d37
Show file tree
Hide file tree
Showing 30 changed files with 396 additions and 355 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@ import { connect } from 'react-redux';
import { Wizard, WizardStep } from '@patternfly/react-core';
import { FLAGS } from '@console/shared';
import {
featureReducerName,
connectToFlags,
featureReducerName,
FlagsObject,
} from '@console/internal/reducers/features';
import { TemplateModel } from '@console/internal/models';
Expand Down Expand Up @@ -38,7 +38,7 @@ import {
VMWizardStorage,
VMWizardTab,
} from './types';
import { CREATE_VM, CREATE_VM_TEMPLATE, TabTitleResolver, IMPORT_VM } from './strings/strings';
import { CREATE_VM, CREATE_VM_TEMPLATE, IMPORT_VM, TabTitleResolver } from './strings/strings';
import { vmWizardActions } from './redux/actions';
import { ActionType } from './redux/types';
import { getResultInitialState } from './redux/initial-state/result-tab-initial-state';
Expand All @@ -58,6 +58,7 @@ import { ResultTab } from './tabs/result-tab/result-tab';
import { StorageTab } from './tabs/storage-tab/storage-tab';
import { CloudInitTab } from './tabs/cloud-init-tab/cloud-init-tab';
import { VirtualHardwareTab } from './tabs/virtual-hardware-tab/virtual-hardware-tab';
import { useStorageClassConfigMapWrapped } from '../../hooks/storage-class-config-map';

import './create-vm-wizard.scss';

Expand Down Expand Up @@ -88,10 +89,18 @@ export class CreateVMWizardComponent extends React.Component<CreateVMWizardCompo
const referencesChanged = !_.isEqual(prevProps.dataIDReferences, this.props.dataIDReferences);

if (changedProps.size > 0 || referencesChanged) {
this.props.onCommonDataChanged(
referencesChanged ? { dataIDReferences: this.props.dataIDReferences } : undefined,
changedProps,
);
let commonDataUpdate: CommonData = referencesChanged
? { dataIDReferences: this.props.dataIDReferences }
: undefined;
if (changedProps.has(VMWizardProps.storageClassConfigMap)) {
commonDataUpdate = {
...commonDataUpdate,
data: {
[VMWizardProps.storageClassConfigMap]: this.props[VMWizardProps.storageClassConfigMap],
},
};
}
this.props.onCommonDataChanged(commonDataUpdate, changedProps);
}
}

Expand Down Expand Up @@ -144,6 +153,7 @@ export class CreateVMWizardComponent extends React.Component<CreateVMWizardCompo
iCommonTemplates,
namespace: activeNamespace,
openshiftFlag,
isTemplate: isCreateTemplate,
})
.then(() => getResults(enhancedK8sMethods))
.catch((error) => cleanupAndGetResults(enhancedK8sMethods, error))
Expand Down Expand Up @@ -314,10 +324,12 @@ export class CreateVMWizardComponent extends React.Component<CreateVMWizardCompo
const wizardStateToProps = (state, { reduxID }) => ({
stepData: iGetCreateVMWizardTabs(state, reduxID),
// fetch data from store to detect and fire changes
...[...DetectCommonDataChanges].reduce((acc, propName) => {
acc[propName] = iGetCommonData(state, reduxID, propName);
return acc;
}, {}),
...[...DetectCommonDataChanges]
.filter((v) => v !== VMWizardProps.storageClassConfigMap) // passed directly
.reduce((acc, propName) => {
acc[propName] = iGetCommonData(state, reduxID, propName);
return acc;
}, {}),
});

const wizardDispatchToProps = (dispatch, props) => ({
Expand All @@ -328,9 +340,10 @@ const wizardDispatchToProps = (dispatch, props) => ({
isCreateTemplate: props.isCreateTemplate,
isProviderImport: props.isProviderImport,
userTemplateName: props.userTemplateName,
storageClassConfigMap: undefined,
},
dataIDReferences: props.dataIDReferences,
}),
} as CommonData),
);
},
onCommonDataChanged: (commonData: CommonData, changedCommonData: ChangedCommonData) => {
Expand Down Expand Up @@ -391,6 +404,8 @@ export const CreateVMWizardPageComponent: React.FC<CreateVMWizardPageComponentPr
);
}

const storageClassConfigMap = useStorageClassConfigMapWrapped();

const searchParams = new URLSearchParams(search);
const userMode = searchParams.get('mode') || VMWizardMode.VM;
const userTemplateName = (userMode === VMWizardMode.VM && searchParams.get('template')) || '';
Expand All @@ -407,6 +422,7 @@ export const CreateVMWizardPageComponent: React.FC<CreateVMWizardPageComponentPr
isProviderImport={userMode === VMWizardMode.IMPORT}
userTemplateName={userTemplateName}
dataIDReferences={dataIDReferences}
storageClassConfigMap={storageClassConfigMap}
reduxID={reduxID}
onClose={() => history.goBack()}
/>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,17 +14,20 @@ import {
VMWareProviderProps,
VMWizardNetwork,
VMWizardNetworkType,
VMWizardProps,
VMWizardStorage,
VMWizardStorageType,
} from '../../../../types';
import { iGetLoadedCommonData } from '../../../../selectors/immutable/selectors';
import { vmWizardInternalActions } from '../../../internal-actions';
import {
AccessMode,
CUSTOM_FLAVOR,
DiskBus,
DiskType,
NetworkInterfaceModel,
NetworkInterfaceType,
VolumeMode,
VolumeType,
} from '../../../../../../constants/vm';
import { toShallowJS } from '../../../../../../utils/immutable';
Expand All @@ -36,6 +39,11 @@ import { CONVERSION_POD_TEMP_MOUNT_PATH } from '../../../../../../constants/v2v'
import { PersistentVolumeClaimWrapper } from '../../../../../../k8s/wrapper/vm/persistent-volume-claim-wrapper';
import { getStringEnumValues } from '../../../../../../utils/types';
import { BinaryUnit } from '../../../../../form/size-unit-utils';
import {
getDefaultSCAccessModes,
getDefaultSCVolumeMode,
} from '../../../../../../selectors/config-map/sc-defaults';
import { ConfigMapKind } from '@console/internal/module/k8s';

const convert = (value: number, unit: BinaryUnit) => {
const units = getStringEnumValues<BinaryUnit>(BinaryUnit);
Expand Down Expand Up @@ -76,7 +84,7 @@ export const getNics = (parsedVm): VMWizardNetwork[] => {
});
};

export const getDisks = (parsedVm): VMWizardStorage[] => {
export const getDisks = (parsedVm, storageClassConfigMap: ConfigMapKind): VMWizardStorage[] => {
const devices = _.get(parsedVm, ['Config', 'Hardware', 'Device']);

// if the device is a disk, it has "capacityInKB" present
Expand Down Expand Up @@ -120,6 +128,8 @@ export const getDisks = (parsedVm): VMWizardStorage[] => {
size: size.value,
unit: size.unit,
})
.setVolumeMode(getDefaultSCVolumeMode(storageClassConfigMap))
.setAccessModes(getDefaultSCAccessModes(storageClassConfigMap))
.asResource(),
importData: {
fileName: _.get(device, ['Backing', 'FileName']),
Expand Down Expand Up @@ -150,6 +160,8 @@ export const getDisks = (parsedVm): VMWizardStorage[] => {
size: 2,
unit: BinaryUnit.Gi,
})
.setVolumeMode(VolumeMode.FILESYSTEM)
.setAccessModes([AccessMode.READ_WRITE_ONCE])
.asResource(),
importData: {
mountPath: CONVERSION_POD_TEMP_MOUNT_PATH, // hardcoded; always Filesystem mode
Expand All @@ -166,6 +178,12 @@ export const prefillUpdateCreator = (options: UpdateOptions) => {
const vm = iGetVMWareFieldAttribute(state, id, VMWareProviderField.VM, 'vm');
const vmwareToKubevirtOsConfigMap = toShallowJS(
iGetLoadedCommonData(state, id, VMWareProviderProps.vmwareToKubevirtOsConfigMap),
undefined,
true,
);
const storageClassConfigMap = toShallowJS(
iGetLoadedCommonData(state, id, VMWizardProps.storageClassConfigMap),
undefined,
true,
);

Expand Down Expand Up @@ -204,5 +222,10 @@ export const prefillUpdateCreator = (options: UpdateOptions) => {
}),
);
dispatch(vmWizardInternalActions[InternalActionType.SetNetworks](id, getNics(parsedVm)));
dispatch(vmWizardInternalActions[InternalActionType.SetStorages](id, getDisks(parsedVm)));
dispatch(
vmWizardInternalActions[InternalActionType.SetStorages](
id,
getDisks(parsedVm, storageClassConfigMap),
),
);
};
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ export const setStoragesTabValidity = (options: UpdateOptions) => {
(storageBundle) =>
iGetIn(storageBundle, ['disk', 'bootOrder']) === 1 &&
iGetIn(storageBundle, ['disk', 'disk']) &&
(iGetIn(storageBundle, ['volume', 'dataVolume']) ||
(iGetIn(storageBundle, ['volume', 'dataVolume', 'spec', 'source', 'pvc']) ||
iGetIn(storageBundle, ['volume', 'persistentVolumeClaim'])),
);
if (!hasBootSource) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import * as React from 'react';
import { connect } from 'react-redux';
import { Firehose } from '@console/internal/components/utils';
import { Firehose, FirehoseResult } from '@console/internal/components/utils';
import { createModalLauncher, ModalComponentProps } from '@console/internal/components/factory';
import {
NamespaceModel,
Expand All @@ -26,6 +26,8 @@ import { VM_TEMPLATE_NAME_PARAMETER } from '../../../../constants/vm-templates';
import { PersistentVolumeClaimWrapper } from '../../../../k8s/wrapper/vm/persistent-volume-claim-wrapper';
import { TemplateValidations } from '../../../../utils/validations/template/template-validations';
import { getTemplateValidation } from '../../selectors/template';
import { ConfigMapKind } from '@console/internal/module/k8s';
import { toShallowJS } from '../../../../utils/immutable';

const VMWizardStorageModal: React.FC<VMWizardStorageModalProps> = (props) => {
const {
Expand All @@ -37,6 +39,7 @@ const VMWizardStorageModal: React.FC<VMWizardStorageModalProps> = (props) => {
addUpdateStorage,
storages,
templateValidations,
storageClassConfigMap,
...restProps
} = props;
const {
Expand Down Expand Up @@ -87,6 +90,7 @@ const VMWizardStorageModal: React.FC<VMWizardStorageModalProps> = (props) => {
<Firehose resources={resources}>
<DiskModal
{...restProps}
storageClassConfigMap={storageClassConfigMap}
vmName={VM_TEMPLATE_NAME_PARAMETER}
vmNamespace={vmNamespace}
namespace={namespace}
Expand Down Expand Up @@ -144,6 +148,7 @@ type VMWizardStorageModalProps = ModalComponentProps & {
namespace: string;
useProjects?: boolean;
isCreateTemplate: boolean;
storageClassConfigMap: FirehoseResult<ConfigMapKind>;
storages: VMWizardStorageWithWrappers[];
addUpdateStorage: (storage: VMWizardStorage) => void;
templateValidations: TemplateValidations;
Expand All @@ -155,6 +160,9 @@ const stateToProps = (state, { wizardReduxID }) => {
useProjects,
namespace: iGetCommonData(state, wizardReduxID, VMWizardProps.activeNamespace),
isCreateTemplate: iGetCommonData(state, wizardReduxID, VMWizardProps.isCreateTemplate),
storageClassConfigMap: toShallowJS(
iGetCommonData(state, wizardReduxID, VMWizardProps.storageClassConfigMap),
),
storages: getStoragesWithWrappers(state, wizardReduxID),
templateValidations: getTemplateValidation(state, wizardReduxID),
};
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { FirehoseResult } from '@console/internal/components/utils';
import { TemplateKind } from '@console/internal/module/k8s';
import { ConfigMapKind, TemplateKind } from '@console/internal/module/k8s';
import { getStringEnumValues } from '../../utils/types';
import { V1Network, V1NetworkInterface, VMKind } from '../../types/vm';
import { NetworkInterfaceWrapper } from '../../k8s/wrapper/vm/network-interface-wrapper';
Expand Down Expand Up @@ -37,6 +37,7 @@ export enum VMWizardProps {
userTemplates = 'userTemplates',
commonTemplates = 'commonTemplates',
dataVolumes = 'dataVolumes',
storageClassConfigMap = 'storageClassConfigMap',
}

export const ALL_VM_WIZARD_TABS = getStringEnumValues<VMWizardTab>(VMWizardTab);
Expand Down Expand Up @@ -121,6 +122,7 @@ export type ChangedCommonDataProp =
| VMWizardProps.userTemplates
| VMWizardProps.commonTemplates
| VMWizardProps.dataVolumes
| VMWizardProps.storageClassConfigMap
| VMWizardProps.openshiftFlag
| VMWareProviderProps.deployment
| VMWareProviderProps.deploymentPods
Expand All @@ -143,6 +145,7 @@ export const DetectCommonDataChanges = new Set<ChangedCommonDataProp>([
VMWizardProps.virtualMachines,
VMWizardProps.userTemplates,
VMWizardProps.commonTemplates,
VMWizardProps.storageClassConfigMap,
VMWizardProps.dataVolumes,
VMWareProviderProps.deployment,
VMWareProviderProps.deploymentPods,
Expand All @@ -157,6 +160,11 @@ export type CommonData = {
isCreateTemplate?: boolean;
isProviderImport?: boolean;
userTemplateName?: string;
storageClassConfigMap?: {
loaded: boolean;
loadError: string;
data: ConfigMapKind;
};
};
dataIDReferences?: IDReferences;
};
Expand All @@ -172,6 +180,7 @@ export type CreateVMWizardComponentProps = {
userTemplates: FirehoseResult<TemplateKind[]>;
commonTemplates: FirehoseResult<TemplateKind[]>;
virtualMachines: FirehoseResult<VMKind[]>;
storageClassConfigMap: FirehoseResult<ConfigMapKind>;
onInitialize: () => void;
onClose: (disposeOnly: boolean) => void;
onCommonDataChanged: (commonData: CommonData, commonDataChanged: ChangedCommonData) => void;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import { initialDisk, WINTOOLS_CONTAINER_NAMES, StorageType } from './constants'
import './cdrom-modal.scss';
import { CD, CDMap } from './types';
import { VMKind } from '../../../types/vm';
import { useStorageClassConfigMap } from '../../../hooks/storage-class-config-map';

export const AddCDButton = ({ className, text, onClick, isDisabled }: AddCDButtonProps) => (
<div className={className}>
Expand Down Expand Up @@ -60,7 +61,7 @@ export const CDRomModal = withHandlePromise((props: CDRomModalProps) => {
const {
vmLikeEntity,
handlePromise,
inProgress,
inProgress: _inProgress,
errorMessage,
persistentVolumeClaims,
storageClasses,
Expand All @@ -70,6 +71,9 @@ export const CDRomModal = withHandlePromise((props: CDRomModalProps) => {
} = props;
const vm = asVM(vmLikeEntity);

const [storageClassConfigMap, isStorageClassConfigMapLoaded] = useStorageClassConfigMap();
const inProgress = _inProgress || !isStorageClassConfigMapLoaded;

const mapCDsToSource = (cds) =>
Object.assign(
{},
Expand Down Expand Up @@ -151,8 +155,11 @@ export const CDRomModal = withHandlePromise((props: CDRomModalProps) => {
const submit = async (e) => {
e.preventDefault();
if (shouldPatch) {
const patch = await getCDsPatch(vmLikeEntity, Object.values(cds));
const promise = k8sPatch(getVMLikeModel(vmLikeEntity), vmLikeEntity, patch);
const promise = k8sPatch(
getVMLikeModel(vmLikeEntity),
vmLikeEntity,
getCDsPatch(vmLikeEntity, Object.values(cds), storageClassConfigMap),
);
handlePromise(promise).then(close); // eslint-disable-line promise/catch-or-return
} else {
close();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import { TemplateValidations } from '../../../utils/validations/template/templat
import { V1Disk } from '../../../types/vm/disk/V1Disk';
import { V1Volume } from '../../../types/vm/disk/V1Volume';
import { V1alpha1DataVolume } from '../../../types/vm/disk/V1alpha1DataVolume';
import { useStorageClassConfigMapWrapped } from '../../../hooks/storage-class-config-map';

const DiskModalFirehoseComponent: React.FC<DiskModalFirehoseComponentProps> = (props) => {
const { disk, volume, dataVolume, vmLikeEntity, vmLikeEntityLoading, ...restProps } = props;
Expand All @@ -40,7 +41,7 @@ const DiskModalFirehoseComponent: React.FC<DiskModalFirehoseComponentProps> = (p
k8sPatch(
getVMLikeModel(vmLikeEntity),
vmLikeEntity,
await getUpdateDiskPatches(vmLikeEntity, {
getUpdateDiskPatches(vmLikeEntity, {
disk: new DiskWrapper(diskWrapper, true).mergeWith(resultDisk).asResource(),
volume: new VolumeWrapper(volumeWrapper, true).mergeWith(resultVolume).asResource(),
dataVolume:
Expand All @@ -52,9 +53,12 @@ const DiskModalFirehoseComponent: React.FC<DiskModalFirehoseComponentProps> = (p
}),
);

const storageClassConfigMap = useStorageClassConfigMapWrapped();

return (
<DiskModal
{...restProps}
storageClassConfigMap={storageClassConfigMap}
usedDiskNames={combinedDiskFactory.getUsedDiskNames(diskWrapper.getName())}
usedPVCNames={combinedDiskFactory.getUsedDataVolumeNames(dataVolumeWrapper.getName())}
vmName={getName(vm)}
Expand Down

0 comments on commit 04a8d37

Please sign in to comment.