From f6e029e6c7849d4361abf7f1d749b5d528364ac4 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Thu, 11 May 2023 16:25:46 +0800 Subject: [PATCH] Make repo migration cancelable and fix various bugs (#24605) Replace #12917 Close #24601 Close #12845 ![image](https://github.com/go-gitea/gitea/assets/2114189/39378118-064d-40fb-8396-4579ebf33917) ![image](https://github.com/go-gitea/gitea/assets/2114189/faf37418-191c-46a6-90a8-353141e00e2d) ![image](https://github.com/go-gitea/gitea/assets/2114189/fdc8ee4d-125f-4737-9990-89bcdf9eb388) ![image](https://github.com/go-gitea/gitea/assets/2114189/9a3bd2c2-fe20-4011-81f0-990ed869d139) --------- Co-authored-by: Yarden Shoham Co-authored-by: silverwind Co-authored-by: Giteabot --- models/admin/task.go | 33 +--------- modules/structs/task.go | 9 +-- options/locale/locale_en-US.ini | 4 +- routers/web/repo/migrate.go | 18 ++++++ routers/web/web.go | 1 + services/migrations/gitea_uploader.go | 5 +- services/task/migrate.go | 49 +++++++++------ services/task/task.go | 2 +- templates/repo/migrate/migrating.tmpl | 23 ++++++- web_src/js/features/repo-migrate.js | 90 ++++++++++++++------------- 10 files changed, 128 insertions(+), 106 deletions(-) diff --git a/models/admin/task.go b/models/admin/task.go index 9e661b9997..fc40b13d02 100644 --- a/models/admin/task.go +++ b/models/admin/task.go @@ -17,8 +17,6 @@ import ( "code.gitea.io/gitea/modules/structs" "code.gitea.io/gitea/modules/timeutil" "code.gitea.io/gitea/modules/util" - - "xorm.io/builder" ) // Task represents a task @@ -35,7 +33,7 @@ type Task struct { StartTime timeutil.TimeStamp EndTime timeutil.TimeStamp PayloadContent string `xorm:"TEXT"` - Message string `xorm:"TEXT"` // if task failed, saved the error reason + Message string `xorm:"TEXT"` // if task failed, saved the error reason, it could be a JSON string of TranslatableMessage or a plain message Created timeutil.TimeStamp `xorm:"created"` } @@ -185,14 +183,6 @@ func GetMigratingTask(repoID int64) (*Task, error) { return &task, nil } -// HasFinishedMigratingTask returns if a finished migration task exists for the repo. -func HasFinishedMigratingTask(repoID int64) (bool, error) { - return db.GetEngine(db.DefaultContext). - Where("repo_id=? AND type=? AND status=?", repoID, structs.TaskTypeMigrateRepo, structs.TaskStatusFinished). - Table("task"). - Exist() -} - // GetMigratingTaskByID returns the migrating task by repo's id func GetMigratingTaskByID(id, doerID int64) (*Task, *migration.MigrateOptions, error) { task := Task{ @@ -214,27 +204,6 @@ func GetMigratingTaskByID(id, doerID int64) (*Task, *migration.MigrateOptions, e return &task, &opts, nil } -// FindTaskOptions find all tasks -type FindTaskOptions struct { - Status int -} - -// ToConds generates conditions for database operation. -func (opts FindTaskOptions) ToConds() builder.Cond { - cond := builder.NewCond() - if opts.Status >= 0 { - cond = cond.And(builder.Eq{"status": opts.Status}) - } - return cond -} - -// FindTasks find all tasks -func FindTasks(opts FindTaskOptions) ([]*Task, error) { - tasks := make([]*Task, 0, 10) - err := db.GetEngine(db.DefaultContext).Where(opts.ToConds()).Find(&tasks) - return tasks, err -} - // CreateTask creates a task on database func CreateTask(task *Task) error { return db.Insert(db.DefaultContext, task) diff --git a/modules/structs/task.go b/modules/structs/task.go index c73e51b1ab..ed11a33e28 100644 --- a/modules/structs/task.go +++ b/modules/structs/task.go @@ -6,10 +6,7 @@ package structs // TaskType defines task type type TaskType int -// all kinds of task types -const ( - TaskTypeMigrateRepo TaskType = iota // migrate repository from external or local disk -) +const TaskTypeMigrateRepo TaskType = iota // migrate repository from external or local disk // Name returns the task type name func (taskType TaskType) Name() string { @@ -25,9 +22,9 @@ type TaskStatus int // enumerate all the kinds of task status const ( - TaskStatusQueue TaskStatus = iota // 0 task is queue + TaskStatusQueued TaskStatus = iota // 0 task is queued TaskStatusRunning // 1 task is running - TaskStatusStopped // 2 task is stopped + TaskStatusStopped // 2 task is stopped (never used) TaskStatusFailed // 3 task is failed TaskStatusFinished // 4 task is finished ) diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini index e5963e34c6..ccac8faf99 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -1038,7 +1038,7 @@ migrated_from_fake = Migrated From %[1]s 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.error = Failed to migrate: %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. @@ -1055,6 +1055,8 @@ migrate.migrating_labels = Migrating Labels migrate.migrating_releases = Migrating Releases migrate.migrating_issues = Migrating Issues migrate.migrating_pulls = Migrating Pull Requests +migrate.cancel_migrating_title = Cancel Migration +migrate.cancel_migrating_confirm = Do you want to cancel this migration? mirror_from = mirror of forked_from = forked from diff --git a/routers/web/repo/migrate.go b/routers/web/repo/migrate.go index bbc349a0a5..b918650d1d 100644 --- a/routers/web/repo/migrate.go +++ b/routers/web/repo/migrate.go @@ -10,6 +10,7 @@ import ( "strings" "code.gitea.io/gitea/models" + admin_model "code.gitea.io/gitea/models/admin" "code.gitea.io/gitea/models/db" repo_model "code.gitea.io/gitea/models/repo" user_model "code.gitea.io/gitea/models/user" @@ -257,3 +258,20 @@ func setMigrationContextData(ctx *context.Context, serviceType structs.GitServic ctx.Data["Services"] = append([]structs.GitServiceType{structs.PlainGitService}, structs.SupportedFullGitService...) ctx.Data["service"] = serviceType } + +func MigrateCancelPost(ctx *context.Context) { + migratingTask, err := admin_model.GetMigratingTask(ctx.Repo.Repository.ID) + if err != nil { + log.Error("GetMigratingTask: %v", err) + ctx.Redirect(ctx.Repo.Repository.Link()) + return + } + if migratingTask.Status == structs.TaskStatusRunning { + taskUpdate := &admin_model.Task{ID: migratingTask.ID, Status: structs.TaskStatusFailed, Message: "canceled"} + if err = taskUpdate.UpdateCols("status", "message"); err != nil { + ctx.ServerError("task.UpdateCols", err) + return + } + } + ctx.Redirect(ctx.Repo.Repository.Link()) +} diff --git a/routers/web/web.go b/routers/web/web.go index d0deb845fc..8784b7c5f7 100644 --- a/routers/web/web.go +++ b/routers/web/web.go @@ -939,6 +939,7 @@ func registerRoutes(m *web.Route) { addSettingsRunnersRoutes() addSettingsSecretsRoutes() }, actions.MustEnableActions) + m.Post("/migrate/cancel", repo.MigrateCancelPost) // this handler must be under "settings", otherwise this incomplete repo can't be accessed }, ctxDataSet("PageIsRepoSettings", true, "LFSStartServer", setting.LFS.StartServer)) }, reqSignIn, context.RepoAssignment, context.UnitTypes(), reqRepoAdmin, context.RepoRef()) diff --git a/services/migrations/gitea_uploader.go b/services/migrations/gitea_uploader.go index 0eb34b5fe5..ad4293184e 100644 --- a/services/migrations/gitea_uploader.go +++ b/services/migrations/gitea_uploader.go @@ -923,9 +923,8 @@ func (g *GiteaLocalUploader) CreateReviews(reviews ...*base.Review) error { func (g *GiteaLocalUploader) Rollback() error { if g.repo != nil && g.repo.ID > 0 { g.gitRepo.Close() - if err := models.DeleteRepository(g.doer, g.repo.OwnerID, g.repo.ID); err != nil { - return err - } + + // do not delete the repository, otherwise the end users won't be able to see the last error message } return nil } diff --git a/services/task/migrate.go b/services/task/migrate.go index 03d083e596..98454482b5 100644 --- a/services/task/migrate.go +++ b/services/task/migrate.go @@ -7,8 +7,8 @@ import ( "errors" "fmt" "strings" + "time" - "code.gitea.io/gitea/models" admin_model "code.gitea.io/gitea/models/admin" "code.gitea.io/gitea/models/db" repo_model "code.gitea.io/gitea/models/repo" @@ -28,13 +28,13 @@ import ( func handleCreateError(owner *user_model.User, err error) error { switch { case repo_model.IsErrReachLimitOfRepo(err): - return fmt.Errorf("You have already reached your limit of %d repositories", owner.MaxCreationLimit()) + return fmt.Errorf("you have already reached your limit of %d repositories", owner.MaxCreationLimit()) case repo_model.IsErrRepoAlreadyExist(err): - return errors.New("The repository name is already used") + return errors.New("the repository name is already used") case db.IsErrNameReserved(err): - return fmt.Errorf("The repository name '%s' is reserved", err.(db.ErrNameReserved).Name) + return fmt.Errorf("the repository name '%s' is reserved", err.(db.ErrNameReserved).Name) case db.IsErrNamePatternNotAllowed(err): - return fmt.Errorf("The pattern '%s' is not allowed in a repository name", err.(db.ErrNamePatternNotAllowed).Pattern) + return fmt.Errorf("the pattern '%s' is not allowed in a repository name", err.(db.ErrNamePatternNotAllowed).Pattern) default: return err } @@ -57,22 +57,17 @@ func runMigrateTask(t *admin_model.Task) (err error) { log.Error("FinishMigrateTask[%d] by DoerID[%d] to RepoID[%d] for OwnerID[%d] failed: %v", t.ID, t.DoerID, t.RepoID, t.OwnerID, err) } + log.Error("runMigrateTask[%d] by DoerID[%d] to RepoID[%d] for OwnerID[%d] failed: %v", t.ID, t.DoerID, t.RepoID, t.OwnerID, err) + 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 { + if err := t.UpdateCols("status", "message", "end_time"); err != nil { log.Error("Task UpdateCols failed: %v", err) } - if t.Repo != nil { - if errDelete := models.DeleteRepository(t.Doer, t.OwnerID, t.Repo.ID); errDelete != nil { - log.Error("DeleteRepository: %v", errDelete) - } - } + // then, do not delete the repository, otherwise the users won't be able to see the last error }() if err = t.LoadRepo(); err != nil { @@ -100,7 +95,7 @@ func runMigrateTask(t *admin_model.Task) (err error) { opts.MigrateToRepoID = t.RepoID pm := process.GetManager() - ctx, _, finished := pm.AddContext(graceful.GetManager().ShutdownContext(), fmt.Sprintf("MigrateTask: %s/%s", t.Owner.Name, opts.RepoName)) + ctx, cancel, finished := pm.AddContext(graceful.GetManager().ShutdownContext(), fmt.Sprintf("MigrateTask: %s/%s", t.Owner.Name, opts.RepoName)) defer finished() t.StartTime = timeutil.TimeStampNow() @@ -109,6 +104,23 @@ func runMigrateTask(t *admin_model.Task) (err error) { return } + // check whether the task should be canceled, this goroutine is also managed by process manager + go func() { + for { + select { + case <-time.After(2 * time.Second): + case <-ctx.Done(): + return + } + task, _ := admin_model.GetMigratingTask(t.RepoID) + if task != nil && task.Status != structs.TaskStatusRunning { + log.Debug("MigrateTask[%d] by DoerID[%d] to RepoID[%d] for OwnerID[%d] is canceled due to status is not 'running'", t.ID, t.DoerID, t.RepoID, t.OwnerID) + cancel() + return + } + } + }() + t.Repo, err = migrations.MigrateRepository(ctx, t.Doer, t.Owner.Name, *opts, func(format string, args ...interface{}) { message := admin_model.TranslatableMessage{ Format: format, @@ -118,13 +130,14 @@ func runMigrateTask(t *admin_model.Task) (err error) { t.Message = string(bs) _ = t.UpdateCols("message") }) + if err == nil { log.Trace("Repository migrated [%d]: %s/%s", t.Repo.ID, t.Owner.Name, t.Repo.Name) return } if repo_model.IsErrRepoAlreadyExist(err) { - err = errors.New("The repository name is already used") + err = errors.New("the repository name is already used") return } @@ -132,9 +145,9 @@ func runMigrateTask(t *admin_model.Task) (err error) { err = util.SanitizeErrorCredentialURLs(err) if strings.Contains(err.Error(), "Authentication failed") || strings.Contains(err.Error(), "could not read Username") { - return fmt.Errorf("Authentication failed: %w", err) + return fmt.Errorf("authentication failed: %w", err) } else if strings.Contains(err.Error(), "fatal:") { - return fmt.Errorf("Migration failed: %w", err) + return fmt.Errorf("migration failed: %w", err) } // do not be tempted to coalesce this line with the return diff --git a/services/task/task.go b/services/task/task.go index 4f1ba3a60b..493586a645 100644 --- a/services/task/task.go +++ b/services/task/task.go @@ -95,7 +95,7 @@ func CreateMigrateTask(doer, u *user_model.User, opts base.MigrateOptions) (*adm DoerID: doer.ID, OwnerID: u.ID, Type: structs.TaskTypeMigrateRepo, - Status: structs.TaskStatusQueue, + Status: structs.TaskStatusQueued, PayloadContent: string(bs), } diff --git a/templates/repo/migrate/migrating.tmpl b/templates/repo/migrate/migrating.tmpl index 72c3fe5664..58c453fe54 100644 --- a/templates/repo/migrate/migrating.tmpl +++ b/templates/repo/migrate/migrating.tmpl @@ -7,7 +7,7 @@ {{template "base/alert" .}}
-
+
@@ -32,10 +32,14 @@ {{end}}

- {{if and .Failed .Permission.IsAdmin}} + {{if .Permission.IsAdmin}}
+ {{if .Failed}} + {{else}} + + {{end}}
{{end}}
@@ -45,6 +49,7 @@
+ + + + {{template "base/footer" .}} diff --git a/web_src/js/features/repo-migrate.js b/web_src/js/features/repo-migrate.js index 93d1390b4b..e57348d31b 100644 --- a/web_src/js/features/repo-migrate.js +++ b/web_src/js/features/repo-migrate.js @@ -1,51 +1,55 @@ import $ from 'jquery'; import {hideElem, showElem} from '../utils/dom.js'; -const {appSubUrl, csrfToken} = window.config; +const {appSubUrl} = window.config; export function initRepoMigrationStatusChecker() { - const migrating = $('#repo_migrating'); - hideElem($('#repo_migrating_failed')); - hideElem($('#repo_migrating_failed_image')); - hideElem($('#repo_migrating_progress_message')); - if (migrating) { - const task = migrating.attr('task'); - if (task === undefined) { - return; + const $repoMigrating = $('#repo_migrating'); + if (!$repoMigrating.length) return; + + const task = $repoMigrating.attr('data-migrating-task-id'); + + // returns true if the refresh still need to be called after a while + const refresh = async () => { + const res = await fetch(`${appSubUrl}/user/task/${task}`); + if (res.status !== 200) return true; // continue to refresh if network error occurs + + const data = await res.json(); + + // for all status + if (data.message) { + $('#repo_migrating_progress_message').text(data.message); } - $.ajax({ - type: 'GET', - url: `${appSubUrl}/user/task/${task}`, - data: { - _csrf: csrfToken, - }, - complete(xhr) { - if (xhr.status === 200 && xhr.responseJSON) { - if (xhr.responseJSON.status === 4) { - window.location.reload(); - return; - } else if (xhr.responseJSON.status === 3) { - hideElem($('#repo_migrating_progress')); - hideElem($('#repo_migrating')); - showElem($('#repo_migrating_failed')); - showElem($('#repo_migrating_failed_image')); - $('#repo_migrating_failed_error').text(xhr.responseJSON.message); - return; - } - if (xhr.responseJSON.message) { - showElem($('#repo_migrating_progress_message')); - $('#repo_migrating_progress_message').text(xhr.responseJSON.message); - } - setTimeout(() => { - initRepoMigrationStatusChecker(); - }, 2000); - return; - } - hideElem($('#repo_migrating_progress')); - hideElem($('#repo_migrating')); - showElem($('#repo_migrating_failed')); - showElem($('#repo_migrating_failed_image')); + + // TaskStatusFinished + if (data.status === 4) { + window.location.reload(); + return false; + } + + // TaskStatusFailed + if (data.status === 3) { + hideElem('#repo_migrating_progress'); + hideElem('#repo_migrating'); + showElem('#repo_migrating_failed'); + showElem('#repo_migrating_failed_image'); + $('#repo_migrating_failed_error').text(data.message); + return false; + } + + return true; // continue to refresh + }; + + const syncTaskStatus = async () => { + let doNextRefresh = true; + try { + doNextRefresh = await refresh(); + } finally { + if (doNextRefresh) { + setTimeout(syncTaskStatus, 2000); } - }); - } + } + }; + + syncTaskStatus(); // no await }