Zip file is kept after a successful Install

[mafredri]

Hi,

I wanted to check what the purpose is behind keeping the zip file after a successful install?

hc-install/releases/exact_version.go

Lines 121 to 127 in a49457e

	zipFilePath, err := d.DownloadAndUnpack(ctx, pv, dstDir)
	if zipFilePath != "" {
	ev.pathsToRemove = append(ev.pathsToRemove, zipFilePath)
	}
	if err != nil {
	return "", err
	}

I understand that ev.Remove(ctx) can be used, but this will remove the binary as well (which makes sense).

I think ideally the zip file would be removed after successful extract since it's never re-used, so it's unclear why we would leave it in /tmp.

Other options:

• Add a Clean method to remove temporary files
• Add a temp dir field (ev.TempDir = myTempDir) where the zip file is stored, to allow manual deletion

(PS. I can submit a PR if you let me know which approach you'd prefer.)

[radeksimko]

Hi @mafredri
Good question!

I provided some answers to a very similar question in the PR which added the LOC you linked to above. #74 (review)

TL;DR it wasn't necessarily intention but rather a pragmatic decision

I don't have too strong opinion on the solutions although I may have slight preference for one which doesn't expand the API or introduce more edge cases, i.e. delete the ZIP file after successful installation.

Before we proceed with choosing a solution though, would you mind describing briefly the concern/problem you experienced (e.g. running out of disk space) and/or some additional context just to help us understand how you use the library?

cc @alenkacz in case you have opinions on this

[radeksimko]

btw. we could also look into a solution which avoids writing the ZIP to disk altogether and keeps it in memory until it is verified for integrity and unzipped, I'm just not 100% confident about the side effects this may introduce (e.g. wrt memory consumption), so we'd need to check with some downstream consumers first.

[mafredri]

Thanks for sharing that bit of context @radeksimko, it's a sensible conclusion given the requirements.

Before we proceed with choosing a solution though, would you mind describing briefly the concern/problem you experienced (e.g. running out of disk space) and/or some additional context just to help us understand how you use the library?

Sure! We use this library in coder/coder, and as part of our test-suite we create at least 9 of these zip files when testing our own code for the installation, but if terraform is missing, we create 20+ (~460MB presently).

I'm sure we can find some workarounds in our test suite, but it'd be nice if we had a robust way to clean up the zips (or didn't need to).

The final, and perhaps most important, reason is that /tmp can often times be located in RAM, and we'd like to avoid increasing RAM usage needlessly on potential deployments that rely on the installation feature.

btw. we could also look into a solution which avoids writing the ZIP to disk altogether and keeps it in memory until it is verified for integrity and unzipped, I'm just not 100% confident about the side effects this may introduce (e.g. wrt memory consumption), so we'd need to check with some downstream consumers first.

I do like this solution, but I also understand the need to be cautious here. I have no strong preference which (memory or disk+delete), but I think one of these would be best. 👍🏻