Skip to content

Commit

Permalink
Merge pull request juju#18393 from ycliuhw/model-status-service-layer
Browse files Browse the repository at this point in the history
juju#18393

This PR introduces the a new service method in the `model` domain package for the model status feature: 

- Status - the model status will be computed in the service layer and returned as:
 - `destroying` if the model is being destroyed;
 - `suspended` if the model has an invalid cloud credential;
 - `busy` if the model is being migrated;
 - in other cases, it returns `available`;

Drive-by: 
- after discussion, we decided to compute all model statuses, so the model status DDL has been removed.

## 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
- [ ] ~[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

unit tests.

## Documentation changes

No

## Links

**Jira card:** [JUJU-7099](https://warthogs.atlassian.net/browse/JUJU-7099)



[JUJU-7099]: https://warthogs.atlassian.net/browse/JUJU-7099?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ
  • Loading branch information
jujubot authored Nov 26, 2024
2 parents 96b8e89 + 5d26091 commit 2c0a80f
Show file tree
Hide file tree
Showing 7 changed files with 203 additions and 45 deletions.
80 changes: 65 additions & 15 deletions domain/model/service/modelservice.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,12 @@ package service
import (
"context"

"github.com/juju/clock"

coremodel "github.com/juju/juju/core/model"
corestatus "github.com/juju/juju/core/status"
"github.com/juju/juju/domain/model"
"github.com/juju/juju/internal/errors"
"github.com/juju/juju/internal/uuid"
)

Expand All @@ -24,37 +28,43 @@ type ModelState interface {
Model(context.Context) (coremodel.ReadOnlyModel, error)
}

// ModelGetterState represents the state required for reading all model
// information.
type ModelGetterState interface {
// ControllerState is the controller state required by this service. This is the
// controller database, not the model state.
type ControllerState interface {
// GetModel returns the model with the given UUID.
GetModel(context.Context, coremodel.UUID) (coremodel.Model, error)

// GetModelState returns the model state for the given model.
// It returns [modelerrors.NotFound] if the model does not exist for the given UUID.
GetModelState(context.Context, coremodel.UUID) (model.ModelState, error)
}

// ModelService defines a service for interacting with the underlying model
// state, as opposed to the controller state.
type ModelService struct {
modelID coremodel.UUID
modelGetterSt ModelGetterState
st ModelState
clock clock.Clock
modelID coremodel.UUID
controllerSt ControllerState
modelSt ModelState
}

// NewModelService returns a new Service for interacting with a models state.
func NewModelService(
modelID coremodel.UUID,
modelGetterSt ModelGetterState,
st ModelState,
controllerSt ControllerState,
modelSt ModelState,
) *ModelService {
return &ModelService{
modelID: modelID,
modelGetterSt: modelGetterSt,
st: st,
modelID: modelID,
controllerSt: controllerSt,
modelSt: modelSt,
}
}

// GetModelInfo returns the readonly model information for the model in
// question.
func (s *ModelService) GetModelInfo(ctx context.Context) (coremodel.ReadOnlyModel, error) {
return s.st.Model(ctx)
return s.modelSt.Model(ctx)
}

// CreateModel is responsible for creating a new model within the model
Expand All @@ -66,7 +76,7 @@ func (s *ModelService) CreateModel(
ctx context.Context,
controllerUUID uuid.UUID,
) error {
m, err := s.modelGetterSt.GetModel(ctx, s.modelID)
m, err := s.controllerSt.GetModel(ctx, s.modelID)
if err != nil {
return err
}
Expand All @@ -84,7 +94,7 @@ func (s *ModelService) CreateModel(
CredentialName: m.Credential.Name,
}

return s.st.Create(ctx, args)
return s.modelSt.Create(ctx, args)
}

// DeleteModel is responsible for removing a model from the system.
Expand All @@ -94,5 +104,45 @@ func (s *ModelService) CreateModel(
func (s *ModelService) DeleteModel(
ctx context.Context,
) error {
return s.st.Delete(ctx, s.modelID)
return s.modelSt.Delete(ctx, s.modelID)
}

// Status returns the current status of the model.
//
// The following error types can be expected to be returned:
// - [modelerrors.NotFound]: When the model does not exist.
func (s *ModelService) Status(ctx context.Context) (model.StatusInfo, error) {
modelState, err := s.controllerSt.GetModelState(ctx, s.modelID)
if err != nil {
return model.StatusInfo{}, errors.Capture(err)
}

now := s.clock.Now()
if modelState.HasInvalidCloudCredential {
return model.StatusInfo{
Status: corestatus.Suspended,
Message: "suspended since cloud credential is not valid",
Reason: modelState.InvalidCloudCredentialReason,
Since: now,
}, nil
}
if modelState.Destroying {
return model.StatusInfo{
Status: corestatus.Destroying,
Message: "the model is being destroyed",
Since: now,
}, nil
}
if modelState.Migrating {
return model.StatusInfo{
Status: corestatus.Busy,
Message: "the model is being migrated",
Since: now,
}, nil
}

return model.StatusInfo{
Status: corestatus.Available,
Since: now,
}, nil
}
107 changes: 106 additions & 1 deletion domain/model/service/modelservice_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,17 @@ package service

import (
"context"
"time"

"github.com/juju/clock/testclock"
"github.com/juju/testing"
jc "github.com/juju/testing/checkers"
gc "gopkg.in/check.v1"

corecredential "github.com/juju/juju/core/credential"
coremodel "github.com/juju/juju/core/model"
modeltesting "github.com/juju/juju/core/model/testing"
corestatus "github.com/juju/juju/core/status"
"github.com/juju/juju/domain/model"
modelerrors "github.com/juju/juju/domain/model/errors"
"github.com/juju/juju/internal/uuid"
Expand All @@ -21,6 +24,8 @@ import (
type dummyModelState struct {
models map[coremodel.UUID]model.ReadOnlyModelCreationArgs
setID coremodel.UUID

modelState map[coremodel.UUID]model.ModelState
}

func (d *dummyModelState) Create(ctx context.Context, args model.ReadOnlyModelCreationArgs) error {
Expand Down Expand Up @@ -80,6 +85,14 @@ func (d *dummyModelState) Delete(ctx context.Context, modelUUID coremodel.UUID)
return nil
}

func (d *dummyModelState) GetModelState(_ context.Context, modelUUID coremodel.UUID) (model.ModelState, error) {
mState, ok := d.modelState[modelUUID]
if !ok {
return model.ModelState{}, modelerrors.NotFound
}
return mState, nil
}

type modelServiceSuite struct {
testing.IsolationSuite

Expand All @@ -91,7 +104,8 @@ var _ = gc.Suite(&modelServiceSuite{})

func (s *modelServiceSuite) SetUpTest(c *gc.C) {
s.state = &dummyModelState{
models: map[coremodel.UUID]model.ReadOnlyModelCreationArgs{},
models: map[coremodel.UUID]model.ReadOnlyModelCreationArgs{},
modelState: map[coremodel.UUID]model.ModelState{},
}

s.controllerUUID = uuid.MustNewUUID()
Expand Down Expand Up @@ -146,3 +160,94 @@ func (s *modelServiceSuite) TestModelDeletion(c *gc.C) {
_, exists := s.state.models[id]
c.Assert(exists, jc.IsFalse)
}

func (s *modelServiceSuite) TestStatusSuspended(c *gc.C) {
id := modeltesting.GenModelUUID(c)
svc := NewModelService(id, s.state, s.state)
svc.clock = testclock.NewClock(time.Time{})

s.state.setID = id
s.state.modelState[id] = model.ModelState{
HasInvalidCloudCredential: true,
InvalidCloudCredentialReason: "invalid credential",
}

now := svc.clock.Now()
status, err := svc.Status(context.Background())
c.Assert(err, jc.ErrorIsNil)
c.Assert(status.Status, gc.Equals, corestatus.Suspended)
c.Assert(status.Message, gc.Equals, "suspended since cloud credential is not valid")
c.Assert(status.Reason, gc.Equals, "invalid credential")
c.Assert(status.Since, jc.Almost, now)
}

func (s *modelServiceSuite) TestStatusDestroying(c *gc.C) {
id := modeltesting.GenModelUUID(c)
svc := &ModelService{
clock: testclock.NewClock(time.Time{}),
modelID: id,
controllerSt: s.state,
modelSt: s.state,
}

s.state.setID = id
s.state.modelState[id] = model.ModelState{
Destroying: true,
}

now := svc.clock.Now()
status, err := svc.Status(context.Background())
c.Assert(err, jc.ErrorIsNil)
c.Assert(status.Status, gc.Equals, corestatus.Destroying)
c.Assert(status.Message, gc.Equals, "the model is being destroyed")
c.Assert(status.Since, jc.Almost, now)
}

func (s *modelServiceSuite) TestStatusBusy(c *gc.C) {
id := modeltesting.GenModelUUID(c)
svc := &ModelService{
clock: testclock.NewClock(time.Time{}),
modelID: id,
controllerSt: s.state,
modelSt: s.state,
}

s.state.setID = id
s.state.modelState[id] = model.ModelState{
Migrating: true,
}

now := svc.clock.Now()
status, err := svc.Status(context.Background())
c.Assert(err, jc.ErrorIsNil)
c.Assert(status.Status, gc.Equals, corestatus.Busy)
c.Assert(status.Message, gc.Equals, "the model is being migrated")
c.Assert(status.Since, jc.Almost, now)
}

func (s *modelServiceSuite) TestStatus(c *gc.C) {
id := modeltesting.GenModelUUID(c)
svc := &ModelService{
clock: testclock.NewClock(time.Time{}),
modelID: id,
controllerSt: s.state,
modelSt: s.state,
}

s.state.setID = id
s.state.modelState[id] = model.ModelState{}

now := svc.clock.Now()
status, err := svc.Status(context.Background())
c.Assert(err, jc.ErrorIsNil)
c.Assert(status.Status, gc.Equals, corestatus.Available)
c.Assert(status.Since, jc.Almost, now)
}

func (s *modelServiceSuite) TestStatusFaildModelNotFound(c *gc.C) {
id := modeltesting.GenModelUUID(c)
svc := NewModelService(id, s.state, s.state)

_, err := svc.Status(context.Background())
c.Assert(err, jc.ErrorIs, modelerrors.NotFound)
}
Original file line number Diff line number Diff line change
Expand Up @@ -1680,3 +1680,9 @@ AND ot.type = $dbPermission.object_type
}
return nil
}

// GetModelState returns the model state for the given model UUID.
func (s *State) GetModelState(context.Context, coremodel.UUID) (model.ModelState, error) {
// TODO: Implement GetModelState
return model.ModelState{}, nil
}
File renamed without changes.
26 changes: 26 additions & 0 deletions domain/model/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,14 @@ package model

import (
"fmt"
"time"

"github.com/juju/errors"
"github.com/juju/version/v2"

"github.com/juju/juju/core/credential"
coremodel "github.com/juju/juju/core/model"
corestatus "github.com/juju/juju/core/status"
"github.com/juju/juju/core/user"
"github.com/juju/juju/internal/uuid"
)
Expand Down Expand Up @@ -177,3 +179,27 @@ func WithDeleteDB() DeleteModelOption {
o.deleteDB = true
}
}

// StatusInfo holds a Status and associated information of a model.
type StatusInfo struct {
// Status is the current status of the model.
Status corestatus.Status
// Message is a human-readable message that describes the current status of the model.
Message string
// Reason is a human-readable message that describes the reason for the current status of the model.
Reason string
// Since is the time when the model entered the current status.
Since time.Time
}

// ModelState describes the state of a model.
type ModelState struct {
// Destroying is a boolean value that indicates if the model is being destroyed.
Destroying bool
// Migrating is a boolean value that indicates if the model is being migrated.
Migrating bool
// HasInvalidCloudCredential is a boolean value that indicates if the model's cloud credential is invalid.
HasInvalidCloudCredential bool
// InvalidCloudCredentialReason is a string that describes the reason for the model's cloud credential being invalid.
InvalidCloudCredentialReason string
}
25 changes: 0 additions & 25 deletions domain/schema/model/sql/0025-model-status.sql

This file was deleted.

4 changes: 0 additions & 4 deletions domain/schema/schema_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -296,10 +296,6 @@ func (s *schemaSuite) TestModelTables(c *gc.C) {
// Model config
"model_config",

// Model status
"model_status",
"model_status_value",

// Object store metadata
"object_store_metadata",
"object_store_metadata_path",
Expand Down

0 comments on commit 2c0a80f

Please sign in to comment.