From 012e45a4c1bbcf48bb7c1328caa7db5f30440cad Mon Sep 17 00:00:00 2001 From: zeripath Date: Sat, 25 Dec 2021 15:45:51 +0000 Subject: [PATCH] Correctly handle failed migrations (#17575) (#18099) * Correctly handle failed migrations There is a bug in handling failed migrations whereby the migration task gets decoupled from the migration repository. This leads to a failure of the task to get deleted with the repository and also leads to the migration failed page resulting in a ISE. This PR removes the zeroing out of the task id from the migration but also makes the migration handler tolerate missing tasks much nicer. Fix #17571 Signed-off-by: Andrew Thornton Co-authored-by: wxiaoguang --- modules/task/migrate.go | 3 +++ modules/task/task.go | 6 +++--- options/locale/locale_en-US.ini | 11 ++++++----- routers/web/repo/view.go | 7 +++++++ routers/web/user/task.go | 7 +++++++ templates/repo/migrate/migrating.tmpl | 6 +++++- 6 files changed, 31 insertions(+), 9 deletions(-) diff --git a/modules/task/migrate.go b/modules/task/migrate.go index 707a5193561..7a989ac98f1 100644 --- a/modules/task/migrate.go +++ b/modules/task/migrate.go @@ -58,6 +58,9 @@ func runMigrateTask(t *models.Task) (err error) { t.EndTime = timeutil.TimeStampNow() t.Status = structs.TaskStatusFailed t.Message = err.Error() + // Ensure that the repo loaded before we zero out the repo ID from the task - thus ensuring that we can delete it + _ = t.LoadRepo() + t.RepoID = 0 if err := t.UpdateCols("status", "errors", "repo_id", "end_time"); err != nil { log.Error("Task UpdateCols failed: %v", err) diff --git a/modules/task/task.go b/modules/task/task.go index 1c0a87e1f61..15e0f30ecca 100644 --- a/modules/task/task.go +++ b/modules/task/task.go @@ -92,7 +92,7 @@ func CreateMigrateTask(doer, u *models.User, opts base.MigrateOptions) (*models. return nil, err } - var task = models.Task{ + var task = &models.Task{ DoerID: doer.ID, OwnerID: u.ID, Type: structs.TaskTypeMigrateRepo, @@ -100,7 +100,7 @@ func CreateMigrateTask(doer, u *models.User, opts base.MigrateOptions) (*models. PayloadContent: string(bs), } - if err := models.CreateTask(&task); err != nil { + if err := models.CreateTask(task); err != nil { return nil, err } @@ -128,5 +128,5 @@ func CreateMigrateTask(doer, u *models.User, opts base.MigrateOptions) (*models. return nil, err } - return &task, nil + return task, nil } diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini index aad7a68ffa1..8e6352a910b 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -896,11 +896,12 @@ migrate.migrate = Migrate From %s migrate.migrating = Migrating from %s ... migrate.migrating_failed = Migrating from %s failed. migrate.migrating_failed.error = Error: %s -migrate.github.description = Migrating data from Github.com or Github Enterprise. -migrate.git.description = Migrating or Mirroring git data from Git services -migrate.gitlab.description = Migrating data from GitLab.com or Self-Hosted gitlab server. -migrate.gitea.description = Migrating data from Gitea.com or Self-Hosted Gitea server. -migrate.gogs.description = Migrating data from notabug.org or other Self-Hosted Gogs server. +migrate.github.description = Migrate data from github.com or other Github instances. +migrate.git.description = Migrate a repository only from any Git service. +migrate.gitlab.description = Migrate data from gitlab.com or other GitLab instances. +migrate.gitea.description = Migrate data from gitea.com or other Gitea instances. +migrate.gogs.description = Migrate data from notabug.org or other Gogs instances. +migrate.onedev.description = Migrate data from code.onedev.io or other OneDev instances. migrate.migrating_git = Migrating Git Data migrate.migrating_topics = Migrating Topics migrate.migrating_milestones = Migrating Milestones diff --git a/routers/web/repo/view.go b/routers/web/repo/view.go index 120d82b79c8..4000e492522 100644 --- a/routers/web/repo/view.go +++ b/routers/web/repo/view.go @@ -619,6 +619,13 @@ func Home(ctx *context.Context) { if ctx.Repo.Repository.IsBeingCreated() { task, err := models.GetMigratingTask(ctx.Repo.Repository.ID) if err != nil { + if models.IsErrTaskDoesNotExist(err) { + ctx.Data["Repo"] = ctx.Repo + ctx.Data["CloneAddr"] = "" + ctx.Data["Failed"] = true + ctx.HTML(http.StatusOK, tplMigrating) + return + } ctx.ServerError("models.GetMigratingTask", err) return } diff --git a/routers/web/user/task.go b/routers/web/user/task.go index 8e7b66ef953..edbc2a89f28 100644 --- a/routers/web/user/task.go +++ b/routers/web/user/task.go @@ -6,6 +6,7 @@ package user import ( "net/http" + "strconv" "code.gitea.io/gitea/models" "code.gitea.io/gitea/modules/context" @@ -16,6 +17,12 @@ import ( func TaskStatus(ctx *context.Context) { task, opts, err := models.GetMigratingTaskByID(ctx.ParamsInt64("task"), ctx.User.ID) if err != nil { + if models.IsErrTaskDoesNotExist(err) { + ctx.JSON(http.StatusNotFound, map[string]interface{}{ + "error": "task `" + strconv.FormatInt(ctx.ParamsInt64("task"), 10) + "` does not exist", + }) + return + } ctx.JSON(http.StatusInternalServerError, map[string]interface{}{ "err": err, }) diff --git a/templates/repo/migrate/migrating.tmpl b/templates/repo/migrate/migrating.tmpl index cc12243205c..113518d4142 100644 --- a/templates/repo/migrate/migrating.tmpl +++ b/templates/repo/migrate/migrating.tmpl @@ -25,7 +25,11 @@

{{if and .Failed .Permission.IsAdmin}}