search

teamhephy / workflow-cli

MIT License
2 stars 13 forks source link

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

Open Cryptophobia opened 6 years ago

Cryptophobia commented 6 years ago

From @slack on May 14, 2016 23:25

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.

Copied from original issue: deis/workflow-cli#62

Cryptophobia commented 6 years ago

From @slack on May 14, 2016 23:36

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.

Cryptophobia commented 6 years ago

From @Joshua-Anderson on May 14, 2016 23:49

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

Cryptophobia commented 6 years ago

From @slack on May 15, 2016 0:48

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.

Cryptophobia commented 6 years ago

From @bacongobbler on May 19, 2016 17:56

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

Cryptophobia commented 6 years ago

From @vdice on May 24, 2016 1:34

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...