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

feat(api/admin): add endpoint to clean pending / running builds #863

Merged
merged 8 commits into from
May 30, 2023

Conversation

ecrupper
Copy link
Contributor

Oftentimes during upgrades, network blips, or failures in worker-server communication, builds, steps, and or services, never make it out of pending or running states. This endpoint gives platform admins the power to clean build resources that have been pending or running for longer than a specified time as well as give the builds a specified error message.

@ecrupper ecrupper self-assigned this May 26, 2023
@ecrupper ecrupper requested a review from a team as a code owner May 26, 2023 18:00
@ecrupper ecrupper marked this pull request as draft May 26, 2023 18:00
@codecov
Copy link

codecov bot commented May 26, 2023

Codecov Report

Merging #863 (c1199a6) into main (f7c3c6b) will increase coverage by 0.13%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #863      +/-   ##
==========================================
+ Coverage   66.13%   66.26%   +0.13%     
==========================================
  Files         297      300       +3     
  Lines       13876    13930      +54     
==========================================
+ Hits         9177     9231      +54     
  Misses       4252     4252              
  Partials      447      447              
Impacted Files Coverage Δ
database/build/clean.go 100.00% <100.00%> (ø)
database/service/clean.go 100.00% <100.00%> (ø)
database/step/clean.go 100.00% <100.00%> (ø)

@ecrupper ecrupper marked this pull request as ready for review May 26, 2023 18:46
Copy link
Member

@wass3rw3rk wass3rw3rk left a comment

Choose a reason for hiding this comment

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

i wouldn't mind seeing some INFO-level logging between each table cleanup.

database/build/clean.go Show resolved Hide resolved
database/service/clean.go Show resolved Hide resolved

// ensure the mock expects the name query
_mock.ExpectExec(`UPDATE "builds" SET "status"=$1,"error"=$2,"finished"=$3,"deploy_payload"=$4 WHERE created < $5 AND (status = 'running' OR status = 'pending')`).
WithArgs("error", "msg", time.Now().UTC().Unix(), AnyArgument{}, 3).
Copy link
Member

Choose a reason for hiding this comment

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

Is this going to create a flaky test? Because timestamp is calculated both in the test and in the code under test.

Ditto for the step and service clean tests.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It would if we were comparing the objects, but all we do is check the count in the test

Copy link
Contributor

@plyr4 plyr4 left a comment

Choose a reason for hiding this comment

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

nice

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants