Merge pull request juju#18459 from nvinuesa/fix-modelmanager-machines

juju#18459

Before this patch, any calls to MachineModelInfo() (located in apiserver/common) would have actually be returning info from the controller model machines.
This was clearly reproducible if you had a model with 2 machines (and a no-ha controller), then a call to `juju show-model` would return with an error stating that machine "1" wasn't found. Another way of reproducing was to migrate that model.

The fix is to inject the correct MachineService on the modelmanager facade.

<!-- 
The PR title should match: <type>(optional <scope>): <description>.

Please also ensure all commits in this PR comply with our conventional commits specification:
https://github.com/juju/juju/blob/main/doc/conventional-commits.md
-->

<!-- Why this change is needed and what it does. -->


## QA steps

Calls to `show-model` and `migrate` should work fine:

```
$ juju bootstrap lxd dst
$ juju bootstrap lxd src
$ juju add-model m
$ juju add-machine
$ juju add-machine
$ juju show-model
m:
 name: admin/m
 short-name: m
 model-uuid: 8d208308-f412-451e-8605-5899d621d8ed
 model-type: iaas
 controller-uuid: b3867b85-1284-4524-8ca4-13c50e9de91f
 controller-name: src
 is-controller: false
 owner: admin
 cloud: localhost
 region: localhost
 type: lxd
 life: alive
 status:
 current: available
 since: 6 seconds ago
 users:
 admin:
 display-name: admin
 access: admin
 last-connection: 11 seconds ago
 machines:
 "0":
 cores: 0
 "1":
 cores: 0
 secret-backends:
 internal:
 num-secrets: 0
 status: active
 agent-version: 4.0-beta5.1
 credential:
 name: localhost
 owner: admin
 cloud: localhost
 validity-check: valid
 supported-features:
 - name: juju
 description: the version of Juju used by the model
 version: 4.0-beta5.1
$ juju migrate m dst
$ juju switch dst:m
$ juju show-model
m:
 name: admin/m
 short-name: m
 model-uuid: 8d208308-f412-451e-8605-5899d621d8ed
 model-type: iaas
 controller-uuid: b3867b85-1284-4524-8ca4-13c50e9de91f
 controller-name: dst
 is-controller: false
 owner: admin
 cloud: localhost
 region: localhost
 type: lxd
 life: alive
 status:
 current: available
 since: 6 seconds ago
 users:
 admin:
 display-name: admin
 access: admin
 last-connection: 11 seconds ago
 machines:
 "0":
 cores: 0
 "1":
 cores: 0
 secret-backends:
 internal:
 num-secrets: 0
 status: active
 agent-version: 4.0-beta5.1
 credential:
 name: localhost
 owner: admin
 cloud: localhost
 validity-check: valid
 supported-features:
 - name: juju
 description: the version of Juju used by the model
 version: 4.0-beta5.1
```
## Documentation changes

<!-- How it affects user workflow (CLI or API). -->

## Links

<!-- Link to all relevant specification, documentation, bug, issue or JIRA card. -->

**Launchpad bug:** https://bugs.launchpad.net/juju/+bug/

**Jira card:** JUJU-

apiserver/facades/client/modelmanager/modelinfo_test.go

@@ -210,6 +210,8 @@ func (s *modelInfoSuite) getAPI(c *gc.C) (*modelmanager.ModelManagerAPI, *gomock
 	modelInfoService := mocks.NewMockModelInfoService(ctrl)
 	mockModelDomainServices.EXPECT().ModelInfo().Return(modelInfoService)
 
+	mockModelDomainServices.EXPECT().Machine().Return(s.mockMachineService).AnyTimes()
+
 	modelAgentService.EXPECT().GetModelTargetAgentVersion(gomock.Any()).Return(jujuversion.Current, nil)
 	modelInfoService.EXPECT().GetModelInfo(gomock.Any()).Return(coremodel.ReadOnlyModel{
 		AgentVersion:   version.MustParse("1.99.9"),
@@ -435,6 +437,10 @@ func (s *modelInfoSuite) TestModelInfoWriteAccess(c *gc.C) {
 	api, ctrl := s.getAPIWithUser(c, mary)
 	defer ctrl.Finish()
 	maryName := coreusertesting.GenNewName(c, "mary")
+
+	s.mockMachineService = mocks.NewMockMachineService(ctrl)
+	s.mockModelDomainServices.EXPECT().Machine().Return(s.mockMachineService)
+
 	s.mockAccessService.EXPECT().ReadUserAccessLevelForTarget(gomock.Any(), user.NameFromTag(maryName), permission.ID{
 		ObjectType: permission.Model,
 		Key:        s.st.model.cfg.UUID(),
@@ -569,6 +575,7 @@ func (s *modelInfoSuite) TestModelInfoErrorNoAccess(c *gc.C) {
 func (s *modelInfoSuite) TestRunningMigration(c *gc.C) {
 	api, ctrl := s.getAPI(c)
 	defer ctrl.Finish()
+
 	s.mockSecretBackendService.EXPECT().BackendSummaryInfoForModel(gomock.Any(), coremodel.UUID(s.st.model.cfg.UUID())).Return(nil, nil)
 	s.mockModelService.EXPECT().GetModelUsers(gomock.Any(), coremodel.UUID(coretesting.ModelTag.Id())).Return(s.modelUserInfo, nil)
 	start := time.Now().Add(-20 * time.Minute)
@@ -625,6 +632,7 @@ func (s *modelInfoSuite) TestFailedMigration(c *gc.C) {
 func (s *modelInfoSuite) TestNoMigration(c *gc.C) {
 	api, ctrl := s.getAPI(c)
 	defer ctrl.Finish()
+
 	s.mockSecretBackendService.EXPECT().BackendSummaryInfoForModel(gomock.Any(), coremodel.UUID(s.st.model.cfg.UUID())).Return(nil, nil)
 	s.mockModelService.EXPECT().GetModelUsers(gomock.Any(), coremodel.UUID(coretesting.ModelTag.Id())).Return(s.modelUserInfo, nil)
 	s.mockMachineService.EXPECT().GetMachineUUID(gomock.Any(), machine.Name("1")).Return("deadbeef1", nil)
@@ -734,6 +742,8 @@ func (s *modelInfoSuite) TestDeadModelWithGetModelInfoFailure(c *gc.C) {
 	modelDomainServices.EXPECT().Agent().Return(modelAgentService)
 	modelAgentService.EXPECT().GetModelTargetAgentVersion(gomock.Any()).Return(jujuversion.Current, nil)
 
+	modelDomainServices.EXPECT().Machine().Return(s.mockMachineService)
+
 	s.assertSuccess(c, api, s.st.model.cfg.UUID(), state.Dead, life.Dead)
 }
 
@@ -758,6 +768,8 @@ func (s *modelInfoSuite) TestDeadModelWithGetModelTargetAgentVersionFailure(c *g
 	modelDomainServices.EXPECT().Agent().Return(modelAgentService)
 	modelAgentService.EXPECT().GetModelTargetAgentVersion(gomock.Any()).Return(version.Zero, errors.NotFoundf("model agent version"))
 
+	modelDomainServices.EXPECT().Machine().Return(s.mockMachineService)
+
 	s.assertSuccess(c, api, s.st.model.cfg.UUID(), state.Dead, life.Dead)
 }
 
@@ -819,6 +831,8 @@ func (s *modelInfoSuite) TestDyingModelWithGetModelInfoFailure(c *gc.C) {
 	modelDomainServices.EXPECT().Agent().Return(modelAgentService)
 	modelAgentService.EXPECT().GetModelTargetAgentVersion(gomock.Any()).Return(jujuversion.Current, nil)
 
+	modelDomainServices.EXPECT().Machine().Return(s.mockMachineService)
+
 	s.assertSuccess(c, api, s.st.model.cfg.UUID(), state.Dying, life.Dying)
 }
 
@@ -843,6 +857,8 @@ func (s *modelInfoSuite) TestDyingModelWithGetModelTargetAgentVersionFailure(c *
 	modelDomainServices.EXPECT().Agent().Return(modelAgentService)
 	modelAgentService.EXPECT().GetModelTargetAgentVersion(gomock.Any()).Return(jujuversion.Current, errors.NotFoundf("model agent version"))
 
+	modelDomainServices.EXPECT().Machine().Return(s.mockMachineService)
+
 	s.assertSuccess(c, api, s.st.model.cfg.UUID(), state.Dying, life.Dying)
 }
 
@@ -920,6 +936,8 @@ func (s *modelInfoSuite) TestImportingModelWithGetModelInfoFailure(c *gc.C) {
 	modelDomainServices.EXPECT().Agent().Return(modelAgentService)
 	modelAgentService.EXPECT().GetModelTargetAgentVersion(gomock.Any()).Return(jujuversion.Current, nil)
 
+	modelDomainServices.EXPECT().Machine().Return(s.mockMachineService)
+
 	s.assertSuccess(c, api, s.st.model.cfg.UUID(), state.Alive, life.Alive)
 }
 
@@ -945,6 +963,8 @@ func (s *modelInfoSuite) TestImportingModelWithGetModelTargetAgentVersionFailure
 	modelDomainServices.EXPECT().Agent().Return(modelAgentService)
 	modelAgentService.EXPECT().GetModelTargetAgentVersion(gomock.Any()).Return(version.Zero, errors.NotFoundf("model agent version"))
 
+	modelDomainServices.EXPECT().Machine().Return(s.mockMachineService)
+
 	s.assertSuccess(c, api, s.st.model.cfg.UUID(), state.Alive, life.Alive)
 }
 

apiserver/facades/client/modelmanager/modelmanager.go

@@ -1192,7 +1192,7 @@ func (m *ModelManagerAPI) getModelInfo(ctx context.Context, tag names.ModelTag,
 		}
 	}
 	if canSeeMachinesAndSecrets {
-		if info.Machines, err = common.ModelMachineInfo(ctx, st, m.machineService); shouldErr(err) {
+		if info.Machines, err = common.ModelMachineInfo(ctx, st, modelDomainServices.Machine()); shouldErr(err) {
 			return params.ModelInfo{}, err
 		}
 	}

apiserver/facades/client/modelmanager/modelmanager_test.go

@@ -378,12 +378,15 @@ func (s *modelManagerSuite) expectCreateModelOnModelDB(
 	// Expect calls to get various model services.
 	modelInfoService := mocks.NewMockModelInfoService(ctrl)
 	networkService := mocks.NewMockNetworkService(ctrl)
+	machineService := mocks.NewMockMachineService(ctrl)
+
 	s.modelConfigService = mocks.NewMockModelConfigService(ctrl)
 	modelAgentService := mocks.NewMockModelAgentService(ctrl)
 	modelDomainServices.EXPECT().ModelInfo().Return(modelInfoService).AnyTimes()
 	modelDomainServices.EXPECT().Network().Return(networkService)
 	modelDomainServices.EXPECT().Config().Return(s.modelConfigService).AnyTimes()
 	modelDomainServices.EXPECT().Agent().Return(modelAgentService).AnyTimes()
+	modelDomainServices.EXPECT().Machine().Return(machineService)
 
 	// Expect calls to functions of the model services.
 	modelInfoService.EXPECT().CreateModel(gomock.Any(), s.controllerUUID)
@@ -1130,11 +1133,13 @@ func (s *modelManagerStateSuite) expectCreateModelStateSuite(
 	modelConfigService := mocks.NewMockModelConfigService(ctrl)
 	modelInfoService := mocks.NewMockModelInfoService(ctrl)
 	networkService := mocks.NewMockNetworkService(ctrl)
+	machineService := mocks.NewMockMachineService(ctrl)
 
 	modelDomainServices.EXPECT().Agent().Return(modelAgentService).AnyTimes()
 	modelDomainServices.EXPECT().Config().Return(modelConfigService).AnyTimes()
 	modelDomainServices.EXPECT().ModelInfo().Return(modelInfoService).AnyTimes()
 	modelDomainServices.EXPECT().Network().Return(networkService)
+	modelDomainServices.EXPECT().Machine().Return(machineService)
 
 	blockCommandService := mocks.NewMockBlockCommandService(ctrl)
 	modelDomainServices.EXPECT().BlockCommand().Return(blockCommandService).AnyTimes()

apiserver/facades/client/modelmanager/services.go

@@ -51,6 +51,9 @@ type ModelDomainServices interface {
 	// Network returns the space service.
 	Network() NetworkService
 	BlockCommand() BlockCommandService
+
+	// Machine returns the machine service.
+	Machine() MachineService
 }
 
 // DomainServicesGetter is a factory for creating model services.
@@ -320,6 +323,10 @@ func (s domainServices) Network() NetworkService {
 	return s.domainServices.Network()
 }
 
+func (s domainServices) Machine() MachineService {
+	return s.domainServices.Machine()
+}
+
 func (s domainServices) BlockCommand() BlockCommandService {
 	return s.domainServices.BlockCommand()
 }