Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 36 additions & 12 deletions frontend/packages/dev-console/src/components/hpa/HPADetailsForm.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 to return null when HPA has no metrics defined', () => {
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 no 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(50);
expect(metric).toBeNull();
expect(index).toBe(1);
});

Expand Down
40 changes: 13 additions & 27 deletions frontend/packages/dev-console/src/components/hpa/hpa-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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,
Expand All @@ -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 };
};
Expand All @@ -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 => {
Expand Down
2 changes: 1 addition & 1 deletion frontend/public/models/yaml-templates.ts
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,7 @@ spec:
resource:
name: cpu
target:
averageUtilization: 50
averageUtilization: 80
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unsure why the default value for the UI was different from the API default. Changed it to be consistent with the API default.

type: Utilization
`,
)
Expand Down