Skip to content

Commit

Permalink
Correctly handle failed migrations (go-gitea#17575)
Browse files Browse the repository at this point in the history
* 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 go-gitea#17571

Signed-off-by: Andrew Thornton <art27@cantab.net>
  • Loading branch information
zeripath authored and Stelios Malathouras committed Mar 28, 2022
1 parent 94cb580 commit cf93c27
Show file tree
Hide file tree
Showing 6 changed files with 26 additions and 4 deletions.
3 changes: 3 additions & 0 deletions modules/task/migrate.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
6 changes: 3 additions & 3 deletions modules/task/task.go
Original file line number Diff line number Diff line change
Expand Up @@ -90,15 +90,15 @@ 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,
Status: structs.TaskStatusQueue,
PayloadContent: string(bs),
}

if err := models.CreateTask(&task); err != nil {
if err := models.CreateTask(task); err != nil {
return nil, err
}

Expand Down Expand Up @@ -126,5 +126,5 @@ func CreateMigrateTask(doer, u *models.User, opts base.MigrateOptions) (*models.
return nil, err
}

return &task, nil
return task, nil
}
1 change: 1 addition & 0 deletions options/locale/locale_en-US.ini
Original file line number Diff line number Diff line change
Expand Up @@ -904,6 +904,7 @@ 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.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.
Expand Down
7 changes: 7 additions & 0 deletions routers/web/repo/view.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
7 changes: 7 additions & 0 deletions routers/web/user/task.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ package user

import (
"net/http"
"strconv"

"code.gitea.io/gitea/models"
"code.gitea.io/gitea/modules/context"
Expand All @@ -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,
})
Expand Down
6 changes: 5 additions & 1 deletion templates/repo/migrate/migrating.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -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_no_addr" | Safe}}</p>
{{end}}
<p id="repo_migrating_failed_error"></p>
</div>
{{if and .Failed .Permission.IsAdmin}}
Expand Down

0 comments on commit cf93c27

Please sign in to comment.