Merge pull request #3879 from yaacov/kubevirt-memeory-cleanups

Nitpicks from #3839
openshift · Jan 9, 2020 · cab16cd · cab16cd
2 parents 2d66960 + e14d085
commit cab16cd
Show file tree

Hide file tree

Showing 4 changed files with 53 additions and 48 deletions.
diff --git a/...e-vm-wizard/redux/validations/common-template-validations/common-templates-validations.ts b/...e-vm-wizard/redux/validations/common-template-validations/common-templates-validations.ts
@@ -1,21 +1,20 @@
+import { TemplateKind } from '@console/internal/module/k8s';
 import { getRelevantTemplates } from '../../../../../selectors/vm-template/selectors';
-import { getFieldId } from '../../../utils/vm-settings-tab-utils';
 import { UpdateOptions } from '../../types';
 import {
   iGetVmSettingAttribute,
   iGetVmSettingValue,
 } from '../../../selectors/immutable/vm-settings';
 import { iGetLoadedCommonData, iGetName } from '../../../selectors/immutable/selectors';
 import { VMSettingsField, VMWizardProps, VMSettingsRenderableField } from '../../../types';
-import { iGetIn } from '../../../../../utils/immutable';
 import { CommonTemplatesValidation } from './validation-types';
-import { getValidationsFromTemplates, getFieldValidations } from './selectors';
+import { getValidationsFromTemplates } from './selectors';
 
 // TODO: Add all the fields in the form
 // For each field we need to check for validations
 const IDToJsonPath = {
-  [getFieldId(VMSettingsField.MEMORY)]: '.spec.domain.resources.requests.memory',
-  [getFieldId(VMSettingsField.CPU)]: '.spec.domain.cpu.cores',
+  [VMSettingsField.MEMORY]: 'jsonpath::.spec.domain.resources.requests.memory',
+  [VMSettingsField.CPU]: 'jsonpath::.spec.domain.cpu.cores',
 };
 
 export const getTemplateValidations = (
@@ -29,14 +28,6 @@ export const getTemplateValidations = (
   const { getState, id } = options;
   const state = getState();
 
-  // Get OS, workload-profile and flavor attributes
-  const os = iGetVmSettingAttribute(state, id, VMSettingsField.OPERATING_SYSTEM);
-  const flavor = iGetVmSettingAttribute(state, id, VMSettingsField.FLAVOR);
-  const workloadProfile = iGetVmSettingAttribute(state, id, VMSettingsField.WORKLOAD_PROFILE);
-
-  // Get all the templates from common-templates
-  const commonTemplates = iGetLoadedCommonData(state, id, VMWizardProps.commonTemplates);
-
   // Get userTemplate if it was chosen
   const userTemplateName = iGetVmSettingValue(state, id, VMSettingsField.USER_TEMPLATE);
   const iUserTemplates = iGetLoadedCommonData(state, id, VMWizardProps.userTemplates);
@@ -45,18 +36,23 @@ export const getTemplateValidations = (
       ? iUserTemplates.find((template) => iGetName(template) === userTemplateName)
       : null;
 
+  if (iUserTemplate) {
+    return getValidationsFromTemplates([iUserTemplate] as TemplateKind[], IDToJsonPath[fieldId]);
+  }
+
+  // Get OS, workload-profile and flavor attributes
+  const os = iGetVmSettingAttribute(state, id, VMSettingsField.OPERATING_SYSTEM);
+  const flavor = iGetVmSettingAttribute(state, id, VMSettingsField.FLAVOR);
+  const workloadProfile = iGetVmSettingAttribute(state, id, VMSettingsField.WORKLOAD_PROFILE);
+
+  // Get all the templates from common-templates
+  const commonTemplates = iGetLoadedCommonData(state, id, VMWizardProps.commonTemplates).toArray();
+
   // Get all the validations from the relevant templates:
   // Get the validations from the user template, if chosen.
   // If not, get the validations from Common-Templates based on OS, Workload-Profile and Flavor
-  const validations = iUserTemplate
-    ? JSON.parse(iGetIn(iUserTemplate, ['metadata', 'annotations', 'validations']))
-    : getValidationsFromTemplates(
-        // Get templates based on OS, Workload-Profile and Flavor
-        getRelevantTemplates(commonTemplates, os, workloadProfile, flavor),
-      );
-
-  // Return all the validations which are relevant for the field
-  return getFieldValidations(validations, IDToJsonPath[fieldId]);
+  const templates = getRelevantTemplates(commonTemplates, os, workloadProfile, flavor);
+  return getValidationsFromTemplates(templates, IDToJsonPath[fieldId]);
 };
 
 export const runValidation = (

diff --git a/...rc/components/create-vm-wizard/redux/validations/common-template-validations/selectors.ts b/...rc/components/create-vm-wizard/redux/validations/common-template-validations/selectors.ts
@@ -1,19 +1,23 @@
+import * as _ from 'lodash';
+import { TemplateKind } from '@console/internal/module/k8s';
 import { iGetIn } from '../../../../../utils/immutable';
 import { CommonTemplatesValidation } from './validation-types';
 
-export const getValidationsFromTemplates = (templates): CommonTemplatesValidation[] => {
-  return templates
-    .map((relevantTemplate) =>
-      JSON.parse(iGetIn(relevantTemplate, ['metadata', 'annotations', 'validations'])),
-    )
-    .valueSeq()
-    .toArray()
-    .flat();
-};
-
-export const getFieldValidations = (
-  validations: CommonTemplatesValidation[],
+export const getValidationsFromTemplates = (
+  templates: TemplateKind[],
   jsonPath: string,
 ): CommonTemplatesValidation[] => {
-  return validations.filter((validation) => validation.path.includes(jsonPath));
+  const templateValidations: CommonTemplatesValidation[][] = templates.map((relevantTemplate) =>
+    JSON.parse(iGetIn(relevantTemplate, ['metadata', 'annotations', 'validations'])).filter(
+      (validation: CommonTemplatesValidation) => validation.path.includes(jsonPath),
+    ),
+  );
+
+  // If we have a template with no restrictions, ignore all other validation rules, the most
+  // relax option take
+  if (templateValidations.find((validations) => validations.length === 0)) {
+    return [];
+  }
+
+  return _.flatten(templateValidations);
 };
diff --git a/...rt-plugin/src/components/create-vm-wizard/redux/validations/vm-settings-tab-validation.ts b/...rt-plugin/src/components/create-vm-wizard/redux/validations/vm-settings-tab-validation.ts
@@ -29,11 +29,7 @@ import {
   VIRTUAL_MACHINE_EXISTS,
   VIRTUAL_MACHINE_TEMPLATE_EXISTS,
 } from '../../../../utils/validations/strings';
-import {
-  getFieldReadableTitle,
-  getFieldTitle,
-  getFieldId,
-} from '../../utils/vm-settings-tab-utils';
+import { getFieldReadableTitle, getFieldTitle } from '../../utils/vm-settings-tab-utils';
 import { concatImmutableLists, iGet } from '../../../../utils/immutable';
 import {
   checkTabValidityChanged,
@@ -109,15 +105,28 @@ const memoryValidation: VmSettingsValidator = (field, options): ValidationObject
     return null;
   }
   const memValueBytes = memValueGB * 1024 ** 3;
-  const validations = getTemplateValidations(options, getFieldId(VMSettingsField.MEMORY));
+  const validations = getTemplateValidations(options, VMSettingsField.MEMORY);
   if (validations.length === 0) {
     return null;
   }
 
   const validationResult = runValidation(validations, memValueBytes);
 
   if (!validationResult.isValid) {
-    return asValidationObject(validationResult.errorMsg, ValidationErrorType.Error);
+    // Must have failed all validations, including first one:
+    const validation = validations[0];
+    let customMessage = validationResult.errorMsg;
+
+    if ('min' in validation && 'max' in validation) {
+      customMessage = `Memory must be between ${validation.min / 1024 ** 3}GB and ${validation.max /
+        1024 ** 3} GB`;
+    } else if ('min' in validation) {
+      customMessage = `Memory must be above ${validation.min / 1024 ** 3}GB`;
+    } else if ('max' in validation) {
+      customMessage = `Memory must be below ${validation.max / 1024 ** 3}GB`;
+    }
+
+    return asValidationObject(customMessage, ValidationErrorType.Error);
   }
 
   return null;

diff --git a/frontend/packages/kubevirt-plugin/src/selectors/vm-template/selectors.ts b/frontend/packages/kubevirt-plugin/src/selectors/vm-template/selectors.ts
@@ -140,11 +140,8 @@ export const getRelevantTemplates = (
   os: string,
   workloadProfile: string,
   flavor: string,
-) => {
-  if (!commonTemplates || commonTemplates.length === 0 || !os || !workloadProfile) {
-    return [];
-  }
-  return (commonTemplates || []).filter(
+) =>
+  (commonTemplates || []).filter(
     (template) =>
       iGetIn(template, ['metadata', 'labels', TEMPLATE_TYPE_LABEL]) === 'base' &&
       (!os || iGetIn(template, ['metadata', 'labels', `${TEMPLATE_OS_LABEL}/${os}`])) &&
@@ -157,4 +154,3 @@ export const getRelevantTemplates = (
       (flavor === 'Custom' ||
         iGetIn(template, ['metadata', 'labels', `${TEMPLATE_FLAVOR_LABEL}/${flavor}`])),
   );
-};