Merge pull request #4386 from dimitern/maas-spaces-merge-master-c215028

Merge master @ c215028 into maas-spaces

Hopefully for the last time..

(Review request: http://reviews.vapour.ws/r/3829/)

apiserver/params/apierror.go

@@ -100,18 +100,19 @@ func ErrCode(err error) string {
 // ClientError maps errors returned from an RPC call into local errors with
 // appropriate values.
 func ClientError(err error) error {
-	rerr, ok := err.(*rpc.RequestError)
-	if !ok {
+	switch err := errors.Cause(err).(type) {
+	case *rpc.RequestError:
+		// We use our own error type rather than rpc.ServerError
+		// because we don't want the code or the "server error" prefix
+		// within the error message. Also, it's best not to make clients
+		// know that we're using the rpc package.
+		return &Error{
+			Message: err.Message,
+			Code:    err.Code,
+		}
+	default:
 		return err
 	}
-	// We use our own error type rather than rpc.ServerError
-	// because we don't want the code or the "server error" prefix
-	// within the error message. Also, it's best not to make clients
-	// know that we're using the rpc package.
-	return &Error{
-		Message: rerr.Message,
-		Code:    rerr.Code,
-	}
 }
 
 func IsCodeActionNotAvailable(err error) bool {

cmd/jujud/bootstrap.go

@@ -180,6 +180,13 @@ func (c *BootstrapCommand) Run(_ *cmd.Context) error {
 		return err
 	}
 
+	// When machine addresses are reported from state, they have
+	// duplicates removed.  We should do the same here so that
+	// there is not unnecessary churn in the mongo replicaset.
+	// TODO (cherylj) Add explicit unit tests for this - tracked
+	// by bug #1544158.
+	addrs = network.MergedAddresses([]network.Address{}, addrs)
+
 	// Generate a private SSH key for the controllers, and add
 	// the public key to the environment config. We'll add the
 	// private key to StateServingInfo below.

network/address.go

@@ -11,6 +11,7 @@ import (
 	"sort"
 
 	"github.com/juju/errors"
+	"github.com/juju/utils/set"
 )
 
 // Private network ranges for IPv4 and IPv6.
@@ -619,3 +620,25 @@ func ResolvableHostnames(addrs []Address) []Address {
 	}
 	return resolveableAddrs
 }
+
+// MergedAddresses provides a single list of addresses without duplicates
+// suitable for returning as an address list for a machine.
+// TODO (cherylj) Add explicit unit tests - tracked with bug #1544158
+func MergedAddresses(machineAddresses, providerAddresses []Address) []Address {
+	merged := make([]Address, 0, len(providerAddresses)+len(machineAddresses))
+	providerValues := set.NewStrings()
+	for _, address := range providerAddresses {
+		// Older versions of Juju may have stored an empty address so ignore it here.
+		if address.Value == "" || providerValues.Contains(address.Value) {
+			continue
+		}
+		providerValues.Add(address.Value)
+		merged = append(merged, address)
+	}
+	for _, address := range machineAddresses {
+		if !providerValues.Contains(address.Value) {
+			merged = append(merged, address)
+		}
+	}
+	return merged
+}

rpc/client.go

@@ -4,8 +4,9 @@
 package rpc
 
 import (
-	"errors"
 	"strings"
+
+	"github.com/juju/errors"
 )
 
 var ErrShutdown = errors.New("connection is shut down")
@@ -26,11 +27,10 @@ type RequestError struct {
 }
 
 func (e *RequestError) Error() string {
-	m := "request error: " + e.Message
 	if e.Code != "" {
-		m += " (" + e.Code + ")"
+		return e.Message + " (" + e.Code + ")"
 	}
-	return m
+	return e.Message
 }
 
 func (e *RequestError) ErrorCode() string {
@@ -124,11 +124,7 @@ func (conn *Conn) handleResponse(hdr *Header) error {
 		}
 		call.done()
 	}
-	if err != nil {
-		logger.Errorf("error handling response: %v", err)
-	}
-
-	return err
+	return errors.Annotate(err, "error handling response")
 }
 
 func (call *Call) done() {
@@ -142,15 +138,25 @@ func (call *Call) done() {
 	}
 }
 
-// Call invokes the named action on the object of the given type with
-// the given id.  The returned values will be stored in response, which
-// should be a pointer.  If the action fails remotely, the returned
-// error will be of type RequestError.  The params value may be nil if
-// no parameters are provided; the response value may be nil to indicate
-// that any result should be discarded.
+// Call invokes the named action on the object of the given type with the given
+// id. The returned values will be stored in response, which should be a pointer.
+// If the action fails remotely, the error will have a cause of type RequestError.
+// The params value may be nil if no parameters are provided; the response value
+// may be nil to indicate that any result should be discarded.
 func (conn *Conn) Call(req Request, params, response interface{}) error {
 	call := <-conn.Go(req, params, response, make(chan *Call, 1)).Done
-	return call.Error
+	switch call.Error.(type) {
+	case *RequestError:
+		// TODO(dfc) many callers assert the error.Error() value has a
+		// "request error: " prefix. Rather than encoding this text in
+		// the .Error() string value itself, use Annotate. This lets callers
+		// unwrap the error if needed to assert it by type or value or
+		// test its expected string value if required. The latter behaviour
+		// should be removed.
+		return errors.Annotate(call.Error, "request error")
+	default:
+		return errors.Trace(call.Error)
+	}
 }
 
 // Go invokes the request asynchronously.  It returns the Call structure representing

rpc/rpc_test.go

@@ -13,6 +13,7 @@ import (
 	stdtesting "testing"
 	"time"
 
+	"github.com/juju/errors"
 	"github.com/juju/loggo"
 	jc "github.com/juju/testing/checkers"
 	gc "gopkg.in/check.v1"
@@ -470,7 +471,7 @@ func (root *Root) testCall(c *gc.C, p testCallParams) {
 	err := p.client.Call(p.request(), stringVal{"arg"}, &r)
 	switch {
 	case p.retErr && p.testErr:
-		c.Assert(err, gc.DeepEquals, &rpc.RequestError{
+		c.Assert(errors.Cause(err), gc.DeepEquals, &rpc.RequestError{
 			Message: p.errorMessage(),
 		})
 		c.Assert(r, gc.Equals, stringVal{})
@@ -610,7 +611,7 @@ func (*rpcSuite) TestInterfaceMethods(c *gc.C) {
 	// CodeNotImplemented.
 	var r stringVal
 	err := client.Call(rpc.Request{"InterfaceMethods", 0, "a99", "Call0r0"}, stringVal{"arg"}, &r)
-	c.Assert(err, gc.DeepEquals, &rpc.RequestError{
+	c.Assert(errors.Cause(err), gc.DeepEquals, &rpc.RequestError{
 		Message: "no such request - method InterfaceMethods.Call0r0 is not implemented",
 		Code:    rpc.CodeNotImplemented,
 	})
@@ -637,7 +638,7 @@ func (*rpcSuite) TestCustomMethodFinderV0(c *gc.C) {
 	// Call1r1 is exposed in version 1, but not in version 0.
 	var r stringVal
 	err := client.Call(rpc.Request{"MultiVersion", 0, "a99", "Call1r1"}, stringVal{"arg"}, &r)
-	c.Assert(err, gc.DeepEquals, &rpc.RequestError{
+	c.Assert(errors.Cause(err), gc.DeepEquals, &rpc.RequestError{
 		Message: "no such request - method MultiVersion.Call1r1 is not implemented",
 		Code:    rpc.CodeNotImplemented,
 	})
@@ -664,7 +665,7 @@ func (*rpcSuite) TestCustomMethodFinderV1(c *gc.C) {
 	// Call0r1 is exposed in version 0, but not in version 1.
 	var r stringVal
 	err := client.Call(rpc.Request{"MultiVersion", 1, "a99", "Call0r1"}, nil, &r)
-	c.Assert(err, gc.DeepEquals, &rpc.RequestError{
+	c.Assert(errors.Cause(err), gc.DeepEquals, &rpc.RequestError{
 		Message: "no such request - method MultiVersion(1).Call0r1 is not implemented",
 		Code:    rpc.CodeNotImplemented,
 	})
@@ -692,7 +693,7 @@ func (*rpcSuite) TestCustomMethodFinderV2(c *gc.C) {
 	// in InterfaceMethods.
 	var r stringVal
 	err := client.Call(rpc.Request{"MultiVersion", 2, "a99", "Call0r1e"}, nil, &r)
-	c.Assert(err, gc.DeepEquals, &rpc.RequestError{
+	c.Assert(errors.Cause(err), gc.DeepEquals, &rpc.RequestError{
 		Message: `no such request - method MultiVersion(2).Call0r1e is not implemented`,
 		Code:    rpc.CodeNotImplemented,
 	})
@@ -705,7 +706,7 @@ func (*rpcSuite) TestCustomMethodFinderUnknownVersion(c *gc.C) {
 	var r stringVal
 	// Unknown version 5
 	err := client.Call(rpc.Request{"MultiVersion", 5, "a99", "Call0r1"}, nil, &r)
-	c.Assert(err, gc.DeepEquals, &rpc.RequestError{
+	c.Assert(errors.Cause(err), gc.DeepEquals, &rpc.RequestError{
 		Message: `unknown version (5) of interface "MultiVersion"`,
 		Code:    rpc.CodeNotImplemented,
 	})
@@ -770,7 +771,7 @@ func (*rpcSuite) TestErrorCode(c *gc.C) {
 	defer closeClient(c, client, srvDone)
 	err := client.Call(rpc.Request{"ErrorMethods", 0, "", "Call"}, nil, nil)
 	c.Assert(err, gc.ErrorMatches, `request error: message \(code\)`)
-	c.Assert(err.(rpc.ErrorCoder).ErrorCode(), gc.Equals, "code")
+	c.Assert(errors.Cause(err).(rpc.ErrorCoder).ErrorCode(), gc.Equals, "code")
 }
 
 func (*rpcSuite) TestTransformErrors(c *gc.C) {
@@ -791,21 +792,21 @@ func (*rpcSuite) TestTransformErrors(c *gc.C) {
 	defer closeClient(c, client, srvDone)
 	// First, we don't transform methods we can't find.
 	err := client.Call(rpc.Request{"foo", 0, "", "bar"}, nil, nil)
-	c.Assert(err, gc.DeepEquals, &rpc.RequestError{
+	c.Assert(errors.Cause(err), gc.DeepEquals, &rpc.RequestError{
 		Message: `unknown object type "foo"`,
 		Code:    rpc.CodeNotImplemented,
 	})
 
 	err = client.Call(rpc.Request{"ErrorMethods", 0, "", "NoMethod"}, nil, nil)
-	c.Assert(err, gc.DeepEquals, &rpc.RequestError{
+	c.Assert(errors.Cause(err), gc.DeepEquals, &rpc.RequestError{
 		Message: "no such request - method ErrorMethods.NoMethod is not implemented",
 		Code:    rpc.CodeNotImplemented,
 	})
 
 	// We do transform any errors that happen from calling the RootMethod
 	// and beyond.
 	err = client.Call(rpc.Request{"ErrorMethods", 0, "", "Call"}, nil, nil)
-	c.Assert(err, gc.DeepEquals, &rpc.RequestError{
+	c.Assert(errors.Cause(err), gc.DeepEquals, &rpc.RequestError{
 		Message: "transformed: message",
 		Code:    "transformed: code",
 	})
@@ -816,7 +817,7 @@ func (*rpcSuite) TestTransformErrors(c *gc.C) {
 
 	root.errorInst = nil
 	err = client.Call(rpc.Request{"ErrorMethods", 0, "", "Call"}, nil, nil)
-	c.Assert(err, gc.DeepEquals, &rpc.RequestError{
+	c.Assert(errors.Cause(err), gc.DeepEquals, &rpc.RequestError{
 		Message: "transformed: no error methods",
 	})
 
@@ -839,7 +840,7 @@ func (*rpcSuite) TestServerWaitsForOutstandingCalls(c *gc.C) {
 	go func() {
 		var r stringVal
 		err := client.Call(rpc.Request{"DelayedMethods", 0, "1", "Delay"}, nil, &r)
-		c.Check(err, gc.Equals, rpc.ErrShutdown)
+		c.Check(errors.Cause(err), gc.Equals, rpc.ErrShutdown)
 		done <- struct{}{}
 	}()
 	chanRead(c, ready, "DelayedMethods.Delay ready")
@@ -1045,7 +1046,7 @@ func (*rpcSuite) TestErrorAfterClientClose(c *gc.C) {
 	err := client.Close()
 	c.Assert(err, jc.ErrorIsNil)
 	err = client.Call(rpc.Request{"Foo", 0, "", "Bar"}, nil, nil)
-	c.Assert(err, gc.Equals, rpc.ErrShutdown)
+	c.Assert(errors.Cause(err), gc.Equals, rpc.ErrShutdown)
 	err = chanReadError(c, srvDone, "server done")
 	c.Assert(err, jc.ErrorIsNil)
 }

scripts/win-installer/setup.iss

@@ -2,7 +2,7 @@
 ; SEE THE DOCUMENTATION FOR DETAILS ON CREATING INNO SETUP SCRIPT FILES!
 
 #define MyAppName "Juju"
-#define MyAppVersion "2.0-alpha2"
+#define MyAppVersion "2.0-beta1"
 #define MyAppPublisher "Canonical, Ltd"
 #define MyAppURL "http://juju.ubuntu.com/"
 #define MyAppExeName "juju.exe"

state/allwatcher.go

@@ -140,7 +140,7 @@ func (m *backingMachine) updated(st *State, store *multiwatcherStore, id string)
 		Life:                     multiwatcher.Life(m.Life.String()),
 		Series:                   m.Series,
 		Jobs:                     paramsJobsFromJobs(m.Jobs),
-		Addresses:                mergedAddresses(m.MachineAddresses, m.Addresses),
+		Addresses:                network.MergedAddresses(networkAddresses(m.MachineAddresses), networkAddresses(m.Addresses)),
 		SupportedContainers:      m.SupportedContainers,
 		SupportedContainersKnown: m.SupportedContainersKnown,
 		HasVote:                  m.HasVote,

state/machine.go

@@ -1141,25 +1141,6 @@ func (m *Machine) SetInstanceInfo(
 	return m.SetProvisioned(id, nonce, characteristics)
 }
 
-func mergedAddresses(machineAddresses, providerAddresses []address) []network.Address {
-	merged := make([]network.Address, 0, len(providerAddresses)+len(machineAddresses))
-	providerValues := set.NewStrings()
-	for _, address := range providerAddresses {
-		// Older versions of Juju may have stored an empty address so ignore it here.
-		if address.Value == "" || providerValues.Contains(address.Value) {
-			continue
-		}
-		providerValues.Add(address.Value)
-		merged = append(merged, address.networkAddress())
-	}
-	for _, address := range machineAddresses {
-		if !providerValues.Contains(address.Value) {
-			merged = append(merged, address.networkAddress())
-		}
-	}
-	return merged
-}
-
 // Addresses returns any hostnames and ips associated with a machine,
 // determined both by the machine itself, and by asking the provider.
 //
@@ -1168,7 +1149,7 @@ func mergedAddresses(machineAddresses, providerAddresses []address) []network.Ad
 // Provider-reported addresses always come before machine-reported
 // addresses. Duplicates are removed.
 func (m *Machine) Addresses() (addresses []network.Address) {
-	return mergedAddresses(m.doc.MachineAddresses, m.doc.Addresses)
+	return network.MergedAddresses(networkAddresses(m.doc.MachineAddresses), networkAddresses(m.doc.Addresses))
 }
 
 func containsAddress(addresses []address, address address) bool {

version/version.go

@@ -22,7 +22,7 @@ import (
 // The presence and format of this constant is very important.
 // The debian/rules build recipe uses this value for the version
 // number of the release package.
-const version = "2.0-alpha2"
+const version = "2.0-beta1"
 
 // The version that we switched over from old style numbering to new style.
 var switchOverVersion = MustParse("1.19.9")