Merge lp:~thumper/juju-core/local-provider into lp:~go-bot/juju-core/trunk

[Tim Penhey]

Enable the local provider.

This branch changes how the local storage is handled.
It really wasn't possible to have the client keep serving
the storage on dynamic ports and expect other running
services to work.

Also we need something always acting as the storage. After
discussion with William last night, we agreed that this
should be a worker in the machine agent.

https://codereview.appspot.com/11333043/

Enable the local provider.

This branch changes how the local storage is handled.
It really wasn't possible to have the client keep serving
the storage on dynamic ports and expect other running
services to work.

Also we need something always acting as the storage. After
discussion with William last night, we agreed that this
should be a worker in the machine agent.

https://codereview.appspot.com/11333043/

[Tim Penhey (thumper)]

Reviewers: mp+174930_code.launchpad.net,

Message:
Please take a look.

Description:
Enable the local provider.

This branch changes how the local storage is handled.
It really wasn't possible to have the client keep serving
the storage on dynamic ports and expect other running
services to work.

Also we need something always acting as the storage. After
discussion with William last night, we agreed that this
should be a worker in the machine agent.

https://code.launchpad.net/~thumper/juju-core/local-provider/+merge/174930

Requires:
https://code.launchpad.net/~thumper/juju-core/selectively-install-lxc/+merge/174928

(do not edit description out of merge proposal)

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

Affected files:
   A [revision details]
   M cmd/jujud/machine.go
   M environs/all/all.go
   M environs/local/config.go
   M environs/local/environ.go
   M environs/local/environprovider.go
   A environs/local/storage/worker.go

[Frank Mueller (themue)]

LGTM, great work, but some comments.

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

https://codereview.appspot.com/11333043/diff/1/cmd/jujud/machine.go#newcode189
cmd/jujud/machine.go:189: if providerType != "local" &&
m.ContainerType() != instance.LXC {
Make "local" a const.

https://codereview.appspot.com/11333043/diff/1/cmd/jujud/machine.go#newcode197
cmd/jujud/machine.go:197: if providerType == "local" && m.Id() == "0" {
Also a const with a speaking name for the "0".

https://codereview.appspot.com/11333043/diff/1/environs/local/environ.go
File environs/local/environ.go (right):

https://codereview.appspot.com/11333043/diff/1/environs/local/environ.go#newcode199
environs/local/environ.go:199: // If we get to here, it is because we
haven't yet bootstrapped an
Interesting hurdle. ;) But I would like a construct like

if !config.bootstrapped() {
     return env.<findAGoodNameForTheStuffBelow>()
}
return nil

more. So that logical block of code has its own method.

https://codereview.appspot.com/11333043/diff/1/environs/local/environ.go#newcode229
environs/local/environ.go:229: func (env *localEnviron)
setupLocalStorage() error {
Appreciate comment event for private methods.

https://codereview.appspot.com/11333043/diff/1/environs/local/environ.go#newcode287
environs/local/environ.go:287: if inst.Id() == "localhost" {
Again I like a const here. In the error you name it "bootstrap
instance". So

if inst.Id() == bootstrapInstance { ... }

would read better.

https://codereview.appspot.com/11333043/diff/1/environs/local/environ.go#newcode311
environs/local/environ.go:311: instances = append(instances,
&localInstance{"localhost", env})
Const, see above.

https://codereview.appspot.com/11333043/

[Martin Packman (gz)]

Unless I'm missing something, having both config and JUJU_ envvars
appears to be redundant. I would strongly prefer not having the storage
ports configurable, and just dynamically assign them and pass through.

Have yet to actually play with this, but looks pretty close to useful.

https://codereview.appspot.com/11333043/diff/1/environs/local/config.go
File environs/local/config.go (right):

https://codereview.appspot.com/11333043/diff/1/environs/local/config.go#newcode31
environs/local/config.go:31: "shared-storage-port": 8041,
Any reasoning behind these default ports?

https://codereview.appspot.com/11333043/diff/1/environs/local/environ.go
File environs/local/environ.go (right):

https://codereview.appspot.com/11333043/diff/1/environs/local/environ.go#newcode298
environs/local/environ.go:298: func (env *localEnviron) Instances(ids
[]instance.Id) ([]instance.Instance, error) {
It's now possible to implement this correctly, by for instance calling
AllInstances then filtering.

https://codereview.appspot.com/11333043/

[Tim Penhey (thumper)]

Please take a look.

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

https://codereview.appspot.com/11333043/diff/1/cmd/jujud/machine.go#newcode189
cmd/jujud/machine.go:189: if providerType != "local" &&
m.ContainerType() != instance.LXC {
On 2013/07/16 10:16:35, mue wrote:
> Make "local" a const.

Done.

https://codereview.appspot.com/11333043/diff/1/cmd/jujud/machine.go#newcode197
cmd/jujud/machine.go:197: if providerType == "local" && m.Id() == "0" {
On 2013/07/16 10:16:35, mue wrote:
> Also a const with a speaking name for the "0".

Done.

https://codereview.appspot.com/11333043/diff/1/environs/local/config.go
File environs/local/config.go (right):

https://codereview.appspot.com/11333043/diff/1/environs/local/config.go#newcode31
environs/local/config.go:31: "shared-storage-port": 8041,
On 2013/07/16 10:44:18, gz wrote:
> Any reasoning behind these default ports?

None at all, but I needed some defaults.

8000 and 8080 are often taken by other web thingies, and in the 8000
range is often thought of as http type ones, so I thought it reasonable.

https://codereview.appspot.com/11333043/diff/1/environs/local/environ.go
File environs/local/environ.go (right):

https://codereview.appspot.com/11333043/diff/1/environs/local/environ.go#newcode229
environs/local/environ.go:229: func (env *localEnviron)
setupLocalStorage() error {
On 2013/07/16 10:16:35, mue wrote:
> Appreciate comment event for private methods.

Done.

https://codereview.appspot.com/11333043/diff/1/environs/local/environ.go#newcode287
environs/local/environ.go:287: if inst.Id() == "localhost" {
On 2013/07/16 10:16:35, mue wrote:
> Again I like a const here. In the error you name it "bootstrap
instance". So

> if inst.Id() == bootstrapInstance { ... }

> would read better.

Done.

https://codereview.appspot.com/11333043/diff/1/environs/local/environ.go#newcode311
environs/local/environ.go:311: instances = append(instances,
&localInstance{"localhost", env})
On 2013/07/16 10:16:35, mue wrote:
> Const, see above.

Done.

https://codereview.appspot.com/11333043/

[Tim Penhey (thumper)]

On 2013/07/16 10:44:18, gz wrote:
> Unless I'm missing something, having both config and JUJU_ envvars
appears to be
> redundant. I would strongly prefer not having the storage ports
configurable,
> and just dynamically assign them and pass through.

Perhaps this was just being a little lazy as it meant that the worker
didn't
need to watch for environ changes in the configuration. It made the
worker
simpler, and that was what I was going for initially.

https://codereview.appspot.com/11333043/diff/1/environs/local/environ.go#newcode298
> environs/local/environ.go:298: func (env *localEnviron) Instances(ids
> []instance.Id) ([]instance.Instance, error) {
> It's now possible to implement this correctly, by for instance calling
> AllInstances then filtering.

I still kinda argue the point about caring for this.

https://codereview.appspot.com/11333043/

[Ian Booth (wallyworld)]

I shared Martin's concerns about the env variables. But we could land
this and fix it before the next release.

LGTM

https://codereview.appspot.com/11333043/diff/1/environs/local/environ.go
File environs/local/environ.go (right):

https://codereview.appspot.com/11333043/diff/1/environs/local/environ.go#newcode341
environs/local/environ.go:341: return err
Should we try and do as much removal as we can here even if something
errors?

https://codereview.appspot.com/11333043/

[Tim Penhey (thumper)]

https://codereview.appspot.com/11333043/diff/1/environs/local/environ.go
File environs/local/environ.go (right):

https://codereview.appspot.com/11333043/diff/1/environs/local/environ.go#newcode341
environs/local/environ.go:341: return err
On 2013/07/17 03:40:55, wallyworld wrote:
> Should we try and do as much removal as we can here even if something
errors?

While I don't expect errors here, it seems weird to try to special case
it.

https://codereview.appspot.com/11333043/

[Go Bot (go-bot)]

The attempt to merge lp:~thumper/juju-core/local-provider into lp:juju-core failed. Below is the output from the failed tests.

ok launchpad.net/juju-core/agent 2.512s
ok launchpad.net/juju-core/bzr 6.897s
ok launchpad.net/juju-core/cert 3.634s
ok launchpad.net/juju-core/charm 0.564s
? launchpad.net/juju-core/charm/hooks [no test files]
ok launchpad.net/juju-core/cloudinit 0.009s
ok launchpad.net/juju-core/cmd 0.230s
? launchpad.net/juju-core/cmd/builddb [no test files]
? launchpad.net/juju-core/cmd/charmd [no test files]
? launchpad.net/juju-core/cmd/charmload [no test files]
ok launchpad.net/juju-core/cmd/juju 112.606s
ok launchpad.net/juju-core/cmd/jujud 41.977s
ok launchpad.net/juju-core/constraints 0.013s
ok launchpad.net/juju-core/container/lxc 0.325s
? launchpad.net/juju-core/container/lxc/mock [no test files]
ok launchpad.net/juju-core/downloader 5.333s
ok launchpad.net/juju-core/environs 1.895s
? launchpad.net/juju-core/environs/all [no test files]
ok launchpad.net/juju-core/environs/azure 2.780s
ok launchpad.net/juju-core/environs/cloudinit 0.580s
ok launchpad.net/juju-core/environs/config 1.085s
ok launchpad.net/juju-core/environs/dummy 16.618s
ok launchpad.net/juju-core/environs/ec2 175.430s
ok launchpad.net/juju-core/environs/imagemetadata 0.267s
ok launchpad.net/juju-core/environs/instances 0.254s
ok launchpad.net/juju-core/environs/jujutest 0.257s

----------------------------------------------------------------------
FAIL: environ_test.go:35: environSuite.TestNameAndStorage

[LOG] 42.62519 INFO juju.environs.local opening environment "test"
[LOG] 42.67884 TRACE juju.environs.local creating directory /tmp/gocheck-220031192/0/.juju/test/shared-storage
[LOG] 42.67907 TRACE juju.environs.local creating directory /tmp/gocheck-220031192/0/.juju/test/storage
[LOG] 42.67912 TRACE juju.environs.local creating directory /tmp/gocheck-220031192/0/.juju/test/db
[LOG] 42.67916 TRACE juju.environs.local creating directory /tmp/gocheck-220031192/0/.juju/test/log
[LOG] 42.67937 ERROR juju.environs.local cannot find network interface "lxcbr0": net: no such interface
[LOG] 42.67938 ERROR juju.environs.local failure setting config: net: no such interface
environ_test.go:38:
    c.Assert(err, gc.IsNil)
... value *errors.errorString = &errors.errorString{s:"net: no such interface"} ("net: no such interface")

----------------------------------------------------------------------
FAIL: environ_test.go:54: localJujuTestSuite.SetUpTest

[LOG] 42.86084 TRACE juju.environs.local creating directory /tmp/gocheck-2031484958/0/.juju/test/shared-storage
[LOG] 42.86103 TRACE juju.environs.local creating directory /tmp/gocheck-2031484958/0/.juju/test/storage
[LOG] 42.86107 TRACE juju.environs.local creating directory /tmp/gocheck-2031484958/0/.juju/test/db
[LOG] 42.86111 TRACE juju.environs.local creating directory /tmp/gocheck-2031484958/0/.juju/test/log
[LOG] 42.89145 INFO juju.environs.local opening environment "test"
[LOG] 42.94455 ERROR juju.environs.local cannot find network interface "lxcbr0": net: no such interface
[LOG] 42.94458 ERROR juju.environs.local failure setting config: net: no such in...

[Go Bot (go-bot)]

The attempt to merge lp:~thumper/juju-core/local-provider into lp:juju-core failed. Below is the output from the failed tests.

ok launchpad.net/juju-core/agent 2.061s
ok launchpad.net/juju-core/bzr 6.829s
ok launchpad.net/juju-core/cert 4.137s
ok launchpad.net/juju-core/charm 0.565s
? launchpad.net/juju-core/charm/hooks [no test files]
ok launchpad.net/juju-core/cloudinit 0.009s
ok launchpad.net/juju-core/cmd 0.221s
? launchpad.net/juju-core/cmd/builddb [no test files]
? launchpad.net/juju-core/cmd/charmd [no test files]
? launchpad.net/juju-core/cmd/charmload [no test files]
ok launchpad.net/juju-core/cmd/juju 110.572s
ok launchpad.net/juju-core/cmd/jujud 41.854s
ok launchpad.net/juju-core/constraints 0.013s
ok launchpad.net/juju-core/container/lxc 0.326s
? launchpad.net/juju-core/container/lxc/mock [no test files]
ok launchpad.net/juju-core/downloader 5.314s
ok launchpad.net/juju-core/environs 1.924s
? launchpad.net/juju-core/environs/all [no test files]
ok launchpad.net/juju-core/environs/azure 2.728s
ok launchpad.net/juju-core/environs/cloudinit 0.575s
ok launchpad.net/juju-core/environs/config 1.038s
ok launchpad.net/juju-core/environs/dummy 16.877s
ok launchpad.net/juju-core/environs/ec2 175.395s
ok launchpad.net/juju-core/environs/imagemetadata 0.253s
ok launchpad.net/juju-core/environs/instances 0.267s
ok launchpad.net/juju-core/environs/jujutest 0.243s

----------------------------------------------------------------------
FAIL: environ_test.go:71: localJujuTestSuite.TearDownTest

environ_test.go:72:
    s.Tests.TearDownTest(c)
/home/tarmac/trees/src/launchpad.net/juju-core/environs/jujutest/tests.go:70:
    c.Check(err, IsNil)
... value *golxc.Error = &golxc.Error{Name:"lxc-ls", Err:(*exec.Error)(0xf840169cc0), Output:[]string(nil)} ("error executing \"lxc-ls\": exec: \"lxc-ls\": executable file not found in $PATH")

----------------------------------------------------------------------
PANIC: environ_test.go:100: localJujuTestSuite.TestBootstrap

[LOG] 62.75077 TRACE juju.environs.local creating directory /tmp/gocheck-2031484958/0/.juju/test/shared-storage
[LOG] 62.75101 TRACE juju.environs.local creating directory /tmp/gocheck-2031484958/0/.juju/test/storage
[LOG] 62.75105 TRACE juju.environs.local creating directory /tmp/gocheck-2031484958/0/.juju/test/db
[LOG] 62.75109 TRACE juju.environs.local creating directory /tmp/gocheck-2031484958/0/.juju/test/log
[LOG] 62.78159 INFO juju.environs.local opening environment "test"
[LOG] 62.83570 DEBUG juju.environs.local getAddressForInterface called for lxcbr0
[LOG] 62.83574 DEBUG juju.environs.local found "127.0.0.1" as address for "lxcbr0"
[LOG] 62.88695 DEBUG juju.environs.local checking 127.0.0.1:8040 to see if machine agent running storage listener
[LOG] 62.88740 DEBUG juju.environs.local yes, don't start local storage listeners
[LOG] 62.91520 ERROR juju.container.lxc failed getting all instances: error executing "lxc-ls": exec: "lxc-ls": executable file not found in $PATH
... Panic: Fixture has panicked (see related PANIC)
OOPS: 9 passed, 2 skipped, 1 FAILED, 3 MISSED
--- FAIL: TestLocal (0.78 seconds)
FAIL
FAIL launchpad.ne...

=== modified file 'cmd/jujud/machine.go'
--- cmd/jujud/machine.go	2013-07-16 06:18:45 +0000
+++ cmd/jujud/machine.go	2013-07-17 05:56:24 +0000
@@ -5,9 +5,17 @@
 
 import (
 	"fmt"
+	"os"
+	"path/filepath"
+	"time"
+
 	"launchpad.net/gnuflag"
+	"launchpad.net/tomb"
+
 	"launchpad.net/juju-core/charm"
 	"launchpad.net/juju-core/cmd"
+	localstorage "launchpad.net/juju-core/environs/local/storage"
+	"launchpad.net/juju-core/environs/provider"
 	"launchpad.net/juju-core/instance"
 	"launchpad.net/juju-core/log"
 	"launchpad.net/juju-core/state"
@@ -21,11 +29,10 @@
 	"launchpad.net/juju-core/worker/machiner"
 	"launchpad.net/juju-core/worker/provisioner"
 	"launchpad.net/juju-core/worker/resumer"
-	"launchpad.net/tomb"
-	"path/filepath"
-	"time"
 )
 
+const bootstrapMachineId = "0"
+
 var retryDelay = 3 * time.Second
 
 // MachineAgent is a cmd.Command responsible for running a machine agent.
@@ -100,7 +107,7 @@
 			return a.StateWorker()
 		})
 	}
-	if a.MachineId == "0" {
+	if a.MachineId == bootstrapMachineId {
 		// If we're bootstrapping, we don't have an API
 		// server to connect to, so start the state worker regardless.
 
@@ -181,12 +188,20 @@
 	// containers, it is likely that we will want an LXC provisioner on a KVM
 	// machine, and once we get nested LXC containers, we can remove this
 	// check.
-	if m.ContainerType() != instance.LXC {
+	providerType := os.Getenv("JUJU_PROVIDER_TYPE")
+	if providerType != provider.Local && m.ContainerType() != instance.LXC {
 		workerName := fmt.Sprintf("%s-provisioner", provisioner.LXC)
 		runner.StartWorker(workerName, func() (worker.Worker, error) {
 			return provisioner.NewProvisioner(provisioner.LXC, st, a.MachineId, dataDir), nil
 		})
 	}
+	// Take advantage of special knowledge here in that we will only ever want
+	// the storage provider on one machine, and that is the "bootstrap" node.
+	if providerType == provider.Local && m.Id() == bootstrapMachineId {
+		runner.StartWorker("local-storage", func() (worker.Worker, error) {
+			return localstorage.NewWorker(), nil
+		})
+	}
 	for _, job := range m.Jobs() {
 		switch job {
 		case state.JobHostUnits:

=== modified file 'environs/all/all.go'
--- environs/all/all.go	2013-07-16 03:33:07 +0000
+++ environs/all/all.go	2013-07-17 05:56:24 +0000
@@ -7,6 +7,7 @@
 import (
 	_ "launchpad.net/juju-core/environs/azure"
 	_ "launchpad.net/juju-core/environs/ec2"
+	_ "launchpad.net/juju-core/environs/local"
 	_ "launchpad.net/juju-core/environs/maas"
 	_ "launchpad.net/juju-core/environs/openstack"
 )

=== modified file 'environs/local/config.go'
--- environs/local/config.go	2013-07-16 21:56:23 +0000
+++ environs/local/config.go	2013-07-17 05:56:24 +0000
@@ -17,13 +17,23 @@
 	return os.Getuid() == 0
 }
 
-var configChecker = schema.StrictFieldMap(
-	schema.Fields{
-		"root-dir": schema.String(),
-	},
-	schema.Defaults{
-		"root-dir": "",
-	},
+var (
+	configFields = schema.Fields{
+		"root-dir":            schema.String(),
+		"bootstrap-ip":        schema.String(),
+		"storage-port":        schema.Int(),
+		"shared-storage-port": schema.Int(),
+	}
+	// The port defaults below are not entirely arbitrary.  Local user web
+	// frameworks often use 8000 or 8080, so I didn't want to use either of
+	// these, but did want the familiarity of using something in the 8000
+	// range.
+	configDefaults = schema.Defaults{
+		"root-dir":            "",
+		"bootstrap-ip":        schema.Omit,
+		"storage-port":        8040,
+		"shared-storage-port": 8041,
+	}
 )
 
 type environConfig struct {
@@ -77,6 +87,36 @@
 	return filepath.Join(c.rootDir(), "log")
 }
 
+// A config is bootstrapped if the bootstrap-ip address has been set.
+func (c *environConfig) bootstrapped() bool {
+	_, found := c.attrs["bootstrap-ip"]
+	return found
+}
+
+func (c *environConfig) bootstrapIPAddress() string {
+	addr, found := c.attrs["bootstrap-ip"]
+	if found {
+		return addr.(string)
+	}
+	return ""
+}
+
+func (c *environConfig) storagePort() int {
+	return int(c.attrs["storage-port"].(int64))
+}
+
+func (c *environConfig) sharedStoragePort() int {
+	return int(c.attrs["shared-storage-port"].(int64))
+}
+
+func (c *environConfig) storageAddr() string {
+	return fmt.Sprintf("%s:%d", c.bootstrapIPAddress(), c.storagePort())
+}
+
+func (c *environConfig) sharedStorageAddr() string {
+	return fmt.Sprintf("%s:%d", c.bootstrapIPAddress(), c.sharedStoragePort())
+}
+
 func (c *environConfig) configFile(filename string) string {
 	return filepath.Join(c.rootDir(), filename)
 }

=== modified file 'environs/local/environ.go'
--- environs/local/environ.go	2013-07-17 01:04:36 +0000
+++ environs/local/environ.go	2013-07-17 05:56:24 +0000
@@ -15,6 +15,7 @@
 
 	"launchpad.net/juju-core/agent"
 	"launchpad.net/juju-core/constraints"
+	"launchpad.net/juju-core/container/lxc"
 	"launchpad.net/juju-core/environs"
 	"launchpad.net/juju-core/environs/config"
 	"launchpad.net/juju-core/environs/localstorage"
@@ -31,6 +32,10 @@
 // containers being created are able to communicate with it simply.
 const lxcBridgeName = "lxcbr0"
 
+// boostrapInstanceId is just the name we give to the bootstrap machine.
+// Using "localhost" because it is, and it makes sense.
+const boostrapInstanceId = "localhost"
+
 // upstartScriptLocation is parameterised purely for testing purposes as we
 // don't really want to be installing and starting scripts as root for
 // testing.
@@ -55,6 +60,7 @@
 	name                  string
 	sharedStorageListener net.Listener
 	storageListener       net.Listener
+	containerManager      lxc.ContainerManager
 }
 
 // Name is specified in the Environ interface.
@@ -119,16 +125,9 @@
 		return err
 	}
 
-	// Work out the ip address of the lxc bridge, and use that for the mongo config.
-	bridgeAddress, err := env.findBridgeAddress()
-	if err != nil {
-		return err
-	}
-	logger.Debugf("found %q as address for %q", bridgeAddress, lxcBridgeName)
-
 	// Before we write the agent config file, we need to make sure the
 	// instance is saved in the StateInfo.
-	bootstrapId := instance.Id("localhost")
+	bootstrapId := instance.Id(boostrapInstanceId)
 	if err := environs.SaveState(env.Storage(), &environs.BootstrapState{[]instance.Id{bootstrapId}}); err != nil {
 		logger.Errorf("failed to save state instances: %v", err)
 		return err
@@ -143,7 +142,7 @@
 
 	// Have to initialize the state configuration with localhost so we get
 	// "special" permissions.
-	stateConnection, err := env.initialStateConfiguration("localhost", cons)
+	stateConnection, err := env.initialStateConfiguration(boostrapInstanceId, cons)
 	if err != nil {
 		return err
 	}
@@ -164,7 +163,7 @@
 	return env.config.Config
 }
 
-func createLocalStorageListener(dir string) (net.Listener, error) {
+func createLocalStorageListener(dir, address string) (net.Listener, error) {
 	info, err := os.Stat(dir)
 	if os.IsNotExist(err) {
 		return nil, fmt.Errorf("storage directory %q does not exist, bootstrap first", dir)
@@ -173,8 +172,7 @@
 	} else if !info.Mode().IsDir() {
 		return nil, fmt.Errorf("%q exists but is not a directory (and it needs to be)", dir)
 	}
-	// TODO(thumper): this needs fixing when we have actual machines.
-	return localstorage.Serve("localhost:0", dir)
+	return localstorage.Serve(address, dir)
 }
 
 // SetConfig is specified in the Environ interface.
@@ -189,28 +187,77 @@
 	env.config = config
 	env.name = config.Name()
 
-	if err := config.createDirs(); err != nil {
-		return err
-	}
-
-	sharedStorageListener, err := createLocalStorageListener(config.sharedStorageDir())
-	if err != nil {
-		return err
-	}
-
-	storageListener, err := createLocalStorageListener(config.storageDir())
-	if err != nil {
-		sharedStorageListener.Close()
-		return err
-	}
-	if env.sharedStorageListener != nil {
-		env.sharedStorageListener.Close()
-	}
-	if env.storageListener != nil {
-		env.storageListener.Close()
-	}
-	env.sharedStorageListener = sharedStorageListener
-	env.storageListener = storageListener
+	env.containerManager = lxc.NewContainerManager(
+		lxc.ManagerConfig{
+			Name:   env.config.namespace(),
+			LogDir: env.config.logDir(),
+		})
+
+	// Here is the end of normal config setting.
+	if config.bootstrapped() {
+		return nil
+	}
+	return env.bootstrapAddressAndStorage(cfg)
+}
+
+// bootstrapAddressAndStorage finishes up the setup of the environment in
+// situations where there is no machine agent running yet.
+func (env *localEnviron) bootstrapAddressAndStorage(cfg *config.Config) error {
+	// If we get to here, it is because we haven't yet bootstrapped an
+	// environment, and saved the config in it, or we are running a command
+	// from the command line, so it is ok to work on the assumption that we
+	// have direct access to the directories.
+	if err := env.config.createDirs(); err != nil {
+		return err
+	}
+
+	bridgeAddress, err := env.findBridgeAddress()
+	if err != nil {
+		return err
+	}
+	logger.Debugf("found %q as address for %q", bridgeAddress, lxcBridgeName)
+	cfg, err = cfg.Apply(map[string]interface{}{
+		"bootstrap-ip": bridgeAddress,
+	})
+	if err != nil {
+		logger.Errorf("failed to apply new addresses to config: %v", err)
+		return err
+	}
+	config, err := provider.newConfig(cfg)
+	if err != nil {
+		logger.Errorf("failed to create new environ config: %v", err)
+		return err
+	}
+	env.config = config
+
+	return env.setupLocalStorage()
+}
+
+// setupLocalStorage looks to see if there is someone listening on the storage
+// address port.  If there is we assume that it is ours and all is good.  If
+// there is no one listening on that port, create listeners for both storage
+// and the shared storage for the duration of the commands execution.
+func (env *localEnviron) setupLocalStorage() error {
+	// Try to listen to the storageAddress.
+	logger.Debugf("checking %s to see if machine agent running storage listener", env.config.storageAddr())
+	connection, err := net.Dial("tcp", env.config.storageAddr())
+	if err != nil {
+		logger.Debugf("nope, start some")
+		// These listeners are part of the environment structure so as to remain
+		// referenced for the duration of the open environment.  This is only for
+		// environs that have been created due to a user command.
+		env.storageListener, err = createLocalStorageListener(env.config.storageDir(), env.config.storageAddr())
+		if err != nil {
+			return err
+		}
+		env.sharedStorageListener, err = createLocalStorageListener(env.config.sharedStorageDir(), env.config.sharedStorageAddr())
+		if err != nil {
+			return err
+		}
+	} else {
+		logger.Debugf("yes, don't start local storage listeners")
+		connection.Close()
+	}
 	return nil
 }
 
@@ -218,15 +265,44 @@
 func (env *localEnviron) StartInstance(
 	machineId, machineNonce, series string,
 	cons constraints.Value,
-	info *state.Info,
+	stateInfo *state.Info,
 	apiInfo *api.Info,
 ) (instance.Instance, *instance.HardwareCharacteristics, error) {
-	return nil, nil, fmt.Errorf("start instance not implemented")
+	// We pretty much ignore the constraints.
+	logger.Debugf("StartInstance: %q, %s", machineId, series)
+	possibleTools, err := environs.FindInstanceTools(env, series, cons)
+	if err != nil {
+		return nil, nil, err
+	}
+	if len(possibleTools) == 0 {
+		return nil, nil, fmt.Errorf("could not find appropriate tools")
+	}
+
+	tools := possibleTools[0]
+	logger.Debugf("tools: %#v", tools)
+
+	inst, err := env.containerManager.StartContainer(
+		machineId, series, machineNonce,
+		tools, env.config.Config,
+		stateInfo, apiInfo)
+	if err != nil {
+		return nil, nil, err
+	}
+	// TODO(thumper): return some hardware characteristics.
+	return inst, nil, nil
 }
 
 // StopInstances is specified in the Environ interface.
-func (env *localEnviron) StopInstances([]instance.Instance) error {
-	return fmt.Errorf("stop instance not implemented")
+func (env *localEnviron) StopInstances(instances []instance.Instance) error {
+	for _, inst := range instances {
+		if inst.Id() == boostrapInstanceId {
+			return fmt.Errorf("cannot stop the bootstrap instance")
+		}
+		if err := env.containerManager.StopContainer(inst); err != nil {
+			return err
+		}
+	}
+	return nil
 }
 
 // Instances is specified in the Environ interface.
@@ -246,19 +322,26 @@
 
 // AllInstances is specified in the Environ interface.
 func (env *localEnviron) AllInstances() (instances []instance.Instance, err error) {
-	// TODO(thumper): get all the instances from the container manager
-	instances = append(instances, &localInstance{"localhost", env})
+	instances = append(instances, &localInstance{boostrapInstanceId, env})
+	// Add in all the containers as well.
+	lxcInstances, err := env.containerManager.ListContainers()
+	if err != nil {
+		return nil, err
+	}
+	for _, inst := range lxcInstances {
+		instances = append(instances, &localInstance{inst.Id(), env})
+	}
 	return instances, nil
 }
 
 // Storage is specified in the Environ interface.
 func (env *localEnviron) Storage() environs.Storage {
-	return localstorage.Client(env.storageListener.Addr().String())
+	return localstorage.Client(env.config.storageAddr())
 }
 
 // PublicStorage is specified in the Environ interface.
 func (env *localEnviron) PublicStorage() environs.StorageReader {
-	return localstorage.Client(env.sharedStorageListener.Addr().String())
+	return localstorage.Client(env.config.sharedStorageAddr())
 }
 
 // Destroy is specified in the Environ interface.
@@ -266,6 +349,24 @@
 	if !env.config.runningAsRoot {
 		return fmt.Errorf("destroying a local environment must be done as root")
 	}
+	// Kill all running instances.
+	containers, err := env.containerManager.ListContainers()
+	if err != nil {
+		return err
+	}
+	for _, inst := range containers {
+		if err := env.containerManager.StopContainer(inst); err != nil {
+			return err
+		}
+	}
+
+	logger.Infof("removing service %s", env.machineAgentServiceName())
+	machineAgent := upstart.NewService(env.machineAgentServiceName())
+	machineAgent.InitDir = upstartScriptLocation
+	if err := machineAgent.Remove(); err != nil {
+		logger.Errorf("could not remove machine agent service: %v", err)
+		return err
+	}
 
 	logger.Infof("removing service %s", env.mongoServiceName())
 	mongo := upstart.NewService(env.mongoServiceName())
@@ -354,7 +455,7 @@
 	}
 	// unpack the first tools into the agent dir.
 	tools := toolList[0]
-	logger.Infof("tools: %#v", tools)
+	logger.Debugf("tools: %#v", tools)
 	// brutally abuse our knowledge of storage to directly open the file
 	toolsUrl, err := url.Parse(tools.URL)
 	toolsLocation := filepath.Join(env.config.storageDir(), toolsUrl.Path)
@@ -381,6 +482,10 @@
 		toolsDir, dataDir, logDir, tag, machineId, logConfig, env.config.Type())
 	agent.Env["USER"] = env.config.user
 	agent.Env["HOME"] = os.Getenv("HOME")
+	agent.Env["JUJU_STORAGE_DIR"] = env.config.storageDir()
+	agent.Env["JUJU_STORAGE_ADDR"] = env.config.storageAddr()
+	agent.Env["JUJU_SHARED_STORAGE_DIR"] = env.config.sharedStorageDir()
+	agent.Env["JUJU_SHARED_STORAGE_ADDR"] = env.config.sharedStorageAddr()
 
 	agent.InitDir = upstartScriptLocation
 	logger.Infof("installing service %s to %s", env.machineAgentServiceName(), agent.InitDir)
@@ -391,20 +496,6 @@
 	return nil
 }
 
-func getAddressForInterface(interfaceName string) (string, error) {
-	bridge, err := net.InterfaceByName(interfaceName)
-	if err != nil {
-		logger.Errorf("cannot find network interface %q: %v", interfaceName, err)
-		return "", err
-	}
-	addrs, err := bridge.Addrs()
-	if err != nil {
-		logger.Errorf("cannot get addresses for network interface %q: %v", interfaceName, err)
-		return "", err
-	}
-	return utils.GetIPv4Address(addrs)
-}
-
 func (env *localEnviron) findBridgeAddress() (string, error) {
 	return getAddressForInterface(lxcBridgeName)
 }

=== modified file 'environs/local/environprovider.go'
--- environs/local/environprovider.go	2013-07-17 02:34:39 +0000
+++ environs/local/environprovider.go	2013-07-17 05:56:24 +0000
@@ -54,11 +54,11 @@
 	if err := config.Validate(cfg, old); err != nil {
 		return nil, err
 	}
-	v, err := configChecker.Coerce(cfg.UnknownAttrs(), nil)
+	validated, err := cfg.ValidateUnknownAttrs(configFields, configDefaults)
 	if err != nil {
 		return nil, err
 	}
-	localConfig := newEnvironConfig(cfg, v.(map[string]interface{}))
+	localConfig := newEnvironConfig(cfg, validated)
 	// Before potentially creating directories, make sure that the
 	// root directory has not changed.
 	if old != nil {
@@ -71,6 +71,16 @@
 				oldLocalConfig.rootDir(),
 				localConfig.rootDir())
 		}
+		if localConfig.storagePort() != oldLocalConfig.storagePort() {
+			return nil, fmt.Errorf("cannot change storage-port from %v to %v",
+				oldLocalConfig.storagePort(),
+				localConfig.storagePort())
+		}
+		if localConfig.sharedStoragePort() != oldLocalConfig.sharedStoragePort() {
+			return nil, fmt.Errorf("cannot change shared-storage-port from %v to %v",
+				oldLocalConfig.sharedStoragePort(),
+				localConfig.sharedStoragePort())
+		}
 	}
 	dir := utils.NormalizePath(localConfig.rootDir())
 	if dir == "." {
@@ -92,6 +102,12 @@
   # The default location is $JUJU_HOME/<ENV>.
   # $JUJU_HOME defaults to ~/.juju
   # root-dir: ~/.juju/local
+  # Override the storage port if you have multiple local providers, or if the
+  # default port is used by another program.
+  # storage-port: 8040
+  # Override the shared storage port if you have multiple local providers, or if the
+  # default port is used by another program.
+  # shared-storage-port: 8041
 
 `[1:]
 }
@@ -114,7 +130,8 @@
 
 // PrivateAddress implements environs.EnvironProvider.PrivateAddress.
 func (environProvider) PrivateAddress() (string, error) {
-	return "", fmt.Errorf("private address not implemented")
+	// Get the IPv4 address from eth0
+	return getAddressForInterface("eth0")
 }
 
 // InstanceId implements environs.EnvironProvider.InstanceId.

=== modified file 'environs/local/environprovider_test.go'
--- environs/local/environprovider_test.go	2013-07-16 03:28:34 +0000
+++ environs/local/environprovider_test.go	2013-07-17 05:56:24 +0000
@@ -7,21 +7,29 @@
 	gc "launchpad.net/gocheck"
 	"launchpad.net/loggo"
 
+	"launchpad.net/juju-core/container/lxc"
+	"launchpad.net/juju-core/environs/local"
 	"launchpad.net/juju-core/testing"
 )
 
 type baseProviderSuite struct {
 	testing.LoggingSuite
-	home *testing.FakeHome
+	lxc.TestSuite
+	home    *testing.FakeHome
+	restore func()
 }
 
 func (s *baseProviderSuite) SetUpTest(c *gc.C) {
 	s.LoggingSuite.SetUpTest(c)
+	s.TestSuite.SetUpTest(c)
 	s.home = testing.MakeFakeHomeNoEnvironments(c, "test")
 	loggo.GetLogger("juju.environs.local").SetLogLevel(loggo.TRACE)
+	s.restore = local.MockAddressForInterface()
 }
 
 func (s *baseProviderSuite) TearDownTest(c *gc.C) {
+	s.restore()
 	s.home.Restore()
+	s.TestSuite.TearDownTest(c)
 	s.LoggingSuite.TearDownTest(c)
 }

=== modified file 'environs/local/export_test.go'
--- environs/local/export_test.go	2013-07-16 21:56:22 +0000
+++ environs/local/export_test.go	2013-07-17 05:56:24 +0000
@@ -8,7 +8,10 @@
 	"launchpad.net/juju-core/environs/config"
 )
 
-var Provider = provider
+var (
+	Provider      = provider
+	SudoCallerIds = sudoCallerIds
+)
 
 // SetRootCheckFunction allows tests to override the check for a root user.
 // The return value is the function to restore the old value.
@@ -51,6 +54,14 @@
 	}
 }
 
-func SudoCallerIds() (uid, gid int, err error) {
-	return sudoCallerIds()
+// MockAddressForInterface replaces the getAddressForInterface with a function
+// that returns a constant localhost ip address.
+func MockAddressForInterface() func() {
+	getAddressForInterface = func(name string) (string, error) {
+		logger.Debugf("getAddressForInterface called for %s", name)
+		return "127.0.0.1", nil
+	}
+	return func() {
+		getAddressForInterface = getAddressForInterfaceImpl
+	}
 }

=== added file 'environs/local/net.go'
--- environs/local/net.go	1970-01-01 00:00:00 +0000
+++ environs/local/net.go	2013-07-17 05:56:24 +0000
@@ -0,0 +1,27 @@
+// Copyright 2013 Canonical Ltd.
+// Licensed under the AGPLv3, see LICENCE file for details.
+package local
+
+import (
+	"net"
+
+	"launchpad.net/juju-core/utils"
+)
+
+var getAddressForInterface = getAddressForInterfaceImpl
+
+// getAddressForInterfaceImpl looks for the network interface
+// and returns the IPv4 address from the possible addresses.
+func getAddressForInterfaceImpl(interfaceName string) (string, error) {
+	iface, err := net.InterfaceByName(interfaceName)
+	if err != nil {
+		logger.Errorf("cannot find network interface %q: %v", interfaceName, err)
+		return "", err
+	}
+	addrs, err := iface.Addrs()
+	if err != nil {
+		logger.Errorf("cannot get addresses for network interface %q: %v", interfaceName, err)
+		return "", err
+	}
+	return utils.GetIPv4Address(addrs)
+}

=== added directory 'environs/local/storage'
=== added file 'environs/local/storage/worker.go'
--- environs/local/storage/worker.go	1970-01-01 00:00:00 +0000
+++ environs/local/storage/worker.go	2013-07-17 05:56:24 +0000
@@ -0,0 +1,67 @@
+package storage
+
+import (
+	"os"
+
+	"launchpad.net/loggo"
+	"launchpad.net/tomb"
+
+	"launchpad.net/juju-core/environs/localstorage"
+	"launchpad.net/juju-core/worker"
+)
+
+var logger = loggo.GetLogger("juju.local.storage")
+
+type storageWorker struct {
+	tomb tomb.Tomb
+}
+
+func NewWorker() worker.Worker {
+	w := &storageWorker{}
+	go func() {
+		defer w.tomb.Done()
+		w.tomb.Kill(w.waitForDeath())
+	}()
+	return w
+}
+
+// Kill implements worker.Worker.Kill.
+func (s *storageWorker) Kill() {
+	s.tomb.Kill(nil)
+}
+
+// Wait implements worker.Worker.Wait.
+func (s *storageWorker) Wait() error {
+	return s.tomb.Wait()
+}
+
+func (s *storageWorker) waitForDeath() error {
+	storageDir := os.Getenv("JUJU_STORAGE_DIR")
+	storageAddr := os.Getenv("JUJU_STORAGE_ADDR")
+	logger.Infof("serving %s on %s", storageDir, storageAddr)
+
+	storageListener, err := localstorage.Serve(storageAddr, storageDir)
+	if err != nil {
+		logger.Errorf("error with local storage: %v", err)
+		return err
+	}
+	defer storageListener.Close()
+
+	sharedStorageDir := os.Getenv("JUJU_SHARED_STORAGE_DIR")
+	sharedStorageAddr := os.Getenv("JUJU_SHARED_STORAGE_ADDR")
+	logger.Infof("serving %s on %s", sharedStorageDir, sharedStorageAddr)
+
+	sharedStorageListener, err := localstorage.Serve(sharedStorageAddr, sharedStorageDir)
+	if err != nil {
+		logger.Errorf("error with local storage: %v", err)
+		return err
+	}
+	defer sharedStorageListener.Close()
+
+	logger.Infof("storage routines started, awaiting death")
+
+	<-s.tomb.Dying()
+
+	logger.Infof("dying, closing storage listeners")
+	return tomb.ErrDying
+}