service create with cpu requests and limits

[csantanapr]

No description provided.

[csantanapr]

/milestone v0.1.0

[knative-prow-robot]

@csantanapr: You must be a member of the knative/knative-milestone-maintainers github team to set the milestone.

In response to this:

/milestone v0.1.0

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[maximilien]

What does this look like? I suggest:

# specifying value as millicore
$kn service create --cpu-requests 50m --cpu-limits 75m

or

# specifying value as fraction
$kn service create --cpu-requests 0.5 --cpu-limits .1

Working on this based on above assumption as specified here

[rhuss]

I would turn it around (as in --requests-cpu) as imo one should also add --requests-memory as well for the resource restrictions, so reflect the hierarchy like in resources spec itself.

BTW, any reason why memory was not included for this feature request ?

[navidshaikh]

I agree with @rhuss for --requests-cpu, --limits-cpu, --requests-memory and --limits-memory.

Regarding the unit of resources, I think we should support both as described here.

[maximilien]

OK makes sense @rhuss and @navidshaikh. Let me see if I can push something soon. Back at it this week so hopefully no more distractions. Best.

[evankanderson]

+1 on supporting units -- having the default units be different than 1 = 1 core seems a little confusing, so I might require units for CPU (or at least suggest that 75 is a bit wild).

…

On Thu, Apr 11, 2019 at 11:50 PM Navid Shaikh ***@***.***> wrote: I agree with @rhuss <https://github.com/rhuss> for --requests-cpu, --limits-cpu, --requests-memory and --limits-memory. Regarding the unit of resources, I think we should support both. The detailed info on the units can be found here <https://kubernetes.io/docs/concepts/configuration/manage-compute-resources-container/#resource-requests-and-limits-of-pod-and-container> . — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#50 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AHlyN5SiXvBT87YDfoiI23hzjJaZSwR6ks5vgCzEgaJpZM4cXG8T> .

-- Evan Anderson <argent@google.com>

[maximilien]

All, this is addressed in PR #81. I suggest we close unless something is missing. And if what is missing is "small change" to current implementation, just open a separate issue and I will address it.

[sixolet]

Done by #81