Skip to content
This repository has been archived by the owner on Nov 1, 2022. It is now read-only.

Make clone cleanups more robust #2788

Merged
merged 4 commits into from
Jan 27, 2020

Conversation

2opremio
Copy link
Contributor

@2opremio 2opremio commented Jan 27, 2020

  • Fixes a systematic clone leak (introduced in Read-only mode #1807 , released in 1.14 ) in every bump of the sync tag. This probably justifies a new patch release and it probably fixes Flux may be leaking files in /tmp #2713 . I still don't understand how this has escaped our radar until Flux may be leaking files in /tmp #2713
  • Removes the temporary cloning directory if git clone fails for whatever reason
  • It stops hiding underlying errors inExport.Clean() and prints them wherever a logger is available (not perfect, but better).

@2opremio
Copy link
Contributor Author

As a future optimization, we don't really need a working directory for bumping and pushing the tag, which would save quite a bit of space in big repos (e.g. in #2713 ~1GB of data would be saved)

@2opremio 2opremio added the bug label Jan 27, 2020
Copy link
Member

@hiddeco hiddeco left a comment

Choose a reason for hiding this comment

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

Given the fact that the errors were silenced and a repository with just YAMLs shouldn’t be really big in size for most clusters, I can understand why we didn’t notice earlier.

Looks good to me, thanks Fons 🎖

@squaremo
Copy link
Member

we don't really need a working directory for bumping and pushing the tag

You do need to have the changed tag visible locally only when it has been pushed to the upstream repo, though.

Copy link
Member

@squaremo squaremo left a comment

Choose a reason for hiding this comment

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

Looks reasonable, thanks Fons. I honestly cannot remember why the clone was not cleaned up -- I either persuaded myself that it was not necessary, or I just forgot. Owps!

pkg/git/repo.go Outdated Show resolved Hide resolved
@2opremio 2opremio merged commit 2079bc8 into fluxcd:master Jan 27, 2020
@2opremio 2opremio deleted the more-robust-clone-cleanup branch January 27, 2020 14:13
@2opremio 2opremio added this to the 1.18.0 milestone Jan 27, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Flux may be leaking files in /tmp
3 participants