Merge lp:~maas-maintainers/juju-core/maas-provider-skeleton into lp:~juju/juju-core/trunk

[Jeroen T. Vermeulen]

Feature branch integration: MAAS provider.

What you see here is not yet a complete implementation of the MAAS provider. It's an ongoing feature branch that we would like to integrate at this point, before the interfaces diverge further. We apologize for the long diff! We kept it separate so that you wouldn't have to review all the intermediate steps.

I also offer my apologies for submitting this for review in the usual Canonical style rather than going through lbox like the juju-core team does. I tried to submit through lbox but it seems to have problems with team-owned branches. We work against this branch as a team, and plan to go through the review collaboratively, so taking ownership was not the sensible thing to do. Pushing a personal copy (like lbox did at one point) seemed like a recipe for confusion.

We're on a very short schedule with a very long branch, so reviewers, we would like to ask you: please keep a careful distinction between things that absolutely need fixing before the code can land, and things that need fixing before the code can be used or maintained. Landing is the first priority, and we can track and resolve the other issues on an ongoing basis. Landing first will allow upcoming refactorings in the main codebase to take this new provider into account, and so save us all time.

As is the practice for other providers, we based much of this work on the EC2 provider. But MAAS is a different beast, and so we didn't copy as much code unchanged as you'd see in the OpenStack provider. With Bootstrap() in particular we had to simplify code in order to track what data goes where, and be sure it suited our needs. The same refactoring would work for the other providers, so we also posted it on the juju-dev mailing list.

A few more things you'll want to know before reading the code:
 * Instance IDs in the MAAS provider are implemented as node URIs on the MAAS API.
 * We require that each environment have its own MAAS user.
 * A MAAS provides file storage in separate per-user namespaces (there is no global namespace).

[William Reade (fwereade)]

> so we didn't copy as much code unchanged as you'd see in the OpenStack provider.

/me cheers, dances, gets down to reviewing

[William Reade (fwereade)]

Preliminary comments on the Environ...

Any points not badged with [FIX] should be considered usability/maintainability issues of varying importance.

1) _ "launchpad.net/juju-core/environs/maas"

Consider adding an environs/all package, that registers all providers except the dummy.

2) envCfg.attrs = v.(map[string]interface{})

Hmm... the attrs are all stored in the Config() anyway. Why are we duplicating them?

3) s/ecfgMutext/ecfgMutex/

4) quiesceStateFile [FIX if not necessary]

Only necessary if maas storage is not consistent.

5) uploadTools() [FIX: not necessary]

(Bootstrap no longer accepts uploadTools)

6) for a := longAttempt.Start(); len(stateAddrs) == 0 && a.Next(); { [FIX as (4)]

7) var err error = fmt.Errorf("(no error)")

I think this would be much clearer if we just break on err == nil inside the loop

8) StateInfo()

can't this use instances()?

9) environs.HighestVersion in StartInstance [FIX unless not unique]

This is wrong, but it might still match other providers.

10) environ.StopInstances([]environs.Instance{&instance})

Shouldn't this be a release?

[William Reade (fwereade)]

11) "maas-oauth": schema.String(),

This should probably be a custom schema.Checker

12) validCfg, err := prov.Validate(cfg, nil)

thumper's just landing some changes around this area: there's another method you need to call in Validate, IIRC, that checks old/new configs match sanely (no changing env names, for example).

13) "ca-private-key": testing.CAKey,

I'd have authorized-keys and admin-secret in here rather than duplicated in every test. While you're at it, please add `"agent-version": "1.2.3",` or something (it'll fit better with an upcoming branch of mine).

14) inst, err := env.startBootstrapNode(tools,

An upcoming branch will change this almost beyond recognition, but only for the better. BTW, is constraints support expected imminently? If not, please log.Warningf that they're ignored.

15) env.ecfgUnlocked = ecfg

There should be some validation that the two configs match, per thumper's branch. If we forgot it elsewhere, which we might have done, don't consider this a blocker.

16) environ.StopInstances([]environs.Instance{&instance})

upgrading (10) to a FIX

17) flags := environs.HighestVersion

crossgrading (9) to a changing-beyond-recognition

18) _, err := blah();\n if err != nil {

several places:

if _, err := blah(); err != nil {

19) client := environ.maasClientUnlocked.GetSubObject("nodes/") [FIX]

The intended model is that Environ methods be safe from any goroutine. Use a maasClient() helper throughout to get a client safely. (see `// To properly observe e.storageUnlocked `)

It may very well be the case that there's actually no point making Environs goroutine-safe, but I'm not going to analyse that now.

20) errSetConfig := env.SetConfig(cfg2)

Changing names is actively bad behaviour, please stick to sensible changes like default-series and authorized-keys :).

21) package maas

I don't really like having all these tests internal -- most of them should really go into maas_test, I think

22) func fakeWriteCertAndKey(

unnecessary?

23) === added file 'environs/maas/environprovider.go'

(and similar small files with clear purposes) yay! I like this, we should do it more ;)

24) func createTempFile(c *C

create it inside a c.MkDir() and it'll be cleaned up for you

25) info.serializeYAML()

Please use goyaml.Marshal and export/tag the struct fields where necessary

~~~~~~~~~~~~~~

Thanks, and sorry for the delay. This is looking really solid so far; if it's actually spinning up environments and deploying units now, then I'd like to get it in as soon as possible and starting working on rationalizing the testing then. I have no problem with the coverage, just of where the tests are (and that none of the jujutest tests have been hooked).

review: Approve

[Jeroen T. Vermeulen (jtv)]

On point 12, thumper explains:

The new method is environs.config.Validate(). Our Validate() should upcall to that, so that it can do its basic non-specific verification of the proposed config changes. See ec2/config.go, and similar for the openstack and dummy providers.

[Jeroen T. Vermeulen (jtv)]

A note on point 15:

«15) env.ecfgUnlocked = ecfg

«There should be some validation that the two configs match, per thumper's branch. If we forgot it elsewhere, which we might have done, don't consider this a blocker.»

The preceding call to maasEnvironProvider.newConfig() calls Validate(), which in turn will validate the state transition. So that should do the trick, right?

[Jeroen T. Vermeulen (jtv)]

A note on points 10 & 16:

«10) environ.StopInstances([]environs.Instance{&instance})

«Shouldn't this be a release?»

«16) environ.StopInstances([]environs.Instance{&instance})

«upgrading (10) to a FIX»

It may just be me but I have no idea at all what you mean! How is a release related to the action of stopping instances? I can't find anything in the other providers that might make it clearer either.

[Jeroen T. Vermeulen (jtv)]

Note about points 9 and 17:

«9) environs.HighestVersion in StartInstance [FIX unless not unique]

«This is wrong, but it might still match other providers.»

«17) flags := environs.HighestVersion

«crossgrading (9) to a changing-beyond-recognition»

It's still the same in the EC2 provider, so leaving this be for now.

[Raphaël Badin (rvb)]

> Thanks, and sorry for the delay. This is looking really solid so far; if it's actually spinning up environments and
> deploying units now, then I'd like to get it in as soon as possible and starting working on rationalizing the testing
> then. I have no problem with the coverage, just of where the tests are (and that none of the jujutest tests have been
> hooked).

Thanks a lot for the reviews. We're in the process of fixing the issues you mentioned in your reviews but yes, we've been able to deploy a working mediawiki service in the MAAS lab today: http://paste.ubuntu.com/5697858/

[Raphaël Badin (rvb)]

> 8) StateInfo()
>
> can't this use instances()?

Do you mean you'd prefer StateInfo() to use the internal method env.instances() instead of env.Instances()?

[William Reade (fwereade)]

26) forgot to mention this: env.Provider().(*maasEnvironProvider)

just use providerInstance?

27) var _ environs.EnvironProvider = (*maasEnvironProvider)(nil)

no fields, no pointer receivers -- you may as well assert that the value implements EnvironProvider, use the value everywhere, and just forget about the &s and *s.

[William Reade (fwereade)]

> A note on point 15:
>
> «15) env.ecfgUnlocked = ecfg
>
> «There should be some validation that the two configs match, per thumper's
> branch. If we forgot it elsewhere, which we might have done, don't consider
> this a blocker.»
>
> The preceding call to maasEnvironProvider.newConfig() calls Validate(), which
> in turn will validate the state transition. So that should do the trick,
> right?

I don't think so... ISTM that we would validate against nil, rather than against the old config.

[William Reade (fwereade)]

> A note on points 10 & 16:
>
> «10) environ.StopInstances([]environs.Instance{&instance})
>
> «Shouldn't this be a release?»
>
> «16) environ.StopInstances([]environs.Instance{&instance})
>
> «upgrading (10) to a FIX»
>
> It may just be me but I have no idea at all what you mean! How is a release
> related to the action of stopping instances? I can't find anything in the
> other providers that might make it clearer either.

Ah, sorry -- I'm saying that if start has failed we should certainly release the nodes back to the pool if we can, but it's confusing to stop something that was never actually started.

[William Reade (fwereade)]

> Note about points 9 and 17:
>
> «9) environs.HighestVersion in StartInstance [FIX unless not unique]
>
> «This is wrong, but it might still match other providers.»
>
> «17) flags := environs.HighestVersion
>
> «crossgrading (9) to a changing-beyond-recognition»
>
> It's still the same in the EC2 provider, so leaving this be for now.

It'll be a race to land then, still a WIP on my end. If you win, I'm pretty sure it'll be np for me; if I win it should be simple for you but I'm always here to discuss if there are complications.

[William Reade (fwereade)]

> > 8) StateInfo()
> >
> > can't this use instances()?
>
> Do you mean you'd prefer StateInfo() to use the internal method
> env.instances() instead of env.Instances()?

It was really just a question, there may be some detail that makes it inappropriate, but at first blush it seemed like the ideal candidate. Your judgment call.

[Jeroen T. Vermeulen (jtv)]

I don't think instances() would make a good replacement for Instances(). It's basically the same thing, except instances() has a quirk to its API that made it suit the implementation of multiple exported functions. Instances() says what we want more precisely and with less risk of accident.

[Jeroen T. Vermeulen (jtv)]

More notes! On point #2 this time:

«2) envCfg.attrs = v.(map[string]interface{})

«Hmm... the attrs are all stored in the Config() anyway. Why are we duplicating them?»

Is there any more convenient access path (from outside the config package) than through configchecker.Coerce(cfg.UnknownAttrs(), nil)? If we skip the Coerce(), my understanding is that the variables won't be appropriately typed. Not an issue now, but very possibly problematic in the future. And UnknownAttrs() tediously duplicates attributes, so we might as well keep the result around.

Actually we're just doing the same thing as the EC2 provider here, only more explicitly — so maybe this was always an issue but it went unnoticed before. Here's how the EC2 code sets the equivalent "attrs" field:

        ecfg := &environConfig{cfg, v.(map[string]interface{})}

(And "v" is produced in the same way that we do it.)

Does this make sense? Or is there still a change that needs to be made here?

[Julian Edwards (julian-edwards)]

review: Approve

[William Reade (fwereade)]

> Actually we're just doing the same thing as the EC2 provider here, only more explicitly — so maybe this was always an issue but it went unnoticed before. Here's how the EC2 code sets the equivalent "attrs" field:
>
> ecfg := &environConfig{cfg, v.(map[string]interface{})}
>
> (And "v" is produced in the same way that we do it.)
>
> Does this make sense? Or is there still a change that needs to be made here?

I think it's probably OK. Which is to say that environment configuration remains hairy and problematic and weird, because it's trying to be at *least* 3 different things, all of which have different sets of valid keys and values in various situations, but it's not very amenable to fixing at the moment.

=== modified file 'cmd/builddb/main.go'
--- cmd/builddb/main.go	2013-04-11 15:46:31 +0000
+++ cmd/builddb/main.go	2013-04-12 10:00:43 +0000
@@ -11,9 +11,11 @@
 	"os"
 	"path/filepath"
 	"time"
+)
 
-	// Register the provider
-	_ "launchpad.net/juju-core/environs/ec2"
+// Import the providers.
+import (
+	_ "launchpad.net/juju-core/environs/all"
 )
 
 func main() {
 
=== modified file 'cmd/juju/main.go'
--- cmd/juju/main.go	2013-04-09 22:56:52 +0000
+++ cmd/juju/main.go	2013-04-12 10:00:43 +0000
@@ -7,12 +7,9 @@
 	"os"
 )
 
-// When we import an environment provider implementation
-// here, it will register itself with environs, and hence
-// be available to the juju command.
+// Import the providers.
 import (
-	_ "launchpad.net/juju-core/environs/ec2"
-	_ "launchpad.net/juju-core/environs/openstack"
+	_ "launchpad.net/juju-core/environs/all"
 )
 
 var jujuDoc = `
 
=== modified file 'cmd/jujud/main.go'
--- cmd/jujud/main.go	2013-03-26 20:17:23 +0000
+++ cmd/jujud/main.go	2013-04-12 10:00:43 +0000
@@ -9,11 +9,9 @@
 	"path/filepath"
 )
 
-// When we import an environment provider implementation
-// here, it will register itself with environs.
+// Import the providers.
 import (
-	_ "launchpad.net/juju-core/environs/ec2"
-	_ "launchpad.net/juju-core/environs/openstack"
+	_ "launchpad.net/juju-core/environs/all"
 )
 
 var jujudDoc = `
 
=== added directory 'environs/all'
=== added file 'environs/all/all.go'
--- environs/all/all.go	1970-01-01 00:00:00 +0000
+++ environs/all/all.go	2013-04-12 10:00:43 +0000
@@ -0,0 +1,8 @@
+package all
+
+// Register all the available providers.
+import (
+	_ "launchpad.net/juju-core/environs/ec2"
+	_ "launchpad.net/juju-core/environs/maas"
+	_ "launchpad.net/juju-core/environs/openstack"
+)
 
=== modified file 'environs/cloudinit/cloudinit.go'
--- environs/cloudinit/cloudinit.go	2013-04-11 02:13:40 +0000
+++ environs/cloudinit/cloudinit.go	2013-04-12 10:00:43 +0000
@@ -98,11 +98,14 @@
 }
 
 func New(cfg *MachineConfig) (*cloudinit.Config, error) {
+	c := cloudinit.New()
+	return Configure(cfg, c)
+}
+
+func Configure(cfg *MachineConfig, c *cloudinit.Config) (*cloudinit.Config, error) {
 	if err := verifyConfig(cfg); err != nil {
 		return nil, err
 	}
-	c := cloudinit.New()
-
 	c.AddSSHAuthorizedKeys(cfg.AuthorizedKeys)
 	c.AddPackage("git")
 
 
=== modified file 'environs/cloudinit/cloudinit_test.go'
--- environs/cloudinit/cloudinit_test.go	2013-04-11 07:54:47 +0000
+++ environs/cloudinit/cloudinit_test.go	2013-04-12 10:00:43 +0000
@@ -4,6 +4,7 @@
 	"encoding/base64"
 	. "launchpad.net/gocheck"
 	"launchpad.net/goyaml"
+	cloudinit_core "launchpad.net/juju-core/cloudinit"
 	"launchpad.net/juju-core/constraints"
 	"launchpad.net/juju-core/environs/cloudinit"
 	"launchpad.net/juju-core/environs/config"
@@ -264,6 +265,38 @@
 	}
 }
 
+func (*cloudinitSuite) TestCloudInitConfigure(c *C) {
+	for i, test := range cloudinitTests {
+		test.cfg.Config = minimalConfig(c)
+		c.Logf("test %d (Configure)", i)
+		cloudcfg := cloudinit_core.New()
+		ci, err := cloudinit.Configure(&test.cfg, cloudcfg)
+		c.Assert(err, IsNil)
+		c.Check(ci, NotNil)
+	}
+}
+
+func (*cloudinitSuite) TestCloudInitConfigureUsesGivenConfig(c *C) {
+	// Create a simple cloudinit config with a 'runcmd' statement.
+	cloudcfg := cloudinit_core.New()
+	script := "test script"
+	cloudcfg.AddRunCmd(script)
+	cloudinitTests[0].cfg.Config = minimalConfig(c)
+	ci, err := cloudinit.Configure(&cloudinitTests[0].cfg, cloudcfg)
+	c.Assert(err, IsNil)
+	c.Check(ci, NotNil)
+	data, err := ci.Render()
+	c.Assert(err, IsNil)
+
+	ciContent := make(map[interface{}]interface{})
+	err = goyaml.Unmarshal(data, &ciContent)
+	c.Assert(err, IsNil)
+	// The 'runcmd' statement is at the beginning of the list
+	// of 'runcmd' statements.
+	runCmd := ciContent["runcmd"].([]interface{})
+	c.Check(runCmd[0], Equals, script)
+}
+
 func getScripts(x map[interface{}]interface{}) []string {
 	var scripts []string
 	for _, s := range x["runcmd"].([]interface{}) {
 
=== added directory 'environs/maas'
=== added file 'environs/maas/config.go'
--- environs/maas/config.go	1970-01-01 00:00:00 +0000
+++ environs/maas/config.go	2013-04-12 10:00:43 +0000
@@ -0,0 +1,72 @@
+package maas
+
+import (
+	"errors"
+	"fmt"
+	"launchpad.net/juju-core/environs/config"
+	"launchpad.net/juju-core/schema"
+	"net/url"
+	"strings"
+)
+
+var maasConfigChecker = schema.StrictFieldMap(
+	schema.Fields{
+		"maas-server": schema.String(),
+		// maas-oauth is a colon-separated triplet of:
+		// consumer-key:resource-token:resource-secret
+		"maas-oauth": schema.String(),
+	},
+	schema.Defaults{},
+)
+
+type maasEnvironConfig struct {
+	*config.Config
+	attrs map[string]interface{}
+}
+
+func (cfg *maasEnvironConfig) MAASServer() string {
+	return cfg.attrs["maas-server"].(string)
+}
+
+func (cfg *maasEnvironConfig) MAASOAuth() string {
+	return cfg.attrs["maas-oauth"].(string)
+}
+
+func (prov maasEnvironProvider) newConfig(cfg *config.Config) (*maasEnvironConfig, error) {
+	validCfg, err := prov.Validate(cfg, nil)
+	if err != nil {
+		return nil, err
+	}
+	result := new(maasEnvironConfig)
+	result.Config = validCfg
+	result.attrs = validCfg.UnknownAttrs()
+	return result, nil
+}
+
+var errMalformedMaasOAuth = errors.New("malformed maas-oauth (3 items separated by colons)")
+
+func (prov maasEnvironProvider) Validate(cfg, oldCfg *config.Config) (*config.Config, error) {
+	// Validate base configuration change before validating MAAS specifics.
+	err := config.Validate(cfg, oldCfg)
+	if err != nil {
+		return nil, err
+	}
+
+	v, err := maasConfigChecker.Coerce(cfg.UnknownAttrs(), nil)
+	if err != nil {
+		return nil, err
+	}
+	envCfg := new(maasEnvironConfig)
+	envCfg.Config = cfg
+	envCfg.attrs = v.(map[string]interface{})
+	server := envCfg.MAASServer()
+	serverURL, err := url.Parse(server)
+	if err != nil || serverURL.Scheme == "" || serverURL.Host == "" {
+		return nil, fmt.Errorf("malformed maas-server URL '%v': %s", server, err)
+	}
+	oauth := envCfg.MAASOAuth()
+	if strings.Count(oauth, ":") != 2 {
+		return nil, errMalformedMaasOAuth
+	}
+	return cfg.Apply(envCfg.attrs)
+}
 
=== added file 'environs/maas/config_test.go'
--- environs/maas/config_test.go	1970-01-01 00:00:00 +0000
+++ environs/maas/config_test.go	2013-04-12 10:00:43 +0000
@@ -0,0 +1,94 @@
+package maas
+
+import (
+	. "launchpad.net/gocheck"
+	"launchpad.net/juju-core/environs"
+	"launchpad.net/juju-core/testing"
+	"launchpad.net/juju-core/version"
+)
+
+type ConfigSuite struct{}
+
+var _ = Suite(new(ConfigSuite))
+
+// copyAttrs copies values from src into dest.  If src contains a key that was
+// already in dest, its value in dest will still be updated to the one from
+// src.
+func copyAttrs(src, dest map[string]interface{}) {
+	for k, v := range src {
+		dest[k] = v
+	}
+}
+
+// newConfig creates a MAAS environment config from attributes.
+func newConfig(values map[string]interface{}) (*maasEnvironConfig, error) {
+	defaults := map[string]interface{}{
+		"name":            "testenv",
+		"type":            "maas",
+		"admin-secret":    "ssshhhhhh",
+		"authorized-keys": "I-am-not-a-real-key",
+		"agent-version":   version.CurrentNumber().String(),
+		// These are not needed by MAAS, but juju-core breaks without them. Needs
+		// fixing there.
+		"ca-cert":        testing.CACert,
+		"ca-private-key": testing.CAKey,
+	}
+	cfg := make(map[string]interface{})
+	copyAttrs(defaults, cfg)
+	copyAttrs(values, cfg)
+	env, err := environs.NewFromAttrs(cfg)
+	if err != nil {
+		return nil, err
+	}
+	return env.(*maasEnviron).ecfg(), nil
+}
+
+func (ConfigSuite) TestParsesMAASSettings(c *C) {
+	server := "http://maas.example.com/maas/"
+	oauth := "consumer-key:resource-token:resource-secret"
+	ecfg, err := newConfig(map[string]interface{}{
+		"maas-server": server,
+		"maas-oauth":  oauth,
+	})
+	c.Assert(err, IsNil)
+	c.Check(ecfg.MAASServer(), Equals, server)
+	c.Check(ecfg.MAASOAuth(), DeepEquals, oauth)
+}
+
+func (ConfigSuite) TestChecksWellFormedMaasServer(c *C) {
+	_, err := newConfig(map[string]interface{}{
+		"maas-server": "This should have been a URL.",
+		"maas-oauth":  "consumer-key:resource-token:resource-secret",
+	})
+	c.Assert(err, NotNil)
+	c.Check(err, ErrorMatches, ".*malformed maas-server.*")
+}
+
+func (ConfigSuite) TestChecksWellFormedMaasOAuth(c *C) {
+	_, err := newConfig(map[string]interface{}{
+		"maas-server": "http://maas.example.com/maas/",
+		"maas-oauth":  "This should have been a 3-part token.",
+	})
+	c.Assert(err, NotNil)
+	c.Check(err, ErrorMatches, ".*malformed maas-oauth.*")
+}
+
+func (ConfigSuite) TestValidateUpcallsEnvironsConfigValidate(c *C) {
+	// The base Validate() function will not allow an environment to
+	// change its name.  Trigger that error so as to prove that the
+	// environment provider's Validate() calls the base Validate().
+	baseAttrs := map[string]interface{}{
+		"maas-server": "http://maas.example.com/maas/",
+		"maas-oauth":  "consumer-key:resource-token:resource-secret",
+	}
+	oldCfg, err := newConfig(baseAttrs)
+	c.Assert(err, IsNil)
+	newName := oldCfg.Name() + "-but-different"
+	newCfg, err := oldCfg.Apply(map[string]interface{}{"name": newName})
+	c.Assert(err, IsNil)
+
+	_, err = maasEnvironProvider{}.Validate(newCfg, oldCfg.Config)
+
+	c.Assert(err, NotNil)
+	c.Check(err, ErrorMatches, ".*cannot change name.*")
+}
 
=== added file 'environs/maas/environ.go'
--- environs/maas/environ.go	1970-01-01 00:00:00 +0000
+++ environs/maas/environ.go	2013-04-12 10:00:43 +0000
@@ -0,0 +1,519 @@
+package maas
+
+import (
+	"encoding/base64"
+	"errors"
+	"fmt"
+	"launchpad.net/gomaasapi"
+	"launchpad.net/juju-core/constraints"
+	"launchpad.net/juju-core/environs"
+	"launchpad.net/juju-core/environs/cloudinit"
+	"launchpad.net/juju-core/environs/config"
+	"launchpad.net/juju-core/log"
+	"launchpad.net/juju-core/state"
+	"launchpad.net/juju-core/state/api"
+	"launchpad.net/juju-core/state/api/params"
+	"launchpad.net/juju-core/trivial"
+	"launchpad.net/juju-core/version"
+	"net/url"
+	"sync"
+	"time"
+)
+
+const (
+	mgoPort     = 37017
+	apiPort     = 17070
+	jujuDataDir = "/var/lib/juju"
+	// We're using v1.0 of the MAAS API.
+	apiVersion = "1.0"
+)
+
+var mgoPortSuffix = fmt.Sprintf(":%d", mgoPort)
+var apiPortSuffix = fmt.Sprintf(":%d", apiPort)
+
+var longAttempt = trivial.AttemptStrategy{
+	Total: 3 * time.Minute,
+	Delay: 1 * time.Second,
+}
+
+type maasEnviron struct {
+	name string
+
+	// ecfgMutex protects the *Unlocked fields below.
+	ecfgMutex sync.Mutex
+
+	ecfgUnlocked       *maasEnvironConfig
+	maasClientUnlocked *gomaasapi.MAASObject
+	storageUnlocked    environs.Storage
+}
+
+var _ environs.Environ = (*maasEnviron)(nil)
+
+var couldNotAllocate = errors.New("Could not allocate MAAS environment object.")
+
+func NewEnviron(cfg *config.Config) (*maasEnviron, error) {
+	env := new(maasEnviron)
+	if env == nil {
+		return nil, couldNotAllocate
+	}
+	err := env.SetConfig(cfg)
+	if err != nil {
+		return nil, err
+	}
+	env.storageUnlocked = NewStorage(env)
+	return env, nil
+}
+
+func (env *maasEnviron) Name() string {
+	return env.name
+}
+
+// TODO: this code is cargo-culted from the openstack/ec2 providers.
+func (env *maasEnviron) findTools() (*state.Tools, error) {
+	flags := environs.HighestVersion | environs.CompatVersion
+	v := version.Current
+	v.Series = env.Config().DefaultSeries()
+	return environs.FindTools(env, v, flags)
+}
+
+// makeMachineConfig sets up a basic machine configuration for use with
+// userData().  You may still need to supply more information, but this takes
+// care of the fixed entries and the ones that are always needed.
+func (env *maasEnviron) makeMachineConfig(machineID, machineNonce string, stateInfo *state.Info, apiInfo *api.Info, tools *state.Tools) *cloudinit.MachineConfig {
+	return &cloudinit.MachineConfig{
+		// Fixed entries.
+		MongoPort: mgoPort,
+		APIPort:   apiPort,
+		DataDir:   jujuDataDir,
+
+		// Entries based purely on what's in the environment.
+		AuthorizedKeys: env.ecfg().AuthorizedKeys(),
+
+		// Parameter entries.
+		MachineId:    machineID,
+		MachineNonce: machineNonce,
+		StateInfo:    stateInfo,
+		APIInfo:      apiInfo,
+		Tools:        tools,
+	}
+}
+
+// startBootstrapNode starts the juju bootstrap node for this environment.
+func (env *maasEnviron) startBootstrapNode(tools *state.Tools, cert, key []byte, password string) (environs.Instance, error) {
+	config, err := environs.BootstrapConfig(env.Provider(), env.Config(), tools)
+	if err != nil {
+		return nil, fmt.Errorf("unable to determine initial configuration: %v", err)
+	}
+	caCert, hasCert := env.Config().CACert()
+	if !hasCert {
+		return nil, fmt.Errorf("no CA certificate in environment configuration")
+	}
+	stateInfo := state.Info{
+		Password: trivial.PasswordHash(password),
+		CACert:   caCert,
+	}
+	apiInfo := api.Info{
+		Password: trivial.PasswordHash(password),
+		CACert:   caCert,
+	}
+
+	// The bootstrap instance gets machine id "0".  This is not related to
+	// instance ids or MAAS system ids.  Juju assigns the machine ID.
+	const machineID = "0"
+
+	mcfg := env.makeMachineConfig(machineID, state.BootstrapNonce, &stateInfo, &apiInfo, tools)
+	mcfg.StateServer = true
+	mcfg.StateServerCert = cert
+	mcfg.StateServerKey = key
+	mcfg.Config = config
+
+	inst, err := env.obtainNode(machineID, &stateInfo, &apiInfo, tools, mcfg)
+	if err != nil {
+		return nil, fmt.Errorf("cannot start bootstrap instance: %v", err)
+	}
+	return inst, nil
+}
+
+// Bootstrap is specified in the Environ interface.
+func (env *maasEnviron) Bootstrap(cons constraints.Value, stateServerCert, stateServerKey []byte) error {
+	constraints := cons.String()
+	if constraints != "" {
+		log.Warningf("ignoring constraints '%s' (not implemented)", constraints)
+	}
+
+	// This was all cargo-culted from the EC2 provider.
+	password := env.Config().AdminSecret()
+	if password == "" {
+		return fmt.Errorf("admin-secret is required for bootstrap")
+	}
+	log.Debugf("environs/maas: bootstrapping environment %q.", env.Name())
+	tools, err := env.findTools()
+	if err != nil {
+		return err
+	}
+	inst, err := env.startBootstrapNode(tools, stateServerCert, stateServerKey, password)
+	if err != nil {
+		return err
+	}
+	err = env.saveState(&bootstrapState{StateInstances: []state.InstanceId{inst.Id()}})
+	if err != nil {
+		env.releaseInstance(inst)
+		return fmt.Errorf("cannot save state: %v", err)
+	}
+
+	// TODO make safe in the case of racing Bootstraps
+	// If two Bootstraps are called concurrently, there's
+	// no way to make sure that only one succeeds.
+
+	return nil
+}
+
+// StateInfo is specified in the Environ interface.
+func (env *maasEnviron) StateInfo() (*state.Info, *api.Info, error) {
+	// This code is cargo-culted from the openstack/ec2 providers.
+	// It's a bit unclear what the "longAttempt" loop is actually for
+	// but this should probably be refactored outside of the provider
+	// code.
+	st, err := env.loadState()
+	if err != nil {
+		return nil, nil, err
+	}
+	cert, hasCert := env.Config().CACert()
+	if !hasCert {
+		return nil, nil, fmt.Errorf("no CA certificate in environment configuration")
+	}
+	var stateAddrs []string
+	var apiAddrs []string
+	// Wait for the DNS names of any of the instances
+	// to become available.
+	log.Debugf("environs/maas: waiting for DNS name(s) of state server instances %v", st.StateInstances)
+	for a := longAttempt.Start(); len(stateAddrs) == 0 && a.Next(); {
+		insts, err := env.Instances(st.StateInstances)
+		if err != nil && err != environs.ErrPartialInstances {
+			log.Debugf("error getting state instance: %v", err.Error())
+			return nil, nil, err
+		}
+		log.Debugf("started processing instances: %#v", insts)
+		for _, inst := range insts {
+			if inst == nil {
+				continue
+			}
+			name, err := inst.DNSName()
+			if err != nil {
+				continue
+			}
+			if name != "" {
+				stateAddrs = append(stateAddrs, name+mgoPortSuffix)
+				apiAddrs = append(apiAddrs, name+apiPortSuffix)
+			}
+		}
+	}
+	if len(stateAddrs) == 0 {
+		return nil, nil, fmt.Errorf("timed out waiting for mgo address from %v", st.StateInstances)
+	}
+	return &state.Info{
+			Addrs:  stateAddrs,
+			CACert: cert,
+		}, &api.Info{
+			Addrs:  apiAddrs,
+			CACert: cert,
+		}, nil
+}
+
+// ecfg returns the environment's maasEnvironConfig, and protects it with a
+// mutex.
+func (env *maasEnviron) ecfg() *maasEnvironConfig {
+	env.ecfgMutex.Lock()
+	defer env.ecfgMutex.Unlock()
+	return env.ecfgUnlocked
+}
+
+// Config is specified in the Environ interface.
+func (env *maasEnviron) Config() *config.Config {
+	return env.ecfg().Config
+}
+
+// SetConfig is specified in the Environ interface.
+func (env *maasEnviron) SetConfig(cfg *config.Config) error {
+	env.ecfgMutex.Lock()
+	defer env.ecfgMutex.Unlock()
+
+	// The new config has already been validated by itself, but now we
+	// validate the transition from the old config to the new.
+	var oldCfg *config.Config
+	if env.ecfgUnlocked != nil {
+		oldCfg = env.ecfgUnlocked.Config
+	}
+	cfg, err := env.Provider().Validate(cfg, oldCfg)
+	if err != nil {
+		return err
+	}
+
+	ecfg, err := providerInstance.newConfig(cfg)
+	if err != nil {
+		return err
+	}
+
+	env.name = cfg.Name()
+	env.ecfgUnlocked = ecfg
+
+	authClient, err := gomaasapi.NewAuthenticatedClient(ecfg.MAASServer(), ecfg.MAASOAuth(), apiVersion)
+	if err != nil {
+		return err
+	}
+	env.maasClientUnlocked = gomaasapi.NewMAAS(*authClient)
+
+	return nil
+}
+
+// getMAASClient returns a MAAS client object to use for a request, in a
+// lock-protected fashioon.
+func (env *maasEnviron) getMAASClient() *gomaasapi.MAASObject {
+	env.ecfgMutex.Lock()
+	defer env.ecfgMutex.Unlock()
+
+	return env.maasClientUnlocked
+}
+
+// acquireNode allocates a node from the MAAS.
+func (environ *maasEnviron) acquireNode() (gomaasapi.MAASObject, error) {
+	retry := trivial.AttemptStrategy{
+		Total: 5 * time.Second,
+		Delay: 200 * time.Millisecond,
+	}
+	var result gomaasapi.JSONObject
+	var err error
+	for a := retry.Start(); a.Next(); {
+		client := environ.getMAASClient().GetSubObject("nodes/")
+		result, err = client.CallPost("acquire", nil)
+		if err == nil {
+			break
+		}
+	}
+	if err != nil {
+		return gomaasapi.MAASObject{}, err
+	}
+	node, err := result.GetMAASObject()
+	if err != nil {
+		msg := fmt.Errorf("unexpected result from 'acquire' on MAAS API: %v", err)
+		return gomaasapi.MAASObject{}, msg
+	}
+	return node, nil
+}
+
+// startNode installs and boots a node.
+func (environ *maasEnviron) startNode(node gomaasapi.MAASObject, tools *state.Tools, userdata []byte) error {
+	retry := trivial.AttemptStrategy{
+		Total: 5 * time.Second,
+		Delay: 200 * time.Millisecond,
+	}
+	userDataParam := base64.StdEncoding.EncodeToString(userdata)
+	params := url.Values{
+		"distro_series": {tools.Series},
+		"user_data":     {userDataParam},
+	}
+	// Initialize err to a non-nil value as a sentinel for the following
+	// loop.
+	err := fmt.Errorf("(no error)")
+	for a := retry.Start(); a.Next() && err != nil; {
+		_, err = node.CallPost("start", params)
+	}
+	return err
+}
+
+// obtainNode allocates and starts a MAAS node.  It is used both for the
+// implementation of StartInstance, and to initialize the bootstrap node.
+func (environ *maasEnviron) obtainNode(machineId string, stateInfo *state.Info, apiInfo *api.Info, tools *state.Tools, mcfg *cloudinit.MachineConfig) (*maasInstance, error) {
+
+	log.Debugf("environs/maas: starting machine %s in $q running tools version %q from %q", machineId, environ.name, tools.Binary, tools.URL)
+
+	node, err := environ.acquireNode()
+	if err != nil {
+		return nil, fmt.Errorf("cannot run instances: %v", err)
+	}
+
+	hostname, err := node.GetField("hostname")
+	if err != nil {
+		return nil, err
+	}
+	instance := maasInstance{&node, environ}
+	info := machineInfo{string(instance.Id()), hostname}
+	runCmd, err := info.cloudinitRunCmd()
+	if err != nil {
+		return nil, err
+	}
+	userdata, err := userData(mcfg, runCmd)
+	if err != nil {
+		msg := fmt.Errorf("could not compose userdata for bootstrap node: %v", err)
+		return nil, msg
+	}
+	err = environ.startNode(node, tools, userdata)
+	if err != nil {
+		environ.releaseInstance(&instance)
+		return nil, fmt.Errorf("cannot start instance: %v", err)
+	}
+	log.Debugf("environs/maas: started instance %q", instance.Id())
+	return &instance, nil
+}
+
+// StartInstance is specified in the Environ interface.
+func (environ *maasEnviron) StartInstance(machineID, machineNonce string, series string, cons constraints.Value, stateInfo *state.Info, apiInfo *api.Info) (environs.Instance, error) {
+	// TODO: Support series and constraints.  They were added to the
+	// interface after we implemented.
+	flags := environs.HighestVersion | environs.CompatVersion
+	var err error
+	tools, err := environs.FindTools(environ, version.Current, flags)
+	if err != nil {
+		return nil, err
+	}
+
+	mcfg := environ.makeMachineConfig(machineID, machineNonce, stateInfo, apiInfo, tools)
+	return environ.obtainNode(machineID, stateInfo, apiInfo, tools, mcfg)
+}
+
+// StopInstances is specified in the Environ interface.
+func (environ *maasEnviron) StopInstances(instances []environs.Instance) error {
+	// Shortcut to exit quickly if 'instances' is an empty slice or nil.
+	if len(instances) == 0 {
+		return nil
+	}
+	// Tell MAAS to release each of the instances.  If there are errors,
+	// return only the first one (but release all instances regardless).
+	// Note that releasing instances also turns them off.
+	var firstErr error
+	for _, instance := range instances {
+		err := environ.releaseInstance(instance)
+		if firstErr == nil {
+			firstErr = err
+		}
+	}
+	return firstErr
+}
+
+// releaseInstance releases a single instance.
+func (environ *maasEnviron) releaseInstance(inst environs.Instance) error {
+	maasInst := inst.(*maasInstance)
+	maasObj := maasInst.maasObject
+	_, err := maasObj.CallPost("release", nil)
+	if err != nil {
+		log.Debugf("environs/maas: error releasing instance %v", maasInst)
+	}
+	return err
+}
+
+// Instances returns the environs.Instance objects corresponding to the given
+// slice of state.InstanceId.  Similar to what the ec2 provider does,
+// Instances returns nil if the given slice is empty or nil.
+func (environ *maasEnviron) Instances(ids []state.InstanceId) ([]environs.Instance, error) {
+	if len(ids) == 0 {
+		return nil, nil
+	}
+	return environ.instances(ids)
+}
+
+// instances is an internal method which returns the instances matching the
+// given instance ids or all the instances if 'ids' is empty.
+// If the some of the intances could not be found, it returns the instance
+// that could be found plus the error environs.ErrPartialInstances in the error
+// return.
+func (environ *maasEnviron) instances(ids []state.InstanceId) ([]environs.Instance, error) {
+	nodeListing := environ.getMAASClient().GetSubObject("nodes")
+	filter := getSystemIdValues(ids)
+	listNodeObjects, err := nodeListing.CallGet("list", filter)
+	if err != nil {
+		return nil, err
+	}
+	listNodes, err := listNodeObjects.GetArray()
+	if err != nil {
+		return nil, err
+	}
+	instances := make([]environs.Instance, len(listNodes))
+	for index, nodeObj := range listNodes {
+		node, err := nodeObj.GetMAASObject()
+		if err != nil {
+			return nil, err
+		}
+		instances[index] = &maasInstance{
+			maasObject: &node,
+			environ:    environ,
+		}
+	}
+	if len(ids) != 0 && len(ids) != len(instances) {
+		return instances, environs.ErrPartialInstances
+	}
+	return instances, nil
+}
+
+// AllInstances returns all the environs.Instance in this provider.
+func (environ *maasEnviron) AllInstances() ([]environs.Instance, error) {
+	return environ.instances(nil)
+}
+
+// Storage is defined by the Environ interface.
+func (env *maasEnviron) Storage() environs.Storage {
+	env.ecfgMutex.Lock()
+	defer env.ecfgMutex.Unlock()
+	return env.storageUnlocked
+}
+
+// PublicStorage is defined by the Environ interface.
+func (env *maasEnviron) PublicStorage() environs.StorageReader {
+	// MAAS does not have a shared storage.
+	return environs.EmptyStorage
+}
+
+func (environ *maasEnviron) Destroy(ensureInsts []environs.Instance) error {
+	log.Debugf("environs/maas: destroying environment %q", environ.name)
+	insts, err := environ.AllInstances()
+	if err != nil {
+		return fmt.Errorf("cannot get instances: %v", err)
+	}
+	found := make(map[state.InstanceId]bool)
+	for _, inst := range insts {
+		found[inst.Id()] = true
+	}
+
+	// Add any instances we've been told about but haven't yet shown
+	// up in the instance list.
+	for _, inst := range ensureInsts {
+		id := inst.Id()
+		if !found[id] {
+			insts = append(insts, inst)
+			found[id] = true
+		}
+	}
+	err = environ.StopInstances(insts)
+	if err != nil {
+		return err
+	}
+
+	// To properly observe e.storageUnlocked we need to get its value while
+	// holding e.ecfgMutex. e.Storage() does this for us, then we convert
+	// back to the (*storage) to access the private deleteAll() method.
+	st := environ.Storage().(*maasStorage)
+	return st.deleteAll()
+}
+
+func (*maasEnviron) AssignmentPolicy() state.AssignmentPolicy {
+	return state.AssignUnused
+}
+
+// MAAS does not do firewalling so these port methods do nothing.
+func (*maasEnviron) OpenPorts([]params.Port) error {
+	log.Debugf("environs/maas: unimplemented OpenPorts() called")
+	return nil
+}
+
+func (*maasEnviron) ClosePorts([]params.Port) error {
+	log.Debugf("environs/maas: unimplemented ClosePorts() called")
+	return nil
+}
+
+func (*maasEnviron) Ports() ([]params.Port, error) {
+	log.Debugf("environs/maas: unimplemented Ports() called")
+	return []params.Port{}, nil
+}
+
+func (*maasEnviron) Provider() environs.EnvironProvider {
+	return &providerInstance
+}
 
=== added file 'environs/maas/environ_test.go'
--- environs/maas/environ_test.go	1970-01-01 00:00:00 +0000
+++ environs/maas/environ_test.go	2013-04-12 10:00:43 +0000
@@ -0,0 +1,379 @@
+package maas
+
+import (
+	"encoding/base64"
+	. "launchpad.net/gocheck"
+	"launchpad.net/gomaasapi"
+	"launchpad.net/goyaml"
+	"launchpad.net/juju-core/constraints"
+	"launchpad.net/juju-core/environs"
+	"launchpad.net/juju-core/environs/config"
+	envtesting "launchpad.net/juju-core/environs/testing"
+	"launchpad.net/juju-core/state"
+	"launchpad.net/juju-core/testing"
+	"launchpad.net/juju-core/trivial"
+	"launchpad.net/juju-core/version"
+)
+
+type EnvironSuite struct {
+	ProviderSuite
+}
+
+var _ = Suite(new(EnvironSuite))
+
+// getTestConfig creates a customized sample MAAS provider configuration.
+func getTestConfig(name, server, oauth, secret string) *config.Config {
+	ecfg, err := newConfig(map[string]interface{}{
+		"name":            name,
+		"maas-server":     server,
+		"maas-oauth":      oauth,
+		"admin-secret":    secret,
+		"authorized-keys": "I-am-not-a-real-key",
+	})
+	if err != nil {
+		panic(err)
+	}
+	return ecfg.Config
+}
+
+// makeEnviron creates a functional maasEnviron for a test.  Its configuration
+// is a bit arbitrary and none of the test code's business.
+func (suite *EnvironSuite) makeEnviron() *maasEnviron {
+	config, err := config.New(map[string]interface{}{
+		"name":            suite.environ.Name(),
+		"type":            "maas",
+		"admin-secret":    "local-secret",
+		"authorized-keys": "foo",
+		"agent-version":   version.CurrentNumber().String(),
+		"maas-oauth":      "a:b:c",
+		"maas-server":     suite.testMAASObject.TestServer.URL,
+		// These are not needed by MAAS, but juju-core breaks without them. Needs
+		// fixing there.
+		"ca-cert":        testing.CACert,
+		"ca-private-key": testing.CAKey,
+	})
+	if err != nil {
+		panic(err)
+	}
+	env, err := NewEnviron(config)
+	if err != nil {
+		panic(err)
+	}
+	return env
+}
+
+func (suite *EnvironSuite) setupFakeProviderStateFile(c *C) {
+	suite.testMAASObject.TestServer.NewFile("provider-state", []byte("test file content"))
+}
+
+func (suite *EnvironSuite) setupFakeTools(c *C) {
+	storage := NewStorage(suite.environ)
+	envtesting.PutFakeTools(c, storage)
+}
+
+func (EnvironSuite) TestSetConfigValidatesFirst(c *C) {
+	// SetConfig() validates the config change and disallows, for example,
+	// changes in the environment name.
+	server := "http://maas.example.com"
+	oauth := "a:b:c"
+	secret := "pssst"
+	oldCfg := getTestConfig("old-name", server, oauth, secret)
+	newCfg := getTestConfig("new-name", server, oauth, secret)
+	env, err := NewEnviron(oldCfg)
+	c.Assert(err, IsNil)
+
+	// SetConfig() fails, even though both the old and the new config are
+	// individually valid.
+	err = env.SetConfig(newCfg)
+	c.Assert(err, NotNil)
+	c.Check(err, ErrorMatches, ".*cannot change name.*")
+
+	// The old config is still in place.  The new config never took effect.
+	c.Check(env.Name(), Equals, "old-name")
+}
+
+func (EnvironSuite) TestSetConfigUpdatesConfig(c *C) {
+	name := "test env"
+	cfg := getTestConfig(name, "http://maas2.example.com", "a:b:c", "secret")
+	env, err := NewEnviron(cfg)
+	c.Check(err, IsNil)
+	c.Check(env.name, Equals, "test env")
+
+	anotherServer := "http://maas.example.com"
+	anotherOauth := "c:d:e"
+	anotherSecret := "secret2"
+	cfg2 := getTestConfig(name, anotherServer, anotherOauth, anotherSecret)
+	errSetConfig := env.SetConfig(cfg2)
+	c.Check(errSetConfig, IsNil)
+	c.Check(env.name, Equals, name)
+	authClient, _ := gomaasapi.NewAuthenticatedClient(anotherServer, anotherOauth, apiVersion)
+	maas := gomaasapi.NewMAAS(*authClient)
+	MAASServer := env.maasClientUnlocked
+	c.Check(MAASServer, DeepEquals, maas)
+}
+
+func (EnvironSuite) TestNewEnvironSetsConfig(c *C) {
+	name := "test env"
+	cfg := getTestConfig(name, "http://maas.example.com", "a:b:c", "secret")
+
+	env, err := NewEnviron(cfg)
+
+	c.Check(err, IsNil)
+	c.Check(env.name, Equals, name)
+}
+
+func (suite *EnvironSuite) TestInstancesReturnsInstances(c *C) {
+	input := `{"system_id": "test"}`
+	node := suite.testMAASObject.TestServer.NewNode(input)
+	resourceURI, _ := node.GetField("resource_uri")
+	instanceIds := []state.InstanceId{state.InstanceId(resourceURI)}
+
+	instances, err := suite.environ.Instances(instanceIds)
+
+	c.Check(err, IsNil)
+	c.Check(len(instances), Equals, 1)
+	c.Check(string(instances[0].Id()), Equals, resourceURI)
+}
+
+func (suite *EnvironSuite) TestInstancesReturnsNilIfEmptyParameter(c *C) {
+	// Instances returns nil if the given parameter is empty.
+	input := `{"system_id": "test"}`
+	suite.testMAASObject.TestServer.NewNode(input)
+	instances, err := suite.environ.Instances([]state.InstanceId{})
+
+	c.Check(err, IsNil)
+	c.Check(instances, IsNil)
+}
+
+func (suite *EnvironSuite) TestInstancesReturnsNilIfNilParameter(c *C) {
+	// Instances returns nil if the given parameter is nil.
+	input := `{"system_id": "test"}`
+	suite.testMAASObject.TestServer.NewNode(input)
+	instances, err := suite.environ.Instances(nil)
+
+	c.Check(err, IsNil)
+	c.Check(instances, IsNil)
+}
+
+func (suite *EnvironSuite) TestAllInstancesReturnsAllInstances(c *C) {
+	input := `{"system_id": "test"}`
+	node := suite.testMAASObject.TestServer.NewNode(input)
+	resourceURI, _ := node.GetField("resource_uri")
+
+	instances, err := suite.environ.AllInstances()
+
+	c.Check(err, IsNil)
+	c.Check(len(instances), Equals, 1)
+	c.Check(string(instances[0].Id()), Equals, resourceURI)
+}
+
+func (suite *EnvironSuite) TestAllInstancesReturnsEmptySliceIfNoInstance(c *C) {
+	instances, err := suite.environ.AllInstances()
+
+	c.Check(err, IsNil)
+	c.Check(len(instances), Equals, 0)
+}
+
+func (suite *EnvironSuite) TestInstancesReturnsErrorIfPartialInstances(c *C) {
+	input1 := `{"system_id": "test"}`
+	node1 := suite.testMAASObject.TestServer.NewNode(input1)
+	resourceURI1, _ := node1.GetField("resource_uri")
+	input2 := `{"system_id": "test2"}`
+	suite.testMAASObject.TestServer.NewNode(input2)
+	instanceId1 := state.InstanceId(resourceURI1)
+	instanceId2 := state.InstanceId("unknown systemID")
+	instanceIds := []state.InstanceId{instanceId1, instanceId2}
+
+	instances, err := suite.environ.Instances(instanceIds)
+
+	c.Check(err, Equals, environs.ErrPartialInstances)
+	c.Check(len(instances), Equals, 1)
+	c.Check(string(instances[0].Id()), Equals, resourceURI1)
+}
+
+func (suite *EnvironSuite) TestStorageReturnsStorage(c *C) {
+	env := suite.makeEnviron()
+	storage := env.Storage()
+	c.Check(storage, NotNil)
+	// The Storage object is really a maasStorage.
+	specificStorage := storage.(*maasStorage)
+	// Its environment pointer refers back to its environment.
+	c.Check(specificStorage.environUnlocked, Equals, env)
+}
+
+func (suite *EnvironSuite) TestPublicStorageReturnsEmptyStorage(c *C) {
+	env := suite.makeEnviron()
+	storage := env.PublicStorage()
+	c.Assert(storage, NotNil)
+	c.Check(storage, Equals, environs.EmptyStorage)
+}
+
+func decodeUserData(userData string) ([]byte, error) {
+	data, err := base64.StdEncoding.DecodeString(userData)
+	if err != nil {
+		return []byte(""), err
+	}
+	return trivial.Gunzip(data)
+}
+
+func (suite *EnvironSuite) TestStartInstanceStartsInstance(c *C) {
+	suite.setupFakeTools(c)
+	env := suite.makeEnviron()
+	// Create node 0: it will be used as the bootstrap node.
+	suite.testMAASObject.TestServer.NewNode(`{"system_id": "node0", "hostname": "host0"}`)
+	err := environs.Bootstrap(env, constraints.Value{})
+	c.Assert(err, IsNil)
+	// The bootstrap node has been started.
+	operations := suite.testMAASObject.TestServer.NodeOperations()
+	actions, found := operations["node0"]
+	c.Check(found, Equals, true)
+	c.Check(actions, DeepEquals, []string{"start"})
+
+	// Create node 1: it will be used as instance number 1.
+	suite.testMAASObject.TestServer.NewNode(`{"system_id": "node1", "hostname": "host1"}`)
+	stateInfo, apiInfo, err := env.StateInfo()
+	c.Assert(err, IsNil)
+	stateInfo.Tag = "machine-1"
+	apiInfo.Tag = "machine-1"
+	series := version.Current.Series
+	nonce := "12345"
+	instance, err := env.StartInstance("1", nonce, series, constraints.Value{}, stateInfo, apiInfo)
+	c.Assert(err, IsNil)
+	c.Check(instance, NotNil)
+
+	// The instance number 1 has been started.
+	actions, found = operations["node1"]
+	c.Assert(found, Equals, true)
+	c.Check(actions, DeepEquals, []string{"start"})
+
+	// The value of the "user data" parameter used when starting the node
+	// contains the run cmd used to write the machine information onto
+	// the node's filesystem.
+	requestValues := suite.testMAASObject.TestServer.NodeOperationRequestValues()
+	nodeRequestValues, found := requestValues["node1"]
+	c.Assert(found, Equals, true)
+	userData := nodeRequestValues[0].Get("user_data")
+	decodedUserData, err := decodeUserData(userData)
+	c.Assert(err, IsNil)
+	info := machineInfo{string(instance.Id()), "host1"}
+	cloudinitRunCmd, err := info.cloudinitRunCmd()
+	c.Assert(err, IsNil)
+	data, err := goyaml.Marshal(cloudinitRunCmd)
+	c.Assert(err, IsNil)
+	c.Check(string(decodedUserData), Matches, "(.|\n)*"+string(data)+"(\n|.)*")
+}
+
+func (suite *EnvironSuite) getInstance(systemId string) *maasInstance {
+	input := `{"system_id": "` + systemId + `"}`
+	node := suite.testMAASObject.TestServer.NewNode(input)
+	return &maasInstance{&node, suite.environ}
+}
+
+func (suite *EnvironSuite) TestStopInstancesReturnsIfParameterEmpty(c *C) {
+	suite.getInstance("test1")
+
+	err := suite.environ.StopInstances([]environs.Instance{})
+	c.Check(err, IsNil)
+	operations := suite.testMAASObject.TestServer.NodeOperations()
+	c.Check(operations, DeepEquals, map[string][]string{})
+}
+
+func (suite *EnvironSuite) TestStopInstancesStopsAndReleasesInstances(c *C) {
+	instance1 := suite.getInstance("test1")
+	instance2 := suite.getInstance("test2")
+	suite.getInstance("test3")
+	instances := []environs.Instance{instance1, instance2}
+
+	err := suite.environ.StopInstances(instances)
+
+	c.Check(err, IsNil)
+	operations := suite.testMAASObject.TestServer.NodeOperations()
+	expectedOperations := map[string][]string{"test1": {"release"}, "test2": {"release"}}
+	c.Check(operations, DeepEquals, expectedOperations)
+}
+
+func (suite *EnvironSuite) TestStateInfo(c *C) {
+	env := suite.makeEnviron()
+	hostname := "test"
+	input := `{"system_id": "system_id", "hostname": "` + hostname + `"}`
+	node := suite.testMAASObject.TestServer.NewNode(input)
+	instance := &maasInstance{&node, suite.environ}
+	err := env.saveState(&bootstrapState{StateInstances: []state.InstanceId{instance.Id()}})
+	c.Assert(err, IsNil)
+
+	stateInfo, apiInfo, err := env.StateInfo()
+
+	c.Assert(err, IsNil)
+	c.Assert(stateInfo.Addrs, DeepEquals, []string{hostname + mgoPortSuffix})
+	c.Assert(apiInfo.Addrs, DeepEquals, []string{hostname + apiPortSuffix})
+}
+
+func (suite *EnvironSuite) TestStateInfoFailsIfNoStateInstances(c *C) {
+	env := suite.makeEnviron()
+
+	_, _, err := env.StateInfo()
+
+	c.Check(err, FitsTypeOf, &environs.NotFoundError{})
+}
+
+func (suite *EnvironSuite) TestDestroy(c *C) {
+	env := suite.makeEnviron()
+	suite.getInstance("test1")
+	instance := suite.getInstance("test2")
+	data := makeRandomBytes(10)
+	suite.testMAASObject.TestServer.NewFile("filename", data)
+	storage := env.Storage()
+
+	err := env.Destroy([]environs.Instance{instance})
+
+	c.Check(err, IsNil)
+	// Instances have been stopped.
+	operations := suite.testMAASObject.TestServer.NodeOperations()
+	expectedOperations := map[string][]string{"test1": {"release"}, "test2": {"release"}}
+	c.Check(operations, DeepEquals, expectedOperations)
+	// Files have been cleaned up.
+	listing, err := storage.List("")
+	c.Assert(err, IsNil)
+	c.Check(listing, DeepEquals, []string{})
+}
+
+// It would be nice if we could unit-test Bootstrap() in more detail, but
+// at the time of writing that would require more support from gomaasapi's
+// testing service than we have.
+func (suite *EnvironSuite) TestBootstrapSucceeds(c *C) {
+	suite.setupFakeTools(c)
+	env := suite.makeEnviron()
+	suite.testMAASObject.TestServer.NewNode(`{"system_id": "thenode", "hostname": "host"}`)
+	cert := []byte{1, 2, 3}
+	key := []byte{4, 5, 6}
+
+	err := env.Bootstrap(constraints.Value{}, cert, key)
+	c.Assert(err, IsNil)
+}
+
+func (suite *EnvironSuite) TestBootstrapFailsIfNoNodes(c *C) {
+	suite.setupFakeTools(c)
+	env := suite.makeEnviron()
+	cert := []byte{1, 2, 3}
+	key := []byte{4, 5, 6}
+	err := env.Bootstrap(constraints.Value{}, cert, key)
+	// Since there are no nodes, the attempt to allocate one returns a
+	// 409: Conflict.
+	c.Check(err, ErrorMatches, ".*409.*")
+}
+
+func (suite *EnvironSuite) TestBootstrapIntegratesWithEnvirons(c *C) {
+	suite.setupFakeTools(c)
+	env := suite.makeEnviron()
+	suite.testMAASObject.TestServer.NewNode(`{"system_id": "bootstrapnode", "hostname": "host"}`)
+
+	// environs.Bootstrap calls Environ.Bootstrap.  This works.
+	err := environs.Bootstrap(env, constraints.Value{})
+	c.Assert(err, IsNil)
+}
+
+func (suite *EnvironSuite) TestAssignmentPolicy(c *C) {
+	env := suite.makeEnviron()
+
+	c.Check(env.AssignmentPolicy(), Equals, state.AssignUnused)
+}
 
=== added file 'environs/maas/environprovider.go'
--- environs/maas/environprovider.go	1970-01-01 00:00:00 +0000
+++ environs/maas/environprovider.go	2013-04-12 10:00:43 +0000
@@ -0,0 +1,81 @@
+package maas
+
+import (
+	"launchpad.net/juju-core/environs"
+	"launchpad.net/juju-core/environs/config"
+	"launchpad.net/juju-core/log"
+	"launchpad.net/juju-core/state"
+)
+
+type maasEnvironProvider struct{}
+
+var _ environs.EnvironProvider = (*maasEnvironProvider)(nil)
+
+var providerInstance maasEnvironProvider
+
+func init() {
+	environs.RegisterProvider("maas", maasEnvironProvider{})
+}
+
+func (maasEnvironProvider) Open(cfg *config.Config) (environs.Environ, error) {
+	log.Debugf("environs/maas: opening environment %q.", cfg.Name())
+	return NewEnviron(cfg)
+}
+
+// Boilerplate config YAML.  Don't mess with the indentation or add newlines!
+const boilerplateYAML = `maas:
+  type: maas
+  # Change this to where your MAAS server lives.  It must specify the base path.
+  maas-server: 'http://192.168.1.1/MAAS/'
+  maas-oauth: '<add your OAuth credentials from MAAS here>'
+  admin-secret: {{rand}}
+  default-series: precise
+  authorized-keys-path: ~/.ssh/authorized_keys # or any file you want.
+  # Or:
+  # authorized-keys: ssh-rsa keymaterialhere
+`
+
+// BoilerplateConfig is specified in the EnvironProvider interface.
+func (maasEnvironProvider) BoilerplateConfig() string {
+	return boilerplateYAML
+}
+
+// SecretAttrs is specified in the EnvironProvider interface.
+func (prov maasEnvironProvider) SecretAttrs(cfg *config.Config) (map[string]interface{}, error) {
+	secretAttrs := make(map[string]interface{})
+	maasCfg, err := prov.newConfig(cfg)
+	if err != nil {
+		return nil, err
+	}
+	secretAttrs["maas-oauth"] = maasCfg.MAASOAuth()
+	return secretAttrs, nil
+}
+
+func (maasEnvironProvider) hostname() (string, error) {
+	info := machineInfo{}
+	err := info.load()
+	if err != nil {
+		return "", err
+	}
+	return info.Hostname, nil
+}
+
+// PublicAddress is specified in the EnvironProvider interface.
+func (prov maasEnvironProvider) PublicAddress() (string, error) {
+	return prov.hostname()
+}
+
+// PrivateAddress is specified in the EnvironProvider interface.
+func (prov maasEnvironProvider) PrivateAddress() (string, error) {
+	return prov.hostname()
+}
+
+// InstanceId is specified in the EnvironProvider interface.
+func (maasEnvironProvider) InstanceId() (state.InstanceId, error) {
+	info := machineInfo{}
+	err := info.load()
+	if err != nil {
+		return "", err
+	}
+	return state.InstanceId(info.InstanceId), nil
+}
 
=== added file 'environs/maas/environprovider_test.go'
--- environs/maas/environprovider_test.go	1970-01-01 00:00:00 +0000
+++ environs/maas/environprovider_test.go	2013-04-12 10:00:43 +0000
@@ -0,0 +1,94 @@
+package maas
+
+import (
+	"io/ioutil"
+	. "launchpad.net/gocheck"
+	"launchpad.net/goyaml"
+	"launchpad.net/juju-core/environs/config"
+	"launchpad.net/juju-core/state"
+)
+
+type EnvironProviderSuite struct {
+	ProviderSuite
+}
+
+var _ = Suite(new(EnvironProviderSuite))
+
+func (suite *EnvironProviderSuite) TestSecretAttrsReturnsSensitiveMAASAttributes(c *C) {
+	testJujuHome := c.MkDir()
+	defer config.SetJujuHome(config.SetJujuHome(testJujuHome))
+	const oauth = "aa:bb:cc"
+	attrs := map[string]interface{}{
+		"maas-oauth":      oauth,
+		"maas-server":     "http://maas.example.com/maas/",
+		"name":            "wheee",
+		"type":            "maas",
+		"authorized-keys": "I-am-not-a-real-key",
+	}
+	config, err := config.New(attrs)
+	c.Assert(err, IsNil)
+
+	secretAttrs, err := suite.environ.Provider().SecretAttrs(config)
+	c.Assert(err, IsNil)
+
+	expectedAttrs := map[string]interface{}{"maas-oauth": oauth}
+	c.Check(secretAttrs, DeepEquals, expectedAttrs)
+}
+
+// create a temporary file with the given content.  The file will be cleaned
+// up at the end of the test calling this method.
+func createTempFile(c *C, content []byte) string {
+	file, err := ioutil.TempFile(c.MkDir(), "")
+	c.Assert(err, IsNil)
+	filename := file.Name()
+	err = ioutil.WriteFile(filename, content, 0644)
+	c.Assert(err, IsNil)
+	return filename
+}
+
+// InstanceId returns the instanceId of the machine read from the file
+// _MAASInstanceFilename.
+func (suite *EnvironProviderSuite) TestInstanceIdReadsInstanceIdFromMachineFile(c *C) {
+	instanceId := "instance-id"
+	info := machineInfo{instanceId, "hostname"}
+	yaml, err := goyaml.Marshal(info)
+	c.Assert(err, IsNil)
+	// Create a temporary file to act as the file where the instanceID
+	// is stored.
+	filename := createTempFile(c, yaml)
+	// "Monkey patch" the value of _MAASInstanceFilename with the path
+	// to the temporary file.
+	old_MAASInstanceFilename := _MAASInstanceFilename
+	_MAASInstanceFilename = filename
+	defer func() { _MAASInstanceFilename = old_MAASInstanceFilename }()
+
+	provider := suite.environ.Provider()
+	returnedInstanceId, err := provider.InstanceId()
+	c.Assert(err, IsNil)
+	c.Check(returnedInstanceId, Equals, state.InstanceId(instanceId))
+}
+
+// PublicAddress and PrivateAddress return the hostname of the machine read
+// from the file _MAASInstanceFilename.
+func (suite *EnvironProviderSuite) TestPrivatePublicAddressReadsHostnameFromMachineFile(c *C) {
+	hostname := "myhostname"
+	info := machineInfo{"instance-id", hostname}
+	yaml, err := goyaml.Marshal(info)
+	c.Assert(err, IsNil)
+	// Create a temporary file to act as the file where the instanceID
+	// is stored.
+	filename := createTempFile(c, yaml)
+	// "Monkey patch" the value of _MAASInstanceFilename with the path
+	// to the temporary file.
+	old_MAASInstanceFilename := _MAASInstanceFilename
+	_MAASInstanceFilename = filename
+	defer func() { _MAASInstanceFilename = old_MAASInstanceFilename }()
+
+	provider := suite.environ.Provider()
+	publicAddress, err := provider.PublicAddress()
+	c.Assert(err, IsNil)
+	c.Check(publicAddress, Equals, hostname)
+	privateAddress, err := provider.PrivateAddress()
+	c.Assert(err, IsNil)
+	c.Check(privateAddress, Equals, hostname)
+}
 
=== added file 'environs/maas/instance.go'
--- environs/maas/instance.go	1970-01-01 00:00:00 +0000
+++ environs/maas/instance.go	2013-04-12 10:00:43 +0000
@@ -0,0 +1,63 @@
+package maas
+
+import (
+	"launchpad.net/gomaasapi"
+	"launchpad.net/juju-core/environs"
+	"launchpad.net/juju-core/log"
+	"launchpad.net/juju-core/state"
+	"launchpad.net/juju-core/state/api/params"
+)
+
+type maasInstance struct {
+	maasObject *gomaasapi.MAASObject
+	environ    *maasEnviron
+}
+
+var _ environs.Instance = (*maasInstance)(nil)
+
+func (instance *maasInstance) Id() state.InstanceId {
+	// Use the node's 'resource_uri' value.
+	return state.InstanceId((*instance.maasObject).URI().String())
+}
+
+// refreshInstance refreshes the instance with the most up-to-date information
+// from the MAAS server.
+func (instance *maasInstance) refreshInstance() error {
+	insts, err := instance.environ.Instances([]state.InstanceId{instance.Id()})
+	if err != nil {
+		return err
+	}
+	newMaasObject := insts[0].(*maasInstance).maasObject
+	instance.maasObject = newMaasObject
+	return nil
+}
+
+func (instance *maasInstance) DNSName() (string, error) {
+	hostname, err := (*instance.maasObject).GetField("hostname")
+	if err != nil {
+		return "", err
+	}
+	return hostname, nil
+}
+
+func (instance *maasInstance) WaitDNSName() (string, error) {
+	// A MAAS nodes gets his DNS name when it's created.  WaitDNSName,
+	// (same as DNSName) just returns the hostname of the node.
+	return instance.DNSName()
+}
+
+// MAAS does not do firewalling so these port methods do nothing.
+func (instance *maasInstance) OpenPorts(machineId string, ports []params.Port) error {
+	log.Debugf("environs/maas: unimplemented OpenPorts() called")
+	return nil
+}
+
+func (instance *maasInstance) ClosePorts(machineId string, ports []params.Port) error {
+	log.Debugf("environs/maas: unimplemented ClosePorts() called")
+	return nil
+}
+
+func (instance *maasInstance) Ports(machineId string) ([]params.Port, error) {
+	log.Debugf("environs/maas: unimplemented Ports() called")
+	return []params.Port{}, nil
+}
 
=== added file 'environs/maas/instance_test.go'
--- environs/maas/instance_test.go	1970-01-01 00:00:00 +0000
+++ environs/maas/instance_test.go	2013-04-12 10:00:43 +0000
@@ -0,0 +1,51 @@
+package maas
+
+import (
+	. "launchpad.net/gocheck"
+)
+
+type InstanceTest struct {
+	ProviderSuite
+}
+
+var _ = Suite(&InstanceTest{})
+
+func (s *InstanceTest) TestId(c *C) {
+	jsonValue := `{"system_id": "system_id", "test": "test"}`
+	obj := s.testMAASObject.TestServer.NewNode(jsonValue)
+	resourceURI, _ := obj.GetField("resource_uri")
+	instance := maasInstance{&obj, s.environ}
+
+	c.Check(string(instance.Id()), Equals, resourceURI)
+}
+
+func (s *InstanceTest) TestRefreshInstance(c *C) {
+	jsonValue := `{"system_id": "system_id", "test": "test"}`
+	obj := s.testMAASObject.TestServer.NewNode(jsonValue)
+	s.testMAASObject.TestServer.ChangeNode("system_id", "test2", "test2")
+	instance := maasInstance{&obj, s.environ}
+
+	err := instance.refreshInstance()
+
+	c.Check(err, IsNil)
+	testField, err := (*instance.maasObject).GetField("test2")
+	c.Check(err, IsNil)
+	c.Check(testField, Equals, "test2")
+}
+
+func (s *InstanceTest) TestDNSName(c *C) {
+	jsonValue := `{"hostname": "DNS name", "system_id": "system_id"}`
+	obj := s.testMAASObject.TestServer.NewNode(jsonValue)
+	instance := maasInstance{&obj, s.environ}
+
+	dnsName, err := instance.DNSName()
+
+	c.Check(err, IsNil)
+	c.Check(dnsName, Equals, "DNS name")
+
+	// WaitDNSName() currently simply calls DNSName().
+	dnsName, err = instance.WaitDNSName()
+
+	c.Check(err, IsNil)
+	c.Check(dnsName, Equals, "DNS name")
+}
 
=== added file 'environs/maas/maas_test.go'
--- environs/maas/maas_test.go	1970-01-01 00:00:00 +0000
+++ environs/maas/maas_test.go	2013-04-12 10:00:43 +0000
@@ -0,0 +1,32 @@
+package maas
+
+import (
+	. "launchpad.net/gocheck"
+	"launchpad.net/gomaasapi"
+	"testing"
+)
+
+func TestMAAS(t *testing.T) {
+	TestingT(t)
+}
+
+type ProviderSuite struct {
+	environ        *maasEnviron
+	testMAASObject *gomaasapi.TestMAASObject
+}
+
+var _ = Suite(&ProviderSuite{})
+
+func (s *ProviderSuite) SetUpSuite(c *C) {
+	TestMAASObject := gomaasapi.NewTestMAAS("1.0")
+	s.testMAASObject = TestMAASObject
+	s.environ = &maasEnviron{name: "test env", maasClientUnlocked: &TestMAASObject.MAASObject}
+}
+
+func (s *ProviderSuite) TearDownTest(c *C) {
+	s.testMAASObject.TestServer.Clear()
+}
+
+func (s *ProviderSuite) TearDownSuite(c *C) {
+	s.testMAASObject.Close()
+}
 
=== added file 'environs/maas/state.go'
--- environs/maas/state.go	1970-01-01 00:00:00 +0000
+++ environs/maas/state.go	2013-04-12 10:00:43 +0000
@@ -0,0 +1,47 @@
+package maas
+
+import (
+	"bytes"
+	"fmt"
+	"io/ioutil"
+	"launchpad.net/goyaml"
+	"launchpad.net/juju-core/state"
+)
+
+const stateFile = "provider-state"
+
+// Persistent environment state.  An environment needs to know what instances
+// it manages.
+type bootstrapState struct {
+	StateInstances []state.InstanceId `yaml:"state-instances"`
+}
+
+// saveState writes the environment's state to the provider-state file stored
+// in the environment's storage.
+func (env *maasEnviron) saveState(state *bootstrapState) error {
+	data, err := goyaml.Marshal(state)
+	if err != nil {
+		return err
+	}
+	buf := bytes.NewBuffer(data)
+	return env.Storage().Put(stateFile, buf, int64(len(data)))
+}
+
+// loadState reads the environment's state from storage.
+func (env *maasEnviron) loadState() (*bootstrapState, error) {
+	r, err := env.Storage().Get(stateFile)
+	if err != nil {
+		return nil, err
+	}
+	defer r.Close()
+	data, err := ioutil.ReadAll(r)
+	if err != nil {
+		return nil, fmt.Errorf("error reading %q: %v", stateFile, err)
+	}
+	var state bootstrapState
+	err = goyaml.Unmarshal(data, &state)
+	if err != nil {
+		return nil, fmt.Errorf("error unmarshalling %q: %v", stateFile, err)
+	}
+	return &state, nil
+}
 
=== added file 'environs/maas/state_test.go'
--- environs/maas/state_test.go	1970-01-01 00:00:00 +0000
+++ environs/maas/state_test.go	2013-04-12 10:00:43 +0000
@@ -0,0 +1,23 @@
+package maas
+
+import (
+	. "launchpad.net/gocheck"
+	"launchpad.net/juju-core/environs"
+)
+
+type StateSuite struct {
+	ProviderSuite
+}
+
+var _ = Suite(new(StateSuite))
+
+func (suite *StateSuite) TestLoadStateReturnsNotFoundPointerForMissingFile(c *C) {
+	serverURL := suite.testMAASObject.URL().String()
+	config := getTestConfig("loadState-test", serverURL, "a:b:c", "foo")
+	env, err := NewEnviron(config)
+	c.Assert(err, IsNil)
+
+	_, err = env.loadState()
+
+	c.Check(err, FitsTypeOf, &environs.NotFoundError{})
+}
 
=== added file 'environs/maas/storage.go'
--- environs/maas/storage.go	1970-01-01 00:00:00 +0000
+++ environs/maas/storage.go	2013-04-12 10:00:43 +0000
@@ -0,0 +1,207 @@
+package maas
+
+import (
+	"bytes"
+	"encoding/base64"
+	"fmt"
+	"io"
+	"io/ioutil"
+	"launchpad.net/gomaasapi"
+	"launchpad.net/juju-core/environs"
+	"net/url"
+	"sort"
+	"sync"
+)
+
+type maasStorage struct {
+	// Mutex protects the "*Unlocked" fields.
+	sync.Mutex
+
+	// The Environ that this Storage is for.
+	environUnlocked *maasEnviron
+
+	// Reference to the URL on the API where files are stored.
+	maasClientUnlocked gomaasapi.MAASObject
+}
+
+var _ environs.Storage = (*maasStorage)(nil)
+
+func NewStorage(env *maasEnviron) environs.Storage {
+	storage := new(maasStorage)
+	storage.environUnlocked = env
+	storage.maasClientUnlocked = env.getMAASClient().GetSubObject("files")
+	return storage
+}
+
+// getSnapshot returns a consistent copy of a maasStorage.  Use this if you
+// need a consistent view of the object's entire state, without having to
+// lock the object the whole time.
+//
+// An easy mistake to make with "defer" is to keep holding a lock without
+// realizing it, while you go on to block on http requests or other slow
+// things that don't actually require the lock.  In most cases you can just
+// create a snapshot first (releasing the lock immediately) and then do the
+// rest of the work with the snapshot.
+func (stor *maasStorage) getSnapshot() *maasStorage {
+	stor.Lock()
+	defer stor.Unlock()
+
+	return &maasStorage{
+		environUnlocked:    stor.environUnlocked,
+		maasClientUnlocked: stor.maasClientUnlocked,
+	}
+}
+
+// addressFileObject creates a MAASObject pointing to a given file.
+// Takes out a lock on the storage object to get a consistent view.
+func (stor *maasStorage) addressFileObject(name string) gomaasapi.MAASObject {
+	stor.Lock()
+	defer stor.Unlock()
+	return stor.maasClientUnlocked.GetSubObject(name)
+}
+
+// retrieveFileObject retrieves the information of the named file, including
+// its download URL and its contents, as a MAASObject.
+//
+// This may return many different errors, but specifically, it returns
+// (a pointer to) environs.NotFoundError if the file did not exist.
+//
+// The function takes out a lock on the storage object.
+func (stor *maasStorage) retrieveFileObject(name string) (gomaasapi.MAASObject, error) {
+	obj, err := stor.addressFileObject(name).Get()
+	if err != nil {
+		noObj := gomaasapi.MAASObject{}
+		serverErr, ok := err.(gomaasapi.ServerError)
+		if ok && serverErr.StatusCode == 404 {
+			msg := fmt.Errorf("file '%s' not found", name)
+			return noObj, &environs.NotFoundError{msg}
+		}
+		msg := fmt.Errorf("could not access file '%s': %v", name, err)
+		return noObj, msg
+	}
+	return obj, nil
+}
+
+// Get is specified in the Storage interface.
+func (stor *maasStorage) Get(name string) (io.ReadCloser, error) {
+	fileObj, err := stor.retrieveFileObject(name)
+	if err != nil {
+		return nil, err
+	}
+	data, err := fileObj.GetField("content")
+	if err != nil {
+		return nil, fmt.Errorf("could not extract file content for %s: %v", name, err)
+	}
+	buf, err := base64.StdEncoding.DecodeString(data)
+	if err != nil {
+		return nil, fmt.Errorf("bad data in file '%s': %v", name, err)
+	}
+	return ioutil.NopCloser(bytes.NewReader(buf)), nil
+}
+
+// extractFilenames returns the filenames from a "list" operation on the
+// MAAS API, sorted by name.
+func (stor *maasStorage) extractFilenames(listResult gomaasapi.JSONObject) ([]string, error) {
+	list, err := listResult.GetArray()
+	if err != nil {
+		return nil, err
+	}
+	result := make([]string, len(list))
+	for index, entry := range list {
+		file, err := entry.GetMap()
+		if err != nil {
+			return nil, err
+		}
+		filename, err := file["filename"].GetString()
+		if err != nil {
+			return nil, err
+		}
+		result[index] = filename
+	}
+	sort.Strings(result)
+	return result, nil
+}
+
+// List is specified in the Storage interface.
+func (stor *maasStorage) List(prefix string) ([]string, error) {
+	params := make(url.Values)
+	params.Add("prefix", prefix)
+	snapshot := stor.getSnapshot()
+	obj, err := snapshot.maasClientUnlocked.CallGet("list", params)
+	if err != nil {
+		return nil, err
+	}
+	return snapshot.extractFilenames(obj)
+}
+
+// URL is specified in the Storage interface.
+func (stor *maasStorage) URL(name string) (string, error) {
+	fileObj, err := stor.retrieveFileObject(name)
+	if err != nil {
+		return "", err
+	}
+	uri, err := fileObj.GetField("anon_resource_uri")
+	if err != nil {
+		msg := fmt.Errorf("could not get file's download URL (may be an outdated MAAS): %s", err)
+		return "", msg
+	}
+
+	partialURL, err := url.Parse(uri)
+	if err != nil {
+		return "", err
+	}
+	fullURL := fileObj.URL().ResolveReference(partialURL)
+	return fullURL.String(), nil
+}
+
+// Put is specified in the Storage interface.
+func (stor *maasStorage) Put(name string, r io.Reader, length int64) error {
+	data, err := ioutil.ReadAll(io.LimitReader(r, length))
+	if err != nil {
+		return err
+	}
+	params := url.Values{"filename": {name}}
+	files := map[string][]byte{"file": data}
+	snapshot := stor.getSnapshot()
+	_, err = snapshot.maasClientUnlocked.CallPostFiles("add", params, files)
+	return err
+}
+
+// Remove is specified in the Storage interface.
+func (stor *maasStorage) Remove(name string) error {
+	// The only thing that can go wrong here, really, is that the file
+	// does not exist.  But deletion is idempotent: deleting a file that
+	// is no longer there anyway is success, not failure.
+	stor.getSnapshot().maasClientUnlocked.GetSubObject(name).Delete()
+	return nil
+}
+
+func (stor *maasStorage) deleteAll() error {
+	names, err := stor.List("")
+	if err != nil {
+		return err
+	}
+	// Remove all the objects in parallel so that we incur less round-trips.
+	// If we're in danger of having hundreds of objects,
+	// we'll want to change this to limit the number
+	// of concurrent operations.
+	var wg sync.WaitGroup
+	wg.Add(len(names))
+	errc := make(chan error, len(names))
+	for _, name := range names {
+		name := name
+		go func() {
+			defer wg.Done()
+			if err := stor.Remove(name); err != nil {
+				errc <- err
+			}
+		}()
+	}
+	wg.Wait()
+	select {
+	case err := <-errc:
+		return fmt.Errorf("cannot delete all provider state: %v", err)
+	default:
+	}
+	return nil
+}
 
=== added file 'environs/maas/storage_test.go'
--- environs/maas/storage_test.go	1970-01-01 00:00:00 +0000
+++ environs/maas/storage_test.go	2013-04-12 10:00:43 +0000
@@ -0,0 +1,385 @@
+package maas
+
+import (
+	"bytes"
+	"encoding/base64"
+	"io/ioutil"
+	. "launchpad.net/gocheck"
+	"launchpad.net/gomaasapi"
+	"launchpad.net/juju-core/environs"
+	"math/rand"
+	"net/http"
+	"net/url"
+	"sync"
+)
+
+type StorageSuite struct {
+	ProviderSuite
+}
+
+var _ = Suite(new(StorageSuite))
+
+// makeStorage creates a MAAS storage object for the running test.
+func (s *StorageSuite) makeStorage(name string) *maasStorage {
+	maasobj := s.testMAASObject.MAASObject
+	env := maasEnviron{name: name, maasClientUnlocked: &maasobj}
+	return NewStorage(&env).(*maasStorage)
+}
+
+// makeRandomBytes returns an array of arbitrary byte values.
+func makeRandomBytes(length int) []byte {
+	data := make([]byte, length)
+	for index := range data {
+		data[index] = byte(rand.Intn(256))
+	}
+	return data
+}
+
+// fakeStoredFile creates a file directly in the (simulated) MAAS file store.
+// It will contain an arbitrary amount of random data.  The contents are also
+// returned.
+//
+// If you want properly random data here, initialize the randomizer first.
+// Or don't, if you want consistent (and debuggable) results.
+func (s *StorageSuite) fakeStoredFile(storage environs.Storage, name string) gomaasapi.MAASObject {
+	data := makeRandomBytes(rand.Intn(10))
+	return s.testMAASObject.TestServer.NewFile(name, data)
+}
+
+func (s *StorageSuite) TestGetSnapshotCreatesClone(c *C) {
+	original := s.makeStorage("storage-name")
+	snapshot := original.getSnapshot()
+	c.Check(snapshot.environUnlocked, Equals, original.environUnlocked)
+	c.Check(snapshot.maasClientUnlocked.URL().String(), Equals, original.maasClientUnlocked.URL().String())
+	// Snapshotting locks the original internally, but does not leave
+	// either the original or the snapshot locked.
+	unlockedMutexValue := sync.Mutex{}
+	c.Check(original.Mutex, Equals, unlockedMutexValue)
+	c.Check(snapshot.Mutex, Equals, unlockedMutexValue)
+}
+
+func (s *StorageSuite) TestGetRetrievesFile(c *C) {
+	const filename = "stored-data"
+	storage := s.makeStorage("get-retrieves-file")
+	file := s.fakeStoredFile(storage, filename)
+	base64Content, err := file.GetField("content")
+	c.Assert(err, IsNil)
+	content, err := base64.StdEncoding.DecodeString(base64Content)
+	c.Assert(err, IsNil)
+
+	reader, err := storage.Get(filename)
+	c.Assert(err, IsNil)
+	defer reader.Close()
+
+	buf, err := ioutil.ReadAll(reader)
+	c.Assert(err, IsNil)
+	c.Check(len(buf), Equals, len(content))
+	c.Check(buf, DeepEquals, content)
+}
+
+func (s *StorageSuite) TestRetrieveFileObjectReturnsFileObject(c *C) {
+	const filename = "myfile"
+	stor := s.makeStorage("rfo-test")
+	file := s.fakeStoredFile(stor, filename)
+	fileURI, err := file.GetField("anon_resource_uri")
+	c.Assert(err, IsNil)
+	fileContent, err := file.GetField("content")
+	c.Assert(err, IsNil)
+
+	obj, err := stor.retrieveFileObject(filename)
+	c.Assert(err, IsNil)
+
+	uri, err := obj.GetField("anon_resource_uri")
+	c.Assert(err, IsNil)
+	c.Check(uri, Equals, fileURI)
+	content, err := obj.GetField("content")
+	c.Check(content, Equals, fileContent)
+}
+
+func (s *StorageSuite) TestRetrieveFileObjectReturnsNotFoundForMissingFile(c *C) {
+	stor := s.makeStorage("rfo-test")
+	_, err := stor.retrieveFileObject("nonexistent-file")
+	c.Assert(err, NotNil)
+	c.Check(err, FitsTypeOf, &environs.NotFoundError{})
+}
+
+func (s *StorageSuite) TestRetrieveFileObjectEscapesName(c *C) {
+	const filename = "#a?b c&d%e!"
+	data := []byte("File contents here")
+	stor := s.makeStorage("rfo-test")
+	err := stor.Put(filename, bytes.NewReader(data), int64(len(data)))
+	c.Assert(err, IsNil)
+
+	obj, err := stor.retrieveFileObject(filename)
+	c.Assert(err, IsNil)
+
+	base64Content, err := obj.GetField("content")
+	c.Assert(err, IsNil)
+	content, err := base64.StdEncoding.DecodeString(base64Content)
+	c.Assert(err, IsNil)
+	c.Check(content, DeepEquals, data)
+}
+
+func (s *StorageSuite) TestFileContentsAreBinary(c *C) {
+	const filename = "myfile.bin"
+	data := []byte{0, 1, 255, 2, 254, 3}
+	stor := s.makeStorage("binary-test")
+
+	err := stor.Put(filename, bytes.NewReader(data), int64(len(data)))
+	c.Assert(err, IsNil)
+	file, err := stor.Get(filename)
+	c.Assert(err, IsNil)
+	content, err := ioutil.ReadAll(file)
+	c.Assert(err, IsNil)
+
+	c.Check(content, DeepEquals, data)
+}
+
+func (s *StorageSuite) TestGetReturnsNotFoundErrorIfNotFound(c *C) {
+	const filename = "lost-data"
+	storage := NewStorage(s.environ)
+	_, err := storage.Get(filename)
+	c.Assert(err, FitsTypeOf, &environs.NotFoundError{})
+}
+
+func (s *StorageSuite) TestListReturnsEmptyIfNoFilesStored(c *C) {
+	storage := NewStorage(s.environ)
+	listing, err := storage.List("")
+	c.Assert(err, IsNil)
+	c.Check(listing, DeepEquals, []string{})
+}
+
+func (s *StorageSuite) TestListReturnsAllFilesIfPrefixEmpty(c *C) {
+	storage := NewStorage(s.environ)
+	files := []string{"1a", "2b", "3c"}
+	for _, name := range files {
+		s.fakeStoredFile(storage, name)
+	}
+
+	listing, err := storage.List("")
+	c.Assert(err, IsNil)
+	c.Check(listing, DeepEquals, files)
+}
+
+func (s *StorageSuite) TestListSortsResults(c *C) {
+	storage := NewStorage(s.environ)
+	files := []string{"4d", "1a", "3c", "2b"}
+	for _, name := range files {
+		s.fakeStoredFile(storage, name)
+	}
+
+	listing, err := storage.List("")
+	c.Assert(err, IsNil)
+	c.Check(listing, DeepEquals, []string{"1a", "2b", "3c", "4d"})
+}
+
+func (s *StorageSuite) TestListReturnsNoFilesIfNoFilesMatchPrefix(c *C) {
+	storage := NewStorage(s.environ)
+	s.fakeStoredFile(storage, "foo")
+
+	listing, err := storage.List("bar")
+	c.Assert(err, IsNil)
+	c.Check(listing, DeepEquals, []string{})
+}
+
+func (s *StorageSuite) TestListReturnsOnlyFilesWithMatchingPrefix(c *C) {
+	storage := NewStorage(s.environ)
+	s.fakeStoredFile(storage, "abc")
+	s.fakeStoredFile(storage, "xyz")
+
+	listing, err := storage.List("x")
+	c.Assert(err, IsNil)
+	c.Check(listing, DeepEquals, []string{"xyz"})
+}
+
+func (s *StorageSuite) TestListMatchesPrefixOnly(c *C) {
+	storage := NewStorage(s.environ)
+	s.fakeStoredFile(storage, "abc")
+	s.fakeStoredFile(storage, "xabc")
+
+	listing, err := storage.List("a")
+	c.Assert(err, IsNil)
+	c.Check(listing, DeepEquals, []string{"abc"})
+}
+
+func (s *StorageSuite) TestListOperatesOnFlatNamespace(c *C) {
+	storage := NewStorage(s.environ)
+	s.fakeStoredFile(storage, "a/b/c/d")
+
+	listing, err := storage.List("a/b")
+	c.Assert(err, IsNil)
+	c.Check(listing, DeepEquals, []string{"a/b/c/d"})
+}
+
+// getFileAtURL requests, and returns, the file at the given URL.
+func getFileAtURL(fileURL string) ([]byte, error) {
+	response, err := http.Get(fileURL)
+	if err != nil {
+		return nil, err
+	}
+	body, err := ioutil.ReadAll(response.Body)
+	if err != nil {
+		return nil, err
+	}
+	return body, nil
+}
+
+func (s *StorageSuite) TestURLReturnsURLCorrespondingToFile(c *C) {
+	const filename = "my-file.txt"
+	storage := NewStorage(s.environ).(*maasStorage)
+	file := s.fakeStoredFile(storage, filename)
+	// The file contains an anon_resource_uri, which lacks a network part
+	// (but will probably contain a query part).  anonURL will be the
+	// file's full URL.
+	anonURI, err := file.GetField("anon_resource_uri")
+	c.Assert(err, IsNil)
+	parsedURI, err := url.Parse(anonURI)
+	c.Assert(err, IsNil)
+	anonURL := storage.maasClientUnlocked.URL().ResolveReference(parsedURI)
+	c.Assert(err, IsNil)
+
+	fileURL, err := storage.URL(filename)
+	c.Assert(err, IsNil)
+
+	c.Check(fileURL, NotNil)
+	c.Check(fileURL, Equals, anonURL.String())
+}
+
+func (s *StorageSuite) TestPutStoresRetrievableFile(c *C) {
+	const filename = "broken-toaster.jpg"
+	contents := []byte("Contents here")
+	length := int64(len(contents))
+	storage := NewStorage(s.environ)
+
+	err := storage.Put(filename, bytes.NewReader(contents), length)
+
+	reader, err := storage.Get(filename)
+	c.Assert(err, IsNil)
+	defer reader.Close()
+
+	buf, err := ioutil.ReadAll(reader)
+	c.Assert(err, IsNil)
+	c.Check(buf, DeepEquals, contents)
+}
+
+func (s *StorageSuite) TestPutOverwritesFile(c *C) {
+	const filename = "foo.bar"
+	storage := NewStorage(s.environ)
+	s.fakeStoredFile(storage, filename)
+	newContents := []byte("Overwritten")
+
+	err := storage.Put(filename, bytes.NewReader(newContents), int64(len(newContents)))
+	c.Assert(err, IsNil)
+
+	reader, err := storage.Get(filename)
+	c.Assert(err, IsNil)
+	defer reader.Close()
+
+	buf, err := ioutil.ReadAll(reader)
+	c.Assert(err, IsNil)
+	c.Check(len(buf), Equals, len(newContents))
+	c.Check(buf, DeepEquals, newContents)
+}
+
+func (s *StorageSuite) TestPutStopsAtGivenLength(c *C) {
+	const filename = "xyzzyz.2.xls"
+	const length = 5
+	contents := []byte("abcdefghijklmnopqrstuvwxyz")
+	storage := NewStorage(s.environ)
+
+	err := storage.Put(filename, bytes.NewReader(contents), length)
+	c.Assert(err, IsNil)
+
+	reader, err := storage.Get(filename)
+	c.Assert(err, IsNil)
+	defer reader.Close()
+
+	buf, err := ioutil.ReadAll(reader)
+	c.Assert(err, IsNil)
+	c.Check(len(buf), Equals, length)
+}
+
+func (s *StorageSuite) TestPutToExistingFileTruncatesAtGivenLength(c *C) {
+	const filename = "a-file-which-is-mine"
+	oldContents := []byte("abcdefghijklmnopqrstuvwxyz")
+	newContents := []byte("xyz")
+	storage := NewStorage(s.environ)
+	err := storage.Put(filename, bytes.NewReader(oldContents), int64(len(oldContents)))
+	c.Assert(err, IsNil)
+
+	err = storage.Put(filename, bytes.NewReader(newContents), int64(len(newContents)))
+	c.Assert(err, IsNil)
+
+	reader, err := storage.Get(filename)
+	c.Assert(err, IsNil)
+	defer reader.Close()
+
+	buf, err := ioutil.ReadAll(reader)
+	c.Assert(err, IsNil)
+	c.Check(len(buf), Equals, len(newContents))
+	c.Check(buf, DeepEquals, newContents)
+}
+
+func (s *StorageSuite) TestRemoveDeletesFile(c *C) {
+	const filename = "doomed.txt"
+	storage := NewStorage(s.environ)
+	s.fakeStoredFile(storage, filename)
+
+	err := storage.Remove(filename)
+	c.Assert(err, IsNil)
+
+	_, err = storage.Get(filename)
+	c.Assert(err, FitsTypeOf, &environs.NotFoundError{})
+
+	listing, err := storage.List(filename)
+	c.Assert(err, IsNil)
+	c.Assert(listing, DeepEquals, []string{})
+}
+
+func (s *StorageSuite) TestRemoveIsIdempotent(c *C) {
+	const filename = "half-a-file"
+	storage := NewStorage(s.environ)
+	s.fakeStoredFile(storage, filename)
+
+	err := storage.Remove(filename)
+	c.Assert(err, IsNil)
+
+	err = storage.Remove(filename)
+	c.Assert(err, IsNil)
+}
+
+func (s *StorageSuite) TestNamesMayHaveSlashes(c *C) {
+	const filename = "name/with/slashes"
+	content := []byte("File contents")
+	storage := NewStorage(s.environ)
+
+	err := storage.Put(filename, bytes.NewReader(content), int64(len(content)))
+	c.Assert(err, IsNil)
+
+	// There's not much we can say about the anonymous URL, except that
+	// we get one.
+	anonURL, err := storage.URL(filename)
+	c.Assert(err, IsNil)
+	c.Check(anonURL, Matches, "http[s]*://.*")
+
+	reader, err := storage.Get(filename)
+	c.Assert(err, IsNil)
+	defer reader.Close()
+	data, err := ioutil.ReadAll(reader)
+	c.Assert(err, IsNil)
+	c.Check(data, DeepEquals, content)
+}
+
+func (s *StorageSuite) TestDeleteAllDeletesAllFiles(c *C) {
+	storage := s.makeStorage("get-retrieves-file")
+	const filename1 = "stored-data1"
+	s.fakeStoredFile(storage, filename1)
+	const filename2 = "stored-data2"
+	s.fakeStoredFile(storage, filename2)
+
+	err := storage.deleteAll()
+	c.Assert(err, IsNil)
+	listing, err := storage.List("")
+	c.Assert(err, IsNil)
+	c.Assert(listing, DeepEquals, []string{})
+}
 
=== added file 'environs/maas/util.go'
--- environs/maas/util.go	1970-01-01 00:00:00 +0000
+++ environs/maas/util.go	2013-04-12 10:00:43 +0000
@@ -0,0 +1,83 @@
+package maas
+
+import (
+	"fmt"
+	"launchpad.net/goyaml"
+	cloudinit_core "launchpad.net/juju-core/cloudinit"
+	"launchpad.net/juju-core/environs/cloudinit"
+	"launchpad.net/juju-core/log"
+	"launchpad.net/juju-core/state"
+	"launchpad.net/juju-core/trivial"
+	"net/url"
+	"strings"
+)
+
+// extractSystemId extracts the 'system_id' part from an InstanceId.
+// "/MAAS/api/1.0/nodes/system_id/" => "system_id"
+func extractSystemId(instanceId state.InstanceId) string {
+	trimmed := strings.TrimRight(string(instanceId), "/")
+	split := strings.Split(trimmed, "/")
+	return split[len(split)-1]
+}
+
+// getSystemIdValues returns a url.Values object with all the 'system_ids'
+// from the given instanceIds stored under the key 'id'.  This is used
+// to filter out instances when listing the nodes objects.
+func getSystemIdValues(instanceIds []state.InstanceId) url.Values {
+	values := url.Values{}
+	for _, instanceId := range instanceIds {
+		values.Add("id", extractSystemId(instanceId))
+	}
+	return values
+}
+
+// userData returns a zipped cloudinit config.
+func userData(cfg *cloudinit.MachineConfig, scripts ...string) ([]byte, error) {
+	cloudcfg := cloudinit_core.New()
+	for _, script := range scripts {
+		cloudcfg.AddRunCmd(script)
+	}
+	cloudcfg, err := cloudinit.Configure(cfg, cloudcfg)
+	if err != nil {
+		return nil, err
+	}
+	data, err := cloudcfg.Render()
+	if err != nil {
+		return nil, err
+	}
+	cdata := trivial.Gzip(data)
+	log.Debugf("environs/maas: maas user data; %d bytes", len(cdata))
+	return cdata, nil
+}
+
+// machineInfo is the structure used to pass information between the provider
+// and the agent running on a node.
+// When a node is started, the provider code creates a machineInfo object
+// containing information about the node being started and configures
+// cloudinit to get a YAML representation of that object written on the node's
+// filesystem during its first startup.  That file is then read by the juju
+// agent running on the node and converted back into a machineInfo object.
+type machineInfo struct {
+	InstanceId string `yaml:,omitempty`
+	Hostname   string `yaml:,omitempty`
+}
+
+var _MAASInstanceFilename = jujuDataDir + "/MAASmachine.txt"
+
+// cloudinitRunCmd returns the shell command that, when run, will create the
+// "machine info" file containing the instanceId and the hostname of a machine.
+// That command is destined to be used by cloudinit.
+func (info *machineInfo) cloudinitRunCmd() (string, error) {
+	yaml, err := goyaml.Marshal(info)
+	if err != nil {
+		return "", err
+	}
+	script := fmt.Sprintf(`mkdir -p %s; echo -n %s > %s`, trivial.ShQuote(jujuDataDir), trivial.ShQuote(string(yaml)), trivial.ShQuote(_MAASInstanceFilename))
+	return script, nil
+}
+
+// load loads the "machine info" file and parse the content into the info
+// object.
+func (info *machineInfo) load() error {
+	return trivial.ReadYaml(_MAASInstanceFilename, info)
+}
 
=== added file 'environs/maas/util_test.go'
--- environs/maas/util_test.go	1970-01-01 00:00:00 +0000
+++ environs/maas/util_test.go	2013-04-12 10:00:43 +0000
@@ -0,0 +1,129 @@
+package maas
+
+import (
+	"fmt"
+	. "launchpad.net/gocheck"
+	"launchpad.net/goyaml"
+	"launchpad.net/juju-core/environs/cloudinit"
+	"launchpad.net/juju-core/environs/config"
+	"launchpad.net/juju-core/state"
+	"launchpad.net/juju-core/state/api"
+	"launchpad.net/juju-core/testing"
+	"launchpad.net/juju-core/trivial"
+	"launchpad.net/juju-core/version"
+)
+
+type UtilSuite struct{}
+
+var _ = Suite(&UtilSuite{})
+
+func (s *UtilSuite) TestExtractSystemId(c *C) {
+	instanceId := state.InstanceId("/MAAS/api/1.0/nodes/system_id/")
+
+	systemId := extractSystemId(instanceId)
+
+	c.Check(systemId, Equals, "system_id")
+}
+
+func (s *UtilSuite) TestGetSystemIdValues(c *C) {
+	instanceId1 := state.InstanceId("/MAAS/api/1.0/nodes/system_id1/")
+	instanceId2 := state.InstanceId("/MAAS/api/1.0/nodes/system_id2/")
+	instanceIds := []state.InstanceId{instanceId1, instanceId2}
+
+	values := getSystemIdValues(instanceIds)
+
+	c.Check(values["id"], DeepEquals, []string{"system_id1", "system_id2"})
+}
+
+func (s *UtilSuite) TestUserData(c *C) {
+	testJujuHome := c.MkDir()
+	defer config.SetJujuHome(config.SetJujuHome(testJujuHome))
+	tools := &state.Tools{
+		URL:    "http://foo.com/tools/juju1.2.3-linux-amd64.tgz",
+		Binary: version.MustParseBinary("1.2.3-linux-amd64"),
+	}
+	envConfig, err := config.New(map[string]interface{}{
+		"type":            "maas",
+		"name":            "foo",
+		"default-series":  "series",
+		"authorized-keys": "keys",
+		"ca-cert":         testing.CACert,
+	})
+	c.Assert(err, IsNil)
+
+	cfg := &cloudinit.MachineConfig{
+		MachineId:       "10",
+		MachineNonce:    "5432",
+		Tools:           tools,
+		StateServerCert: []byte(testing.ServerCert),
+		StateServerKey:  []byte(testing.ServerKey),
+		StateInfo: &state.Info{
+			Password: "pw1",
+			CACert:   []byte("CA CERT\n" + testing.CACert),
+		},
+		APIInfo: &api.Info{
+			Password: "pw2",
+			CACert:   []byte("CA CERT\n" + testing.CACert),
+		},
+		DataDir:     jujuDataDir,
+		MongoPort:   mgoPort,
+		Config:      envConfig,
+		APIPort:     apiPort,
+		StateServer: true,
+	}
+	script1 := "script1"
+	script2 := "script2"
+	scripts := []string{script1, script2}
+	result, err := userData(cfg, scripts...)
+	c.Assert(err, IsNil)
+
+	unzipped, err := trivial.Gunzip(result)
+	c.Assert(err, IsNil)
+
+	config := make(map[interface{}]interface{})
+	err = goyaml.Unmarshal(unzipped, &config)
+	c.Assert(err, IsNil)
+
+	// Just check that the cloudinit config looks good.
+	c.Check(config["apt_upgrade"], Equals, true)
+	// The scripts given to userData where added as the first
+	// commands to be run.
+	runCmd := config["runcmd"].([]interface{})
+	c.Check(runCmd[0], Equals, script1)
+	c.Check(runCmd[1], Equals, script2)
+}
+
+func (s *UtilSuite) TestMachineInfoCloudinitRunCmd(c *C) {
+	instanceId := "instanceId"
+	hostname := "hostname"
+	filename := "path/to/file"
+	old_MAASInstanceFilename := _MAASInstanceFilename
+	_MAASInstanceFilename = filename
+	defer func() { _MAASInstanceFilename = old_MAASInstanceFilename }()
+	info := machineInfo{instanceId, hostname}
+
+	script, err := info.cloudinitRunCmd()
+
+	c.Assert(err, IsNil)
+	yaml, err := goyaml.Marshal(info)
+	c.Assert(err, IsNil)
+	expected := fmt.Sprintf("mkdir -p '%s'; echo -n '%s' > '%s'", jujuDataDir, yaml, filename)
+	c.Check(script, Equals, expected)
+}
+
+func (s *UtilSuite) TestMachineInfoLoad(c *C) {
+	instanceId := "instanceId"
+	hostname := "hostname"
+	yaml := fmt.Sprintf("instanceid: %s\nhostname: %s\n", instanceId, hostname)
+	filename := createTempFile(c, []byte(yaml))
+	old_MAASInstanceFilename := _MAASInstanceFilename
+	_MAASInstanceFilename = filename
+	defer func() { _MAASInstanceFilename = old_MAASInstanceFilename }()
+	info := machineInfo{}
+
+	err := info.load()
+
+	c.Assert(err, IsNil)
+	c.Check(info.InstanceId, Equals, instanceId)
+	c.Check(info.Hostname, Equals, hostname)
+}