From 9d9226bda5683d621a0e767213bf1bddb82398b3 Mon Sep 17 00:00:00 2001 From: Simon Richardson Date: Wed, 20 Mar 2024 16:13:17 +0000 Subject: [PATCH 1/2] Pass controller and model txn runners in bootstrap During bootstrap we need to access both the controller (global db) and the model (controller model db). To prevent the need for hacks to get that information, this revisits the bootstrap concerns and moves back to bootstrap options. The options take the controller and the model. Thus you can access any part of each model during bootstrap. --- agent/agentbootstrap/bootstrap.go | 39 ++-- agent/agentbootstrap/bootstrap_test.go | 16 +- .../agent/caasapplication/application_test.go | 2 +- cmd/jujud-controller/agent/agenttest/agent.go | 2 + cmd/jujud-controller/agent/bootstrap_test.go | 11 +- cmd/jujud/agent/agenttest/agent.go | 2 + core/model/model.go | 9 + domain/cloud/bootstrap/bootstrap.go | 13 +- domain/cloud/bootstrap/bootstrap_test.go | 14 +- .../controllerconfig/bootstrap/bootstrap.go | 16 +- .../bootstrap/bootstrap_test.go | 2 +- .../controllerconfig/controllerconfig_test.go | 2 +- domain/credential/bootstrap/bootstrap.go | 7 +- domain/credential/bootstrap/bootstrap_test.go | 4 +- domain/machine/bootstrap/bootstrap.go | 7 +- domain/machine/bootstrap/bootstrap_test.go | 2 +- domain/model/bootstrap/bootstrap.go | 15 +- domain/model/bootstrap/bootstrap_test.go | 12 +- domain/modelconfig/bootstrap/bootstrap.go | 11 +- .../modelconfig/bootstrap/bootstrap_test.go | 2 +- domain/servicefactory/testing/suite.go | 10 +- domain/testing/controllerconfigsuite.go | 15 +- domain/user/bootstrap/bootstrap.go | 62 +++--- domain/user/bootstrap/bootstrap_test.go | 4 +- internal/database/bootstrap.go | 205 ++++++++---------- internal/database/bootstrap_test.go | 7 +- internal/database/testing/dqlitesuite.go | 27 +++ juju/testing/apiserver.go | 19 +- 28 files changed, 287 insertions(+), 250 deletions(-) diff --git a/agent/agentbootstrap/bootstrap.go b/agent/agentbootstrap/bootstrap.go index 6d0a6264fa5..d0b5fe7b283 100644 --- a/agent/agentbootstrap/bootstrap.go +++ b/agent/agentbootstrap/bootstrap.go @@ -56,8 +56,9 @@ import ( type DqliteInitializerFunc func( ctx stdcontext.Context, mgr database.BootstrapNodeManager, + modelUUID model.UUID, logger database.Logger, - concerns ...database.BootstrapConcern, + concerns ...database.BootstrapOpt, ) error // Logger describes methods for emitting log output. @@ -250,29 +251,24 @@ func (b *AgentBootstrap) Initialize(ctx stdcontext.Context) (_ *state.Controller stateParams.ControllerInheritedConfig, stateParams.RegionInheritedConfig[stateParams.ControllerCloudRegion]) - databaseBootstrapConcerns := []database.BootstrapConcern{ - database.BootstrapControllerConcern( - // The admin user needs to be added before everything else that - // requires being owned by a Juju user. - addAdminUser, - ccbootstrap.InsertInitialControllerConfig(stateParams.ControllerConfig), - cloudbootstrap.InsertCloud(stateParams.ControllerCloud), - credbootstrap.InsertCredential(credential.IdFromTag(cloudCredTag), cloudCred), - cloudbootstrap.SetCloudDefaults(stateParams.ControllerCloud.Name, stateParams.ControllerInheritedConfig), - controllerModelCreateFunc, - ), - database.BootstrapModelConcern(controllerModelUUID, - modelbootstrap.CreateReadOnlyModel(controllerModelArgs.AsReadOnly()), - modelconfigbootstrap.SetModelConfig(stateParams.ControllerModelConfig, controllerModelDefaults), - ), + databaseBootstrapOptions := []database.BootstrapOpt{ + // The admin user needs to be added before everything else that + // requires being owned by a Juju user. + addAdminUser, + ccbootstrap.InsertInitialControllerConfig(stateParams.ControllerConfig), + cloudbootstrap.InsertCloud(stateParams.ControllerCloud), + credbootstrap.InsertCredential(credential.IdFromTag(cloudCredTag), cloudCred), + cloudbootstrap.SetCloudDefaults(stateParams.ControllerCloud.Name, stateParams.ControllerInheritedConfig), + controllerModelCreateFunc, + modelbootstrap.CreateReadOnlyModel(controllerModelArgs.AsReadOnly()), + modelconfigbootstrap.SetModelConfig(stateParams.ControllerModelConfig, controllerModelDefaults), } isCAAS := cloud.CloudIsCAAS(stateParams.ControllerCloud) if !isCAAS { // TODO(wallyworld) - this is just a placeholder for now - databaseBootstrapConcerns = append(databaseBootstrapConcerns, - database.BootstrapModelConcern(controllerModelUUID, - machinebootstrap.InsertMachine(agent.BootstrapControllerId), - )) + databaseBootstrapOptions = append(databaseBootstrapOptions, + machinebootstrap.InsertMachine(agent.BootstrapControllerId), + ) } // If we're running caas, we need to bind to the loopback address @@ -285,8 +281,9 @@ func (b *AgentBootstrap) Initialize(ctx stdcontext.Context) (_ *state.Controller if err := b.bootstrapDqlite( ctx, database.NewNodeManager(b.agentConfig, isLoopbackPreferred, b.logger, coredatabase.NoopSlowQueryLogger{}), + controllerModelUUID, b.logger, - databaseBootstrapConcerns..., + databaseBootstrapOptions..., ); err != nil { return nil, errors.Trace(err) } diff --git a/agent/agentbootstrap/bootstrap_test.go b/agent/agentbootstrap/bootstrap_test.go index e6c7797db36..29b7d84bc64 100644 --- a/agent/agentbootstrap/bootstrap_test.go +++ b/agent/agentbootstrap/bootstrap_test.go @@ -580,11 +580,17 @@ func (e *fakeEnviron) Provider() environs.EnvironProvider { return e.provider } -func bootstrapDqliteWithDummyCloudType(ctx context.Context, mgr database.BootstrapNodeManager, logger database.Logger, concerns ...database.BootstrapConcern) error { +func bootstrapDqliteWithDummyCloudType( + ctx context.Context, + mgr database.BootstrapNodeManager, + modelUUID model.UUID, + logger database.Logger, + opts ...database.BootstrapOpt, +) error { // The dummy cloud type needs to be inserted before the other operations. - concerns = append([]database.BootstrapConcern{ - database.BootstrapControllerInitConcern(database.EmptyInit, jujujujutesting.InsertDummyCloudType), - }, concerns...) + opts = append([]database.BootstrapOpt{ + jujujujutesting.InsertDummyCloudType, + }, opts...) - return database.BootstrapDqlite(ctx, mgr, logger, concerns...) + return database.BootstrapDqlite(ctx, mgr, modelUUID, logger, opts...) } diff --git a/apiserver/facades/agent/caasapplication/application_test.go b/apiserver/facades/agent/caasapplication/application_test.go index 100731d4682..077c86f2be5 100644 --- a/apiserver/facades/agent/caasapplication/application_test.go +++ b/apiserver/facades/agent/caasapplication/application_test.go @@ -44,7 +44,7 @@ func (s *CAASApplicationSuite) SetUpTest(c *gc.C) { s.ServiceFactorySuite.SetUpTest(c) controllerConfig := coretesting.FakeControllerConfig() - err := controllerconfigbootstrap.InsertInitialControllerConfig(controllerConfig)(context.Background(), s.TxnRunner()) + err := controllerconfigbootstrap.InsertInitialControllerConfig(controllerConfig)(context.Background(), s.TxnRunner(), s.NoopTxnRunner()) c.Assert(err, jc.ErrorIsNil) s.clock = testclock.NewClock(time.Now()) diff --git a/cmd/jujud-controller/agent/agenttest/agent.go b/cmd/jujud-controller/agent/agenttest/agent.go index 72fa85ed6b8..dac87827d4d 100644 --- a/cmd/jujud-controller/agent/agenttest/agent.go +++ b/cmd/jujud-controller/agent/agenttest/agent.go @@ -24,6 +24,7 @@ import ( cmdutil "github.com/juju/juju/cmd/jujud-controller/util" "github.com/juju/juju/controller" coredatabase "github.com/juju/juju/core/database" + "github.com/juju/juju/core/model" "github.com/juju/juju/core/network" "github.com/juju/juju/environs" "github.com/juju/juju/environs/filestorage" @@ -236,6 +237,7 @@ func (s *AgentSuite) PrimeStateAgentVersion(c *gc.C, tag names.Tag, password str err = database.BootstrapDqlite( context.Background(), database.NewNodeManager(conf, true, logger, coredatabase.NoopSlowQueryLogger{}), + model.MustNewUUID(), logger, ) c.Assert(err, jc.ErrorIsNil) diff --git a/cmd/jujud-controller/agent/bootstrap_test.go b/cmd/jujud-controller/agent/bootstrap_test.go index 08d5147eeaa..926a73e49a5 100644 --- a/cmd/jujud-controller/agent/bootstrap_test.go +++ b/cmd/jujud-controller/agent/bootstrap_test.go @@ -600,13 +600,14 @@ func nullContext() environs.BootstrapContext { func bootstrapDqliteWithDummyCloudType( ctx context.Context, mgr database.BootstrapNodeManager, + modelUUID model.UUID, logger database.Logger, - concerns ...database.BootstrapConcern, + opts ...database.BootstrapOpt, ) error { // The dummy cloud type needs to be inserted before the other operations. - concerns = append([]database.BootstrapConcern{ - database.BootstrapControllerInitConcern(database.EmptyInit, jujutesting.InsertDummyCloudType), - }, concerns...) + opts = append([]database.BootstrapOpt{ + jujutesting.InsertDummyCloudType, + }, opts...) - return database.BootstrapDqlite(ctx, mgr, logger, concerns...) + return database.BootstrapDqlite(ctx, mgr, modelUUID, logger, opts...) } diff --git a/cmd/jujud/agent/agenttest/agent.go b/cmd/jujud/agent/agenttest/agent.go index 90062d8e98f..d0514ee03ac 100644 --- a/cmd/jujud/agent/agenttest/agent.go +++ b/cmd/jujud/agent/agenttest/agent.go @@ -21,6 +21,7 @@ import ( agenttools "github.com/juju/juju/agent/tools" "github.com/juju/juju/controller" coredatabase "github.com/juju/juju/core/database" + "github.com/juju/juju/core/model" "github.com/juju/juju/core/network" "github.com/juju/juju/environs" "github.com/juju/juju/environs/filestorage" @@ -170,6 +171,7 @@ func (s *AgentSuite) PrimeStateAgentVersion(c *gc.C, tag names.Tag, password str err = database.BootstrapDqlite( context.Background(), database.NewNodeManager(conf, true, logger, coredatabase.NoopSlowQueryLogger{}), + model.MustNewUUID(), logger, ) c.Assert(err, jc.ErrorIsNil) diff --git a/core/model/model.go b/core/model/model.go index fc06b7bb10b..acab6562fa4 100644 --- a/core/model/model.go +++ b/core/model/model.go @@ -85,6 +85,15 @@ func NewUUID() (UUID, error) { return UUID(uuid.String()), nil } +// MustNewUUID is a convenience function for generating a new model uuid. +func MustNewUUID() UUID { + uuid, err := NewUUID() + if err != nil { + panic(err) + } + return uuid +} + // String implements the stringer interface for UUID. func (u UUID) String() string { return string(u) diff --git a/domain/cloud/bootstrap/bootstrap.go b/domain/cloud/bootstrap/bootstrap.go index b82e148e3c4..0dca0a0fe8e 100644 --- a/domain/cloud/bootstrap/bootstrap.go +++ b/domain/cloud/bootstrap/bootstrap.go @@ -15,17 +15,18 @@ import ( "github.com/juju/juju/core/database" "github.com/juju/juju/domain/cloud/state" modelconfigservice "github.com/juju/juju/domain/modelconfig/service" + internaldatabase "github.com/juju/juju/internal/database" "github.com/juju/juju/internal/uuid" ) // InsertCloud inserts the initial cloud during bootstrap. -func InsertCloud(cloud cloud.Cloud) func(context.Context, database.TxnRunner) error { - return func(ctx context.Context, db database.TxnRunner) error { +func InsertCloud(cloud cloud.Cloud) internaldatabase.BootstrapOpt { + return func(ctx context.Context, controller, model database.TxnRunner) error { cloudUUID, err := uuid.NewUUID() if err != nil { return errors.Trace(err) } - return errors.Trace(db.Txn(ctx, func(ctx context.Context, tx *sqlair.TX) error { + return errors.Trace(controller.Txn(ctx, func(ctx context.Context, tx *sqlair.TX) error { if err := state.CreateCloud(ctx, tx, cloudUUID.String(), cloud); err != nil { return errors.Annotate(err, "creating bootstrap cloud") } @@ -42,14 +43,14 @@ func InsertCloud(cloud cloud.Cloud) func(context.Context, database.TxnRunner) er func SetCloudDefaults( cloudName string, defaults map[string]any, -) func(context.Context, database.TxnRunner) error { - return func(ctx context.Context, db database.TxnRunner) error { +) internaldatabase.BootstrapOpt { + return func(ctx context.Context, controller, model database.TxnRunner) error { strDefaults, err := modelconfigservice.CoerceConfigForStorage(defaults) if err != nil { return fmt.Errorf("coercing cloud %q default values for storage: %w", cloudName, err) } - err = db.StdTxn(ctx, func(ctx context.Context, tx *sql.Tx) error { + err = controller.StdTxn(ctx, func(ctx context.Context, tx *sql.Tx) error { return state.SetCloudDefaults(ctx, tx, cloudName, strDefaults) }) diff --git a/domain/cloud/bootstrap/bootstrap_test.go b/domain/cloud/bootstrap/bootstrap_test.go index 2b480aeb77b..fe7025ba420 100644 --- a/domain/cloud/bootstrap/bootstrap_test.go +++ b/domain/cloud/bootstrap/bootstrap_test.go @@ -23,7 +23,7 @@ var _ = gc.Suite(&bootstrapSuite{}) func (s *bootstrapSuite) TestInsertCloud(c *gc.C) { cld := cloud.Cloud{Name: "cirrus", Type: "ec2", AuthTypes: cloud.AuthTypes{cloud.UserPassAuthType}} - err := InsertCloud(cld)(context.Background(), s.TxnRunner()) + err := InsertCloud(cld)(context.Background(), s.TxnRunner(), s.NoopTxnRunner()) c.Assert(err, jc.ErrorIsNil) var name string @@ -39,7 +39,7 @@ func (s *bootstrapSuite) TestSetCloudDefaultsNoExist(c *gc.C) { "HTTP_PROXY": "[2001:0DB8::1]:80", }) - err := set(context.Background(), s.TxnRunner()) + err := set(context.Background(), s.TxnRunner(), s.NoopTxnRunner()) c.Check(err, jc.ErrorIs, clouderrors.NotFound) var count int @@ -56,14 +56,14 @@ func (s *bootstrapSuite) TestSetCloudDefaults(c *gc.C) { Type: "ec2", AuthTypes: cloud.AuthTypes{cloud.UserPassAuthType}, } - err := InsertCloud(cld)(context.Background(), s.TxnRunner()) + err := InsertCloud(cld)(context.Background(), s.TxnRunner(), s.NoopTxnRunner()) c.Check(err, jc.ErrorIsNil) set := SetCloudDefaults("cirrus", map[string]any{ "HTTP_PROXY": "[2001:0DB8::1]:80", }) - err = set(context.Background(), s.TxnRunner()) + err = set(context.Background(), s.TxnRunner(), s.NoopTxnRunner()) c.Check(err, jc.ErrorIsNil) st := state.NewState(s.TxnRunnerFactory()) @@ -82,14 +82,14 @@ func (s *bootstrapSuite) TestSetCloudDefaultsOverides(c *gc.C) { Type: "ec2", AuthTypes: cloud.AuthTypes{cloud.UserPassAuthType}, } - err := InsertCloud(cld)(context.Background(), s.TxnRunner()) + err := InsertCloud(cld)(context.Background(), s.TxnRunner(), s.NoopTxnRunner()) c.Check(err, jc.ErrorIsNil) set := SetCloudDefaults("cirrus", map[string]any{ "HTTP_PROXY": "[2001:0DB8::1]:80", }) - err = set(context.Background(), s.TxnRunner()) + err = set(context.Background(), s.TxnRunner(), s.NoopTxnRunner()) c.Check(err, jc.ErrorIsNil) st := state.NewState(s.TxnRunnerFactory()) @@ -105,7 +105,7 @@ func (s *bootstrapSuite) TestSetCloudDefaultsOverides(c *gc.C) { "foo": "bar", }) - err = set(context.Background(), s.TxnRunner()) + err = set(context.Background(), s.TxnRunner(), s.NoopTxnRunner()) c.Check(err, jc.ErrorIsNil) st = state.NewState(s.TxnRunnerFactory()) diff --git a/domain/controllerconfig/bootstrap/bootstrap.go b/domain/controllerconfig/bootstrap/bootstrap.go index b151f3b68e2..4806f0416a4 100644 --- a/domain/controllerconfig/bootstrap/bootstrap.go +++ b/domain/controllerconfig/bootstrap/bootstrap.go @@ -9,21 +9,21 @@ import ( "github.com/juju/errors" - "github.com/juju/juju/controller" + jujucontroller "github.com/juju/juju/controller" "github.com/juju/juju/core/database" - jujudatabase "github.com/juju/juju/internal/database" + internaldatabase "github.com/juju/juju/internal/database" ) // InsertInitialControllerConfig inserts the initial controller configuration // into the database. -func InsertInitialControllerConfig(cfg controller.Config) func(context.Context, database.TxnRunner) error { - return func(ctx context.Context, db database.TxnRunner) error { - values, err := controller.EncodeToString(cfg) +func InsertInitialControllerConfig(cfg jujucontroller.Config) internaldatabase.BootstrapOpt { + return func(ctx context.Context, controller, model database.TxnRunner) error { + values, err := jujucontroller.EncodeToString(cfg) if err != nil { return errors.Trace(err) } - fields, _, err := controller.ConfigSchema.ValidationSchema() + fields, _, err := jujucontroller.ConfigSchema.ValidationSchema() if err != nil { return errors.Trace(err) } @@ -39,10 +39,10 @@ func InsertInitialControllerConfig(cfg controller.Config) func(context.Context, query := "INSERT INTO controller_config (key, value) VALUES (?, ?)" - return errors.Trace(db.StdTxn(ctx, func(ctx context.Context, tx *sql.Tx) error { + return errors.Trace(controller.StdTxn(ctx, func(ctx context.Context, tx *sql.Tx) error { for k, v := range values { if _, err := tx.ExecContext(ctx, query, k, v); err != nil { - if jujudatabase.IsErrConstraintPrimaryKey(errors.Cause(err)) { + if internaldatabase.IsErrConstraintPrimaryKey(errors.Cause(err)) { return errors.AlreadyExistsf("controller configuration key %q", k) } return errors.Annotatef(err, "inserting controller configuration %q, %v", k, v) diff --git a/domain/controllerconfig/bootstrap/bootstrap_test.go b/domain/controllerconfig/bootstrap/bootstrap_test.go index e184a9fb2cb..e3886937711 100644 --- a/domain/controllerconfig/bootstrap/bootstrap_test.go +++ b/domain/controllerconfig/bootstrap/bootstrap_test.go @@ -22,7 +22,7 @@ var _ = gc.Suite(&bootstrapSuite{}) func (s *bootstrapSuite) TestInsertInitialControllerConfig(c *gc.C) { cfg := controller.Config{controller.CACertKey: testing.CACert} - err := InsertInitialControllerConfig(cfg)(context.Background(), s.TxnRunner()) + err := InsertInitialControllerConfig(cfg)(context.Background(), s.TxnRunner(), s.NoopTxnRunner()) c.Assert(err, jc.ErrorIsNil) var cert string diff --git a/domain/controllerconfig/controllerconfig_test.go b/domain/controllerconfig/controllerconfig_test.go index 62de8aa7747..8bddcce0ad9 100644 --- a/domain/controllerconfig/controllerconfig_test.go +++ b/domain/controllerconfig/controllerconfig_test.go @@ -47,7 +47,7 @@ func (s *controllerconfigSuite) TestControllerConfigRoundTrips(c *gc.C) { ) c.Assert(err, jc.ErrorIsNil) - err = bootstrap.InsertInitialControllerConfig(cfgIn)(ctx.Background(), s.TxnRunner()) + err = bootstrap.InsertInitialControllerConfig(cfgIn)(ctx.Background(), s.TxnRunner(), s.NoopTxnRunner()) c.Assert(err, jc.ErrorIsNil) cfgOut, err := srv.ControllerConfig(ctx.Background()) diff --git a/domain/credential/bootstrap/bootstrap.go b/domain/credential/bootstrap/bootstrap.go index 1b96e9f027e..16352155ad0 100644 --- a/domain/credential/bootstrap/bootstrap.go +++ b/domain/credential/bootstrap/bootstrap.go @@ -14,12 +14,13 @@ import ( "github.com/juju/juju/core/database" "github.com/juju/juju/domain/credential" "github.com/juju/juju/domain/credential/state" + internaldatabase "github.com/juju/juju/internal/database" "github.com/juju/juju/internal/uuid" ) // InsertCredential inserts a cloud credential into dqlite. -func InsertCredential(id corecredential.ID, cred cloud.Credential) func(context.Context, database.TxnRunner) error { - return func(ctx context.Context, db database.TxnRunner) error { +func InsertCredential(id corecredential.ID, cred cloud.Credential) internaldatabase.BootstrapOpt { + return func(ctx context.Context, controller, model database.TxnRunner) error { if id.IsZero() { return nil } @@ -28,7 +29,7 @@ func InsertCredential(id corecredential.ID, cred cloud.Credential) func(context. if err != nil { return errors.Trace(err) } - return errors.Trace(db.Txn(ctx, func(ctx context.Context, tx *sqlair.TX) error { + return errors.Trace(controller.Txn(ctx, func(ctx context.Context, tx *sqlair.TX) error { if err := state.CreateCredential(ctx, tx, credentialUUID.String(), id, credential.CloudCredentialInfo{ AuthType: string(cred.AuthType()), Attributes: cred.Attributes(), diff --git a/domain/credential/bootstrap/bootstrap_test.go b/domain/credential/bootstrap/bootstrap_test.go index c8f52644844..675cac49547 100644 --- a/domain/credential/bootstrap/bootstrap_test.go +++ b/domain/credential/bootstrap/bootstrap_test.go @@ -27,7 +27,7 @@ var _ = gc.Suite(&bootstrapSuite{}) func (s *bootstrapSuite) TestInsertInitialControllerConfig(c *gc.C) { ctx := context.Background() cld := cloud.Cloud{Name: "cirrus", Type: "ec2", AuthTypes: cloud.AuthTypes{cloud.UserPassAuthType}} - err := cloudbootstrap.InsertCloud(cld)(ctx, s.TxnRunner()) + err := cloudbootstrap.InsertCloud(cld)(ctx, s.TxnRunner(), s.NoopTxnRunner()) c.Assert(err, jc.ErrorIsNil) userUUID, err := user.NewUUID() @@ -50,7 +50,7 @@ func (s *bootstrapSuite) TestInsertInitialControllerConfig(c *gc.C) { Name: "foo", } - err = InsertCredential(id, cred)(ctx, s.TxnRunner()) + err = InsertCredential(id, cred)(ctx, s.TxnRunner(), s.NoopTxnRunner()) c.Assert(err, jc.ErrorIsNil) var owner, cloudName string diff --git a/domain/machine/bootstrap/bootstrap.go b/domain/machine/bootstrap/bootstrap.go index 69e645681dd..8bcfd55c6d7 100644 --- a/domain/machine/bootstrap/bootstrap.go +++ b/domain/machine/bootstrap/bootstrap.go @@ -11,13 +11,14 @@ import ( "github.com/juju/juju/core/database" "github.com/juju/juju/domain/life" + internaldatabase "github.com/juju/juju/internal/database" "github.com/juju/juju/internal/uuid" ) // InsertMachine inserts a machine during bootstrap. // TODO - this just creates a minimal row for now. -func InsertMachine(machineId string) func(context.Context, database.TxnRunner) error { - return func(ctx context.Context, db database.TxnRunner) error { +func InsertMachine(machineId string) internaldatabase.BootstrapOpt { + return func(ctx context.Context, controller, model database.TxnRunner) error { createMachine := ` INSERT INTO machine (uuid, net_node_uuid, machine_id, life_id) @@ -43,7 +44,7 @@ VALUES ($M.machine_uuid, $M.net_node_uuid, $M.machine_id, $M.life_id) return errors.Trace(err) } - return errors.Trace(db.Txn(ctx, func(ctx context.Context, tx *sqlair.TX) error { + return errors.Trace(model.Txn(ctx, func(ctx context.Context, tx *sqlair.TX) error { createParams := sqlair.M{ "machine_uuid": machineUUID.String(), "net_node_uuid": nodeUUID.String(), diff --git a/domain/machine/bootstrap/bootstrap_test.go b/domain/machine/bootstrap/bootstrap_test.go index e55f9844e81..54903af824a 100644 --- a/domain/machine/bootstrap/bootstrap_test.go +++ b/domain/machine/bootstrap/bootstrap_test.go @@ -19,7 +19,7 @@ type bootstrapSuite struct { var _ = gc.Suite(&bootstrapSuite{}) func (s *bootstrapSuite) TestInsertBootstrapMachine(c *gc.C) { - err := InsertMachine("666")(context.Background(), s.TxnRunner()) + err := InsertMachine("666")(context.Background(), s.NoopTxnRunner(), s.TxnRunner()) c.Assert(err, jc.ErrorIsNil) var machineId string diff --git a/domain/model/bootstrap/bootstrap.go b/domain/model/bootstrap/bootstrap.go index 78d017baf69..87a86c41270 100644 --- a/domain/model/bootstrap/bootstrap.go +++ b/domain/model/bootstrap/bootstrap.go @@ -16,6 +16,7 @@ import ( modelerrors "github.com/juju/juju/domain/model/errors" "github.com/juju/juju/domain/model/service" "github.com/juju/juju/domain/model/state" + internaldatabase "github.com/juju/juju/internal/database" jujuversion "github.com/juju/juju/version" ) @@ -41,7 +42,7 @@ func (m modelTypeStateFunc) CloudType(c context.Context, n string) (string, erro // - [modelerrors.AgentVersionNotSupported] func CreateModel( args model.ModelCreationArgs, -) (coremodel.UUID, func(context.Context, database.TxnRunner) error) { +) (coremodel.UUID, internaldatabase.BootstrapOpt) { var uuidError error uuid := args.UUID if uuid == "" { @@ -49,12 +50,12 @@ func CreateModel( } if uuidError != nil { - return coremodel.UUID(""), func(_ context.Context, _ database.TxnRunner) error { + return coremodel.UUID(""), func(_ context.Context, _, _ database.TxnRunner) error { return fmt.Errorf("generating bootstrap model %q uuid: %w", args.Name, uuidError) } } - return uuid, func(ctx context.Context, db database.TxnRunner) error { + return uuid, func(ctx context.Context, controller, model database.TxnRunner) error { if err := args.Validate(); err != nil { return fmt.Errorf("model creation args: %w", err) } @@ -73,7 +74,7 @@ func CreateModel( return fmt.Errorf("invalid model uuid: %w", err) } - return db.StdTxn(ctx, func(ctx context.Context, tx *sql.Tx) error { + return controller.StdTxn(ctx, func(ctx context.Context, tx *sql.Tx) error { modelTypeState := modelTypeStateFunc( func(ctx context.Context, cloudName string) (string, error) { return state.CloudType()(ctx, tx, cloudName) @@ -97,13 +98,13 @@ func CreateModel( // once created. func CreateReadOnlyModel( args model.ReadOnlyModelCreationArgs, -) func(context.Context, database.TxnRunner) error { - return func(ctx context.Context, db database.TxnRunner) error { +) internaldatabase.BootstrapOpt { + return func(ctx context.Context, controller, model database.TxnRunner) error { if err := args.Validate(); err != nil { return fmt.Errorf("model creation args: %w", err) } - return db.StdTxn(ctx, func(ctx context.Context, tx *sql.Tx) error { + return model.StdTxn(ctx, func(ctx context.Context, tx *sql.Tx) error { return state.CreateReadOnlyModel(ctx, args, tx) }) } diff --git a/domain/model/bootstrap/bootstrap_test.go b/domain/model/bootstrap/bootstrap_test.go index 67c4306dd4f..f1ced7dd24b 100644 --- a/domain/model/bootstrap/bootstrap_test.go +++ b/domain/model/bootstrap/bootstrap_test.go @@ -37,7 +37,7 @@ func (s *bootstrapSuite) SetUpTest(c *gc.C) { s.ControllerSuite.SetUpTest(c) uuid, fn := userbootstrap.AddUser(coreuser.AdminUserName, permission.ControllerForAccess(permission.SuperuserAccess)) - err := fn(context.Background(), s.ControllerTxnRunner()) + err := fn(context.Background(), s.ControllerTxnRunner(), s.NoopTxnRunner()) c.Assert(err, jc.ErrorIsNil) s.adminUserUUID = uuid @@ -48,7 +48,7 @@ func (s *bootstrapSuite) SetUpTest(c *gc.C) { AuthTypes: cloud.AuthTypes{cloud.EmptyAuthType}, }) - err = fn(context.Background(), s.ControllerTxnRunner()) + err = fn(context.Background(), s.ControllerTxnRunner(), s.NoopTxnRunner()) c.Assert(err, jc.ErrorIsNil) s.credentialName = "test" @@ -60,7 +60,7 @@ func (s *bootstrapSuite) SetUpTest(c *gc.C) { cloud.NewCredential(cloud.EmptyAuthType, nil), ) - err = fn(context.Background(), s.ControllerTxnRunner()) + err = fn(context.Background(), s.ControllerTxnRunner(), s.NoopTxnRunner()) c.Assert(err, jc.ErrorIsNil) } @@ -77,7 +77,7 @@ func (s *bootstrapSuite) TestUUIDIsCreated(c *gc.C) { Owner: s.adminUserUUID, }) - err := fn(context.Background(), s.ControllerTxnRunner()) + err := fn(context.Background(), s.ControllerTxnRunner(), s.NoopTxnRunner()) c.Assert(err, jc.ErrorIsNil) c.Check(uuid.String() == "", jc.IsFalse) @@ -99,7 +99,7 @@ func (s *bootstrapSuite) TestUUIDIsRespected(c *gc.C) { UUID: modelUUID, }) - err := fn(context.Background(), s.ControllerTxnRunner()) + err := fn(context.Background(), s.ControllerTxnRunner(), s.NoopTxnRunner()) c.Assert(err, jc.ErrorIsNil) c.Check(uuid, gc.Equals, modelUUID) @@ -120,6 +120,6 @@ func (s *modelBootstrapSuite) TestCreateReadOnlyModel(c *gc.C) { CloudRegion: "myregion", }) - err := fn(context.Background(), s.ModelTxnRunner()) + err := fn(context.Background(), s.NoopTxnRunner(), s.ModelTxnRunner()) c.Assert(err, jc.ErrorIsNil) } diff --git a/domain/modelconfig/bootstrap/bootstrap.go b/domain/modelconfig/bootstrap/bootstrap.go index 1a10d34a1c5..62e8c30296f 100644 --- a/domain/modelconfig/bootstrap/bootstrap.go +++ b/domain/modelconfig/bootstrap/bootstrap.go @@ -12,6 +12,7 @@ import ( "github.com/juju/juju/domain/modelconfig/service" "github.com/juju/juju/domain/modelconfig/state" "github.com/juju/juju/environs/config" + internaldatabase "github.com/juju/juju/internal/database" ) // SetModelConfig will remove any existing model config for the model and @@ -20,8 +21,8 @@ import ( func SetModelConfig( cfg *config.Config, defaultsProvider service.ModelDefaultsProvider, -) func(context.Context, database.TxnRunner) error { - return func(ctx context.Context, db database.TxnRunner) error { +) internaldatabase.BootstrapOpt { + return func(ctx context.Context, controller, model database.TxnRunner) error { attrs := cfg.AllAttrs() defaults, err := defaultsProvider.ModelDefaults(ctx) if err != nil { @@ -45,9 +46,7 @@ func SetModelConfig( // - model agent version in the model database correctly. // - change any client code that is passing the value via config. // - add migration logic to get rid of agent version out of config. - if _, exists := attrs[config.AgentVersionKey]; exists { - delete(attrs, config.AgentVersionKey) - } + delete(attrs, config.AgentVersionKey) cfg, err = config.New(config.NoDefaults, attrs) if err != nil { @@ -64,7 +63,7 @@ func SetModelConfig( return fmt.Errorf("coercing model config for storage: %w", err) } - return db.StdTxn(ctx, func(ctx context.Context, tx *sql.Tx) error { + return model.StdTxn(ctx, func(ctx context.Context, tx *sql.Tx) error { return state.SetModelConfig(ctx, rawCfg, tx) }) } diff --git a/domain/modelconfig/bootstrap/bootstrap_test.go b/domain/modelconfig/bootstrap/bootstrap_test.go index a34be7dcf43..058af795d66 100644 --- a/domain/modelconfig/bootstrap/bootstrap_test.go +++ b/domain/modelconfig/bootstrap/bootstrap_test.go @@ -45,7 +45,7 @@ func (s *bootstrapSuite) TestSetModelConfig(c *gc.C) { }) c.Assert(err, jc.ErrorIsNil) - err = SetModelConfig(cfg, defaults)(context.Background(), s.TxnRunner()) + err = SetModelConfig(cfg, defaults)(context.Background(), s.NoopTxnRunner(), s.TxnRunner()) c.Assert(err, jc.ErrorIsNil) rows, err := s.DB().Query("SELECT * FROM model_config") diff --git a/domain/servicefactory/testing/suite.go b/domain/servicefactory/testing/suite.go index 01001eec337..61eb5bf4071 100644 --- a/domain/servicefactory/testing/suite.go +++ b/domain/servicefactory/testing/suite.go @@ -83,7 +83,7 @@ func (s *ServiceFactorySuite) SeedAdminUser(c *gc.C) { permission.ControllerForAccess(permission.SuperuserAccess), ) s.AdminUserUUID = uuid - err := fn(context.Background(), s.ControllerTxnRunner()) + err := fn(context.Background(), s.ControllerTxnRunner(), s.NoopTxnRunner()) c.Assert(err, jc.ErrorIsNil) } @@ -101,7 +101,7 @@ func (s *ServiceFactorySuite) SeedCloudAndCredential(c *gc.C) { Name: "dummy-region", }, }, - })(context.Background(), s.ControllerTxnRunner()) + })(context.Background(), s.ControllerTxnRunner(), s.NoopTxnRunner()) c.Assert(err, jc.ErrorIsNil) s.CredentialID = credential.ID{ @@ -115,7 +115,7 @@ func (s *ServiceFactorySuite) SeedCloudAndCredential(c *gc.C) { "username": "dummy", "password": "secret", }), - )(context.Background(), s.ControllerTxnRunner()) + )(context.Background(), s.ControllerTxnRunner(), s.NoopTxnRunner()) c.Assert(err, jc.ErrorIsNil) } @@ -132,7 +132,7 @@ func (s *ServiceFactorySuite) SeedModelDatabases(c *gc.C) { } uuid, fn := modelbootstrap.CreateModel(controllerArgs) - err := fn(context.Background(), s.ControllerTxnRunner()) + err := fn(context.Background(), s.ControllerTxnRunner(), s.NoopTxnRunner()) c.Assert(err, jc.ErrorIsNil) s.ControllerModelUUID = uuid @@ -146,7 +146,7 @@ func (s *ServiceFactorySuite) SeedModelDatabases(c *gc.C) { } uuid, fn = modelbootstrap.CreateModel(modelArgs) - err = fn(context.Background(), s.ControllerTxnRunner()) + err = fn(context.Background(), s.ControllerTxnRunner(), s.NoopTxnRunner()) c.Assert(err, jc.ErrorIsNil) s.DefaultModelUUID = uuid } diff --git a/domain/testing/controllerconfigsuite.go b/domain/testing/controllerconfigsuite.go index 849d5302f41..272ae355f4b 100644 --- a/domain/testing/controllerconfigsuite.go +++ b/domain/testing/controllerconfigsuite.go @@ -5,7 +5,10 @@ package testing import ( "context" + "database/sql" + "github.com/canonical/sqlair" + "github.com/juju/errors" jc "github.com/juju/testing/checkers" gc "gopkg.in/check.v1" @@ -23,7 +26,17 @@ func SeedControllerConfig( config controller.Config, provider ControllerTxnProvider, ) controller.Config { - err := bootstrap.InsertInitialControllerConfig(config)(context.Background(), provider.ControllerTxnRunner()) + err := bootstrap.InsertInitialControllerConfig(config)(context.Background(), provider.ControllerTxnRunner(), noopTxnRunner{}) c.Assert(err, jc.ErrorIsNil) return config } + +type noopTxnRunner struct{} + +func (noopTxnRunner) Txn(context.Context, func(context.Context, *sqlair.TX) error) error { + return errors.NotImplemented +} + +func (noopTxnRunner) StdTxn(context.Context, func(context.Context, *sql.Tx) error) error { + return errors.NotImplemented +} diff --git a/domain/user/bootstrap/bootstrap.go b/domain/user/bootstrap/bootstrap.go index 4785290c622..89c9b3155b5 100644 --- a/domain/user/bootstrap/bootstrap.go +++ b/domain/user/bootstrap/bootstrap.go @@ -16,6 +16,7 @@ import ( domainuser "github.com/juju/juju/domain/user" "github.com/juju/juju/domain/user/state" "github.com/juju/juju/internal/auth" + internaldatabase "github.com/juju/juju/internal/database" ) // AddUser is responsible for registering a new user in the system at bootstrap @@ -26,22 +27,20 @@ import ( // // If the username passed to this function is invalid an error satisfying // [github.com/juju/juju/domain/user/errors.UsernameNotValid] is returned. -func AddUser(name string, access permission.AccessSpec) (user.UUID, func(context.Context, database.TxnRunner) error) { +func AddUser(name string, access permission.AccessSpec) (user.UUID, internaldatabase.BootstrapOpt) { if err := domainuser.ValidateUserName(name); err != nil { - return user.UUID(""), func(context.Context, database.TxnRunner) error { - return fmt.Errorf("validating bootstrap add user %q: %w", name, err) - } + return user.UUID(""), bootstrapErr( + fmt.Errorf("validating bootstrap add user %q: %w", name, err)) } uuid, err := user.NewUUID() if err != nil { - return user.UUID(""), func(_ context.Context, _ database.TxnRunner) error { - return fmt.Errorf("generating bootstrap user %q uuid: %w", name, err) - } + return user.UUID(""), bootstrapErr( + fmt.Errorf("generating bootstrap user %q uuid: %w", name, err)) } - return uuid, func(ctx context.Context, db database.TxnRunner) error { - err := db.Txn(ctx, func(ctx context.Context, tx *sqlair.TX) error { + return uuid, func(ctx context.Context, controller, model database.TxnRunner) error { + err := controller.Txn(ctx, func(ctx context.Context, tx *sqlair.TX) error { return state.AddUser(ctx, tx, uuid, name, "", uuid, access) }) @@ -58,47 +57,38 @@ func AddUser(name string, access permission.AccessSpec) (user.UUID, func(context // // If the username passed to this function is invalid an error satisfying // [github.com/juju/juju/domain/user/errors.UsernameNotValid] is returned. -func AddUserWithPassword(name string, password auth.Password, access permission.AccessSpec) (user.UUID, func(context.Context, database.TxnRunner) error) { +func AddUserWithPassword(name string, password auth.Password, access permission.AccessSpec) (user.UUID, internaldatabase.BootstrapOpt) { defer password.Destroy() if err := domainuser.ValidateUserName(name); err != nil { - return user.UUID(""), func(context.Context, database.TxnRunner) error { - return fmt.Errorf("validating bootstrap add user %q with password: %w", name, err) - } + return user.UUID(""), bootstrapErr( + fmt.Errorf("validating bootstrap add user %q with password: %w", + name, err)) } uuid, err := user.NewUUID() if err != nil { - return "", func(context.Context, database.TxnRunner) error { - return fmt.Errorf( - "generating uuid for bootstrap add user %q with password: %w", - name, err, - ) - } + return "", bootstrapErr(fmt.Errorf( + "generating uuid for bootstrap add user %q with password: %w", + name, err)) } salt, err := auth.NewSalt() if err != nil { - return "", func(context.Context, database.TxnRunner) error { - return fmt.Errorf( - "generating salt for bootstrap add user %q with password: %w", - name, err, - ) - } + return "", bootstrapErr(fmt.Errorf( + "generating salt for bootstrap add user %q with password: %w", + name, err)) } pwHash, err := auth.HashPassword(password, salt) if err != nil { - return "", func(context.Context, database.TxnRunner) error { - return fmt.Errorf( - "generating password hash for bootstrap add user %q with password: %w", - name, err, - ) - } + return "", bootstrapErr(fmt.Errorf( + "generating password hash for bootstrap add user %q with password: %w", + name, err)) } - return uuid, func(ctx context.Context, db database.TxnRunner) error { - return errors.Trace(db.Txn(ctx, func(ctx context.Context, tx *sqlair.TX) error { + return uuid, func(ctx context.Context, controller, model database.TxnRunner) error { + return errors.Trace(controller.Txn(ctx, func(ctx context.Context, tx *sqlair.TX) error { if err = state.AddUserWithPassword( ctx, tx, uuid, @@ -115,3 +105,9 @@ func AddUserWithPassword(name string, password auth.Password, access permission. })) } } + +func bootstrapErr(err error) func(context.Context, database.TxnRunner, database.TxnRunner) error { + return func(context.Context, database.TxnRunner, database.TxnRunner) error { + return err + } +} diff --git a/domain/user/bootstrap/bootstrap_test.go b/domain/user/bootstrap/bootstrap_test.go index f44f172f060..3c970dfc946 100644 --- a/domain/user/bootstrap/bootstrap_test.go +++ b/domain/user/bootstrap/bootstrap_test.go @@ -30,7 +30,7 @@ func (s *bootstrapSuite) TestAddUser(c *gc.C) { Key: database.ControllerNS, }, }) - err := addAdminUser(ctx, s.TxnRunner()) + err := addAdminUser(ctx, s.TxnRunner(), s.NoopTxnRunner()) c.Assert(err, jc.ErrorIsNil) c.Assert(uuid.Validate(), jc.ErrorIsNil) @@ -51,7 +51,7 @@ func (s *bootstrapSuite) TestAddUserWithPassword(c *gc.C) { Key: database.ControllerNS, }, }) - err := addAdminUser(ctx, s.TxnRunner()) + err := addAdminUser(ctx, s.TxnRunner(), s.NoopTxnRunner()) c.Assert(err, jc.ErrorIsNil) c.Assert(uuid.Validate(), jc.ErrorIsNil) diff --git a/internal/database/bootstrap.go b/internal/database/bootstrap.go index 40c1d3958bc..5d24d0e2c0c 100644 --- a/internal/database/bootstrap.go +++ b/internal/database/bootstrap.go @@ -11,14 +11,15 @@ import ( "github.com/juju/errors" coredatabase "github.com/juju/juju/core/database" - coreschema "github.com/juju/juju/core/database/schema" - coremodel "github.com/juju/juju/core/model" + "github.com/juju/juju/core/model" "github.com/juju/juju/core/network" "github.com/juju/juju/domain/schema" "github.com/juju/juju/internal/database/app" "github.com/juju/juju/internal/database/pragma" ) +// BootstrapNodeManager is an interface for managing the bootstrap of a Dqlite +// node. type BootstrapNodeManager interface { // EnsureDataDir ensures that a directory for Dqlite data exists at // a path determined by the agent config, then returns that path. @@ -51,120 +52,12 @@ type BootstrapNodeManager interface { WithTracingOption() app.Option } -// txnRunner is the simplest implementation of TxnRunner, wrapping a -// sql.DB reference. It is recruited to run the bootstrap DB migration, -// where we do not yet have access to a transaction runner sourced from -// dbaccessor worker. -type txnRunner struct { - db *sql.DB -} - -func (r *txnRunner) Txn(ctx context.Context, f func(context.Context, *sqlair.TX) error) error { - return errors.Trace(Txn(ctx, sqlair.NewDB(r.db), f)) -} - -func (r *txnRunner) StdTxn(ctx context.Context, f func(context.Context, *sql.Tx) error) error { - return errors.Trace(StdTxn(ctx, r.db, f)) -} - -// BootstrapConcern is a type for describing a set of bootstrap operations to -// perform on a dqlite application. -type BootstrapConcern = func(ctx context.Context, logger Logger, dqlite *app.App) error - -// BootstrapInit is a type for describing a bootstrap operation that -// initialises a database. -type BootstrapInit = func(ctx context.Context, runner coredatabase.TxnRunner, dqlite *app.App) error - -// BootstrapControllerConcern is a BootstrapConcern type that will run the -// provided BootstrapOpts on the controller database. -func BootstrapControllerConcern(ops ...BootstrapOpt) BootstrapConcern { - return bootstrapDBConcern(coredatabase.ControllerNS, schema.ControllerDDL(), controllerBootstrapInit, ops...) -} - -// BootstrapModelConcern is a BootstrapConcern type that will run the -// provided BootstrapOpts on the specified model database. -func BootstrapModelConcern(uuid coremodel.UUID, ops ...BootstrapOpt) BootstrapConcern { - return bootstrapDBConcern(uuid.String(), schema.ModelDDL(), EmptyInit, ops...) -} - -// BootstrapControllerInitConcern is a BootstrapConcern type that will run the -// provided BootstrapOpts on the controller database, after first running the -// provided BootstrapInit. -func BootstrapControllerInitConcern(bootstrapInit BootstrapInit, ops ...BootstrapOpt) BootstrapConcern { - return bootstrapDBConcern(coredatabase.ControllerNS, schema.ControllerDDL(), bootstrapInit, ops...) -} - -// controllerBootstrapInit is used to initialise the controller database with -// a controller node ID. The controller node ID is required to be present in -// the controller_node table as this is used for referential integrity. -func controllerBootstrapInit(ctx context.Context, runner coredatabase.TxnRunner, dqlite *app.App) error { - if err := InsertControllerNodeID(ctx, runner, dqlite.ID()); err != nil { - // If the controller node ID already exists, we assume that - // the database has already been bootstrapped. Mask the unique - // constraint error with a more user-friendly error. - if IsErrConstraintUnique(err) { - return errors.AlreadyExistsf("controller node ID") - } - return errors.Annotatef(err, "inserting controller node ID") - } - return nil -} - -// EmptyInit is a BootstrapInit type that does nothing. -func EmptyInit(context.Context, coredatabase.TxnRunner, *app.App) error { - return nil -} - -func bootstrapDBConcern( - namespace string, - namespaceSchema *coreschema.Schema, - bootstrapInit BootstrapInit, - ops ...BootstrapOpt, -) BootstrapConcern { - return func(ctx context.Context, logger Logger, dqlite *app.App) error { - db, err := dqlite.Open(ctx, namespace) - if err != nil { - return errors.Annotatef(err, "opening database for namespace %q", namespace) - } - - if err := pragma.SetPragma(ctx, db, pragma.ForeignKeysPragma, true); err != nil { - return errors.Annotatef(err, "setting foreign keys pragma for namespace %q", namespace) - } - - defer func() { - if err := db.Close(); err != nil { - logger.Errorf("closing database with namespace %q: %v", namespace, err) - } - }() - - runner := &txnRunner{db: db} - - migration := NewDBMigration(runner, logger, namespaceSchema) - if err := migration.Apply(ctx); err != nil { - return errors.Annotatef(err, "creating database with namespace %q schema", namespace) - } - - if err := bootstrapInit(ctx, runner, dqlite); err != nil { - return errors.Annotatef(err, "running bootstrap init for database with namespace %q", namespace) - } - - for i, op := range ops { - if err := op(ctx, runner); err != nil { - return errors.Annotatef( - err, - "running bootstrap operation at index %d for database with namespace %q", - i, - namespace, - ) - } - } - return nil - } -} - // BootstrapOpt is a function run when bootstrapping a database, // used to insert initial data into the model. -type BootstrapOpt func(context.Context, coredatabase.TxnRunner) error +type BootstrapOpt func( + ctx context.Context, + controller, model coredatabase.TxnRunner, +) error // BootstrapDqlite opens a new database for the controller, and runs the // DDL to create its schema. @@ -174,8 +67,9 @@ type BootstrapOpt func(context.Context, coredatabase.TxnRunner) error func BootstrapDqlite( ctx context.Context, mgr BootstrapNodeManager, + uuid model.UUID, logger Logger, - concerns ...BootstrapConcern, + opts ...BootstrapOpt, ) error { dir, err := mgr.EnsureDataDir() if err != nil { @@ -213,15 +107,49 @@ func BootstrapDqlite( return errors.Annotatef(err, "waiting for Dqlite readiness") } - for i, concern := range concerns { - if err := concern(ctx, logger, dqlite); err != nil { - return errors.Annotatef(err, "running bootstrap concern at index %d", i) + controller, err := runMigration(ctx, dqlite, coredatabase.ControllerNS, schema.ControllerDDL(), controllerBootstrapInit) + if err != nil { + return errors.Annotate(err, "running controller migration") + } + + model, err := runMigration(ctx, dqlite, uuid.String(), schema.ModelDDL(), emptyInit) + if err != nil { + return errors.Annotate(err, "running model migration") + } + + for i, op := range opts { + if err := op(ctx, controller, model); err != nil { + return errors.Annotatef(err, "running bootstrap operation at index %d", i) } } return nil } +func runMigration(ctx context.Context, dqlite *app.App, namespace string, schema Schema, init bootstrapInit) (coredatabase.TxnRunner, error) { + db, err := dqlite.Open(ctx, namespace) + if err != nil { + return nil, errors.Annotatef(err, "opening database for namespace %q", namespace) + } + + if err := pragma.SetPragma(ctx, db, pragma.ForeignKeysPragma, true); err != nil { + return nil, errors.Annotatef(err, "setting foreign keys pragma for namespace %q", namespace) + } + + runner := &txnRunner{db: db} + + migration := NewDBMigration(runner, logger, schema) + if err := migration.Apply(ctx); err != nil { + return nil, errors.Annotatef(err, "creating database with namespace %q schema", namespace) + } + + if err := init(ctx, runner, dqlite); err != nil { + return nil, errors.Annotatef(err, "running init for database with namespace %q", namespace) + } + + return runner, nil +} + // InsertControllerNodeID inserts the node ID of the controller node // into the controller_node table. func InsertControllerNodeID(ctx context.Context, runner coredatabase.TxnRunner, nodeID uint64) error { @@ -248,3 +176,44 @@ VALUES ('0', ?, '127.0.0.1');` return nil }) } + +// txnRunner is the simplest implementation of TxnRunner, wrapping a +// sql.DB reference. It is recruited to run the bootstrap DB migration, +// where we do not yet have access to a transaction runner sourced from +// dbaccessor worker. +type txnRunner struct { + db *sql.DB +} + +func (r *txnRunner) Txn(ctx context.Context, f func(context.Context, *sqlair.TX) error) error { + return errors.Trace(Txn(ctx, sqlair.NewDB(r.db), f)) +} + +func (r *txnRunner) StdTxn(ctx context.Context, f func(context.Context, *sql.Tx) error) error { + return errors.Trace(StdTxn(ctx, r.db, f)) +} + +// bootstrapInit is a type for describing a bootstrap operation that +// initialises a database. +type bootstrapInit = func(ctx context.Context, runner coredatabase.TxnRunner, dqlite *app.App) error + +// controllerBootstrapInit is used to initialise the controller database with +// a controller node ID. The controller node ID is required to be present in +// the controller_node table as this is used for referential integrity. +func controllerBootstrapInit(ctx context.Context, runner coredatabase.TxnRunner, dqlite *app.App) error { + if err := InsertControllerNodeID(ctx, runner, dqlite.ID()); err != nil { + // If the controller node ID already exists, we assume that + // the database has already been bootstrapped. Mask the unique + // constraint error with a more user-friendly error. + if IsErrConstraintUnique(err) { + return errors.AlreadyExistsf("controller node ID") + } + return errors.Annotatef(err, "inserting controller node ID") + } + return nil +} + +// emptyInit is a BootstrapInit type that does nothing. +func emptyInit(context.Context, coredatabase.TxnRunner, *app.App) error { + return nil +} diff --git a/internal/database/bootstrap_test.go b/internal/database/bootstrap_test.go index 0f714158afc..2c51e3e4e84 100644 --- a/internal/database/bootstrap_test.go +++ b/internal/database/bootstrap_test.go @@ -15,6 +15,7 @@ import ( gc "gopkg.in/check.v1" "github.com/juju/juju/core/database" + "github.com/juju/juju/core/model" "github.com/juju/juju/core/network" "github.com/juju/juju/internal/database/app" "github.com/juju/juju/internal/database/client" @@ -31,8 +32,8 @@ func (s *bootstrapSuite) TestBootstrapSuccess(c *gc.C) { // check tests the variadic operation functionality // and ensures that bootstrap applied the DDL. - check := func(ctx context.Context, db database.TxnRunner) error { - return db.StdTxn(ctx, func(ctx context.Context, tx *sql.Tx) error { + check := func(ctx context.Context, controller, model database.TxnRunner) error { + return controller.StdTxn(ctx, func(ctx context.Context, tx *sql.Tx) error { rows, err := tx.QueryContext(ctx, "SELECT COUNT(*) FROM lease_type") if err != nil { return err @@ -76,7 +77,7 @@ func (s *bootstrapSuite) TestBootstrapSuccess(c *gc.C) { }) } - err := BootstrapDqlite(context.Background(), mgr, stubLogger{}, BootstrapControllerConcern(check)) + err := BootstrapDqlite(context.Background(), mgr, model.MustNewUUID(), stubLogger{}, check) c.Assert(err, jc.ErrorIsNil) } diff --git a/internal/database/testing/dqlitesuite.go b/internal/database/testing/dqlitesuite.go index 78c63bc1f8e..889efabec92 100644 --- a/internal/database/testing/dqlitesuite.go +++ b/internal/database/testing/dqlitesuite.go @@ -17,6 +17,7 @@ import ( "sync/atomic" "github.com/canonical/sqlair" + "github.com/juju/errors" "github.com/juju/testing" jc "github.com/juju/testing/checkers" _ "github.com/mattn/go-sqlite3" @@ -131,14 +132,17 @@ func (s *DqliteSuite) TxnRunner() coredatabase.TxnRunner { return s.trackedDB } +// DBApp returns the dqlite app. func (s *DqliteSuite) DBApp() *app.App { return s.dqlite } +// RootPath returns the root path for the dqlite database. func (s *DqliteSuite) RootPath() string { return s.rootPath } +// DBPath returns the path to the dqlite database. func (s *DqliteSuite) DBPath() string { return s.dbPath } @@ -195,6 +199,13 @@ func (s *DqliteSuite) TxnRunnerFactory() func() (coredatabase.TxnRunner, error) } } +// NoopTxnRunner returns a no-op transaction runner. +// Each call to this function will return the same instance, which will return +// a not implemented error when used. +func (s *DqliteSuite) NoopTxnRunner() coredatabase.TxnRunner { + return noopTxnRunner{} +} + func (s *DqliteSuite) cleanupDB(c *gc.C, namespace string, db *sql.DB) { s.mutex.Lock() defer s.mutex.Unlock() @@ -215,3 +226,19 @@ func FindTCPPort(c *gc.C) int { c.Assert(l.Close(), jc.ErrorIsNil) return l.Addr().(*net.TCPAddr).Port } + +type noopTxnRunner struct{} + +// Txn manages the application of a SQLair transaction within which the +// input function is executed. See https://github.com/canonical/sqlair. +// The input context can be used by the caller to cancel this process. +func (noopTxnRunner) Txn(context.Context, func(context.Context, *sqlair.TX) error) error { + return errors.NotImplemented +} + +// StdTxn manages the application of a standard library transaction within +// which the input function is executed. +// The input context can be used by the caller to cancel this process. +func (noopTxnRunner) StdTxn(context.Context, func(context.Context, *sql.Tx) error) error { + return errors.NotImplemented +} diff --git a/juju/testing/apiserver.go b/juju/testing/apiserver.go index 0abb9569349..6c30f9fff64 100644 --- a/juju/testing/apiserver.go +++ b/juju/testing/apiserver.go @@ -6,6 +6,7 @@ package testing import ( "context" "crypto/tls" + "database/sql" "fmt" "net" "net/http" @@ -316,7 +317,7 @@ func (s *ApiServerSuite) setupControllerModel(c *gc.C, controllerCfg controller. c.Assert(err, jc.ErrorIsNil) // Allow "dummy" cloud. - err = InsertDummyCloudType(context.Background(), s.TxnRunner()) + err = InsertDummyCloudType(context.Background(), s.TxnRunner(), s.NoopTxnRunner()) c.Assert(err, jc.ErrorIsNil) // Seed the test database with the controller cloud and credential etc. @@ -430,8 +431,8 @@ func (s *ApiServerSuite) TearDownTest(c *gc.C) { } // InsertDummyCloudType is a db bootstrap option which inserts the dummy cloud type. -func InsertDummyCloudType(ctx context.Context, db database.TxnRunner) error { - return cloudstate.AllowCloudType(ctx, db, 666, "dummy") +func InsertDummyCloudType(ctx context.Context, controller, model database.TxnRunner) error { + return cloudstate.AllowCloudType(ctx, controller, 666, "dummy") } // URL returns a URL for this server with the given path and @@ -650,7 +651,7 @@ func (s *ApiServerSuite) SeedCAASCloud(c *gc.C) { // SeedDatabase the database with a supplied controller config, and dummy // cloud and dummy credentials. func SeedDatabase(c *gc.C, runner database.TxnRunner, controllerConfig controller.Config) { - err := controllerconfigbootstrap.InsertInitialControllerConfig(controllerConfig)(context.Background(), runner) + err := controllerconfigbootstrap.InsertInitialControllerConfig(controllerConfig)(context.Background(), runner, noopTxnRunner{}) c.Assert(err, jc.ErrorIsNil) } @@ -801,3 +802,13 @@ func (f *fakePresence) AgentStatus(agent string) (presence.Status, error) { } return presence.Alive, nil } + +type noopTxnRunner struct{} + +func (noopTxnRunner) Txn(context.Context, func(context.Context, *sqlair.TX) error) error { + return errors.NotImplemented +} + +func (noopTxnRunner) StdTxn(context.Context, func(context.Context, *sql.Tx) error) error { + return errors.NotImplemented +} From b98e191045f5efd8c64e6eff162c55ce874afb0b Mon Sep 17 00:00:00 2001 From: Simon Richardson Date: Thu, 21 Mar 2024 10:21:02 +0000 Subject: [PATCH 2/2] Reuse the model/testing package for UUID generation We have a better pattern for model UUID generation, use that instead of the Must pattern. --- agent/agentbootstrap/bootstrap.go | 2 +- cmd/jujud-controller/agent/agenttest/agent.go | 4 ++-- cmd/jujud/agent/agenttest/agent.go | 4 ++-- core/model/model.go | 9 --------- internal/database/bootstrap_test.go | 4 ++-- 5 files changed, 7 insertions(+), 16 deletions(-) diff --git a/agent/agentbootstrap/bootstrap.go b/agent/agentbootstrap/bootstrap.go index d0b5fe7b283..a9b38e45a4f 100644 --- a/agent/agentbootstrap/bootstrap.go +++ b/agent/agentbootstrap/bootstrap.go @@ -58,7 +58,7 @@ type DqliteInitializerFunc func( mgr database.BootstrapNodeManager, modelUUID model.UUID, logger database.Logger, - concerns ...database.BootstrapOpt, + options ...database.BootstrapOpt, ) error // Logger describes methods for emitting log output. diff --git a/cmd/jujud-controller/agent/agenttest/agent.go b/cmd/jujud-controller/agent/agenttest/agent.go index dac87827d4d..141ce5b9755 100644 --- a/cmd/jujud-controller/agent/agenttest/agent.go +++ b/cmd/jujud-controller/agent/agenttest/agent.go @@ -24,7 +24,7 @@ import ( cmdutil "github.com/juju/juju/cmd/jujud-controller/util" "github.com/juju/juju/controller" coredatabase "github.com/juju/juju/core/database" - "github.com/juju/juju/core/model" + modeltesting "github.com/juju/juju/core/model/testing" "github.com/juju/juju/core/network" "github.com/juju/juju/environs" "github.com/juju/juju/environs/filestorage" @@ -237,7 +237,7 @@ func (s *AgentSuite) PrimeStateAgentVersion(c *gc.C, tag names.Tag, password str err = database.BootstrapDqlite( context.Background(), database.NewNodeManager(conf, true, logger, coredatabase.NoopSlowQueryLogger{}), - model.MustNewUUID(), + modeltesting.GenModelUUID(c), logger, ) c.Assert(err, jc.ErrorIsNil) diff --git a/cmd/jujud/agent/agenttest/agent.go b/cmd/jujud/agent/agenttest/agent.go index d0514ee03ac..730aef027e3 100644 --- a/cmd/jujud/agent/agenttest/agent.go +++ b/cmd/jujud/agent/agenttest/agent.go @@ -21,7 +21,7 @@ import ( agenttools "github.com/juju/juju/agent/tools" "github.com/juju/juju/controller" coredatabase "github.com/juju/juju/core/database" - "github.com/juju/juju/core/model" + modeltesting "github.com/juju/juju/core/model/testing" "github.com/juju/juju/core/network" "github.com/juju/juju/environs" "github.com/juju/juju/environs/filestorage" @@ -171,7 +171,7 @@ func (s *AgentSuite) PrimeStateAgentVersion(c *gc.C, tag names.Tag, password str err = database.BootstrapDqlite( context.Background(), database.NewNodeManager(conf, true, logger, coredatabase.NoopSlowQueryLogger{}), - model.MustNewUUID(), + modeltesting.GenModelUUID(c), logger, ) c.Assert(err, jc.ErrorIsNil) diff --git a/core/model/model.go b/core/model/model.go index acab6562fa4..fc06b7bb10b 100644 --- a/core/model/model.go +++ b/core/model/model.go @@ -85,15 +85,6 @@ func NewUUID() (UUID, error) { return UUID(uuid.String()), nil } -// MustNewUUID is a convenience function for generating a new model uuid. -func MustNewUUID() UUID { - uuid, err := NewUUID() - if err != nil { - panic(err) - } - return uuid -} - // String implements the stringer interface for UUID. func (u UUID) String() string { return string(u) diff --git a/internal/database/bootstrap_test.go b/internal/database/bootstrap_test.go index 2c51e3e4e84..4eeeccffab3 100644 --- a/internal/database/bootstrap_test.go +++ b/internal/database/bootstrap_test.go @@ -15,7 +15,7 @@ import ( gc "gopkg.in/check.v1" "github.com/juju/juju/core/database" - "github.com/juju/juju/core/model" + modeltesting "github.com/juju/juju/core/model/testing" "github.com/juju/juju/core/network" "github.com/juju/juju/internal/database/app" "github.com/juju/juju/internal/database/client" @@ -77,7 +77,7 @@ func (s *bootstrapSuite) TestBootstrapSuccess(c *gc.C) { }) } - err := BootstrapDqlite(context.Background(), mgr, model.MustNewUUID(), stubLogger{}, check) + err := BootstrapDqlite(context.Background(), mgr, modeltesting.GenModelUUID(c), stubLogger{}, check) c.Assert(err, jc.ErrorIsNil) }