Here we need to determine whether it can be created in V1, because there are different between v1 and v2.

Once you apply my other suggestion to only add --cpu-percent this check will be simpler, and will only check that one flag.

done

I see that in comment kubernetes/kubectl#1481 (comment) the OP mentioned all the possible values, but I'm definitely against such an approach. I haven't seen any particular preference between the percent and quantity so I'm leaning towards the --mem-percent to match the pre-existing --cpu-percent flag. So please drop all else and we'll only support that one addition. If users are interested in tweaking the generated HPA they are free to do it outside this command.

sounds reasonable to me. done

Apologies for not thinking more about this, but I was considering a better approach for the future.
The original implementation utilizing --cpu-percent was based entirely on autoscaling/v1 types.
But now that most of our users have the ability to use autoscaling/v2 where we can present both types I'm inclined to deprecate the current --cpu-percent flag, and replace it with just --cpu where it would accept either a percent value (10%, or just a number 10 which would mean it's a percentage), or users could specify a quantity (in case of cpu that'd be 5m). Now, in a similar fashion we'd add --memory flag which would also accept a number (or with %), or a quantity. For parsing the first number you can use intstr.IntOrString type, and if that fails, it means you were given a quantity, which also have parsing utilities.

Sorry once again for the changes, but I believe that will better match all the possible use cases. What do you think?

OK. This is more flexible on the flag, but we need to do more job of verifying the input params. I will adjust it and ping you again when I am done.

After thinking about the design, maybe we can do this: we provide --cpu, --memory flags. Users can specify ":" to specify the metric type and support multiple units. For cases where units are not specified, the default configuration is used. 🤔 @soltysh PTAL

CPU Units:

• m,milli: milliCPU, e.g., 500m means 0.5 cores.
• core: core count, e.g., 1core or 1 means 1 core.
• Default to m if no unit is provided.

Memory Units:

• "Mi", "Mib", "MiB": mebibytes, e.g., 512Mi means 512 megabytes.
• "Gi", "Gib", "GiB": gibibytes, e.g., 1Gi means 1 gigabyte.
• Default to Mi if no unit is provided.

Percentage Format: If the value ends with %, it defaults to Utilization.
Quantity Format: If the value is a number or has a unit:
For --cpu, default to m if no unit is provided; parse directly if m is specified.
For --memory, default to Mi if no unit is provided; parse directly if Mi or Gi is specified.
Explicit Metric Type: Users can specify :utilization, :value, or :averagevalue explicitly.

Possible Cases

For --cpu

1. Percentage with Default Type (Utilization):

• Example: --cpu=80%
• Description: CPU average utilization of 80%.

2. Quantity with Default Type (Value):

• Example: --cpu=500 (defaults to 500m)
• Description: 500 milliCPU.

3. Percentage with Explicit Utilization:

• Example: --cpu=80%:utilization
• Description: Explicitly specifies CPU average utilization of 80%.

4. Quantity with Explicit Value:

• Example: --cpu=500m:value
• Description: Explicitly specifies 500 milliCPU.
• Example: --cpu=1core:value
• Description: Explicitly specifies 1 CPU core.

5. Quantity with Explicit AverageValue:

• Example: --cpu=500m:averagevalue
• Description: Explicitly specifies an average of 500 milliCPU.

For --memory

1. Percentage with Default Type (Utilization):

• Example: --memory=80%
• Description: Memory average utilization of 80%.

2. Quantity with Default Type (Value):

• Example: --memory=512 (defaults to 512Mi)
• Description: 512 mebibytes of memory.

3. Percentage with Explicit Utilization:

• Example: --memory=80%:utilization
• Description: Explicitly specifies memory average utilization of 80%.

4. Quantity with Explicit Value:

• Example: --memory=512Mi:value
• Description: Explicitly specifies 512 mebibytes of memory.
• Example: --memory=1Gi:value
• Description: Explicitly specifies 1 gibibyte of memory.

5. Quantity with Explicit AverageValue:

• Example: --memory=512Mi:averagevalue
• Description: Explicitly specifies an average of 512 mebibytes of memory.
• Example: --memory=1Gi:averagevalue
• Description: Explicitly specifies an average of 1 gibibyte of memory.

What you wrote above is close to what I've meant, although introducing : in the middle is not expected. I'm worried that it might overly complicate things, and I prefer something a bit simpler.

So:

1. For both cpu and memory, a percentage number would automatically mean average value in autoscale's metric type (.target.averageUtilization).
2. For both cpu and memory passing a unit-based value (ie. with milicores or MiB, etc) or just a number will automatically means the user wants to specify a quantity, so that's the average value (.target.averageValue) in autoscale's metric type. This would match what we have in quantity API.

For no. 2 you don't need to invent any logic for parsing the quantities, since we already have k8s.io/apimachinery/pkg/api/resource#ParseQuantity to handle that for you.

So looking at your examples:

1. Percentage with Default Type (Utilization):
* Example: --cpu=80%
* Description: CPU average utilization of 80%.

👍

2. Quantity with Default Type (Value):
* Example: --cpu=500 (defaults to 500m)
* Description: 500 milliCPU.

👍

3. Percentage with Explicit Utilization:
* Example: --cpu=80%:utilization
* Description: Explicitly specifies CPU average utilization of 80%.

--cpu=80%:utilization no need to specify it's utilization. Just --cpu=80% should be sufficient.

4. Quantity with Explicit Value:
* Example: --cpu=500m:value
* Description: Explicitly specifies 500 milliCPU.
* Example: --cpu=1core:value
* Description: Explicitly specifies 1 CPU core.

In both cases no need for :value, --cpu=500m is correct and ParseQuantity will handle that. --cpu=1core is not correct. That's not supported by our quantity API.

5. Quantity with Explicit AverageValue:
* Example: --cpu=500m:averagevalue
* Description: Explicitly specifies an average of 500 milliCPU.

Again, no need for :averagevalue, just --cpu=500m will be sufficient to express 500 milicores.

1. Percentage with Default Type (Utilization):
* Example: --memory=80%
* Description: Memory average utilization of 80%.

👍

2. Quantity with Default Type (Value):
* Example: --memory=512 (defaults to 512Mi)
* Description: 512 mebibytes of memory.

👍

3. Percentage with Explicit Utilization:
* Example: --memory=80%:utilization
* Description: Explicitly specifies memory average utilization of 80%.

No need for :utilization, just --memory=80%.

4. Quantity with Explicit Value:
* Example: --memory=512Mi:value
* Description: Explicitly specifies 512 mebibytes of memory.
* Example: --memory=1Gi:value
* Description: Explicitly specifies 1 gibibyte of memory.

No need for :value, just --memory=512Mi or --memory=1Gi is sufficient.

5. Quantity with Explicit AverageValue:
* Example: --memory=512Mi:averagevalue
* Description: Explicitly specifies an average of 512 mebibytes of memory.
* Example: --memory=1Gi:averagevalue
* Description: Explicitly specifies an average of 1 gibibyte of memory.

No need for :averagevalue, just --memory=512Mi or --memory=1Gi is sufficient.

thanks for feedback. I will handle these days 😄

Once you apply my other suggestion to only add --cpu-percent this check will be simpler, and will only check that one flag.

Yes --cpu-percent should be marked as deprecated, we can't remove without a proper deprecation period, which lasts a year.

This still holds, I don't see that flag marked deprecated. Also while doing so make sure to update the example

This logic here is not quite correct, what if I pass --memory=5Gi, the above code will not work correctly, and will fail in the next step when parsing int, because 5Gi is not the correct value. Why you want to manually parse this value, rather than just trust that ParseQuantity?

In general our API accepts both value with units and without them. You can see that pattern being used even in our docs. So I'd drop the part where you're adding units by hand entirely, and just parse the part with %, to handle the percentage utilization, and pass the other value as is straight into API.

Lastly, please add a unit tests covering all those cases to make sure we won't break anything.

This code correctly handles various input formats:

• percentages like 70%
• numbers like 500 (auto-converted to 500m or 500Mi)
• Full quantities with units like 500m, 2Gi, and — even 5Gi is handled properly by Kubernetes' quantity parser.

The reason we need case 2 is that we want to distinguish and fill in the correct default unit.

        // Case 1: Percentage
	if strings.HasSuffix(input, "%") {
		trimmed := strings.TrimSuffix(input, "%")
		value, err := strconv.ParseInt(trimmed, 10, 64)
		if err != nil || value < 0 {
			return fmt.Errorf("invalid percentage value: %s", trimmed)
		}
		return nil
	}

	// Case 2: Try parse as pure integer and apply default unit
	var unit string
	switch resourceType {
	case corev1.ResourceCPU:
		unit = "m"
	case corev1.ResourceMemory:
		unit = "Mi"
	default:
		return fmt.Errorf("unsupported resource type: %v", resourceType)
	}

	if _, err := strconv.ParseFloat(input, 64); err == nil {
		input += unit
	}

	// Case 3: Parse full quantity
	if _, err := apiresource.ParseQuantity(input); err != nil {
		return fmt.Errorf("invalid %s target: %w", strings.ToLower(string(resourceType)), err)
	}

Hmm... maybe I was wrong when testing last time, but I remember it didn't work the way you're describing, but rather I run into the described issue. Anyway, currently it's working as expected.

Nit:

.Kind is a string, so it's better to match the print type with that 😉

done

Don't change this line, the MarkDeprecated is sufficient because it will hide this flag from --help output and produce the necessary information for users still relying on that flag.

removed

What you currently have will produce:

Flag --cpu-percent has been deprecated, Flag deprecated. Use --cpu with percentage or resource quantity format (e.g., '70%' for utilization or '500m' for milliCPU).

The suggested change will give you:

Flag --cpu-percent has been deprecated, Use --cpu with percentage or resource quantity format (e.g., '70%' for utilization or '500m' for milliCPU).

So no need to repeat the information about deprecation 😉

got this. 😄 thanks

Hmm... maybe I was wrong when testing last time, but I remember it didn't work the way you're describing, but rather I run into the described issue. Anyway, currently it's working as expected.

Either don't introduce that error returned from createHorizontalPodAutoscalerV2 or make sure to act on it. Swallowing errors is bad.

Modified to handle this error.

done

and you can drop that variable, it's simple enough to read 😉

done

I'm confused, you have two functions doing very similar checks, iow. you're duplicating code here and inside validateResourceTarget. Can you combine the shared bits of code?