deis / workflow-cli

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

feat(limits-cmd): accept new limits:set value type #263

Closed zinuzoid closed 7 years ago

zinuzoid commented 7 years ago

Accept new cli format limits:set web=500m/2000m Value is map to requests/limits parameter in Kubernetes resource management. See https://github.com/deis/workflow/issues/562 e2e https://github.com/deis/workflow-e2e/pull/335 Requires https://github.com/deis/controller/pull/1118

deis-bot commented 7 years ago

@Joshua-Anderson, @ultimateboy and @bacongobbler are potential reviewers of this pull request based on my analysis of git blame information. Thanks @zinuzoid!

Joshua-Anderson commented 7 years ago

Overall looks good! Looks like limits_tests.go needs to be formatted with gofmt -s though. Thanks for the contribution!

codecov-io commented 7 years ago

Current coverage is 72.45% (diff: 100%)

Merging #263 into master will increase coverage by 0.02%

@@             master       #263   diff @@
==========================================
  Files            57         57          
  Lines          3925       3928     +3   
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
+ Hits           2843       2846     +3   
  Misses          773        773          
  Partials        309        309          

Powered by Codecov. Last update 50451f8...f83362c

zinuzoid commented 7 years ago

@Joshua-Anderson Thanks, I just notice now that make test didn't include test-style. Not sure if there's a reason for that. Anyway, this one is green now.

kmala commented 7 years ago

Tested manually and this looks good,

 deis limits:set web=10M/100M
Applying limits... done

=== syrupy-yeomanry Limits

--- Memory
web     10M/100M

--- CPU
Unlimited

deis limits:set web=100M
Applying limits... done

=== syrupy-yeomanry Limits

--- Memory
web     100M

--- CPU
Unlimited
kmala commented 7 years ago

It will be nice to add details of setting the value in the new way https://github.com/deis/workflow-cli/blob/master/parser/limits.go#L78-L89

kmala commented 7 years ago

Jenkins, Add to whitelist.