launchpad-reviewers team mailing list archive

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

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

Commit message:
Implement the configuration portion of the MAAS provider.

Requested reviews:
  MAAS Maintainers (maas-maintainers)

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

Some of this was cargo-culted from the EC2 provider, with help from Dimiter and Roger.

The attributes defined in this config.go file are ones in addition to those that Juju knows globally.  (Beware: re-declaring them in an environ's config schema will cause strange effects!)

We can declare defaults for config values, but not in this case because both custom configuration attributes are mandatory.  An attribute without a default is required.


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

=== added file 'environs/maas/config.go'
--- environs/maas/config.go	1970-01-01 00:00:00 +0000
+++ environs/maas/config.go	2013-02-05 09:22:25 +0000
@@ -0,0 +1,70 @@
+package maas
+
+import (
+	"errors"
+	"launchpad.net/juju-core/environs/config"
+	"launchpad.net/juju-core/schema"
+	"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 {
+	value, ok := cfg.attrs["maas-server"]
+	if !ok {
+		return ""
+	}
+	return value.(string)
+}
+
+func (cfg *maasEnvironConfig) MAASOAuth() string {
+	value, ok := cfg.attrs["maas-oauth"]
+	if !ok {
+		return ""
+	}
+	return value.(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 noMaasServer = errors.New("No maas-server configured.")
+var noMaasOAuth = errors.New("No maas-oauth configured.")
+var malformedMaasOAuth = errors.New("Malformed maas-oauth (3 items separated by colons).")
+var noAdminSecret = errors.New("No admin-secret configured.")
+
+func (prov maasEnvironProvider) Validate(cfg, oldCfg *config.Config) (*config.Config, error) {
+	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{})
+	oauth := envCfg.MAASOAuth()
+	if strings.Count(oauth, ":") != 2 {
+		return nil, malformedMaasOAuth
+	}
+	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-02-05 09:22:25 +0000
@@ -0,0 +1,79 @@
+package maas
+
+import (
+	. "launchpad.net/gocheck"
+	"launchpad.net/juju-core/environs"
+	"launchpad.net/juju-core/testing"
+)
+
+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",
+		"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 := "maas.example.com"
+	oauth := "consumer-key:resource-token:resource-secret"
+	secret := "ssssssht"
+	ecfg, err := newConfig(map[string]interface{}{
+		"maas-server":  server,
+		"maas-oauth":   oauth,
+		"admin-secret": secret,
+	})
+	c.Assert(err, IsNil)
+	c.Check(ecfg.MAASServer(), Equals, server)
+	c.Check(ecfg.MAASOAuth(), DeepEquals, oauth)
+	c.Check(ecfg.AdminSecret(), Equals, secret)
+}
+
+func (ConfigSuite) TestRequiresMaasServer(c *C) {
+	oauth := "consumer-key:resource-token:resource-secret"
+	_, err := newConfig(map[string]interface{}{
+		"maas-oauth":   oauth,
+		"admin-secret": "secret",
+	})
+	c.Check(err, NotNil)
+}
+
+func (ConfigSuite) TestRequiresOAuth(c *C) {
+	_, err := newConfig(map[string]interface{}{
+		"maas-server":  "maas.example.com",
+		"admin-secret": "secret",
+	})
+	c.Check(err, NotNil)
+}
+
+func (ConfigSuite) TestChecksWellFormedOAuth(c *C) {
+	_, err := newConfig(map[string]interface{}{
+		"maas-server":  "maas.example.com",
+		"maas-oauth":   "This should have been a 3-part token.",
+		"admin-secret": "secret",
+	})
+	c.Check(err, NotNil)
+}

=== modified file 'environs/maas/environ.go'
--- environs/maas/environ.go	2013-01-29 09:51:16 +0000
+++ environs/maas/environ.go	2013-02-05 09:22:25 +0000
@@ -7,10 +7,16 @@
 	"launchpad.net/juju-core/log"
 	"launchpad.net/juju-core/state"
 	"launchpad.net/juju-core/state/api"
+	"sync"
 )
 
 type maasEnviron struct {
 	name string
+
+	// ecfgMutext protects the *Unlocked fields below.
+	ecfgMutex sync.Mutex
+
+	ecfgUnlocked *maasEnvironConfig
 }
 
 var _ environs.Environ = (*maasEnviron)(nil)
@@ -42,13 +48,31 @@
 	panic("Not implemented.")
 }
 
-func (*maasEnviron) Config() *config.Config {
-	panic("Not implemented.")
+// 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
+}
+
+func (env *maasEnviron) Config() *config.Config {
+	return env.ecfg().Config
 }
 
 func (env *maasEnviron) SetConfig(cfg *config.Config) error {
+	ecfg, err := env.Provider().(*maasEnvironProvider).newConfig(cfg)
+	if err != nil {
+		return err
+	}
+
+	env.ecfgMutex.Lock()
+	defer env.ecfgMutex.Unlock()
+
 	env.name = cfg.Name()
-	panic("Not implemented.")
+	env.ecfgUnlocked = ecfg
+
+	return nil
 }
 
 func (*maasEnviron) StartInstance(machineId string, info *state.Info, apiInfo *api.Info, tools *state.Tools) (environs.Instance, error) {
@@ -97,5 +121,5 @@
 }
 
 func (*maasEnviron) Provider() environs.EnvironProvider {
-	panic("Not implemented.")
+	return &providerInstance
 }

=== modified file 'environs/maas/environprovider.go'
--- environs/maas/environprovider.go	2013-01-31 06:27:18 +0000
+++ environs/maas/environprovider.go	2013-02-05 09:22:25 +0000
@@ -10,15 +10,17 @@
 
 var _ environs.EnvironProvider = (*maasEnvironProvider)(nil)
 
+var providerInstance maasEnvironProvider
+
+func init() {
+	environs.RegisterProvider("maas", &providerInstance)
+}
+
 func (*maasEnvironProvider) Open(cfg *config.Config) (environs.Environ, error) {
 	log.Printf("environs/maas: opening environment %q.", cfg.Name())
 	return NewEnviron(cfg)
 }
 
-func (*maasEnvironProvider) Validate(cfg, old *config.Config) (*config.Config, error) {
-	return cfg, nil
-}
-
 func (*maasEnvironProvider) SecretAttrs(*config.Config) (map[string]interface{}, error) {
 	panic("Not implemented.")
 }