Review comments implemented

MalloZup · Feb 6, 2015 · 044629e · 044629e
1 parent 3089068
commit 044629e
Show file tree

Hide file tree

Showing 10 changed files with 38 additions and 33 deletions.
diff --git a/provider/cloudsigma/client.go b/provider/cloudsigma/client.go
@@ -100,6 +100,7 @@ func (c *environClient) instances() ([]gosigma.Server, error) {
 func (c *environClient) instanceMap() (map[string]gosigma.Server, error) {
 	servers, err := c.conn.ServersFiltered(gosigma.RequestDetail, c.isMyServer)
 	if err != nil {
+		errors.Trace(err)
 		return nil, err
 	}
 
@@ -117,6 +118,7 @@ func (c *environClient) getStateServerIds() (ids []instance.Id, err error) {
 
 	servers, err := c.conn.ServersFiltered(gosigma.RequestDetail, c.isMyStateServer)
 	if err != nil {
+		errors.Trace(err)
 		return []instance.Id{}, err
 	}
 
@@ -143,6 +145,7 @@ func (c *environClient) stopInstance(id instance.Id) error {
 
 	s, err := c.conn.Server(uuid)
 	if err != nil {
+		errors.Trace(err)
 		return err
 	}
 
@@ -180,10 +183,7 @@ func (c *environClient) newInstance(args environs.StartInstanceParams, img *imag
 	logger.Debugf("Juju Constraints:" + args.Constraints.String())
 	logger.Debugf("MachineConfig: %#v", args.MachineConfig)
 
-	constraints, err := newConstraints(args.MachineConfig.Bootstrap, args.Constraints, img)
-	if err != nil {
-		return nil, nil, "", err
-	}
+	constraints := newConstraints(args.MachineConfig.Bootstrap, args.Constraints, img)
 	logger.Debugf("CloudSigma Constraints: %v", constraints)
 
 	originalDrive, err := c.conn.Drive(constraints.driveTemplate, gosigma.LibraryMedia)
@@ -230,6 +230,7 @@ func (c *environClient) newInstance(args environs.StartInstanceParams, img *imag
 		arch = ar.I386
 	default:
 		err = errors.Errorf("unknown arch: %v", arch)
+		return nil, nil, "", err
 	}
 
 	return srv, drv, arch, nil
@@ -262,6 +263,7 @@ func (c *environClient) generateSigmaComponents(baseName string, constraints *si
 	cc.SetMeta(jujuMetaEnvironment, c.uuid)
 	data, err := utils.Gunzip(userData)
 	if err != nil {
+		errors.Trace(err)
 		return cc, err
 	}
 	cc.SetMeta(jujuMetaCoudInit, base64.StdEncoding.EncodeToString(data))

diff --git a/provider/cloudsigma/client_test.go b/provider/cloudsigma/client_test.go
@@ -233,7 +233,7 @@ func (s *clientSuite) TestClientNewInstance(c *gc.C) {
 	img := &imagemetadata.ImageMetadata{
 		Id: validImageId,
 	}
-	cs, err := newConstraints(params.MachineConfig.Bootstrap, params.Constraints, img)
+	cs := newConstraints(params.MachineConfig.Bootstrap, params.Constraints, img)
 	c.Assert(cs, gc.NotNil)
 	c.Check(err, gc.IsNil)
 

diff --git a/provider/cloudsigma/config.go b/provider/cloudsigma/config.go
@@ -59,6 +59,7 @@ func prepareConfig(cfg *config.Config) (*config.Config, error) {
 	if _, ok := attrs["uuid"]; !ok {
 		uuid, err := utils.NewUUID()
 		if err != nil {
+			errors.Trace(err)
 			return nil, err
 		}
 		attrs["uuid"] = uuid.String()
@@ -74,6 +75,7 @@ func validateConfig(cfg *config.Config, old *environConfig) (*environConfig, err
 		oldCfg = old.Config
 	}
 	if err := config.Validate(cfg, oldCfg); err != nil {
+		errors.Trace(err)
 		return nil, err
 	}
 
@@ -90,6 +92,7 @@ func validateConfig(cfg *config.Config, old *environConfig) (*environConfig, err
 	// will probably not have those variables set.
 	newAttrs, err := cfg.ValidateUnknownAttrs(configFields, configDefaultFields)
 	if err != nil {
+		errors.Trace(err)
 		return nil, err
 	}
 	for field := range configFields {
@@ -114,6 +117,7 @@ func validateConfig(cfg *config.Config, old *environConfig) (*environConfig, err
 	// to ensure the object we return is internally consistent.
 	newCfg, err := cfg.Apply(newAttrs)
 	if err != nil {
+		errors.Trace(err)
 		return nil, err
 	}
 	ecfg := &environConfig{

diff --git a/provider/cloudsigma/constraints.go b/provider/cloudsigma/constraints.go
@@ -27,7 +27,7 @@ const (
 )
 
 // newConstraints creates new CloudSigma constraints from juju common constraints
-func newConstraints(bootstrap bool, jc constraints.Value, img *imagemetadata.ImageMetadata) (*sigmaConstraints, error) {
+func newConstraints(bootstrap bool, jc constraints.Value, img *imagemetadata.ImageMetadata) *sigmaConstraints {
 	var sc sigmaConstraints
 
 	sc.driveTemplate = img.Id
@@ -62,7 +62,7 @@ func newConstraints(bootstrap bool, jc constraints.Value, img *imagemetadata.Ima
 		sc.mem = 2 * gosigma.Gigabyte
 	}
 
-	return &sc, nil
+	return &sc
 }
 
 func (c *sigmaConstraints) String() string {

diff --git a/provider/cloudsigma/constraints_test.go b/provider/cloudsigma/constraints_test.go
@@ -94,15 +94,9 @@ func (s *constraintsSuite) TestConstraints(c *gc.C) {
 		if t.disk != nil {
 			cv.RootDisk = &t.disk.v
 		}
-		v, err := newConstraints(t.bootstrap, cv, img)
-		if t.err == nil {
-			if !c.Check(*v, gc.Equals, t.expected) {
-				c.Logf("test (%d): %+v", i, t)
-			}
-		} else {
-			if !c.Check(err, gc.ErrorMatches, t.err.v) {
-				c.Logf("test (%d): %+v", i, t)
-			}
+		v := newConstraints(t.bootstrap, cv, img)
+		if !c.Check(*v, gc.Equals, t.expected) {
+			c.Logf("test (%d): %+v", i, t)
 		}
 	}
 }
@@ -117,7 +111,6 @@ func (s *constraintsSuite) TestConstraintsArch(c *gc.C) {
 		mem:           2 * gosigma.Gigabyte,
 	}
 
-	sc, err := newConstraints(true, cv, img)
-	c.Check(err, gc.IsNil)
+	sc := newConstraints(true, cv, img)
 	c.Check(*sc, gc.Equals, expected)
 }
diff --git a/provider/cloudsigma/environ.go b/provider/cloudsigma/environ.go
@@ -7,6 +7,7 @@ import (
 	"sync"
 
 	"github.com/Altoros/gosigma"
+	"github.com/juju/errors"
 
 	"github.com/juju/juju/constraints"
 	"github.com/juju/juju/environs"
@@ -58,12 +59,14 @@ func (env *environ) SetConfig(cfg *config.Config) error {
 
 	ecfg, err := validateConfig(cfg, env.ecfg)
 	if err != nil {
+		errors.Trace(err)
 		return err
 	}
 
 	if env.client == nil || env.ecfg == nil || env.ecfg.clientConfigChanged(ecfg) {
 		client, err := newClient(ecfg)
 		if err != nil {
+			errors.Trace(err)
 			return err
 		}
 
@@ -136,6 +139,7 @@ func (env *environ) Region() (simplestreams.CloudSpec, error) {
 func (env *environ) cloudSpec(region string) (simplestreams.CloudSpec, error) {
 	endpoint, err := gosigma.ResolveEndpoint(region)
 	if err != nil {
+		errors.Trace(err)
 		return simplestreams.CloudSpec{}, err
 	}
 	return simplestreams.CloudSpec{

diff --git a/provider/cloudsigma/environ_test.go b/provider/cloudsigma/environ_test.go
@@ -70,7 +70,6 @@ func (s *environSuite) TestBase(c *gc.C) {
 	c.Check(validator, gc.NotNil)
 	c.Check(err, gc.IsNil)
 
-	c.Check(env.SupportNetworks(), gc.Equals, false)
 	c.Check(env.SupportsUnitPlacement(), gc.IsNil)
 
 	c.Check(env.OpenPorts(nil), gc.IsNil)

diff --git a/provider/cloudsigma/environcaps.go b/provider/cloudsigma/environcaps.go
@@ -7,7 +7,6 @@ import (
 	"github.com/juju/juju/constraints"
 	"github.com/juju/juju/environs/imagemetadata"
 	"github.com/juju/juju/environs/simplestreams"
-	"github.com/juju/juju/network"
 	"github.com/juju/juju/provider/common"
 )
 
@@ -31,10 +30,6 @@ func (env *environ) SupportedArchitectures() ([]string, error) {
 	return env.supportedArchitectures, err
 }
 
-func (env *environ) SupportAddressAllocation(netId network.Id) (bool, error) {
-	return false, nil
-}
-
 var unsupportedConstraints = []string{
 	constraints.Container,
 	constraints.InstanceType,

diff --git a/provider/cloudsigma/environinstance_test.go b/provider/cloudsigma/environinstance_test.go
@@ -15,7 +15,6 @@ import (
 	"github.com/juju/juju/environs/config"
 	"github.com/juju/juju/environs/imagemetadata"
 	"github.com/juju/juju/instance"
-	"github.com/juju/juju/network"
 	"github.com/juju/juju/testing"
 	"github.com/juju/juju/tools"
 	"github.com/juju/juju/version"
@@ -154,13 +153,6 @@ func (s *environInstanceSuite) TestInstancesFail(c *gc.C) {
 	c.Assert(err, gc.NotNil)
 }
 
-func (s *environInstanceSuite) TestAllocateAddress(c *gc.C) {
-	environ := s.createEnviron(c, nil)
-
-	err := environ.AllocateAddress(instance.Id(""), network.Id(""), network.Address{})
-	c.Check(err, gc.ErrorMatches, "AllocateAddress not supported")
-}
-
 func (s *environInstanceSuite) TestStartInstanceError(c *gc.C) {
 	environ := s.createEnviron(c, nil)
 

diff --git a/provider/cloudsigma/provider.go b/provider/cloudsigma/provider.go
@@ -83,11 +83,27 @@ func (environProvider) Open(cfg *config.Config) (environs.Environ, error) {
 	return env, nil
 }
 
+// RestrictedConfigAttributes are provider specific attributes stored in
+// the config that really cannot or should not be changed across
+// environments running inside a single juju server.
+func (environProvider) RestrictedConfigAttributes() []string {
+	return []string{"region"}
+}
+
+// PrepareForCreateEnvironment prepares an environment for creation. Any
+// additional configuration attributes are added to the config passed in
+// and returned.  This allows providers to add additional required config
+// for new environments that may be created in an existing juju server.
+func (environProvider) PrepareForCreateEnvironment(cfg *config.Config) (*config.Config, error) {
+	// Not even sure if this will ever make sense.
+	return nil, errors.NotImplementedf("PrepareForCreateEnvironment")
+}
+
 // Prepare prepares an environment for use. Any additional
 // configuration attributes in the returned environment should
 // be saved to be used later. If the environment is already
 // prepared, this call is equivalent to Open.
-func (environProvider) Prepare(ctx environs.BootstrapContext, cfg *config.Config) (environs.Environ, error) {
+func (environProvider) PrepareForBootstrap(ctx environs.BootstrapContext, cfg *config.Config) (environs.Environ, error) {
 	logger.Infof("preparing environment %q", cfg.Name())
 	return providerInstance.Open(cfg)
 }