Skip to content

Commit

Permalink
Merge pull request #6999 from suomiy/fixwizarddata
Browse files Browse the repository at this point in the history
fix cnv base images loading
  • Loading branch information
openshift-merge-robot committed Oct 27, 2020
2 parents b3522fa + 0932100 commit a580a96
Show file tree
Hide file tree
Showing 6 changed files with 85 additions and 50 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import {
import { TemplateModel } from '@console/internal/models';
import { Firehose, history } from '@console/internal/components/utils';
import { usePrevious } from '@console/shared/src/hooks/previous';
import { PersistentVolumeClaimKind, referenceForModel } from '@console/internal/module/k8s';
import { referenceForModel } from '@console/internal/module/k8s';
import { NetworkAttachmentDefinitionModel } from '@console/network-attachment-definition-plugin';
import { Location } from 'history';
import { match as RouterMatch } from 'react-router';
Expand All @@ -38,6 +38,7 @@ import {
VMWizardProps,
VMWizardTab,
VMWizardTabsMetadata,
DirectCommonDataProps,
} from './types';
import { CREATE_VM, CREATE_VM_TEMPLATE, IMPORT_VM, TabTitleResolver } from './strings/strings';
import { vmWizardActions } from './redux/actions';
Expand Down Expand Up @@ -96,6 +97,7 @@ const CreateVMWizardComponent: React.FC<CreateVMWizardComponentProps> = (props)
// eslint-disable-next-line react-hooks/exhaustive-deps
[],
);

React.useEffect(() => {
props.onInitialize();

Expand All @@ -106,23 +108,11 @@ const CreateVMWizardComponent: React.FC<CreateVMWizardComponentProps> = (props)
// eslint-disable-next-line react-hooks/exhaustive-deps
}, []);

const commonTemplates = React.useMemo(
() => immutableListToShallowJS(iGetLoadedData(props.commonTemplates)),
[props.commonTemplates],
);
const [dataVolumePVCs, dataVolumePVCsLoaded, dataVolumePVCsLoadError] = usePVCBaseImages(
commonTemplates,
);

// Store previuse props
const prevProps = usePrevious<CreateVMWizardComponentProps>(props);
const prevDataVolumePVCData = usePrevious<[PersistentVolumeClaimKind[], boolean, string]>([
dataVolumePVCs,
dataVolumePVCsLoaded,
dataVolumePVCsLoadError,
]);

// componentDidUpdate
// takes care about firing onCommonDataChanged when fresh data is detected in props
React.useEffect(() => {
if (closed.current || !prevProps) {
return;
Expand All @@ -134,38 +124,25 @@ const CreateVMWizardComponent: React.FC<CreateVMWizardComponentProps> = (props)
}
return changedPropsAcc;
}, new Set()) as ChangedCommonData;
// lightweight equal - heavy computation should not occur here.
const referencesChanged = !_.isEqual(prevProps.dataIDReferences, props.dataIDReferences);
const dataVolumePVCsChanged = !_.isEqual(prevDataVolumePVCData, [
dataVolumePVCs,
dataVolumePVCsLoaded,
dataVolumePVCsLoadError,
]);

if (changedProps.size > 0 || referencesChanged || dataVolumePVCsChanged) {
if (changedProps.size > 0 || referencesChanged) {
let commonDataUpdate: CommonData = referencesChanged
? { dataIDReferences: props.dataIDReferences }
: undefined;
if (changedProps.has(VMWizardProps.storageClassConfigMap)) {
commonDataUpdate = {
...commonDataUpdate,
data: {
[VMWizardProps.storageClassConfigMap]: props[VMWizardProps.storageClassConfigMap],
},
};
}
if (dataVolumePVCsChanged) {
commonDataUpdate = {
...commonDataUpdate,
data: {
...commonDataUpdate?.data,
[VMWizardProps.openshiftCNVBaseImages]: {
data: dataVolumePVCs,
loaded: dataVolumePVCsLoaded,
loadError: dataVolumePVCsLoadError,

commonDataUpdate = [...DirectCommonDataProps]
.filter((propName) => changedProps.has(propName))
.reduce((updateAcc, propName) => {
return {
...updateAcc,
data: {
...updateAcc?.data,
[propName]: props[propName],
},
},
};
}
};
}, commonDataUpdate);
props.onCommonDataChanged(commonDataUpdate, changedProps);
}
});
Expand Down Expand Up @@ -332,7 +309,7 @@ const wizardStateToProps = (state, { reduxID }) => ({
iUserTemplate: iGetLoadedCommonData(state, reduxID, VMWizardProps.userTemplate),
// fetch data from store to detect and fire changes
...[...DetectCommonDataChanges]
.filter((v) => v !== VMWizardProps.storageClassConfigMap) // passed directly
.filter((v) => !DirectCommonDataProps.has(v)) // ignore props that are passed directly
.reduce((acc, propName) => {
acc[propName] = iGetCommonData(state, reduxID, propName);
return acc;
Expand All @@ -348,6 +325,7 @@ const wizardDispatchToProps = (dispatch, props) => ({
isProviderImport: props.isProviderImport,
isUserTemplateInitialized: false,
storageClassConfigMap: undefined,
openshiftCNVBaseImages: undefined,
isSimpleView: props.isSimpleView,
},
dataIDReferences: props.dataIDReferences,
Expand Down Expand Up @@ -394,6 +372,7 @@ export const CreateVMWizardPageComponent: React.FC<CreateVMWizardPageComponentPr
location,
flags,
wsResources,
commonTemplates,
hasCompleted,
}) => {
const activeNamespace = match && match.params && match.params.ns;
Expand Down Expand Up @@ -464,6 +443,25 @@ export const CreateVMWizardPageComponent: React.FC<CreateVMWizardPageComponentPr

const storageClassConfigMap = useStorageClassConfigMapWrapped();

const loadedCommonTemplates = React.useMemo(
() =>
userMode === VMWizardMode.IMPORT // make empty for import so no PVCBaseImages get loaded
? []
: immutableListToShallowJS(iGetLoadedData(commonTemplates)),
[commonTemplates, userMode],
);

const openshiftCNVBaseImagesListResult = usePVCBaseImages(loadedCommonTemplates);
// TODO integrate the list of watches into the redux store to prevent unnecessary copying of data
const openshiftCNVBaseImages = React.useMemo(
() => ({
data: openshiftCNVBaseImagesListResult[0],
loaded: openshiftCNVBaseImagesListResult[1],
loadError: openshiftCNVBaseImagesListResult[2],
}),
[openshiftCNVBaseImagesListResult],
);

const dataIDReferences = makeIDReferences(resources);

dataIDReferences[VMWizardProps.activeNamespace] = ['UI', 'activeNamespace'];
Expand All @@ -481,6 +479,7 @@ export const CreateVMWizardPageComponent: React.FC<CreateVMWizardPageComponentPr
isSimpleView={isSimpleView}
dataIDReferences={dataIDReferences}
storageClassConfigMap={storageClassConfigMap}
openshiftCNVBaseImages={openshiftCNVBaseImages}
reduxID={reduxID}
onClose={() => history.goBack()}
/>
Expand All @@ -492,6 +491,7 @@ type CreateVMWizardPageComponentProps = {
reduxID?: string;
location?: Location;
wsResources?: FirehoseResourceEnhanced[];
commonTemplates?: any;
hasCompleted: boolean;
match?: RouterMatch<{ ns: string; plural: string; appName?: string }>;
flags: FlagsObject;
Expand All @@ -504,6 +504,11 @@ export const CreateVMWizardPage = compose(
(state, props: any) => ({
hasCompleted: isStepValid(state, props.reduxID, VMWizardTab.RESULT),
wsResources: getExtraWSQueries(state, props.reduxID),
[VMWizardProps.commonTemplates]: iGetCommonData(
state,
props.reduxID,
VMWizardProps.commonTemplates,
),
}),
undefined,
undefined,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import { FormPFSelect } from '../../../form/form-pf-select';
export const Flavor: React.FC<FlavorProps> = React.memo(
({
iUserTemplate,
cnvBaseImages,
commonTemplates,
os,
flavorField,
Expand All @@ -50,14 +51,18 @@ export const Flavor: React.FC<FlavorProps> = React.memo(

const workloadProfiles = getWorkloadProfiles(templates, params);

const loadingResources = openshiftFlag
const loadingResources: any = openshiftFlag
? {
commonTemplates,
}
: {};

if (openshiftFlag && iUserTemplate) {
Object.assign(loadingResources, { iUserTemplate });
if (iUserTemplate) {
loadingResources.iUserTemplate = iUserTemplate;
}

if (cnvBaseImages && !iGetIsLoaded(cnvBaseImages) && !iGetLoadError(cnvBaseImages)) {
loadingResources.cnvBaseImages = cnvBaseImages;
}

let flavorValidation;
Expand Down Expand Up @@ -108,5 +113,6 @@ type FlavorProps = {
os: string;
workloadProfile: string;
openshiftFlag: boolean;
cnvBaseImages: any;
onChange: (key: string, value: string | boolean) => void;
};
Original file line number Diff line number Diff line change
Expand Up @@ -93,14 +93,18 @@ export const OS: React.FC<OSProps> = React.memo(

const workloadProfiles = getWorkloadProfiles(templates, params);

const loadingResources = openshiftFlag
const loadingResources: any = openshiftFlag
? {
commonTemplates,
}
: {};

if (openshiftFlag && iUserTemplate) {
Object.assign(loadingResources, { iUserTemplate });
if (iUserTemplate) {
loadingResources.iUserTemplate = iUserTemplate;
}

if (cnvBaseImages && !iGetIsLoaded(cnvBaseImages) && !iGetLoadError(cnvBaseImages)) {
loadingResources.cnvBaseImages = cnvBaseImages;
}

let operatingSystemValidation;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,7 @@ export const VMSettingsTabComponent: React.FC<VMSettingsTabComponentProps> = ({
flavorField={getField(VMSettingsField.FLAVOR)}
workloadProfile={getFieldValue(VMSettingsField.WORKLOAD_PROFILE)}
onChange={onFieldChange}
cnvBaseImages={cnvBaseImages}
openshiftFlag={openshiftFlag}
/>
<MemoryCPU
Expand All @@ -123,6 +124,7 @@ export const VMSettingsTabComponent: React.FC<VMSettingsTabComponentProps> = ({
workloadProfileField={getField(VMSettingsField.WORKLOAD_PROFILE)}
operatingSystem={getFieldValue(VMSettingsField.OPERATING_SYSTEM)}
flavor={getFieldValue(VMSettingsField.FLAVOR)}
cnvBaseImages={cnvBaseImages}
onChange={onFieldChange}
/>
</Form>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,15 @@ import { iGetFieldValue } from '../../selectors/immutable/field';
import { FormPFSelect } from '../../../form/form-pf-select';

export const WorkloadProfile: React.FC<WorkloadProps> = React.memo(
({ iUserTemplate, commonTemplates, workloadProfileField, operatingSystem, flavor, onChange }) => {
({
iUserTemplate,
cnvBaseImages,
commonTemplates,
workloadProfileField,
operatingSystem,
flavor,
onChange,
}) => {
const isUserTemplateValid = iGetIsLoaded(iUserTemplate) && !iGetLoadError(iUserTemplate);

const templates = iUserTemplate
Expand All @@ -33,12 +41,16 @@ export const WorkloadProfile: React.FC<WorkloadProps> = React.memo(
}),
);

const loadingResources = {
const loadingResources: any = {
commonTemplates,
};

if (iUserTemplate) {
Object.assign(loadingResources, { iUserTemplate });
loadingResources.iUserTemplate = iUserTemplate;
}

if (cnvBaseImages && !iGetIsLoaded(cnvBaseImages) && !iGetLoadError(cnvBaseImages)) {
loadingResources.cnvBaseImages = cnvBaseImages;
}

return (
Expand Down Expand Up @@ -71,5 +83,6 @@ type WorkloadProps = {
workloadProfileField: any;
flavor: string;
operatingSystem: string;
cnvBaseImages: any;
onChange: (key: string, value: string) => void;
};
Original file line number Diff line number Diff line change
Expand Up @@ -261,6 +261,11 @@ export const DetectCommonDataChanges = new Set<ChangedCommonDataProp>([
OvirtProviderProps.networkAttachmentDefinitions,
]);

export const DirectCommonDataProps = new Set<ChangedCommonDataProp>([
VMWizardProps.storageClassConfigMap,
VMWizardProps.openshiftCNVBaseImages,
]);

export type CommonData = {
data?: {
isSimpleView?: boolean;
Expand Down

0 comments on commit a580a96

Please sign in to comment.