Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Artifact deletion in actions ui #27172

Merged
merged 21 commits into from
Feb 18, 2024
Merged
Show file tree
Hide file tree
Changes from 16 commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
72ac7a0
add delete button for artifact
fuxiaohei Sep 19, 2023
fa96647
add cron task to delete need-delete artifacts
fuxiaohei Sep 21, 2023
5646de0
update lint
fuxiaohei Sep 22, 2023
cdaacaa
artifact: only uploadconfirmed artifact can be downloaded from web ui
fuxiaohei Oct 17, 2023
f6843b2
artifact: update web ui deletion js as async syntax
fuxiaohei Oct 17, 2023
4901bc7
artifact: add batch size to delete expired artifacts in cron task
fuxiaohei Oct 17, 2023
2161da6
artifact: update comment and locale
fuxiaohei Oct 19, 2023
4013e2b
Merge branch 'main' into feat/artifact-deletion
fuxiaohei Oct 19, 2023
6ae3299
fix: actions ui locale is in ctx.Locale now
fuxiaohei Oct 19, 2023
0f9196a
Update web_src/js/components/RepoActionView.vue
silverwind Oct 19, 2023
aa1e8b6
Merge branch 'main' into feat/artifact-deletion
GiteaBot Oct 19, 2023
e5d42a3
Merge branch 'main' into feat/artifact-deletion
GiteaBot Oct 19, 2023
56ba94d
refactor: rename ArtifactStatusNeedDelete to ArtifactStatusPendingDel…
fuxiaohei Oct 19, 2023
ace7a73
Merge branch 'main' into feat/artifact-deletion
fuxiaohei Oct 21, 2023
fa06309
fix: update artifact deletion confirm label
fuxiaohei Jan 17, 2024
7032e6d
Merge branch 'main' into feat/artifact-deletion
fuxiaohei Jan 17, 2024
fceea7f
Merge branch 'main' into feat/artifact-deletion
silverwind Feb 15, 2024
4fff2c1
Update artifact-related functions and upddate canDeleteArtifact flag
fuxiaohei Feb 18, 2024
925197e
Merge branch 'main' into feat/artifact-deletion
GiteaBot Feb 18, 2024
171ff63
Merge branch 'main' into feat/artifact-deletion
GiteaBot Feb 18, 2024
650bacb
Merge branch 'main' into feat/artifact-deletion
GiteaBot Feb 18, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 22 additions & 0 deletions models/actions/artifact.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ const (
ArtifactStatusUploadConfirmed // 2, ArtifactStatusUploadConfirmed is the status of an artifact upload that is confirmed
ArtifactStatusUploadError // 3, ArtifactStatusUploadError is the status of an artifact upload that is errored
ArtifactStatusExpired // 4, ArtifactStatusExpired is the status of an artifact that is expired
ArtifactStatusPendingDeletion // 5, ArtifactStatusPendingDeletion is the status of an artifact that is pending deletion
ArtifactStatusDeleted // 6, ArtifactStatusDeleted is the status of an artifact that is deleted
)

func init() {
Expand Down Expand Up @@ -147,8 +149,28 @@ func ListNeedExpiredArtifacts(ctx context.Context) ([]*ActionArtifact, error) {
Where("expired_unix < ? AND status = ?", timeutil.TimeStamp(time.Now().Unix()), ArtifactStatusUploadConfirmed).Find(&arts)
}

// ListNeedDeleteArtifacts returns all artifacts in need-delete status.
// limit is the max number of artifacts to return.
func ListNeedDeleteArtifacts(ctx context.Context, limit int) ([]*ActionArtifact, error) {
arts := make([]*ActionArtifact, 0, 10)
fuxiaohei marked this conversation as resolved.
Show resolved Hide resolved
return arts, db.GetEngine(ctx).
Where("status = ?", ArtifactStatusPendingDeletion).Limit(limit).Find(&arts)
}

// SetArtifactExpired sets an artifact to expired
func SetArtifactExpired(ctx context.Context, artifactID int64) error {
_, err := db.GetEngine(ctx).Where("id=? AND status = ?", artifactID, ArtifactStatusUploadConfirmed).Cols("status").Update(&ActionArtifact{Status: int64(ArtifactStatusExpired)})
return err
}

// SetArtifactNeedDelete sets an artifact to need-delete, cron job will delete it
func SetArtifactNeedDelete(ctx context.Context, runID int64, name string) error {
_, err := db.GetEngine(ctx).Where("run_id=? AND artifact_name=? AND status = ?", runID, name, ArtifactStatusUploadConfirmed).Cols("status").Update(&ActionArtifact{Status: int64(ArtifactStatusPendingDeletion)})
return err
}

// SetArtifactDeleted sets an artifact to deleted
func SetArtifactDeleted(ctx context.Context, artifactID int64) error {
_, err := db.GetEngine(ctx).ID(artifactID).Cols("status").Update(&ActionArtifact{Status: int64(ArtifactStatusDeleted)})
return err
}
1 change: 1 addition & 0 deletions options/locale/locale_en-US.ini
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,7 @@ pin = Pin
unpin = Unpin

artifacts = Artifacts
confirm_delete_artifact = Are you sure you want to delete the artifact '%s' ?

archived = Archived

Expand Down
33 changes: 33 additions & 0 deletions routers/web/repo/actions/view.go
Original file line number Diff line number Diff line change
Expand Up @@ -524,6 +524,31 @@ func ArtifactsView(ctx *context_module.Context) {
ctx.JSON(http.StatusOK, artifactsResponse)
}

func ArtifactsDeleteView(ctx *context_module.Context) {
if !ctx.Repo.CanWrite(unit.TypeActions) {
ctx.Error(http.StatusForbidden, "no permission")
return
}

runIndex := ctx.ParamsInt64("run")
artifactName := ctx.Params("artifact_name")

run, err := actions_model.GetRunByIndex(ctx, ctx.Repo.Repository.ID, runIndex)
if err != nil {
if errors.Is(err, util.ErrNotExist) {
ctx.Error(http.StatusNotFound, err.Error())
return
}
ctx.Error(http.StatusInternalServerError, err.Error())
fuxiaohei marked this conversation as resolved.
Show resolved Hide resolved
return
}
if err = actions_model.SetArtifactNeedDelete(ctx, run.ID, artifactName); err != nil {
ctx.Error(http.StatusInternalServerError, err.Error())
return
}
ctx.JSON(http.StatusOK, struct{}{})
}

func ArtifactsDownloadView(ctx *context_module.Context) {
fuxiaohei marked this conversation as resolved.
Show resolved Hide resolved
runIndex := ctx.ParamsInt64("run")
artifactName := ctx.Params("artifact_name")
Expand Down Expand Up @@ -551,6 +576,14 @@ func ArtifactsDownloadView(ctx *context_module.Context) {
return
}

// if artifacts status is not uploaded-confirmed, treat it as not found
for _, art := range artifacts {
if art.Status != int64(actions_model.ArtifactStatusUploadConfirmed) {
ctx.Error(http.StatusNotFound, "artifact not found")
return
}
}

ctx.Resp.Header().Set("Content-Disposition", fmt.Sprintf("attachment; filename=%s.zip; filename*=UTF-8''%s.zip", url.PathEscape(artifactName), artifactName))

writer := zip.NewWriter(ctx.Resp)
Expand Down
1 change: 1 addition & 0 deletions routers/web/web.go
Original file line number Diff line number Diff line change
Expand Up @@ -1366,6 +1366,7 @@ func registerRoutes(m *web.Route) {
m.Post("/approve", reqRepoActionsWriter, actions.Approve)
m.Post("/artifacts", actions.ArtifactsView)
m.Get("/artifacts/{artifact_name}", actions.ArtifactsDownloadView)
m.Delete("/artifacts/{artifact_name}", actions.ArtifactsDeleteView)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does it require reqRepoActionsWriter permission?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. The check is in the function ArtifactsDeleteView internal code but not here. It's better to move it out of that function.

m.Post("/rerun", reqRepoActionsWriter, actions.Rerun)
})
}, reqRepoActionsReader, actions.MustEnableActions)
Expand Down
38 changes: 37 additions & 1 deletion services/actions/cleanup.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,15 @@ func Cleanup(taskCtx context.Context, olderThan time.Duration) error {
return CleanupArtifacts(taskCtx)
}

// CleanupArtifacts removes expired artifacts and set records expired status
// CleanupArtifacts removes expired add need-deleted artifacts and set records expired status
func CleanupArtifacts(taskCtx context.Context) error {
if err := cleanExpiredArtifacts(taskCtx); err != nil {
return err
}
return cleanNeedDeleteArtifacts(taskCtx)
}

func cleanExpiredArtifacts(taskCtx context.Context) error {
artifacts, err := actions.ListNeedExpiredArtifacts(taskCtx)
if err != nil {
return err
Expand All @@ -40,3 +47,32 @@ func CleanupArtifacts(taskCtx context.Context) error {
}
return nil
}

// deleteArtifactBatchSize is the batch size of deleting artifacts
const deleteArtifactBatchSize = 100

func cleanNeedDeleteArtifacts(taskCtx context.Context) error {
for {
artifacts, err := actions.ListNeedDeleteArtifacts(taskCtx, deleteArtifactBatchSize)
if err != nil {
return err
}
log.Info("Found %d need-deleted artifacts", len(artifacts))
fuxiaohei marked this conversation as resolved.
Show resolved Hide resolved
for _, artifact := range artifacts {
if err := storage.ActionsArtifacts.Delete(artifact.StoragePath); err != nil {
log.Error("Cannot delete artifact %d: %v", artifact.ID, err)
continue
}
Comment on lines +62 to +65
Copy link
Contributor

@wxiaoguang wxiaoguang Feb 18, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the file doesn't exist in the storage (eg: site admin deleted them manually), this function won't function anymore (always "continue", and then will be a deadloop if there are a lot pending but non-existing items)

if err := actions.SetArtifactDeleted(taskCtx, artifact.ID); err != nil {
log.Error("Cannot set artifact %d deleted: %v", artifact.ID, err)
continue
}
log.Info("Artifact %d set deleted", artifact.ID)
}
if len(artifacts) < deleteArtifactBatchSize {
log.Debug("No more need-deleted artifacts")
break
}
}
return nil
}
1 change: 1 addition & 0 deletions templates/repo/actions/view.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
data-locale-status-skipped="{{ctx.Locale.Tr "actions.status.skipped"}}"
data-locale-status-blocked="{{ctx.Locale.Tr "actions.status.blocked"}}"
data-locale-artifacts-title="{{ctx.Locale.Tr "artifacts"}}"
data-locale-confirm-delete-artifact="{{ctx.Locale.Tr "confirm_delete_artifact"}}"
data-locale-show-timestamps="{{ctx.Locale.Tr "show_timestamps"}}"
data-locale-show-log-seconds="{{ctx.Locale.Tr "show_log_seconds"}}"
data-locale-show-full-screen="{{ctx.Locale.Tr "show_full_screen"}}"
Expand Down
15 changes: 14 additions & 1 deletion web_src/js/components/RepoActionView.vue
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import {createApp} from 'vue';
import {toggleElem} from '../utils/dom.js';
import {getCurrentLocale} from '../utils.js';
import {renderAnsi} from '../render/ansi.js';
import {POST} from '../modules/fetch.js';
import {POST, DELETE} from '../modules/fetch.js';

const sfc = {
name: 'RepoActionView',
Expand Down Expand Up @@ -200,6 +200,12 @@ const sfc = {
return await resp.json();
},

async deleteArtifact(name) {
if (!window.confirm(this.locale.confirmDeleteArtifact.replace('%s', name))) return;
await DELETE(`${this.run.link}/artifacts/${name}`);
await this.loadJob();
},

async fetchJob() {
const logCursors = this.currentJobStepsStates.map((it, idx) => {
// cursor is used to indicate the last position of the logs
Expand Down Expand Up @@ -329,6 +335,8 @@ export function initRepositoryActionView() {
cancel: el.getAttribute('data-locale-cancel'),
rerun: el.getAttribute('data-locale-rerun'),
artifactsTitle: el.getAttribute('data-locale-artifacts-title'),
areYouSure: el.getAttribute('data-locale-are-you-sure'),
confirmDeleteArtifact: el.getAttribute('data-locale-confirm-delete-artifact'),
rerun_all: el.getAttribute('data-locale-rerun-all'),
showTimeStamps: el.getAttribute('data-locale-show-timestamps'),
showLogSeconds: el.getAttribute('data-locale-show-log-seconds'),
Expand Down Expand Up @@ -404,6 +412,9 @@ export function initRepositoryActionView() {
<a class="job-artifacts-link" target="_blank" :href="run.link+'/artifacts/'+artifact.name">
<SvgIcon name="octicon-file" class="ui text black job-artifacts-icon"/>{{ artifact.name }}
</a>
<a v-if="run.canRerun" @click="deleteArtifact(artifact.name)" class="job-artifacts-delete">
fuxiaohei marked this conversation as resolved.
Show resolved Hide resolved
<SvgIcon name="octicon-trash" class="ui text black job-artifacts-icon"/>
</a>
</li>
</ul>
</div>
Expand Down Expand Up @@ -528,6 +539,8 @@ export function initRepositoryActionView() {
.job-artifacts-item {
margin: 5px 0;
padding: 6px;
display: flex;
justify-content: space-between;
}

.job-artifacts-list {
Expand Down
2 changes: 2 additions & 0 deletions web_src/js/svg.js
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ import octiconStrikethrough from '../../public/assets/img/svg/octicon-strikethro
import octiconSync from '../../public/assets/img/svg/octicon-sync.svg';
import octiconTable from '../../public/assets/img/svg/octicon-table.svg';
import octiconTag from '../../public/assets/img/svg/octicon-tag.svg';
import octiconTrash from '../../public/assets/img/svg/octicon-trash.svg';
import octiconTriangleDown from '../../public/assets/img/svg/octicon-triangle-down.svg';
import octiconX from '../../public/assets/img/svg/octicon-x.svg';
import octiconXCircleFill from '../../public/assets/img/svg/octicon-x-circle-fill.svg';
Expand Down Expand Up @@ -137,6 +138,7 @@ const svgs = {
'octicon-sync': octiconSync,
'octicon-table': octiconTable,
'octicon-tag': octiconTag,
'octicon-trash': octiconTrash,
'octicon-triangle-down': octiconTriangleDown,
'octicon-x': octiconX,
'octicon-x-circle-fill': octiconXCircleFill,
Expand Down