*** Submitted:

cmd/juju: bootstrap accepts --constraints

...and passes them on to environs.Bootstrap, which passes them on to
Environ.Bootstrap. Only the dummy provider actually handles constraints
at the moment -- so we can test the bootstrap command -- all the others
will be implemented in followups.

R=dimitern, jameinel, dfc, thumper
CC=
https://codereview.appspot.com/7610048



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

https://codereview.appspot.com/7610048/diff/1/cmd/juju/bootstrap.go#newcode29
cmd/juju/bootstrap.go:29: f.Var(state.ConstraintsValue{&c.Constraints},
"constraints", "set environment constraints")
On 2013/03/15 17:33:20, dimitern wrote:
> This was included in the prereq CL with a slightly different wording -
won't it
> result in a merge conflict?

That was jujud, not juju

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

https://codereview.appspot.com/7610048/diff/1/cmd/juju/bootstrap_test.go#newcode63
cmd/juju/bootstrap_test.go:63: defer testing.MakeFakeHome(c, envConfig,
"brokenenv").Restore()
On 2013/03/15 17:33:20, dimitern wrote:
> this is iffy and kinda confusing - what's broken about it?

Dropped.

https://codereview.appspot.com/7610048/diff/1/environs/bootstrap.go
File environs/bootstrap.go (right):

https://codereview.appspot.com/7610048/diff/1/environs/bootstrap.go#newcode22
environs/bootstrap.go:22: // uploaded, as documented in
Environ.Bootstrap.
On 2013/03/15 17:33:20, dimitern wrote:
> please, update the comment to include the constraints arg.

Done.

https://codereview.appspot.com/7610048/diff/1/environs/bootstrap_test.go
File environs/bootstrap_test.go (right):

https://codereview.appspot.com/7610048/diff/1/environs/bootstrap_test.go#newcode39
environs/bootstrap_test.go:39: err := environs.Bootstrap(env,
state.Constraints{}, false, nil)
On 2013/03/18 22:54:54, dfc wrote:
> On 2013/03/18 18:04:58, jameinel wrote:
> > +1, as it also means the struct doesn't get copied when it doesn't
need to.
> > Though I suppose we want to avoid the side-effect of passing in
constraints
> and
> > then changing the in-memory struct later?
> > But that could just be the final e.constraints = *cons (so it does
do an
> > explicit copy at that point).

> I've pondered on this as well, passing a struct is less common than a
pointer to
> a struct, but in this case I think it is correct, if a little
unorthodox.

> By creating a new empty (zero value) constraint set here we guarantee
that it
> can't be mutated afterwards by the caller. This is a useful property
for
> something as contentious as constraints.


Value semantics are useful; there's no reason to use nil when there's a
perfectly good zero value; and I don't believe the overhead of copying
these structs around a bit is going to hurt us.

https://codereview.appspot.com/7610048/diff/1/environs/interface.go
File environs/interface.go (right):

https://codereview.appspot.com/7610048/diff/1/environs/interface.go#newcode151
environs/interface.go:151: Bootstrap(cons state.Constraints, uploadTools
bool, stateServerCert, stateServerKey []byte) error
On 2013/03/15 17:33:20, dimitern wrote:
> please update the comment as well.

Done.

https://codereview.appspot.com/7610048/