environs: split out config validation

environs/ec2/ec2.go

 package ec2
 
 import (
         "fmt"
         "launchpad.net/goamz/aws"
         "launchpad.net/goamz/ec2"
         "launchpad.net/goamz/s3"
         "launchpad.net/juju-core/environs"
         "launchpad.net/juju-core/environs/config"
         "launchpad.net/juju-core/log"
@@ skipping 21 matching lines @@
         Total: 3 * time.Minute,
         Delay: 1 * time.Second,
 }
 
 func init() {
         environs.RegisterProvider("ec2", environProvider{})
 }
 
 type environProvider struct{}
 
-var _ environs.EnvironProvider = environProvider{}
+var providerInstance environProvider
 
 type environ struct {
         name string
 
-        // configMutex protects the *Unlocked fields below.
-        configMutex           sync.Mutex
-        configUnlocked        *providerConfig
+        // ecfgMutex protects the *Unlocked fields below.
+        ecfgMutex             sync.Mutex
+        ecfgUnlocked          *environConfig
         ec2Unlocked           *ec2.EC2
         s3Unlocked            *s3.S3
         storageUnlocked       *storage
         publicStorageUnlocked *storage // optional.
 }
 
 var _ environs.Environ = (*environ)(nil)
 
 type instance struct {
         e *environ
@@ skipping 42 matching lines @@
         log.Printf("environs/ec2: opening environment %q", cfg.Name())
         e := new(environ)
         err := e.SetConfig(cfg)
         if err != nil {
                 return nil, err
         }
         return e, nil
 }
 
 func (e *environ) SetConfig(cfg *config.Config) error {
-        config, err := newConfig(cfg)
+        ecfg, err := providerInstance.newConfig(cfg)
         if err != nil {
                 return err
         }
-        if aws.Regions[config.region].EC2Endpoint == "" {
-                return fmt.Errorf("environment %q references unknown EC2 region %q", config.name, config.region)

[fwereade, 2012/07/20 14:25:08]

...but I guess it's not worth checking for. I always did wonder about that :).

[niemeyer, 2012/07/20 18:14:45]

Agreed.

-        }
-        e.configMutex.Lock()
-        defer e.configMutex.Unlock()
-        // TODO(dfc) bug #1018207, renaming an environment once it is in use should be forbidden
-        e.name = config.name
-        e.configUnlocked = config
-        e.ec2Unlocked = ec2.New(config.auth, aws.Regions[config.region])
-        e.s3Unlocked = s3.New(config.auth, aws.Regions[config.region])
+        e.ecfgMutex.Lock()
+        defer e.ecfgMutex.Unlock()
+        e.name = ecfg.Name()
+        e.ecfgUnlocked = ecfg
+
+        auth := aws.Auth{ecfg.accessKey(), ecfg.secretKey()}
+        region := aws.Regions[ecfg.region()]
+        e.ec2Unlocked = ec2.New(auth, region)
+        e.s3Unlocked = s3.New(auth, region)
 
         // create new storage instances, existing instances continue
         // to reference their existing configuration.
         e.storageUnlocked = &storage{
-                bucket: e.s3Unlocked.Bucket(config.bucket),
+                bucket: e.s3Unlocked.Bucket(ecfg.controlBucket()),
         }
-        if config.publicBucket != "" {
+        if ecfg.publicBucket() != "" {
                 e.publicStorageUnlocked = &storage{
-                        bucket: e.s3Unlocked.Bucket(config.publicBucket),
+                        bucket: e.s3Unlocked.Bucket(ecfg.publicBucket()),
                 }
         } else {
                 e.publicStorageUnlocked = nil
         }
         return nil
 }
 
-func (e *environ) config() *providerConfig {
-        e.configMutex.Lock()
-        defer e.configMutex.Unlock()
-        return e.configUnlocked
+func (e *environ) ecfg() *environConfig {
+        e.ecfgMutex.Lock()
+        ecfg := e.ecfgUnlocked
+        e.ecfgMutex.Unlock()
+        return ecfg
 }
 
 func (e *environ) ec2() *ec2.EC2 {
-        e.configMutex.Lock()
-        defer e.configMutex.Unlock()
-        return e.ec2Unlocked
+        e.ecfgMutex.Lock()
+        ec2 := e.ec2Unlocked
+        e.ecfgMutex.Unlock()
+        return ec2
 }
 
 func (e *environ) s3() *s3.S3 {
-        e.configMutex.Lock()
-        defer e.configMutex.Unlock()
-        return e.s3Unlocked
+        e.ecfgMutex.Lock()
+        s3 := e.s3Unlocked
+        e.ecfgMutex.Unlock()
+        return s3
 }
 
 func (e *environ) Name() string {
-        return e.config().name
+        return e.name
 }
 
 func (e *environ) Storage() environs.Storage {
-        e.configMutex.Lock()
-        defer e.configMutex.Unlock()
-        return e.storageUnlocked
+        e.ecfgMutex.Lock()
+        storage := e.storageUnlocked
+        e.ecfgMutex.Unlock()
+        return storage
 }
 
 func (e *environ) PublicStorage() environs.StorageReader {
-        e.configMutex.Lock()
-        defer e.configMutex.Unlock()
+        e.ecfgMutex.Lock()
+        defer e.ecfgMutex.Unlock()
         if e.publicStorageUnlocked == nil {
                 return environs.EmptyStorage
         }
         return e.publicStorageUnlocked
 }
 
 func (e *environ) Bootstrap(uploadTools bool) error {
         log.Printf("environs/ec2: bootstrapping environment %q", e.name)
         _, err := e.loadState()
         if err == nil {
@@ skipping 76 matching lines @@
 
 func (e *environ) userData(machineId int, info *state.Info, master bool, toolsURL string) ([]byte, error) {
         cfg := &machineConfig{
                 provisioner:        master,
                 zookeeper:          master,
                 stateInfo:          info,
                 instanceIdAccessor: "$(curl http://169.254.169.254/1.0/meta-data/instance-id)",
                 providerType:       "ec2",
                 toolsURL:           toolsURL,
                 machineId:          machineId,
-                authorizedKeys:     e.config().AuthorizedKeys(),
+                authorizedKeys:     e.ecfg().AuthorizedKeys(),
         }
         cloudcfg, err := newCloudInit(cfg)
         if err != nil {
                 return nil, err
         }
         return cloudcfg.Render()
 }
 
 // startInstance is the internal version of StartInstance, used by Bootstrap
 // as well as via StartInstance itself. If master is true, a bootstrap
 // instance will be started.
 func (e *environ) startInstance(machineId int, info *state.Info, master bool) (environs.Instance, error) {
         spec, err := findInstanceSpec(&instanceConstraint{
                 series: config.CurrentSeries,
                 arch:   config.CurrentArch,
-                region: e.config().region,
+                region: e.ecfg().region(),
         })
         if err != nil {
                 return nil, fmt.Errorf("cannot find image satisfying constraints: %v", err)
         }
         log.Debugf("looking for tools for version %v; instance spec %#v", version.Current, spec)
         toolsURL, err := environs.FindTools(e, version.Current, spec.series, spec.arch)
         if err != nil {
                 return nil, fmt.Errorf("cannot find juju tools that would work on the specified instance: %v", err)
         }
         userData, err := e.userData(machineId, info, master, toolsURL)
@@ skipping 156 matching lines @@
                 id := inst.(*instance).InstanceId
                 if !found[id] {
                         ids = append(ids, id)
                         found[id] = true
                 }
         }
         err = e.terminateInstances(ids)
         if err != nil {
                 return err
         }
-        // to properly observe e.storageUnlocked we need to get it's value while
-        // holding e.configMutex. e.Storage() does this for us, then we convert
+
+        // 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 := e.Storage().(*storage)
         err = st.deleteAll()
         if err != nil {
                 return err
         }
         return nil
 }
 
 func (e *environ) terminateInstances(ids []string) error {
         if len(ids) == 0 {
                 return nil
         }
         var err error
         ec2inst := e.ec2()
         for a := shortAttempt.Start(); a.Next(); {
                 _, err = ec2inst.TerminateInstances(ids)
                 if err == nil || ec2ErrCode(err) != "InvalidInstanceID.NotFound" {
                         return err
                 }
         }
         if len(ids) == 1 {
                 return err
         }
-        var firstErr error
         // If we get a NotFound error, it means that no instances have been
         // terminated even if some exist, so try them one by one, ignoring
         // NotFound errors.
+        var firstErr error
         for _, id := range ids {
                 _, err = ec2inst.TerminateInstances([]string{id})
                 if ec2ErrCode(err) == "InvalidInstanceID.NotFound" {
                         err = nil
                 }
                 if err != nil && firstErr == nil {
                         firstErr = err
                 }
         }
         return firstErr
@@ skipping 235 matching lines @@
 
 // If the err is of type *ec2.Error, ec2ErrCode returns
 // its code, otherwise it returns the empty string.
 func ec2ErrCode(err error) string {
         ec2err, _ := err.(*ec2.Error)
         if ec2err == nil {
                 return ""
         }
         return ec2err.Code
 }