launchpad-reviewers team mailing list archive

["Jeroen T. Vermeulen" <jtv@xxxxxxxxxxxxx>]

Jeroen T. Vermeulen has proposed merging lp:~jtv/juju-core/mpv-bootstrap into lp:~maas-maintainers/juju-core/maas-provider-skeleton.

Commit message:
Sketch out Bootstrap implementation.

Requested reviews:
  MAAS Maintainers (maas-maintainers)

For more details, see:
https://code.launchpad.net/~jtv/juju-core/mpv-bootstrap/+merge/152347

There are still plenty of things missing, unfortunately:

 * We need slashes in upload filenames, it turns out.  That needs supporting changes in multiple projects.

 * StartInstance does not do what the interface expects (it starts an existing node by MAAS system ID, but it needs to allocate some indeterminate node with a juju-assigned machine ID).

 * Bootstrap deserves real tests.  What we have so far has been useful in finding things that were missing, but all it really does right now is test against crashes.

We'll get on those things first, then come back and complete the Bootstrap work.


Jeroen
-- 
https://code.launchpad.net/~jtv/juju-core/mpv-bootstrap/+merge/152347
Your team MAAS Maintainers is requested to review the proposed merge of lp:~jtv/juju-core/mpv-bootstrap into lp:~maas-maintainers/juju-core/maas-provider-skeleton.

=== modified file 'environs/maas/environ.go'
--- environs/maas/environ.go	2013-02-15 13:58:22 +0000
+++ environs/maas/environ.go	2013-03-08 09:29:27 +0000
@@ -2,14 +2,17 @@
 
 import (
 	"errors"
+	"fmt"
 	"launchpad.net/gomaasapi"
 	"launchpad.net/juju-core/environs"
 	"launchpad.net/juju-core/environs/config"
 	"launchpad.net/juju-core/log"
 	"launchpad.net/juju-core/state"
 	"launchpad.net/juju-core/state/api"
-	"net/url"
+	"launchpad.net/juju-core/trivial"
+	"launchpad.net/juju-core/version"
 	"sync"
+	"time"
 )
 
 type maasEnviron struct {
@@ -20,6 +23,7 @@
 
 	ecfgUnlocked       *maasEnvironConfig
 	maasClientUnlocked *gomaasapi.MAASObject
+	storageUnlocked    environs.Storage
 }
 
 var _ environs.Environ = (*maasEnviron)(nil)
@@ -35,6 +39,7 @@
 	if err != nil {
 		return nil, err
 	}
+	env.storageUnlocked = NewStorage(env)
 	return env, nil
 }
 
@@ -42,11 +47,139 @@
 	return env.name
 }
 
+// quiesceStateFile waits (up to a few seconds) for any existing state file to
+// disappear.
+//
+// This is used when bootstrapping, to deal with any previous state file that
+// may have been removed by a Destroy that hasn't reached its eventual
+// consistent state yet.
+func (env *maasEnviron) quiesceStateFile() error {
+	// This was all cargo-culted off the EC2 provider.
+	var err error
+	retry := trivial.AttemptStrategy{
+		Total: 5 * time.Second,
+		Delay: 200 * time.Millisecond,
+	}
+	for a := retry.Start(); err == nil && a.Next(); {
+		_, err = env.loadState()
+	}
+	if err == nil {
+		// The state file outlived the timeout.  Looks like it wasn't
+		// being destroyed after all.
+		return fmt.Errorf("environment is already bootstrapped")
+	}
+	if _, notFound := err.(*environs.NotFoundError); !notFound {
+		return fmt.Errorf("cannot query old bootstrap state: %v", err)
+	}
+	// Got to this point?  Then the error was "not found," which is the
+	// state we're looking for.
+	return nil
+}
+
+// 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)
+	if err != nil {
+		return nil, fmt.Errorf("cannot upload tools: %v", err)
+	}
+	return tools, nil
+}
+
+// findTools looks for a current version of the juju tools that is already
+// uploaded in the environment.
+func (env *maasEnviron) findTools() (*state.Tools, error) {
+	flags := environs.HighestVersion | environs.CompatVersion
+	v := version.Current
+	v.Series = env.Config().DefaultSeries()
+	tools, err := environs.FindTools(env, v, flags)
+	if err != nil {
+		return nil, fmt.Errorf("cannot find tools: %v", err)
+	}
+	return tools, nil
+}
+
+// getMongoURL returns the URL to the appropriate MongoDB instance.
+func (env *maasEnviron) getMongoURL(tools *state.Tools) string {
+	v := version.Current
+	v.Series = tools.Series
+	v.Arch = tools.Arch
+	return environs.MongoURL(env, v)
+}
+
+// Suppress compiler errors for unused variables.
+// TODO: Eliminate all usage of this function.  It's just for development.
+func unused(...interface{}) {}
+
+// 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 inital configuration: %v", err)
+	}
+	caCert, hasCert := env.Config().CACert()
+	if !hasCert {
+		return nil, fmt.Errorf("no CA certificate in environment configuration")
+	}
+	mongoURL := env.getMongoURL(tools)
+	stateInfo := state.Info{
+		Password: trivial.PasswordHash(password),
+		CACert:   caCert,
+	}
+	apiInfo := api.Info{
+		Password: trivial.PasswordHash(password),
+		CACert:   caCert,
+	}
+	// TODO: mongoURL, cert/key, and config need to go into the userdata somehow.
+	unused(mongoURL, cert, key, config)
+	inst, err := env.StartInstance("0", &stateInfo, &apiInfo, tools)
+	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(uploadTools bool, stateServerCert, stateServerKey []byte) error {
+	// 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.Printf("environs/maas: bootstrapping environment %q.", env.Name())
-	panic("Not implemented.")
+	err := env.quiesceStateFile()
+	if err != nil {
+		return err
+	}
+	var tools *state.Tools
+	if uploadTools {
+		tools, err = env.uploadTools()
+	} else {
+		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.stopInstance(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.
+	// Perhaps consider using SimpleDB for state storage
+	// which would enable that possibility.
+
+	return nil
 }
 
+// StateInfo is specified in the Environ interface.
 func (*maasEnviron) StateInfo() (*state.Info, *api.Info, error) {
 	panic("Not implemented.")
 }
@@ -59,10 +192,12 @@
 	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 {
 	ecfg, err := env.Provider().(*maasEnvironProvider).newConfig(cfg)
 	if err != nil {
@@ -84,40 +219,45 @@
 	return nil
 }
 
+// StartInstance is specified in the Environ interface.
 func (environ *maasEnviron) StartInstance(machineId string, info *state.Info, apiInfo *api.Info, tools *state.Tools) (environs.Instance, error) {
-	node := environ.maasClientUnlocked.GetSubObject(machineId)
-	refreshedNode, err := node.Get()
-	if err != nil {
-		return nil, err
-	}
-	_, refreshErr := refreshedNode.CallPost("start", url.Values{})
-	if refreshErr != nil {
-		return nil, refreshErr
-	}
-	instance := &maasInstance{maasObject: &refreshedNode, environ: environ}
+	node, err := environ.maasClientUnlocked.GetSubObject(machineId).Get()
+	if err != nil {
+		return nil, err
+	}
+	_, err = node.CallPost("start", nil)
+	if err != nil {
+		return nil, err
+	}
+	instance := &maasInstance{maasObject: &node, environ: environ}
 	return instance, nil
 }
 
+// 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
 	}
-	// Iterate over all the instances and send the "stop" signal,
-	// collecting the errors returned.
-	var errors []error
+	// Tell MAAS to shut down each of the instances.  If there are errors,
+	// return only the first one (but shut down all instances regardless).
+	var firstErr error
 	for _, instance := range instances {
-		maasInstance := instance.(*maasInstance)
-		_, errPost := (*maasInstance.maasObject).CallPost("stop", url.Values{})
-		errors = append(errors, errPost)
-	}
-	// Return the first error encountered, if any.
-	for _, err := range errors {
-		if err != nil {
-			return err
+		err := environ.stopInstance(instance)
+		if firstErr == nil {
+			firstErr = err
 		}
 	}
-	return nil
+	return firstErr
+}
+
+// stopInstance stops a single instance.  Avoid looping over this in bulk
+// operations: use StopInstances for those.
+func (environ *maasEnviron) stopInstance(inst environs.Instance) error {
+	maasInst := inst.(*maasInstance)
+	maasObj := maasInst.maasObject
+	_, err := maasObj.CallPost("stop", nil)
+	return err
 }
 
 // Instances returns the environs.Instance objects corresponding to the given
@@ -168,8 +308,10 @@
 	return environ.instances(nil)
 }
 
-func (*maasEnviron) Storage() environs.Storage {
-	panic("Not implemented.")
+func (env *maasEnviron) Storage() environs.Storage {
+	env.ecfgMutex.Lock()
+	defer env.ecfgMutex.Unlock()
+	return env.storageUnlocked
 }
 
 func (*maasEnviron) PublicStorage() environs.StorageReader {

=== modified file 'environs/maas/environ_test.go'
--- environs/maas/environ_test.go	2013-02-08 11:30:22 +0000
+++ environs/maas/environ_test.go	2013-03-08 09:29:27 +0000
@@ -6,6 +6,7 @@
 	"launchpad.net/juju-core/environs"
 	"launchpad.net/juju-core/environs/config"
 	"launchpad.net/juju-core/state"
+	"launchpad.net/juju-core/testing"
 )
 
 type EnvironSuite struct {
@@ -169,3 +170,26 @@
 	expectedOperations := map[string][]string{"test1": {"stop"}, "test2": {"stop"}}
 	c.Check(operations, DeepEquals, expectedOperations)
 }
+
+func (suite *EnvironSuite) TestBootstrap(c *C) {
+	config, err := config.New(map[string]interface{}{
+		"name":            suite.environ.Name(),
+		"type":            "maas",
+		"admin-secret":    "local-secret",
+		"authorized-keys": "foo",
+		"ca-cert":         testing.CACert,
+		"ca-private-key":  testing.CAKey,
+		"maas-oauth":      "a:b:c",
+		"maas-server":     suite.testMAASObject.URL().String(),
+	})
+	c.Assert(err, IsNil)
+	env, err := NewEnviron(config)
+	c.Assert(err, IsNil)
+	env.ecfgUnlocked = &maasEnvironConfig{Config: config}
+
+	err = env.Bootstrap(true, []byte{}, []byte{})
+	// TODO: Get this to succeed.
+	// c.Assert(err, IsNil)
+
+	// TODO: Verify a simile of success.
+}

=== added file 'environs/maas/state.go'
--- environs/maas/state.go	1970-01-01 00:00:00 +0000
+++ environs/maas/state.go	2013-03-08 09:29:27 +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-03-08 09:29:27 +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{})
+}

=== modified file 'environs/maas/storage.go'
--- environs/maas/storage.go	2013-03-06 03:55:40 +0000
+++ environs/maas/storage.go	2013-03-08 09:29:27 +0000
@@ -68,7 +68,7 @@
 // its download URL and its contents, as a MAASObject.
 //
 // This may return many different errors, but specifically, it returns
-// environs.NotFoundError if the file did not exist.
+// (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) {
@@ -78,7 +78,7 @@
 		serverErr, ok := err.(gomaasapi.ServerError)
 		if ok && serverErr.StatusCode == 404 {
 			msg := fmt.Errorf("file '%s' not found", name)
-			return noObj, environs.NotFoundError{msg}
+			return noObj, &environs.NotFoundError{msg}
 		}
 		msg := fmt.Errorf("could not access file '%s': %v", name, err)
 		return noObj, msg

=== modified file 'environs/maas/storage_test.go'
--- environs/maas/storage_test.go	2013-03-06 03:55:40 +0000
+++ environs/maas/storage_test.go	2013-03-08 09:29:27 +0000
@@ -96,11 +96,11 @@
 	c.Check(content, Equals, fileContent)
 }
 
-func (s *StorageSuite) TestRetrieveFileObjectReturnsNotFound(c *C) {
+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{})
+	c.Check(err, FitsTypeOf, &environs.NotFoundError{})
 }
 
 func (s *StorageSuite) TestRetrieveFileObjectEscapesName(c *C) {
@@ -139,7 +139,7 @@
 	const filename = "lost-data"
 	storage := NewStorage(s.environ)
 	_, err := storage.Get(filename)
-	c.Assert(err, FitsTypeOf, environs.NotFoundError{})
+	c.Assert(err, FitsTypeOf, &environs.NotFoundError{})
 }
 
 func (s *StorageSuite) TestListReturnsEmptyIfNoFilesStored(c *C) {
@@ -329,7 +329,7 @@
 	c.Assert(err, IsNil)
 
 	_, err = storage.Get(filename)
-	c.Assert(err, FitsTypeOf, environs.NotFoundError{})
+	c.Assert(err, FitsTypeOf, &environs.NotFoundError{})
 
 	listing, err := storage.List(filename)
 	c.Assert(err, IsNil)