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 <art27@cantab.net>
Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>

diff --git a/modules/task/migrate.go b/modules/task/migrate.go
index 707a519356143dfb503fd5941fa5edf91ad8c0ec..7a989ac98f1ab9565a421c11268e007a6d93b095 100644 (file)
--- 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 1c0a87e1f61a44b286db72bcf48ec57c996c4eca..15e0f30ecca6edf4cddae9cda6e75d1d5ad8a076 100644 (file)
--- 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 aad7a68ffa136e7c9e7facffca7695a3678edceb..8e6352a910ba1be15ce09701c058b7aee63fe7fd 100644 (file)
--- 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 <b>%s</b> ...
 migrate.migrating_failed = Migrating from <b>%s</b> 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 120d82b79c818b4b0244b0789f0f3a30afc81bd6..4000e492522bb330d163070e8054359fa4c24f08 100644 (file)
--- 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 8e7b66ef9538e4047b87982f38ab8e55b964cc40..edbc2a89f286c880a40d72e6ca2665beec093d41 100644 (file)
--- 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 cc12243205c15cc619e2c1a3d89796fcdd6bd69a..113518d41422709cd7a11832b29005f37fb5a503 100644 (file)
--- a/templates/repo/migrate/migrating.tmpl
+++ b/templates/repo/migrate/migrating.tmpl
@@ -25,7 +25,11 @@
                                                                <p id="repo_migrating_progress_message"></p>
                                                        </div>
                                                        <div id="repo_migrating_failed" hidden>
-                                                               <p>{{.i18n.Tr "repo.migrate.migrating_failed" .CloneAddr | Safe}}</p>
+                                                               {{if .CloneAddr}}
+                                                                       <p>{{.i18n.Tr "repo.migrate.migrating_failed" .CloneAddr | Safe}}</p>
+                                                               {{else}}
+                                                                       <p>{{.i18n.Tr "repo.migrate.migrating_failed" "<nil>" | Safe}}</p>
+                                                               {{end}}
                                                                <p id="repo_migrating_failed_error"></p>
                                                        </div>
                                                        {{if and .Failed .Permission.IsAdmin}}