Merge lp:~fwereade/juju-core/bootstrap-constraints-2 into lp:~juju/juju-core/trunk

[William Reade]

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.

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

[William Reade (fwereade)]

Reviewers: mp+153600_code.launchpad.net,

Message:
Please take a look.

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

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

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

(do not edit description out of merge proposal)

Please review this at https://codereview.appspot.com/7610048/

Affected files:
   A [revision details]
   M cmd/builddb/main.go
   M cmd/juju/bootstrap.go
   M cmd/juju/bootstrap_test.go
   M environs/bootstrap.go
   M environs/bootstrap_test.go
   M environs/dummy/environs.go
   M environs/ec2/ec2.go
   M environs/ec2/local_test.go
   M environs/interface.go
   M environs/jujutest/livetests.go
   M environs/jujutest/tests.go
   M environs/open_test.go
   M environs/openstack/local_test.go
   M environs/openstack/provider.go
   M juju/apiconn_test.go
   M juju/conn_test.go
   M juju/testing/conn.go

[Dimiter Naydenov (dimitern)]

LGTM, with some suggestions/questions.

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")
This was included in the prereq CL with a slightly different wording -
won't it result in a merge conflict?

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()
this is iffy and kinda confusing - what's broken about it?

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.
please, update the comment to include the constraints arg.

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)
couldn't this be *state.Constraints instead, so we can pass nil?
(elsewhere as well)

https://codereview.appspot.com/7610048/diff/1/environs/bootstrap_test.go#newcode185
environs/bootstrap_test.go:185: func (e *bootstrapEnviron)
Bootstrap(cons state.Constraints, uploadTools bool, certPEM, keyPEM
[]byte) error {
see above

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
please update the comment as well.

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

[John A Meinel (jameinel)]

LGTM

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

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

[Tim Penhey (thumper)]

On 2013/03/18 22:54:54, dfc wrote:
> LGTM.

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

This is about to conflict wonderfully with the branch I'm about to
merge.

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

[William Reade (fwereade)]

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

=== modified file 'cmd/juju/bootstrap.go'
--- cmd/juju/bootstrap.go	2013-03-18 22:33:50 +0000
+++ cmd/juju/bootstrap.go	2013-03-19 10:29:25 +0000
@@ -5,6 +5,7 @@
 	"launchpad.net/gnuflag"
 	"launchpad.net/juju-core/cmd"
 	"launchpad.net/juju-core/environs"
+	"launchpad.net/juju-core/state"
 	"os"
 )
 
@@ -12,6 +13,7 @@
 // environment, and setting up everything necessary to continue working.
 type BootstrapCommand struct {
 	EnvCommandBase
+	Constraints state.Constraints
 	UploadTools bool
 }
 
@@ -24,6 +26,7 @@
 
 func (c *BootstrapCommand) SetFlags(f *gnuflag.FlagSet) {
 	c.EnvCommandBase.SetFlags(f)
+	f.Var(state.ConstraintsValue{&c.Constraints}, "constraints", "set environment constraints")
 	f.BoolVar(&c.UploadTools, "upload-tools", false, "upload local version of tools before bootstrapping")
 }
 
@@ -43,11 +46,10 @@
 		}
 		return err
 	}
-
 	// TODO: if in verbose mode, write out to Stdout if a new cert was created.
 	_, err = environs.EnsureCertificate(environ, environs.WriteCertAndKeyToHome)
 	if err != nil {
 		return err
 	}
-	return environs.Bootstrap(environ, c.UploadTools)
+	return environs.Bootstrap(environ, c.Constraints, c.UploadTools)
 }

=== modified file 'cmd/juju/bootstrap_test.go'
--- cmd/juju/bootstrap_test.go	2013-03-18 22:45:01 +0000
+++ cmd/juju/bootstrap_test.go	2013-03-19 10:29:25 +0000
@@ -7,6 +7,7 @@
 	"launchpad.net/juju-core/environs"
 	"launchpad.net/juju-core/environs/agent"
 	"launchpad.net/juju-core/environs/dummy"
+	"launchpad.net/juju-core/state"
 	"launchpad.net/juju-core/testing"
 	"launchpad.net/juju-core/version"
 	"net/http"
@@ -42,6 +43,15 @@
 	dummy.Reset()
 }
 
+func (*BootstrapSuite) TestBasic(c *C) {
+	defer testing.MakeFakeHome(c, envConfig).Restore()
+	opc, errc := runCommand(new(BootstrapCommand))
+	c.Check(<-errc, IsNil)
+	opBootstrap := (<-opc).(dummy.OpBootstrap)
+	c.Check(opBootstrap.Env, Equals, "peckham")
+	c.Check(opBootstrap.Constraints, DeepEquals, state.Constraints{})
+}
+
 func (*BootstrapSuite) TestRunGeneratesCertificate(c *C) {
 	defer testing.MakeFakeHome(c, envConfig).Restore()
 	envName := "peckham"
@@ -62,14 +72,19 @@
 	c.Assert(err, IsNil)
 }
 
-func (*BootstrapSuite) TestBootstrapCommandNoParams(c *C) {
-	defer testing.MakeFakeHome(c, envConfig).Restore()
-	opc, errc := runCommand(new(BootstrapCommand))
+func (*BootstrapSuite) TestConstraints(c *C) {
+	defer testing.MakeFakeHome(c, envConfig, "brokenenv").Restore()
+	scons := " cpu-cores=2   mem=4G"
+	cons, err := state.ParseConstraints(scons)
+	c.Assert(err, IsNil)
+	opc, errc := runCommand(new(BootstrapCommand), "--constraints", scons)
 	c.Check(<-errc, IsNil)
-	c.Check((<-opc).(dummy.OpBootstrap).Env, Equals, "peckham")
+	opBootstrap := (<-opc).(dummy.OpBootstrap)
+	c.Check(opBootstrap.Env, Equals, "peckham")
+	c.Check(opBootstrap.Constraints, DeepEquals, cons)
 }
 
-func (*BootstrapSuite) TestBootstrapCommandUploadTools(c *C) {
+func (*BootstrapSuite) TestUploadTools(c *C) {
 	defer testing.MakeFakeHome(c, envConfig).Restore()
 	// bootstrap with tool uploading - checking that a file
 	// is uploaded should be sufficient, as the detailed semantics
@@ -77,24 +92,26 @@
 	opc, errc := runCommand(new(BootstrapCommand), "--upload-tools")
 	c.Check(<-errc, IsNil)
 	c.Check((<-opc).(dummy.OpPutFile).Env, Equals, "peckham")
-	c.Check((<-opc).(dummy.OpBootstrap).Env, Equals, "peckham")
+	opBootstrap := (<-opc).(dummy.OpBootstrap)
+	c.Check(opBootstrap.Env, Equals, "peckham")
+	c.Check(opBootstrap.Constraints, DeepEquals, state.Constraints{})
 
+	// Check that some file was uploaded and can be unpacked; detailed
+	// semantics tested elsewhere.
 	envs, err := environs.ReadEnvirons("")
 	c.Assert(err, IsNil)
 	env, err := envs.Open("peckham")
 	c.Assert(err, IsNil)
-
 	tools, err := environs.FindTools(env, version.Current, environs.CompatVersion)
 	c.Assert(err, IsNil)
 	resp, err := http.Get(tools.URL)
 	c.Assert(err, IsNil)
 	defer resp.Body.Close()
-
 	err = agent.UnpackTools(c.MkDir(), tools, resp.Body)
 	c.Assert(err, IsNil)
 }
 
-func (*BootstrapSuite) TestBootstrapCommandBrokenEnvironment(c *C) {
+func (*BootstrapSuite) TestBrokenEnvironment(c *C) {
 	defer testing.MakeFakeHome(c, envConfig).Restore()
 	opc, errc := runCommand(new(BootstrapCommand), "-e", "brokenenv")
 	c.Check(<-errc, ErrorMatches, "dummy.Bootstrap is broken")
@@ -103,7 +120,6 @@
 
 func (*BootstrapSuite) TestMissingEnvironment(c *C) {
 	defer testing.MakeFakeHomeNoEnvironments(c, "empty").Restore()
-	// bootstrap without an environments.yaml
 	ctx := testing.Context(c)
 	code := cmd.Main(&BootstrapCommand{}, ctx, nil)
 	c.Check(code, Equals, 1)

=== modified file 'environs/bootstrap.go'
--- environs/bootstrap.go	2013-03-15 01:31:54 +0000
+++ environs/bootstrap.go	2013-03-19 10:29:25 +0000
@@ -3,19 +3,16 @@
 import (
 	"fmt"
 	"launchpad.net/juju-core/cert"
+	"launchpad.net/juju-core/state"
 	"time"
 )
 
-// Bootstrap bootstraps the given environment.  If the environment does
-// not contain a CA certificate, a new certificate and key pair are
-// generated, added to the environment configuration, and writeCertAndKey
-// will be called to save them.  If writeCertFile is nil, the generated
-// certificate and key will be saved to ~/.juju/<environ-name>-cert.pem
-// and ~/.juju/<environ-name>-private-key.pem.
-//
-// If uploadTools is true, the current version of the juju tools will be
-// uploaded, as documented in Environ.Bootstrap.
-func Bootstrap(environ Environ, uploadTools bool) error {
+// Bootstrap bootstraps the given environment. The supplied constraints are
+// used to provision the instance, and are also set within the bootstrapped
+// environment. The uploadTools parameter requires that the juju-core source
+// code be available within $GOPATH; if that is the case, that code will be
+// built locally and made available to the environment.
+func Bootstrap(environ Environ, cons state.Constraints, uploadTools bool) error {
 	cfg := environ.Config()
 	caCert, hasCACert := cfg.CACert()
 	caKey, hasCAKey := cfg.CAPrivateKey()
@@ -31,5 +28,5 @@
 	if err != nil {
 		return fmt.Errorf("cannot generate bootstrap certificate: %v", err)
 	}
-	return environ.Bootstrap(uploadTools, cert, key)
+	return environ.Bootstrap(cons, uploadTools, cert, key)
 }

=== modified file 'environs/bootstrap_test.go'
--- environs/bootstrap_test.go	2013-03-15 00:23:00 +0000
+++ environs/bootstrap_test.go	2013-03-19 10:29:25 +0000
@@ -7,6 +7,7 @@
 	"launchpad.net/juju-core/cert"
 	"launchpad.net/juju-core/environs"
 	"launchpad.net/juju-core/environs/config"
+	"launchpad.net/juju-core/state"
 	"launchpad.net/juju-core/testing"
 	"time"
 )
@@ -34,13 +35,13 @@
 
 func (s *bootstrapSuite) TestBootstrapNeedsConfigCert(c *C) {
 	env := newEnviron("bar", noKeysDefined)
-	err := environs.Bootstrap(env, false)
+	err := environs.Bootstrap(env, state.Constraints{}, false)
 	c.Assert(err, ErrorMatches, "environment configuration missing CA certificate")
 }
 
 func (s *bootstrapSuite) TestBootstrapKeyGeneration(c *C) {
 	env := newEnviron("foo", useDefaultKeys)
-	err := environs.Bootstrap(env, false)
+	err := environs.Bootstrap(env, state.Constraints{}, false)
 	c.Assert(err, IsNil)
 	c.Assert(env.bootstrapCount, Equals, 1)
 
@@ -55,18 +56,34 @@
 
 func (s *bootstrapSuite) TestBootstrapUploadTools(c *C) {
 	env := newEnviron("foo", useDefaultKeys)
-	err := environs.Bootstrap(env, false)
+	err := environs.Bootstrap(env, state.Constraints{}, false)
 	c.Assert(err, IsNil)
 	c.Assert(env.bootstrapCount, Equals, 1)
 	c.Assert(env.uploadTools, Equals, false)
 
 	env = newEnviron("foo", useDefaultKeys)
-	err = environs.Bootstrap(env, true)
+	err = environs.Bootstrap(env, state.Constraints{}, true)
 	c.Assert(err, IsNil)
 	c.Assert(env.bootstrapCount, Equals, 1)
 	c.Assert(env.uploadTools, Equals, true)
 }
 
+func (s *bootstrapSuite) TestBootstrapConstraints(c *C) {
+	env := newEnviron("foo", useDefaultKeys)
+	err := environs.Bootstrap(env, state.Constraints{}, false)
+	c.Assert(err, IsNil)
+	c.Assert(env.bootstrapCount, Equals, 1)
+	c.Assert(env.constraints, DeepEquals, state.Constraints{})
+
+	env = newEnviron("foo", useDefaultKeys)
+	cons, err := state.ParseConstraints("cpu-cores=2 mem=4G")
+	c.Assert(err, IsNil)
+	err = environs.Bootstrap(env, cons, false)
+	c.Assert(err, IsNil)
+	c.Assert(env.bootstrapCount, Equals, 1)
+	c.Assert(env.constraints, DeepEquals, cons)
+}
+
 type bootstrapEnviron struct {
 	name             string
 	cfg              *config.Config
@@ -74,6 +91,7 @@
 
 	// The following fields are filled in when Bootstrap is called.
 	bootstrapCount int
+	constraints    state.Constraints
 	uploadTools    bool
 	certPEM        []byte
 	keyPEM         []byte
@@ -105,8 +123,9 @@
 	return e.name
 }
 
-func (e *bootstrapEnviron) Bootstrap(uploadTools bool, certPEM, keyPEM []byte) error {
+func (e *bootstrapEnviron) Bootstrap(cons state.Constraints, uploadTools bool, certPEM, keyPEM []byte) error {
 	e.bootstrapCount++
+	e.constraints = cons
 	e.uploadTools = uploadTools
 	e.certPEM = certPEM
 	e.keyPEM = keyPEM

=== modified file 'environs/dummy/environs.go'
--- environs/dummy/environs.go	2013-03-14 02:38:14 +0000
+++ environs/dummy/environs.go	2013-03-19 10:29:25 +0000
@@ -59,7 +59,10 @@
 	Env string
 }
 
-type OpBootstrap GenericOperation
+type OpBootstrap struct {
+	Env         string
+	Constraints state.Constraints
+}
 
 type OpDestroy GenericOperation
 
@@ -425,7 +428,7 @@
 	return e.state.name
 }
 
-func (e *environ) Bootstrap(uploadTools bool, cert, key []byte) error {
+func (e *environ) Bootstrap(cons state.Constraints, uploadTools bool, cert, key []byte) error {
 	defer delay()
 	if err := e.checkBroken("Bootstrap"); err != nil {
 		return err
@@ -453,7 +456,7 @@
 	}
 	e.state.mu.Lock()
 	defer e.state.mu.Unlock()
-	e.state.ops <- OpBootstrap{Env: e.state.name}
+	e.state.ops <- OpBootstrap{Env: e.state.name, Constraints: cons}
 	if e.state.bootstrapped {
 		return fmt.Errorf("environment is already bootstrapped")
 	}
@@ -467,6 +470,9 @@
 		if err != nil {
 			panic(err)
 		}
+		if err := st.SetEnvironConstraints(cons); err != nil {
+			panic(err)
+		}
 		if err := st.SetAdminMongoPassword(trivial.PasswordHash(password)); err != nil {
 			panic(err)
 		}

=== modified file 'environs/ec2/ec2.go'
--- environs/ec2/ec2.go	2013-03-14 02:38:14 +0000
+++ environs/ec2/ec2.go	2013-03-19 10:29:25 +0000
@@ -236,7 +236,8 @@
 	return e.publicStorageUnlocked
 }
 
-func (e *environ) Bootstrap(uploadTools bool, cert, key []byte) error {
+func (e *environ) Bootstrap(cons state.Constraints, uploadTools bool, cert, key []byte) error {
+	// TODO(fwereade): handle bootstrap constraints
 	password := e.Config().AdminSecret()
 	if password == "" {
 		return fmt.Errorf("admin-secret is required for bootstrap")

=== modified file 'environs/ec2/local_test.go'
--- environs/ec2/local_test.go	2013-03-13 05:03:32 +0000
+++ environs/ec2/local_test.go	2013-03-19 10:29:25 +0000
@@ -237,7 +237,7 @@
 	policy := t.env.AssignmentPolicy()
 	c.Assert(policy, Equals, state.AssignUnused)
 
-	err := environs.Bootstrap(t.env, true)
+	err := environs.Bootstrap(t.env, state.Constraints{}, true)
 	c.Assert(err, IsNil)
 
 	// check that the state holds the id of the bootstrap machine.

=== modified file 'environs/interface.go'
--- environs/interface.go	2013-02-27 13:28:43 +0000
+++ environs/interface.go	2013-03-19 10:29:25 +0000
@@ -145,10 +145,13 @@
 	// environment via the juju package, the password hash will be
 	// automatically replaced by the real password.
 	//
+	// The supplied constraints are used to choose the initial instance
+	// specification, and will be stored in the new environment's state.
+	//
 	// The stateServerCertand stateServerKey parameters hold
 	// both the certificate and the respective private key to be
 	// used by the initial state server, in PEM format.
-	Bootstrap(uploadTools bool, stateServerCert, stateServerKey []byte) error
+	Bootstrap(cons state.Constraints, uploadTools bool, stateServerCert, stateServerKey []byte) error
 
 	// StateInfo returns information on the state initialized
 	// by Bootstrap.

=== modified file 'environs/jujutest/livetests.go'
--- environs/jujutest/livetests.go	2013-03-13 05:03:32 +0000
+++ environs/jujutest/livetests.go	2013-03-19 10:29:25 +0000
@@ -81,7 +81,7 @@
 	}
 	// We only build and upload tools if there will be a state agent that
 	// we could connect to (actual live tests, rather than local-only)
-	err := environs.Bootstrap(t.Env, t.CanOpenState)
+	err := environs.Bootstrap(t.Env, state.Constraints{}, t.CanOpenState)
 	c.Assert(err, IsNil)
 	t.bootstrapped = true
 }
@@ -301,7 +301,7 @@
 func (t *LiveTests) TestBootstrapMultiple(c *C) {
 	t.BootstrapOnce(c)
 
-	err := environs.Bootstrap(t.Env, false)
+	err := environs.Bootstrap(t.Env, state.Constraints{}, false)
 	c.Assert(err, ErrorMatches, "environment is already bootstrapped")
 
 	c.Logf("destroy env")
@@ -731,7 +731,7 @@
 	err = storageCopy(dummyStorage, currentPath, envStorage, otherPath)
 	c.Assert(err, IsNil)
 
-	err = environs.Bootstrap(env, false)
+	err = environs.Bootstrap(env, state.Constraints{}, false)
 	c.Assert(err, IsNil)
 	defer env.Destroy(nil)
 

=== modified file 'environs/jujutest/tests.go'
--- environs/jujutest/tests.go	2013-03-13 05:03:32 +0000
+++ environs/jujutest/tests.go	2013-03-19 10:29:25 +0000
@@ -52,7 +52,7 @@
 	delete(m, "admin-secret")
 	env, err := environs.NewFromAttrs(m)
 	c.Assert(err, IsNil)
-	err = environs.Bootstrap(env, false)
+	err = environs.Bootstrap(env, state.Constraints{}, false)
 	c.Assert(err, ErrorMatches, ".*admin-secret is required for bootstrap")
 }
 
@@ -107,18 +107,18 @@
 func (t *Tests) TestBootstrap(c *C) {
 	// TODO tests for Bootstrap(true)
 	e := t.Open(c)
-	err := environs.Bootstrap(e, false)
+	err := environs.Bootstrap(e, state.Constraints{}, false)
 	c.Assert(err, IsNil)
 
 	info, apiInfo, err := e.StateInfo()
 	c.Check(info.Addrs, Not(HasLen), 0)
 	c.Check(apiInfo.Addrs, Not(HasLen), 0)
 
-	err = environs.Bootstrap(e, false)
+	err = environs.Bootstrap(e, state.Constraints{}, false)
 	c.Assert(err, ErrorMatches, "environment is already bootstrapped")
 
 	e2 := t.Open(c)
-	err = environs.Bootstrap(e2, false)
+	err = environs.Bootstrap(e2, state.Constraints{}, false)
 	c.Assert(err, ErrorMatches, "environment is already bootstrapped")
 
 	info2, apiInfo2, err := e2.StateInfo()
@@ -131,10 +131,10 @@
 	// Open again because Destroy invalidates old environments.
 	e3 := t.Open(c)
 
-	err = environs.Bootstrap(e3, false)
+	err = environs.Bootstrap(e3, state.Constraints{}, false)
 	c.Assert(err, IsNil)
 
-	err = environs.Bootstrap(e3, false)
+	err = environs.Bootstrap(e3, state.Constraints{}, false)
 	c.Assert(err, NotNil)
 }
 

=== modified file 'environs/open_test.go'
--- environs/open_test.go	2013-03-06 20:21:43 +0000
+++ environs/open_test.go	2013-03-19 10:29:25 +0000
@@ -4,6 +4,7 @@
 	. "launchpad.net/gocheck"
 	"launchpad.net/juju-core/environs"
 	"launchpad.net/juju-core/environs/dummy"
+	"launchpad.net/juju-core/state"
 	"launchpad.net/juju-core/testing"
 )
 
@@ -28,7 +29,7 @@
 	}
 	env, err := environs.NewFromAttrs(config)
 	c.Assert(err, IsNil)
-	c.Assert(env.Bootstrap(false, nil, nil), IsNil)
+	c.Assert(env.Bootstrap(state.Constraints{}, false, nil, nil), IsNil)
 }
 
 func (OpenSuite) TestNewUnknownEnviron(c *C) {

=== modified file 'environs/openstack/local_test.go'
--- environs/openstack/local_test.go	2013-03-13 05:03:32 +0000
+++ environs/openstack/local_test.go	2013-03-19 10:29:25 +0000
@@ -232,7 +232,7 @@
 	newconfig["use-floating-ip"] = true
 	env, err := environs.NewFromAttrs(newconfig)
 	c.Assert(err, IsNil)
-	err = environs.Bootstrap(env, s.CanOpenState)
+	err = environs.Bootstrap(env, state.Constraints{}, s.CanOpenState)
 	c.Assert(err, ErrorMatches, ".*cannot allocate a public IP as needed.*")
 }
 
@@ -254,7 +254,7 @@
 		},
 	)
 	defer cleanup()
-	err := environs.Bootstrap(s.Env, false)
+	err := environs.Bootstrap(s.Env, state.Constraints{}, false)
 	c.Assert(err, IsNil)
 	inst, err := s.Env.StartInstance("100", testing.InvalidStateInfo("100"), testing.InvalidAPIInfo("100"), nil)
 	c.Assert(err, IsNil)
@@ -355,7 +355,7 @@
 	policy := t.env.AssignmentPolicy()
 	c.Assert(policy, Equals, state.AssignUnused)
 
-	err := environs.Bootstrap(t.env, false)
+	err := environs.Bootstrap(t.env, state.Constraints{}, false)
 	c.Assert(err, IsNil)
 
 	// check that the state holds the id of the bootstrap machine.

=== modified file 'environs/openstack/provider.go'
--- environs/openstack/provider.go	2013-03-14 02:38:14 +0000
+++ environs/openstack/provider.go	2013-03-19 10:29:25 +0000
@@ -416,7 +416,8 @@
 	return e.publicStorageUnlocked
 }
 
-func (e *environ) Bootstrap(uploadTools bool, cert, key []byte) error {
+func (e *environ) Bootstrap(cons state.Constraints, uploadTools bool, cert, key []byte) error {
+	// TODO(fwereade): handle bootstrap constraints
 	password := e.Config().AdminSecret()
 	if password == "" {
 		return fmt.Errorf("admin-secret is required for bootstrap")

=== modified file 'juju/apiconn_test.go'
--- juju/apiconn_test.go	2013-03-13 05:03:32 +0000
+++ juju/apiconn_test.go	2013-03-19 10:29:25 +0000
@@ -5,6 +5,7 @@
 	"launchpad.net/juju-core/environs"
 	"launchpad.net/juju-core/environs/dummy"
 	"launchpad.net/juju-core/juju"
+	"launchpad.net/juju-core/state"
 	coretesting "launchpad.net/juju-core/testing"
 )
 
@@ -32,7 +33,7 @@
 	}
 	env, err := environs.NewFromAttrs(attrs)
 	c.Assert(err, IsNil)
-	err = environs.Bootstrap(env, false)
+	err = environs.Bootstrap(env, state.Constraints{}, false)
 	c.Assert(err, IsNil)
 
 	conn, err := juju.NewConn(env)

=== modified file 'juju/conn_test.go'
--- juju/conn_test.go	2013-03-13 05:03:32 +0000
+++ juju/conn_test.go	2013-03-19 10:29:25 +0000
@@ -44,7 +44,7 @@
 	}
 	env, err := environs.NewFromAttrs(attrs)
 	c.Assert(err, IsNil)
-	err = environs.Bootstrap(env, false)
+	err = environs.Bootstrap(env, state.Constraints{}, false)
 	c.Assert(err, IsNil)
 
 	delete(attrs, "admin-secret")
@@ -65,7 +65,7 @@
 func bootstrapEnv(c *C, envName string) {
 	environ, err := environs.NewFromName(envName)
 	c.Assert(err, IsNil)
-	err = environs.Bootstrap(environ, false)
+	err = environs.Bootstrap(environ, state.Constraints{}, false)
 	c.Assert(err, IsNil)
 }
 
@@ -112,7 +112,7 @@
 	}
 	env, err := environs.NewFromAttrs(attrs)
 	c.Assert(err, IsNil)
-	err = environs.Bootstrap(env, false)
+	err = environs.Bootstrap(env, state.Constraints{}, false)
 	c.Assert(err, IsNil)
 	info, _, err := env.StateInfo()
 	c.Assert(err, IsNil)
@@ -152,7 +152,7 @@
 	}
 	env, err := environs.NewFromAttrs(attrs)
 	c.Assert(err, IsNil)
-	err = environs.Bootstrap(env, false)
+	err = environs.Bootstrap(env, state.Constraints{}, false)
 	c.Assert(err, IsNil)
 
 	// Make a new Conn, which will push the secrets.
@@ -190,7 +190,7 @@
 		"ca-private-key":  coretesting.CAKey,
 	})
 	c.Assert(err, IsNil)
-	err = environs.Bootstrap(env, false)
+	err = environs.Bootstrap(env, state.Constraints{}, false)
 	c.Assert(err, IsNil)
 
 	// Check that Bootstrap has correctly used a hash
@@ -248,7 +248,7 @@
 	}
 	environ, err := environs.NewFromAttrs(attrs)
 	c.Assert(err, IsNil)
-	err = environs.Bootstrap(environ, false)
+	err = environs.Bootstrap(environ, state.Constraints{}, false)
 	c.Assert(err, IsNil)
 	s.conn, err = juju.NewConn(environ)
 	c.Assert(err, IsNil)

=== modified file 'juju/testing/conn.go'
--- juju/testing/conn.go	2013-03-18 23:15:14 +0000
+++ juju/testing/conn.go	2013-03-19 10:29:25 +0000
@@ -150,7 +150,7 @@
 	c.Assert(err, IsNil)
 	// sanity check we've got the correct environment.
 	c.Assert(environ.Name(), Equals, "dummyenv")
-	c.Assert(environs.Bootstrap(environ, false), IsNil)
+	c.Assert(environs.Bootstrap(environ, state.Constraints{}, false), IsNil)
 
 	conn, err := juju.NewConn(environ)
 	c.Assert(err, IsNil)