space rename: add mock model and skip if not controllermodel

juju · Jan 31, 2020 · 6e173d5 · 6e173d5
1 parent 778d402
commit 6e173d5
Show file tree

Hide file tree

Showing 6 changed files with 117 additions and 8 deletions.
diff --git a/apiserver/facades/client/spaces/mocks/package_mock.go b/apiserver/facades/client/spaces/mocks/package_mock.go
diff --git a/apiserver/facades/client/spaces/opfactory.go b/apiserver/facades/client/spaces/opfactory.go
@@ -34,5 +34,9 @@ func (f *opFactory) NewRenameSpaceModelOp(fromName, toName string) (RenameSpaceM
 	if err != nil {
 		return nil, errors.Trace(err)
 	}
-	return NewRenameSpaceModelOp(controllerSettings, &renameSpaceStateShim{f.st}, space, toName), nil
+	model, err := f.st.Model()
+	if err != nil {
+		return nil, errors.Trace(err)
+	}
+	return NewRenameSpaceModelOp(model, controllerSettings, &renameSpaceStateShim{f.st}, space, toName), nil
 }
diff --git a/apiserver/facades/client/spaces/package_test.go b/apiserver/facades/client/spaces/package_test.go
@@ -9,7 +9,7 @@ import (
 	gc "gopkg.in/check.v1"
 )
 
-//go:generate mockgen -package mocks -destination mocks/package_mock.go github.com/juju/juju/apiserver/facades/client/spaces Backing,BlockChecker,Machine,RenameSpaceModelOp,RenameSpace,RenameSpaceState,Settings,OpFactory
+//go:generate mockgen -package mocks -destination mocks/package_mock.go github.com/juju/juju/apiserver/facades/client/spaces Backing,BlockChecker,Machine,RenameSpaceModelOp,RenameSpace,RenameSpaceState,Settings,OpFactory,Model
 
 func TestPackage(t *stdtesting.T) {
 	gc.TestingT(t)

diff --git a/apiserver/facades/client/spaces/spaces_rename.go b/apiserver/facades/client/spaces/spaces_rename.go
@@ -55,22 +55,30 @@ type Settings interface {
 	DeltaOps(key string, delta settings.ItemChanges) ([]txn.Op, error)
 }
 
+// Model describes methods for interacting with Model to
+// check whether the current model is a controllerModel.
+type Model interface {
+	IsControllerModel() bool
+}
+
 type spaceRenameModelOp struct {
 	st       RenameSpaceState
 	space    RenameSpace
 	settings Settings
+	model    Model
 	toName   string
 }
 
 func (o *spaceRenameModelOp) Done(err error) error {
 	return err
 }
 
-func NewRenameSpaceModelOp(settings Settings, st RenameSpaceState, space RenameSpace, toName string) *spaceRenameModelOp {
+func NewRenameSpaceModelOp(model Model, settings Settings, st RenameSpaceState, space RenameSpace, toName string) *spaceRenameModelOp {
 	return &spaceRenameModelOp{
 		st:       st,
 		settings: settings,
 		space:    space,
+		model:    model,
 		toName:   toName,
 	}
 }
@@ -142,7 +150,11 @@ func (o *spaceRenameModelOp) getConstraintsChanges(fromSpaceName, toName string)
 	return currentConstraints, nil
 }
 
+// getSettingsChanges get's skipped and returns nil if we are not in the controllerModel
 func (o *spaceRenameModelOp) getSettingsChanges(fromSpaceName, toName string) (settings.ItemChanges, error) {
+	if !o.model.IsControllerModel() {
+		return nil, nil
+	}
 	currentControllerConfig, err := o.st.ControllerConfig()
 	if err != nil {
 		return nil, errors.Trace(err)

diff --git a/apiserver/facades/client/spaces/spaces_rename_test.go b/apiserver/facades/client/spaces/spaces_rename_test.go
@@ -24,6 +24,7 @@ type SpaceRenameSuite struct {
 	state    *mocks.MockRenameSpaceState
 	space    *mocks.MockRenameSpace
 	settings *mocks.MockSettings
+	model    *mocks.MockModel
 }
 
 var _ = gc.Suite(&SpaceRenameSuite{})
@@ -44,6 +45,7 @@ func (s *SpaceRenameSuite) TestSuccess(c *gc.C) {
 	config := s.getDefaultControllerConfig(c, map[string]interface{}{controller.JujuHASpace: fromName, controller.JujuManagementSpace: "nochange"})
 	s.state.EXPECT().ControllerConfig().Return(config, nil)
 
+	s.model.EXPECT().IsControllerModel().Return(true)
 	s.state.EXPECT().ControllerSettingsGlobalKey().Return(controllerKey)
 
 	currentConstraints := map[string]constraints.Value{
@@ -70,7 +72,45 @@ func (s *SpaceRenameSuite) TestSuccess(c *gc.C) {
 	s.state.EXPECT().GetConstraintsOps(expectedNewConstraints).Return([]txn.Op{}, nil)
 	s.space.EXPECT().RenameSpaceCompleteOps(toName).Return(nil, nil)
 
-	op := spaces.NewRenameSpaceModelOp(s.settings, s.state, s.space, toName)
+	op := spaces.NewRenameSpaceModelOp(s.model, s.settings, s.state, s.space, toName)
+	ops, err := op.Build(0)
+	c.Assert(err, jc.ErrorIsNil)
+
+	// this is because the code itself does not test for the ops but for expected constraints and delta,
+	// which are used to create the ops.
+	c.Assert(ops, gc.HasLen, 0)
+}
+
+func (s *SpaceRenameSuite) TestNotControllerModelSuccess(c *gc.C) {
+	toName := "blub"
+	fromName := "db"
+	controllerKey := "something"
+
+	ctrl := s.setupMocks(c)
+	defer ctrl.Finish()
+
+	s.space.EXPECT().Name().Return(fromName).Times(2)
+	s.state.EXPECT().ControllerSettingsGlobalKey().Return(controllerKey)
+	s.model.EXPECT().IsControllerModel().Return(false)
+
+	currentConstraints := map[string]constraints.Value{
+		"DOCID_1": {Spaces: &[]string{fromName, "nochange"}},
+		"DOCID_2": {Spaces: &[]string{"nochange"}},
+		"DOCID_3": {},
+	}
+	s.state.EXPECT().ConstraintsBySpaceName(fromName).Return(currentConstraints, nil)
+
+	expectedNewConstraints := map[string]constraints.Value{
+		"DOCID_1": {Spaces: &[]string{toName, "nochange"}},
+		"DOCID_2": {Spaces: &[]string{"nochange"}},
+		"DOCID_3": {},
+	}
+	s.settings.EXPECT().DeltaOps(controllerKey, nil).Return(nil, nil)
+
+	s.state.EXPECT().GetConstraintsOps(expectedNewConstraints).Return([]txn.Op{}, nil)
+	s.space.EXPECT().RenameSpaceCompleteOps(toName).Return(nil, nil)
+
+	op := spaces.NewRenameSpaceModelOp(s.model, s.settings, s.state, s.space, toName)
 	ops, err := op.Build(0)
 	c.Assert(err, jc.ErrorIsNil)
 
@@ -89,9 +129,10 @@ func (s *SpaceRenameSuite) TestErrorSettingsChanges(c *gc.C) {
 	s.space.EXPECT().Name().Return(fromName).Times(1)
 
 	bamErr := errors.New("bam")
+	s.model.EXPECT().IsControllerModel().Return(true)
 	s.state.EXPECT().ControllerConfig().Return(nil, bamErr)
 
-	op := spaces.NewRenameSpaceModelOp(s.settings, s.state, s.space, toName)
+	op := spaces.NewRenameSpaceModelOp(s.model, s.settings, s.state, s.space, toName)
 	ops, err := op.Build(0)
 	c.Assert(err, gc.ErrorMatches, fmt.Sprintf("retrieving setting changes: %v", bamErr.Error()))
 
@@ -106,13 +147,13 @@ func (s *SpaceRenameSuite) TestErrorConstraintsChanges(c *gc.C) {
 	defer ctrl.Finish()
 
 	s.space.EXPECT().Name().Return(fromName).Times(2)
-
+	s.model.EXPECT().IsControllerModel().Return(true)
 	s.state.EXPECT().ControllerConfig().Return(nil, nil)
 
 	bamErr := errors.New("bam")
 	s.state.EXPECT().ConstraintsBySpaceName(fromName).Return(nil, bamErr)
 
-	op := spaces.NewRenameSpaceModelOp(s.settings, s.state, s.space, toName)
+	op := spaces.NewRenameSpaceModelOp(s.model, s.settings, s.state, s.space, toName)
 	ops, err := op.Build(0)
 	c.Assert(err, gc.ErrorMatches, fmt.Sprintf("retrieving constraint changes: %v", bamErr.Error()))
 
@@ -129,6 +170,7 @@ func (s *SpaceRenameSuite) setupMocks(c *gc.C) *gomock.Controller {
 	ctrl := gomock.NewController(c)
 
 	s.space = mocks.NewMockRenameSpace(ctrl)
+	s.model = mocks.NewMockModel(ctrl)
 	s.state = mocks.NewMockRenameSpaceState(ctrl)
 	s.settings = mocks.NewMockSettings(ctrl)
 

diff --git a/state/settings_test.go b/state/settings_test.go
@@ -631,6 +631,20 @@ func (s *SettingsSuite) TestDeltaOpsSuccess(c *gc.C) {
 	})
 }
 
+func (s *SettingsSuite) TestDeltaOpsNoChanges(c *gc.C) {
+	s1, err := s.createSettings(s.key, map[string]interface{}{
+		"foo": []interface{}{"bar"},
+	})
+	c.Assert(err, jc.ErrorIsNil)
+	_, err = s1.Write()
+	c.Assert(err, jc.ErrorIsNil)
+
+	settings := s.state.NewSettings()
+	ops, err := settings.DeltaOps(s.key, nil)
+	c.Assert(err, jc.ErrorIsNil)
+	c.Assert(ops, gc.IsNil)
+}
+
 func (s *SettingsSuite) TestDeltaOpsChangedError(c *gc.C) {
 	s1, err := s.createSettings(s.key, map[string]interface{}{
 		"foo": []interface{}{"bar"},