Merge lp:~rvb/juju-core/fix-maas-pvd-conf into lp:~juju/juju-core/trunk

[Raphaël Badin]

Fix maasEnvironProvider.Open() so that it returns a nil object when it fails.

[Note: I did not use rietveld because I ran into errors like: http://paste.ubuntu.com/5710470/ when trying to use lbox propose]

This branch fixes a problem we saw happening in the MAAS lab while testing the MAAS provider. The symptom was that, after having installed the bootstrap node ok, Juju was unable to deploy any service. The error found in the bootstrap node's logs: http://paste.ubuntu.com/5709878/.

The problem is that, from this code in cmd/jujud/upgrade.go: http://paste.ubuntu.com/5710182/, environs/maas/environ.go:SetConfig ends up being called on a nil object. After investigating the problem, I found that environs/maas/environprovider.go:Open(), when the config is invalid, returns a interface that has a nil value but a non-nil type (http://golang.org/doc/faq#nil_error) and thus, in cmd/jujud/upgrade.go (see snippet above), 'environ.SetConfig(cfg)' gets called on a nil 'environ'.

[Dimiter Naydenov (dimitern)]

LGTM

review: Approve

[William Reade (fwereade)]

[0] env, err := NewEnviron(cfg)

This deserves a comment. But otherwise LGTM trivial.

[Dimiter Naydenov (dimitern)]

Due to the problems rvba had with lbox, he asked to land this for him.
It was proposed as https://codereview.appspot.com/8772043
So I'm setting this as Rejected and the one above will be merged.

=== modified file 'environs/maas/environprovider.go'
--- environs/maas/environprovider.go	2013-04-12 09:45:01 +0000
+++ environs/maas/environprovider.go	2013-04-15 14:31:30 +0000
@@ -19,7 +19,11 @@
 
 func (maasEnvironProvider) Open(cfg *config.Config) (environs.Environ, error) {
 	log.Debugf("environs/maas: opening environment %q.", cfg.Name())
-	return NewEnviron(cfg)
+	env, err := NewEnviron(cfg)
+	if err != nil {
+		return nil, err
+	}
+	return env, nil
 }
 
 // Boilerplate config YAML.  Don't mess with the indentation or add newlines!
 
=== modified file 'environs/maas/environprovider_test.go'
--- environs/maas/environprovider_test.go	2013-04-11 10:33:21 +0000
+++ environs/maas/environprovider_test.go	2013-04-15 14:31:30 +0000
@@ -92,3 +92,24 @@
 	c.Assert(err, IsNil)
 	c.Check(privateAddress, Equals, hostname)
 }
+
+func (suite *EnvironProviderSuite) TestOpenReturnsNilInterfaceUponFailure(c *C) {
+	testJujuHome := c.MkDir()
+	defer config.SetJujuHome(config.SetJujuHome(testJujuHome))
+	const oauth = "wrongly-formatted-oauth-string"
+	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)
+	env, err := suite.environ.Provider().Open(config)
+	// When Open() fails (i.e. returns a non-nil error), it returns an
+	// environs.Environ interface object with a nil value and a nil
+	// type.
+	c.Check(env, Equals, nil)
+	c.Check(err, ErrorMatches, ".*malformed maas-oauth.*")
+}