Merge pull request juju#8665 from wallyworld/iaas-only-commands

juju#8665 ## Description of change We need to gracefully reject IAAS only commands run on a CAAS model. There's 2 commits here owing to the testing fallout caused by the change. Commit 1: introduce a marker interface which is embedded in IAAS only commands. The base model command determines the model type and rejects IAAS only commands on CAAS models. The lookup of model type is done as early as possible, starting with during Init(). This allows the user to get feedback ASAP that the command isn;t suitable, even if they haven't supplied all required args etc This resulted in some refactoring of how client store etc was obtained. There was a large test fallout. Commit 2: fix all the failing tests. The main requirement was to ensure a valid client store was available for all commands, even for tests which only called Init(). ## QA steps Run up both a CAAS and IAAS model. Ensure common commands work across both models, and IAAS only commands fail on the CAAS model. Use combinations of -m <model> and switch etc to test different model selection scenarios.
MalloZup · Apr 27, 2018 · b47854c · b47854c
2 parents 280d8e4 + 9fa8b37
commit b47854c
Show file tree

Hide file tree

Showing 97 changed files with 806 additions and 274 deletions.
diff --git a/cmd/juju/action/action.go b/cmd/juju/action/action.go
@@ -61,6 +61,7 @@ type APIClient interface {
 // ActionCommandBase is the base type for action sub-commands.
 type ActionCommandBase struct {
 	modelcmd.ModelCommandBase
+	modelcmd.IAASOnlyCommand
 }
 
 // NewActionAPIClient returns a client for the action api endpoint.

diff --git a/cmd/juju/action/package_test.go b/cmd/juju/action/package_test.go
@@ -52,6 +52,9 @@ func (s *BaseActionSuite) SetUpTest(c *gc.C) {
 
 	s.store = jujuclient.NewMemStore()
 	s.store.CurrentControllerName = "ctrl"
+	s.store.Controllers["ctrl"] = jujuclient.ControllerDetails{}
+	s.store.Models["ctrl"] = &jujuclient.ControllerModels{
+		Models: map[string]jujuclient.ModelDetails{"admin/admin": {ModelType: "iaas"}}}
 	s.store.Accounts["ctrl"] = jujuclient.AccountDetails{
 		User: "admin",
 	}

diff --git a/cmd/juju/application/addrelation_test.go b/cmd/juju/application/addrelation_test.go
@@ -15,6 +15,7 @@ import (
 	"github.com/juju/juju/apiserver/common"
 	"github.com/juju/juju/apiserver/params"
 	"github.com/juju/juju/core/crossmodel"
+	"github.com/juju/juju/jujuclient/jujuclienttesting"
 	jtesting "github.com/juju/juju/testing"
 )
 
@@ -37,7 +38,7 @@ var _ = gc.Suite(&AddRelationSuite{})
 
 func (s *AddRelationSuite) runAddRelation(c *gc.C, args ...string) error {
 	cmd := NewAddRelationCommandForTest(s.mockAPI, s.mockAPI)
-	cmd.SetClientStore(NewMockStore())
+	cmd.SetClientStore(jujuclienttesting.MinimalStore())
 	_, err := cmdtesting.RunCommand(c, cmd, args...)
 	return err
 }
@@ -86,7 +87,7 @@ func (s *AddRelationSuite) TestAddRelationUnauthorizedMentionsJujuGrant(c *gc.C)
 		Code:    params.CodeUnauthorized,
 	})
 	cmd := NewAddRelationCommandForTest(s.mockAPI, s.mockAPI)
-	cmd.SetClientStore(NewMockStore())
+	cmd.SetClientStore(jujuclienttesting.MinimalStore())
 	ctx, _ := cmdtesting.RunCommand(c, cmd, "application1", "application2")
 	errString := strings.Replace(cmdtesting.Stderr(ctx), "\n", " ", -1)
 	c.Assert(errString, gc.Matches, `.*juju grant.*`)

diff --git a/cmd/juju/application/addunit_test.go b/cmd/juju/application/addunit_test.go
@@ -17,6 +17,7 @@ import (
 	"github.com/juju/juju/cmd/juju/application"
 	"github.com/juju/juju/environs/config"
 	"github.com/juju/juju/instance"
+	"github.com/juju/juju/jujuclient/jujuclienttesting"
 	"github.com/juju/juju/testing"
 )
 
@@ -107,13 +108,13 @@ var initAddUnitErrorTests = []struct {
 func (s *AddUnitSuite) TestInitErrors(c *gc.C) {
 	for i, t := range initAddUnitErrorTests {
 		c.Logf("test %d", i)
-		err := cmdtesting.InitCommand(application.NewAddUnitCommandForTest(s.fake), t.args)
+		err := cmdtesting.InitCommand(application.NewAddUnitCommandForTest(s.fake, jujuclienttesting.MinimalStore()), t.args)
 		c.Check(err, gc.ErrorMatches, t.err)
 	}
 }
 
 func (s *AddUnitSuite) runAddUnit(c *gc.C, args ...string) error {
-	_, err := cmdtesting.RunCommand(c, application.NewAddUnitCommandForTest(s.fake), args...)
+	_, err := cmdtesting.RunCommand(c, application.NewAddUnitCommandForTest(s.fake, jujuclienttesting.MinimalStore()), args...)
 	return err
 }
 
@@ -176,7 +177,8 @@ func (s *AddUnitSuite) TestUnauthorizedMentionsJujuGrant(c *gc.C) {
 		Message: "permission denied",
 		Code:    params.CodeUnauthorized,
 	}
-	ctx, _ := cmdtesting.RunCommand(c, application.NewAddUnitCommandForTest(s.fake), "some-application-name")
+	ctx, _ := cmdtesting.RunCommand(c, application.NewAddUnitCommandForTest(
+		s.fake, jujuclienttesting.MinimalStore()), "some-application-name")
 	errString := strings.Replace(cmdtesting.Stderr(ctx), "\n", " ", -1)
 	c.Assert(errString, gc.Matches, `.*juju grant.*`)
 }

diff --git a/cmd/juju/application/cmd_test.go b/cmd/juju/application/cmd_test.go
@@ -11,7 +11,7 @@ import (
 	gc "gopkg.in/check.v1"
 
 	"github.com/juju/juju/cmd/modelcmd"
-	"github.com/juju/juju/jujuclient"
+	"github.com/juju/juju/jujuclient/jujuclienttesting"
 	coretesting "github.com/juju/juju/testing"
 )
 
@@ -77,7 +77,7 @@ func (s *CmdSuite) TestDeployCommandInit(c *gc.C) {
 	for i, t := range deployTests {
 		c.Logf("\ntest %d: %s", i, t.about)
 		wrappedDeployCmd := NewDeployCommandForTest(nil, nil)
-		wrappedDeployCmd.SetClientStore(jujuclient.NewMemStore())
+		wrappedDeployCmd.SetClientStore(jujuclienttesting.MinimalStore())
 		err := cmdtesting.InitCommand(wrappedDeployCmd, t.args)
 		if t.expectError != "" {
 			c.Assert(err, gc.ErrorMatches, t.expectError)
@@ -100,7 +100,7 @@ func (s *CmdSuite) TestDeployCommandInit(c *gc.C) {
 
 func (*CmdSuite) TestExposeCommandInitWithMissingArgs(c *gc.C) {
 	cmd := NewExposeCommand()
-	cmd.SetClientStore(NewMockStore())
+	cmd.SetClientStore(jujuclienttesting.MinimalStore())
 	err := cmdtesting.InitCommand(cmd, nil)
 	c.Assert(err, gc.ErrorMatches, "no application name specified")
 
@@ -109,34 +109,21 @@ func (*CmdSuite) TestExposeCommandInitWithMissingArgs(c *gc.C) {
 
 func (*CmdSuite) TestUnexposeCommandInitWithMissingArgs(c *gc.C) {
 	cmd := NewUnexposeCommand()
-	cmd.SetClientStore(NewMockStore())
+	cmd.SetClientStore(jujuclienttesting.MinimalStore())
 	err := cmdtesting.InitCommand(cmd, nil)
 	c.Assert(err, gc.ErrorMatches, "no application name specified")
 }
 
 func (*CmdSuite) TestRemoveUnitCommandInitMissingArgs(c *gc.C) {
 	cmd := NewRemoveUnitCommand()
-	cmd.SetClientStore(NewMockStore())
+	cmd.SetClientStore(jujuclienttesting.MinimalStore())
 	err := cmdtesting.InitCommand(cmd, nil)
 	c.Assert(err, gc.ErrorMatches, "no units specified")
 }
 
 func (*CmdSuite) TestRemoveUnitCommandInitInvalidUnit(c *gc.C) {
 	cmd := NewRemoveUnitCommand()
-	cmd.SetClientStore(NewMockStore())
+	cmd.SetClientStore(jujuclienttesting.MinimalStore())
 	err := cmdtesting.InitCommand(cmd, []string{"seven/nine"})
 	c.Assert(err, gc.ErrorMatches, `invalid unit name "seven/nine"`)
 }
-
-func NewMockStore() *jujuclient.MemStore {
-	store := jujuclient.NewMemStore()
-	store.CurrentControllerName = "foo"
-	store.Controllers["foo"] = jujuclient.ControllerDetails{
-		APIEndpoints: []string{"0.1.2.3:1234"},
-	}
-	store.Models["foo"] = &jujuclient.ControllerModels{
-		CurrentModel: "admin/bar",
-		Models:       map[string]jujuclient.ModelDetails{"admin/bar": {}},
-	}
-	return store
-}
diff --git a/cmd/juju/application/config.go b/cmd/juju/application/config.go
@@ -20,6 +20,7 @@ import (
 	"github.com/juju/juju/cmd/juju/block"
 	"github.com/juju/juju/cmd/modelcmd"
 	"github.com/juju/juju/cmd/output"
+	"github.com/juju/juju/jujuclient"
 )
 
 const maxValueSize = 5242880 // Max size for a config file.
@@ -55,10 +56,12 @@ func NewConfigCommand() cmd.Command {
 }
 
 // NewConfigCommandForTest returns a SetCommand with the api provided as specified.
-func NewConfigCommandForTest(api applicationAPI) modelcmd.ModelCommand {
-	return modelcmd.Wrap(&configCommand{
+func NewConfigCommandForTest(api applicationAPI, store jujuclient.ClientStore) modelcmd.ModelCommand {
+	cmd := modelcmd.Wrap(&configCommand{
 		api: api,
 	})
+	cmd.SetClientStore(store)
+	return cmd
 }
 
 type attributes map[string]string

diff --git a/cmd/juju/application/config_test.go b/cmd/juju/application/config_test.go
@@ -19,13 +19,16 @@ import (
 
 	"github.com/juju/juju/apiserver/common"
 	"github.com/juju/juju/cmd/juju/application"
+	"github.com/juju/juju/jujuclient"
+	"github.com/juju/juju/jujuclient/jujuclienttesting"
 	coretesting "github.com/juju/juju/testing"
 )
 
 type configCommandSuite struct {
 	coretesting.FakeJujuXDGDataHomeSuite
 	dir                string
 	fake               *fakeApplicationAPI
+	store              jujuclient.ClientStore
 	defaultCharmValues map[string]interface{}
 	defaultAppValues   map[string]interface{}
 }
@@ -109,6 +112,7 @@ func (s *configCommandSuite) SetUpTest(c *gc.C) {
 		appValues:   s.defaultAppValues,
 		version:     6}
 	s.FakeJujuXDGDataHomeSuite.SetUpTest(c)
+	s.store = jujuclienttesting.MinimalStore()
 
 	s.dir = c.MkDir()
 	c.Assert(utf8.ValidString(validSetTestValue), jc.IsTrue)
@@ -121,18 +125,18 @@ func (s *configCommandSuite) SetUpTest(c *gc.C) {
 
 func (s *configCommandSuite) TestGetCommandInit(c *gc.C) {
 	// missing args
-	err := cmdtesting.InitCommand(application.NewConfigCommandForTest(s.fake), []string{})
+	err := cmdtesting.InitCommand(application.NewConfigCommandForTest(s.fake, s.store), []string{})
 	c.Assert(err, gc.ErrorMatches, "no application name specified")
 }
 
 func (s *configCommandSuite) TestGetCommandInitWithApplication(c *gc.C) {
-	err := cmdtesting.InitCommand(application.NewConfigCommandForTest(s.fake), []string{"app"})
+	err := cmdtesting.InitCommand(application.NewConfigCommandForTest(s.fake, s.store), []string{"app"})
 	// everything ok
 	c.Assert(err, jc.ErrorIsNil)
 }
 
 func (s *configCommandSuite) TestGetCommandInitWithKey(c *gc.C) {
-	err := cmdtesting.InitCommand(application.NewConfigCommandForTest(s.fake), []string{"app", "key"})
+	err := cmdtesting.InitCommand(application.NewConfigCommandForTest(s.fake, s.store), []string{"app", "key"})
 	// everything ok
 	c.Assert(err, jc.ErrorIsNil)
 }
@@ -143,7 +147,7 @@ func (s *configCommandSuite) TestGetConfig(c *gc.C) {
 			s.fake.appValues = nil
 		}
 		ctx := cmdtesting.Context(c)
-		code := cmd.Main(application.NewConfigCommandForTest(s.fake), ctx, []string{t.application})
+		code := cmd.Main(application.NewConfigCommandForTest(s.fake, s.store), ctx, []string{t.application})
 		c.Check(code, gc.Equals, 0)
 		c.Assert(ctx.Stderr.(*bytes.Buffer).String(), gc.Equals, "")
 		// round trip via goyaml to avoid being sucked into a quagmire of
@@ -164,22 +168,22 @@ func (s *configCommandSuite) TestGetConfig(c *gc.C) {
 
 func (s *configCommandSuite) TestGetCharmConfigKey(c *gc.C) {
 	ctx := cmdtesting.Context(c)
-	code := cmd.Main(application.NewConfigCommandForTest(s.fake), ctx, []string{"dummy-application", "title"})
+	code := cmd.Main(application.NewConfigCommandForTest(s.fake, s.store), ctx, []string{"dummy-application", "title"})
 	c.Check(code, gc.Equals, 0)
 	c.Assert(ctx.Stderr.(*bytes.Buffer).String(), gc.Equals, "")
 	c.Assert(ctx.Stdout.(*bytes.Buffer).String(), gc.Equals, "Nearly There\n")
 }
 
 func (s *configCommandSuite) TestGetAppConfigKey(c *gc.C) {
 	ctx := cmdtesting.Context(c)
-	code := cmd.Main(application.NewConfigCommandForTest(s.fake), ctx, []string{"dummy-application", "juju-external-hostname"})
+	code := cmd.Main(application.NewConfigCommandForTest(s.fake, s.store), ctx, []string{"dummy-application", "juju-external-hostname"})
 	c.Check(code, gc.Equals, 0)
 	c.Assert(ctx.Stderr.(*bytes.Buffer).String(), gc.Equals, "")
 	c.Assert(ctx.Stdout.(*bytes.Buffer).String(), gc.Equals, "ext-host\n")
 }
 
 func (s *configCommandSuite) TestGetConfigKeyNotFound(c *gc.C) {
-	_, err := cmdtesting.RunCommand(c, application.NewConfigCommandForTest(s.fake), "dummy-application", "invalid")
+	_, err := cmdtesting.RunCommand(c, application.NewConfigCommandForTest(s.fake, s.store), "dummy-application", "invalid")
 	c.Assert(err, gc.ErrorMatches, `key "invalid" not found in "dummy-application" application config or charm settings.`, gc.Commentf("details: %v", errors.Details(err)))
 }
 
@@ -225,10 +229,10 @@ var setCommandInitErrorTests = []struct {
 }}
 
 func (s *configCommandSuite) TestSetCommandInitError(c *gc.C) {
-	testStore := application.NewMockStore()
+	testStore := jujuclienttesting.MinimalStore()
 	for i, test := range setCommandInitErrorTests {
 		c.Logf("test %d: %s", i, test.about)
-		cmd := application.NewConfigCommandForTest(s.fake)
+		cmd := application.NewConfigCommandForTest(s.fake, s.store)
 		cmd.SetClientStore(testStore)
 		err := cmdtesting.InitCommand(cmd, test.args)
 		c.Assert(err, gc.ErrorMatches, test.expectError)
@@ -315,7 +319,7 @@ func (s *configCommandSuite) TestSetCharmConfigFromYAML(c *gc.C) {
 	}, ".*"+utils.NoSuchFileErrRegexp)
 
 	ctx := cmdtesting.ContextForDir(c, s.dir)
-	code := cmd.Main(application.NewConfigCommandForTest(s.fake), ctx, []string{
+	code := cmd.Main(application.NewConfigCommandForTest(s.fake, s.store), ctx, []string{
 		"dummy-application",
 		"--file",
 		"testconfig.yaml"})
@@ -328,7 +332,7 @@ func (s *configCommandSuite) TestSetFromStdin(c *gc.C) {
 	s.fake = &fakeApplicationAPI{name: "dummy-application"}
 	ctx := cmdtesting.Context(c)
 	ctx.Stdin = strings.NewReader("settings:\n  username:\n  value: world\n")
-	code := cmd.Main(application.NewConfigCommandForTest(s.fake), ctx, []string{
+	code := cmd.Main(application.NewConfigCommandForTest(s.fake, s.store), ctx, []string{
 		"dummy-application",
 		"--file",
 		"-"})
@@ -360,8 +364,8 @@ func (s *configCommandSuite) TestResetAppConfig(c *gc.C) {
 func (s *configCommandSuite) TestBlockSetConfig(c *gc.C) {
 	// Block operation
 	s.fake.err = common.OperationBlockedError("TestBlockSetConfig")
-	cmd := application.NewConfigCommandForTest(s.fake)
-	cmd.SetClientStore(application.NewMockStore())
+	cmd := application.NewConfigCommandForTest(s.fake, s.store)
+	cmd.SetClientStore(jujuclienttesting.MinimalStore())
 	_, err := cmdtesting.RunCommandInDir(c, cmd, []string{
 		"dummy-application",
 		"--file",
@@ -376,8 +380,8 @@ func (s *configCommandSuite) assertSetSuccess(
 	c *gc.C, dir string, args []string,
 	expectAppValues map[string]interface{}, expectCharmValues map[string]interface{},
 ) {
-	cmd := application.NewConfigCommandForTest(s.fake)
-	cmd.SetClientStore(application.NewMockStore())
+	cmd := application.NewConfigCommandForTest(s.fake, s.store)
+	cmd.SetClientStore(jujuclienttesting.MinimalStore())
 
 	args = append([]string{"dummy-application"}, args...)
 	_, err := cmdtesting.RunCommandInDir(c, cmd, args, dir)
@@ -405,8 +409,8 @@ func (s *configCommandSuite) assertResetSuccess(
 	c *gc.C, dir string, args []string,
 	expectAppValues map[string]interface{}, expectCharmValues map[string]interface{},
 ) {
-	cmd := application.NewConfigCommandForTest(s.fake)
-	cmd.SetClientStore(application.NewMockStore())
+	cmd := application.NewConfigCommandForTest(s.fake, s.store)
+	cmd.SetClientStore(jujuclienttesting.MinimalStore())
 
 	args = append([]string{"dummy-application"}, args...)
 	_, err := cmdtesting.RunCommandInDir(c, cmd, args, dir)
@@ -417,17 +421,17 @@ func (s *configCommandSuite) assertResetSuccess(
 
 // assertSetFail sets configuration options and checks the expected error.
 func (s *configCommandSuite) assertSetFail(c *gc.C, dir string, args []string, expectErr string) {
-	cmd := application.NewConfigCommandForTest(s.fake)
-	cmd.SetClientStore(application.NewMockStore())
+	cmd := application.NewConfigCommandForTest(s.fake, s.store)
+	cmd.SetClientStore(jujuclienttesting.MinimalStore())
 
 	args = append([]string{"dummy-application"}, args...)
 	_, err := cmdtesting.RunCommandInDir(c, cmd, args, dir)
 	c.Assert(err, gc.ErrorMatches, expectErr)
 }
 
 func (s *configCommandSuite) assertSetWarning(c *gc.C, dir string, args []string, w string) {
-	cmd := application.NewConfigCommandForTest(s.fake)
-	cmd.SetClientStore(application.NewMockStore())
+	cmd := application.NewConfigCommandForTest(s.fake, s.store)
+	cmd.SetClientStore(jujuclienttesting.MinimalStore())
 	_, err := cmdtesting.RunCommandInDir(c, cmd, append([]string{"dummy-application"}, args...), dir)
 	c.Assert(err, jc.ErrorIsNil)
 	c.Assert(strings.Replace(c.GetTestLog(), "\n", " ", -1), gc.Matches, ".*WARNING.*"+w+".*")

diff --git a/cmd/juju/application/constraints_test.go b/cmd/juju/application/constraints_test.go
@@ -9,6 +9,7 @@ import (
 	gc "gopkg.in/check.v1"
 
 	"github.com/juju/juju/cmd/juju/application"
+	"github.com/juju/juju/jujuclient/jujuclienttesting"
 	"github.com/juju/juju/testing"
 )
 
@@ -41,7 +42,7 @@ func (s *ServiceConstraintsCommandsSuite) TestSetInit(c *gc.C) {
 		args: []string{"mysql", "cpu-power=250"},
 	}} {
 		cmd := application.NewServiceSetConstraintsCommand()
-		cmd.SetClientStore(application.NewMockStore())
+		cmd.SetClientStore(jujuclienttesting.MinimalStore())
 		err := cmdtesting.InitCommand(cmd, test.args)
 		if test.err == "" {
 			c.Check(err, jc.ErrorIsNil)
@@ -73,7 +74,7 @@ func (s *ServiceConstraintsCommandsSuite) TestGetInit(c *gc.C) {
 		},
 	} {
 		cmd := application.NewServiceGetConstraintsCommand()
-		cmd.SetClientStore(application.NewMockStore())
+		cmd.SetClientStore(jujuclienttesting.MinimalStore())
 		err := cmdtesting.InitCommand(cmd, test.args)
 		if test.err == "" {
 			c.Check(err, jc.ErrorIsNil)

diff --git a/cmd/juju/application/consume_test.go b/cmd/juju/application/consume_test.go
@@ -39,7 +39,7 @@ func (s *ConsumeSuite) SetUpTest(c *gc.C) {
 	s.store.CurrentControllerName = controllerName
 	s.store.Controllers[controllerName] = jujuclient.ControllerDetails{}
 	s.store.Models[controllerName] = &jujuclient.ControllerModels{
-		CurrentModel: "fred/test",
+		CurrentModel: "bob/test",
 		Models: map[string]jujuclient.ModelDetails{
 			"bob/test": {ModelUUID: "test-uuid", ModelType: model.IAAS},
 			"bob/prod": {ModelUUID: "prod-uuid", ModelType: model.IAAS},