go getter #6999

azr · 2018-11-14T17:03:51Z

The intent of this PR is to start using hashicorp/go-getter to download files.

The go getter understands a bit more protocols and will do a lot of work we were doing manually.

use go getter
if the checksum of the destination file is correct, skip dl; done in don't get file when checksum of destination is valid go-getter#125
show download progress & continue stopped downloads in Client options + http progress + http byte range requets go-getter#131
allow to download a checksum file in go-get checksum file & guess checksum go-getter#136
make a download cancellable by context in Gracefull (context) cancellation go-getter#133
refactor iso-download docs into a partial
delete a cache download when checksumming fails, first needs add a public type for a checksum error go-getter#168
add more tests for StepDownload
update docs

I'm opening this PR early to check if this is okay with everyone.

I removed the packer.Cache struct and usages as it's functions were only used in StepDownload and replaced it by a packer.CachePath func, that is only used in StepDownload.

close #6991
close #7202
close #5621
close #6443
close #7288
close #7309

boumenot · 2018-11-14T17:57:17Z

builder/azure/arm/builder.go

@@ -51,7 +51,7 @@ func (b *Builder) Prepare(raws ...interface{}) ([]string, error) {
 	return warnings, errs
 }

-func (b *Builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packer.Artifact, error) {


chhaj5236 · 2018-12-05T07:08:35Z

builder/alicloud/ecs/builder.go

@@ -71,7 +71,7 @@ func (b *Builder) Prepare(raws ...interface{}) ([]string, error) {
 	return nil, nil
 }

-func (b *Builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packer.Artifact, error) {
+func (b *Builder) Run(ui packer.Ui, hook packer.Hook) (packer.Artifact, error) {


azr · 2019-01-22T10:53:32Z

website/source/docs/builders/vmware-iso.html.md.erb

@@ -82,6 +82,8 @@ builder.
 -   `iso_url` (string) - A URL to the ISO containing the installation image.
    This URL can be either an HTTP URL or a file URL (or path to a file). If
    this is an HTTP URL, Packer will download it and cache it between runs.
+    Packer uses hashicorp/go-getter in file mode in order to perform a download.
+    See usage of [hashicorp/go-getter](https://github.com/hashicorp/go-getter).


Is it okay to link go-getter docs here, or should I instead describe what can packer do ?

azr · 2019-01-22T11:12:13Z

Howdy @SwampDragons & @rickard-von-essen, I think this is ready for review. If you have time ! 🙏

To recap what I did :

removed the packer.Cache struct and usages as it's functions were only used in StepDownload and replaced it by a packer.CachePath func, that is only used in StepDownload. ( this simplification will also simplify our job to go to hcl2, I believe )
mirrored the downloading features of packer in the hashicorp/go-getter
files are downloaded under hash(file_url+file_hash) in the cache folder; this should lower the chances of collision
downloads are protected by a lockfile named hash(file_url+file_hash).lock in the cache folder; this should prevent things like Packer processes download the same iso in parallel #6991
updated the docs
updated go.mod and vendors
- go mod vendor also removed huge number of fixture/fuzz files in vendor/github.com/pierrec/lz4/fuzz/corpus/

azr · 2019-01-22T13:36:29Z

Ah, I just realised that by downloading to hash(file_url+sha), if the sha was wrong , changing the sha invalidates the cache. I still think this is ok, since this will happen on edge cases, but we will probably get a few issues opened for this.

Edit: I think this is very right after reading this: #6443 (comment)

azr · 2019-02-25T08:37:44Z

Hey @ladar 🙂

Yes because the same url is used twice: if the first download fails, packer will delete the file from the cache, then the second dl will be a 'fresh' download. I think this gives more guaranty that the download/build will work in your daily case for example.

Can you use a hash of the contents provided by iso_checksum_url as your filename? Instead of hashing just the URL?

Not yet, we'd have to change the go-getter for this but this would constitute a breaking change, meaning I have to do an internal RFC because many projects are using the go-getter, etc.

Details

package getter
// [...]
func (c *Client) GetFile() error {}
// ^ this would need to change to something like

// GetOutcome tells where the file was downloaded in 
// case the checksum was used to name the destination file.
func (c *Client) GetFile() (GetOutcome, error) {}

I hope this happens soon ! 🙂

I don't see why an auto retry wouldn't be safe, presuming packer was trying to resume a download, and the checksum fails? You could add a command line option to enable/disable this if you're worried.

I agree this would be nice to have but I just think this PR is already way too big and I think this would constitute another PR. Scoping it will make sure the code review will be easier and that the behaviour of such a feature is well defined. 🙂

azr · 2019-02-25T09:07:07Z

Please wait for review I'd like to add more unit tests !

azr · 2019-02-25T13:30:27Z

There, this should be good ! 🙂

stale

SwampDragons · 2019-02-25T17:30:15Z

Looks like there are some conflicts to be resolved, so I'll wait until tomorrow to review.

azr · 2019-02-26T09:38:23Z

There, this looks better 🙂

azr · 2019-02-26T16:27:18Z

Ah, wait before reviewing. I found another locking issue.
Edit: okay a quick refactor solved it.

azr · 2019-03-06T11:14:46Z

Okay I used the latest go-getter tag in the vendors which imported a lot of vendors - that I also checked in - they come from the fact that the go-getter now understands gcs links.

SwampDragons · 2019-03-12T16:55:16Z

We're gonna want to squash before merging this.

azr · 2019-03-12T17:14:04Z

Yes I wouldn't mind with all these 103 commits ! >.< squash and merge from github ? or do you think I should rather squash & force push ?

SwampDragons · 2019-03-12T17:55:04Z

I prefer squash and force push but I think that's entirely a matter of taste.

* removed packer.Cache and references since packer.Cache is never used except in the download step. The download step now uses the new func packer.CachePath(targetPath) for this, the behavior is the same. * removed download code from packer that was reimplemented into the go-getter library: progress bar, http download restart, checksuming from file, skip already downloaded files, symlinking, make a download cancellable by context. * on windows if packer is running without symlinking rights and we are getting a local file, the file will be copied instead to avoid errors. * added unit tests for step_download that are now CI tested on windows, mac & linux. * files are now downloaded under cache dir `sha1(filename + "?checksum=" + checksum) + file_extension` * since the output dir is based on the source url and the checksum, when the checksum fails, the file is auto deleted. * a download file is protected and locked by a file lock, * updated docs * updated go modules and vendors

azr · 2019-03-13T11:15:40Z

Here we go ! I think it's nice to have a single commit also because it allowed me to describe everything happening in there 🙂

azr · 2019-03-14T09:16:28Z

yay ! 😄

ghost · 2020-03-29T02:17:52Z

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

azr requested review from boumenot, ChrisLundquist, jasmingacic, jen20, prydie, taliesins, YuSungDuk, zhuzhih2017 and a team as code owners November 14, 2018 17:03

boumenot approved these changes Nov 14, 2018

View reviewed changes

azr force-pushed the gogetter branch from 6d50fe0 to dbd83e9 Compare December 4, 2018 15:56

azr requested a review from chhaj5236 as a code owner December 4, 2018 15:56

chhaj5236 approved these changes Dec 5, 2018

View reviewed changes

azr force-pushed the gogetter branch from 2a8ec1b to 67b6101 Compare December 7, 2018 15:11

azr mentioned this pull request Dec 10, 2018

Packer processes download the same iso in parallel #6991

Closed

azr force-pushed the gogetter branch from 5bd0167 to 03e429f Compare January 9, 2019 12:41

This was referenced Jan 11, 2019

Enhancement: allow relative local file for checksums #5621

Closed

"checksums didn't match expected" doesn't say what the checksum was #7202

Closed

azr force-pushed the gogetter branch 4 times, most recently from cdd8727 to 1709f97 Compare January 21, 2019 16:22

azr changed the title ~~[WIP] go getter~~ go getter Jan 21, 2019

azr force-pushed the gogetter branch 2 times, most recently from 2fdffdb to f4f45e4 Compare January 22, 2019 10:48

azr commented Jan 22, 2019

View reviewed changes

SwampDragons added this to the 1.3.5 milestone Jan 23, 2019

azr mentioned this pull request Jan 24, 2019

416 Requested Range Not Satifiable #6443

Closed

azr requested a review from SwampDragons February 25, 2019 13:30

azr changed the title ~~go getter~~ [WIP] go getter Feb 26, 2019

azr changed the title ~~[WIP] go getter~~ go getter Feb 26, 2019

SwampDragons modified the milestones: 1.3.5, v1.4.0 Feb 28, 2019

azr force-pushed the gogetter branch from 56ad11d to a0bf543 Compare March 12, 2019 14:40

azr force-pushed the gogetter branch from d3800c9 to 9f82b75 Compare March 13, 2019 11:14

SwampDragons merged commit e4a189c into master Mar 13, 2019

azr deleted the gogetter branch March 14, 2019 09:16

azr mentioned this pull request Mar 18, 2019

new go-getter provisioner #351

Closed

azr mentioned this pull request Apr 2, 2019

Packer Crash on Windows 2K16 with hyperv-iso #7454

Closed

SwampDragons mentioned this pull request Nov 18, 2019

vmware-iso: error downloading ISO when ISO is stored in local datastore #8378

Closed

sylviamoss mentioned this pull request Dec 16, 2019

Remove unnecessary source_path validation for virtualbox-ovf #8489

Merged

joshuaspence mentioned this pull request Jan 8, 2020

ZIP file is downloaded every time Packer runs #8586

Open

ghost locked and limited conversation to collaborators Mar 29, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

go getter #6999

go getter #6999

azr commented Nov 14, 2018 •

edited

Loading

boumenot Nov 14, 2018

chhaj5236 Dec 5, 2018

azr Jan 22, 2019

azr commented Jan 22, 2019

azr commented Jan 22, 2019 •

edited

Loading

azr commented Feb 25, 2019 •

edited

Loading

azr commented Feb 25, 2019

azr commented Feb 25, 2019

SwampDragons commented Feb 25, 2019

azr commented Feb 26, 2019

azr commented Feb 26, 2019 •

edited

Loading

azr commented Mar 6, 2019

SwampDragons commented Mar 12, 2019

azr commented Mar 12, 2019

SwampDragons commented Mar 12, 2019

azr commented Mar 13, 2019 •

edited

Loading

azr commented Mar 14, 2019

ghost commented Mar 29, 2020

go getter #6999

go getter #6999

Conversation

azr commented Nov 14, 2018 • edited Loading

boumenot Nov 14, 2018

Choose a reason for hiding this comment

chhaj5236 Dec 5, 2018

Choose a reason for hiding this comment

azr Jan 22, 2019

Choose a reason for hiding this comment

azr commented Jan 22, 2019

azr commented Jan 22, 2019 • edited Loading

azr commented Feb 25, 2019 • edited Loading

azr commented Feb 25, 2019

azr commented Feb 25, 2019

SwampDragons commented Feb 25, 2019

azr commented Feb 26, 2019

azr commented Feb 26, 2019 • edited Loading

azr commented Mar 6, 2019

SwampDragons commented Mar 12, 2019

azr commented Mar 12, 2019

SwampDragons commented Mar 12, 2019

azr commented Mar 13, 2019 • edited Loading

azr commented Mar 14, 2019

ghost commented Mar 29, 2020

azr commented Nov 14, 2018 •

edited

Loading

azr commented Jan 22, 2019 •

edited

Loading

azr commented Feb 25, 2019 •

edited

Loading

azr commented Feb 26, 2019 •

edited

Loading

azr commented Mar 13, 2019 •

edited

Loading