diff --git a/apiserver/facades/agent/caasapplication/application_test.go b/apiserver/facades/agent/caasapplication/application_test.go index ab32c57df4a..100731d4682 100644 --- a/apiserver/facades/agent/caasapplication/application_test.go +++ b/apiserver/facades/agent/caasapplication/application_test.go @@ -21,6 +21,7 @@ import ( "github.com/juju/juju/caas" controllerconfigbootstrap "github.com/juju/juju/domain/controllerconfig/bootstrap" "github.com/juju/juju/domain/servicefactory/testing" + "github.com/juju/juju/internal/storage/provider" "github.com/juju/juju/rpc/params" "github.com/juju/juju/state" coretesting "github.com/juju/juju/testing" @@ -63,7 +64,7 @@ func (s *CAASApplicationSuite) SetUpTest(c *gc.C) { s.authorizer, s.st, s.st, s.ControllerServiceFactory(c).ControllerConfig(), - s.DefaultModelServiceFactory(c).Application(), + s.DefaultModelServiceFactory(c).Application(provider.CommonStorageProviders()), s.broker, s.clock, loggo.GetLogger("juju.apiserver.caasaplication"), diff --git a/apiserver/facades/agent/caasapplication/mock_test.go b/apiserver/facades/agent/caasapplication/mock_test.go index 09ffff299d5..fb6dd79a13b 100644 --- a/apiserver/facades/agent/caasapplication/mock_test.go +++ b/apiserver/facades/agent/caasapplication/mock_test.go @@ -16,6 +16,7 @@ import ( "github.com/juju/juju/controller" jujucontroller "github.com/juju/juju/controller" "github.com/juju/juju/core/network" + "github.com/juju/juju/environs/config" "github.com/juju/juju/state" jtesting "github.com/juju/juju/testing" ) @@ -98,6 +99,11 @@ type mockModel struct { tag names.Tag } +func (st *mockModel) Config() (*config.Config, error) { + attr := jtesting.FakeConfig() + return config.New(config.UseDefaults, attr) +} + func (st *mockModel) Containers(providerIds ...string) ([]state.CloudContainer, error) { st.MethodCall(st, "Containers", providerIds) return st.containers, st.NextErr() diff --git a/apiserver/facades/agent/caasapplication/register.go b/apiserver/facades/agent/caasapplication/register.go index 5cd69c28f5b..1a7b57add77 100644 --- a/apiserver/facades/agent/caasapplication/register.go +++ b/apiserver/facades/agent/caasapplication/register.go @@ -41,13 +41,14 @@ func newStateFacade(ctx facade.ModelContext) (*Facade, error) { if err != nil { return nil, errors.Trace(err) } + registry := stateenvirons.NewStorageProviderRegistry(broker) return NewFacade( resources, authorizer, systemState, &stateShim{State: st}, serviceFactory.ControllerConfig(), - serviceFactory.Application(), + serviceFactory.Application(registry), broker, ctx.StatePool().Clock(), ctx.Logger().Child("caasapplication"), diff --git a/apiserver/facades/client/application/application.go b/apiserver/facades/client/application/application.go index 5c98875255d..ce1743b4788 100644 --- a/apiserver/facades/client/application/application.go +++ b/apiserver/facades/client/application/application.go @@ -44,6 +44,7 @@ import ( "github.com/juju/juju/core/permission" "github.com/juju/juju/core/secrets" "github.com/juju/juju/core/status" + applicationservice "github.com/juju/juju/domain/application/service" "github.com/juju/juju/environs" "github.com/juju/juju/environs/bootstrap" environsconfig "github.com/juju/juju/environs/config" @@ -125,21 +126,23 @@ func newFacadeBase(stdCtx context.Context, ctx facade.ModelContext) (*APIBase, e stateCharm := CharmToStateCharm serviceFactory := ctx.ServiceFactory() - var ( - storagePoolGetter StoragePoolGetter - registry storage.ProviderRegistry - caasBroker caas.Broker + registry, err := stateenvirons.NewStorageProviderRegistryForModel( + m, serviceFactory.Cloud(), serviceFactory.Credential(), + stateenvirons.GetNewEnvironFunc(environs.New), + stateenvirons.GetNewCAASBrokerFunc(caas.New), ) - modelType := model.Type() - if modelType == state.ModelTypeCAAS { + if err != nil { + return nil, errors.Annotate(err, "getting storage provider registry") + } + storagePoolGetter := serviceFactory.Storage(registry) + + var caasBroker caas.Broker + if model.Type() == state.ModelTypeCAAS { caasBroker, err = stateenvirons.GetNewCAASBrokerFunc(caas.New)(model, serviceFactory.Cloud(), serviceFactory.Credential()) if err != nil { return nil, errors.Annotate(err, "getting caas client") } - registry = stateenvirons.NewStorageProviderRegistry(caasBroker) - storagePoolGetter = serviceFactory.Storage(registry) } - resources := ctx.Resources() leadershipReader, err := ctx.LeadershipReader() @@ -181,7 +184,8 @@ func newFacadeBase(stdCtx context.Context, ctx facade.ModelContext) (*APIBase, e state: state, storagePoolGetter: storagePoolGetter, } - repoDeploy := NewDeployFromRepositoryAPI(state, serviceFactory.Application(), ctx.ObjectStore(), makeDeployFromRepositoryValidator(stdCtx, validatorCfg)) + applicationService := serviceFactory.Application(registry) + repoDeploy := NewDeployFromRepositoryAPI(state, applicationService, ctx.ObjectStore(), makeDeployFromRepositoryValidator(stdCtx, validatorCfg)) return NewAPIBase( state, @@ -195,7 +199,7 @@ func newFacadeBase(stdCtx context.Context, ctx facade.ModelContext) (*APIBase, e serviceFactory.Cloud(), serviceFactory.Credential(), serviceFactory.Machine(), - serviceFactory.Application(), + applicationService, leadershipReader, stateCharm, DeployApplication, @@ -1103,6 +1107,7 @@ func (api *APIBase) applicationSetCharm( logger.Warningf("proceeding with upgrade of application %q even though the charm feature requirements could not be met as --force was specified", params.AppName) } + // force := params.Force cfg := state.SetCharmConfig{ Charm: api.stateCharm(newCharm), @@ -1127,10 +1132,31 @@ func (api *APIBase) applicationSetCharm( return errors.New("cannot downgrade from v2 charm format to v1") } - // TODO(wallyworld) - do in a single transaction + // TODO(dqlite) - remove SetCharm (replaced below with UpdateApplicationCharm). if err := params.Application.SetCharm(cfg, api.store); err != nil { return errors.Annotate(err, "updating charm config") } + + var storageConstraints map[string]storage.Constraints + if len(params.StorageConstraints) > 0 { + storageConstraints = make(map[string]storage.Constraints) + for name, cons := range params.StorageConstraints { + sc := storage.Constraints{Pool: cons.Pool} + if cons.Size != nil { + sc.Size = *cons.Size + } + if cons.Count != nil { + sc.Count = *cons.Count + } + storageConstraints[name] = sc + } + } + if err := api.applicationService.UpdateApplicationCharm(ctx, params.AppName, applicationservice.UpdateCharmParams{ + Charm: newCharm, + Storage: storageConstraints, + }); err != nil { + return errors.Annotatef(err, "updating charm for application %q", params.AppName) + } if attr := appConfig.Attributes(); len(attr) > 0 { return params.Application.UpdateApplicationConfig(attr, nil, appSchema, appDefaults) } diff --git a/apiserver/facades/client/application/application_test.go b/apiserver/facades/client/application/application_test.go index a1c0a7daeac..9c6efac121e 100644 --- a/apiserver/facades/client/application/application_test.go +++ b/apiserver/facades/client/application/application_test.go @@ -103,7 +103,7 @@ func (s *applicationSuite) makeAPI(c *gc.C) *application.APIBase { serviceFactory.Cloud(), serviceFactory.Credential(), serviceFactory.Machine(), - serviceFactory.Application(), + serviceFactory.Application(registry), nil, // leadership not used in these tests. application.CharmToStateCharm, application.DeployApplication, diff --git a/apiserver/facades/client/application/application_unit_test.go b/apiserver/facades/client/application/application_unit_test.go index e0560119c69..3c0b747fe13 100644 --- a/apiserver/facades/client/application/application_unit_test.go +++ b/apiserver/facades/client/application/application_unit_test.go @@ -323,6 +323,11 @@ func (s *ApplicationSuite) TestSetCharm(c *gc.C) { app.EXPECT().SetCharm(setCharmConfigMatcher{c: c, expected: cfg}, s.store) s.backend.EXPECT().Application("postgresql").Return(app, nil) + s.applicationService.EXPECT().UpdateApplicationCharm(gomock.Any(), "postgresql", applicationservice.UpdateCharmParams{ + Charm: ch, + Storage: nil, + }).Return(nil) + err := s.api.SetCharm(context.Background(), params.ApplicationSetCharm{ ApplicationName: "postgresql", CharmURL: curl, @@ -351,6 +356,11 @@ func (s *ApplicationSuite) TestSetCharmEverything(c *gc.C) { app.EXPECT().UpdateApplicationConfig(coreconfig.ConfigAttributes{"trust": true}, nil, schemaFields, defaults) s.backend.EXPECT().Application("postgresql").Return(app, nil) + s.applicationService.EXPECT().UpdateApplicationCharm(gomock.Any(), "postgresql", applicationservice.UpdateCharmParams{ + Charm: ch, + Storage: nil, + }).Return(nil) + err = s.api.SetCharm(context.Background(), params.ApplicationSetCharm{ ApplicationName: "postgresql", CharmURL: curl, @@ -407,6 +417,12 @@ func (s *ApplicationSuite) TestSetCharmForceUnits(c *gc.C) { app.EXPECT().SetCharm(setCharmConfigMatcher{c: c, expected: cfg}, s.store) s.backend.EXPECT().Application("postgresql").Return(app, nil) + + s.applicationService.EXPECT().UpdateApplicationCharm(gomock.Any(), "postgresql", applicationservice.UpdateCharmParams{ + Charm: ch, + Storage: nil, + }).Return(nil) + err := s.api.SetCharm(context.Background(), params.ApplicationSetCharm{ ApplicationName: "postgresql", CharmURL: curl, @@ -459,6 +475,16 @@ func (s *ApplicationSuite) TestSetCharmStorageConstraints(c *gc.C) { s.backend.EXPECT().Application("postgresql").Return(app, nil) + s.applicationService.EXPECT().UpdateApplicationCharm(gomock.Any(), "postgresql", applicationservice.UpdateCharmParams{ + Charm: ch, + Storage: map[string]storage.Constraints{ + "a": {}, + "b": {Pool: "radiant"}, + "c": {Size: 123}, + "d": {Count: 456}, + }, + }).Return(nil) + toUint64Ptr := func(v uint64) *uint64 { return &v } @@ -533,6 +559,11 @@ func (s *ApplicationSuite) TestSetCharmConfigSettings(c *gc.C) { }, gomock.Any()).Return(nil) s.backend.EXPECT().Application("postgresql").Return(app, nil) + s.applicationService.EXPECT().UpdateApplicationCharm(gomock.Any(), "postgresql", applicationservice.UpdateCharmParams{ + Charm: ch, + Storage: nil, + }).Return(nil) + err := s.api.SetCharm(context.Background(), params.ApplicationSetCharm{ ApplicationName: "postgresql", CharmURL: "ch:postgresql", @@ -577,6 +608,11 @@ func (s *ApplicationSuite) TestSetCharmConfigSettingsYAML(c *gc.C) { }, gomock.Any()).Return(nil) s.backend.EXPECT().Application("postgresql").Return(app, nil) + s.applicationService.EXPECT().UpdateApplicationCharm(gomock.Any(), "postgresql", applicationservice.UpdateCharmParams{ + Charm: ch, + Storage: nil, + }).Return(nil) + err := s.api.SetCharm(context.Background(), params.ApplicationSetCharm{ ApplicationName: "postgresql", CharmURL: curl, @@ -616,6 +652,11 @@ func (s *ApplicationSuite) TestLXDProfileSetCharmWithNewerAgentVersion(c *gc.C) s.model.EXPECT().AgentVersion().Return(version.Number{Major: 2, Minor: 6, Patch: 0}, nil) + s.applicationService.EXPECT().UpdateApplicationCharm(gomock.Any(), "postgresql", applicationservice.UpdateCharmParams{ + Charm: ch, + Storage: nil, + }).Return(nil) + err := s.api.SetCharm(context.Background(), params.ApplicationSetCharm{ ApplicationName: "postgresql", CharmURL: curl, @@ -669,6 +710,11 @@ func (s *ApplicationSuite) TestLXDProfileSetCharmWithEmptyProfile(c *gc.C) { s.model.EXPECT().AgentVersion().Return(version.Number{Major: 2, Minor: 6, Patch: 0}, nil) + s.applicationService.EXPECT().UpdateApplicationCharm(gomock.Any(), "postgresql", applicationservice.UpdateCharmParams{ + Charm: ch, + Storage: nil, + }).Return(nil) + err := s.api.SetCharm(context.Background(), params.ApplicationSetCharm{ ApplicationName: "postgresql", CharmURL: curl, @@ -751,6 +797,11 @@ func (s *ApplicationSuite) TestSetCharmAssumesNotSatisfiedWithForce(c *gc.C) { app.EXPECT().SetCharm(gomock.Any(), gomock.Any()).Return(nil) s.backend.EXPECT().Application("postgresql").Return(app, nil) + s.applicationService.EXPECT().UpdateApplicationCharm(gomock.Any(), "postgresql", applicationservice.UpdateCharmParams{ + Charm: ch, + Storage: nil, + }).Return(nil) + // Try to upgrade the charm err := s.api.SetCharm(context.Background(), params.ApplicationSetCharm{ ApplicationName: "postgresql", diff --git a/apiserver/facades/client/application/deploy.go b/apiserver/facades/client/application/deploy.go index f08ab0be315..ff91c4c428f 100644 --- a/apiserver/facades/client/application/deploy.go +++ b/apiserver/facades/client/application/deploy.go @@ -79,6 +79,7 @@ type MachineService interface { type ApplicationService interface { CreateApplication(ctx context.Context, name string, params applicationservice.AddApplicationParams, units ...applicationservice.AddUnitParams) error AddUnits(ctx context.Context, name string, units ...applicationservice.AddUnitParams) error + UpdateApplicationCharm(ctx context.Context, name string, params applicationservice.UpdateCharmParams) error } // DeployApplication takes a charm and various parameters and deploys it. @@ -146,6 +147,7 @@ func DeployApplication( asa.Constraints = args.Constraints } + // TODO(dqlite) - remove mongo AddApplication call. // To ensure dqlite unit names match those created in mongo, grab the next unit // sequence number before writing the mongo units. nextUnitNum, err := st.ReadSequence(args.ApplicationName) @@ -158,8 +160,13 @@ func DeployApplication( unitArgs[i].UnitName = &n } app, err := st.AddApplication(asa, store) + + // Dual write storage constraints to dqlite. if err == nil { - err = applicationService.CreateApplication(ctx, args.ApplicationName, applicationservice.AddApplicationParams{}, unitArgs...) + err = applicationService.CreateApplication(ctx, args.ApplicationName, applicationservice.AddApplicationParams{ + Charm: args.Charm, + Storage: args.Storage, + }, unitArgs...) } return app, errors.Trace(err) } diff --git a/apiserver/facades/client/application/deploy_test.go b/apiserver/facades/client/application/deploy_test.go index fd884818cec..25a1706da1a 100644 --- a/apiserver/facades/client/application/deploy_test.go +++ b/apiserver/facades/client/application/deploy_test.go @@ -23,6 +23,7 @@ import ( "github.com/juju/juju/core/model" "github.com/juju/juju/core/network" "github.com/juju/juju/core/objectstore" + "github.com/juju/juju/internal/storage/provider" "github.com/juju/juju/juju/testing" jujutesting "github.com/juju/juju/juju/testing" "github.com/juju/juju/state" @@ -68,7 +69,7 @@ func (s *DeployLocalSuite) TestDeployControllerNotAllowed(c *gc.C) { s.ControllerModel(c), serviceFactory.Cloud(), serviceFactory.Credential(), - serviceFactory.Application(), + serviceFactory.Application(provider.CommonStorageProviders()), jujutesting.NewObjectStore(c, s.ControllerModelUUID()), application.DeployApplicationParams{ ApplicationName: "my-controller", @@ -88,7 +89,7 @@ func (s *DeployLocalSuite) TestDeployMinimal(c *gc.C) { s.ControllerModel(c), serviceFactory.Cloud(), serviceFactory.Credential(), - serviceFactory.Application(), + serviceFactory.Application(provider.CommonStorageProviders()), jujutesting.NewObjectStore(c, s.ControllerModelUUID()), application.DeployApplicationParams{ ApplicationName: "bob", @@ -114,7 +115,7 @@ func (s *DeployLocalSuite) TestDeployChannel(c *gc.C) { s.ControllerModel(c), serviceFactory.Cloud(), serviceFactory.Credential(), - serviceFactory.Application(), + serviceFactory.Application(provider.CommonStorageProviders()), jujutesting.NewObjectStore(c, s.ControllerModelUUID()), application.DeployApplicationParams{ ApplicationName: "bob", @@ -141,7 +142,7 @@ func (s *DeployLocalSuite) TestDeployWithImplicitBindings(c *gc.C) { s.ControllerModel(c), serviceFactory.Cloud(), serviceFactory.Credential(), - serviceFactory.Application(), + serviceFactory.Application(provider.CommonStorageProviders()), jujutesting.NewObjectStore(c, s.ControllerModelUUID()), application.DeployApplicationParams{ ApplicationName: "bob", @@ -213,7 +214,7 @@ func (s *DeployLocalSuite) TestDeployWithSomeSpecifiedBindings(c *gc.C) { model, serviceFactory.Cloud(), serviceFactory.Credential(), - serviceFactory.Application(), + serviceFactory.Application(provider.CommonStorageProviders()), jujutesting.NewObjectStore(c, s.ControllerModelUUID()), application.DeployApplicationParams{ ApplicationName: "bob", @@ -264,7 +265,7 @@ func (s *DeployLocalSuite) TestDeployWithBoundRelationNamesAndExtraBindingsNames model, serviceFactory.Cloud(), serviceFactory.Credential(), - serviceFactory.Application(), + serviceFactory.Application(provider.CommonStorageProviders()), jujutesting.NewObjectStore(c, s.ControllerModelUUID()), application.DeployApplicationParams{ ApplicationName: "bob", @@ -314,7 +315,7 @@ func (s *DeployLocalSuite) TestDeployWithInvalidSpace(c *gc.C) { model, serviceFactory.Cloud(), serviceFactory.Credential(), - serviceFactory.Application(), + serviceFactory.Application(provider.CommonStorageProviders()), jujutesting.NewObjectStore(c, s.ControllerModelUUID()), application.DeployApplicationParams{ ApplicationName: "bob", @@ -343,7 +344,7 @@ func (s *DeployLocalSuite) TestDeployResources(c *gc.C) { s.ControllerModel(c), serviceFactory.Cloud(), serviceFactory.Credential(), - serviceFactory.Application(), + serviceFactory.Application(provider.CommonStorageProviders()), jujutesting.NewObjectStore(c, s.ControllerModelUUID()), application.DeployApplicationParams{ ApplicationName: "bob", @@ -371,7 +372,7 @@ func (s *DeployLocalSuite) TestDeploySettings(c *gc.C) { s.ControllerModel(c), serviceFactory.Cloud(), serviceFactory.Credential(), - serviceFactory.Application(), + serviceFactory.Application(provider.CommonStorageProviders()), jujutesting.NewObjectStore(c, s.ControllerModelUUID()), application.DeployApplicationParams{ ApplicationName: "bob", @@ -400,7 +401,7 @@ func (s *DeployLocalSuite) TestDeploySettingsError(c *gc.C) { s.ControllerModel(c), serviceFactory.Cloud(), serviceFactory.Credential(), - serviceFactory.Application(), + serviceFactory.Application(provider.CommonStorageProviders()), jujutesting.NewObjectStore(c, s.ControllerModelUUID()), application.DeployApplicationParams{ ApplicationName: "bob", @@ -441,7 +442,7 @@ func (s *DeployLocalSuite) TestDeployWithApplicationConfig(c *gc.C) { s.ControllerModel(c), serviceFactory.Cloud(), serviceFactory.Credential(), - serviceFactory.Application(), + serviceFactory.Application(provider.CommonStorageProviders()), jujutesting.NewObjectStore(c, s.ControllerModelUUID()), application.DeployApplicationParams{ ApplicationName: "bob", @@ -471,7 +472,7 @@ func (s *DeployLocalSuite) TestDeployConstraints(c *gc.C) { s.ControllerModel(c), serviceFactory.Cloud(), serviceFactory.Credential(), - serviceFactory.Application(), + serviceFactory.Application(provider.CommonStorageProviders()), jujutesting.NewObjectStore(c, s.ControllerModelUUID()), application.DeployApplicationParams{ ApplicationName: "bob", @@ -495,7 +496,7 @@ func (s *DeployLocalSuite) TestDeployNumUnits(c *gc.C) { s.ControllerModel(c), serviceFactory.Cloud(), serviceFactory.Credential(), - serviceFactory.Application(), + serviceFactory.Application(provider.CommonStorageProviders()), jujutesting.NewObjectStore(c, s.ControllerModelUUID()), application.DeployApplicationParams{ ApplicationName: "bob", @@ -524,7 +525,7 @@ func (s *DeployLocalSuite) TestDeployForceMachineId(c *gc.C) { s.ControllerModel(c), serviceFactory.Cloud(), serviceFactory.Credential(), - serviceFactory.Application(), + serviceFactory.Application(provider.CommonStorageProviders()), jujutesting.NewObjectStore(c, s.ControllerModelUUID()), application.DeployApplicationParams{ ApplicationName: "bob", @@ -556,7 +557,7 @@ func (s *DeployLocalSuite) TestDeployForceMachineIdWithContainer(c *gc.C) { s.ControllerModel(c), serviceFactory.Cloud(), serviceFactory.Credential(), - serviceFactory.Application(), + serviceFactory.Application(provider.CommonStorageProviders()), jujutesting.NewObjectStore(c, s.ControllerModelUUID()), application.DeployApplicationParams{ ApplicationName: "bob", @@ -593,7 +594,7 @@ func (s *DeployLocalSuite) TestDeploy(c *gc.C) { s.ControllerModel(c), serviceFactory.Cloud(), serviceFactory.Credential(), - serviceFactory.Application(), + serviceFactory.Application(provider.CommonStorageProviders()), jujutesting.NewObjectStore(c, s.ControllerModelUUID()), application.DeployApplicationParams{ ApplicationName: "bob", @@ -634,7 +635,7 @@ func (s *DeployLocalSuite) TestDeployWithUnmetCharmRequirements(c *gc.C) { model, serviceFactory.Cloud(), serviceFactory.Credential(), - serviceFactory.Application(), + serviceFactory.Application(provider.CommonStorageProviders()), jujutesting.NewObjectStore(c, s.ControllerModelUUID()), application.DeployApplicationParams{ ApplicationName: "assume-metal", @@ -667,7 +668,7 @@ func (s *DeployLocalSuite) TestDeployWithUnmetCharmRequirementsAndForce(c *gc.C) model, serviceFactory.Cloud(), serviceFactory.Credential(), - serviceFactory.Application(), + serviceFactory.Application(provider.CommonStorageProviders()), jujutesting.NewObjectStore(c, s.ControllerModelUUID()), application.DeployApplicationParams{ ApplicationName: "assume-metal", @@ -692,7 +693,7 @@ func (s *DeployLocalSuite) TestDeployWithFewerPlacement(c *gc.C) { s.ControllerModel(c), serviceFactory.Cloud(), serviceFactory.Credential(), - serviceFactory.Application(), + serviceFactory.Application(provider.CommonStorageProviders()), jujutesting.NewObjectStore(c, s.ControllerModelUUID()), application.DeployApplicationParams{ ApplicationName: "bob", diff --git a/apiserver/facades/client/application/deployrepository.go b/apiserver/facades/client/application/deployrepository.go index 42614628ea9..47976b0b5c6 100644 --- a/apiserver/facades/client/application/deployrepository.go +++ b/apiserver/facades/client/application/deployrepository.go @@ -167,7 +167,10 @@ func (api *DeployFromRepositoryAPI) DeployFromRepository(ctx context.Context, ar n := fmt.Sprintf("%s/%d", dt.applicationName, nextUnitNum+i) unitArgs[i].UnitName = &n } - addApplicationErr = api.applicationService.CreateApplication(ctx, dt.applicationName, applicationservice.AddApplicationParams{}, unitArgs...) + addApplicationErr = api.applicationService.CreateApplication(ctx, dt.applicationName, applicationservice.AddApplicationParams{ + Charm: ch, + Storage: dt.storage, + }, unitArgs...) } if addApplicationErr != nil { diff --git a/apiserver/facades/client/application/deployrepository_test.go b/apiserver/facades/client/application/deployrepository_test.go index 149abb7f5bb..17d0954b8ab 100644 --- a/apiserver/facades/client/application/deployrepository_test.go +++ b/apiserver/facades/client/application/deployrepository_test.go @@ -1098,8 +1098,10 @@ func (s *deployRepositorySuite) TestDeployFromRepositoryAPI(c *gc.C) { } s.state.EXPECT().ReadSequence("metadata-name").Return(0, nil) s.state.EXPECT().AddApplication(addApplicationArgsMatcher{c: c, expectedArgs: addAppArgs}, gomock.Any()).Return(s.application, nil) - s.applicationService.EXPECT().CreateApplication(gomock.Any(), "metadata-name", applicationservice.AddApplicationParams{}, applicationservice.AddUnitParams{UnitName: ptr("metadata-name/0")}) - + s.applicationService.EXPECT().CreateApplication(gomock.Any(), "metadata-name", applicationservice.AddApplicationParams{ + Charm: s.charm, + Storage: nil, + }, applicationservice.AddUnitParams{UnitName: ptr("metadata-name/0")}) deployFromRepositoryAPI := s.getDeployFromRepositoryAPI() obtainedInfo, resources, errs := deployFromRepositoryAPI.DeployFromRepository(context.Background(), arg) @@ -1229,7 +1231,10 @@ func (s *deployRepositorySuite) TestAddPendingResourcesForDeployFromRepositoryAP } s.state.EXPECT().ReadSequence("metadata-name").Return(0, nil) s.state.EXPECT().AddApplication(addApplicationArgsMatcher{c: c, expectedArgs: addAppArgs}, gomock.Any()).Return(s.application, nil) - s.applicationService.EXPECT().CreateApplication(gomock.Any(), "metadata-name", applicationservice.AddApplicationParams{}, applicationservice.AddUnitParams{UnitName: ptr("metadata-name/0")}) + s.applicationService.EXPECT().CreateApplication(gomock.Any(), "metadata-name", applicationservice.AddApplicationParams{ + Charm: s.charm, + Storage: nil, + }, applicationservice.AddUnitParams{UnitName: ptr("metadata-name/0")}) deployFromRepositoryAPI := s.getDeployFromRepositoryAPI() diff --git a/apiserver/facades/client/application/domain_mock.go b/apiserver/facades/client/application/domain_mock.go index 5c207ed04e5..d32c3f747ff 100644 --- a/apiserver/facades/client/application/domain_mock.go +++ b/apiserver/facades/client/application/domain_mock.go @@ -116,6 +116,20 @@ func (mr *MockApplicationServiceMockRecorder) CreateApplication(arg0, arg1, arg2 return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CreateApplication", reflect.TypeOf((*MockApplicationService)(nil).CreateApplication), varargs...) } +// UpdateApplicationCharm mocks base method. +func (m *MockApplicationService) UpdateApplicationCharm(arg0 context.Context, arg1 string, arg2 service.UpdateCharmParams) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "UpdateApplicationCharm", arg0, arg1, arg2) + ret0, _ := ret[0].(error) + return ret0 +} + +// UpdateApplicationCharm indicates an expected call of UpdateApplicationCharm. +func (mr *MockApplicationServiceMockRecorder) UpdateApplicationCharm(arg0, arg1, arg2 any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UpdateApplicationCharm", reflect.TypeOf((*MockApplicationService)(nil).UpdateApplicationCharm), arg0, arg1, arg2) +} + // MockStoragePoolGetter is a mock of StoragePoolGetter interface. type MockStoragePoolGetter struct { ctrl *gomock.Controller diff --git a/apiserver/facades/client/application/get_test.go b/apiserver/facades/client/application/get_test.go index d02ae12c049..64ca7fbb219 100644 --- a/apiserver/facades/client/application/get_test.go +++ b/apiserver/facades/client/application/get_test.go @@ -15,6 +15,7 @@ import ( "github.com/juju/juju/apiserver/common" "github.com/juju/juju/apiserver/facades/client/application" apiservertesting "github.com/juju/juju/apiserver/testing" + "github.com/juju/juju/caas" "github.com/juju/juju/caas/kubernetes/provider" k8stesting "github.com/juju/juju/caas/kubernetes/provider/testing" coreconfig "github.com/juju/juju/core/config" @@ -22,6 +23,7 @@ import ( "github.com/juju/juju/core/model" "github.com/juju/juju/core/network" "github.com/juju/juju/environs" + storageprovider "github.com/juju/juju/internal/storage/provider" jujutesting "github.com/juju/juju/juju/testing" "github.com/juju/juju/rpc/params" "github.com/juju/juju/state" @@ -70,7 +72,7 @@ func (s *getSuite) SetUpTest(c *gc.C) { serviceFactory.Cloud(), serviceFactory.Credential(), serviceFactory.Machine(), - serviceFactory.Application(), + serviceFactory.Application(storageprovider.CommonStorageProviders()), nil, // leadership not used in this suite. application.CharmToStateCharm, application.DeployApplication, @@ -191,10 +193,12 @@ func (s *getSuite) TestClientApplicationGetCAASModelSmokeTest(c *gc.C) { c.Assert(err, jc.ErrorIsNil) serviceFactory := s.DefaultModelServiceFactory(c) - envFunc := stateenvirons.GetNewEnvironFunc(environs.New) - env, err := envFunc(s.ControllerModel(c), serviceFactory.Cloud(), serviceFactory.Credential()) + registry, err := stateenvirons.NewStorageProviderRegistryForModel( + mod, serviceFactory.Cloud(), serviceFactory.Credential(), + stateenvirons.GetNewEnvironFunc(environs.New), + stateenvirons.GetNewCAASBrokerFunc(caas.New), + ) c.Assert(err, jc.ErrorIsNil) - registry := stateenvirons.NewStorageProviderRegistry(env) api, err := application.NewAPIBase( application.GetState(st, state.NoopInstancePrechecker{}), @@ -208,7 +212,7 @@ func (s *getSuite) TestClientApplicationGetCAASModelSmokeTest(c *gc.C) { serviceFactory.Cloud(), serviceFactory.Credential(), serviceFactory.Machine(), - serviceFactory.Application(), + serviceFactory.Application(registry), nil, // leadership not used in this suite. application.CharmToStateCharm, application.DeployApplication, diff --git a/apiserver/facades/client/highavailability/highavailability.go b/apiserver/facades/client/highavailability/highavailability.go index cf62f4bc9fe..fea00163f07 100644 --- a/apiserver/facades/client/highavailability/highavailability.go +++ b/apiserver/facades/client/highavailability/highavailability.go @@ -42,9 +42,9 @@ type MachineService interface { CreateMachine(context.Context, string) error } -// ApplicationService instances save an application to dqlite state. +// ApplicationService instances add units to an application in dqlite state. type ApplicationService interface { - CreateApplication(ctx context.Context, name string, params applicationservice.AddApplicationParams, units ...applicationservice.AddUnitParams) error + AddUnits(ctx context.Context, name string, units ...applicationservice.AddUnitParams) error } // ControllerConfigGetter instances read the controller config. @@ -177,7 +177,7 @@ func (api *HighAvailabilityAPI) enableHASingle(ctx context.Context, spec params. n := addedUnits[i] addUnitArgs[i].UnitName = &n } - if err := api.applicationService.CreateApplication(ctx, application.ControllerApplicationName, applicationservice.AddApplicationParams{}, addUnitArgs...); err != nil { + if err := api.applicationService.AddUnits(ctx, application.ControllerApplicationName, addUnitArgs...); err != nil { return params.ControllersChanges{}, err } } diff --git a/apiserver/facades/client/highavailability/register.go b/apiserver/facades/client/highavailability/register.go index ca47c6327cf..8b7abd6419c 100644 --- a/apiserver/facades/client/highavailability/register.go +++ b/apiserver/facades/client/highavailability/register.go @@ -52,18 +52,18 @@ func newHighAvailabilityAPI(ctx facade.ModelContext) (*HighAvailabilityAPI, erro } serviceFactory := ctx.ServiceFactory() - prechecker, err := stateenvirons.NewInstancePrechecker(st, serviceFactory.Cloud(), serviceFactory.Credential()) if err != nil { return nil, errors.Trace(err) } return &HighAvailabilityAPI{ - st: st, - prechecker: prechecker, - nodeService: serviceFactory.ControllerNode(), - machineService: serviceFactory.Machine(), - applicationService: serviceFactory.Application(), + st: st, + prechecker: prechecker, + nodeService: serviceFactory.ControllerNode(), + machineService: serviceFactory.Machine(), + // For adding additional controller units, we don't need a storage registry. + applicationService: serviceFactory.Application(nil), controllerConfig: serviceFactory.ControllerConfig(), authorizer: authorizer, logger: ctx.Logger().Child("highavailability"), diff --git a/apiserver/facades/controller/cleaner/cleaner_test.go b/apiserver/facades/controller/cleaner/cleaner_test.go index 28b3d87beeb..dd983ed9823 100644 --- a/apiserver/facades/controller/cleaner/cleaner_test.go +++ b/apiserver/facades/controller/cleaner/cleaner_test.go @@ -7,6 +7,7 @@ import ( "context" "github.com/juju/errors" + "github.com/juju/loggo/v2" "github.com/juju/testing" jc "github.com/juju/testing/checkers" gc "gopkg.in/check.v1" @@ -21,6 +22,7 @@ import ( machineservice "github.com/juju/juju/domain/machine/service" servicefactorytesting "github.com/juju/juju/domain/servicefactory/testing" unitservice "github.com/juju/juju/domain/unit/service" + "github.com/juju/juju/internal/storage" "github.com/juju/juju/rpc/params" "github.com/juju/juju/state" coretesting "github.com/juju/juju/testing" @@ -50,7 +52,7 @@ func (s *CleanerSuite) SetUpTest(c *gc.C) { var err error res := common.NewResources() s.machineService = machineservice.NewService(nil) - s.applicationService = applicationservice.NewService(nil) + s.applicationService = applicationservice.NewService(nil, loggo.GetLogger("test"), storage.NotImplementedProviderRegistry{}) s.unitService = unitservice.NewService(nil) s.api, err = cleaner.NewCleanerAPI(facadetest.ModelContext{ Resources_: res, diff --git a/apiserver/facades/controller/cleaner/register.go b/apiserver/facades/controller/cleaner/register.go index 454e6d9ceca..cbcfd60c4fd 100644 --- a/apiserver/facades/controller/cleaner/register.go +++ b/apiserver/facades/controller/cleaner/register.go @@ -24,12 +24,15 @@ func newCleanerAPI(ctx facade.ModelContext) (*CleanerAPI, error) { if !authorizer.AuthController() { return nil, apiservererrors.ErrPerm } + + serviceFactory := ctx.ServiceFactory() return &CleanerAPI{ st: getState(ctx.State()), resources: ctx.Resources(), objectStore: ctx.ObjectStore(), - machineRemover: ctx.ServiceFactory().Machine(), - appRemover: ctx.ServiceFactory().Application(), - unitRemover: ctx.ServiceFactory().Unit(), + machineRemover: serviceFactory.Machine(), + // For removing applications, we don't need a storage registry. + appRemover: serviceFactory.Application(nil), + unitRemover: serviceFactory.Unit(), }, nil } diff --git a/apiserver/facades/controller/migrationtarget/servicefactory_mock_test.go b/apiserver/facades/controller/migrationtarget/servicefactory_mock_test.go index e3798287603..0492b3888e6 100644 --- a/apiserver/facades/controller/migrationtarget/servicefactory_mock_test.go +++ b/apiserver/facades/controller/migrationtarget/servicefactory_mock_test.go @@ -127,17 +127,17 @@ func (mr *MockServiceFactoryMockRecorder) Annotation() *gomock.Call { } // Application mocks base method. -func (m *MockServiceFactory) Application() *service0.Service { +func (m *MockServiceFactory) Application(arg0 storage.ProviderRegistry) *service0.Service { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "Application") + ret := m.ctrl.Call(m, "Application", arg0) ret0, _ := ret[0].(*service0.Service) return ret0 } // Application indicates an expected call of Application. -func (mr *MockServiceFactoryMockRecorder) Application() *gomock.Call { +func (mr *MockServiceFactoryMockRecorder) Application(arg0 any) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Application", reflect.TypeOf((*MockServiceFactory)(nil).Application)) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Application", reflect.TypeOf((*MockServiceFactory)(nil).Application), arg0) } // AutocertCache mocks base method. diff --git a/domain/application/config.go b/domain/application/config.go new file mode 100644 index 00000000000..423def6ad6d --- /dev/null +++ b/domain/application/config.go @@ -0,0 +1,13 @@ +// Copyright 2024 Canonical Ltd. +// Licensed under the AGPLv3, see LICENCE file for details. + +package application + +// TODO(dqlite) - we don't want to reference environs/config here but need a place for config key names. +const ( + // StorageDefaultBlockSourceKey is the key for the default block storage source. + StorageDefaultBlockSourceKey = "storage-default-block-source" + + // StorageDefaultFilesystemSourceKey is the key for the default filesystem storage source. + StorageDefaultFilesystemSourceKey = "storage-default-filesystem-source" +) diff --git a/domain/application/errors/errors.go b/domain/application/errors/errors.go index 7016ccb8b95..5953969d35b 100644 --- a/domain/application/errors/errors.go +++ b/domain/application/errors/errors.go @@ -8,11 +8,12 @@ import ( ) const ( - // NotFound describes an error that occurs when the application being operated on + // ApplicationNotFound describes an error that occurs when the application being operated on // does not exist. - NotFound = errors.ConstError("application not found") - - // HasUnits describes an error that occurs when the application being deleted still + ApplicationNotFound = errors.ConstError("application not found") + // ApplicationHasUnits describes an error that occurs when the application being deleted still // has associated units. - HasUnits = errors.ConstError("application has units") + ApplicationHasUnits = errors.ConstError("application has units") + // MissingStorageConstraints describes an error that occurs when expected storage constraints are missing. + MissingStorageConstraints = errors.ConstError("no storage constraints specified") ) diff --git a/domain/application/modelmigration/import.go b/domain/application/modelmigration/import.go index 9774484708f..7156dce5234 100644 --- a/domain/application/modelmigration/import.go +++ b/domain/application/modelmigration/import.go @@ -13,6 +13,7 @@ import ( "github.com/juju/juju/core/modelmigration" "github.com/juju/juju/domain/application/service" "github.com/juju/juju/domain/application/state" + "github.com/juju/juju/internal/storage" ) var logger = loggo.GetLogger("juju.migration.applications") @@ -24,14 +25,15 @@ type Coordinator interface { // RegisterImport register's a new model migration importer into the supplied // coordinator. -func RegisterImport(coordinator Coordinator) { - coordinator.Add(&importOperation{}) +func RegisterImport(coordinator Coordinator, registry storage.ProviderRegistry) { + coordinator.Add(&importOperation{registry: registry}) } type importOperation struct { modelmigration.BaseOperation - service ImportService + service ImportService + registry storage.ProviderRegistry } // ImportService defines the application service used to import applications @@ -44,6 +46,8 @@ type ImportService interface { func (i *importOperation) Setup(scope modelmigration.Scope) error { i.service = service.NewService( state.NewState(scope.ModelDB(), logger), + logger, + i.registry, ) return nil } diff --git a/domain/application/service/package_mock_test.go b/domain/application/service/package_mock_test.go index 05078759e96..5009f704740 100644 --- a/domain/application/service/package_mock_test.go +++ b/domain/application/service/package_mock_test.go @@ -1,9 +1,9 @@ // Code generated by MockGen. DO NOT EDIT. -// Source: github.com/juju/juju/domain/application/service (interfaces: State) +// Source: github.com/juju/juju/domain/application/service (interfaces: State,Charm) // // Generated by this command: // -// mockgen -package service -destination package_mock_test.go github.com/juju/juju/domain/application/service State +// mockgen -package service -destination package_mock_test.go github.com/juju/juju/domain/application/service State,Charm // // Package service is a generated GoMock package. @@ -13,7 +13,9 @@ import ( context "context" reflect "reflect" + charm "github.com/juju/charm/v13" application "github.com/juju/juju/domain/application" + storage "github.com/juju/juju/domain/storage" gomock "go.uber.org/mock/gomock" ) @@ -73,6 +75,36 @@ func (mr *MockStateMockRecorder) DeleteApplication(arg0, arg1 any) *gomock.Call return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeleteApplication", reflect.TypeOf((*MockState)(nil).DeleteApplication), arg0, arg1) } +// GetStoragePoolByName mocks base method. +func (m *MockState) GetStoragePoolByName(arg0 context.Context, arg1 string) (storage.StoragePoolDetails, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetStoragePoolByName", arg0, arg1) + ret0, _ := ret[0].(storage.StoragePoolDetails) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// GetStoragePoolByName indicates an expected call of GetStoragePoolByName. +func (mr *MockStateMockRecorder) GetStoragePoolByName(arg0, arg1 any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetStoragePoolByName", reflect.TypeOf((*MockState)(nil).GetStoragePoolByName), arg0, arg1) +} + +// StorageDefaults mocks base method. +func (m *MockState) StorageDefaults(arg0 context.Context) (storage.StorageDefaults, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "StorageDefaults", arg0) + ret0, _ := ret[0].(storage.StorageDefaults) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// StorageDefaults indicates an expected call of StorageDefaults. +func (mr *MockStateMockRecorder) StorageDefaults(arg0 any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "StorageDefaults", reflect.TypeOf((*MockState)(nil).StorageDefaults), arg0) +} + // UpsertApplication mocks base method. func (m *MockState) UpsertApplication(arg0 context.Context, arg1 string, arg2 ...application.AddUnitParams) error { m.ctrl.T.Helper() @@ -91,3 +123,40 @@ func (mr *MockStateMockRecorder) UpsertApplication(arg0, arg1 any, arg2 ...any) varargs := append([]any{arg0, arg1}, arg2...) return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UpsertApplication", reflect.TypeOf((*MockState)(nil).UpsertApplication), varargs...) } + +// MockCharm is a mock of Charm interface. +type MockCharm struct { + ctrl *gomock.Controller + recorder *MockCharmMockRecorder +} + +// MockCharmMockRecorder is the mock recorder for MockCharm. +type MockCharmMockRecorder struct { + mock *MockCharm +} + +// NewMockCharm creates a new mock instance. +func NewMockCharm(ctrl *gomock.Controller) *MockCharm { + mock := &MockCharm{ctrl: ctrl} + mock.recorder = &MockCharmMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use. +func (m *MockCharm) EXPECT() *MockCharmMockRecorder { + return m.recorder +} + +// Meta mocks base method. +func (m *MockCharm) Meta() *charm.Meta { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "Meta") + ret0, _ := ret[0].(*charm.Meta) + return ret0 +} + +// Meta indicates an expected call of Meta. +func (mr *MockCharmMockRecorder) Meta() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Meta", reflect.TypeOf((*MockCharm)(nil).Meta)) +} diff --git a/domain/application/service/package_test.go b/domain/application/service/package_test.go index 4ceec95883d..5abbc5f2460 100644 --- a/domain/application/service/package_test.go +++ b/domain/application/service/package_test.go @@ -9,7 +9,7 @@ import ( gc "gopkg.in/check.v1" ) -//go:generate go run go.uber.org/mock/mockgen -package service -destination package_mock_test.go github.com/juju/juju/domain/application/service State +//go:generate go run go.uber.org/mock/mockgen -package service -destination package_mock_test.go github.com/juju/juju/domain/application/service State,Charm func TestPackage(t *testing.T) { gc.TestingT(t) diff --git a/domain/application/service/params.go b/domain/application/service/params.go index 75a2037d418..bc71127a15a 100644 --- a/domain/application/service/params.go +++ b/domain/application/service/params.go @@ -3,10 +3,23 @@ package service +import ( + "github.com/juju/charm/v13" + + "github.com/juju/juju/internal/storage" +) + +// Charm represents an application's charm. +type Charm interface { + Meta() *charm.Meta +} + // AddApplicationParams contain parameters for adding an application to the model. type AddApplicationParams struct { - // Application constraints go here. - // Storage constraints go here. + // Charm is the application's charm. + Charm Charm + // Storage contains the application's storage constraints. + Storage map[string]storage.Constraints } // AddUnitParams contains parameters for adding a unit to the model. @@ -23,3 +36,20 @@ type UpsertCAASUnitParams struct { // UnitName is for CAAS models when creating stateful units. UnitName *string } + +// UpdateCharmParams contains the parameters for updating +// an application's charm and storage. +type UpdateCharmParams struct { + // Charm is the new charm to use for the application. New units + // will be started with this charm, and existing units will be + // upgraded to use it. + Charm Charm + + // Storage contains the storage constraints to add or update when + // upgrading the charm. + // + // Any existing storage instances for the named stores will be + // unaffected; the storage constraints will only be used for + // provisioning new storage instances. + Storage map[string]storage.Constraints +} diff --git a/domain/application/service/service.go b/domain/application/service/service.go index d2098490bd1..52416451cd0 100644 --- a/domain/application/service/service.go +++ b/domain/application/service/service.go @@ -5,14 +5,32 @@ package service import ( "context" + "fmt" + "github.com/juju/charm/v13" "github.com/juju/errors" + coremodel "github.com/juju/juju/core/model" "github.com/juju/juju/domain/application" + applicationerrors "github.com/juju/juju/domain/application/errors" + domainstorage "github.com/juju/juju/domain/storage" + "github.com/juju/juju/internal/storage" ) +// Logger facilitates emitting log messages. +type Logger interface { + Warningf(string, ...interface{}) +} + // State describes retrieval and persistence methods for applications. type State interface { + // StorageDefaults returns the default storage sources for a model. + StorageDefaults(ctx context.Context) (domainstorage.StorageDefaults, error) + + // GetStoragePoolByName returns the storage pool with the specified name, returning an error + // satisfying [storageerrors.PoolNotFoundError] if it doesn't exist. + GetStoragePoolByName(ctx context.Context, name string) (domainstorage.StoragePoolDetails, error) + // UpsertApplication persists the input Application entity. UpsertApplication(context.Context, string, ...application.AddUnitParams) error @@ -25,24 +43,49 @@ type State interface { // Service provides the API for working with applications. type Service struct { - st State + st State + logger Logger + + registry storage.ProviderRegistry + modelType coremodel.ModelType } // NewService returns a new service reference wrapping the input state. -func NewService(st State) *Service { +func NewService(st State, logger Logger, registry storage.ProviderRegistry) *Service { + // Some uses of application service don't need to supply a storage registry, eg cleaner facade. + // In such cases it'd wasteful to create one as an environ instance would be needed. + if registry == nil { + registry = storage.NotImplementedProviderRegistry{} + } return &Service{ - st: st, + st: st, + logger: logger, + registry: registry, + // TODO(storage) - pass in model info getter + modelType: coremodel.IAAS, } } // CreateApplication creates the specified application and units if required. func (s *Service) CreateApplication(ctx context.Context, name string, params AddApplicationParams, units ...AddUnitParams) error { + cons := make(map[string]storage.Constraints) + for n, sc := range params.Storage { + cons[n] = sc + } + if err := s.addDefaultStorageConstraints(ctx, s.modelType, cons, params.Charm.Meta()); err != nil { + return errors.Annotate(err, "adding default storage constraints") + } + if err := s.validateStorageConstraints(ctx, s.modelType, cons, params.Charm); err != nil { + return errors.Annotate(err, "invalid storage constraints") + } args := make([]application.AddUnitParams, len(units)) for i, u := range units { args[i] = application.AddUnitParams{ UnitName: u.UnitName, } } + //TODO(storage) - insert storage cons for app + err := s.st.UpsertApplication(ctx, name, args...) return errors.Annotatef(err, "saving application %q", name) } @@ -73,3 +116,39 @@ func (s *Service) DeleteApplication(ctx context.Context, name string) error { err := s.st.DeleteApplication(ctx, name) return errors.Annotatef(err, "deleting application %q", name) } + +// UpdateApplicationCharm sets a new charm for the application, validating that aspects such +// as storage are still viable with the new charm. +func (s *Service) UpdateApplicationCharm(ctx context.Context, name string, params UpdateCharmParams) error { + //TODO(storage) - update charm and storage cons for app + return nil +} + +// addDefaultStorageConstraints fills in default constraint values, replacing any empty/missing values +// in the specified constraints. +func (s *Service) addDefaultStorageConstraints(ctx context.Context, modelType coremodel.ModelType, allCons map[string]storage.Constraints, charmMeta *charm.Meta) error { + defaults, err := s.st.StorageDefaults(ctx) + if err != nil { + return errors.Annotate(err, "getting storage defaults") + } + return domainstorage.StorageConstraintsWithDefaults(charmMeta.Storage, modelType, defaults, allCons) +} + +func (s *Service) validateStorageConstraints(ctx context.Context, modelType coremodel.ModelType, allCons map[string]storage.Constraints, charm Charm) error { + validator, err := domainstorage.NewStorageConstraintsValidator(modelType, s.registry, s.st) + if err != nil { + return errors.Trace(err) + } + err = validator.ValidateStorageConstraintsAgainstCharm(ctx, allCons, charm) + if err != nil { + return errors.Trace(err) + } + // Ensure all stores have constraints specified. Defaults should have + // been set by this point, if the user didn't specify constraints. + for name, charmStorage := range charm.Meta().Storage { + if _, ok := allCons[name]; !ok && charmStorage.CountMin > 0 { + return fmt.Errorf("%w for store %q", applicationerrors.MissingStorageConstraints, name) + } + } + return nil +} diff --git a/domain/application/service/service_test.go b/domain/application/service/service_test.go index 3fcddc7016a..7c9295977cc 100644 --- a/domain/application/service/service_test.go +++ b/domain/application/service/service_test.go @@ -6,18 +6,28 @@ package service import ( "context" + "github.com/juju/charm/v13" "github.com/juju/errors" + "github.com/juju/loggo/v2" "github.com/juju/testing" jc "github.com/juju/testing/checkers" "go.uber.org/mock/gomock" gc "gopkg.in/check.v1" "github.com/juju/juju/domain/application" + domainstorage "github.com/juju/juju/domain/storage" + storageerrors "github.com/juju/juju/domain/storage/errors" + "github.com/juju/juju/internal/storage" + "github.com/juju/juju/internal/storage/provider" + dummystorage "github.com/juju/juju/internal/storage/provider/dummy" ) type serviceSuite struct { testing.IsolationSuite - state *MockState + + state *MockState + charm *MockCharm + service *Service } var _ = gc.Suite(&serviceSuite{}) @@ -25,6 +35,13 @@ var _ = gc.Suite(&serviceSuite{}) func (s *serviceSuite) setupMocks(c *gc.C) *gomock.Controller { ctrl := gomock.NewController(c) s.state = NewMockState(ctrl) + s.charm = NewMockCharm(ctrl) + registry := storage.ChainedProviderRegistry{ + dummystorage.StorageProviders(), + provider.CommonStorageProviders(), + } + s.service = NewService(s.state, loggo.GetLogger("test"), registry) + return ctrl } @@ -32,28 +49,219 @@ func ptr[T any](v T) *T { return &v } -func (s *serviceSuite) TestUpdateSuccess(c *gc.C) { +func (s *serviceSuite) TestCreateApplication(c *gc.C) { + defer s.setupMocks(c).Finish() + + u := application.AddUnitParams{ + UnitName: ptr("foo/666"), + } + s.state.EXPECT().StorageDefaults(gomock.Any()).Return(domainstorage.StorageDefaults{}, nil) + s.state.EXPECT().UpsertApplication(gomock.Any(), "666", u).Return(nil) + s.charm.EXPECT().Meta().Return(&charm.Meta{}).AnyTimes() + + a := AddUnitParams{ + UnitName: ptr("foo/666"), + } + err := s.service.CreateApplication(context.Background(), "666", AddApplicationParams{ + Charm: s.charm, + }, a) + c.Assert(err, jc.ErrorIsNil) +} + +func (s *serviceSuite) TestCreateWithStorageBlock(c *gc.C) { + defer s.setupMocks(c).Finish() + + u := application.AddUnitParams{ + UnitName: ptr("foo/666"), + } + s.state.EXPECT().StorageDefaults(gomock.Any()).Return(domainstorage.StorageDefaults{}, nil) + s.state.EXPECT().UpsertApplication(gomock.Any(), "666", u).Return(nil) + s.charm.EXPECT().Meta().Return(&charm.Meta{ + Storage: map[string]charm.Storage{ + "data": { + Name: "data", + Type: charm.StorageBlock, + Shared: false, + CountMin: 1, + CountMax: 2, + MinimumSize: 10, + }, + }, + }).AnyTimes() + pool := domainstorage.StoragePoolDetails{Name: "loop", Provider: "loop"} + s.state.EXPECT().GetStoragePoolByName(gomock.Any(), "loop").Return(pool, nil) + + a := AddUnitParams{ + UnitName: ptr("foo/666"), + } + err := s.service.CreateApplication(context.Background(), "666", AddApplicationParams{ + Charm: s.charm, + }, a) + c.Assert(err, jc.ErrorIsNil) +} + +func (s *serviceSuite) TestCreateWithStorageBlockDefaultSource(c *gc.C) { defer s.setupMocks(c).Finish() u := application.AddUnitParams{ UnitName: ptr("foo/666"), } + s.state.EXPECT().StorageDefaults(gomock.Any()).Return(domainstorage.StorageDefaults{DefaultBlockSource: ptr("fast")}, nil) s.state.EXPECT().UpsertApplication(gomock.Any(), "666", u).Return(nil) + s.charm.EXPECT().Meta().Return(&charm.Meta{ + Storage: map[string]charm.Storage{ + "data": { + Name: "data", + Type: charm.StorageBlock, + Shared: false, + CountMin: 1, + CountMax: 2, + MinimumSize: 10, + }, + }, + }).AnyTimes() + pool := domainstorage.StoragePoolDetails{Name: "fast", Provider: "modelscoped-block"} + s.state.EXPECT().GetStoragePoolByName(gomock.Any(), "fast").Return(pool, nil) a := AddUnitParams{ UnitName: ptr("foo/666"), } - err := NewService(s.state).CreateApplication(context.Background(), "666", AddApplicationParams{}, a) + err := s.service.CreateApplication(context.Background(), "666", AddApplicationParams{ + Charm: s.charm, + Storage: map[string]storage.Constraints{ + "data": {Count: 2}, + }, + }, a) c.Assert(err, jc.ErrorIsNil) } -func (s *serviceSuite) TestUpdateError(c *gc.C) { +func (s *serviceSuite) TestCreateWithStorageFilesystem(c *gc.C) { + defer s.setupMocks(c).Finish() + + u := application.AddUnitParams{ + UnitName: ptr("foo/666"), + } + s.state.EXPECT().StorageDefaults(gomock.Any()).Return(domainstorage.StorageDefaults{}, nil) + s.state.EXPECT().UpsertApplication(gomock.Any(), "666", u).Return(nil) + s.charm.EXPECT().Meta().Return(&charm.Meta{ + Storage: map[string]charm.Storage{ + "data": { + Name: "data", + Type: charm.StorageFilesystem, + Shared: false, + CountMin: 1, + CountMax: 2, + MinimumSize: 10, + }, + }, + }).AnyTimes() + pool := domainstorage.StoragePoolDetails{Name: "rootfs", Provider: "rootfs"} + s.state.EXPECT().GetStoragePoolByName(gomock.Any(), "rootfs").Return(pool, nil) + + a := AddUnitParams{ + UnitName: ptr("foo/666"), + } + err := s.service.CreateApplication(context.Background(), "666", AddApplicationParams{ + Charm: s.charm, + }, a) + c.Assert(err, jc.ErrorIsNil) +} + +func (s *serviceSuite) TestCreateWithStorageFilesystemDefaultSource(c *gc.C) { + defer s.setupMocks(c).Finish() + + u := application.AddUnitParams{ + UnitName: ptr("foo/666"), + } + s.state.EXPECT().StorageDefaults(gomock.Any()).Return(domainstorage.StorageDefaults{DefaultFilesystemSource: ptr("fast")}, nil) + s.state.EXPECT().UpsertApplication(gomock.Any(), "666", u).Return(nil) + s.charm.EXPECT().Meta().Return(&charm.Meta{ + Storage: map[string]charm.Storage{ + "data": { + Name: "data", + Type: charm.StorageFilesystem, + CountMin: 1, + CountMax: 2, + MinimumSize: 10, + }, + }, + }).AnyTimes() + pool := domainstorage.StoragePoolDetails{Name: "fast", Provider: "modelscoped"} + s.state.EXPECT().GetStoragePoolByName(gomock.Any(), "fast").Return(pool, nil) + + a := AddUnitParams{ + UnitName: ptr("foo/666"), + } + err := s.service.CreateApplication(context.Background(), "666", AddApplicationParams{ + Charm: s.charm, + Storage: map[string]storage.Constraints{ + "data": {Count: 2}, + }, + }, a) + c.Assert(err, jc.ErrorIsNil) +} + +func (s *serviceSuite) TestCreateWithSharedStorageMissingConstraints(c *gc.C) { + defer s.setupMocks(c).Finish() + + s.state.EXPECT().StorageDefaults(gomock.Any()).Return(domainstorage.StorageDefaults{}, nil) + s.charm.EXPECT().Meta().Return(&charm.Meta{ + Storage: map[string]charm.Storage{ + "data": { + Name: "data", + Type: charm.StorageBlock, + Shared: true, + }, + }, + }).AnyTimes() + + a := AddUnitParams{ + UnitName: ptr("foo/666"), + } + err := s.service.CreateApplication(context.Background(), "666", AddApplicationParams{ + Charm: s.charm, + }, a) + c.Assert(err, jc.ErrorIs, storageerrors.MissingSharedStorageConstraintError) + c.Assert(err, gc.ErrorMatches, `adding default storage constraints: no storage constraints specified for shared charm storage "data"`) +} + +func (s *serviceSuite) TestCreateWithStorageValidates(c *gc.C) { + defer s.setupMocks(c).Finish() + + s.state.EXPECT().StorageDefaults(gomock.Any()).Return(domainstorage.StorageDefaults{}, nil) + s.charm.EXPECT().Meta().Return(&charm.Meta{ + Name: "mine", + Storage: map[string]charm.Storage{ + "data": { + Name: "data", + Type: charm.StorageBlock, + }, + }, + }).AnyTimes() + + a := AddUnitParams{ + UnitName: ptr("foo/666"), + } + err := s.service.CreateApplication(context.Background(), "666", AddApplicationParams{ + Charm: s.charm, + Storage: map[string]storage.Constraints{ + "logs": {Count: 2}, + }, + }, a) + c.Assert(err, gc.ErrorMatches, `invalid storage constraints: charm "mine" has no store called "logs"`) +} + +func (s *serviceSuite) TestCreateApplicationError(c *gc.C) { defer s.setupMocks(c).Finish() rErr := errors.New("boom") + s.state.EXPECT().StorageDefaults(gomock.Any()).Return(domainstorage.StorageDefaults{}, nil) s.state.EXPECT().UpsertApplication(gomock.Any(), "666").Return(rErr) + s.charm.EXPECT().Meta().Return(&charm.Meta{}).AnyTimes() - err := NewService(s.state).CreateApplication(context.Background(), "666", AddApplicationParams{}) + err := s.service.CreateApplication(context.Background(), "666", AddApplicationParams{ + Charm: s.charm, + }) c.Check(err, jc.ErrorIs, rErr) c.Assert(err, gc.ErrorMatches, `saving application "666": boom`) } @@ -63,7 +271,7 @@ func (s *serviceSuite) TestDeleteApplicationSuccess(c *gc.C) { s.state.EXPECT().DeleteApplication(gomock.Any(), "666").Return(nil) - err := NewService(s.state).DeleteApplication(context.Background(), "666") + err := s.service.DeleteApplication(context.Background(), "666") c.Assert(err, jc.ErrorIsNil) } @@ -73,7 +281,7 @@ func (s *serviceSuite) TestDeleteApplicationError(c *gc.C) { rErr := errors.New("boom") s.state.EXPECT().DeleteApplication(gomock.Any(), "666").Return(rErr) - err := NewService(s.state).DeleteApplication(context.Background(), "666") + err := s.service.DeleteApplication(context.Background(), "666") c.Check(err, jc.ErrorIs, rErr) c.Assert(err, gc.ErrorMatches, `deleting application "666": boom`) } @@ -89,7 +297,7 @@ func (s *serviceSuite) TestAddUnits(c *gc.C) { a := AddUnitParams{ UnitName: ptr("foo/666"), } - err := NewService(s.state).AddUnits(context.Background(), "666", a) + err := s.service.AddUnits(context.Background(), "666", a) c.Assert(err, jc.ErrorIsNil) } @@ -104,6 +312,6 @@ func (s *serviceSuite) TestAddUpsertCAASUnit(c *gc.C) { p := UpsertCAASUnitParams{ UnitName: ptr("foo/666"), } - err := NewService(s.state).UpsertCAASUnit(context.Background(), "foo", p) + err := s.service.UpsertCAASUnit(context.Background(), "foo", p) c.Assert(err, jc.ErrorIsNil) } diff --git a/domain/application/state/state.go b/domain/application/state/state.go index 0ac888108f4..3712d66d09a 100644 --- a/domain/application/state/state.go +++ b/domain/application/state/state.go @@ -8,6 +8,7 @@ import ( "fmt" "github.com/canonical/sqlair" + "github.com/juju/collections/transform" "github.com/juju/errors" coredb "github.com/juju/juju/core/database" @@ -15,6 +16,8 @@ import ( "github.com/juju/juju/domain/application" applicationerrors "github.com/juju/juju/domain/application/errors" "github.com/juju/juju/domain/life" + domainstorage "github.com/juju/juju/domain/storage" + storagestate "github.com/juju/juju/domain/storage/state" "github.com/juju/juju/internal/uuid" ) @@ -40,8 +43,8 @@ func NewState(factory coredb.TxnRunnerFactory, logger Logger) *State { // UpsertApplication creates or updates the specified application, // also adding any units if specified. // TODO - this just creates a minimal row for now. -func (s *State) UpsertApplication(ctx context.Context, name string, units ...application.AddUnitParams) error { - db, err := s.DB() +func (st *State) UpsertApplication(ctx context.Context, name string, units ...application.AddUnitParams) error { + db, err := st.DB() if err != nil { return errors.Trace(err) } @@ -105,8 +108,8 @@ VALUES ($M.application_uuid, $M.name, $M.life_id) } // DeleteApplication deletes the specified application. -func (s *State) DeleteApplication(ctx context.Context, name string) error { - db, err := s.DB() +func (st *State) DeleteApplication(ctx context.Context, name string) error { + db, err := st.DB() if err != nil { return errors.Trace(err) } @@ -153,11 +156,11 @@ func (s *State) DeleteApplication(ctx context.Context, name string) error { } numUnits, _ := result["count"].(int64) if numUnits > 0 { - return fmt.Errorf("cannot delete application %q as it still has %d unit(s)%w", name, numUnits, errors.Hide(applicationerrors.HasUnits)) + return fmt.Errorf("cannot delete application %q as it still has %d unit(s)%w", name, numUnits, errors.Hide(applicationerrors.ApplicationHasUnits)) } if err := tx.Query(ctx, deleteApplicationStmt, appNameParam).Run(); err != nil { - return errors.Annotatef(err, "deleting application %q", name) + return errors.Trace(err) } return nil }) @@ -166,8 +169,8 @@ func (s *State) DeleteApplication(ctx context.Context, name string) error { // AddUnits adds the specified units to the application. // TODO - this just creates a minimal row for now. -func (s *State) AddUnits(ctx context.Context, applicationName string, args ...application.AddUnitParams) error { - db, err := s.DB() +func (st *State) AddUnits(ctx context.Context, applicationName string, args ...application.AddUnitParams) error { + db, err := st.DB() if err != nil { return errors.Trace(err) } @@ -237,7 +240,7 @@ VALUES ($M.unit_uuid, $M.net_node_uuid, $M.unit_id, $M.life_id, $M.application_u } } if len(result) == 0 { - return fmt.Errorf("application %q not found%w", applicationName, errors.Hide(applicationerrors.NotFound)) + return fmt.Errorf("application %q not found%w", applicationName, errors.Hide(applicationerrors.ApplicationNotFound)) } applicationUUID := result["uuid"].(string) @@ -277,3 +280,52 @@ VALUES ($M.unit_uuid, $M.net_node_uuid, $M.unit_id, $M.life_id, $M.application_u }, nil } + +// StorageDefaults returns the default storage sources for a model. +func (st *State) StorageDefaults(ctx context.Context) (domainstorage.StorageDefaults, error) { + rval := domainstorage.StorageDefaults{} + + db, err := st.DB() + if err != nil { + return rval, errors.Trace(err) + } + + attrs := []string{application.StorageDefaultBlockSourceKey, application.StorageDefaultFilesystemSourceKey} + attrsSlice := sqlair.S(transform.Slice(attrs, func(s string) any { return any(s) })) + stmt, err := sqlair.Prepare(` +SELECT &KeyValue.* FROM model_config WHERE key IN ($S[:]) +`, sqlair.S{}, KeyValue{}) + if err != nil { + return rval, errors.Trace(err) + } + + return rval, db.Txn(ctx, func(ctx context.Context, tx *sqlair.TX) error { + var values []KeyValue + err := tx.Query(ctx, stmt, attrsSlice).GetAll(&values) + if err != nil { + return fmt.Errorf("getting model config attrs for storage defaults: %w", err) + } + + for _, kv := range values { + switch k := kv.Key; k { + case application.StorageDefaultBlockSourceKey: + v := fmt.Sprint(kv.Value) + rval.DefaultBlockSource = &v + case application.StorageDefaultFilesystemSourceKey: + v := fmt.Sprint(kv.Value) + rval.DefaultFilesystemSource = &v + } + } + return nil + }) +} + +// GetStoragePoolByName returns the storage pool with the specified name, returning an error +// satisfying [storageerrors.PoolNotFoundError] if it doesn't exist. +func (st *State) GetStoragePoolByName(ctx context.Context, name string) (domainstorage.StoragePoolDetails, error) { + db, err := st.DB() + if err != nil { + return domainstorage.StoragePoolDetails{}, errors.Trace(err) + } + return storagestate.GetStoragePoolByName(ctx, db, name) +} diff --git a/domain/application/state/state_test.go b/domain/application/state/state_test.go index a0eaf0081d9..5f484c523af 100644 --- a/domain/application/state/state_test.go +++ b/domain/application/state/state_test.go @@ -14,6 +14,7 @@ import ( "github.com/juju/juju/domain/application" applicationerrors "github.com/juju/juju/domain/application/errors" schematesting "github.com/juju/juju/domain/schema/testing" + domainstorage "github.com/juju/juju/domain/storage" jujutesting "github.com/juju/juju/testing" ) @@ -143,7 +144,7 @@ func (s *stateSuite) TestDeleteApplicationWithUnits(c *gc.C) { c.Assert(err, jc.ErrorIsNil) err = s.state.DeleteApplication(context.Background(), "666") - c.Assert(err, jc.ErrorIs, applicationerrors.HasUnits) + c.Assert(err, jc.ErrorIs, applicationerrors.ApplicationHasUnits) c.Assert(err, gc.ErrorMatches, `.*cannot delete application "666" as it still has 1 unit\(s\)`) var appCount int @@ -196,5 +197,28 @@ func (s *stateSuite) TestAddUnitsMissingApplication(c *gc.C) { UnitName: ptr("foo/666"), } err := s.state.AddUnits(context.Background(), "666", u) - c.Assert(err, jc.ErrorIs, applicationerrors.NotFound) + c.Assert(err, jc.ErrorIs, applicationerrors.ApplicationNotFound) +} + +func (s *stateSuite) TestStorageDefaultsNone(c *gc.C) { + defaults, err := s.state.StorageDefaults(context.Background()) + c.Assert(err, jc.ErrorIsNil) + c.Assert(defaults, jc.DeepEquals, domainstorage.StorageDefaults{}) +} + +func (s *stateSuite) TestStorageDefaults(c *gc.C) { + db := s.DB() + _, err := db.ExecContext(context.Background(), "INSERT INTO model_config (key, value) VALUES (?, ?)", + "storage-default-block-source", "ebs-fast") + c.Assert(err, jc.ErrorIsNil) + _, err = db.ExecContext(context.Background(), "INSERT INTO model_config (key, value) VALUES (?, ?)", + "storage-default-filesystem-source", "elastic-fs") + c.Assert(err, jc.ErrorIsNil) + + defaults, err := s.state.StorageDefaults(context.Background()) + c.Assert(err, jc.ErrorIsNil) + c.Assert(defaults, jc.DeepEquals, domainstorage.StorageDefaults{ + DefaultBlockSource: ptr("ebs-fast"), + DefaultFilesystemSource: ptr("elastic-fs"), + }) } diff --git a/domain/application/state/types.go b/domain/application/state/types.go new file mode 100644 index 00000000000..e41092b6b22 --- /dev/null +++ b/domain/application/state/types.go @@ -0,0 +1,11 @@ +// Copyright 2024 Canonical Ltd. +// Licensed under the AGPLv3, see LICENCE file for details. + +package state + +// These structs represent the persistent block device entity schema in the database. + +type KeyValue struct { + Key string `db:"key"` + Value string `db:"value"` +} diff --git a/domain/modelmigration/import.go b/domain/modelmigration/import.go index 1e1403d8fff..0d22b4407b9 100644 --- a/domain/modelmigration/import.go +++ b/domain/modelmigration/import.go @@ -39,7 +39,8 @@ func ImportOperations(coordinator Coordinator, logger Logger, registry internals credential.RegisterImport(coordinator) model.RegisterImport(coordinator) machine.RegisterImport(coordinator) - application.RegisterImport(coordinator) + application.RegisterImport(coordinator, registry) blockdevice.RegisterImport(coordinator) + // TODO(storage) - we need to break out storage pools and import BEFORE applications. storage.RegisterImport(coordinator, registry) } diff --git a/domain/servicefactory/model.go b/domain/servicefactory/model.go index 8d6e9b408d2..767905bfda7 100644 --- a/domain/servicefactory/model.go +++ b/domain/servicefactory/model.go @@ -86,11 +86,13 @@ func (s *ModelFactory) BlockDevice() *blockdeviceservice.WatchableService { } // Application returns the model's application service. -func (s *ModelFactory) Application() *applicationservice.Service { +func (s *ModelFactory) Application(registry storage.ProviderRegistry) *applicationservice.Service { return applicationservice.NewService( applicationstate.NewState(changestream.NewTxnRunnerFactory(s.modelDB), s.logger.Child("application"), ), + s.logger.Child("application"), + registry, ) } diff --git a/domain/servicefactory/testing/service.go b/domain/servicefactory/testing/service.go index 403e92e65e6..0ace7345452 100644 --- a/domain/servicefactory/testing/service.go +++ b/domain/servicefactory/testing/service.go @@ -159,7 +159,7 @@ func (s *TestingServiceFactory) BlockDevice() *blockdeviceservice.WatchableServi } // Application returns the block device service. -func (s *TestingServiceFactory) Application() *applicationservice.Service { +func (s *TestingServiceFactory) Application(storage.ProviderRegistry) *applicationservice.Service { if s.applicationServiceGetter == nil { return nil } diff --git a/domain/storage/defaults.go b/domain/storage/defaults.go new file mode 100644 index 00000000000..f06f7b6c2c4 --- /dev/null +++ b/domain/storage/defaults.go @@ -0,0 +1,151 @@ +// Copyright 2024 Canonical Ltd. +// Licensed under the AGPLv3, see LICENCE file for details. + +package storage + +import ( + "fmt" + + "github.com/juju/charm/v13" + "github.com/juju/errors" + + k8sconstants "github.com/juju/juju/caas/kubernetes/provider/constants" + coremodel "github.com/juju/juju/core/model" + storageerrors "github.com/juju/juju/domain/storage/errors" + "github.com/juju/juju/internal/storage" + "github.com/juju/juju/internal/storage/provider" +) + +// StorageDefaults holds the default sources of storage for an application. +type StorageDefaults struct { + DefaultBlockSource *string + DefaultFilesystemSource *string +} + +func storageKind(storageType charm.StorageType) storage.StorageKind { + kind := storage.StorageKindUnknown + switch storageType { + case charm.StorageBlock: + kind = storage.StorageKindBlock + case charm.StorageFilesystem: + kind = storage.StorageKindFilesystem + } + return kind +} + +// StorageConstraintsWithDefaults returns a constraints +// derived from cons, with any defaults filled in. +func StorageConstraintsWithDefaults( + charmStorage map[string]charm.Storage, + modelType coremodel.ModelType, + defaults StorageDefaults, + allCons map[string]storage.Constraints, +) error { + for name, storage := range charmStorage { + cons, ok := allCons[name] + if !ok { + if storage.Shared { + // TODO(axw) get the model's default shared storage pool, and create constraints here. + return fmt.Errorf( + "%w for shared charm storage %q", + storageerrors.MissingSharedStorageConstraintError, + name, + ) + } + } + cons, err := storageConstraintsWithDefaults(storage, modelType, defaults, cons) + if err != nil { + return errors.Trace(err) + } + // Replace in case pool or size were updated. + allCons[name] = cons + } + return nil +} + +func storageConstraintsWithDefaults( + charmStorage charm.Storage, + modelType coremodel.ModelType, + defaults StorageDefaults, + cons storage.Constraints, +) (storage.Constraints, error) { + withDefaults := cons + + // If no pool is specified, determine the pool from the env config and other constraints. + if cons.Pool == "" { + kind := storageKind(charmStorage.Type) + poolName, err := defaultStoragePool(defaults, modelType, kind, cons) + if err != nil { + return withDefaults, errors.Annotatef(err, "finding default pool for %q storage", charmStorage.Name) + } + withDefaults.Pool = poolName + } + + // If no size is specified, we default to the min size specified by the + // charm, or 1GiB. + if cons.Size == 0 { + if charmStorage.MinimumSize > 0 { + withDefaults.Size = charmStorage.MinimumSize + } else { + withDefaults.Size = 1024 + } + } + if cons.Count == 0 { + withDefaults.Count = uint64(charmStorage.CountMin) + } + return withDefaults, nil +} + +func storagePool(pool *string, fallbacks ...*string) string { + if pool != nil { + return *pool + } + for _, f := range fallbacks { + if f == nil { + continue + } + return *f + } + return "" +} + +// defaultStoragePool returns the default storage pool for the model. +// The default pool is either user specified, or one that is registered by the provider itself. +func defaultStoragePool( + defaults StorageDefaults, + modelType coremodel.ModelType, + kind storage.StorageKind, + cons storage.Constraints, +) (string, error) { + switch kind { + case storage.StorageKindBlock: + fallbackPool := string(provider.LoopProviderType) + if modelType == coremodel.CAAS { + fallbackPool = string(k8sconstants.StorageProviderType) + } + + emptyConstraints := storage.Constraints{} + if cons == emptyConstraints { + // No constraints at all: use fallback. + return fallbackPool, nil + } + // Either size or count specified, use env default. + return storagePool(defaults.DefaultBlockSource, &fallbackPool), nil + + case storage.StorageKindFilesystem: + fallbackPool := string(provider.RootfsProviderType) + if modelType == coremodel.CAAS { + fallbackPool = string(k8sconstants.StorageProviderType) + } + emptyConstraints := storage.Constraints{} + if cons == emptyConstraints { + return fallbackPool, nil + } + + // If a filesystem source is specified in config, + // use that; otherwise if a block source is specified, + // use that and create a filesystem within. + return storagePool(defaults.DefaultFilesystemSource, defaults.DefaultBlockSource, &fallbackPool), nil + } + return "", storageerrors.ErrNoDefaultStoragePool +} diff --git a/domain/storage/defaults_test.go b/domain/storage/defaults_test.go new file mode 100644 index 00000000000..8ca50bb7081 --- /dev/null +++ b/domain/storage/defaults_test.go @@ -0,0 +1,246 @@ +// Copyright 2024 Canonical Ltd. +// Licensed under the AGPLv3, see LICENCE file for details. + +package storage_test + +import ( + "github.com/juju/charm/v13" + "github.com/juju/testing" + jc "github.com/juju/testing/checkers" + gc "gopkg.in/check.v1" + + coremodel "github.com/juju/juju/core/model" + domainstorage "github.com/juju/juju/domain/storage" + "github.com/juju/juju/internal/storage" +) + +type defaultsSuite struct { + testing.IsolationSuite +} + +var _ = gc.Suite(&defaultsSuite{}) + +func makeStorageDefaults(b, f string) domainstorage.StorageDefaults { + var result domainstorage.StorageDefaults + if b != "" { + result.DefaultBlockSource = &b + } + if f != "" { + result.DefaultFilesystemSource = &f + } + return result +} + +func (s *defaultsSuite) assertAddApplicationStorageConstraintsDefaults(c *gc.C, pool string, cons, expect map[string]storage.Constraints) { + err := domainstorage.StorageConstraintsWithDefaults( + map[string]charm.Storage{ + "data": {Name: "data", Type: charm.StorageBlock, CountMin: 1, CountMax: -1}, + "allecto": {Name: "allecto", Type: charm.StorageBlock, CountMin: 0, CountMax: -1}, + }, + coremodel.IAAS, + makeStorageDefaults(pool, ""), + cons, + ) + c.Assert(err, jc.ErrorIsNil) + c.Assert(cons, jc.DeepEquals, expect) +} + +func (s *defaultsSuite) TestAddApplicationStorageConstraintsNoConstraintsUsed(c *gc.C) { + storageCons := map[string]storage.Constraints{ + "data": makeStorageCons("", 0, 0), + } + expectedCons := map[string]storage.Constraints{ + "data": makeStorageCons("loop", 1024, 1), + "allecto": makeStorageCons("loop", 1024, 0), + } + s.assertAddApplicationStorageConstraintsDefaults(c, "loop-pool", storageCons, expectedCons) +} + +func (s *defaultsSuite) TestAddApplicationStorageConstraintsJustCount(c *gc.C) { + storageCons := map[string]storage.Constraints{ + "data": makeStorageCons("", 0, 1), + } + expectedCons := map[string]storage.Constraints{ + "data": makeStorageCons("loop-pool", 1024, 1), + "allecto": makeStorageCons("loop", 1024, 0), + } + s.assertAddApplicationStorageConstraintsDefaults(c, "loop-pool", storageCons, expectedCons) +} + +func (s *defaultsSuite) TestAddApplicationStorageConstraintsDefaultPool(c *gc.C) { + storageCons := map[string]storage.Constraints{ + "data": makeStorageCons("", 2048, 1), + } + expectedCons := map[string]storage.Constraints{ + "data": makeStorageCons("loop-pool", 2048, 1), + "allecto": makeStorageCons("loop", 1024, 0), + } + s.assertAddApplicationStorageConstraintsDefaults(c, "loop-pool", storageCons, expectedCons) +} + +func (s *defaultsSuite) TestAddApplicationStorageConstraintsConstraintPool(c *gc.C) { + storageCons := map[string]storage.Constraints{ + "data": makeStorageCons("loop-pool", 2048, 1), + } + expectedCons := map[string]storage.Constraints{ + "data": makeStorageCons("loop-pool", 2048, 1), + "allecto": makeStorageCons("loop", 1024, 0), + } + s.assertAddApplicationStorageConstraintsDefaults(c, "", storageCons, expectedCons) +} + +func (s *defaultsSuite) TestAddApplicationStorageConstraintsNoUserDefaultPool(c *gc.C) { + storageCons := map[string]storage.Constraints{ + "data": makeStorageCons("", 2048, 1), + } + expectedCons := map[string]storage.Constraints{ + "data": makeStorageCons("loop", 2048, 1), + "allecto": makeStorageCons("loop", 1024, 0), + } + s.assertAddApplicationStorageConstraintsDefaults(c, "", storageCons, expectedCons) +} + +func (s *defaultsSuite) TestAddApplicationStorageConstraintsDefaultSizeFallback(c *gc.C) { + storageCons := map[string]storage.Constraints{ + "data": makeStorageCons("loop-pool", 0, 1), + } + expectedCons := map[string]storage.Constraints{ + "data": makeStorageCons("loop-pool", 1024, 1), + "allecto": makeStorageCons("loop", 1024, 0), + } + s.assertAddApplicationStorageConstraintsDefaults(c, "loop-pool", storageCons, expectedCons) +} + +func (s *defaultsSuite) TestAddApplicationStorageConstraintsDefaultSizeFromCharm(c *gc.C) { + storageCons := map[string]storage.Constraints{ + "multi1to10": makeStorageCons("loop", 0, 3), + } + expectedCons := map[string]storage.Constraints{ + "multi1to10": makeStorageCons("loop", 1024, 3), + "multi2up": makeStorageCons("loop", 2048, 2), + } + err := domainstorage.StorageConstraintsWithDefaults( + map[string]charm.Storage{ + "multi1to10": {Name: "multi1to10", Type: charm.StorageBlock, CountMin: 1, CountMax: 10}, + "multi2up": {Name: "multi2up", Type: charm.StorageBlock, CountMin: 2, CountMax: -1, MinimumSize: 2 * 1024}, + }, + coremodel.IAAS, + makeStorageDefaults("", ""), + storageCons, + ) + c.Assert(err, jc.ErrorIsNil) + c.Assert(storageCons, jc.DeepEquals, expectedCons) +} + +func (s *defaultsSuite) TestProviderFallbackToType(c *gc.C) { + storageCons := map[string]storage.Constraints{} + expectedCons := map[string]storage.Constraints{ + "data": makeStorageCons("loop", 1024, 1), + "files": makeStorageCons("rootfs", 1024, 1), + } + err := domainstorage.StorageConstraintsWithDefaults( + map[string]charm.Storage{ + "data": {Name: "data", Type: charm.StorageBlock, CountMin: 1, CountMax: 1}, + "files": {Name: "files", Type: charm.StorageFilesystem, CountMin: 1, CountMax: 1}, + }, + coremodel.IAAS, + makeStorageDefaults("", ""), + storageCons, + ) + c.Assert(err, jc.ErrorIsNil) + c.Assert(storageCons, jc.DeepEquals, expectedCons) +} + +func (s *defaultsSuite) TestProviderFallbackToTypeCaas(c *gc.C) { + storageCons := map[string]storage.Constraints{} + expectedCons := map[string]storage.Constraints{ + "files": makeStorageCons("kubernetes", 1024, 1), + } + err := domainstorage.StorageConstraintsWithDefaults( + map[string]charm.Storage{ + "files": {Name: "files", Type: charm.StorageFilesystem, CountMin: 1, CountMax: 1}, + }, + coremodel.CAAS, + makeStorageDefaults("", ""), + storageCons, + ) + c.Assert(err, jc.ErrorIsNil) + c.Assert(storageCons, jc.DeepEquals, expectedCons) +} + +func (s *defaultsSuite) TestProviderFallbackToTypeWithoutConstraints(c *gc.C) { + storageCons := map[string]storage.Constraints{} + expectedCons := map[string]storage.Constraints{ + "data": makeStorageCons("loop", 1024, 1), + "files": makeStorageCons("rootfs", 1024, 1), + } + err := domainstorage.StorageConstraintsWithDefaults( + map[string]charm.Storage{ + "data": {Name: "data", Type: charm.StorageBlock, CountMin: 1, CountMax: 1}, + "files": {Name: "files", Type: charm.StorageFilesystem, CountMin: 1, CountMax: 1}, + }, + coremodel.IAAS, + makeStorageDefaults("ebs", "tmpfs"), + storageCons, + ) + c.Assert(err, jc.ErrorIsNil) + c.Assert(storageCons, jc.DeepEquals, expectedCons) +} + +func (s *defaultsSuite) TestProviderFallbackToTypeWithoutConstraintsCaas(c *gc.C) { + storageCons := map[string]storage.Constraints{} + expectedCons := map[string]storage.Constraints{ + "files": makeStorageCons("kubernetes", 1024, 1), + } + err := domainstorage.StorageConstraintsWithDefaults( + map[string]charm.Storage{ + "files": {Name: "files", Type: charm.StorageFilesystem, CountMin: 1, CountMax: 1}, + }, + coremodel.CAAS, + makeStorageDefaults("", "tmpfs"), + storageCons, + ) + c.Assert(err, jc.ErrorIsNil) + c.Assert(storageCons, jc.DeepEquals, expectedCons) +} + +func (s *defaultsSuite) TestProviderFallbackToDefaults(c *gc.C) { + storageCons := map[string]storage.Constraints{ + "data": makeStorageCons("", 2048, 1), + "files": makeStorageCons("", 4096, 2), + } + expectedCons := map[string]storage.Constraints{ + "data": makeStorageCons("ebs", 2048, 1), + "files": makeStorageCons("tmpfs", 4096, 2), + } + err := domainstorage.StorageConstraintsWithDefaults( + map[string]charm.Storage{ + "data": {Name: "data", Type: charm.StorageBlock, CountMin: 1, CountMax: 2}, + "files": {Name: "files", Type: charm.StorageFilesystem, CountMin: 1, CountMax: 2}, + }, + coremodel.IAAS, + makeStorageDefaults("ebs", "tmpfs"), + storageCons, + ) + c.Assert(err, jc.ErrorIsNil) + c.Assert(storageCons, jc.DeepEquals, expectedCons) +} + +func (s *defaultsSuite) TestProviderFallbackToDefaultsCaas(c *gc.C) { + storageCons := map[string]storage.Constraints{ + "files": makeStorageCons("", 4096, 2), + } + expectedCons := map[string]storage.Constraints{ + "files": makeStorageCons("tmpfs", 4096, 2), + } + err := domainstorage.StorageConstraintsWithDefaults( + map[string]charm.Storage{ + "files": {Name: "files", Type: charm.StorageFilesystem, CountMin: 1, CountMax: 2}, + }, + coremodel.CAAS, + makeStorageDefaults("", "tmpfs"), + storageCons, + ) + c.Assert(err, jc.ErrorIsNil) + c.Assert(storageCons, jc.DeepEquals, expectedCons) +} diff --git a/domain/storage/errors/errors.go b/domain/storage/errors/errors.go index 604ce954638..6c4e6cc6204 100644 --- a/domain/storage/errors/errors.go +++ b/domain/storage/errors/errors.go @@ -7,15 +7,24 @@ import ( "github.com/juju/errors" ) +// These errors are used for storage pool operations. const ( // MissingPoolTypeError is used when a provider type is empty. MissingPoolTypeError = errors.ConstError("pool provider type is empty") // MissingPoolNameError is used when a name is empty. MissingPoolNameError = errors.ConstError("pool name is empty") - + // InvalidPoolNameError is used when a storage pool name is invalid. InvalidPoolNameError = errors.ConstError("pool name is not valid") - + // PoolNotFoundError is used when a storage pool is not found. PoolNotFoundError = errors.ConstError("storage pool is not found") - + // PoolAlreadyExists is used when a storage pool already exists. PoolAlreadyExists = errors.ConstError("storage pool already exists") + // ErrNoDefaultStoragePool is returned when a storage pool is required but none is specified nor available as a default. + ErrNoDefaultStoragePool = errors.ConstError("no storage pool specified and no default available") +) + +// These errors are used for storage constraints operations. +const ( + // MissingSharedStorageConstraintError is used when a storage constraint for shared storage is not provided. + MissingSharedStorageConstraintError = errors.ConstError("no storage constraints specified") ) diff --git a/domain/storage/state/storagepool.go b/domain/storage/state/storagepool.go index 0cd7ce67463..de42b2fa48e 100644 --- a/domain/storage/state/storagepool.go +++ b/domain/storage/state/storagepool.go @@ -272,7 +272,7 @@ func (st StoragePoolState) ReplaceStoragePool(ctx context.Context, pool domainst type loadStoragePoolsFunc func(ctx context.Context, tx *sqlair.TX) ([]domainstorage.StoragePoolDetails, error) -func (st StoragePoolState) storagePoolsLoader(wantNames domainstorage.Names, wantProviders domainstorage.Providers) (loadStoragePoolsFunc, error) { +func storagePoolsLoader(wantNames domainstorage.Names, wantProviders domainstorage.Providers) (loadStoragePoolsFunc, error) { query := ` SELECT (sp.uuid, sp.name, sp.type) AS (&StoragePool.*), (sp_attr.key, sp_attr.value) AS (&poolAttribute.*) @@ -286,7 +286,7 @@ FROM storage_pool sp } var queryArgs []any - condition, args := st.buildFilter(wantNames, wantProviders) + condition, args := buildStoragePoolsFilter(wantNames, wantProviders) if len(args) > 0 { query = query + "WHERE " + condition types = append(types, args...) @@ -318,7 +318,7 @@ func (st StoragePoolState) ListStoragePools(ctx context.Context, names domainsto return nil, errors.Trace(err) } - storagePoolsLoader, err := st.storagePoolsLoader(names, providers) + storagePoolsLoader, err := storagePoolsLoader(names, providers) if err != nil { return nil, errors.Trace(err) } @@ -332,7 +332,7 @@ func (st StoragePoolState) ListStoragePools(ctx context.Context, names domainsto return result, errors.Trace(domain.CoerceError(err)) } -func (st StoragePoolState) buildFilter(wantNames domainstorage.Names, wantProviders domainstorage.Providers) (string, []any) { +func buildStoragePoolsFilter(wantNames domainstorage.Names, wantProviders domainstorage.Providers) (string, []any) { if len(wantNames) == 0 && len(wantProviders) == 0 { return "", nil } @@ -358,8 +358,14 @@ func (st StoragePoolState) GetStoragePoolByName(ctx context.Context, name string if err != nil { return domainstorage.StoragePoolDetails{}, errors.Trace(err) } + return GetStoragePoolByName(ctx, db, name) +} - storagePoolsLoader, err := st.storagePoolsLoader(domainstorage.Names{name}, nil) +// GetStoragePoolByName returns the storage pool with the specified name, returning an error +// satisfying [storageerrors.PoolNotFoundError] if it doesn't exist. +// Exported for use by other domains that need to load storage pools. +func GetStoragePoolByName(ctx context.Context, db coredatabase.TxnRunner, name string) (domainstorage.StoragePoolDetails, error) { + storagePoolsLoader, err := storagePoolsLoader(domainstorage.Names{name}, nil) if err != nil { return domainstorage.StoragePoolDetails{}, errors.Trace(err) } diff --git a/domain/storage/validation.go b/domain/storage/validation.go new file mode 100644 index 00000000000..b0e369e5f90 --- /dev/null +++ b/domain/storage/validation.go @@ -0,0 +1,188 @@ +// Copyright 2024 Canonical Ltd. +// Licensed under the AGPLv3, see LICENCE file for details. + +package storage + +import ( + "context" + + "github.com/dustin/go-humanize" + "github.com/juju/charm/v13" + "github.com/juju/collections/transform" + "github.com/juju/errors" + + coremodel "github.com/juju/juju/core/model" + storageerrors "github.com/juju/juju/domain/storage/errors" + "github.com/juju/juju/internal/storage" +) + +// StoragePoolGetter provides access to a storage pool getter for validation purposes. +type StoragePoolGetter interface { + GetStoragePoolByName(ctx context.Context, name string) (StoragePoolDetails, error) +} + +// Charm provides access to charm metadata. +type Charm interface { + Meta() *charm.Meta +} + +// StorageConstraintsValidator instances can be used to check storage +// constraints are compatible with a given charm. +type StorageConstraintsValidator interface { + ValidateStorageConstraintsAgainstCharm(ctx context.Context, allCons map[string]storage.Constraints, charm Charm) error +} + +// NewStorageConstraintsValidator creates a validator that can be used to check storage +// constraints are compatible with a given charm. +func NewStorageConstraintsValidator(modelType coremodel.ModelType, registry storage.ProviderRegistry, storagePoolGetter StoragePoolGetter) (*storageConstraintsValidator, error) { + // This should never happen, but we'll be defensive. + if registry == nil { + return nil, errors.New("cannot create storage constraints validator with nil registry") + } + return &storageConstraintsValidator{ + registry: registry, + storagePoolGetter: storagePoolGetter, + modelType: modelType, + }, nil +} + +type storageConstraintsValidator struct { + registry storage.ProviderRegistry + storagePoolGetter StoragePoolGetter + modelType coremodel.ModelType +} + +// ValidateStorageConstraintsAgainstCharm validations storage constraints +// against a given charm and its storage metadata. +func (v storageConstraintsValidator) ValidateStorageConstraintsAgainstCharm( + ctx context.Context, + allCons map[string]storage.Constraints, + charm Charm, +) error { + charmMeta := charm.Meta() + // CAAS charms don't support volume/block storage yet. + if v.modelType == coremodel.CAAS { + for name, charmStorage := range charmMeta.Storage { + if storageKind(charmStorage.Type) != storage.StorageKindBlock { + continue + } + var count uint64 + if arg, ok := allCons[name]; ok { + count = arg.Count + } + if charmStorage.CountMin > 0 || count > 0 { + return errors.NotSupportedf("block storage on a container model") + } + } + } + + for name, cons := range allCons { + charmStorage, ok := charmMeta.Storage[name] + if !ok { + return errors.Errorf("charm %q has no store called %q", charmMeta.Name, name) + } + if charmStorage.Shared { + // TODO(axw) implement shared storage support. + return errors.Errorf( + "charm %q store %q: shared storage support not implemented", + charmMeta.Name, name, + ) + } + if err := v.validateCharmStorageCount(charmStorage, cons.Count); err != nil { + return errors.Annotatef(err, "charm %q store %q", charmMeta.Name, name) + } + if charmStorage.MinimumSize > 0 && cons.Size < charmStorage.MinimumSize { + return errors.Errorf( + "charm %q store %q: minimum storage size is %s, %s specified", + charmMeta.Name, name, + humanize.Bytes(charmStorage.MinimumSize*humanize.MByte), + humanize.Bytes(cons.Size*humanize.MByte), + ) + } + kind := storageKind(charmStorage.Type) + if err := v.validateStoragePool(ctx, cons.Pool, kind); err != nil { + return err + } + } + return nil +} + +func (v storageConstraintsValidator) validateCharmStorageCount(charmStorage charm.Storage, count uint64) error { + if charmStorage.CountMin == 1 && charmStorage.CountMax == 1 && count != 1 { + return errors.Errorf("storage is singular, %d specified", count) + } + if count < uint64(charmStorage.CountMin) { + return errors.Errorf( + "%d instances required, %d specified", + charmStorage.CountMin, count, + ) + } + if charmStorage.CountMax >= 0 && count > uint64(charmStorage.CountMax) { + return errors.Errorf( + "at most %d instances supported, %d specified", + charmStorage.CountMax, count, + ) + } + return nil +} + +// validateStoragePool validates the storage pool for the model. +func (v storageConstraintsValidator) validateStoragePool( + ctx context.Context, + poolName string, kind storage.StorageKind, +) error { + if poolName == "" { + return errors.New("pool name is required") + } + providerType, aProvider, poolConfig, err := v.poolStorageProvider(ctx, poolName) + if err != nil { + return errors.Trace(err) + } + + // Ensure the storage provider supports the specified kind. + kindSupported := aProvider.Supports(kind) + if !kindSupported && kind == storage.StorageKindFilesystem { + // Filesystems can be created if either filesystem + // or block storage are supported. The scope of the + // filesystem is the same as the backing volume. + kindSupported = aProvider.Supports(storage.StorageKindBlock) + } + if !kindSupported { + return errors.Errorf("%q provider does not support %q storage", providerType, kind) + } + + if v.modelType == coremodel.CAAS { + if err := aProvider.ValidateForK8s(poolConfig); err != nil { + return errors.Annotatef(err, "invalid storage config") + } + } + return nil +} + +func (v storageConstraintsValidator) poolStorageProvider( + ctx context.Context, + poolName string, +) (storage.ProviderType, storage.Provider, storage.Attrs, error) { + pool, err := v.storagePoolGetter.GetStoragePoolByName(ctx, poolName) + if errors.Is(err, storageerrors.PoolNotFoundError) { + // If there's no pool called poolName, maybe a provider type + // has been specified directly. + providerType := storage.ProviderType(poolName) + aProvider, err1 := v.registry.StorageProvider(providerType) + if err1 != nil { + // The name can't be resolved as a storage provider type, + // so return the original "pool not found" error. + return "", nil, nil, errors.Trace(err) + } + return providerType, aProvider, nil, nil + } else if err != nil { + return "", nil, nil, errors.Trace(err) + } + providerType := storage.ProviderType(pool.Provider) + aProvider, err := v.registry.StorageProvider(providerType) + if err != nil { + return "", nil, nil, errors.Trace(err) + } + attrs := transform.Map(pool.Attrs, func(k, v string) (string, any) { return k, v }) + return providerType, aProvider, attrs, nil +} diff --git a/domain/storage/validation_test.go b/domain/storage/validation_test.go new file mode 100644 index 00000000000..77ea758910f --- /dev/null +++ b/domain/storage/validation_test.go @@ -0,0 +1,172 @@ +// Copyright 2024 Canonical Ltd. +// Licensed under the AGPLv3, see LICENCE file for details. + +package storage_test + +import ( + "context" + "fmt" + + "github.com/juju/charm/v13" + "github.com/juju/errors" + "github.com/juju/testing" + jc "github.com/juju/testing/checkers" + gc "gopkg.in/check.v1" + + coremodel "github.com/juju/juju/core/model" + domainstorage "github.com/juju/juju/domain/storage" + storageerrors "github.com/juju/juju/domain/storage/errors" + "github.com/juju/juju/internal/storage" + "github.com/juju/juju/internal/storage/provider" +) + +type validationSuite struct { + testing.IsolationSuite + + modelType coremodel.ModelType + charm mockCharm +} + +var _ = gc.Suite(&validationSuite{}) + +func (s *validationSuite) SetUpTest(_ *gc.C) { + s.modelType = coremodel.IAAS + s.charm.meta = &charm.Meta{ + Name: "storage-block2", + Storage: map[string]charm.Storage{ + "multi1to10": { + Name: "multi1to10", + Type: charm.StorageBlock, + CountMin: 1, + CountMax: 10, + }, + "multi2up": { + Name: "multi2up", + Type: charm.StorageBlock, + CountMin: 2, + CountMax: -1, + MinimumSize: 2 * 1024, + }, + }, + } +} + +func makeStorageCons(pool string, size, count uint64) storage.Constraints { + return storage.Constraints{ + Pool: pool, + Size: size, + Count: count, + } +} + +type mockStoragePoolGetter struct{} + +func (mockStoragePoolGetter) GetStoragePoolByName(_ context.Context, name string) (domainstorage.StoragePoolDetails, error) { + switch name { + case "loop-pool": + return domainstorage.StoragePoolDetails{Name: name, Provider: "loop"}, nil + case "rootfs": + return domainstorage.StoragePoolDetails{Name: name, Provider: "rootfs"}, nil + case "tmp": + return domainstorage.StoragePoolDetails{Name: name, Provider: "tmpfs", Attrs: map[string]string{"storage-medium": "foo"}}, nil + } + return domainstorage.StoragePoolDetails{}, fmt.Errorf("storage pool %q not found%w", name, errors.Hide(storageerrors.PoolNotFoundError)) +} + +type mockCharm struct { + meta *charm.Meta +} + +func (m mockCharm) Meta() *charm.Meta { + return m.meta +} + +func (s *validationSuite) validateStorageConstraints(storage map[string]storage.Constraints) error { + validator, err := domainstorage.NewStorageConstraintsValidator(s.modelType, provider.CommonStorageProviders(), mockStoragePoolGetter{}) + if err != nil { + return errors.Trace(err) + } + return validator.ValidateStorageConstraintsAgainstCharm( + context.Background(), + storage, + s.charm, + ) +} + +func (s *validationSuite) TestNilRegistry(c *gc.C) { + _, err := domainstorage.NewStorageConstraintsValidator(s.modelType, nil, mockStoragePoolGetter{}) + c.Assert(err, gc.ErrorMatches, "cannot create storage constraints validator with nil registry") +} + +func (s *validationSuite) TestValidateStorageConstraintsAgainstCharmSuccess(c *gc.C) { + storageCons := map[string]storage.Constraints{ + "multi1to10": makeStorageCons("loop-pool", 1024, 10), + "multi2up": makeStorageCons("loop-pool", 2048, 2), + } + err := s.validateStorageConstraints(storageCons) + c.Assert(err, jc.ErrorIsNil) +} + +func (s *validationSuite) TestValidateStorageConstraintsAgainstCharmStoragePoolNotFound(c *gc.C) { + storageCons := map[string]storage.Constraints{ + "multi1to10": makeStorageCons("ebs-fast", 1024, 10), + "multi2up": makeStorageCons("loop-pool", 2048, 2), + } + err := s.validateStorageConstraints(storageCons) + c.Assert(err, gc.ErrorMatches, `storage pool "ebs-fast" not found`) + c.Assert(err, jc.ErrorIs, storageerrors.PoolNotFoundError) +} + +func (s *validationSuite) TestValidateStorageConstraintsAgainstCharmErrors(c *gc.C) { + assertErr := func(storage map[string]storage.Constraints, expect string) { + err := s.validateStorageConstraints(storage) + c.Assert(err, gc.ErrorMatches, expect) + } + + storageCons := map[string]storage.Constraints{ + "multi1to10": makeStorageCons("loop-pool", 1024, 1), + "multi2up": makeStorageCons("loop-pool", 2048, 1), + } + assertErr(storageCons, `charm "storage-block2" store "multi2up": 2 instances required, 1 specified`) + + storageCons["multi2up"] = makeStorageCons("loop-pool", 1024, 2) + assertErr(storageCons, `charm "storage-block2" store "multi2up": minimum storage size is 2.0 GB, 1.0 GB specified`) + + storageCons["multi2up"] = makeStorageCons("loop-pool", 2048, 2) + storageCons["multi1to10"] = makeStorageCons("loop-pool", 1024, 11) + assertErr(storageCons, `charm "storage-block2" store "multi1to10": at most 10 instances supported, 11 specified`) + + storageCons["multi1to10"] = makeStorageCons("rootfs", 1024, 1) + assertErr(storageCons, `"rootfs" provider does not support "block" storage`) +} + +func (s *validationSuite) TestValidateStorageConstraintsAgainstCharmCaasBlockNotSupported(c *gc.C) { + s.modelType = coremodel.CAAS + storageCons := map[string]storage.Constraints{ + "multi1to10": makeStorageCons("loop-pool", 1024, 1), + "multi2up": makeStorageCons("loop-pool", 2048, 2), + } + err := s.validateStorageConstraints(storageCons) + c.Assert(err, gc.ErrorMatches, `block storage on a container model not supported`) +} + +func (s *validationSuite) TestValidateStorageConstraintsAgainstCharmCaas(c *gc.C) { + s.modelType = coremodel.CAAS + s.charm.meta = &charm.Meta{ + Name: "storage-block2", + Storage: map[string]charm.Storage{ + "files": { + Name: "tmp", + Type: charm.StorageFilesystem, + CountMin: 1, + CountMax: -1, + }, + }, + } + + storageCons := map[string]storage.Constraints{ + "files": makeStorageCons("tmp", 2048, 1), + } + err := s.validateStorageConstraints(storageCons) + c.Assert(err, gc.ErrorMatches, `invalid storage config: storage medium "foo" not valid`) +} diff --git a/internal/bootstrap/deployer.go b/internal/bootstrap/deployer.go index b32747e4883..36a7686cf18 100644 --- a/internal/bootstrap/deployer.go +++ b/internal/bootstrap/deployer.go @@ -408,7 +408,9 @@ func (b *baseDeployer) AddControllerApplication(ctx context.Context, curl string return nil, errors.Trace(err) } unitName := bootstrap.ControllerApplicationName + "/0" - err = b.applicationService.CreateApplication(ctx, bootstrap.ControllerApplicationName, applicationservice.AddApplicationParams{}, applicationservice.AddUnitParams{UnitName: &unitName}) + err = b.applicationService.CreateApplication(ctx, bootstrap.ControllerApplicationName, applicationservice.AddApplicationParams{ + Charm: ch, + }, applicationservice.AddUnitParams{UnitName: &unitName}) if err != nil { return nil, errors.Trace(err) } diff --git a/internal/bootstrap/deployer_test.go b/internal/bootstrap/deployer_test.go index 378c56d0018..5f96fb6837c 100644 --- a/internal/bootstrap/deployer_test.go +++ b/internal/bootstrap/deployer_test.go @@ -261,7 +261,9 @@ func (s *deployerSuite) TestAddControllerApplication(c *gc.C) { unitName := bootstrap.ControllerApplicationName + "/0" s.application.EXPECT().Name().Return(bootstrap.ControllerApplicationName) s.stateBackend.EXPECT().Unit(unitName).Return(s.unit, nil) - s.applicationService.EXPECT().CreateApplication(gomock.Any(), bootstrap.ControllerApplicationName, applicationservice.AddApplicationParams{}, applicationservice.AddUnitParams{UnitName: &unitName}) + s.applicationService.EXPECT().CreateApplication(gomock.Any(), bootstrap.ControllerApplicationName, applicationservice.AddApplicationParams{ + Charm: s.charm, + }, applicationservice.AddUnitParams{UnitName: &unitName}) deployer := s.newBaseDeployer(c, cfg) diff --git a/internal/migration/migration_test.go b/internal/migration/migration_test.go index 1917f28aaec..9d660ecc1ec 100644 --- a/internal/migration/migration_test.go +++ b/internal/migration/migration_test.go @@ -296,7 +296,7 @@ func (s *ImportSuite) setupMocks(c *gc.C) *gomock.Controller { s.serviceFactory.EXPECT().Cloud().Return(nil).AnyTimes() s.serviceFactory.EXPECT().Credential().Return(nil).AnyTimes() s.serviceFactory.EXPECT().Machine().Return(nil) - s.serviceFactory.EXPECT().Application().Return(nil) + s.serviceFactory.EXPECT().Application(gomock.Any()).Return(nil) s.serviceFactoryGetter = NewMockServiceFactoryGetter(ctrl) s.serviceFactoryGetter.EXPECT().FactoryForModel("bd3fae18-5ea1-4bc5-8837-45400cf1f8f6").Return(s.serviceFactory) diff --git a/internal/migration/servicefactory_mock_test.go b/internal/migration/servicefactory_mock_test.go index 7e2c43c3f78..ea22f07e598 100644 --- a/internal/migration/servicefactory_mock_test.go +++ b/internal/migration/servicefactory_mock_test.go @@ -127,17 +127,17 @@ func (mr *MockServiceFactoryMockRecorder) Annotation() *gomock.Call { } // Application mocks base method. -func (m *MockServiceFactory) Application() *service0.Service { +func (m *MockServiceFactory) Application(arg0 storage.ProviderRegistry) *service0.Service { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "Application") + ret := m.ctrl.Call(m, "Application", arg0) ret0, _ := ret[0].(*service0.Service) return ret0 } // Application indicates an expected call of Application. -func (mr *MockServiceFactoryMockRecorder) Application() *gomock.Call { +func (mr *MockServiceFactoryMockRecorder) Application(arg0 any) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Application", reflect.TypeOf((*MockServiceFactory)(nil).Application)) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Application", reflect.TypeOf((*MockServiceFactory)(nil).Application), arg0) } // AutocertCache mocks base method. diff --git a/internal/servicefactory/interface.go b/internal/servicefactory/interface.go index 9a865e71f92..1706baa7386 100644 --- a/internal/servicefactory/interface.go +++ b/internal/servicefactory/interface.go @@ -72,8 +72,8 @@ type ModelServiceFactory interface { Machine() *machineservice.Service // BlockDevice returns the block device service. BlockDevice() *blockdeviceservice.WatchableService - // Application returns the machine service. - Application() *applicationservice.Service + // Application returns the application service. + Application(registry storage.ProviderRegistry) *applicationservice.Service // Unit returns the machine service. Unit() *unitservice.Service // Space returns the space service. diff --git a/internal/storage/registries.go b/internal/storage/registries.go index c98613f805d..91b0a11d1d4 100644 --- a/internal/storage/registries.go +++ b/internal/storage/registries.go @@ -71,3 +71,17 @@ func (r StaticProviderRegistry) StorageProvider(t ProviderType) (Provider, error } return nil, errors.NotFoundf("storage provider %q", t) } + +// NotImplementedProviderRegistry is a storage provider registry that +// returns an error satisfying [errors.NotImplemented] for any method call. +type NotImplementedProviderRegistry struct{} + +// StorageProviderTypes implements ProviderRegistry. +func (r NotImplementedProviderRegistry) StorageProviderTypes() ([]ProviderType, error) { + return nil, errors.NotImplementedf(`"StorageProviderTypes"`) +} + +// StorageProvider implements ProviderRegistry. +func (r NotImplementedProviderRegistry) StorageProvider(t ProviderType) (Provider, error) { + return nil, errors.NotImplementedf(`"StorageProvider"`) +} diff --git a/internal/worker/bootstrap/manifold.go b/internal/worker/bootstrap/manifold.go index 893a43ee210..80430130af5 100644 --- a/internal/worker/bootstrap/manifold.go +++ b/internal/worker/bootstrap/manifold.go @@ -321,16 +321,18 @@ func Manifold(config ManifoldConfig) dependency.Manifold { return nil, errors.Trace(err) } - m, err := systemState.Model() + model, err := systemState.Model() if err != nil { + _ = stTracker.Done() return nil, errors.Trace(err) } registry, err := stateenvirons.NewStorageProviderRegistryForModel( - m, controllerServiceFactory.Cloud(), controllerServiceFactory.Credential(), + model, controllerServiceFactory.Cloud(), controllerServiceFactory.Credential(), stateenvirons.GetNewEnvironFunc(environs.New), stateenvirons.GetNewCAASBrokerFunc(caas.New), ) if err != nil { + _ = stTracker.Done() return nil, errors.Trace(err) } @@ -342,7 +344,7 @@ func Manifold(config ManifoldConfig) dependency.Manifold { CloudService: controllerServiceFactory.Cloud(), StorageService: modelServiceFactory.Storage(registry), ProviderRegistry: registry, - ApplicationService: modelServiceFactory.Application(), + ApplicationService: modelServiceFactory.Application(registry), FlagService: flagService, SpaceService: modelServiceFactory.Space(), SystemState: &stateShim{ diff --git a/internal/worker/bootstrap/worker.go b/internal/worker/bootstrap/worker.go index 40d90364b94..cb609bb1ece 100644 --- a/internal/worker/bootstrap/worker.go +++ b/internal/worker/bootstrap/worker.go @@ -17,6 +17,7 @@ import ( "github.com/juju/juju/core/network" "github.com/juju/juju/core/objectstore" domainstorage "github.com/juju/juju/domain/storage" + storageerrors "github.com/juju/juju/domain/storage/errors" storageservice "github.com/juju/juju/domain/storage/service" "github.com/juju/juju/internal/bootstrap" "github.com/juju/juju/internal/cloudconfig/instancecfg" @@ -275,6 +276,10 @@ func (w *bootstrapWorker) seedStoragePools(ctx context.Context, poolParams map[s } for _, p := range storagePools { if err := w.cfg.StorageService.CreateStoragePool(ctx, p.Name(), p.Provider(), storageservice.PoolAttrs(p.Attrs())); err != nil { + // Allow for bootstrap worker to have been restarted. + if errors.Is(err, storageerrors.PoolAlreadyExists) { + continue + } return errors.Annotatef(err, "saving storage pool %q", p.Name()) } } diff --git a/internal/worker/servicefactory/servicefactory_mock_test.go b/internal/worker/servicefactory/servicefactory_mock_test.go index 5744fee5c0a..05bc6bbdc5d 100644 --- a/internal/worker/servicefactory/servicefactory_mock_test.go +++ b/internal/worker/servicefactory/servicefactory_mock_test.go @@ -281,17 +281,17 @@ func (mr *MockModelServiceFactoryMockRecorder) Annotation() *gomock.Call { } // Application mocks base method. -func (m *MockModelServiceFactory) Application() *service0.Service { +func (m *MockModelServiceFactory) Application(arg0 storage.ProviderRegistry) *service0.Service { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "Application") + ret := m.ctrl.Call(m, "Application", arg0) ret0, _ := ret[0].(*service0.Service) return ret0 } // Application indicates an expected call of Application. -func (mr *MockModelServiceFactoryMockRecorder) Application() *gomock.Call { +func (mr *MockModelServiceFactoryMockRecorder) Application(arg0 any) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Application", reflect.TypeOf((*MockModelServiceFactory)(nil).Application)) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Application", reflect.TypeOf((*MockModelServiceFactory)(nil).Application), arg0) } // BlockDevice mocks base method. @@ -444,17 +444,17 @@ func (mr *MockServiceFactoryMockRecorder) Annotation() *gomock.Call { } // Application mocks base method. -func (m *MockServiceFactory) Application() *service0.Service { +func (m *MockServiceFactory) Application(arg0 storage.ProviderRegistry) *service0.Service { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "Application") + ret := m.ctrl.Call(m, "Application", arg0) ret0, _ := ret[0].(*service0.Service) return ret0 } // Application indicates an expected call of Application. -func (mr *MockServiceFactoryMockRecorder) Application() *gomock.Call { +func (mr *MockServiceFactoryMockRecorder) Application(arg0 any) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Application", reflect.TypeOf((*MockServiceFactory)(nil).Application)) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Application", reflect.TypeOf((*MockServiceFactory)(nil).Application), arg0) } // AutocertCache mocks base method. diff --git a/juju/testing/apiserver.go b/juju/testing/apiserver.go index 7de9d446cf7..0abb9569349 100644 --- a/juju/testing/apiserver.go +++ b/juju/testing/apiserver.go @@ -67,6 +67,8 @@ import ( "github.com/juju/juju/internal/pubsub/centralhub" "github.com/juju/juju/internal/servicefactory" "github.com/juju/juju/internal/storage" + "github.com/juju/juju/internal/storage/provider" + "github.com/juju/juju/internal/storage/provider/dummy" "github.com/juju/juju/internal/uuid" "github.com/juju/juju/internal/worker/lease" wmultiwatcher "github.com/juju/juju/internal/worker/multiwatcher" @@ -533,17 +535,45 @@ func (s *ApiServerSuite) StatePool() *state.StatePool { // NewFactory returns a factory for the given model. func (s *ApiServerSuite) NewFactory(c *gc.C, modelUUID string) (*factory.Factory, func() bool) { + var ( + st *state.State + model *state.Model + releaser func() bool + err error + ) if modelUUID == s.ServiceFactorySuite.ControllerModelUUID.String() { - st, err := s.controller.SystemState() + st, err = s.controller.SystemState() + c.Assert(err, jc.ErrorIsNil) + model = s.ControllerModel(c) + releaser = func() bool { return true } + } else { + pooledSt, err := s.controller.GetState(names.NewModelTag(modelUUID)) + c.Assert(err, jc.ErrorIsNil) + releaser = pooledSt.Release + st = pooledSt.State + model, err = st.Model() c.Assert(err, jc.ErrorIsNil) - return factory.NewFactory(st, s.controller.StatePool()). - WithApplicationService(s.ServiceFactoryGetter(c).FactoryForModel(modelUUID).Application()), - func() bool { return true } } - st, err := s.controller.GetState(names.NewModelTag(modelUUID)) - c.Assert(err, jc.ErrorIsNil) - return factory.NewFactory(st.State, s.controller.StatePool()). - WithApplicationService(s.ServiceFactoryGetter(c).FactoryForModel(modelUUID).Application()), st.Release + + modelServiceFactory := s.ServiceFactoryGetter(c).FactoryForModel(modelUUID) + var registry storage.ProviderRegistry + if model.Type() == state.ModelTypeIAAS { + registry = storage.ChainedProviderRegistry{ + dummy.StorageProviders(), + provider.CommonStorageProviders(), + } + } else { + registry = storage.ChainedProviderRegistry{ + storage.StaticProviderRegistry{ + Providers: map[storage.ProviderType]storage.Provider{ + "kubernetes": &dummy.StorageProvider{}, + }, + }, + provider.CommonStorageProviders(), + } + } + return factory.NewFactory(st, s.controller.StatePool()). + WithApplicationService(modelServiceFactory.Application(registry)), releaser } // ControllerModelUUID returns the controller model uuid. diff --git a/testing/factory/factory.go b/testing/factory/factory.go index c16cc506edb..13310463c32 100644 --- a/testing/factory/factory.go +++ b/testing/factory/factory.go @@ -575,7 +575,18 @@ func (factory *Factory) MakeApplicationReturningPassword(c *gc.C, params *Applic err = application.SetPassword(params.Password) c.Assert(err, jc.ErrorIsNil) if factory.applicationService != nil { - err = factory.applicationService.CreateApplication(context.Background(), params.Name, applicationservice.AddApplicationParams{}) + cons := make(map[string]storage.Constraints) + for name, sc := range params.Storage { + cons[name] = storage.Constraints{ + Pool: sc.Pool, + Size: sc.Size, + Count: sc.Count, + } + } + err = factory.applicationService.CreateApplication(context.Background(), params.Name, applicationservice.AddApplicationParams{ + Charm: params.Charm, + Storage: cons, + }) c.Assert(err, jc.ErrorIsNil) }