From 2c70d10b5744dcd076d237a7a64147cac6116347 Mon Sep 17 00:00:00 2001 From: ale8k Date: Fri, 21 Nov 2025 12:04:11 +0000 Subject: [PATCH 1/2] feat: enable migrate to be dry run Enables migrate being dry run. This runs the migrate command up until the final InitiateMigration call, ensuring pre-checks pass. --- api/controller/controller/controller.go | 3 +- api/controller/controller/controller_test.go | 28 ++++-- .../facades/client/controller/controller.go | 9 +- .../client/controller/controller_test.go | 87 +++++++++++++++++++ apiserver/facades/schema.json | 3 + cmd/juju/commands/migrate.go | 19 +++- cmd/juju/commands/migrate_test.go | 27 +++++- rpc/params/migration.go | 3 +- 8 files changed, 165 insertions(+), 14 deletions(-) diff --git a/api/controller/controller/controller.go b/api/controller/controller/controller.go index 91448c10c1fb..eeaf083c807a 100644 --- a/api/controller/controller/controller.go +++ b/api/controller/controller/controller.go @@ -316,7 +316,7 @@ func (s *MigrationSpec) Validate() error { // The API server supports starting multiple migrations in one request // but we don't need that at the client side yet (and may never) so // this call just supports starting one migration at a time. -func (c *Client) InitiateMigration(spec MigrationSpec) (string, error) { +func (c *Client) InitiateMigration(spec MigrationSpec, dryRun bool) (string, error) { if err := spec.Validate(); err != nil { return "", errors.Annotatef(err, "client-side validation failed") } @@ -327,6 +327,7 @@ func (c *Client) InitiateMigration(spec MigrationSpec) (string, error) { } args := params.InitiateMigrationArgs{ + DryRun: dryRun, Specs: []params.MigrationSpec{{ ModelTag: names.NewModelTag(spec.ModelUUID).String(), TargetInfo: params.MigrationTargetInfo{ diff --git a/api/controller/controller/controller_test.go b/api/controller/controller/controller_test.go index 8e5f1774e7c9..3f0ee6d15059 100644 --- a/api/controller/controller/controller_test.go +++ b/api/controller/controller/controller_test.go @@ -83,6 +83,24 @@ func (s *Suite) TestInitiateMigration(c *gc.C) { s.checkInitiateMigration(c, makeSpec()) } +func (s *Suite) TestInitiateMigrationDryRun(c *gc.C) { + spec := makeSpec() + expectedArgs := specToArgs(spec) + expectedArgs.DryRun = true + + client, stub := makeInitiateMigrationClient(params.InitiateMigrationResults{ + Results: []params.InitiateMigrationResult{{ + MigrationId: "", + }}, + }) + id, err := client.InitiateMigration(spec, true) + c.Assert(err, jc.ErrorIsNil) + c.Check(id, gc.Equals, "") + stub.CheckCalls(c, []jujutesting.StubCall{ + {"Controller.InitiateMigration", []interface{}{expectedArgs}}, + }) +} + func (s *Suite) TestInitiateMigrationEmptyCACert(c *gc.C) { spec := makeSpec() spec.TargetCACert = "" @@ -101,7 +119,7 @@ func (s *Suite) checkInitiateMigration(c *gc.C, spec controller.MigrationSpec) { MigrationId: "id", }}, }) - id, err := client.InitiateMigration(spec) + id, err := client.InitiateMigration(spec, false) c.Assert(err, jc.ErrorIsNil) c.Check(id, gc.Equals, "id") stub.CheckCalls(c, []jujutesting.StubCall{ @@ -142,7 +160,7 @@ func (s *Suite) TestInitiateMigrationError(c *gc.C) { Error: apiservererrors.ServerError(errors.New("boom")), }}, }) - id, err := client.InitiateMigration(makeSpec()) + id, err := client.InitiateMigration(makeSpec(), false) c.Check(id, gc.Equals, "") c.Check(err, gc.ErrorMatches, "boom") } @@ -154,7 +172,7 @@ func (s *Suite) TestInitiateMigrationResultMismatch(c *gc.C) { {MigrationId: "wtf"}, }, }) - id, err := client.InitiateMigration(makeSpec()) + id, err := client.InitiateMigration(makeSpec(), false) c.Check(id, gc.Equals, "") c.Check(err, gc.ErrorMatches, "unexpected number of results returned") } @@ -164,7 +182,7 @@ func (s *Suite) TestInitiateMigrationCallError(c *gc.C) { return errors.New("boom") }) client := controller.NewClient(apiCaller) - id, err := client.InitiateMigration(makeSpec()) + id, err := client.InitiateMigration(makeSpec(), false) c.Check(id, gc.Equals, "") c.Check(err, gc.ErrorMatches, "boom") } @@ -173,7 +191,7 @@ func (s *Suite) TestInitiateMigrationValidationError(c *gc.C) { client, stub := makeInitiateMigrationClient(params.InitiateMigrationResults{}) spec := makeSpec() spec.ModelUUID = "not-a-uuid" - id, err := client.InitiateMigration(spec) + id, err := client.InitiateMigration(spec, false) c.Check(id, gc.Equals, "") c.Check(err, gc.ErrorMatches, "client-side validation failed: model UUID not valid") c.Check(stub.Calls(), gc.HasLen, 0) // API call shouldn't have happened diff --git a/apiserver/facades/client/controller/controller.go b/apiserver/facades/client/controller/controller.go index 558110d0582a..a49c5b7d8fff 100644 --- a/apiserver/facades/client/controller/controller.go +++ b/apiserver/facades/client/controller/controller.go @@ -618,7 +618,7 @@ func (c *ControllerAPI) InitiateMigration(reqArgs params.InitiateMigrationArgs) for i, spec := range reqArgs.Specs { result := &out.Results[i] result.ModelTag = spec.ModelTag - id, err := c.initiateOneMigration(spec) + id, err := c.initiateOneMigration(spec, reqArgs.DryRun) if err != nil { result.Error = apiservererrors.ServerError(err) } else { @@ -628,7 +628,7 @@ func (c *ControllerAPI) InitiateMigration(reqArgs params.InitiateMigrationArgs) return out, nil } -func (c *ControllerAPI) initiateOneMigration(spec params.MigrationSpec) (string, error) { +func (c *ControllerAPI) initiateOneMigration(spec params.MigrationSpec, dryRun bool) (string, error) { modelTag, err := names.ParseModelTag(spec.ModelTag) if err != nil { return "", errors.Annotate(err, "model tag") @@ -694,6 +694,11 @@ func (c *ControllerAPI) initiateOneMigration(spec params.MigrationSpec) (string, return "", errors.Trace(err) } + // We return "" with no error to suggest that a dryrun was requested and successful. + if dryRun { + return "", nil + } + // Trigger the migration. mig, err := hostedState.CreateMigration(state.MigrationSpec{ InitiatedBy: c.apiUser, diff --git a/apiserver/facades/client/controller/controller_test.go b/apiserver/facades/client/controller/controller_test.go index 97e7b9f52de9..7936dd38745d 100644 --- a/apiserver/facades/client/controller/controller_test.go +++ b/apiserver/facades/client/controller/controller_test.go @@ -569,6 +569,93 @@ func (s *controllerSuite) TestInitiateMigration(c *gc.C) { } } +func (s *controllerSuite) TestInitiateMigrationDryRun(c *gc.C) { + st := s.Factory.MakeModel(c, nil) + defer st.Close() + + model, err := st.Model() + c.Assert(err, jc.ErrorIsNil) + + controller.SetPreCheckResult(s, nil) + + args := params.InitiateMigrationArgs{ + DryRun: true, // Set dryrun true and we'll expect no migration to exist. + Specs: []params.MigrationSpec{ + { + ModelTag: model.ModelTag().String(), + TargetInfo: params.MigrationTargetInfo{ + ControllerTag: randomControllerTag(), + ControllerAlias: "target-controller", + Addrs: []string{"3.3.3.3:3333"}, + CACert: "cert", + AuthTag: names.NewUserTag("admin").String(), + Password: "secret", + Token: "token", + }, + }, + }, + } + out, err := s.controller.InitiateMigration(args) + c.Assert(err, jc.ErrorIsNil) + c.Assert(out.Results, gc.HasLen, 1) + + spec := args.Specs[0] + result := out.Results[0] + + c.Assert(result.Error, gc.IsNil) + c.Check(result.ModelTag, gc.Equals, spec.ModelTag) + expectedId := "" // "" Represents a dry run. + c.Check(result.MigrationId, gc.Equals, expectedId) + + // Ensure the migration made did not made it to the db. + _, err = st.LatestMigration() + c.Assert(err, gc.ErrorMatches, errors.NotFoundf("migration").Error()) +} + +func (s *controllerSuite) TestInitiateMigrationDryRunFails(c *gc.C) { + st := s.Factory.MakeModel(c, nil) + defer st.Close() + + model, err := st.Model() + c.Assert(err, jc.ErrorIsNil) + + controller.SetPreCheckResult(s, errors.New("precheck failed")) + + args := params.InitiateMigrationArgs{ + DryRun: true, // Set dryrun true and we'll expect no migration to exist. + Specs: []params.MigrationSpec{ + { + ModelTag: model.ModelTag().String(), + TargetInfo: params.MigrationTargetInfo{ + ControllerTag: randomControllerTag(), + ControllerAlias: "target-controller", + Addrs: []string{"3.3.3.3:3333"}, + CACert: "cert", + AuthTag: names.NewUserTag("admin").String(), + Password: "secret", + Token: "token", + }, + }, + }, + } + out, err := s.controller.InitiateMigration(args) + c.Assert(err, jc.ErrorIsNil) + c.Assert(out.Results, gc.HasLen, 1) + + spec := args.Specs[0] + result := out.Results[0] + + // Ensure the precheck failed is reported on dry runs. + c.Assert(result.Error, gc.ErrorMatches, "precheck failed") + c.Check(result.ModelTag, gc.Equals, spec.ModelTag) + expectedId := "" // "" Represents a dry run. + c.Check(result.MigrationId, gc.Equals, expectedId) + + // Ensure the migration made did not made it to the db. + _, err = st.LatestMigration() + c.Assert(err, gc.ErrorMatches, errors.NotFoundf("migration").Error()) +} + func (s *controllerSuite) TestInitiateMigrationSpecError(c *gc.C) { // Create a hosted model to migrate. st := s.Factory.MakeModel(c, nil) diff --git a/apiserver/facades/schema.json b/apiserver/facades/schema.json index 4b500e153209..d7447bb698c0 100644 --- a/apiserver/facades/schema.json +++ b/apiserver/facades/schema.json @@ -9047,6 +9047,9 @@ "InitiateMigrationArgs": { "type": "object", "properties": { + "dry-run": { + "type": "boolean" + }, "specs": { "type": "array", "items": { diff --git a/cmd/juju/commands/migrate.go b/cmd/juju/commands/migrate.go index 2f849aed1662..007e078fd2b4 100644 --- a/cmd/juju/commands/migrate.go +++ b/cmd/juju/commands/migrate.go @@ -10,6 +10,7 @@ import ( "github.com/juju/cmd/v3" "github.com/juju/collections/set" "github.com/juju/errors" + "github.com/juju/gnuflag" "github.com/juju/names/v5" "gopkg.in/macaroon.v2" @@ -34,6 +35,8 @@ func newMigrateCommand() modelcmd.ModelCommand { // migrateCommand initiates a model migration. type migrateCommand struct { modelcmd.ModelCommandBase + DryRun bool + targetController string // Overridden by tests @@ -44,7 +47,7 @@ type migrateCommand struct { } type migrateAPI interface { - InitiateMigration(spec controller.MigrationSpec) (string, error) + InitiateMigration(spec controller.MigrationSpec, dryRun bool) (string, error) IdentityProviderURL() (string, error) Close() error } @@ -98,6 +101,12 @@ func (c *migrateCommand) Info() *cmd.Info { }) } +func (c *migrateCommand) SetFlags(f *gnuflag.FlagSet) { + c.ModelCommandBase.SetFlags(f) + + f.BoolVar(&c.DryRun, "dry-run", false, "Runs the migration prechecks, but does not start the migration. Returns nothing if the migration can proceed.") +} + // Init implements cmd.Command. func (c *migrateCommand) Init(args []string) error { if len(args) < 1 { @@ -145,10 +154,16 @@ func (c *migrateCommand) Run(ctx *cmd.Context) error { return err } defer func() { _ = api.Close() }() - id, err := api.InitiateMigration(*spec) + id, err := api.InitiateMigration(*spec, c.DryRun) if err != nil { return err } + + if id == "" { + ctx.Infof("Dry run successful: migration can proceed") + return nil + } + ctx.Infof("Migration started with ID %q", id) return nil } diff --git a/cmd/juju/commands/migrate_test.go b/cmd/juju/commands/migrate_test.go index c95746ea35cc..0a1126dac6a2 100644 --- a/cmd/juju/commands/migrate_test.go +++ b/cmd/juju/commands/migrate_test.go @@ -4,6 +4,7 @@ package commands import ( + "errors" "net/http" "net/url" "time" @@ -261,6 +262,19 @@ func (s *MigrateSuite) TestSuccess(c *gc.C) { }) } +func (s *MigrateSuite) TestDryRun(c *gc.C) { + ctx, err := s.makeAndRun(c, "model", "target", "--dry-run") + c.Assert(err, jc.ErrorIsNil) + + c.Check(cmdtesting.Stderr(ctx), gc.Matches, "Dry run successful: migration can proceed\n") +} + +func (s *MigrateSuite) TestDryRunFails(c *gc.C) { + s.api.wantDryRunPrecheckErr = true + _, err := s.makeAndRun(c, "model", "target", "--dry-run") + c.Assert(err, gc.ErrorMatches, "precheck failed") +} + func (s *MigrateSuite) TestSuccessMacaroons(c *gc.C) { err := s.store.UpdateAccount("target", jujuclient.AccountDetails{ User: "targetuser", @@ -439,12 +453,19 @@ func (s *MigrateSuite) makeCommand() modelcmd.ModelCommand { } type fakeMigrateAPI struct { - specSeen *controller.MigrationSpec - identityURL string + specSeen *controller.MigrationSpec + identityURL string + wantDryRunPrecheckErr bool } -func (a *fakeMigrateAPI) InitiateMigration(spec controller.MigrationSpec) (string, error) { +func (a *fakeMigrateAPI) InitiateMigration(spec controller.MigrationSpec, dryRun bool) (string, error) { a.specSeen = &spec + if dryRun { + if a.wantDryRunPrecheckErr { + return "", errors.New("precheck failed") + } + return "", nil + } return "uuid:0", nil } diff --git a/rpc/params/migration.go b/rpc/params/migration.go index 3fed5796a0a5..76adb6bc1830 100644 --- a/rpc/params/migration.go +++ b/rpc/params/migration.go @@ -17,7 +17,8 @@ const MigrationModelHTTPHeader = "X-Juju-Migration-Model-UUID" // InitiateMigrationArgs holds the details required to start one or // more model migrations. type InitiateMigrationArgs struct { - Specs []MigrationSpec `json:"specs"` + Specs []MigrationSpec `json:"specs"` + DryRun bool `json:"dry-run,omitempty"` } // MigrationSpec holds the details required to start the migration of From 57f7cd84ab2f31d5de73bbb56ad641eeeb6925d5 Mon Sep 17 00:00:00 2001 From: ale8k Date: Tue, 13 Jan 2026 14:57:40 +0000 Subject: [PATCH 2/2] docs: update helper comments --- apiserver/facades/client/controller/controller.go | 2 +- apiserver/facades/client/controller/controller_test.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/apiserver/facades/client/controller/controller.go b/apiserver/facades/client/controller/controller.go index a49c5b7d8fff..1e92bc88e8e4 100644 --- a/apiserver/facades/client/controller/controller.go +++ b/apiserver/facades/client/controller/controller.go @@ -694,7 +694,7 @@ func (c *ControllerAPI) initiateOneMigration(spec params.MigrationSpec, dryRun b return "", errors.Trace(err) } - // We return "" with no error to suggest that a dryrun was requested and successful. + // We return "" with no error to signal that a dryrun was requested and successful. if dryRun { return "", nil } diff --git a/apiserver/facades/client/controller/controller_test.go b/apiserver/facades/client/controller/controller_test.go index 7936dd38745d..95427c660ac4 100644 --- a/apiserver/facades/client/controller/controller_test.go +++ b/apiserver/facades/client/controller/controller_test.go @@ -648,7 +648,7 @@ func (s *controllerSuite) TestInitiateMigrationDryRunFails(c *gc.C) { // Ensure the precheck failed is reported on dry runs. c.Assert(result.Error, gc.ErrorMatches, "precheck failed") c.Check(result.ModelTag, gc.Equals, spec.ModelTag) - expectedId := "" // "" Represents a dry run. + expectedId := "" // "" Is the expected response, even on failure. c.Check(result.MigrationId, gc.Equals, expectedId) // Ensure the migration made did not made it to the db.