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

Remove 5-day-old zipped logs from buildslave dirs every night #200

Merged
merged 1 commit into from Feb 23, 2016

Conversation

@edunham
Copy link
Contributor

edunham commented Jan 19, 2016

Low disk space command from https://github.com/servo/servo/wiki/Buildbot-administration#handling-low-disk-space . We should no longer need to escape the semicolon, since Bash eats semicolons but Salt doesn't (right?)

Salt cron syntax from https://docs.saltstack.com/en/latest/ref/states/all/salt.states.cron.html

Review on Reviewable

@@ -58,3 +58,9 @@ buildbot-github-listener:
- mode: 644
- watch_in:
- service: buildbot-github-listener

find /home/servo/buildbot/master/*/*.bz2 -mtime +5 -exec rm {} ;:

This comment has been minimized.

@emilio

emilio Jan 19, 2016

Member

I think you can do -delete instead of -exec rm {} ; (which may or may not need the semicolon) :P

This comment has been minimized.

@edunham

edunham Jan 19, 2016

Author Contributor

Yep, that works. Fixed; thanks for catching that.

@aneeshusa
Copy link
Member

aneeshusa commented Jan 22, 2016

I'd suggest using a more suggestive id and putting the actual command as the name parameter, like so:

remove-old-build-logs:
  cron.present:
    - name: 'find /home/servo/buildbot/master/*/*.bz2 -mtime +5 -delete'
    - user: root
    - minute: 1 
    - hour: 0

I also recommend quoting complicated commands like these to make it clear that they are strings and avoid ambiguity in parsing, but strictly speaking the quotes are optional.

This makes it easier to see what the state is for and easier to reference it from another state. Here's the Salt documentation about names. Additionally, for the cron states in particular, the state id will become an identifier argument to the state, and using a stable identifier will allow for more robust updates in the future. The link @edunham posted to the Salt docs has more information about this behavior. (FYI, the link you posted is for the latest version of Salt, but we're not running 2015.8 yet; click the "2015.5" button when looking at Salt docs to get the right version.)

@larsbergstrom
Copy link
Contributor

larsbergstrom commented Feb 5, 2016

@edunham Can you address this when you get a chance? We hit another out of disk space on buildbot master today :-(

@bors-servo
Copy link
Contributor

bors-servo commented Feb 11, 2016

The latest upstream changes (presumably #148) made this pull request unmergeable. Please resolve the merge conflicts.

@edunham edunham force-pushed the edunham:remove-old-logs branch from 0a06727 to b05e418 Feb 22, 2016
@edunham
Copy link
Contributor Author

edunham commented Feb 22, 2016

Overwrote old branch with @aneeshusa 's recommended change applied to latest master

@larsbergstrom
Copy link
Contributor

larsbergstrom commented Feb 23, 2016

@bors-servo
Copy link
Contributor

bors-servo commented Feb 23, 2016

📌 Commit b05e418 has been approved by larsbergstrom

@bors-servo
Copy link
Contributor

bors-servo commented Feb 23, 2016

Testing commit b05e418 with merge c2c71b6...

bors-servo added a commit that referenced this pull request Feb 23, 2016
Remove 5-day-old zipped logs from buildslave dirs every night

Low disk space command from https://github.com/servo/servo/wiki/Buildbot-administration#handling-low-disk-space . We should no longer need to escape the semicolon, since Bash eats semicolons but Salt doesn't (right?)

Salt cron syntax from https://docs.saltstack.com/en/latest/ref/states/all/salt.states.cron.html

<!-- Reviewable:start -->
[<img src="https://reviewable.io/review_button.png" height=40 alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/saltfs/200)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Feb 23, 2016

☀️ Test successful - travis

@bors-servo bors-servo merged commit b05e418 into servo:master Feb 23, 2016
2 checks passed
2 checks passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
homu Test successful
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

5 participants
You can’t perform that action at this time.