Skip to content

Commit b2ee2f3

Browse files
axwaxw
committed
Remove state.State.ForModel
Drop State.ForModel in favour of using StatePool everywhere.
1 parent 67aba93 commit b2ee2f3

File tree

23 files changed

+98
-150
lines changed

23 files changed

+98
-150
lines changed

‎agent/agentbootstrap/bootstrap_test.go

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -251,9 +251,11 @@ LXC_BRIDGE="ignored"`[1:])
251251
// Check that the hosted model has been added, model constraints
252252
// set, and its config contains the same authorized-keys as the
253253
// controller model.
254-
hostedModelSt, err := st.ForModel(names.NewModelTag(hostedModelUUID))
254+
statePool := state.NewStatePool(st)
255+
defer statePool.Close()
256+
hostedModelSt, release, err := statePool.Get(hostedModelUUID)
255257
c.Assert(err, jc.ErrorIsNil)
256-
defer hostedModelSt.Close()
258+
defer release()
257259
gotModelConstraints, err = hostedModelSt.ModelConstraints()
258260
c.Assert(err, jc.ErrorIsNil)
259261
c.Assert(gotModelConstraints, gc.DeepEquals, expectModelConstraints)

‎apiserver/admin_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -857,9 +857,9 @@ func (s *loginSuite) assertRemoteModel(c *gc.C, api api.Connection, expected nam
857857
// the expected model. We make a change in state on that model, and
858858
// then check that it is picked up by a call to the API.
859859

860-
st, err := s.State.ForModel(tag)
860+
st, release, err := s.StatePool.Get(tag.Id())
861861
c.Assert(err, jc.ErrorIsNil)
862-
defer st.Close()
862+
defer release()
863863

864864
expectedCons := constraints.MustParse("mem=8G")
865865
err = st.SetModelConstraints(expectedCons)

‎apiserver/facades/client/controller/controller.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -503,7 +503,7 @@ func (c *ControllerAPI) ModifyControllerAccess(args params.ModifyControllerAcces
503503
// retrieved from the target controller.
504504
var runMigrationPrechecks = func(st, ctlrSt *state.State, targetInfo *coremigration.TargetInfo) error {
505505
// Check model and source controller.
506-
backend, err := migration.PrecheckShim(st)
506+
backend, err := migration.PrecheckShim(st, ctlrSt)
507507
if err != nil {
508508
return errors.Annotate(err, "creating backend")
509509
}

‎apiserver/facades/client/modelmanager/modelmanager.go

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -763,20 +763,19 @@ func (m *ModelManagerAPI) DestroyModels(args params.DestroyModelsParams) (params
763763
}
764764

765765
destroyModel := func(modelUUID string, destroyStorage *bool) error {
766-
model, releaseModel, err := m.state.GetModel(modelUUID)
766+
st, releaseSt, err := m.state.GetBackend(modelUUID)
767767
if err != nil {
768768
return errors.Trace(err)
769769
}
770-
defer releaseModel()
771-
if err := m.authCheck(model.Owner()); err != nil {
772-
return errors.Trace(err)
773-
}
770+
defer releaseSt()
774771

775-
st, releaseSt, err := m.state.GetBackend(modelUUID)
772+
model, err := st.Model()
776773
if err != nil {
777774
return errors.Trace(err)
778775
}
779-
defer releaseSt()
776+
if err := m.authCheck(model.Owner()); err != nil {
777+
return errors.Trace(err)
778+
}
780779

781780
return errors.Trace(common.DestroyModel(st, destroyStorage))
782781
}

‎apiserver/facades/client/modelmanager/modelmanager_test.go

Lines changed: 14 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -821,8 +821,8 @@ func (s *modelManagerSuite) TestDestroyModelsV3(c *gc.C) {
821821
s.st.CheckCallNames(c,
822822
"ControllerTag",
823823
"ModelUUID",
824-
"GetModel",
825824
"GetBackend",
825+
"Model",
826826
"GetBlockForType",
827827
"GetBlockForType",
828828
"GetBlockForType",
@@ -929,9 +929,9 @@ func (s *modelManagerStateSuite) TestAdminCanCreateModelForSomeoneElse(c *gc.C)
929929
c.Assert(model.Name, gc.Equals, "test-model")
930930
// Make sure that the environment created does actually have the correct
931931
// owner, and that owner is actually allowed to use the environment.
932-
newState, err := s.State.ForModel(names.NewModelTag(model.UUID))
932+
newState, release, err := s.StatePool.Get(model.UUID)
933933
c.Assert(err, jc.ErrorIsNil)
934-
defer newState.Close()
934+
defer release()
935935

936936
newModel, err := newState.Model()
937937
c.Assert(err, jc.ErrorIsNil)
@@ -1110,9 +1110,10 @@ func (s *modelManagerStateSuite) TestDestroyOwnModel(c *gc.C) {
11101110
s.setAPIUser(c, owner)
11111111
m, err := s.modelmanager.CreateModel(createArgs(owner))
11121112
c.Assert(err, jc.ErrorIsNil)
1113-
st, err := s.State.ForModel(names.NewModelTag(m.UUID))
1113+
1114+
st, release, err := s.StatePool.Get(m.UUID)
11141115
c.Assert(err, jc.ErrorIsNil)
1115-
defer st.Close()
1116+
defer release()
11161117
model, err := st.Model()
11171118
c.Assert(err, jc.ErrorIsNil)
11181119

@@ -1145,9 +1146,10 @@ func (s *modelManagerStateSuite) TestAdminDestroysOtherModel(c *gc.C) {
11451146
s.setAPIUser(c, owner)
11461147
m, err := s.modelmanager.CreateModel(createArgs(owner))
11471148
c.Assert(err, jc.ErrorIsNil)
1148-
st, err := s.State.ForModel(names.NewModelTag(m.UUID))
1149+
1150+
st, release, err := s.StatePool.Get(m.UUID)
11491151
c.Assert(err, jc.ErrorIsNil)
1150-
defer st.Close()
1152+
defer release()
11511153
model, err := st.Model()
11521154
c.Assert(err, jc.ErrorIsNil)
11531155

@@ -1180,9 +1182,10 @@ func (s *modelManagerStateSuite) TestDestroyModelErrors(c *gc.C) {
11801182
s.setAPIUser(c, owner)
11811183
m, err := s.modelmanager.CreateModel(createArgs(owner))
11821184
c.Assert(err, jc.ErrorIsNil)
1183-
st, err := s.State.ForModel(names.NewModelTag(m.UUID))
1185+
1186+
st, release, err := s.StatePool.Get(m.UUID)
11841187
c.Assert(err, jc.ErrorIsNil)
1185-
defer st.Close()
1188+
defer release()
11861189
model, err := st.Model()
11871190
c.Assert(err, jc.ErrorIsNil)
11881191

@@ -1212,7 +1215,7 @@ func (s *modelManagerStateSuite) TestDestroyModelErrors(c *gc.C) {
12121215
},
12131216
}, {
12141217
&params.Error{
1215-
Message: "model not found",
1218+
Message: `model "9f484882-2f18-4fd2-967d-db9663db7bea" not found`,
12161219
Code: params.CodeNotFound,
12171220
},
12181221
}, {
@@ -1270,7 +1273,7 @@ func (s *modelManagerStateSuite) TestGrantMissingModelFails(c *gc.C) {
12701273
user := s.Factory.MakeModelUser(c, nil)
12711274
model := names.NewModelTag("17e4bd2d-3e08-4f3d-b945-087be7ebdce4")
12721275
err := s.grant(c, user.UserTag, params.ModelReadAccess, model)
1273-
expectedErr := `.*model not found`
1276+
expectedErr := `.*model "17e4bd2d-3e08-4f3d-b945-087be7ebdce4" not found`
12741277
c.Assert(err, gc.ErrorMatches, expectedErr)
12751278
}
12761279

‎apiserver/facades/controller/migrationmaster/shim.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,8 @@ import (
1616
// NewFacade exists to provide the required signature for API
1717
// registration, converting st to backend.
1818
func NewFacade(ctx facade.Context) (*API, error) {
19-
precheckBackend, err := migration.PrecheckShim(ctx.State())
19+
controllerState := ctx.StatePool().SystemState()
20+
precheckBackend, err := migration.PrecheckShim(ctx.State(), controllerState)
2021
if err != nil {
2122
return nil, errors.Annotate(err, "creating precheck backend")
2223
}

‎apiserver/facades/controller/migrationtarget/migrationtarget.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ func (api *API) Prechecks(model params.MigrationModelInfo) error {
7575
if err != nil {
7676
return errors.Trace(err)
7777
}
78-
backend, err := migration.PrecheckShim(api.state)
78+
backend, err := migration.PrecheckShim(api.state, api.pool.SystemState())
7979
if err != nil {
8080
return errors.Annotate(err, "creating backend")
8181
}

‎apiserver/facades/controller/migrationtarget/migrationtarget_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -158,7 +158,7 @@ func (s *Suite) TestAbortMissingEnv(c *gc.C) {
158158
api := s.mustNewAPI(c)
159159
newUUID := utils.MustNewUUID().String()
160160
err := api.Abort(params.ModelArgs{ModelTag: names.NewModelTag(newUUID).String()})
161-
c.Assert(err, gc.ErrorMatches, `model not found`)
161+
c.Assert(err, gc.ErrorMatches, `model "`+newUUID+`" not found`)
162162
}
163163

164164
func (s *Suite) TestAbortNotImportingEnv(c *gc.C) {
@@ -195,7 +195,7 @@ func (s *Suite) TestActivateMissingEnv(c *gc.C) {
195195
api := s.mustNewAPI(c)
196196
newUUID := utils.MustNewUUID().String()
197197
err := api.Activate(params.ModelArgs{ModelTag: names.NewModelTag(newUUID).String()})
198-
c.Assert(err, gc.ErrorMatches, `model not found`)
198+
c.Assert(err, gc.ErrorMatches, `model "`+newUUID+`" not found`)
199199
}
200200

201201
func (s *Suite) TestActivateNotImportingEnv(c *gc.C) {

‎cmd/jujud/dumplogs/dumplogs.go

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -126,12 +126,15 @@ func (c *dumpLogsCommand) Run(ctx *cmd.Context) error {
126126
}
127127
defer st0.Close()
128128

129+
statePool := state.NewStatePool(st0)
130+
defer statePool.Close()
131+
129132
modelUUIDs, err := st0.AllModelUUIDs()
130133
if err != nil {
131134
return errors.Annotate(err, "failed to look up models")
132135
}
133136
for _, modelUUID := range modelUUIDs {
134-
err := c.dumpLogsForEnv(ctx, st0, names.NewModelTag(modelUUID))
137+
err := c.dumpLogsForEnv(ctx, statePool, names.NewModelTag(modelUUID))
135138
if err != nil {
136139
return errors.Annotatef(err, "failed to dump logs for model %s", modelUUID)
137140
}
@@ -156,12 +159,12 @@ func (c *dumpLogsCommand) findMachineId(dataDir string) (string, error) {
156159
return "", errors.New("no machine agent configuration found")
157160
}
158161

159-
func (c *dumpLogsCommand) dumpLogsForEnv(ctx *cmd.Context, st0 *state.State, tag names.ModelTag) error {
160-
st, err := st0.ForModel(tag)
162+
func (c *dumpLogsCommand) dumpLogsForEnv(ctx *cmd.Context, statePool *state.StatePool, tag names.ModelTag) error {
163+
st, release, err := statePool.Get(tag.Id())
161164
if err != nil {
162165
return errors.Annotate(err, "failed open model")
163166
}
164-
defer st.Close()
167+
defer release()
165168

166169
logName := ctx.AbsPath(filepath.Join(c.outDir, fmt.Sprintf("%s.log", tag.Id())))
167170
ctx.Infof("writing to %s", logName)

‎featuretests/cmd_juju_controller_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -147,9 +147,9 @@ current-model: controller
147147

148148
func (s *cmdControllerSuite) TestListDeadModels(c *gc.C) {
149149
modelInfo := s.createModelAdminUser(c, "new-model", false)
150-
st, err := s.State.ForModel(names.NewModelTag(modelInfo.UUID))
150+
st, release, err := s.StatePool.Get(modelInfo.UUID)
151151
c.Assert(err, jc.ErrorIsNil)
152-
defer st.Close()
152+
defer release()
153153
m, err := st.Model()
154154
c.Assert(err, jc.ErrorIsNil)
155155
err = m.Destroy(state.DestroyModelParams{})

‎migration/precheck.go

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -32,18 +32,11 @@ type PrecheckBackend interface {
3232
AllMachines() ([]PrecheckMachine, error)
3333
AllApplications() ([]PrecheckApplication, error)
3434
AllRelations() ([]PrecheckRelation, error)
35-
ControllerBackend() (PrecheckBackendCloser, error)
35+
ControllerBackend() (PrecheckBackend, error)
3636
CloudCredential(tag names.CloudCredentialTag) (cloud.Credential, error)
3737
ListPendingResources(string) ([]resource.Resource, error)
3838
}
3939

40-
// PrecheckBackendCloser adds the Close method to the standard
41-
// PrecheckBackend.
42-
type PrecheckBackendCloser interface {
43-
PrecheckBackend
44-
Close() error
45-
}
46-
4740
// Pool defines the interface to a StatePool used by the migration
4841
// prechecks.
4942
type Pool interface {
@@ -143,7 +136,6 @@ func SourcePrecheck(backend PrecheckBackend) error {
143136
if err != nil {
144137
return errors.Trace(err)
145138
}
146-
defer controllerBackend.Close()
147139
if err := checkController(controllerBackend); err != nil {
148140
return errors.Annotate(err, "controller")
149141
}

‎migration/precheck_shim.go

Lines changed: 10 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -11,23 +11,25 @@ import (
1111
"github.com/juju/juju/state"
1212
)
1313

14-
// PrecheckShim wraps a *state.State to implement PrecheckBackend.
15-
func PrecheckShim(st *state.State) (PrecheckBackend, error) {
16-
rSt, err := st.Resources()
14+
// PrecheckShim wraps a pair of *state.States to implement PrecheckBackend.
15+
func PrecheckShim(modelState, controllerState *state.State) (PrecheckBackend, error) {
16+
rSt, err := modelState.Resources()
1717
if err != nil {
1818
return nil, errors.Trace(err)
1919
}
2020
return &precheckShim{
21-
State: st,
22-
resourcesSt: rSt,
21+
State: modelState,
22+
controllerState: controllerState,
23+
resourcesSt: rSt,
2324
}, nil
2425
}
2526

2627
// precheckShim is untested, but is simple enough to be verified by
2728
// inspection.
2829
type precheckShim struct {
2930
*state.State
30-
resourcesSt state.Resources
31+
controllerState *state.State
32+
resourcesSt state.Resources
3133
}
3234

3335
// Model implements PrecheckBackend.
@@ -110,20 +112,8 @@ func (s *precheckShim) ListPendingResources(app string) ([]resource.Resource, er
110112
}
111113

112114
// ControllerBackend implements PrecheckBackend.
113-
func (s *precheckShim) ControllerBackend() (PrecheckBackendCloser, error) {
114-
st, err := s.State.ForModel(s.State.ControllerModelTag())
115-
if err != nil {
116-
return nil, errors.Trace(err)
117-
}
118-
rSt, err := st.Resources()
119-
if err != nil {
120-
st.Close()
121-
return nil, errors.Trace(err)
122-
}
123-
return &precheckShim{
124-
State: st,
125-
resourcesSt: rSt,
126-
}, nil
115+
func (s *precheckShim) ControllerBackend() (PrecheckBackend, error) {
116+
return PrecheckShim(s.controllerState, s.controllerState)
127117
}
128118

129119
// PoolShim wraps a state.StatePool to produce a Pool.

‎migration/precheck_test.go

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -743,10 +743,6 @@ type fakeBackend struct {
743743
controllerBackend *fakeBackend
744744
}
745745

746-
func (b *fakeBackend) Close() error {
747-
return nil
748-
}
749-
750746
func (b *fakeBackend) Model() (migration.PrecheckModel, error) {
751747
return &b.model, nil
752748
}
@@ -791,7 +787,7 @@ func (b *fakeBackend) ListPendingResources(app string) ([]resource.Resource, err
791787
return b.pendingResources, b.pendingResourcesErr
792788
}
793789

794-
func (b *fakeBackend) ControllerBackend() (migration.PrecheckBackendCloser, error) {
790+
func (b *fakeBackend) ControllerBackend() (migration.PrecheckBackend, error) {
795791
if b.controllerBackend == nil {
796792
return b, nil
797793
}

‎scripts/juju-force-upgrade/main.go

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -113,7 +113,11 @@ func main() {
113113
checkErr("getting state connection", err)
114114
defer st.Close()
115115

116-
modelSt, err := st.ForModel(names.NewModelTag(modelUUID))
116+
statePool := state.NewStatePool(st)
117+
defer statePool.Close()
118+
modelSt, release, err := statePool.Get(modelUUID)
117119
checkErr("open model", err)
120+
defer release()
121+
118122
checkErr("set model agent version", modelSt.SetModelAgentVersion(agentVersion, true))
119123
}

‎state/model.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -837,7 +837,7 @@ func (m *Model) refresh(uuid string) error {
837837
defer closer()
838838
err := models.FindId(uuid).One(&m.doc)
839839
if err == mgo.ErrNotFound {
840-
return errors.NotFoundf("model")
840+
return errors.NotFoundf("model %q", uuid)
841841
}
842842
return err
843843
}

‎state/modeluser_test.go

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -379,14 +379,10 @@ func (s *ModelUserSuite) TestModelUUIDsForUser(c *gc.C) {
379379
c.Assert(models, jc.DeepEquals, []string{s.State.ModelUUID()})
380380

381381
modelTag := names.NewModelTag(models[0])
382-
st, err := s.State.ForModel(modelTag)
383-
c.Assert(err, jc.ErrorIsNil)
384-
385382
access, err := s.State.UserAccess(user.UserTag(), modelTag)
386383
when, err := s.Model.LastModelConnection(access.UserTag)
387384
c.Assert(err, jc.Satisfies, state.IsNeverConnectedError)
388385
c.Assert(when.IsZero(), jc.IsTrue)
389-
c.Assert(st.Close(), jc.ErrorIsNil)
390386
}
391387

392388
func (s *ModelUserSuite) TestImportingModelUUIDsForUser(c *gc.C) {

0 commit comments

Comments
 (0)