From 84a835ea40a495d997f1b8498e65280e77cebaed Mon Sep 17 00:00:00 2001 From: Devin Pastoor Date: Tue, 12 Oct 2021 22:33:13 -0400 Subject: [PATCH 1/2] partial response to #163 to add retries for dls --- cran/download-package.go | 40 ++++++++++++++++++++++++++++++++++++---- go.mod | 1 + go.sum | 4 ++++ 3 files changed, 41 insertions(+), 4 deletions(-) diff --git a/cran/download-package.go b/cran/download-package.go index 4e48d8e9..553b8d41 100644 --- a/cran/download-package.go +++ b/cran/download-package.go @@ -8,11 +8,13 @@ import ( "net/http" "os" "path/filepath" + "strconv" "strings" "sync" "time" "github.com/dpastoor/goutils" + "github.com/hashicorp/go-retryablehttp" log "github.com/sirupsen/logrus" "github.com/spf13/afero" ) @@ -67,6 +69,15 @@ func getRepos(ds []PkgDl) map[string]RepoURL { return rpm } +func getIntEnv(val string) int { + ret, err := strconv.Atoi(val) + if err != nil { + log.Error("could not parse PKGR_DL_CONCURRENCY to int - reverting to 10") + return 10 + } + return ret +} + // DownloadPackages downloads a set of packages concurrently // noSecure will allow https fetching without validating the certificate chain. // This occasionally is needed for repos that have self signed or certs not fully verifiable @@ -74,7 +85,14 @@ func getRepos(ds []PkgDl) map[string]RepoURL { func DownloadPackages(fs afero.Fs, ds []PkgDl, baseDir string, rv RVersion, noSecure bool) (*PkgMap, error) { startTime := time.Now() result := NewPkgMap() - sem := make(chan struct{}, 10) + + dlConcurrencyStr, set := os.LookupEnv("PKGR_DL_CONCURRENCY") + if !set { + dlConcurrencyStr = "10" + } + dlConcurrency := getIntEnv(dlConcurrencyStr) + log.Infof("downloading packages with concurrency: %v\n", dlConcurrency) + sem := make(chan struct{}, dlConcurrency) wg := sync.WaitGroup{} rpm := getRepos(ds) for _, r := range rpm { @@ -190,13 +208,27 @@ func DownloadPackage(fs afero.Fs, d PkgDl, dest string, rv RVersion, noSecure bo log.WithField("package", d.Package.Package).Info("downloading package ") var from io.ReadCloser + client := retryablehttp.NewClient() + client.RetryMax = 3 + switch log.GetLevel() { + case log.DebugLevel, log.TraceLevel: + // for some reason client.Logger will not take the default log anyway + // this will also make it such that the log will only write at debug/trace levels + // it currently writes out messages like: + // [DEBUG] GET https://cran.r-project.org/bin/macosx/contrib/4.1/scales_1.1.1.tgz + // and get fed to Printf + lg := log.New() + lg.Level = log.GetLevel() + client.Logger = lg + default: + client.Logger = nil + } - client := &http.Client{} if noSecure { tr := &http.Transport{ - TLSClientConfig: &tls.Config{InsecureSkipVerify : true}, + TLSClientConfig: &tls.Config{InsecureSkipVerify: true}, } - client = &http.Client{Transport: tr} + client.HTTPClient.Transport = tr } if strings.HasPrefix(pkgdl, "http") { diff --git a/go.mod b/go.mod index 32f61f7c..81c993ac 100644 --- a/go.mod +++ b/go.mod @@ -5,6 +5,7 @@ require ( github.com/dpastoor/goutils v1.2.0 github.com/fatih/structs v1.1.0 github.com/fatih/structtag v1.2.0 + github.com/hashicorp/go-retryablehttp v0.7.0 github.com/metrumresearchgroup/command v0.1.0 github.com/mholt/archiver/v3 v3.5.0 github.com/mitchellh/go-homedir v1.1.0 diff --git a/go.sum b/go.sum index 9e2034f3..9c8e7236 100644 --- a/go.sum +++ b/go.sum @@ -158,10 +158,14 @@ github.com/grpc-ecosystem/grpc-gateway v1.16.0/go.mod h1:BDjrQk3hbvj6Nolgz8mAMFb github.com/hashicorp/consul/api v1.1.0/go.mod h1:VmuI/Lkw1nC05EYQWNKwWGbkg+FbDBtguAZLlVdkD9Q= github.com/hashicorp/consul/sdk v0.1.1/go.mod h1:VKf9jXwCTEY1QZP2MOLRhb5i/I/ssyNV1vwHyQBF0x8= github.com/hashicorp/errwrap v1.0.0/go.mod h1:YH+1FKiLXxHSkmPseP+kNlulaMuP3n2brvKWEqk/Jc4= +github.com/hashicorp/go-cleanhttp v0.5.1 h1:dH3aiDG9Jvb5r5+bYHsikaOUIpcM0xvgMXVoDkXMzJM= github.com/hashicorp/go-cleanhttp v0.5.1/go.mod h1:JpRdi6/HCYpAwUzNwuwqhbovhLtngrth3wmdIIUrZ80= +github.com/hashicorp/go-hclog v0.9.2/go.mod h1:5CU+agLiy3J7N7QjHK5d05KxGsuXiQLrjA0H7acj2lQ= github.com/hashicorp/go-immutable-radix v1.0.0/go.mod h1:0y9vanUI8NX6FsYoO3zeMjhV/C5i9g4Q3DwcSNZ4P60= github.com/hashicorp/go-msgpack v0.5.3/go.mod h1:ahLV/dePpqEmjfWmKiqvPkv/twdG7iPBM1vqhUKIvfM= github.com/hashicorp/go-multierror v1.0.0/go.mod h1:dHtQlpGsu+cZNNAkkCN/P3hoUDHhCYQXV3UM06sGGrk= +github.com/hashicorp/go-retryablehttp v0.7.0 h1:eu1EI/mbirUgP5C8hVsTNaGZreBDlYiwC1FZWkvQPQ4= +github.com/hashicorp/go-retryablehttp v0.7.0/go.mod h1:vAew36LZh98gCBJNLH42IQ1ER/9wtLZZ8meHqQvEYWY= github.com/hashicorp/go-rootcerts v1.0.0/go.mod h1:K6zTfqpRlCUIjkwsN4Z+hiSfzSTQa6eBIzfwKfwNnHU= github.com/hashicorp/go-sockaddr v1.0.0/go.mod h1:7Xibr9yA9JjQq1JpNB2Vw7kxv8xerXegt+ozgdvDeDU= github.com/hashicorp/go-syslog v1.0.0/go.mod h1:qPfqrKkXGihmCqbJM2mZgkZGvKG1dFdvsLplgctolz4= From d682a886ad5b828709902a49bbbaf99f3ac91ca5 Mon Sep 17 00:00:00 2001 From: Devin Pastoor Date: Tue, 12 Oct 2021 22:43:01 -0400 Subject: [PATCH 2/2] test: add whitespace to test so doesn't also accidentally target new log message --- integration_tests/baseline/cache_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/integration_tests/baseline/cache_test.go b/integration_tests/baseline/cache_test.go index 9c0b888d..72a57fc3 100644 --- a/integration_tests/baseline/cache_test.go +++ b/integration_tests/baseline/cache_test.go @@ -166,7 +166,7 @@ func TestClean(t *testing.T) { if err != nil { t.Fatalf("error running pkgr install: %s", err) } - downloadLogs := CollectGenericLogs(t, capture, "downloading package") + downloadLogs := CollectGenericLogs(t, capture, "downloading package ") assert.Len(t, downloadLogs, 1, "expected exactly one package to be downloaded") assert.Equal(t, "pillar", downloadLogs[0].Package) toInstallLogs := CollectGenericLogs(t, capture, "package installation plan")