Merge pull request #15093 from openshift-cherrypick-robot/cherry-pick-15001-to-release-4.18

[release-4.18] OCPBUGS-56757: HorizontalPodAutoscalers via console require CPU resource limit even though HPAs use requests for scaling

Author: openshift-merge-bot[bot]

frontend/packages/dev-console/locales/en/devconsole.json

@@ -342,13 +342,9 @@
   "Path": "Path",
   "argument": "argument",
   "The command to run inside the Container.": "The command to run inside the Container.",
-  "CPU and memory resource limits must be set if you want to use CPU and memory utilization. The HorizontalPodAutoscaler will not have CPU or memory metrics until resource limits are set.": "CPU and memory resource limits must be set if you want to use CPU and memory utilization. The HorizontalPodAutoscaler will not have CPU or memory metrics until resource limits are set.",
+  "CPU and memory resource requests must be set if you want to use CPU and memory utilization. The HorizontalPodAutoscaler will not have CPU or memory metrics until resource requests are set.": "CPU and memory resource requests must be set if you want to use CPU and memory utilization. The HorizontalPodAutoscaler will not have CPU or memory metrics until resource requests are set.",
   "CPU resource limits must be set if you want to use CPU utilization. The HorizontalPodAutoscaler will not have CPU metrics until resource limits are set.": "CPU resource limits must be set if you want to use CPU utilization. The HorizontalPodAutoscaler will not have CPU metrics until resource limits are set.",
   "Memory resource limits must be set if you want to use memory utilization. The HorizontalPodAutoscaler will not have memory metrics until resource limits are set.": "Memory resource limits must be set if you want to use memory utilization. The HorizontalPodAutoscaler will not have memory metrics until resource limits are set.",
-  "Cannot create a HorizontalPodAutoscaler with no valid metrics.": "Cannot create a HorizontalPodAutoscaler with no valid metrics.",
-  "CPU and memory utilization cannot be used currently.": "CPU and memory utilization cannot be used currently.",
-  "CPU utilization cannot be used currently.": "CPU utilization cannot be used currently.",
-  "Memory utilization cannot be used currently.": "Memory utilization cannot be used currently.",
   "Note: Some fields may not be represented in this form view. Please select \"YAML view\" for full control.": "Note: Some fields may not be represented in this form view. Please select \"YAML view\" for full control.",
   "Minimum Pods": "Minimum Pods",
   "Maximum Pods": "Maximum Pods",
@@ -368,7 +364,6 @@
   "Maximum Pods must be an integer.": "Maximum Pods must be an integer.",
   "Maximum Pods should be greater than or equal to Minimum Pods.": "Maximum Pods should be greater than or equal to Minimum Pods.",
   "Max Pods must be defined.": "Max Pods must be defined.",
-  "Average utilization must be a positive number.": "Average utilization must be a positive number.",
   "Health checks": "Health checks",
   "Resource limits": "Resource limits",
   "Labels": "Labels",

frontend/packages/dev-console/src/components/hpa/HPADetailsForm.tsx

@@ -15,7 +15,7 @@ const HPADetailsForm: React.FC = () => {
   const {
     setFieldValue,
     values: {
-      disabledFields: { name: nameDisabled, cpuUtilization, memoryUtilization },
+      disabledFields: { name: nameDisabled },
       showCanUseYAMLMessage,
     },
   } = useFormikContext<HPAFormValues>();
@@ -84,14 +84,12 @@ const HPADetailsForm: React.FC = () => {
               setOutputAsIntegerFlag
             />
             <HPAUtilizationField
-              disabled={cpuUtilization}
               hpa={field.value}
               label={t('devconsole~CPU')}
               onUpdate={updateField('cpu')}
               type="cpu"
             />
             <HPAUtilizationField
-              disabled={memoryUtilization}
               hpa={field.value}
               label={t('devconsole~Memory')}
               onUpdate={updateField('memory')}

frontend/packages/dev-console/src/components/hpa/HPAFormikForm.tsx

@@ -14,7 +14,6 @@ import { EditorType } from '@console/shared/src/components/synced-editor/editor-
 import { safeYAMLToJS } from '@console/shared/src/utils/yaml';
 import {
   getFormData,
-  getInvalidUsageError,
   getYAMLData,
   hasCustomMetrics,
   isCpuUtilizationPossible,
@@ -50,12 +49,6 @@ const HPAFormikForm: React.FC<HPAFormikFormProps> = ({ existingHPA, targetResour
       values.editorType === EditorType.YAML ? safeYAMLToJS(values.yamlData) : values.formData,
     );
 
-    const invalidUsageError = getInvalidUsageError(hpa, values);
-    if (invalidUsageError) {
-      helpers.setStatus({ submitError: invalidUsageError });
-      return Promise.resolve();
-    }
-
     const method: (
       kind: K8sKind,
       data: HorizontalPodAutoscalerKind,

frontend/packages/dev-console/src/components/hpa/__tests__/hpa-utils.spec.ts

@@ -2,7 +2,6 @@ import { cloneDeep } from 'lodash';
 import { DeploymentKind, HorizontalPodAutoscalerKind } from '@console/internal/module/k8s';
 import {
   getFormData,
-  getInvalidUsageError,
   getLimitWarning,
   getMetricByType,
   getYAMLData,
@@ -12,7 +11,6 @@ import {
   sanitizeHPAToForm,
   sanityForSubmit,
 } from '../hpa-utils';
-import { HPAFormValues } from '../types';
 import { deploymentConfigExamples, deploymentExamples, hpaExamples } from './hpa-utils-data';
 
 describe('isCpuUtilizationPossible provides accurate checks', () => {
@@ -144,30 +142,28 @@ describe('getYAMLData gets back an hpa structured editor string', () => {
 
 describe('getMetricByType returns an appropriate metric and the index it is at', () => {
   it('expect no metrics to return a default metric as a new metric on the end', () => {
-    const { metric, index } = getMetricByType(hpaExamples.noMetrics, 'memory');
+    const { metric } = getMetricByType(hpaExamples.noMetrics, 'memory');
     expect(metric).toBeTruthy();
     expect(metric.resource.name).toBe('memory');
-    expect(metric.resource.target.averageUtilization).toBe(0);
-    expect(index).toBe(0);
+    expect(metric.resource.target.averageUtilization).toBe(50);
   });
 
   it('expect to get back a default memory metric when only cpu metric is available', () => {
     const { metric, index } = getMetricByType(hpaExamples.cpuScaled, 'memory');
     expect(metric).toBeTruthy();
     expect(metric.resource.name).toBe('memory');
-    expect(metric.resource.target.averageUtilization).toBe(0);
+    expect(metric.resource.target.averageUtilization).toBe(50);
     expect(index).toBe(1);
   });
 
   it('expect to get back the cpu metric when it is available', () => {
     const hpaResource: HorizontalPodAutoscalerKind = hpaExamples.cpuScaled;
-    const { metric, index } = getMetricByType(hpaResource, 'cpu');
+    const { metric } = getMetricByType(hpaResource, 'cpu');
     expect(metric).toBeTruthy();
     expect(metric.resource.name).toBe('cpu');
     expect(metric.resource.target.averageUtilization).toBe(
       hpaResource.spec.metrics[0].resource.target.averageUtilization,
     );
-    expect(index).toBe(0);
   });
 });
 
@@ -234,8 +230,8 @@ describe('sanityForSubmit covers some basic field locking and trimming', () => {
 
   it('expect not to have empty cpu or memory metrics', () => {
     const overriddenResource: HorizontalPodAutoscalerKind = cloneDeep(hpaResource);
-    overriddenResource.spec.metrics[0].resource.target.averageUtilization = 0;
-    expect(sanityForSubmit(deploymentResource, overriddenResource).spec.metrics.length).toBe(0);
+    overriddenResource.spec.metrics[0].resource.target.averageUtilization = 50;
+    expect(sanityForSubmit(deploymentResource, overriddenResource).spec.metrics.length).toBe(1);
   });
 
   it('expect not to trim custom resource metrics', () => {
@@ -246,40 +242,6 @@ describe('sanityForSubmit covers some basic field locking and trimming', () => {
   });
 });
 
-describe('getInvalidUsageError returns an error string when limits are not set', () => {
-  const formValues: HPAFormValues = {
-    showCanUseYAMLMessage: false,
-    disabledFields: {
-      name: false,
-      cpuUtilization: true,
-      memoryUtilization: true,
-    },
-    editorType: null,
-    formData: null,
-    yamlData: null,
-  };
-  const hpaResource: HorizontalPodAutoscalerKind = hpaExamples.cpuScaled;
-
-  it('expect no metrics to be an error', () => {
-    const noMetricsHPA: HorizontalPodAutoscalerKind = hpaExamples.noMetrics;
-    expect(typeof getInvalidUsageError(noMetricsHPA, formValues)).toBe('string');
-
-    const emptyMetricsHPA: HorizontalPodAutoscalerKind = cloneDeep(noMetricsHPA);
-    emptyMetricsHPA.spec.metrics = [];
-    expect(typeof getInvalidUsageError(emptyMetricsHPA, formValues)).toBe('string');
-  });
-
-  it('expect cpu metric not to be allowed while disabled', () => {
-    expect(typeof getInvalidUsageError(hpaResource, formValues)).toBe('string');
-  });
-
-  it('expect memory metric to not be allowed while disabled', () => {
-    const memoryHPA = cloneDeep(hpaResource);
-    memoryHPA.spec.metrics[0].resource.name = 'memory';
-    expect(typeof getInvalidUsageError(memoryHPA, formValues)).toBe('string');
-  });
-});
-
 describe('hasCustomMetrics accurately determines if an hpa contains non-default metrics', () => {
   it('expect no metrics to mean no custom metrics', () => {
     expect(hasCustomMetrics(null)).toBe(false);

frontend/packages/dev-console/src/components/hpa/hpa-utils.ts

@@ -10,7 +10,7 @@ import {
 } from '@console/internal/module/k8s';
 import { LimitsData } from '@console/shared/src';
 import { safeJSToYAML, safeYAMLToJS } from '@console/shared/src/utils/yaml';
-import { HPAFormValues, SupportedMetricTypes } from './types';
+import { SupportedMetricTypes } from './types';
 
 export const VALID_HPA_TARGET_KINDS = ['Deployment', 'DeploymentConfig'];
 
@@ -31,7 +31,7 @@ export const getLimitWarning = (resource: K8sResourceKind): string => {
 
   if (!limits) {
     return i18next.t(
-      'devconsole~CPU and memory resource limits must be set if you want to use CPU and memory utilization. The HorizontalPodAutoscaler will not have CPU or memory metrics until resource limits are set.',
+      'devconsole~CPU and memory resource requests must be set if you want to use CPU and memory utilization. The HorizontalPodAutoscaler will not have CPU or memory metrics until resource requests are set.',
     );
   }
 
@@ -58,7 +58,7 @@ const getDefaultMetric = (type: SupportedMetricTypes): HPAMetric => ({
   resource: {
     name: type,
     target: {
-      averageUtilization: 0,
+      averageUtilization: 50,
       type: 'Utilization',
     },
   },
@@ -128,45 +128,9 @@ export const sanityForSubmit = (
     { spec: { scaleTargetRef: createScaleTargetRef(targetResource) } },
   );
 
-  // Remove empty metrics
-  validHPA.spec.metrics = validHPA.spec.metrics?.filter(
-    (metric: HPAMetric) =>
-      !['cpu', 'memory'].includes(metric?.resource?.name?.toLowerCase()) ||
-      (metric.resource.target.type === 'Utilization' &&
-        metric.resource.target.averageUtilization > 0),
-  );
-
   return validHPA;
 };
 
-export const getInvalidUsageError = (
-  hpa: HorizontalPodAutoscalerKind,
-  formValues: HPAFormValues,
-): string => {
-  const lackCPULimits = formValues.disabledFields.cpuUtilization;
-  const lackMemoryLimits = formValues.disabledFields.memoryUtilization;
-  const metricNames = (hpa.spec.metrics || []).map((metric) =>
-    metric.resource?.name?.toLowerCase(),
-  );
-  const invalidCPU = lackCPULimits && metricNames.includes('cpu');
-  const invalidMemory = lackMemoryLimits && metricNames.includes('memory');
-
-  if (metricNames.length === 0) {
-    return i18next.t('devconsole~Cannot create a HorizontalPodAutoscaler with no valid metrics.');
-  }
-  if (invalidCPU && invalidMemory) {
-    return i18next.t('devconsole~CPU and memory utilization cannot be used currently.');
-  }
-  if (invalidCPU) {
-    return i18next.t('devconsole~CPU utilization cannot be used currently.');
-  }
-  if (invalidMemory) {
-    return i18next.t('devconsole~Memory utilization cannot be used currently.');
-  }
-
-  return null;
-};
-
 export const hasCustomMetrics = (hpa?: HorizontalPodAutoscalerKind): boolean => {
   const metrics = hpa?.spec?.metrics;
   if (!metrics) {

frontend/packages/dev-console/src/components/hpa/validation-utils.ts

@@ -51,24 +51,6 @@ export const hpaValidationSchema = (t: TFunction) =>
               },
             )
             .required(t('devconsole~Max Pods must be defined.')),
-          metrics: yup.array(
-            yup.object({
-              resource: yup.object({
-                target: yup.object({
-                  averageUtilization: yup
-                    .mixed()
-                    .test(
-                      'test-for-valid-utilization',
-                      t('devconsole~Average utilization must be a positive number.'),
-                      function (avgUtilization) {
-                        if (!avgUtilization) return true;
-                        return /^\d+$/.test(String(avgUtilization));
-                      },
-                    ),
-                }),
-              }),
-            }),
-          ),
         }),
       }),
     }),