Raphaël Badin has proposed merging lp:~rvb/juju-core/uploadtools into 
lp:~maas-maintainers/juju-core/maas-provider-skeleton.

Commit message:
Always upload the tools.

Requested reviews:
  MAAS Maintainers (maas-maintainers)

For more details, see:
https://code.launchpad.net/~rvb/juju-core/uploadtools/+merge/157610

In the MAAS provider, we should always upload the tools as part of the 
Bootstrap process because they cannot be shared between environment.  This 
branch changes the Bootstrap() so that the tools will always be uploaded for 
the series defined in the environment.  I think this is the right thing to do 
(this is also confirmed by testing) but I added notes to remind us to check 
that this is sane with the juju-core team when this will be landed in juju-core 
trunk.

Note that 'uploadTools' was a parameter passed down to Bootstrap() a few weeks 
ago, now it's up to the provider to decide what it should do.
-- 
https://code.launchpad.net/~rvb/juju-core/uploadtools/+merge/157610
Your team MAAS Maintainers is requested to review the proposed merge of 
lp:~rvb/juju-core/uploadtools into 
lp:~maas-maintainers/juju-core/maas-provider-skeleton.
=== modified file 'environs/maas/environ.go'
--- environs/maas/environ.go	2013-04-05 10:38:15 +0000
+++ environs/maas/environ.go	2013-04-08 09:33:24 +0000
@@ -98,7 +98,9 @@
 // uploadTools builds the current version of the juju tools and uploads them
 // to the environment's Storage.
 func (env *maasEnviron) uploadTools() (*state.Tools, error) {
-	tools, err := environs.PutTools(env.Storage(), nil)
+	// Also upload the tools for the default series taken from the environment.
+	// TODO: Check that this is sane with the juju-core team.
+	tools, err := environs.PutTools(env.Storage(), nil, env.Config().DefaultSeries())
 	if err != nil {
 		return nil, fmt.Errorf("cannot upload tools: %v", err)
 	}
@@ -184,8 +186,6 @@
 
 // Bootstrap is specified in the Environ interface.
 func (env *maasEnviron) Bootstrap(cons constraints.Value, stateServerCert, stateServerKey []byte) error {
-	// TODO: Fix this quick hack.  uploadTools is a now-obsolete parameter.
- 	uploadTools := false
 
 	// This was all cargo-culted from the EC2 provider.
 	password := env.Config().AdminSecret()
@@ -197,12 +197,10 @@
 	if err != nil {
 		return err
 	}
+	// MAAS does not support public storage: always upload the tools.
+	// TODO: Check that this is sane with the juju-core team.
 	var tools *state.Tools
-	if uploadTools {
-		tools, err = env.uploadTools()
-	} else {
-		tools, err = env.findTools()
-	}
+	tools, err = env.uploadTools()
 	if err != nil {
 		return err
 	}

=== modified file 'environs/maas/environ_test.go'
--- environs/maas/environ_test.go	2013-04-08 01:50:24 +0000
+++ environs/maas/environ_test.go	2013-04-08 09:33:24 +0000
@@ -8,7 +8,6 @@
 	"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/version"
@@ -62,11 +61,6 @@
 	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) TestSetConfigUpdatesConfig(c *C) {
 	cfg := getTestConfig("test env", "http://maas2.example.com";, "a:b:c", "secret")
 	env, err := NewEnviron(cfg)
@@ -200,7 +194,6 @@
 // TODO: this test fails from time to time: we need to investigate what's
 // going on (also see the additional remarks below).
 func (suite *EnvironSuite) DisableTestStartInstanceStartsInstance(c *C) {
-	suite.setupFakeTools(c)
 	env := suite.makeEnviron()
 	suite.setupFakeProviderStateFile(c)
 	suite.testMAASObject.TestServer.NewNode(`{"system_id": "node1", "hostname": "host1"}`)
@@ -336,7 +329,6 @@
 // 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"}`)
 	cert := []byte{1, 2, 3}
@@ -347,7 +339,6 @@
 }
 
 func (suite *EnvironSuite) TestBootstrapFailsIfNoNodes(c *C) {
-	suite.setupFakeTools(c)
 	env := suite.makeEnviron()
 	cert := []byte{1, 2, 3}
 	key := []byte{4, 5, 6}
@@ -357,8 +348,19 @@
 	c.Check(err, ErrorMatches, ".*409.*")
 }
 
+func (suite *EnvironSuite) TestBootstrapUploadsTools(c *C) {
+	env := suite.makeEnviron()
+	suite.testMAASObject.TestServer.NewNode(`{"system_id": "thenode"}`)
+	cert := []byte{1, 2, 3}
+	key := []byte{4, 5, 6}
+	err := env.Bootstrap(constraints.Value{}, cert, key)
+	c.Assert(err, IsNil)
+
+	_, err = env.findTools()
+	c.Assert(err, IsNil)
+}
+
 func (suite *EnvironSuite) TestBootstrapIntegratesWithEnvirons(c *C) {
-	suite.setupFakeTools(c)
 	env := suite.makeEnviron()
 	suite.testMAASObject.TestServer.NewNode(`{"system_id": "bootstrapnode"}`)
 

_______________________________________________
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to     : [email protected]
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help   : https://help.launchpad.net/ListHelp

Reply via email to