cmd/jujud: bootstrap-state accepts --constraints

cmd/jujud: bootstrap-state accepts --constraints

...and sets them on the environment. This feature currently has no clients.

https://code.launchpad.net/~fwereade/juju-core/bootstrap-constraints-1/+merge/153523

(do not edit description out of merge proposal)

[fwereade, 2013-03-15 10:14:19]

Please take a look.

[dimitern, 2013-03-15 13:11:38]

LGTM with a trivial suggestion.

https://codereview.appspot.com/7811045/diff/1/cmd/jujud/bootstrap.go
File cmd/jujud/bootstrap.go (right):

https://codereview.appspot.com/7811045/diff/1/cmd/jujud/bootstrap.go#newcode77
cmd/jujud/bootstrap.go:77: if err != nil {
merge lines (if/err=) ?

https://codereview.appspot.com/7811045/diff/1/cmd/jujud/bootstrap_test.go
File cmd/jujud/bootstrap_test.go (right):

https://codereview.appspot.com/7811045/diff/1/cmd/jujud/bootstrap_test.go#new...
cmd/jujud/bootstrap_test.go:219: var testConfig = b64yaml{
nice!

[jameinel, 2013-03-18 17:52:41]

LGTM

https://codereview.appspot.com/7811045/diff/1/cmd/jujud/bootstrap.go
File cmd/jujud/bootstrap.go (right):

https://codereview.appspot.com/7811045/diff/1/cmd/jujud/bootstrap.go#newcode77
cmd/jujud/bootstrap.go:77: if err != nil {
On 2013/03/15 13:11:38, dimitern wrote:
> merge lines (if/err=) ?

That doesn't seem to be the pattern in this file, though I've certainly seen
that used elsewhere in the code.

https://codereview.appspot.com/7811045/diff/1/cmd/jujud/bootstrap_test.go
File cmd/jujud/bootstrap_test.go (right):

https://codereview.appspot.com/7811045/diff/1/cmd/jujud/bootstrap_test.go#new...
cmd/jujud/bootstrap_test.go:85: tcons := state.Constraints{Mem: uint64p(2048),
CpuCores: uint64p(2)}
I'm curious why this takes pointers instead of just uint64 items. But the helper
is certainly nice for this.

[jameinel, 2013-03-18 17:52:42]

LGTM

[fwereade, 2013-03-18 22:36:31]

*** Submitted:

cmd/jujud: bootstrap-state accepts --constraints

...and sets them on the environment. This feature currently has no clients.

R=dimitern, jameinel
CC=
https://codereview.appspot.com/7811045

https://codereview.appspot.com/7811045/diff/1/cmd/jujud/bootstrap.go
File cmd/jujud/bootstrap.go (right):

https://codereview.appspot.com/7811045/diff/1/cmd/jujud/bootstrap.go#newcode77
cmd/jujud/bootstrap.go:77: if err != nil {
On 2013/03/18 17:52:41, jameinel wrote:
> On 2013/03/15 13:11:38, dimitern wrote:
> > merge lines (if/err=) ?
> 
> That doesn't seem to be the pattern in this file, though I've certainly seen
> that used elsewhere in the code.

Heh, there doesn't seem to be much pattern anyway, may as well impose some.
Done.

https://codereview.appspot.com/7811045/diff/1/cmd/jujud/bootstrap_test.go
File cmd/jujud/bootstrap_test.go (right):

https://codereview.appspot.com/7811045/diff/1/cmd/jujud/bootstrap_test.go#new...
cmd/jujud/bootstrap_test.go:85: tcons := state.Constraints{Mem: uint64p(2048),
CpuCores: uint64p(2)}
On 2013/03/18 17:52:41, jameinel wrote:
> I'm curious why this takes pointers instead of just uint64 items. But the
helper
> is certainly nice for this.

For combination later: a zero value is an explicit "anything goes", while a nil
value abdicates choice and falls back (for that value) from service constraints
to environ constraints (and from there implicitly back to the provider
implementation, which is responsible for doing something sane in the face of
empty constraints).

This is related to one of the reasons that I pass around a Constraints not a
*Constraints: that a zero Constraints is a meaningful thing (that explicitly
falls back in every case), while a *Constraints adds another layer of
zeroability that can IMO only obscure clarity. It is also, indeed, convenient to
modify them in place and it is nice that always passing them by value enables
slightly cleaner code in a couple of places.

Fwiw, subsequent branches have led me to believe that I should be exclusively
specifying test constraints with strings, and add a MustParse func for that
purpose, but I know roger's moving constraints into its own package so I'd
rather not muddy that right now.