Merge pull request #5421 from davecheney/client-modeluuid

api: do not return "" for an invalid model uuid

Rather than logging an error (at warning level) when a model has no
valid uuid, return an error.

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

api/client.go

@@ -90,7 +90,7 @@ func (c *Client) StatusHistory(kind status.HistoryKind, tag names.Tag, filter st
 		}
 		// TODO(perrito666) https://launchpad.net/bugs/1577589
 		if !history[i].Kind.Valid() {
-			logger.Warningf("history returned an unknown status kind %q", h.Kind)
+			logger.Errorf("history returned an unknown status kind %q", h.Kind)
 		}
 	}
 	return history, nil
@@ -210,13 +210,12 @@ func (c *Client) ModelInfo() (params.ModelInfo, error) {
 }
 
 // ModelUUID returns the model UUID from the client connection.
-func (c *Client) ModelUUID() string {
+func (c *Client) ModelUUID() (string, error) {
 	tag, err := c.st.ModelTag()
 	if err != nil {
-		logger.Warningf("model tag not an model: %v", err)
-		return ""
+		return "", errors.Annotate(err, "model tag not an model")
 	}
-	return tag.Id()
+	return tag.Id(), nil
 }
 
 // ModelUserInfo returns information on all users in the model.

api/client_test.go

@@ -295,7 +295,9 @@ func (s *clientSuite) TestClientEnvironmentUUID(c *gc.C) {
 	c.Assert(err, jc.ErrorIsNil)
 
 	client := s.APIState.Client()
-	c.Assert(client.ModelUUID(), gc.Equals, environ.Tag().Id())
+	uuid, err := client.ModelUUID()
+	c.Assert(err, jc.ErrorIsNil)
+	c.Assert(uuid, gc.Equals, environ.Tag().Id())
 }
 
 func (s *clientSuite) TestClientEnvironmentUsers(c *gc.C) {

apiserver/admin_test.go

@@ -767,7 +767,9 @@ func (s *loginSuite) assertRemoteEnvironment(c *gc.C, st api.Connection, expecte
 	client := st.Client()
 
 	// ModelUUID looks at the env tag on the api state connection.
-	c.Assert(client.ModelUUID(), gc.Equals, expected.Id())
+	uuid, err := client.ModelUUID()
+	c.Assert(err, jc.ErrorIsNil)
+	c.Assert(uuid, gc.Equals, expected.Id())
 
 	// ModelInfo calls a remote method that looks up the environment.
 	info, err := client.ModelInfo()

cmd/juju/machine/add.go

@@ -151,7 +151,7 @@ type AddMachineAPI interface {
 	Close() error
 	ForceDestroyMachines(machines ...string) error
 	ModelGet() (map[string]interface{}, error)
-	ModelUUID() string
+	ModelUUID() (string, error)
 	ProvisioningScript(params.ProvisioningScriptParams) (script string, err error)
 }
 
@@ -236,12 +236,16 @@ func (c *addCommand) Run(ctx *cmd.Context) error {
 
 	logger.Infof("model provisioning")
 	if c.Placement != nil && c.Placement.Scope == "model-uuid" {
-		c.Placement.Scope = client.ModelUUID()
+		uuid, err := client.ModelUUID()
+		if err != nil {
+			return errors.Trace(err)
+		}
+		c.Placement.Scope = uuid
 	}
 
 	if c.Placement != nil && c.Placement.Scope == instance.MachineScope {
 		// It does not make sense to add-machine <id>.
-		return fmt.Errorf("machine-id cannot be specified when adding machines")
+		return errors.Errorf("machine-id cannot be specified when adding machines")
 	}
 
 	jobs := []multiwatcher.MachineJob{multiwatcher.JobHostUnits}

cmd/juju/machine/add_test.go

@@ -231,8 +231,8 @@ func (f *fakeAddMachineAPI) Close() error {
 	return nil
 }
 
-func (f *fakeAddMachineAPI) ModelUUID() string {
-	return "fake-uuid"
+func (f *fakeAddMachineAPI) ModelUUID() (string, error) {
+	return "fake-uuid", nil
 }
 
 func (f *fakeAddMachineAPI) AddMachines(args []params.AddMachineParams) ([]params.AddMachinesResult, error) {

cmd/juju/service/deploy.go

@@ -586,10 +586,15 @@ func (c *DeployCommand) deployCharm(args deployCharmArgs) (rErr error) {
 		return errors.Trace(err)
 	}
 
+	uuid, err := args.client.ModelUUID()
+	if err != nil {
+		return errors.Trace(err)
+	}
+
 	deployInfo := DeploymentInfo{
 		CharmID:     args.id,
 		ServiceName: serviceName,
-		ModelUUID:   args.client.ModelUUID(),
+		ModelUUID:   uuid,
 	}
 
 	for _, step := range c.Steps {