deis / workflow-cli

The CLI for Deis Workflow
http://deis.com
MIT License
31 stars 43 forks source link

`deis limits:unset web=64M` doesn't unset the limit #62

Open slack opened 8 years ago

slack commented 8 years ago
k:mustache example-go [master]$ deis limits:unset web=64M
Applying limits... done

=== indoor-whitecap Limits

--- Memory
web     64M

--- CPU
web     250m
k:mustache example-go [master]$ deis limits
=== indoor-whitecap Limits

--- Memory
web     64M

--- CPU
web     250m
kubectl --namespace=indoor-whitecap describe po indoor-whitecap-v15-web-amixa
Name:       indoor-whitecap-v15-web-amixa
Containers:
  indoor-whitecap-web:
    QoS Tier:
      cpu:  Guaranteed
      memory:   Guaranteed
    Limits:
      cpu:  250m
      memory:   64Mi
    Requests:
      memory:       64Mi
      cpu:      250m

Looks like deis limits:unset web does, bit of a surprise behavior.

slack commented 8 years ago

If the value is found, it should unset that value, if the value doesn't match it should throw a bad request. If no value is given it should clear the value, like it does now.

Joshua-Anderson commented 8 years ago

At the moment there is no validation on the process given, so the cli is trying to unset limits on the process web=64M, which doesn't exist.

Just to clarify, you would expect:

deis limits:unset web=64M

to return an error like Error: No limits defined for process 'web=64M' or do you expect it to unset the memory limit on process web

slack commented 8 years ago

I would expect deis limits:unset web=64M to clear the limit if it matched the currently set value of '64M', otherwise throw an error and not create a new release.

If deis limits:unset web=64M does match, then unset and create the release.

If deis limits:unset web is called it should unset the memory limit for web, regardless of its current value.

bacongobbler commented 8 years ago

given that we have a workaround, is this something we can get back to after 2.0?

vdice commented 8 years ago

ping @slack as to if this issue needs to be resolved in v2.0-rc1 -- if so, let's label with showstopper -- otherwise, as @bacongobbler mentioned, I might suggest we move to a later milestone...

Cryptophobia commented 6 years ago

This issue was moved to teamhephy/workflow-cli#30