Correctly handle failed migrations #17575

zeripath · 2021-11-06T23:46:23Z

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 reloads the repo before the zeroing out of the repo id from the migration task. This should mean that the repo is much more likely to get deleted but we also make
the migration handler tolerate missing tasks much nicer.

Fix #17571

Signed-off-by: Andrew Thornton art27@cantab.net

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>

modules/task/migrate.go

codecov-commenter · 2021-11-07T00:09:26Z

Codecov Report

Merging #17575 (af3f195) into main (bd613c7) will increase coverage by 0.04%.
The diff coverage is 12.50%.

@@            Coverage Diff             @@
##             main   #17575      +/-   ##
==========================================
+ Coverage   45.49%   45.53%   +0.04%     
==========================================
  Files         793      793              
  Lines       88811    88822      +11     
==========================================
+ Hits        40401    40449      +48     
+ Misses      41897    41857      -40     
- Partials     6513     6516       +3

Impacted Files	Coverage Δ
modules/task/migrate.go	`36.84% <0.00%> (ø)`
routers/web/repo/view.go	`39.79% <0.00%> (-0.41%)`	⬇️
routers/web/user/task.go	`0.00% <0.00%> (ø)`
modules/task/task.go	`59.70% <66.66%> (ø)`
modules/git/repo_base_nogogit.go	`82.85% <0.00%> (-2.86%)`	⬇️
modules/git/utils.go	`68.05% <0.00%> (-2.78%)`	⬇️
modules/process/manager.go	`72.83% <0.00%> (-2.47%)`	⬇️
models/issue_comment.go	`51.31% <0.00%> (-0.59%)`	⬇️
models/webhook.go	`71.50% <0.00%> (+0.53%)`	⬆️
modules/queue/queue_bytefifo.go	`59.88% <0.00%> (+0.59%)`	⬆️
... and 7 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update bd613c7...af3f195. Read the comment docs.

modules/task/migrate.go

zeripath · 2021-11-07T00:14:05Z

Now as a follow-up to this we need to make the UI for the user to view their tasks.

…migration

modules/task/migrate.go

Co-authored-by: Gusted <williamzijl7@hotmail.com>

Signed-off-by: Andrew Thornton <art27@cantab.net>

…migration

* 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>

* 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>

* BUGFIXES * Revert "Fix delete u2f keys bug (go-gitea#18042)" (go-gitea#18107) * Migrating wiki don't require token, so we should move it out of the require form (go-gitea#17645) (go-gitea#18104) * Prevent NPE if gitea uploader fails to open url (go-gitea#18080) (go-gitea#18101) * Reset locale on login (go-gitea#17734) (go-gitea#18100) * Correctly handle failed migrations (go-gitea#17575) (go-gitea#18099) * Instead of using routerCtx just escape the url before routing (go-gitea#18086) (go-gitea#18098) * Quote references to the user table in consistency checks (go-gitea#18072) (go-gitea#18073) * Add NotFound handler (go-gitea#18062) (go-gitea#18067) * Ensure that git repository is closed before transfer (go-gitea#18049) (go-gitea#18057) * Use common sessioner for API and web routes (go-gitea#18114) * TRANSLATION * Fix code search result hint on zh-CN (go-gitea#18053) Signed-off-by: Andrew Thornton <art27@cantab.net>

* BUGFIXES * Revert "Fix delete u2f keys bug (#18042)" (#18107) * Migrating wiki don't require token, so we should move it out of the require form (#17645) (#18104) * Prevent NPE if gitea uploader fails to open url (#18080) (#18101) * Reset locale on login (#17734) (#18100) * Correctly handle failed migrations (#17575) (#18099) * Instead of using routerCtx just escape the url before routing (#18086) (#18098) * Quote references to the user table in consistency checks (#18072) (#18073) * Add NotFound handler (#18062) (#18067) * Ensure that git repository is closed before transfer (#18049) (#18057) * Use common sessioner for API and web routes (#18114) * TRANSLATION * Fix code search result hint on zh-CN (#18053) Signed-off-by: Andrew Thornton <art27@cantab.net> Co-authored-by: Lunny Xiao <xiaolunwen@gmail.com>

* 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>

zeripath added the type/bug label Nov 6, 2021

zeripath added this to the 1.16.0 milestone Nov 6, 2021

zeripath added the backport/v1.15 label Nov 6, 2021

This comment has been minimized.

Sign in to view

GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Nov 6, 2021

zeripath commented Nov 7, 2021

View reviewed changes

modules/task/migrate.go Outdated Show resolved Hide resolved

zeripath commented Nov 7, 2021

View reviewed changes

modules/task/migrate.go Outdated Show resolved Hide resolved

modules/task/migrate.go Outdated Show resolved Hide resolved

Actually let's keep zero out the repo id

01c2d5c

lunny approved these changes Nov 7, 2021

View reviewed changes

GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Nov 7, 2021

Merge branch 'main' into fix-17571-fix-repository-deletion-at-end-of-…

f2f2ddf

…migration

Gusted suggested changes Nov 7, 2021

View reviewed changes

modules/task/migrate.go Outdated Show resolved Hide resolved

zeripath and others added 2 commits November 10, 2021 10:54

Update modules/task/migrate.go

de6ae4a

Co-authored-by: Gusted <williamzijl7@hotmail.com>

formatting

af8db66

Signed-off-by: Andrew Thornton <art27@cantab.net>

Gusted approved these changes Nov 10, 2021

View reviewed changes

GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Nov 10, 2021

lunny and others added 4 commits November 10, 2021 22:45

Merge branch 'main' into fix-17571-fix-repository-deletion-at-end-of-…

f85b721

…migration

Merge branch 'main' into fix-17571-fix-repository-deletion-at-end-of-…

125562e

…migration

Merge branch 'main' into fix-17571-fix-repository-deletion-at-end-of-…

773a756

…migration

Merge branch 'main' into fix-17571-fix-repository-deletion-at-end-of-…

c3a50b8

…migration

wxiaoguang approved these changes Nov 13, 2021

View reviewed changes

wxiaoguang merged commit bab95c3 into go-gitea:main Nov 13, 2021

zeripath deleted the fix-17571-fix-repository-deletion-at-end-of-migration branch December 25, 2021 09:09

zeripath mentioned this pull request Dec 25, 2021

Correctly handle failed migrations (#17575) #18099

Merged

zeripath added the backport/done All backports for this PR have been created label Dec 25, 2021

zeripath mentioned this pull request Dec 28, 2021

Changelog v1.15.9 #18115

Merged

go-gitea locked and limited conversation to collaborators Apr 28, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Correctly handle failed migrations #17575

Correctly handle failed migrations #17575

zeripath commented Nov 6, 2021 •

edited

Loading

This comment has been minimized.

codecov-commenter commented Nov 7, 2021

zeripath commented Nov 7, 2021

Correctly handle failed migrations #17575

Correctly handle failed migrations #17575

Conversation

zeripath commented Nov 6, 2021 • edited Loading

This comment has been minimized.

codecov-commenter commented Nov 7, 2021

Codecov Report

zeripath commented Nov 7, 2021

zeripath commented Nov 6, 2021 •

edited

Loading