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

Add cleanWorkspaceAfterBuild and cleanWorkspaceBuildOutputAfterBuild #2416

Merged
merged 1 commit into from
Feb 2, 2021

Conversation

andrew-m-leonard
Copy link
Contributor

@andrew-m-leonard andrew-m-leonard commented Jan 28, 2021

Add the facility to clean the build workspace or just the workspace/build/src/build and workspace/target binary output at the end of the build. This provides a large saving of disk space, up to 30Gb for Build Output alone and an additional 6Gb for the whole workspace. The default settings for generated pipelines are:

  • cleanWorkspaceAfterBuild : false
  • cleanWorkspaceBuildOutputAfterBuild : true
    Additionally, cleanWorkspaceAfterBuild can be specified in the jdk platform configuration if a specific platform requires cleaning only.

Signed-off-by: Andrew Leonard [email protected]

@andrew-m-leonard andrew-m-leonard marked this pull request as draft January 28, 2021 13:43
@M-Davies M-Davies added enhancement Issues that enhance the code or documentation of the repo in any way jenkins Issues that enhance or fix our jenkins server labels Jan 28, 2021
@AdamBrousseau
Copy link
Contributor

AdamBrousseau commented Jan 29, 2021

I believe this pr addresses #2296

@andrew-m-leonard andrew-m-leonard marked this pull request as ready for review January 29, 2021 18:43
Copy link
Member

@sxa sxa left a comment

Choose a reason for hiding this comment

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

I really don't like that find construct ... If this is likely to only affect one file can we just handle it explicitly or perhaps find . -type f -name \*..\*? Also I'd recommend removing the verbosity here - maybe just keep the rm lines instead of the bit printing them all out?

@andrew-m-leonard
Copy link
Contributor Author

I really don't like that find construct ... If this is likely to only affect one file can we just handle it explicitly or perhaps find . -type f -name \*..\*? Also I'd recommend removing the verbosity here - maybe just keep the rm lines instead of the bit printing them all out?

It's any file that has ".." in it, the 2 i've discovered so far:

17:51:00  workspace/build/src/build/windows-x86_64-normal-clientANDserver-release/support/gensrc/java.base/java/nio/_the...marker
17:31:58  workspace/build/src/build/windows-x86_64-normal-server-release/jdk/gensrc/java/nio/_the..

@andrew-m-leonard
Copy link
Contributor Author

I really don't like that find construct ... If this is likely to only affect one file can we just handle it explicitly or perhaps find . -type f -name \*..\*? Also I'd recommend removing the verbosity here - maybe just keep the rm lines instead of the bit printing them all out?

Yeah, I thought the print was useful debug for the moment, but i'll remove

@andrew-m-leonard
Copy link
Contributor Author

I really don't like that find construct ... If this is likely to only affect one file can we just handle it explicitly or perhaps find . -type f -name \*..\*? Also I'd recommend removing the verbosity here - maybe just keep the rm lines instead of the bit printing them all out?

Yes, I like the -type f, good idea

@andrew-m-leonard
Copy link
Contributor Author

@sxa The other solution is not use CleanWs, just use rm -rf on the specific folders?
The logic behind CleanWs using DirectoryScanner is just overkill, especially for what we're doing

@sxa
Copy link
Member

sxa commented Jan 30, 2021

@sxa The other solution is not use CleanWs, just use rm -rf on the specific folders?

i'm thinking that's probably the best option - keeps it simple, but if we do that let's add a comment pointing to the issue as an explanation of why we're not using cleanWs

@andrew-m-leonard
Copy link
Contributor Author

run tests

@adoptopenjdk-github-bot
Copy link
Contributor

🟠 PR TESTER RESULT 🟠

❎ Some pipelines failed or the job was aborted! ❎
See the pipeline-build-check below for more information...

@andrew-m-leonard
Copy link
Contributor Author

run tests

@adoptopenjdk-github-bot
Copy link
Contributor

🟠 PR TESTER RESULT 🟠

❎ Some pipelines failed or the job was aborted! ❎
See the pipeline-build-check below for more information...

@andrew-m-leonard
Copy link
Contributor Author

run tests

Copy link
Member

@sxa sxa left a comment

Choose a reason for hiding this comment

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

Looking good now but I won't formally approve til I have another look over it in the morning (unless two other people want to approve in the meantime!)

Would be good to check a few random machines to see how much space workspace/build-scripts is taking and compare it to the same numbers a week after merging this when it should have taken effect across most of the machines. Should particularly help the scaleway arm32 build systems

sxa
sxa previously approved these changes Feb 1, 2021
@sxa sxa dismissed their stale review February 1, 2021 21:15

Clicked wrong button - as per previous comment not doing full approval til tomorrow :-)

@adoptopenjdk-github-bot
Copy link
Contributor

🟢 PR TESTER RESULT 🟢

✅ All pipelines passed! ✅

Copy link
Member

@sxa sxa left a comment

Choose a reason for hiding this comment

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

Other than the naming which I think it's now confusing I think this is ready

pipelines/build/common/build_base_file.groovy Outdated Show resolved Hide resolved
@andrew-m-leonard
Copy link
Contributor Author

run tests

Copy link
Member

@sxa sxa left a comment

Choose a reason for hiding this comment

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

Yep I'll take that - thanks. Clear enough now I reckon :-)

@sxa
Copy link
Member

sxa commented Feb 2, 2021

ping @karianna as you are currently blocking this

@adoptopenjdk-github-bot
Copy link
Contributor

🟠 PR TESTER RESULT 🟠

❎ Some pipelines failed or the job was aborted! ❎
See the pipeline-build-check below for more information...

@andrew-m-leonard
Copy link
Contributor Author

run tests

@adoptopenjdk-github-bot
Copy link
Contributor

🟢 PR TESTER RESULT 🟢

✅ All pipelines passed! ✅

context.cleanWs notFailBuild: true, disableDeferredWipeout: true, deleteDirs: true
// Note: Underlying org.apache DirectoryScanner used by cleanWs has a bug scanning where it misses files containing ".." so use rm -rf instead
// Issue: https://issues.jenkins.io/browse/JENKINS-64779
if (context.WORKSPACE != null && !context.WORKSPACE.isEmpty()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

i think if (context.WORKSPACE) achieves the same thing.

// Issue: https://issues.jenkins.io/browse/JENKINS-64779
if (context.WORKSPACE != null && !context.WORKSPACE.isEmpty()) {
context.println "Cleaning workspace files: " + context.WORKSPACE + "/*"
context.sh(script: "rm -rf " + context.WORKSPACE + "/*")
Copy link
Contributor

@AdamBrousseau AdamBrousseau Feb 5, 2021

Choose a reason for hiding this comment

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

I think this ignores the . hidden directories (eg. .git folder). Which is causing me issues on my win boxes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Issues that enhance the code or documentation of the repo in any way jenkins Issues that enhance or fix our jenkins server
Projects
None yet
Development

Successfully merging this pull request may close these issues.

CleanWs at end of build should be runtime configurable
6 participants