-
Notifications
You must be signed in to change notification settings - Fork 3.8k
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
cmd/roachprod: ensure wipe really wipes #41552
cmd/roachprod: ensure wipe really wipes #41552
Conversation
Release note: None
Noticed while debugging the `disk-stall` roachtest. Forcing 0755 permissions on the `cockroach-temp*` directory is done for consistency with every other directory created by cockroach. Release note: None
Previously, `roachprod wipe` was being run by the default user (now `ubuntu`) which meant it couldn't remove files created by root. It was also accidentally swallowing errors. Now `roachprod wipe` runs as root when wiping remote nodes, ensuring it really cleans up any files. The failure to remove root owned files was causing the `disk-stall` tests to be flaky because files created through `charybdefs` are created as root. These root owned files could be removed, but cockroach also creates a root owned directory and the files in that root owned directory could not be removed. If two of the `disk-stall` tests ran on the same cluster, the first could end up leaving around files that the second encountered and complained about. Fixes cockroachdb#41530 Release note: None
I ran |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 1 of 1 files at r1, 1 of 1 files at r2, 2 of 2 files at r3.
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @ajwerner)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @ajwerner)
TFTR! bors r=tbg |
41552: cmd/roachprod: ensure wipe really wipes r=tbg a=petermattis Previously, `roachprod wipe` was being run by the default user (now `ubuntu`) which meant it couldn't remove files created by root. It was also accidentally swallowing errors. Now `roachprod wipe` runs as root when wiping remote nodes, ensuring it really cleans up any files. The failure to remove root owned files was causing the `disk-stall` tests to be flaky because files created through `charybdefs` are created as root. These root owned files could be removed, but cockroach also creates a root owned directory and the files in that root owned directory could not be removed. If two of the `disk-stall` tests ran on the same cluster, the first could end up leaving around files that the second encountered and complained about. Fixes #41530 Release note: None Co-authored-by: Peter Mattis <[email protected]>
Build succeeded |
Previously,
roachprod wipe
was being run by the default user (nowubuntu
) which meant it couldn't remove files created by root. It was alsoaccidentally swallowing errors. Now
roachprod wipe
runs as root whenwiping remote nodes, ensuring it really cleans up any files.
The failure to remove root owned files was causing the
disk-stall
teststo be flaky because files created through
charybdefs
are created as root.These root owned files could be removed, but cockroach also creates a root
owned directory and the files in that root owned directory could not be
removed. If two of the
disk-stall
tests ran on the same cluster, thefirst could end up leaving around files that the second encountered and
complained about.
Fixes #41530
Release note: None