Skip to content

Commit 35a092d

Browse files
committed
HPA Form View in RHOCP Web Console Incorrectly Requires Both CPU and Memory Metrics
1 parent 199697f commit 35a092d

File tree

4 files changed

+63
-37
lines changed

4 files changed

+63
-37
lines changed

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

Lines changed: 24 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -24,23 +24,35 @@ const HPADetailsForm: React.FC = () => {
2424
_event: React.FormEvent<HTMLInputElement>,
2525
value: string,
2626
) => {
27-
const numValue = parseInt(value, 10);
28-
2927
const hpa: HorizontalPodAutoscalerKind = field.value;
3028
const { metric, index } = getMetricByType(hpa, type);
3129
const hpaMetrics = hpa.spec.metrics || [];
3230

31+
// If field is empty, set utilization to undefined (will be filtered at submit)
32+
const numValue = value ? parseInt(value, 10) : undefined;
33+
3334
const updatedMetrics = [...hpaMetrics];
34-
updatedMetrics[index] = {
35-
...metric,
36-
resource: {
37-
...metric.resource,
38-
target: {
39-
...metric.resource.target,
40-
averageUtilization: numValue,
41-
},
42-
},
43-
};
35+
updatedMetrics[index] = metric
36+
? {
37+
...metric,
38+
resource: {
39+
...metric.resource,
40+
target: {
41+
...metric.resource.target,
42+
averageUtilization: numValue,
43+
},
44+
},
45+
}
46+
: {
47+
type: 'Resource',
48+
resource: {
49+
name: type,
50+
target: {
51+
type: 'Utilization',
52+
averageUtilization: numValue,
53+
},
54+
},
55+
};
4456

4557
setFieldValue(name, {
4658
...hpa,

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

Lines changed: 7 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -103,19 +103,16 @@ describe('getYAMLData gets back an hpa structured editor string', () => {
103103
});
104104

105105
describe('getMetricByType returns an appropriate metric and the index it is at', () => {
106-
it('expect no metrics to return a default metric as a new metric on the end', () => {
107-
const { metric } = getMetricByType(hpaExamples.noMetrics, 'memory');
108-
expect(metric).toBeTruthy();
109-
expect(metric.resource.name).toBe('memory');
110-
expect(metric.resource.target.averageUtilization).toBe(50);
106+
it('expect no metrics to return null when metric does not exist', () => {
107+
const { metric, index } = getMetricByType(hpaExamples.noMetrics, 'memory');
108+
expect(metric).toBeNull();
109+
expect(index).toBe(0); // index should be at the end of empty array
111110
});
112111

113-
it('expect to get back a default memory metric when only cpu metric is available', () => {
112+
it('expect to get back null when only cpu metric is available and requesting memory', () => {
114113
const { metric, index } = getMetricByType(hpaExamples.cpuScaled, 'memory');
115-
expect(metric).toBeTruthy();
116-
expect(metric.resource.name).toBe('memory');
117-
expect(metric.resource.target.averageUtilization).toBe(50);
118-
expect(index).toBe(1);
114+
expect(metric).toBeNull();
115+
expect(index).toBe(1); // index should be at the end of the metrics array
119116
});
120117

121118
it('expect to get back the cpu metric when it is available', () => {

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

Lines changed: 31 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ const getDefaultMetric = (type: SupportedMetricTypes): HPAMetric => ({
5555
resource: {
5656
name: type,
5757
target: {
58-
averageUtilization: 50,
58+
averageUtilization: 80,
5959
type: 'Utilization',
6060
},
6161
},
@@ -73,7 +73,7 @@ export const getFormData = (
7373
): HorizontalPodAutoscalerKind => {
7474
const hpa: HorizontalPodAutoscalerKind = existingHPA || safeYAMLToJS(defaultHPAYAML);
7575
if (!existingHPA) {
76-
// If we are working off the default, zero out cpu metrics to make it easier to use non-cpu metrics with the default
76+
// Initialize new HPAs with a default CPU value of 80%.
7777
hpa.spec.metrics = [getDefaultMetric('cpu')];
7878
}
7979

@@ -95,10 +95,11 @@ export const getYAMLData = (
9595
export const getMetricByType = (
9696
hpa: HorizontalPodAutoscalerKind,
9797
type: SupportedMetricTypes,
98-
): { metric: HPAMetric; index: number } => {
98+
): { metric: HPAMetric | null; index: number } => {
9999
const hpaMetrics = hpa.spec.metrics || [];
100100
const metricIndex = hpaMetrics.findIndex((m) => m.resource?.name?.toLowerCase() === type);
101-
const metric: HPAMetric = hpaMetrics[metricIndex] || getDefaultMetric(type);
101+
// Don't create a default metric if it doesn't exist - allow single-metric HPAs
102+
const metric: HPAMetric | null = metricIndex !== -1 ? hpaMetrics[metricIndex] : null;
102103

103104
return { metric, index: metricIndex === -1 ? hpaMetrics.length : metricIndex };
104105
};
@@ -116,16 +117,32 @@ export const sanityForSubmit = (
116117
targetResource: K8sResourceKind,
117118
hpa: HorizontalPodAutoscalerKind,
118119
): HorizontalPodAutoscalerKind => {
119-
const validHPA = merge(
120-
{},
121-
hpa,
122-
// Make sure it's against _this_ namespace
123-
{ metadata: { namespace: targetResource.metadata.namespace } },
124-
// Make sure we kept the target we started with
125-
{ spec: { scaleTargetRef: createScaleTargetRef(targetResource) } },
126-
);
127-
128-
return validHPA;
120+
// Filter out CPU/Memory metrics where utilization is undefined/null (field was empty)
121+
// Keep metrics with any set value (including 0, NaN) - let API validate them
122+
const filteredMetrics = hpa.spec?.metrics?.filter((metric) => {
123+
const metricName = metric?.resource?.name?.toLowerCase();
124+
const isCpuOrMemory = metricName === 'cpu' || metricName === 'memory';
125+
126+
if (!isCpuOrMemory) {
127+
return true; // Keep custom metrics
128+
}
129+
130+
const utilization = metric?.resource?.target?.averageUtilization;
131+
return utilization !== undefined && utilization !== null;
132+
});
133+
134+
return {
135+
...hpa,
136+
metadata: {
137+
...hpa.metadata,
138+
namespace: targetResource.metadata.namespace,
139+
},
140+
spec: {
141+
...hpa.spec,
142+
scaleTargetRef: createScaleTargetRef(targetResource),
143+
metrics: filteredMetrics,
144+
},
145+
};
129146
};
130147

131148
export const hasCustomMetrics = (hpa?: HorizontalPodAutoscalerKind): boolean => {

frontend/public/models/yaml-templates.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -218,7 +218,7 @@ spec:
218218
resource:
219219
name: cpu
220220
target:
221-
averageUtilization: 50
221+
averageUtilization: 80
222222
type: Utilization
223223
`,
224224
)

0 commit comments

Comments
 (0)