HPA Form View in RHOCP Web Console Incorrectly Requires Both CPU and Memory Metrics

Author: cajieh

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

@@ -24,23 +24,47 @@ const HPADetailsForm: React.FC = () => {
     _event: React.FormEvent<HTMLInputElement>,
     value: string,
   ) => {
-    const numValue = parseInt(value, 10);
-
     const hpa: HorizontalPodAutoscalerKind = field.value;
     const { metric, index } = getMetricByType(hpa, type);
     const hpaMetrics = hpa.spec.metrics || [];
 
-    const updatedMetrics = [...hpaMetrics];
-    updatedMetrics[index] = {
-      ...metric,
-      resource: {
-        ...metric.resource,
-        target: {
-          ...metric.resource.target,
-          averageUtilization: numValue,
+    // If field is undefined, remove metric
+    if (!value) {
+      const updatedMetrics = hpaMetrics.filter((_, i) => i !== index);
+      setFieldValue(name, {
+        ...hpa,
+        spec: {
+          ...hpa.spec,
+          metrics: updatedMetrics,
         },
-      },
-    };
+      });
+      return;
+    }
+
+    // Create or update metric
+    const numValue = parseInt(value, 10);
+    const updatedMetrics = [...hpaMetrics];
+    updatedMetrics[index] = metric
+      ? {
+          ...metric,
+          resource: {
+            ...metric.resource,
+            target: {
+              ...metric.resource.target,
+              averageUtilization: numValue,
+            },
+          },
+        }
+      : {
+          type: 'Resource',
+          resource: {
+            name: type,
+            target: {
+              type: 'Utilization',
+              averageUtilization: numValue,
+            },
+          },
+        };
 
     setFieldValue(name, {
       ...hpa,

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

@@ -103,18 +103,15 @@ 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 } = getMetricByType(hpaExamples.noMetrics, 'memory');
-    expect(metric).toBeTruthy();
-    expect(metric.resource.name).toBe('memory');
-    expect(metric.resource.target.averageUtilization).toBe(50);
+  it('expect no metrics to return null when metric does not exist', () => {
+    const { metric, index } = getMetricByType(hpaExamples.noMetrics, 'memory');
+    expect(metric).toBeNull();
+    expect(index).toBe(0);
   });
 
-  it('expect to get back a default memory metric when only cpu metric is available', () => {
+  it('expect to get back null when only cpu metric is available and requesting memory', () => {
     const { metric, index } = getMetricByType(hpaExamples.cpuScaled, 'memory');
-    expect(metric).toBeTruthy();
-    expect(metric.resource.name).toBe('memory');
-    expect(metric.resource.target.averageUtilization).toBe(50);
+    expect(metric).toBeNull();
     expect(index).toBe(1);
   });
 

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

@@ -50,17 +50,6 @@ const defaultHPAYAML = baseTemplates
   .get(referenceForModel(HorizontalPodAutoscalerModel))
   .get('default');
 
-const getDefaultMetric = (type: SupportedMetricTypes): HPAMetric => ({
-  type: 'Resource',
-  resource: {
-    name: type,
-    target: {
-      averageUtilization: 50,
-      type: 'Utilization',
-    },
-  },
-});
-
 const createScaleTargetRef = (resource: K8sResourceKind) => ({
   apiVersion: resource.apiVersion,
   kind: resource.kind,
@@ -72,10 +61,6 @@ export const getFormData = (
   existingHPA?: HorizontalPodAutoscalerKind,
 ): HorizontalPodAutoscalerKind => {
   const hpa: HorizontalPodAutoscalerKind = existingHPA || safeYAMLToJS(defaultHPAYAML);
-  if (!existingHPA) {
-    // If we are working off the default, zero out cpu metrics to make it easier to use non-cpu metrics with the default
-    hpa.spec.metrics = [getDefaultMetric('cpu')];
-  }
 
   return {
     ...hpa,
@@ -95,10 +80,10 @@ export const getYAMLData = (
 export const getMetricByType = (
   hpa: HorizontalPodAutoscalerKind,
   type: SupportedMetricTypes,
-): { metric: HPAMetric; index: number } => {
+): { metric: HPAMetric | null; index: number } => {
   const hpaMetrics = hpa.spec.metrics || [];
   const metricIndex = hpaMetrics.findIndex((m) => m.resource?.name?.toLowerCase() === type);
-  const metric: HPAMetric = hpaMetrics[metricIndex] || getDefaultMetric(type);
+  const metric: HPAMetric | null = hpaMetrics[metricIndex] || null;
 
   return { metric, index: metricIndex === -1 ? hpaMetrics.length : metricIndex };
 };
@@ -116,16 +101,17 @@ export const sanityForSubmit = (
   targetResource: K8sResourceKind,
   hpa: HorizontalPodAutoscalerKind,
 ): HorizontalPodAutoscalerKind => {
-  const validHPA = merge(
-    {},
-    hpa,
-    // Make sure it's against _this_ namespace
-    { metadata: { namespace: targetResource.metadata.namespace } },
-    // Make sure we kept the target we started with
-    { spec: { scaleTargetRef: createScaleTargetRef(targetResource) } },
-  );
-
-  return validHPA;
+  return {
+    ...hpa,
+    metadata: {
+      ...hpa.metadata,
+      namespace: targetResource.metadata.namespace,
+    },
+    spec: {
+      ...hpa.spec,
+      scaleTargetRef: createScaleTargetRef(targetResource),
+    },
+  };
 };
 
 export const hasCustomMetrics = (hpa?: HorizontalPodAutoscalerKind): boolean => {

frontend/public/models/yaml-templates.ts

@@ -218,7 +218,7 @@ spec:
     resource:
       name: cpu
       target:
-        averageUtilization: 50
+        averageUtilization: 80
         type: Utilization
 `,
   )