fix(api,ui): Fix autoscaling target parsing (caraml-dev#380)

* Allow setting of autoscaling targets on the ui without constraints on number of decimal places

* Add parsing of autoscaling targets to int for cpu and rps

* Update min and step size for autoscaling target field

* Update unit tests and error messages

Author: deadlycoconuts

api/turing/cluster/knative_service.go

@@ -196,7 +196,16 @@ func (cfg *KnativeService) buildSvcSpec(
 }
 
 func (cfg *KnativeService) getAutoscalingTarget() (string, error) {
-	if cfg.AutoscalingMetric == "memory" {
+	switch cfg.AutoscalingMetric {
+	case "cpu", "rps":
+		// Parse the autoscaling target to an integer
+		rawTarget, err := strconv.ParseFloat(cfg.AutoscalingTarget, 64)
+		if err != nil {
+			return "", err
+		}
+		targetValue := fmt.Sprintf("%.0f", rawTarget)
+		return targetValue, nil
+	case "memory":
 		// The value is supplied as a % of requested memory but the Knative API expects the value in Mi.
 		targetPercent, err := strconv.ParseFloat(cfg.AutoscalingTarget, 64)
 		if err != nil {
@@ -205,15 +214,16 @@ func (cfg *KnativeService) getAutoscalingTarget() (string, error) {
 		targetResource := ComputeResource(cfg.BaseService.MemoryRequests, targetPercent/100)
 		// Divide value by (1024^2) to convert to Mi
 		return fmt.Sprintf("%.0f", float64(targetResource.Value())/math.Pow(1024, 2)), nil
-
-	} else if cfg.AutoscalingMetric == "concurrency" {
+	case "concurrency":
+		// Parse the autoscaling target to a value up to 2 decimal places because Knative allows it
 		rawTarget, err := strconv.ParseFloat(cfg.AutoscalingTarget, 64)
 		if err != nil {
 			return "", err
 		}
 		targetValue := fmt.Sprintf("%.2f", rawTarget)
 		if targetValue == "0.00" {
-			return "", fmt.Errorf("concurrency target %v should be at least 0.01", cfg.AutoscalingTarget)
+			return "", fmt.Errorf("concurrency target %v should be at least 0.01 after rounding to 2 decimal places",
+				cfg.AutoscalingTarget)
 		}
 		return targetValue, nil
 	}

api/turing/cluster/knative_service_test.go

@@ -512,24 +512,45 @@ func TestGetAutoscalingTarget(t *testing.T) {
 		expectedTarget string
 		expectedErr    string
 	}{
+		"concurrency | target is not a number": {
+			cfg: &KnativeService{
+				AutoscalingMetric: "concurrency",
+				AutoscalingTarget: "10.1.1",
+			},
+			expectedErr: "strconv.ParseFloat: parsing \"10.1.1\": invalid syntax",
+		},
+		"concurrency | target when rounded to 2 decimal places is equals to 0.00": {
+			cfg: &KnativeService{
+				AutoscalingMetric: "concurrency",
+				AutoscalingTarget: "0.004",
+			},
+			expectedErr: "concurrency target 0.004 should be at least 0.01 after rounding to 2 decimal places",
+		},
 		"concurrency": {
 			cfg: &KnativeService{
 				AutoscalingMetric: "concurrency",
 				AutoscalingTarget: "10",
 			},
 			expectedTarget: "10.00",
 		},
+		"rps | target is not a number": {
+			cfg: &KnativeService{
+				AutoscalingMetric: "rps",
+				AutoscalingTarget: "100.1.1",
+			},
+			expectedErr: "strconv.ParseFloat: parsing \"100.1.1\": invalid syntax",
+		},
 		"rps": {
 			cfg: &KnativeService{
 				AutoscalingMetric: "rps",
-				AutoscalingTarget: "100",
+				AutoscalingTarget: "100.1",
 			},
 			expectedTarget: "100",
 		},
 		"cpu": {
 			cfg: &KnativeService{
 				AutoscalingMetric: "cpu",
-				AutoscalingTarget: "80",
+				AutoscalingTarget: "80.2",
 			},
 			expectedTarget: "80",
 		},
@@ -563,7 +584,7 @@ func TestGetAutoscalingTarget(t *testing.T) {
 			},
 			expectedTarget: "1335", // (70/100) * ((2G * 10^9) bytes / 1024^2)Mi
 		},
-		"memory | failure": {
+		"memory | target is not a number": {
 			cfg: &KnativeService{
 				BaseService: &BaseService{
 					MemoryRequests: resource.MustParse("1Gi"),

ui/src/router/components/form/components/autoscaling_policy/AutoscalingPolicyPanel.js

@@ -24,10 +24,10 @@ export const AutoscalingPolicyPanel = ({
   errors = {},
 }) => {
   const { onChange } = useOnChangeHandler(onChangeHandler);
-  // Parse the integer portion of the value
+  // Parse the float portion of the value
   const onChangeTarget = (value) => {
-    const parsedInt = parseInt(value);
-    onChange("target")(isNaN(parsedInt) ? "" : parsedInt.toString());
+    const parsedFloat = parseFloat(value);
+    onChange("target")(isNaN(value) ? "" : parsedFloat.toString());
   };
   // Update default target if the metric is changing
   const onChangeMetric = (value) => {
@@ -93,8 +93,10 @@ export const AutoscalingPolicyPanel = ({
               onChange={(e) => onChangeTarget(e.target.value)}
               isInvalid={!!errors.target}
               name="memory"
-              min={1}
-              step={1}
+              // The min value is set as 0.005 because it's the smallest value, when rounded to 2 decimal places, gives
+              // 0.01, the smallest value accepted as an autoscaling target (concurrency).
+              min={0.005}
+              step={"any"}
               append={selectedMetric.unit}
             />
           </EuiFormRow>

ui/src/router/components/form/validation/schema.js

@@ -233,7 +233,7 @@ const autoscalingPolicySchema = yup.object().shape({
   target: yup
     .string()
     .required("Valid target should be specified")
-    .matches(/^[0-9]+$/, "Must be a number"),
+    .matches(/(\d+(?:\.\d+)?)/, "Must be a number"),
 });
 
 const enricherSchema = yup.object().shape({

ui/src/services/version/RouterVersion.js

@@ -95,7 +95,7 @@ export class RouterVersion {
         resource_request: this.resource_request,
         autoscaling_policy: {
           ...this.autoscaling_policy,
-          target: parseInt(this.autoscaling_policy.target),
+          target: parseFloat(this.autoscaling_policy.target),
         },
         protocol: this.protocol,
       },
@@ -112,7 +112,7 @@ export class RouterVersion {
             resource_request: this.enricher.resource_request,
             autoscaling_policy: {
               ...this.enricher.autoscaling_policy,
-              target: parseInt(this.enricher.autoscaling_policy.target),
+              target: parseFloat(this.enricher.autoscaling_policy.target),
             },
           }
           : {
@@ -136,7 +136,7 @@ export class RouterVersion {
                     ...this.ensembler.docker_config,
                     autoscaling_policy: {
                       ...this.ensembler.docker_config.autoscaling_policy,
-                      target: parseInt(
+                      target: parseFloat(
                         this.ensembler.docker_config.autoscaling_policy.target
                       ),
                     },
@@ -148,7 +148,7 @@ export class RouterVersion {
                       ...this.ensembler.pyfunc_config,
                       autoscaling_policy: {
                         ...this.ensembler.pyfunc_config.autoscaling_policy,
-                        target: parseInt(
+                        target: parseFloat(
                           this.ensembler.pyfunc_config.autoscaling_policy.target
                         ),
                       },