Merge pull request juju#16157 from anvial/meterstatus-controllerconfig

juju#16157

This PR injects controllerConfigService in agent/meterstatus facade. Also, it refactors agent/uniter facade to mock controllerConfigGetter function in unit-tests.

To inject this controllerConfigService (controllerConfigGetter), the PR extract `ControllerConfig()` function from `UniStateShim` in common.UnitStateAPI. To do this there was a need to inject `controllerConfigGetter` in several facades where common APIs used:
- `agent/provisioner`
- `agent/upgrader`
- `client/client`
- `client/machinemanager`
- `client/modelmanager`
- `client/modelupdater`

## Checklist

- [x] Code style: imports ordered, good names, simple structure, etc
- [x] Comments saying why design decisions were made
- [x] Go unit tests, with comments saying what you're testing

## QA steps

```sh
go test github.com/juju/juju/apiserver/facades/agent/meterstatus/... -gocheck.v
go test github.com/juju/juju/apiserver/facades/agent/uniter/... -gocheck.v 
go test github.com/juju/juju/apiserver/facades/agent/provisioner/... -gocheck.v 
go test github.com/juju/juju/apiserver/facades/agent/upgrader/... -gocheck.v 
go test github.com/juju/juju/apiserver/facades/client/client/... -gocheck.v 
go test github.com/juju/juju/apiserver/facades/client/machinemanager/... -gocheck.v 
go test github.com/juju/juju/apiserver/facades/client/modelmanager/... -gocheck.v 
go test github.com/juju/juju/apiserver/facades/client/modelupgrader/... -gocheck.v

juju bootstrap lxd lxd
juju add-model test
juju deploy ubuntu -n 3
```

apiserver/common/package_test.go

@@ -10,6 +10,7 @@ import (
 )
 
 //go:generate go run go.uber.org/mock/mockgen -package mocks -destination mocks/authorizer_mock.go github.com/juju/juju/apiserver/common Authorizer
+//go:generate go run go.uber.org/mock/mockgen -package mocks -destination mocks/domain_mock.go github.com/juju/juju/apiserver/common ControllerConfigGetter
 //go:generate go run go.uber.org/mock/mockgen -package mocks -destination mocks/controllerconfig_mock.go github.com/juju/juju/apiserver/common ControllerConfigState,ExternalControllerService
 //go:generate go run go.uber.org/mock/mockgen -package mocks -destination mocks/tools_mock.go github.com/juju/juju/apiserver/common ToolsFinder,ToolsFindEntity,ToolsURLGetter,APIHostPortsForAgentsGetter,ToolsStorageGetter,AgentTooler
 //go:generate go run go.uber.org/mock/mockgen -package mocks -destination mocks/storage.go github.com/juju/juju/state/binarystorage StorageCloser

apiserver/common/tools.go

@@ -249,7 +249,7 @@ type ToolsFinder interface {
 
 // ControllerConfigGetter defines a method for getting the controller config.
 type ControllerConfigGetter interface {
-	ControllerConfig() (controller.Config, error)
+	ControllerConfig(context.Context) (controller.Config, error)
 }
 
 type toolsFinder struct {
@@ -286,7 +286,7 @@ func (f *toolsFinder) FindAgents(ctx context.Context, args FindAgentsParams) (co
 		return nil, err
 	}
 
-	controllerConfig, err := f.controllerConfigGetter.ControllerConfig()
+	controllerConfig, err := f.controllerConfigGetter.ControllerConfig(ctx)
 	if err != nil {
 		return nil, err
 	}

apiserver/common/tools_test.go

@@ -602,6 +602,6 @@ func (s *getUrlSuite) TestToolsURLGetter(c *gc.C) {
 
 type controllerConfigGetter struct{}
 
-func (controllerConfigGetter) ControllerConfig() (controller.Config, error) {
+func (controllerConfigGetter) ControllerConfig(context.Context) (controller.Config, error) {
 	return coretesting.FakeControllerConfig(), nil
 }

apiserver/common/unitstate.go

@@ -25,7 +25,6 @@ import (
 type UnitStateBackend interface {
 	ApplyOperation(state.ModelOperation) error
 	Unit(string) (UnitStateUnit, error)
-	ControllerConfig() (controller.Config, error)
 }
 
 // UnitStateUnit describes unit-receiver state methods required
@@ -54,8 +53,9 @@ func (s UnitStateState) ControllerConfig() (controller.Config, error) {
 }
 
 type UnitStateAPI struct {
-	backend   UnitStateBackend
-	resources facade.Resources
+	controllerConfigGetter ControllerConfigGetter
+	backend                UnitStateBackend
+	resources              facade.Resources
 
 	logger loggo.Logger
 
@@ -65,29 +65,32 @@ type UnitStateAPI struct {
 
 // NewExternalUnitStateAPI can be used for API registration.
 func NewExternalUnitStateAPI(
+	controllerConfigGetter ControllerConfigGetter,
 	st *state.State,
 	resources facade.Resources,
 	authorizer facade.Authorizer,
 	accessUnit GetAuthFunc,
 	logger loggo.Logger,
 ) *UnitStateAPI {
-	return NewUnitStateAPI(UnitStateState{St: st}, resources, authorizer, accessUnit, logger)
+	return NewUnitStateAPI(controllerConfigGetter, UnitStateState{St: st}, resources, authorizer, accessUnit, logger)
 }
 
 // NewUnitStateAPI returns a new UnitStateAPI. Currently both
 // GetAuthFuncs can used to determine current permissions.
 func NewUnitStateAPI(
+	controllerConfigGetter ControllerConfigGetter,
 	backend UnitStateBackend,
 	resources facade.Resources,
 	authorizer facade.Authorizer,
 	accessUnit GetAuthFunc,
 	logger loggo.Logger,
 ) *UnitStateAPI {
 	return &UnitStateAPI{
-		backend:    backend,
-		resources:  resources,
-		accessUnit: accessUnit,
-		logger:     logger,
+		controllerConfigGetter: controllerConfigGetter,
+		backend:                backend,
+		resources:              resources,
+		accessUnit:             accessUnit,
+		logger:                 logger,
 	}
 }
 
@@ -142,7 +145,7 @@ func (u *UnitStateAPI) SetState(ctx context.Context, args params.SetUnitStateArg
 		return params.ErrorResults{}, errors.Trace(err)
 	}
 
-	ctrlCfg, err := u.backend.ControllerConfig()
+	ctrlCfg, err := u.controllerConfigGetter.ControllerConfig(ctx)
 	if err != nil {
 		return params.ErrorResults{}, errors.Trace(err)
 	}

apiserver/common/unitstate_test.go

@@ -30,6 +30,8 @@ type unitStateSuite struct {
 	mockBackend *mocks.MockUnitStateBackend
 	mockUnit    *mocks.MockUnitStateUnit
 	mockOp      *mocks.MockModelOperation
+
+	controllerConfigGetter *mocks.MockControllerConfigGetter
 }
 
 var _ = gc.Suite(&unitStateSuite{})
@@ -48,6 +50,7 @@ func (s *unitStateSuite) assertBackendApi(c *gc.C) *gomock.Controller {
 	s.mockBackend = mocks.NewMockUnitStateBackend(ctrl)
 	s.mockUnit = mocks.NewMockUnitStateUnit(ctrl)
 	s.mockOp = mocks.NewMockModelOperation(ctrl)
+	s.controllerConfigGetter = mocks.NewMockControllerConfigGetter(ctrl)
 
 	unitAuthFunc := func() (common.AuthFunc, error) {
 		return func(tag names.Tag) bool {
@@ -59,7 +62,7 @@ func (s *unitStateSuite) assertBackendApi(c *gc.C) *gomock.Controller {
 	}
 
 	s.api = common.NewUnitStateAPI(
-		s.mockBackend, resources, authorizer, unitAuthFunc, loggo.GetLogger("juju.apiserver.common"))
+		s.controllerConfigGetter, s.mockBackend, resources, authorizer, unitAuthFunc, loggo.GetLogger("juju.apiserver.common"))
 	return ctrl
 }
 
@@ -100,7 +103,7 @@ func (s *unitStateSuite) expectSetStateOperation() string {
 	unitState.SetUniterState(expUniterState)
 
 	// Mock controller config which provides the limits passed to SetStateOperation.
-	s.mockBackend.EXPECT().ControllerConfig().Return(
+	s.controllerConfigGetter.EXPECT().ControllerConfig(gomock.Any()).Return(
 		controller.Config{
 			"max-charm-state-size": 123,
 			"max-agent-state-size": 456,

apiserver/facades/agent/meterstatus/meterstatus.go

@@ -19,18 +19,20 @@ import (
 	"github.com/juju/juju/state/watcher"
 )
 
+// ControllerConfigGetter defines a method for getting the controller config.
+type ControllerConfigGetter interface {
+	ControllerConfig(context.Context) (controller.Config, error)
+}
+
 // MeterStatus defines the methods exported by the meter status API facade.
 type MeterStatus interface {
 	GetMeterStatus(ctx context.Context, args params.Entities) (params.MeterStatusResults, error)
 	WatchMeterStatus(ctx context.Context, args params.Entities) (params.NotifyWatchResults, error)
 }
 
 // MeterStatusState represents the state of an model required by the MeterStatus.
-//
-//go:generate go run go.uber.org/mock/mockgen -package mocks -destination mocks/meterstatus_mock.go github.com/juju/juju/apiserver/facades/agent/meterstatus MeterStatusState
 type MeterStatusState interface {
 	ApplyOperation(state.ModelOperation) error
-	ControllerConfig() (controller.Config, error)
 
 	// Application returns a application state by name.
 	Application(name string) (*state.Application, error)
@@ -53,6 +55,7 @@ type MeterStatusAPI struct {
 
 // NewMeterStatusAPI creates a new API endpoint for dealing with unit meter status.
 func NewMeterStatusAPI(
+	controllerConfigGetter ControllerConfigGetter,
 	st MeterStatusState,
 	resources facade.Resources,
 	authorizer facade.Authorizer,
@@ -68,6 +71,7 @@ func NewMeterStatusAPI(
 		accessUnit: accessUnit,
 		resources:  resources,
 		UnitStateAPI: common.NewUnitStateAPI(
+			controllerConfigGetter,
 			unitStateShim{st},
 			resources,
 			authorizer,
@@ -157,10 +161,6 @@ func (s unitStateShim) ApplyOperation(op state.ModelOperation) error {
 	return s.st.ApplyOperation(op)
 }
 
-func (s unitStateShim) ControllerConfig() (controller.Config, error) {
-	return s.st.ControllerConfig()
-}
-
 func (s unitStateShim) Unit(name string) (common.UnitStateUnit, error) {
 	return s.st.Unit(name)
 }