From bab95c3a86f63f521311a77e516be5b6c521e0a4 Mon Sep 17 00:00:00 2001 From: zeripath Date: Sat, 13 Nov 2021 11:28:50 +0000 Subject: [PATCH] Correctly handle failed migrations (#17575) * 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 --- modules/task/migrate.go | 3 +++ modules/task/task.go | 6 +++--- options/locale/locale_en-US.ini | 1 + routers/web/repo/view.go | 7 +++++++ routers/web/user/task.go | 7 +++++++ templates/repo/migrate/migrating.tmpl | 6 +++++- 6 files changed, 26 insertions(+), 4 deletions(-) diff --git a/modules/task/migrate.go b/modules/task/migrate.go index 52f4bb91c..715e76b4a 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 4e782869f..51377df78 100644 --- a/modules/task/task.go +++ b/modules/task/task.go @@ -90,7 +90,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, @@ -98,7 +98,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 } @@ -126,5 +126,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 2632531e2..6fad20c87 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -904,6 +904,7 @@ migrate.migrate = Migrate From %s migrate.migrating = Migrating from %s ... migrate.migrating_failed = Migrating from %s failed. migrate.migrating_failed.error = Error: %s +migrate.migrating_failed_no_addr = Migration failed. 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. diff --git a/routers/web/repo/view.go b/routers/web/repo/view.go index 72f12e9bf..cecd8437b 100644 --- a/routers/web/repo/view.go +++ b/routers/web/repo/view.go @@ -583,6 +583,13 @@ func checkHomeCodeViewable(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 c71d43523..4dbd1b853 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 cc1224320..6df7f0a65 100644 --- a/templates/repo/migrate/migrating.tmpl +++ b/templates/repo/migrate/migrating.tmpl @@ -25,7 +25,11 @@

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