Skip to content

Commit

Permalink
Merge pull request juju#17519 from barrettj12/caasmodeloperator-model…
Browse files Browse the repository at this point in the history
…service

juju#17519

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

Refactor the caasmodeloperator facade to use a model config service instead of state. The facade uses the new `ModelConfig` and `Watch` methods instead of the state methods.

Unit tests have been changed accordingly, with the ModelConfigService mocked out by a gomock-generated mock.

As a result, many of the methods/interfaces/structs in `state.go` were able to be removed.

## Checklist

<!-- If an item is not applicable, use `~strikethrough~`. -->

- [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
- ~[ ] [Integration tests](https://github.com/juju/juju/tree/main/tests), with comments saying what you're testing~
- ~[ ] [doc.go](https://discourse.charmhub.io/t/readme-in-packages/451) added or updated in changed packages~

## QA steps

We are going to force a controller upgrade and check that the model operator pod is updated.

First, make sure you have an account on [Docker Hub](https://hub.docker.com/) and that you have `docker login`-ed on your local machine.

Build Juju and set up the image repository:
```bash
export JUJU_BUILD_NUMBER=0
export DOCKER_USERNAME=docker.io/<your-docker-username>
make seed-repository
make install # or go-build
make simplestreams push-release-operator-image
```

In a new terminal window, start the simplestreams server.
```
$ cd _build/simplestreams/tools
$ python3 -m http.server
Serving HTTP on 0.0.0.0 port 8000 (http://0.0.0.0:8000/) ...
```

Back in the original terminal, we can bootstrap Juju with the right simplestreams & repository (providing your computer's IP address as the `agent-metadata-url`):
```
$ juju bootstrap minikube --config agent-metadata-url=http://192.168.1.25:8000/ --config caas-image-repo=$DOCKER_USERNAME
$ juju switch controller
$ juju status
Model Controller Cloud/Region Version Timestamp
controller minikube minikube 4.0-beta4 14:03:34+10:00
```

Go to `version/version.go` and increment the version to `4.0-beta5`. Now rebuild Juju and update the simplestreams/image repo:
```
make install # or go-build
make simplestreams push-release-operator-image
```

Now we should be able to upgrade the controller.
```
$ juju upgrade-controller
best version:
 4.0-beta5
started upgrade to 4.0-beta5
$ juju status
Model Controller Cloud/Region Version Timestamp
controller minikube minikube 4.0-beta5 14:06:12+10:00
$ kubectl -ncontroller-minikube describe deployment modeloperator | grep jujud-operator:
 Image: docker.io/hpidcock/jujud-operator:4.0-beta5
```

## Links

**Jira card:** JUJU-6100
  • Loading branch information
jujubot committed Jun 20, 2024
2 parents 6e60b4e + b934b7b commit ba2f118
Show file tree
Hide file tree
Showing 8 changed files with 192 additions and 69 deletions.
4 changes: 0 additions & 4 deletions apiserver/facades/controller/caasmodeloperator/mock_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,10 +59,6 @@ func (st *mockState) FindEntity(tag names.Tag) (state.Entity, error) {
return nil, errors.NotFoundf("entity %v", tag)
}

func (st *mockState) Model() (Model, error) {
return st.model, nil
}

func (m *mockState) WatchAPIHostPortsForAgents() state.NotifyWatcher {
return m.apiHostPortsForAgentsWatcher
}
Expand Down
40 changes: 19 additions & 21 deletions apiserver/facades/controller/caasmodeloperator/operator.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import (
"github.com/juju/juju/apiserver/internal"
"github.com/juju/juju/controller"
corelogger "github.com/juju/juju/core/logger"
corewatcher "github.com/juju/juju/core/watcher"
"github.com/juju/juju/core/model"
"github.com/juju/juju/core/watcher/eventsource"
"github.com/juju/juju/internal/cloudconfig/podcfg"
"github.com/juju/juju/internal/docker"
Expand All @@ -26,23 +26,19 @@ import (
// TODO (manadart 2020-10-21): Remove the ModelUUID method
// from the next version of this facade.

type ControllerConfigService interface {
ControllerConfig(context.Context) (controller.Config, error)
WatchControllerConfig() (corewatcher.StringsWatcher, error)
}

// API represents the controller model operator facade.
type API struct {
*common.APIAddresser
*common.PasswordChanger

auth facade.Authorizer
ctrlState CAASControllerState
state CAASModelOperatorState
controllerConfigService ControllerConfigService
modelConfigService ModelConfigService
logger corelogger.Logger

resources facade.Resources
modelUUID model.UUID
}

// NewAPI is alternative means of constructing a controller model facade.
Expand All @@ -52,7 +48,9 @@ func NewAPI(
ctrlSt CAASControllerState,
st CAASModelOperatorState,
controllerConfigService ControllerConfigService,
modelConfigService ModelConfigService,
logger corelogger.Logger,
modelUUID model.UUID,
) (*API, error) {

if !authorizer.AuthController() {
Expand All @@ -64,10 +62,11 @@ func NewAPI(
APIAddresser: common.NewAPIAddresser(ctrlSt, resources),
PasswordChanger: common.NewPasswordChanger(st, common.AuthFuncForTagKind(names.ModelTagKind)),
ctrlState: ctrlSt,
state: st,
controllerConfigService: controllerConfigService,
modelConfigService: modelConfigService,
logger: logger,
resources: resources,
modelUUID: modelUUID,
}, nil
}

Expand All @@ -76,11 +75,6 @@ func NewAPI(
func (a *API) WatchModelOperatorProvisioningInfo(ctx context.Context) (params.NotifyWatchResult, error) {
result := params.NotifyWatchResult{}

model, err := a.state.Model()
if err != nil {
return result, errors.Trace(err)
}

controllerConfigWatcher, err := a.controllerConfigService.WatchControllerConfig()
if err != nil {
return result, errors.Trace(err)
Expand All @@ -91,12 +85,20 @@ func (a *API) WatchModelOperatorProvisioningInfo(ctx context.Context) (params.No
}

controllerAPIHostPortsWatcher := a.ctrlState.WatchAPIHostPortsForAgents()
modelConfigWatcher := model.WatchForModelConfigChanges()

modelConfigWatcher, err := a.modelConfigService.Watch()
if err != nil {
return result, errors.Trace(err)
}
modelConfigNotifyWatcher, err := eventsource.NewStringsNotifyWatcher(modelConfigWatcher)
if err != nil {
return result, errors.Trace(err)
}

multiWatcher, err := eventsource.NewMultiNotifyWatcher(ctx,
controllerConfigNotifyWatcher,
controllerAPIHostPortsWatcher,
modelConfigWatcher,
modelConfigNotifyWatcher,
)

if err != nil {
Expand All @@ -120,11 +122,7 @@ func (a *API) ModelOperatorProvisioningInfo(ctx context.Context) (params.ModelOp
return result, err
}

model, err := a.state.Model()
if err != nil {
return result, errors.Trace(err)
}
modelConfig, err := model.ModelConfig(ctx)
modelConfig, err := a.modelConfigService.ModelConfig(ctx)
if err != nil {
return result, errors.Trace(err)
}
Expand Down Expand Up @@ -169,7 +167,7 @@ func (a *API) ModelOperatorProvisioningInfo(ctx context.Context) (params.ModelOp
// embedded APIAddresser *without* bumping the facade version.
// It should be blanked when this facade version is next incremented.
func (a *API) ModelUUID(ctx context.Context) params.StringResult {
return params.StringResult{Result: a.state.ModelUUID()}
return params.StringResult{Result: a.modelUUID.String()}
}

// APIHostPorts returns the API server addresses.
Expand Down
40 changes: 25 additions & 15 deletions apiserver/facades/controller/caasmodeloperator/operator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,17 @@ import (

"github.com/juju/names/v5"
jc "github.com/juju/testing/checkers"
"github.com/juju/version/v2"
"go.uber.org/mock/gomock"
gc "gopkg.in/check.v1"

"github.com/juju/juju/apiserver/common"
apiservertesting "github.com/juju/juju/apiserver/testing"
"github.com/juju/juju/controller"
"github.com/juju/juju/core/model"
"github.com/juju/juju/core/watcher/eventsource"
"github.com/juju/juju/core/watcher/watchertest"
"github.com/juju/juju/environs/config"
"github.com/juju/juju/internal/cloudconfig/podcfg"
loggertesting "github.com/juju/juju/internal/logger/testing"
statetesting "github.com/juju/juju/state/testing"
Expand All @@ -30,6 +33,7 @@ type ModelOperatorSuite struct {
resources *common.Resources
state *mockState
controllerConfigService *MockControllerConfigService
modelConfigService *MockModelConfigService
}

var _ = gc.Suite(&ModelOperatorSuite{})
Expand All @@ -38,6 +42,13 @@ func (m *ModelOperatorSuite) TestProvisioningInfo(c *gc.C) {
ctrl := m.setupMocks(c)
defer ctrl.Finish()

m.modelConfigService.EXPECT().ModelConfig(gomock.Any()).Return(config.New(false, map[string]any{
config.NameKey: "controller",
config.UUIDKey: "deadbeef-0bad-400d-8000-4b1d0d06f00d",
config.TypeKey: "ec2",
config.AgentVersionKey: "4.0.0",
}))

info, err := m.api.ModelOperatorProvisioningInfo(context.Background())
c.Assert(err, jc.ErrorIsNil)

Expand All @@ -51,34 +62,29 @@ func (m *ModelOperatorSuite) TestProvisioningInfo(c *gc.C) {
c.Assert(info.ImageDetails.Auth, gc.Equals, `xxxxx==`)
c.Assert(info.ImageDetails.Repository, gc.Equals, `test-account`)

model, err := m.state.Model()
c.Assert(err, jc.ErrorIsNil)

modelConfig, err := model.ModelConfig(context.Background())
expectedVersion, err := version.Parse("4.0.0")
c.Assert(err, jc.ErrorIsNil)

vers, ok := modelConfig.AgentVersion()
c.Assert(ok, jc.IsTrue)

c.Assert(vers, jc.DeepEquals, info.Version)
c.Assert(info.Version, jc.DeepEquals, expectedVersion)
}

func (m *ModelOperatorSuite) TestWatchProvisioningInfo(c *gc.C) {
defer m.setupMocks(c).Finish()

controllerConfigChanged := make(chan []string, 1)
modelConfigChanged := make(chan struct{}, 1)
modelConfigChanged := make(chan []string, 1)
apiHostPortsForAgentsChanged := make(chan struct{}, 1)

watcher := watchertest.NewMockStringsWatcher(controllerConfigChanged)
m.controllerConfigService.EXPECT().WatchControllerConfig().Return(watcher, nil)
controllerConfigWatcher := watchertest.NewMockStringsWatcher(controllerConfigChanged)
m.controllerConfigService.EXPECT().WatchControllerConfig().Return(controllerConfigWatcher, nil)

m.state.apiHostPortsForAgentsWatcher = statetesting.NewMockNotifyWatcher(apiHostPortsForAgentsChanged)
m.state.model.modelConfigChanged = statetesting.NewMockNotifyWatcher(modelConfigChanged)

modelConfigWatcher := watchertest.NewMockStringsWatcher(modelConfigChanged)
m.modelConfigService.EXPECT().Watch().Return(modelConfigWatcher, nil)

controllerConfigChanged <- []string{}
apiHostPortsForAgentsChanged <- struct{}{}
modelConfigChanged <- struct{}{}
modelConfigChanged <- []string{}

results, err := m.api.WatchModelOperatorProvisioningInfo(context.Background())
c.Assert(err, jc.ErrorIsNil)
Expand All @@ -101,6 +107,8 @@ func (m *ModelOperatorSuite) setupMocks(c *gc.C) *gomock.Controller {
`[1:],
}, nil).AnyTimes()

m.modelConfigService = NewMockModelConfigService(ctrl)

m.resources = common.NewResources()

m.authorizer = &apiservertesting.FakeAuthorizer{
Expand All @@ -118,7 +126,9 @@ func (m *ModelOperatorSuite) setupMocks(c *gc.C) *gomock.Controller {

c.Logf("m.state.1operatorRepo %q", m.state.operatorRepo)

api, err := NewAPI(m.authorizer, m.resources, m.state, m.state, m.controllerConfigService, loggertesting.WrapCheckLog(c))
api, err := NewAPI(m.authorizer, m.resources, m.state, m.state,
m.controllerConfigService, m.modelConfigService,
loggertesting.WrapCheckLog(c), model.UUID(m.authorizer.ModelUUID))
c.Assert(err, jc.ErrorIsNil)

m.api = api
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import (
gc "gopkg.in/check.v1"
)

//go:generate go run go.uber.org/mock/mockgen -typed -package caasmodeloperator -destination package_mock_test.go github.com/juju/juju/apiserver/facades/controller/caasmodeloperator ControllerConfigService
//go:generate go run go.uber.org/mock/mockgen -typed -package caasmodeloperator -destination service_mock_test.go github.com/juju/juju/apiserver/facades/controller/caasmodeloperator ControllerConfigService,ModelConfigService

func TestAll(t *testing.T) {
gc.TestingT(t)
Expand Down
17 changes: 13 additions & 4 deletions apiserver/facades/controller/caasmodeloperator/register.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,23 +15,32 @@ import (
// Register is called to expose a package of facades onto a given registry.
func Register(registry facade.FacadeRegistry) {
registry.MustRegister("CAASModelOperator", 1, func(stdCtx context.Context, ctx facade.ModelContext) (facade.Facade, error) {
return newAPIFromContext(ctx)
return newAPIFromContext(stdCtx, ctx)
}, reflect.TypeOf((*API)(nil)))
}

// newAPIFromContext creates a new controller model facade from the supplied
// context.
func newAPIFromContext(ctx facade.ModelContext) (*API, error) {
func newAPIFromContext(stdCtx context.Context, ctx facade.ModelContext) (*API, error) {
authorizer := ctx.Auth()
resources := ctx.Resources()
systemState, err := ctx.StatePool().SystemState()
if err != nil {
return nil, errors.Trace(err)
}

serviceFactory := ctx.ServiceFactory()
modelInfo, err := serviceFactory.ModelInfo().GetModelInfo(stdCtx)
if err != nil {
return nil, errors.Trace(err)
}

return NewAPI(authorizer, resources,
stateShim{systemState},
stateShim{ctx.State()},
systemState,
ctx.State(),
ctx.ServiceFactory().ControllerConfig(),
ctx.ServiceFactory().Config(),
ctx.Logger().Child("caasmodeloperator"),
modelInfo.UUID,
)
}
30 changes: 30 additions & 0 deletions apiserver/facades/controller/caasmodeloperator/service.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
// Copyright 2024 Canonical Ltd.
// Licensed under the AGPLv3, see LICENCE file for details.

package caasmodeloperator

import (
"context"

"github.com/juju/juju/controller"
"github.com/juju/juju/core/watcher"
"github.com/juju/juju/environs/config"
)

// ControllerConfigService provides access to the controller configuration.
type ControllerConfigService interface {
// ControllerConfig returns the config values for the controller.
ControllerConfig(context.Context) (controller.Config, error)
// WatchControllerConfig returns a watcher that returns keys for any
// changes to controller config.
WatchControllerConfig() (watcher.StringsWatcher, error)
}

// ModelConfigService provides access to the model's configuration.
type ModelConfigService interface {
// ModelConfig returns the current config for the model.
ModelConfig(context.Context) (*config.Config, error)
// Watch returns a watcher that returns keys for any changes to model
// config.
Watch() (watcher.StringsWatcher, error)
}
Loading

0 comments on commit ba2f118

Please sign in to comment.