Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rename vic-create --pool-xxx options to vch-xxx? #1829

Closed
stuclem opened this issue Aug 5, 2016 · 6 comments
Closed

Rename vic-create --pool-xxx options to vch-xxx? #1829

stuclem opened this issue Aug 5, 2016 · 6 comments

Comments

@stuclem
Copy link
Contributor

stuclem commented Aug 5, 2016

I initially thought that you had to use the pool-xxx options in conjuction with --use-rp. But, I just tested them with and without --use-rp and they worked in both cases (which shouldn't have surprised me because I know that a vApp is just a fancy resource pool).

Since the pool-xxx options can be applied to either the regular vApp or to the resource pool (if you use --use-rp), I think that the pool-xxx names are potentially misleading. And they will be even more misleading if the --use-rp option goes away, as @emlin has suggested to me that it might.

Because not all users realize that a vApp is a resource pool, I think that the following names might be more clear:

  • --vch-memory-reservation
  • --vch-memory-limit
  • --vch-memory-shares
  • --vch-cpu-reservation
  • --vch-cpu-limit
  • --vch-cpu-shares

I think that it might have been me who suggested --pool-xxx in the first place, but that was when I thought that these options only worked together with --use-rp. So, sorry for going around in circles.

@emlin
Copy link
Contributor

emlin commented Aug 5, 2016

👍 +1

@emlin
Copy link
Contributor

emlin commented Aug 5, 2016

@hickeng Did we get feedback on this vApp vs RP question? Can we remove the use-rp option at this time?

@hickeng
Copy link
Member

hickeng commented Aug 5, 2016

We didn't get much feedback, only the one person had an opinion and that was in favour of vApp.

I'm not a fan of the --pool- option name anyway. It really should be simply:

--memory=XXGB
--cpu=YYYYMHz

The only reason to have anything else is to differentiate from the appliance memory/cpu settings. However it's an appliance and being able to adjust it's mem/cpu is not something that should be needed or available under normal operation.
When we talk about how much memory a VCH has, we're talking about the resource pool limits/cluster. Likewise when we talk about CPU. In the docker info output we're reporting (or should be) the pool values, not the appliance.

@emlin Is it possible to introduce section breaks into the help output? So we can have a more readable breakdown of the command line help. That was one of the major confusions in the onsite.

Required
--target=
--compute-resource=
--image-store=
...

Optional
--memory=
--cpu=
...

Advanced
--use-rp=
--appliance-mem=
--debug=
...

@emlin
Copy link
Contributor

emlin commented Aug 9, 2016

@hickeng Still need more investigation for if options can be separated by section.

I'm looking at the flag attributes set by urfave, but looks like only hidden is one common, which is used to hide the advanced options.

But for Required and Optional options, still have no idea for how to separate them with existing attributes. Here is an issue opened long time ago in urfave community, urfave/cli#85, but not likely will be there at any time soon.

@mdubya66 mdubya66 removed this from the VIC GA Release milestone Sep 20, 2016
@mdubya66
Copy link
Contributor

Moving to Icebox per 9/20 Triage

@hickeng
Copy link
Member

hickeng commented Apr 27, 2017

This was addressed for 0.8

@hickeng hickeng closed this as completed Apr 27, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants