diff --git a/agent/agentbootstrap/bootstrap.go b/agent/agentbootstrap/bootstrap.go index 17ca16c9f28..0c748649e07 100644 --- a/agent/agentbootstrap/bootstrap.go +++ b/agent/agentbootstrap/bootstrap.go @@ -263,6 +263,7 @@ func (b *AgentBootstrap) Initialize(ctx stdcontext.Context) (_ *state.Controller controllerModelCreateFunc, ), database.BootstrapModelConcern(controllerModelUUID, + modelbootstrap.CreateReadOnlyModel(controllerModelArgs.AsReadOnly()), modelconfigbootstrap.SetModelConfig(stateParams.ControllerModelConfig, controllerModelDefaults), ), } diff --git a/core/modelmigration/coordinator.go b/core/modelmigration/coordinator.go index db48a38d90c..2c79e262dd5 100644 --- a/core/modelmigration/coordinator.go +++ b/core/modelmigration/coordinator.go @@ -29,7 +29,7 @@ func (b *BaseOperation) Execute(context.Context, description.Model) error { } // Rollback is a no-op by default. -func (b *BaseOperation) Rollback(context.Context) error { +func (b *BaseOperation) Rollback(context.Context, description.Model) error { return nil } @@ -54,7 +54,7 @@ type Operation interface { // Rollback should only be called on controller DB operations. The // model DB operations are not rolled back, but instead we remove the // db, clearing the model. - Rollback(context.Context) error + Rollback(context.Context, description.Model) error } // Scope is a collection of database txn runners that can be used by the @@ -121,7 +121,7 @@ func (m *Coordinator) Perform(ctx context.Context, scope Scope, model descriptio defer func() { if err != nil { for ; current >= 0; current-- { - if rollbackErr := m.operations[current].Rollback(ctx); rollbackErr != nil { + if rollbackErr := m.operations[current].Rollback(ctx, model); rollbackErr != nil { err = errors.Annotatef(err, "rollback operation at %d with %v", current, rollbackErr) } } diff --git a/core/modelmigration/coordinator_test.go b/core/modelmigration/coordinator_test.go index f93bc3aeb71..05bf6234f00 100644 --- a/core/modelmigration/coordinator_test.go +++ b/core/modelmigration/coordinator_test.go @@ -63,7 +63,7 @@ func (s *migrationSuite) TestPerformWithRollbackAtSetup(c *gc.C) { // We do care about the order of the calls. gomock.InOrder( s.op.EXPECT().Setup(s.scope).Return(errors.New("boom")), - s.op.EXPECT().Rollback(gomock.Any()).Return(nil), + s.op.EXPECT().Rollback(gomock.Any(), s.model).Return(nil), ) err := m.Perform(context.Background(), s.scope, s.model) @@ -82,7 +82,7 @@ func (s *migrationSuite) TestPerformWithRollbackAtExecution(c *gc.C) { gomock.InOrder( s.op.EXPECT().Setup(s.scope).Return(nil), s.op.EXPECT().Execute(gomock.Any(), s.model).Return(errors.New("boom")), - s.op.EXPECT().Rollback(gomock.Any()).Return(nil), + s.op.EXPECT().Rollback(gomock.Any(), s.model).Return(nil), ) err := m.Perform(context.Background(), s.scope, s.model) @@ -101,7 +101,7 @@ func (s *migrationSuite) TestPerformWithRollbackError(c *gc.C) { gomock.InOrder( s.op.EXPECT().Setup(s.scope).Return(nil), s.op.EXPECT().Execute(gomock.Any(), s.model).Return(errors.New("boom")), - s.op.EXPECT().Rollback(gomock.Any()).Return(errors.New("sad")), + s.op.EXPECT().Rollback(gomock.Any(), s.model).Return(errors.New("sad")), ) err := m.Perform(context.Background(), s.scope, s.model) diff --git a/core/modelmigration/op_mock_test.go b/core/modelmigration/op_mock_test.go index 3728e923369..0cdb7903f0d 100644 --- a/core/modelmigration/op_mock_test.go +++ b/core/modelmigration/op_mock_test.go @@ -55,17 +55,17 @@ func (mr *MockOperationMockRecorder) Execute(arg0, arg1 any) *gomock.Call { } // Rollback mocks base method. -func (m *MockOperation) Rollback(arg0 context.Context) error { +func (m *MockOperation) Rollback(arg0 context.Context, arg1 description.Model) error { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "Rollback", arg0) + ret := m.ctrl.Call(m, "Rollback", arg0, arg1) ret0, _ := ret[0].(error) return ret0 } // Rollback indicates an expected call of Rollback. -func (mr *MockOperationMockRecorder) Rollback(arg0 any) *gomock.Call { +func (mr *MockOperationMockRecorder) Rollback(arg0, arg1 any) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Rollback", reflect.TypeOf((*MockOperation)(nil).Rollback), arg0) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Rollback", reflect.TypeOf((*MockOperation)(nil).Rollback), arg0, arg1) } // Setup mocks base method. diff --git a/core/modelmigration/testing/operation.go b/core/modelmigration/testing/operation.go new file mode 100644 index 00000000000..d0e37d5e10d --- /dev/null +++ b/core/modelmigration/testing/operation.go @@ -0,0 +1,22 @@ +// Copyright 2024 Canonical Ltd. +// Licensed under the AGPLv3, see LICENCE file for details. + +package testing + +import "github.com/juju/juju/core/modelmigration" + +// IgnoredSetupOperation is a helper function to test the operation within a +// coordinator. +// This just ignores the setup call of the coordinator. It is expected that +// the operation will have all the information up front. +func IgnoredSetupOperation(op modelmigration.Operation) modelmigration.Operation { + return &ignoredSetupOperation{Operation: op} +} + +type ignoredSetupOperation struct { + modelmigration.Operation +} + +func (i *ignoredSetupOperation) Setup(modelmigration.Scope) error { + return nil +} diff --git a/domain/model/bootstrap/bootstrap.go b/domain/model/bootstrap/bootstrap.go index 7b2a2b5231b..d09d449502d 100644 --- a/domain/model/bootstrap/bootstrap.go +++ b/domain/model/bootstrap/bootstrap.go @@ -71,3 +71,20 @@ func CreateModel( }) } } + +// CreateReadOnlyModel creates a new model within the model database with all of +// its associated metadata. The data will be read-only and cannot be modified +// once created. +func CreateReadOnlyModel( + args model.ReadOnlyModelCreationArgs, +) func(context.Context, database.TxnRunner) error { + return func(ctx context.Context, db database.TxnRunner) error { + if err := args.Validate(); err != nil { + return fmt.Errorf("model creation args: %w", err) + } + + return db.StdTxn(ctx, func(ctx context.Context, tx *sql.Tx) error { + return state.CreateReadOnlyModel(ctx, args, tx) + }) + } +} diff --git a/domain/model/bootstrap/bootstrap_test.go b/domain/model/bootstrap/bootstrap_test.go index a14add3895c..c47456f4e50 100644 --- a/domain/model/bootstrap/bootstrap_test.go +++ b/domain/model/bootstrap/bootstrap_test.go @@ -105,3 +105,22 @@ func (s *bootstrapSuite) TestUUIDIsRespected(c *gc.C) { c.Check(uuid, gc.Equals, modelUUID) } + +type modelBootstrapSuite struct { + schematesting.ModelSuite +} + +var _ = gc.Suite(&modelBootstrapSuite{}) + +func (s *modelBootstrapSuite) TestCreateReadOnlyModel(c *gc.C) { + fn := CreateReadOnlyModel(model.ReadOnlyModelCreationArgs{ + UUID: modeltesting.GenModelUUID(c), + Name: "test", + Type: coremodel.IAAS, + Cloud: "aws", + CloudRegion: "myregion", + }) + + err := fn(context.Background(), s.ModelTxnRunner()) + c.Assert(err, jc.ErrorIsNil) +} diff --git a/domain/model/modelmigration/import.go b/domain/model/modelmigration/import.go index 579e4e59181..52bf7f295c7 100644 --- a/domain/model/modelmigration/import.go +++ b/domain/model/modelmigration/import.go @@ -14,10 +14,9 @@ import ( coremodel "github.com/juju/juju/core/model" "github.com/juju/juju/core/modelmigration" coreuser "github.com/juju/juju/core/user" - "github.com/juju/juju/domain/model" domainmodel "github.com/juju/juju/domain/model" - "github.com/juju/juju/domain/model/service" - "github.com/juju/juju/domain/model/state" + modelservice "github.com/juju/juju/domain/model/service" + modelstate "github.com/juju/juju/domain/model/state" usererrors "github.com/juju/juju/domain/user/errors" userservice "github.com/juju/juju/domain/user/service" userstate "github.com/juju/juju/domain/user/state" @@ -36,11 +35,19 @@ func RegisterImport(coordinator Coordinator) { coordinator.Add(&importOperation{}) } -// ImportService defines the model service used to import models from another +// ModelService defines the model service used to import models from another // controller to this one. -type ImportService interface { +type ModelService interface { // CreateModel is responsible for creating a new model that is being imported. - CreateModel(context.Context, model.ModelCreationArgs) (coremodel.UUID, error) + CreateModel(context.Context, domainmodel.ModelCreationArgs) (coremodel.UUID, error) + // DeleteModel is responsible for removing a model from the system. + DeleteModel(context.Context, coremodel.UUID) error +} + +type ReadOnlyModelService interface { + // CreateModel is responsible for creating a new read only model + // that is being imported. + CreateModel(context.Context, domainmodel.ReadOnlyModelCreationArgs) error } // UserService defines the user service used for model migration. @@ -57,20 +64,22 @@ type UserService interface { type importOperation struct { modelmigration.BaseOperation - service ImportService - userService UserService + modelService ModelService + readOnlyModelService ReadOnlyModelService + userService UserService } // Setup is responsible for taking the model migration scope and creating the // needed services used during import. func (i *importOperation) Setup(scope modelmigration.Scope) error { - i.service = service.NewService( - state.NewState(scope.ControllerDB()), - service.DefaultAgentBinaryFinder(), + i.modelService = modelservice.NewService( + modelstate.NewState(scope.ControllerDB()), + modelservice.DefaultAgentBinaryFinder(), + ) + i.readOnlyModelService = modelservice.NewModelService( + modelstate.NewModelState(scope.ModelDB()), ) - i.userService = userservice.NewService(userstate.NewState(scope.ControllerDB())) - return nil } @@ -82,52 +91,20 @@ func (i *importOperation) Setup(scope modelmigration.Scope) error { // If the user specified for the model cannot be found an error satisfying // [usererrors.NotFound] will be returned. func (i importOperation) Execute(ctx context.Context, model description.Model) error { - modelConfig := model.Config() - if modelConfig == nil { - return errors.New("model config is empty") - } - - modelNameI, exists := modelConfig[config.NameKey] - if !exists { - return fmt.Errorf( - "importing model during migration %w, no model name found in model config", - errors.NotValid, - ) - } - - modelNameS, ok := modelNameI.(string) - if !ok { - return fmt.Errorf( - "importing model during migration %w, establishing model name type as string. Got unknown type", - errors.NotValid, - ) - } - - uuidI, exists := modelConfig[config.UUIDKey] - if !exists { - return fmt.Errorf( - "importing model during migration %w, no model uuid found in model config", - errors.NotValid, - ) - } - - uuidS, ok := uuidI.(string) - if !ok { - return fmt.Errorf( - "importing model during migration %w, establishing model uuid type as string. Got unknown type", - errors.NotValid, - ) + modelName, uuid, err := i.getModelNameAndUUID(model) + if err != nil { + return fmt.Errorf("importing model during migration %w", errors.NotValid) } user, err := i.userService.GetUserByName(ctx, model.Owner().Name()) if errors.Is(err, usererrors.NotFound) { return fmt.Errorf("cannot import model %q with uuid %q, %w for name %q", - modelNameS, uuidS, usererrors.NotFound, model.Owner().Name(), + modelName, uuid, usererrors.NotFound, model.Owner().Name(), ) } else if err != nil { return fmt.Errorf( "importing model %q with uuid %q during migration, finding user %q: %w", - modelNameS, uuidS, model.Owner().Name(), err, + modelName, uuid, model.Owner().Name(), err, ) } @@ -144,19 +121,88 @@ func (i importOperation) Execute(ctx context.Context, model description.Model) e Cloud: model.Cloud(), CloudRegion: model.CloudRegion(), Credential: credential, - Name: modelNameS, + Name: modelName, Owner: user.UUID, Type: coremodel.ModelType(model.Type()), - UUID: coremodel.UUID(uuidS), + UUID: coremodel.UUID(uuid), } - _, err = i.service.CreateModel(ctx, args) + createdModelUUID, err := i.modelService.CreateModel(ctx, args) if err != nil { return fmt.Errorf( "importing model %q with uuid %q during migration: %w", - modelNameS, uuidS, err, + modelName, uuid, err, + ) + } + if createdModelUUID != args.UUID { + return fmt.Errorf( + "importing model %q with uuid %q during migration, created model uuid %q does not match expected uuid %q", + modelName, uuid, createdModelUUID, args.UUID, + ) + } + + // If the model is read only, we need to create a read only model. + err = i.readOnlyModelService.CreateModel(ctx, args.AsReadOnly()) + if err != nil { + return fmt.Errorf( + "importing read only model %q with uuid %q during migration: %w", + modelName, uuid, err, ) } + // NOTE: If we add any more steps to the import operation, we should + // consider adding a rollback operation to undo the changes made by the + // import operation. + return nil } + +// Rollback will attempt to rollback the import operation if it was +// unsuccessful. +func (i importOperation) Rollback(ctx context.Context, model description.Model) error { + // Attempt to rollback the model database if it was created. + modelName, uuid, err := i.getModelNameAndUUID(model) + if err != nil { + return fmt.Errorf("rollback of model during migration %w", errors.NotValid) + } + + modelUUID := coremodel.UUID(uuid) + + if err := i.modelService.DeleteModel(ctx, modelUUID); err != nil { + return fmt.Errorf( + "rollback of model %q with uuid %q during migration: %w", + modelName, uuid, err, + ) + } + + return nil +} + +func (i importOperation) getModelNameAndUUID(model description.Model) (string, string, error) { + modelConfig := model.Config() + if modelConfig == nil { + return "", "", errors.New("model config is empty") + } + + modelNameI, exists := modelConfig[config.NameKey] + if !exists { + return "", "", fmt.Errorf("no model name found in model config") + } + + modelNameS, ok := modelNameI.(string) + if !ok { + return "", "", fmt.Errorf("establishing model name type as string. Got unknown type") + } + + uuidI, exists := modelConfig[config.UUIDKey] + if !exists { + return "", "", fmt.Errorf("no model uuid found in model config") + } + + uuidS, ok := uuidI.(string) + if !ok { + return "", "", fmt.Errorf("establishing model uuid type as string. Got unknown type") + } + + return modelNameS, uuidS, nil +} diff --git a/domain/model/modelmigration/import_test.go b/domain/model/modelmigration/import_test.go index 94f02fdd59c..30cd69f5568 100644 --- a/domain/model/modelmigration/import_test.go +++ b/domain/model/modelmigration/import_test.go @@ -16,6 +16,8 @@ import ( "github.com/juju/juju/core/credential" coremodel "github.com/juju/juju/core/model" modeltesting "github.com/juju/juju/core/model/testing" + "github.com/juju/juju/core/modelmigration" + modelmigrationtesting "github.com/juju/juju/core/modelmigration/testing" coreuser "github.com/juju/juju/core/user" "github.com/juju/juju/domain/model" usererrors "github.com/juju/juju/domain/user/errors" @@ -24,8 +26,9 @@ import ( ) type importSuite struct { - importService *MockImportService - userService *MockUserService + modelService *MockModelService + readOnlyModelService *MockReadOnlyModelService + userService *MockUserService } var _ = gc.Suite(&importSuite{}) @@ -33,7 +36,8 @@ var _ = gc.Suite(&importSuite{}) func (s *importSuite) setupMocks(c *gc.C) *gomock.Controller { ctrl := gomock.NewController(c) - s.importService = NewMockImportService(ctrl) + s.modelService = NewMockModelService(ctrl) + s.readOnlyModelService = NewMockReadOnlyModelService(ctrl) s.userService = NewMockUserService(ctrl) return ctrl @@ -91,8 +95,8 @@ func (i *importSuite) TestModelOwnerNoExist(c *gc.C) { i.userService.EXPECT().GetUserByName(gomock.Any(), "tlm").Return(coreuser.User{}, usererrors.NotFound) importOp := importOperation{ - service: i.importService, - userService: i.userService, + modelService: i.modelService, + userService: i.userService, } modelUUID := modeltesting.GenModelUUID(c) @@ -121,22 +125,22 @@ func (i *importSuite) TestModelCreate(c *gc.C) { nil, ) - i.importService.EXPECT().CreateModel(gomock.Any(), - model.ModelCreationArgs{ - AgentVersion: jujuversion.Current, - Cloud: "AWS", - CloudRegion: "region1", - Credential: credential.ID{ - Name: "my-credential", - Owner: "tlm", - Cloud: "AWS", - }, - Name: "test-model", - Owner: userUUID, - UUID: modelUUID, - Type: coremodel.CAAS, + args := model.ModelCreationArgs{ + AgentVersion: jujuversion.Current, + Cloud: "AWS", + CloudRegion: "region1", + Credential: credential.ID{ + Name: "my-credential", + Owner: "tlm", + Cloud: "AWS", }, - ).Return(modelUUID, nil) + Name: "test-model", + Owner: userUUID, + UUID: modelUUID, + Type: coremodel.CAAS, + } + i.modelService.EXPECT().CreateModel(gomock.Any(), args).Return(modelUUID, nil) + i.readOnlyModelService.EXPECT().CreateModel(gomock.Any(), args.AsReadOnly()).Return(nil) model := description.NewModel(description.ModelArgs{ Config: map[string]any{ @@ -156,11 +160,73 @@ func (i *importSuite) TestModelCreate(c *gc.C) { Name: "my-credential", }) - importOp := importOperation{ - userService: i.userService, - service: i.importService, + importOp := &importOperation{ + userService: i.userService, + modelService: i.modelService, + readOnlyModelService: i.readOnlyModelService, } - err = importOp.Execute(context.Background(), model) + coordinator := modelmigration.NewCoordinator(modelmigrationtesting.IgnoredSetupOperation(importOp)) + err = coordinator.Perform(context.Background(), modelmigration.NewScope(nil, nil), model) + c.Assert(err, jc.ErrorIsNil) +} + +func (i *importSuite) TestModelCreateRollbacksOnFailure(c *gc.C) { + modelUUID := modeltesting.GenModelUUID(c) + userUUID, err := coreuser.NewUUID() c.Assert(err, jc.ErrorIsNil) + + defer i.setupMocks(c).Finish() + i.userService.EXPECT().GetUserByName(gomock.Any(), "tlm").Return( + coreuser.User{ + UUID: userUUID, + }, + nil, + ) + + args := model.ModelCreationArgs{ + AgentVersion: jujuversion.Current, + Cloud: "AWS", + CloudRegion: "region1", + Credential: credential.ID{ + Name: "my-credential", + Owner: "tlm", + Cloud: "AWS", + }, + Name: "test-model", + Owner: userUUID, + UUID: modelUUID, + Type: coremodel.CAAS, + } + i.modelService.EXPECT().CreateModel(gomock.Any(), args).Return(modelUUID, nil) + i.readOnlyModelService.EXPECT().CreateModel(gomock.Any(), args.AsReadOnly()).Return(errors.New("boom")) + i.modelService.EXPECT().DeleteModel(gomock.Any(), modelUUID).Return(nil) + + model := description.NewModel(description.ModelArgs{ + Config: map[string]any{ + config.NameKey: "test-model", + config.UUIDKey: modelUUID.String(), + }, + Cloud: "AWS", + CloudRegion: "region1", + Owner: names.NewUserTag("tlm"), + LatestToolsVersion: jujuversion.Current, + Type: coremodel.CAAS.String(), + }) + + model.SetCloudCredential(description.CloudCredentialArgs{ + Owner: names.NewUserTag("tlm"), + Cloud: names.NewCloudTag("AWS"), + Name: "my-credential", + }) + + importOp := &importOperation{ + userService: i.userService, + modelService: i.modelService, + readOnlyModelService: i.readOnlyModelService, + } + + coordinator := modelmigration.NewCoordinator(modelmigrationtesting.IgnoredSetupOperation(importOp)) + err = coordinator.Perform(context.Background(), modelmigration.NewScope(nil, nil), model) + c.Assert(err, gc.ErrorMatches, `.*boom.*`) } diff --git a/domain/model/modelmigration/migrations_mock_test.go b/domain/model/modelmigration/migrations_mock_test.go index f7c64dfce24..748a1f638d9 100644 --- a/domain/model/modelmigration/migrations_mock_test.go +++ b/domain/model/modelmigration/migrations_mock_test.go @@ -1,9 +1,9 @@ // Code generated by MockGen. DO NOT EDIT. -// Source: github.com/juju/juju/domain/model/modelmigration (interfaces: ImportService,UserService) +// Source: github.com/juju/juju/domain/model/modelmigration (interfaces: ModelService,ReadOnlyModelService,UserService) // // Generated by this command: // -// mockgen -package modelmigration -destination migrations_mock_test.go github.com/juju/juju/domain/model/modelmigration ImportService,UserService +// mockgen -package modelmigration -destination migrations_mock_test.go github.com/juju/juju/domain/model/modelmigration ModelService,ReadOnlyModelService,UserService // // Package modelmigration is a generated GoMock package. @@ -19,31 +19,31 @@ import ( gomock "go.uber.org/mock/gomock" ) -// MockImportService is a mock of ImportService interface. -type MockImportService struct { +// MockModelService is a mock of ModelService interface. +type MockModelService struct { ctrl *gomock.Controller - recorder *MockImportServiceMockRecorder + recorder *MockModelServiceMockRecorder } -// MockImportServiceMockRecorder is the mock recorder for MockImportService. -type MockImportServiceMockRecorder struct { - mock *MockImportService +// MockModelServiceMockRecorder is the mock recorder for MockModelService. +type MockModelServiceMockRecorder struct { + mock *MockModelService } -// NewMockImportService creates a new mock instance. -func NewMockImportService(ctrl *gomock.Controller) *MockImportService { - mock := &MockImportService{ctrl: ctrl} - mock.recorder = &MockImportServiceMockRecorder{mock} +// NewMockModelService creates a new mock instance. +func NewMockModelService(ctrl *gomock.Controller) *MockModelService { + mock := &MockModelService{ctrl: ctrl} + mock.recorder = &MockModelServiceMockRecorder{mock} return mock } // EXPECT returns an object that allows the caller to indicate expected use. -func (m *MockImportService) EXPECT() *MockImportServiceMockRecorder { +func (m *MockModelService) EXPECT() *MockModelServiceMockRecorder { return m.recorder } // CreateModel mocks base method. -func (m *MockImportService) CreateModel(arg0 context.Context, arg1 model0.ModelCreationArgs) (model.UUID, error) { +func (m *MockModelService) CreateModel(arg0 context.Context, arg1 model0.ModelCreationArgs) (model.UUID, error) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "CreateModel", arg0, arg1) ret0, _ := ret[0].(model.UUID) @@ -52,9 +52,60 @@ func (m *MockImportService) CreateModel(arg0 context.Context, arg1 model0.ModelC } // CreateModel indicates an expected call of CreateModel. -func (mr *MockImportServiceMockRecorder) CreateModel(arg0, arg1 any) *gomock.Call { +func (mr *MockModelServiceMockRecorder) CreateModel(arg0, arg1 any) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CreateModel", reflect.TypeOf((*MockImportService)(nil).CreateModel), arg0, arg1) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CreateModel", reflect.TypeOf((*MockModelService)(nil).CreateModel), arg0, arg1) +} + +// DeleteModel mocks base method. +func (m *MockModelService) DeleteModel(arg0 context.Context, arg1 model.UUID) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "DeleteModel", arg0, arg1) + ret0, _ := ret[0].(error) + return ret0 +} + +// DeleteModel indicates an expected call of DeleteModel. +func (mr *MockModelServiceMockRecorder) DeleteModel(arg0, arg1 any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeleteModel", reflect.TypeOf((*MockModelService)(nil).DeleteModel), arg0, arg1) +} + +// MockReadOnlyModelService is a mock of ReadOnlyModelService interface. +type MockReadOnlyModelService struct { + ctrl *gomock.Controller + recorder *MockReadOnlyModelServiceMockRecorder +} + +// MockReadOnlyModelServiceMockRecorder is the mock recorder for MockReadOnlyModelService. +type MockReadOnlyModelServiceMockRecorder struct { + mock *MockReadOnlyModelService +} + +// NewMockReadOnlyModelService creates a new mock instance. +func NewMockReadOnlyModelService(ctrl *gomock.Controller) *MockReadOnlyModelService { + mock := &MockReadOnlyModelService{ctrl: ctrl} + mock.recorder = &MockReadOnlyModelServiceMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use. +func (m *MockReadOnlyModelService) EXPECT() *MockReadOnlyModelServiceMockRecorder { + return m.recorder +} + +// CreateModel mocks base method. +func (m *MockReadOnlyModelService) CreateModel(arg0 context.Context, arg1 model0.ReadOnlyModelCreationArgs) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "CreateModel", arg0, arg1) + ret0, _ := ret[0].(error) + return ret0 +} + +// CreateModel indicates an expected call of CreateModel. +func (mr *MockReadOnlyModelServiceMockRecorder) CreateModel(arg0, arg1 any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CreateModel", reflect.TypeOf((*MockReadOnlyModelService)(nil).CreateModel), arg0, arg1) } // MockUserService is a mock of UserService interface. diff --git a/domain/model/modelmigration/package_test.go b/domain/model/modelmigration/package_test.go index 3a1a04c3b8b..48d673a6a91 100644 --- a/domain/model/modelmigration/package_test.go +++ b/domain/model/modelmigration/package_test.go @@ -9,7 +9,7 @@ import ( gc "gopkg.in/check.v1" ) -//go:generate go run go.uber.org/mock/mockgen -package modelmigration -destination migrations_mock_test.go github.com/juju/juju/domain/model/modelmigration ImportService,UserService +//go:generate go run go.uber.org/mock/mockgen -package modelmigration -destination migrations_mock_test.go github.com/juju/juju/domain/model/modelmigration ModelService,ReadOnlyModelService,UserService func TestPackage(t *testing.T) { gc.TestingT(t) diff --git a/domain/model/service/modelservice.go b/domain/model/service/modelservice.go new file mode 100644 index 00000000000..a3dc656d9d4 --- /dev/null +++ b/domain/model/service/modelservice.go @@ -0,0 +1,46 @@ +// Copyright 2024 Canonical Ltd. +// Licensed under the AGPLv3, see LICENCE file for details. + +package service + +import ( + "context" + + "github.com/juju/juju/domain/model" +) + +// ModelState is the model state required by this service. This is the model +// database state, not the controller state. +type ModelState interface { + // Create creates a new model with all of its associated metadata. + Create(context.Context, model.ReadOnlyModelCreationArgs) error +} + +// ModelService defines a service for interacting with the underlying model +// state, as opposed to the controller state. +type ModelService struct { + st ModelState +} + +// NewModelService returns a new Service for interacting with a models state. +func NewModelService(st ModelState) *ModelService { + return &ModelService{ + st: st, + } +} + +// CreateModel is responsible for creating a new model within the model +// database. +// +// The following error types can be expected to be returned: +// - [modelerrors.AlreadyExists]: When the model uuid is already in use. +func (s *ModelService) CreateModel( + ctx context.Context, + args model.ReadOnlyModelCreationArgs, +) error { + if err := args.Validate(); err != nil { + return err + } + + return s.st.Create(ctx, args) +} diff --git a/domain/model/service/modelservice_test.go b/domain/model/service/modelservice_test.go new file mode 100644 index 00000000000..d01fb885bb5 --- /dev/null +++ b/domain/model/service/modelservice_test.go @@ -0,0 +1,58 @@ +// Copyright 2024 Canonical Ltd. +// Licensed under the AGPLv3, see LICENCE file for details. + +package service + +import ( + "context" + + "github.com/juju/testing" + jc "github.com/juju/testing/checkers" + gc "gopkg.in/check.v1" + + coremodel "github.com/juju/juju/core/model" + modeltesting "github.com/juju/juju/core/model/testing" + "github.com/juju/juju/domain/model" +) + +type dummyModelState struct { + models map[coremodel.UUID]model.ReadOnlyModelCreationArgs +} + +func (d *dummyModelState) Create(ctx context.Context, args model.ReadOnlyModelCreationArgs) error { + d.models[args.UUID] = args + return nil +} + +type modelServiceSuite struct { + testing.IsolationSuite + + state *dummyModelState +} + +var _ = gc.Suite(&modelServiceSuite{}) + +func (s *modelServiceSuite) SetUpTest(c *gc.C) { + s.state = &dummyModelState{ + models: map[coremodel.UUID]model.ReadOnlyModelCreationArgs{}, + } +} + +func (s *modelServiceSuite) TestModelCreation(c *gc.C) { + svc := NewModelService(s.state) + + id := modeltesting.GenModelUUID(c) + args := model.ReadOnlyModelCreationArgs{ + UUID: id, + Name: "my-awesome-model", + Cloud: "aws", + CloudRegion: "myregion", + Type: coremodel.IAAS, + } + err := svc.CreateModel(context.Background(), args) + c.Assert(err, jc.ErrorIsNil) + + got, exists := s.state.models[id] + c.Assert(exists, jc.IsTrue) + c.Check(got, gc.Equals, args) +} diff --git a/domain/model/service/service.go b/domain/model/service/service.go index 325be43a106..c1b49ec41cd 100644 --- a/domain/model/service/service.go +++ b/domain/model/service/service.go @@ -92,17 +92,17 @@ func agentVersionSelector() version.Number { // If the ModelCreationArgs do not have a credential name set then no cloud // credential will be associated with the model. // -// If the caller has not prescribed a spefici agent version to use for the model -// the current controllers supported agent version will be used. - +// If the caller has not prescribed a specific agent version to use for the +// model the current controllers supported agent version will be used. +// // The following error types can be expected to be returned: // - [modelerrors.AlreadyExists]: When the model uuid is already in use or a model // with the same name and owner already exists. // - [errors.NotFound]: When the cloud, cloud region, or credential do not exist. // - [github.com/juju/juju/domain/user/errors.NotFound]: When the owner of the +// model can not be found. // - [modelerrors.AgentVersionNotSupported]: When the prescribed agent version // cannot be used with this controller. -// mode cannot be found. func (s *Service) CreateModel( ctx context.Context, args model.ModelCreationArgs, diff --git a/domain/model/service/service_test.go b/domain/model/service/service_test.go index 51ad21efa1c..2ca923097a4 100644 --- a/domain/model/service/service_test.go +++ b/domain/model/service/service_test.go @@ -12,7 +12,7 @@ import ( "github.com/juju/testing" jc "github.com/juju/testing/checkers" "github.com/juju/version/v2" - . "gopkg.in/check.v1" + gc "gopkg.in/check.v1" "github.com/juju/juju/core/credential" coremodel "github.com/juju/juju/core/model" @@ -43,7 +43,7 @@ type serviceSuite struct { state *dummyState } -var _ = Suite(&serviceSuite{}) +var _ = gc.Suite(&serviceSuite{}) func (d *dummyState) Create( _ context.Context, @@ -157,7 +157,7 @@ func (d *dummyState) UpdateCredential( return nil } -func (s *serviceSuite) SetUpTest(c *C) { +func (s *serviceSuite) SetUpTest(c *gc.C) { s.modelUUID = modeltesting.GenModelUUID(c) var err error s.userUUID, err = user.NewUUID() @@ -169,13 +169,13 @@ func (s *serviceSuite) SetUpTest(c *C) { } } -func (s *serviceSuite) TestCreateModelInvalidArgs(c *C) { +func (s *serviceSuite) TestCreateModelInvalidArgs(c *gc.C) { svc := NewService(s.state, DefaultAgentBinaryFinder()) _, err := svc.CreateModel(context.Background(), model.ModelCreationArgs{}) c.Assert(err, jc.ErrorIs, errors.NotValid) } -func (s *serviceSuite) TestModelCreation(c *C) { +func (s *serviceSuite) TestModelCreation(c *gc.C) { cred := credential.ID{ Cloud: "aws", Name: "foobar", @@ -205,16 +205,16 @@ func (s *serviceSuite) TestModelCreation(c *C) { // Test that because we have not specified an agent version that the current // controller version is chosen. - c.Check(args.AgentVersion, Equals, jujuversion.Current) + c.Check(args.AgentVersion, gc.Equals, jujuversion.Current) modelList, err := svc.ModelList(context.Background()) c.Assert(err, jc.ErrorIsNil) - c.Check(modelList, DeepEquals, []coremodel.UUID{ + c.Check(modelList, gc.DeepEquals, []coremodel.UUID{ id, }) } -func (s *serviceSuite) TestModelCreationInvalidCloud(c *C) { +func (s *serviceSuite) TestModelCreationInvalidCloud(c *gc.C) { svc := NewService(s.state, DefaultAgentBinaryFinder()) _, err := svc.CreateModel(context.Background(), model.ModelCreationArgs{ Cloud: "aws", @@ -227,7 +227,7 @@ func (s *serviceSuite) TestModelCreationInvalidCloud(c *C) { c.Assert(err, jc.ErrorIs, errors.NotFound) } -func (s *serviceSuite) TestModelCreationNoCloudRegion(c *C) { +func (s *serviceSuite) TestModelCreationNoCloudRegion(c *gc.C) { s.state.clouds["aws"] = dummyStateCloud{ Regions: []string{"myregion"}, } @@ -246,7 +246,7 @@ func (s *serviceSuite) TestModelCreationNoCloudRegion(c *C) { // TestModelCreationOwnerNotFound is testing that if we make a model with an // owner that doesn't exist we get back a [usererrors.NotFound] error. -func (s *serviceSuite) TestModelCreationOwnerNotFound(c *C) { +func (s *serviceSuite) TestModelCreationOwnerNotFound(c *gc.C) { s.state.clouds["aws"] = dummyStateCloud{ Credentials: map[string]credential.ID{}, Regions: []string{"myregion"}, @@ -267,7 +267,7 @@ func (s *serviceSuite) TestModelCreationOwnerNotFound(c *C) { c.Assert(err, jc.ErrorIs, usererrors.NotFound) } -func (s *serviceSuite) TestModelCreationNoCloudCredential(c *C) { +func (s *serviceSuite) TestModelCreationNoCloudCredential(c *gc.C) { s.state.clouds["aws"] = dummyStateCloud{ Credentials: map[string]credential.ID{}, Regions: []string{"myregion"}, @@ -290,7 +290,7 @@ func (s *serviceSuite) TestModelCreationNoCloudCredential(c *C) { c.Assert(err, jc.ErrorIs, errors.NotFound) } -func (s *serviceSuite) TestModelCreationNameOwnerConflict(c *C) { +func (s *serviceSuite) TestModelCreationNameOwnerConflict(c *gc.C) { s.state.clouds["aws"] = dummyStateCloud{ Credentials: map[string]credential.ID{}, Regions: []string{"myregion"}, @@ -318,7 +318,7 @@ func (s *serviceSuite) TestModelCreationNameOwnerConflict(c *C) { c.Assert(err, jc.ErrorIs, modelerrors.AlreadyExists) } -func (s *serviceSuite) TestUpdateModelCredentialForInvalidModel(c *C) { +func (s *serviceSuite) TestUpdateModelCredentialForInvalidModel(c *gc.C) { id := modeltesting.GenModelUUID(c) svc := NewService(s.state, DefaultAgentBinaryFinder()) @@ -330,7 +330,7 @@ func (s *serviceSuite) TestUpdateModelCredentialForInvalidModel(c *C) { c.Assert(err, jc.ErrorIs, modelerrors.NotFound) } -func (s *serviceSuite) TestUpdateModelCredential(c *C) { +func (s *serviceSuite) TestUpdateModelCredential(c *gc.C) { cred := credential.ID{ Cloud: "aws", Owner: s.userUUID.String(), @@ -359,7 +359,7 @@ func (s *serviceSuite) TestUpdateModelCredential(c *C) { c.Assert(err, jc.ErrorIsNil) } -func (s *serviceSuite) TestUpdateModelCredentialReplace(c *C) { +func (s *serviceSuite) TestUpdateModelCredentialReplace(c *gc.C) { cred := credential.ID{ Cloud: "aws", Owner: s.userUUID.String(), @@ -395,7 +395,7 @@ func (s *serviceSuite) TestUpdateModelCredentialReplace(c *C) { c.Assert(err, jc.ErrorIsNil) } -func (s *serviceSuite) TestUpdateModelCredentialZeroValue(c *C) { +func (s *serviceSuite) TestUpdateModelCredentialZeroValue(c *gc.C) { cred := credential.ID{ Cloud: "aws", Owner: s.userUUID.String(), @@ -424,7 +424,7 @@ func (s *serviceSuite) TestUpdateModelCredentialZeroValue(c *C) { c.Assert(err, jc.ErrorIs, errors.NotValid) } -func (s *serviceSuite) TestUpdateModelCredentialDifferentCloud(c *C) { +func (s *serviceSuite) TestUpdateModelCredentialDifferentCloud(c *gc.C) { cred := credential.ID{ Cloud: "aws", Owner: s.userUUID.String(), @@ -465,7 +465,7 @@ func (s *serviceSuite) TestUpdateModelCredentialDifferentCloud(c *C) { c.Assert(err, jc.ErrorIs, errors.NotValid) } -func (s *serviceSuite) TestUpdateModelCredentialNotFound(c *C) { +func (s *serviceSuite) TestUpdateModelCredentialNotFound(c *gc.C) { cred := credential.ID{ Cloud: "aws", Owner: s.userUUID.String(), @@ -500,7 +500,7 @@ func (s *serviceSuite) TestUpdateModelCredentialNotFound(c *C) { c.Assert(err, jc.ErrorIs, errors.NotFound) } -func (s *serviceSuite) TestDeleteModel(c *C) { +func (s *serviceSuite) TestDeleteModel(c *gc.C) { cred := credential.ID{ Cloud: "aws", Name: "foobar", @@ -533,7 +533,7 @@ func (s *serviceSuite) TestDeleteModel(c *C) { c.Assert(exists, jc.IsFalse) } -func (s *serviceSuite) TestDeleteModelNotFound(c *C) { +func (s *serviceSuite) TestDeleteModelNotFound(c *gc.C) { svc := NewService(s.state, DefaultAgentBinaryFinder()) err := svc.DeleteModel(context.Background(), s.modelUUID) c.Assert(err, jc.ErrorIs, modelerrors.NotFound) @@ -542,7 +542,7 @@ func (s *serviceSuite) TestDeleteModelNotFound(c *C) { // TestAgentVersionUnsupportedGreater is asserting that if we try and create a // model with an agent version that is greater then that of the controller the // operation fails with a [modelerrors.AgentVersionNotSupported] error. -func (s *serviceSuite) TestAgentVersionUnsupportedGreater(c *C) { +func (s *serviceSuite) TestAgentVersionUnsupportedGreater(c *gc.C) { cred := credential.ID{ Cloud: "aws", Name: "foobar", @@ -579,7 +579,7 @@ func (s *serviceSuite) TestAgentVersionUnsupportedGreater(c *C) { // model with an agent version that is less then that of the controller the // operation fails with a [modelerrors.AgentVersionNotSupported] error. This // fails because find tools will report [errors.NotFound]. -func (s *serviceSuite) TestAgentVersionUnsupportedLess(c *C) { +func (s *serviceSuite) TestAgentVersionUnsupportedLess(c *gc.C) { cred := credential.ID{ Cloud: "aws", Name: "foobar", diff --git a/domain/model/state/modelstate.go b/domain/model/state/modelstate.go new file mode 100644 index 00000000000..08c49375535 --- /dev/null +++ b/domain/model/state/modelstate.go @@ -0,0 +1,80 @@ +// Copyright 2024 Canonical Ltd. +// Licensed under the AGPLv3, see LICENCE file for details. + +package state + +import ( + "context" + "database/sql" + "fmt" + + "github.com/juju/errors" + + "github.com/juju/juju/core/database" + "github.com/juju/juju/domain" + "github.com/juju/juju/domain/model" + modelerrors "github.com/juju/juju/domain/model/errors" + internaldatabase "github.com/juju/juju/internal/database" +) + +// ModelState represents a type for interacting with the underlying model +// database state. +type ModelState struct { + *domain.StateBase +} + +// NewModelState returns a new State for interacting with the underlying model +// database state. +func NewModelState( + factory database.TxnRunnerFactory, +) *ModelState { + return &ModelState{ + StateBase: domain.NewStateBase(factory), + } +} + +// Create creates a new model with all of its associated metadata. +func (s *ModelState) Create(ctx context.Context, args model.ReadOnlyModelCreationArgs) error { + db, err := s.DB() + if err != nil { + return errors.Trace(err) + } + + return db.StdTxn(ctx, func(ctx context.Context, tx *sql.Tx) error { + return errors.Trace(CreateReadOnlyModel(ctx, args, tx)) + }) +} + +// CreateReadOnlyModel is responsible for creating a new model within the model +// database. +func CreateReadOnlyModel(ctx context.Context, args model.ReadOnlyModelCreationArgs, tx *sql.Tx) error { + stmt := ` +INSERT INTO model (uuid, name, type, cloud, cloud_region) + VALUES (?, ?, ?, ?, ?) + ON CONFLICT (uuid) DO NOTHING; +` + result, err := tx.ExecContext(ctx, stmt, args.UUID, args.Name, args.Type, args.Cloud, args.CloudRegion) + if err != nil { + // If the model already exists, return an error that the model already + // exists. + if internaldatabase.IsErrConstraintUnique(err) { + return fmt.Errorf("model %q already exists: %w%w", args.UUID, modelerrors.AlreadyExists, errors.Hide(err)) + } + // If the model already exists and we try and update it, the trigger + // should catch it and return an error. + if internaldatabase.IsErrConstraintTrigger(err) { + return fmt.Errorf("can not update model: %w%w", modelerrors.AlreadyExists, errors.Hide(err)) + } + return fmt.Errorf("creating model %q: %w", args.UUID, err) + } + + // Double check that it was actually created. + affected, err := result.RowsAffected() + if err != nil { + return fmt.Errorf("creating model %q: %w", args.UUID, err) + } + if affected != 1 { + return modelerrors.AlreadyExists + } + return nil +} diff --git a/domain/model/state/modelstate_test.go b/domain/model/state/modelstate_test.go new file mode 100644 index 00000000000..e4db89012ea --- /dev/null +++ b/domain/model/state/modelstate_test.go @@ -0,0 +1,135 @@ +// Copyright 2023 Canonical Ltd. +// Licensed under the AGPLv3, see LICENCE file for details. + +package state + +import ( + "context" + + jc "github.com/juju/testing/checkers" + gc "gopkg.in/check.v1" + + coremodel "github.com/juju/juju/core/model" + modeltesting "github.com/juju/juju/core/model/testing" + "github.com/juju/juju/domain/model" + modelerrors "github.com/juju/juju/domain/model/errors" + schematesting "github.com/juju/juju/domain/schema/testing" +) + +type modelSuite struct { + schematesting.ModelSuite +} + +var _ = gc.Suite(&modelSuite{}) + +func (s *modelSuite) TestCreateModel(c *gc.C) { + runner := s.TxnRunnerFactory() + state := NewModelState(runner) + + id := modeltesting.GenModelUUID(c) + args := model.ReadOnlyModelCreationArgs{ + UUID: id, + Name: "my-awesome-model", + Type: coremodel.IAAS, + Cloud: "aws", + CloudRegion: "myregion", + } + err := state.Create(context.Background(), args) + c.Assert(err, jc.ErrorIsNil) + + db := s.DB() + row := db.QueryRowContext(context.Background(), "SELECT uuid, name, type, cloud, cloud_region FROM model WHERE uuid = $1", id) + + var got model.ReadOnlyModelCreationArgs + err = row.Scan(&got.UUID, &got.Name, &got.Type, &got.Cloud, &got.CloudRegion) + c.Assert(err, jc.ErrorIsNil) + + c.Check(got, jc.DeepEquals, args) +} + +func (s *modelSuite) TestCreateModelMultipleTimesWithSameUUID(c *gc.C) { + runner := s.TxnRunnerFactory() + state := NewModelState(runner) + + // Ensure that we can't create the same model twice. + + id := modeltesting.GenModelUUID(c) + args := model.ReadOnlyModelCreationArgs{ + UUID: id, + Name: "my-awesome-model", + Type: coremodel.IAAS, + Cloud: "aws", + CloudRegion: "myregion", + } + err := state.Create(context.Background(), args) + c.Assert(err, jc.ErrorIsNil) + err = state.Create(context.Background(), args) + c.Assert(err, jc.ErrorIs, modelerrors.AlreadyExists) +} + +func (s *modelSuite) TestCreateModelMultipleTimesWithDifferentUUID(c *gc.C) { + runner := s.TxnRunnerFactory() + state := NewModelState(runner) + + // Ensure that you can only ever insert one model. + + err := state.Create(context.Background(), model.ReadOnlyModelCreationArgs{ + UUID: modeltesting.GenModelUUID(c), + Name: "my-awesome-model", + Type: coremodel.IAAS, + Cloud: "aws", + CloudRegion: "myregion", + }) + c.Assert(err, jc.ErrorIsNil) + + err = state.Create(context.Background(), model.ReadOnlyModelCreationArgs{ + UUID: modeltesting.GenModelUUID(c), + Name: "my-awesome-model", + Type: coremodel.IAAS, + Cloud: "aws", + CloudRegion: "myregion", + }) + c.Assert(err, jc.ErrorIs, modelerrors.AlreadyExists) +} + +func (s *modelSuite) TestCreateModelAndUpdate(c *gc.C) { + runner := s.TxnRunnerFactory() + state := NewModelState(runner) + + // Ensure that you can't update it. + + id := modeltesting.GenModelUUID(c) + err := state.Create(context.Background(), model.ReadOnlyModelCreationArgs{ + UUID: id, + Name: "my-awesome-model", + Type: coremodel.IAAS, + Cloud: "aws", + CloudRegion: "myregion", + }) + c.Assert(err, jc.ErrorIsNil) + + db := s.DB() + _, err = db.ExecContext(context.Background(), "UPDATE model SET name = 'new-name' WHERE uuid = $1", id) + c.Assert(err, gc.ErrorMatches, `model table is read-only`) +} + +func (s *modelSuite) TestCreateModelAndDelete(c *gc.C) { + runner := s.TxnRunnerFactory() + state := NewModelState(runner) + + // Ensure that you can't update it. + + id := modeltesting.GenModelUUID(c) + err := state.Create(context.Background(), model.ReadOnlyModelCreationArgs{ + UUID: id, + Name: "my-awesome-model", + Type: coremodel.IAAS, + Cloud: "aws", + CloudRegion: "myregion", + }) + c.Assert(err, jc.ErrorIsNil) + + db := s.DB() + _, err = db.ExecContext(context.Background(), "DELETE FROM model WHERE uuid = $1", id) + c.Assert(err, gc.ErrorMatches, `model table is immutable`) +} diff --git a/domain/model/state/state_test.go b/domain/model/state/state_test.go index 479aa041038..b29c0157176 100644 --- a/domain/model/state/state_test.go +++ b/domain/model/state/state_test.go @@ -28,16 +28,17 @@ import ( "github.com/juju/juju/version" ) -type modelSuite struct { +type stateSuite struct { schematesting.ControllerSuite + uuid coremodel.UUID userUUID user.UUID userName string } -var _ = gc.Suite(&modelSuite{}) +var _ = gc.Suite(&stateSuite{}) -func (m *modelSuite) SetUpTest(c *gc.C) { +func (m *stateSuite) SetUpTest(c *gc.C) { m.ControllerSuite.SetUpTest(c) // We need to generate a user in the database so that we can set the model @@ -111,7 +112,7 @@ func (m *modelSuite) SetUpTest(c *gc.C) { c.Assert(err, jc.ErrorIsNil) } -func (m *modelSuite) TestGetModel(c *gc.C) { +func (m *stateSuite) TestGetModel(c *gc.C) { runner := m.TxnRunnerFactory() modelSt := NewState(runner) @@ -131,7 +132,7 @@ func (m *modelSuite) TestGetModel(c *gc.C) { }) } -func (m *modelSuite) TestGetModelNotFound(c *gc.C) { +func (m *stateSuite) TestGetModelNotFound(c *gc.C) { runner := m.TxnRunnerFactory() modelSt := NewState(runner) @@ -143,7 +144,7 @@ func (m *modelSuite) TestGetModelNotFound(c *gc.C) { // TestCreateModelAgentWithNoModel is asserting that if we attempt to make a // model agent record where no model already exists that we get back a // [modelerrors.NotFound] error. -func (m *modelSuite) TestCreateModelAgentWithNoModel(c *gc.C) { +func (m *stateSuite) TestCreateModelAgentWithNoModel(c *gc.C) { runner, err := m.TxnRunnerFactory()() c.Assert(err, jc.ErrorIsNil) @@ -158,7 +159,7 @@ func (m *modelSuite) TestCreateModelAgentWithNoModel(c *gc.C) { // TestCreateModelAgentAlreadyExists is asserting that if we attempt to make a // model agent record when one already exists we get a // [modelerrors.AlreadyExists] back. -func (m *modelSuite) TestCreateModelAgentAlreadyExists(c *gc.C) { +func (m *stateSuite) TestCreateModelAgentAlreadyExists(c *gc.C) { runner, err := m.TxnRunnerFactory()() c.Assert(err, jc.ErrorIsNil) @@ -169,7 +170,7 @@ func (m *modelSuite) TestCreateModelAgentAlreadyExists(c *gc.C) { c.Assert(err, jc.ErrorIs, modelerrors.AlreadyExists) } -func (m *modelSuite) TestCreateModelMetadataWithNoModel(c *gc.C) { +func (m *stateSuite) TestCreateModelMetadataWithNoModel(c *gc.C) { runner, err := m.TxnRunnerFactory()() c.Assert(err, jc.ErrorIsNil) @@ -191,7 +192,7 @@ func (m *modelSuite) TestCreateModelMetadataWithNoModel(c *gc.C) { c.Assert(err, jc.ErrorIs, modelerrors.NotFound) } -func (m *modelSuite) TestCreateModelMetadataWithExistingMetadata(c *gc.C) { +func (m *stateSuite) TestCreateModelMetadataWithExistingMetadata(c *gc.C) { runner, err := m.TxnRunnerFactory()() c.Assert(err, jc.ErrorIsNil) @@ -212,7 +213,7 @@ func (m *modelSuite) TestCreateModelMetadataWithExistingMetadata(c *gc.C) { c.Assert(err, jc.ErrorIs, modelerrors.AlreadyExists) } -func (m *modelSuite) TestCreateModelWithSameNameAndOwner(c *gc.C) { +func (m *stateSuite) TestCreateModelWithSameNameAndOwner(c *gc.C) { modelSt := NewState(m.TxnRunnerFactory()) testUUID := modeltesting.GenModelUUID(c) err := modelSt.Create( @@ -229,7 +230,7 @@ func (m *modelSuite) TestCreateModelWithSameNameAndOwner(c *gc.C) { c.Assert(err, jc.ErrorIs, modelerrors.AlreadyExists) } -func (m *modelSuite) TestCreateModelWithInvalidCloudRegion(c *gc.C) { +func (m *stateSuite) TestCreateModelWithInvalidCloudRegion(c *gc.C) { modelSt := NewState(m.TxnRunnerFactory()) testUUID := modeltesting.GenModelUUID(c) err := modelSt.Create( @@ -249,7 +250,7 @@ func (m *modelSuite) TestCreateModelWithInvalidCloudRegion(c *gc.C) { // TestCreateModelWithNonExistentOwner is here to assert that if we try and make // a model with a user/owner that does not exist a [usererrors.NotFound] error // is returned. -func (m *modelSuite) TestCreateModelWithNonExistentOwner(c *gc.C) { +func (m *stateSuite) TestCreateModelWithNonExistentOwner(c *gc.C) { modelSt := NewState(m.TxnRunnerFactory()) testUUID := modeltesting.GenModelUUID(c) err := modelSt.Create( @@ -269,7 +270,7 @@ func (m *modelSuite) TestCreateModelWithNonExistentOwner(c *gc.C) { // TestCreateModelWithRemovedOwner is here to test that if we try and create a // new model with an owner that has been removed from the Juju user base that // the operation fails with a [usererrors.NotFound] error. -func (m *modelSuite) TestCreateModelWithRemovedOwner(c *gc.C) { +func (m *stateSuite) TestCreateModelWithRemovedOwner(c *gc.C) { userState := userstate.NewState(m.TxnRunnerFactory()) err := userState.RemoveUser(context.Background(), m.userName) c.Assert(err, jc.ErrorIsNil) @@ -290,7 +291,7 @@ func (m *modelSuite) TestCreateModelWithRemovedOwner(c *gc.C) { c.Assert(err, jc.ErrorIs, usererrors.NotFound) } -func (m *modelSuite) TestCreateModelWithInvalidCloud(c *gc.C) { +func (m *stateSuite) TestCreateModelWithInvalidCloud(c *gc.C) { modelSt := NewState(m.TxnRunnerFactory()) testUUID := modeltesting.GenModelUUID(c) err := modelSt.Create( @@ -307,7 +308,7 @@ func (m *modelSuite) TestCreateModelWithInvalidCloud(c *gc.C) { c.Assert(err, jc.ErrorIs, errors.NotFound) } -func (m *modelSuite) TestUpdateCredentialForDifferentCloud(c *gc.C) { +func (m *stateSuite) TestUpdateCredentialForDifferentCloud(c *gc.C) { cloudSt := dbcloud.NewState(m.TxnRunnerFactory()) err := cloudSt.UpsertCloud(context.Background(), cloud.Cloud{ Name: "my-cloud2", @@ -358,7 +359,7 @@ func (m *modelSuite) TestUpdateCredentialForDifferentCloud(c *gc.C) { // for the same cloud as the model when no cloud region has been set. This is a // regression test discovered during DQlite development where we messed up the // logic assuming that a cloud region was always set for a model. -func (m *modelSuite) TestSetModelCloudCredentialWithoutRegion(c *gc.C) { +func (m *stateSuite) TestSetModelCloudCredentialWithoutRegion(c *gc.C) { cloudSt := dbcloud.NewState(m.TxnRunnerFactory()) err := cloudSt.UpsertCloud(context.Background(), cloud.Cloud{ Name: "minikube", @@ -411,7 +412,7 @@ func (m *modelSuite) TestSetModelCloudCredentialWithoutRegion(c *gc.C) { // TestDeleteModel tests that we can delete a model that is already created in // the system. We also confirm that list models returns no models after the // deletion. -func (m *modelSuite) TestDeleteModel(c *gc.C) { +func (m *stateSuite) TestDeleteModel(c *gc.C) { modelSt := NewState(m.TxnRunnerFactory()) err := modelSt.Delete( context.Background(), @@ -449,7 +450,7 @@ func (m *modelSuite) TestDeleteModel(c *gc.C) { c.Check(modelUUIDS, gc.HasLen, 0) } -func (m *modelSuite) TestDeleteModelNotFound(c *gc.C) { +func (m *stateSuite) TestDeleteModelNotFound(c *gc.C) { uuid := modeltesting.GenModelUUID(c) modelSt := NewState(m.TxnRunnerFactory()) err := modelSt.Delete(context.Background(), uuid) @@ -458,7 +459,7 @@ func (m *modelSuite) TestDeleteModelNotFound(c *gc.C) { // TestListModels is testing that once we have created several models calling // list returns all of the models created. -func (m *modelSuite) TestListModels(c *gc.C) { +func (m *stateSuite) TestListModels(c *gc.C) { uuid1 := modeltesting.GenModelUUID(c) modelSt := NewState(m.TxnRunnerFactory()) err := modelSt.Create( diff --git a/domain/model/types.go b/domain/model/types.go index 54e0fb772f0..0911c4ce19d 100644 --- a/domain/model/types.go +++ b/domain/model/types.go @@ -78,3 +78,59 @@ func (m ModelCreationArgs) Validate() error { } return nil } + +// AsReadOnly returns a ReadOnlyModelCreationArgs struct that is a copy of the +// ModelCreationArgs struct. This is useful when you want to create a model +// within the model database. +func (m ModelCreationArgs) AsReadOnly() ReadOnlyModelCreationArgs { + return ReadOnlyModelCreationArgs{ + UUID: m.UUID, + Type: m.Type, + Cloud: m.Cloud, + CloudRegion: m.CloudRegion, + Name: m.Name, + } +} + +// ReadOnlyModelCreationArgs is a struct that is used to create a model +// within the model database. This struct is used to create a model with all of +// its associated metadata. +type ReadOnlyModelCreationArgs struct { + // UUID represents the unique id for the model when being created. This + // value is optional and if omitted will be generated for the caller. Use + // this value when you are trying to import a model during model migration. + UUID coremodel.UUID + + // Type is the type of the model. + // Type must satisfy IsValid() for a valid struct. + Type coremodel.ModelType + + // Cloud is the name of the cloud to associate with the model. + // Must not be empty for a valid struct. + Cloud string + + // CloudRegion is the region that the model will use in the cloud. + CloudRegion string + + // Name is the name of the model. + // Must not be empty for a valid struct. + Name string +} + +// Validate is responsible for checking all of the fields of ModelCreationArgs +// are in a set state that is valid for use. +func (m ReadOnlyModelCreationArgs) Validate() error { + if m.Cloud == "" { + return fmt.Errorf("%w cloud cannot be empty", errors.NotValid) + } + if m.Name == "" { + return fmt.Errorf("%w name cannot be empty", errors.NotValid) + } + if !m.Type.IsValid() { + return fmt.Errorf("%w model type of %q", errors.NotSupported, m.Type) + } + if err := m.UUID.Validate(); err != nil { + return fmt.Errorf("uuid: %w", err) + } + return nil +} diff --git a/domain/schema/model.go b/domain/schema/model.go index d346e4f2f50..12350463a4b 100644 --- a/domain/schema/model.go +++ b/domain/schema/model.go @@ -35,6 +35,7 @@ func ModelDDL() *schema.Schema { lifeSchema, changeLogSchema, changeLogModelNamespaceSchema, + modelSchema, modelConfigSchema, objectStoreMetadataSchema, applicationSchema, @@ -158,6 +159,37 @@ INSERT INTO change_log_namespace VALUES `) } +func modelSchema() schema.Patch { + return schema.MakePatch(` +-- The model table represents a readonly denormalised model data. The intended +-- use is to provide a read-only view of the model data for the purpose of +-- accessing common model data without the need to span multiple databases. +CREATE TABLE model ( + uuid TEXT PRIMARY KEY, + name TEXT NOT NULL, + type TEXT NOT NULL, + cloud TEXT NOT NULL, + cloud_region TEXT NOT NULL +); + +-- A unique constraint over a constant index ensures only 1 entry matching the +-- condition can exist. +CREATE UNIQUE INDEX idx_singleton_model ON model ((1)); + +-- The model table is read-only, so we create a trigger to prevent updates. +CREATE TRIGGER trg_readonly_model_update +BEFORE UPDATE ON model +BEGIN + SELECT RAISE(abort, 'model table is read-only'); +END; +CREATE TRIGGER trg_readonly_model_delete +BEFORE DELETE ON model +BEGIN + SELECT RAISE(abort, 'model table is immutable'); +END; +`) +} + func modelConfigSchema() schema.Patch { return schema.MakePatch(` CREATE TABLE model_config ( diff --git a/domain/schema/schema_test.go b/domain/schema/schema_test.go index 5fbc791033e..88c8f94b778 100644 --- a/domain/schema/schema_test.go +++ b/domain/schema/schema_test.go @@ -211,6 +211,9 @@ func (s *schemaSuite) TestModelTables(c *gc.C) { "change_log_namespace", "change_log_witness", + // Model + "model", + // Model config "model_config", @@ -415,7 +418,15 @@ func (s *schemaSuite) TestModelChangeLogTriggers(c *gc.C) { "trg_log_storage_volume_update", "trg_log_storage_volume_delete", ) - c.Assert(readEntityNames(c, s.DB(), "trigger"), jc.SameContents, expected.SortedValues()) + + // These are additional triggers that are not change log triggers, but + // will be present in the schema. + additional := set.NewStrings( + "trg_readonly_model_delete", + "trg_readonly_model_update", + ) + + c.Assert(readEntityNames(c, s.DB(), "trigger"), jc.SameContents, expected.Union(additional).SortedValues()) } func (s *schemaSuite) assertChangeLogCount(c *gc.C, editType int, namespaceID tableNamespaceID, expectedCount int) { diff --git a/domain/schema/testing/modelsuite.go b/domain/schema/testing/modelsuite.go index 5ee8145ff7b..3b54fe421cb 100644 --- a/domain/schema/testing/modelsuite.go +++ b/domain/schema/testing/modelsuite.go @@ -6,6 +6,7 @@ package testing import ( gc "gopkg.in/check.v1" + coredatabase "github.com/juju/juju/core/database" "github.com/juju/juju/domain/schema" "github.com/juju/juju/internal/database/testing" "github.com/juju/juju/internal/uuid" @@ -19,6 +20,11 @@ type ModelSuite struct { modelUUID string } +// ModelTxnRunner returns a txn runner attached to the model database. +func (s *ModelSuite) ModelTxnRunner() coredatabase.TxnRunner { + return s.TxnRunner() +} + // SetUpTest is responsible for setting up a testing database suite initialised // with the model schema. func (s *ModelSuite) SetUpTest(c *gc.C) {