-
Notifications
You must be signed in to change notification settings - Fork 5
fix: empty tmp dir instead of removing it #2203
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
fix: empty tmp dir instead of removing it #2203
Conversation
The tmp directory is used for a number of things including when editig k8s resources using kubectl edit. Removing it after running commands will lead to failures in certain commands. Signed-off-by: Evans Mungai <[email protected]>
Signed-off-by: Evans Mungai <[email protected]>
This PR has been released (on staging) and is available for download with a embedded-cluster-smoke-test-staging-app license ID. Online Installer:
Airgap Installer (may take a few minutes before the airgap bundle is built):
Happy debugging! |
pkg/runtimeconfig/runtimeconfig.go
Outdated
@@ -27,7 +27,23 @@ func Get() *ecv1beta1.RuntimeConfigSpec { | |||
} | |||
|
|||
func Cleanup() { | |||
os.RemoveAll(EmbeddedClusterTmpSubDir()) | |||
emptyTmpDir() |
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.
we already have helpers.RemoveAll
. can you please use that instead? it also has tests
Signed-off-by: Evans Mungai <[email protected]>
0c0d7fb
to
181dce8
Compare
Signed-off-by: Evans Mungai <[email protected]>
Merging as change is unrelated and tests are flaky right now. |
What this PR does / why we need it:
The
tmp
directory is used for a number of things including when edittig k8s resources usingkubectl edit
. Removing it after running commands will lead to failures in certain situations.Which issue(s) this PR fixes:
sc-124106
Does this PR require a test?
Unit test added
Does this PR require a release note?
Does this PR require documentation?
NONE