Update `fetchBeatsBinary` to be reused in elastic-agent-poc #1984

narph · 2022-01-10T14:28:17Z

made method public to avoid any binary name construction
added download path to download artifacts in the specified location

mergify · 2022-01-10T14:28:46Z

This pull request does not have a backport label. Could you fix it @narph? 🙏
To fixup this pull request, you need to add the backport labels for the needed
branches, such as:

backport-v./d./d./d is the label to automatically backport to the 7./d branch. /d is the digit
NOTE: backport-skip has been added to this pull request.

elasticmachine · 2022-01-10T14:33:19Z

💔 Tests Failed

the below badges are clickable and redirect to their specific view in the CI or DOCS

Expand to view the summary

Build stats

Start Time: 2022-01-17T15:50:07.866+0000
Duration: 62 min 15 sec
Commit: 2e3ecea

Test stats 🧪

Test	Results
Failed	1
Passed	211
Skipped	0
Total	212

Test errors

Expand to view the tests failures

`Initializing / End-To-End Tests / kubernetes_autodiscover_elastic-agent / [empty] – TEST-x86_64-kubernetes-autodiscover-76fd501a-2022-01-17-16:09:29.xml`

no error details

Expand to view the stacktrace

 Test report file /var/lib/jenkins/workspace/PR-1984-9-38bb1d4c-6670-4d1a-9133-0593b100d919/outputs/18.116.170.52/TEST-x86_64-kubernetes-autodiscover-76fd501a-2022-01-17-16:09:29.xml was length 0

🐛 Flaky test report

❕ There are test failures but not known flaky tests.

Expand to view the summary

Genuine test errors

💔 There are test failures but not known flaky tests, most likely a genuine test failure.

Name: Initializing / End-To-End Tests / kubernetes_autodiscover_elastic-agent / [empty] – TEST-x86_64-kubernetes-autodiscover-76fd501a-2022-01-17-16:09:29.xml

🤖 GitHub comments

To re-run your PR in the CI, just comment with:

/test : Re-trigger the build.

mdelapenya · 2022-01-10T15:33:11Z

internal/utils/utils.go

-	filepathFull := tempFile.Name()
+	if downloadPath != "" {
+		filepathFull = downloadPath
+	} else {


Could you move https://github.com/elastic/e2e-testing/pull/1984/files#diff-a7cadd8b7616e655fd258138320b33eb96f7cce8e8663ec92651f49ce07df000R45-R46 right here, covered by the else branch? We will be creating the temp file if and only if the downloadPath is not empty

pkg/downloads/versions.go

pkg/downloads/versions_test.go

mdelapenya · 2022-01-10T15:59:56Z

internal/utils/utils.go

@@ -40,7 +40,8 @@ func GetArchitecture() string {
 // DownloadFile will download a url and store it in a temporary path.
 // It writes to the destination file as it downloads it, without
 // loading the entire file into memory.
-func DownloadFile(url string) (string, error) {
+func DownloadFile(url string, downloadPath string) (string, error) {


I'm looking at the implementation and it's not clear to me if we need to return the path again, as it's passed in the download path. I can picture a new struct (i.e. DownloadRequest) with two attributes: url and downloadPath. The DownloadFile method would receive a pointer to an instance of this struct. If the downloadPath is empty, then create a temporary file (as it does now), populating the download path, otherwise use the value in the request. The method would still return the request's download path.

Wdyt?

Yea if we're already passing in the downloadPath then a struct containing that information could be easily reused/referenced elsewhere. I also tend to go with defined structs as it improves readability and intent

internal/utils/utils.go

pkg/downloads/versions.go

mdelapenya · 2022-01-11T16:12:58Z

@narph how would you see delegating the download of the sha512 file to the handleDownload file? If it receives the new DownloadRequest struct instead of the URL, then it can internally handle the download of both files, the binary and its sha512. Then, we would be able to remove the duplication of the calls to the handleDownload in both scenarios: the artifactory and the bucket.

* update func * fix path * work on download * small fix * remove test * add sha to google * fix typo * add comment (cherry picked from commit d3365c9)

* update func * fix path * work on download * small fix * remove test * add sha to google * fix typo * add comment (cherry picked from commit d3365c9) # Conflicts: # pkg/downloads/versions.go

…2009) * update func * fix path * work on download * small fix * remove test * add sha to google * fix typo * add comment (cherry picked from commit d3365c9) Co-authored-by: Mariana Dima <[email protected]>

…tic-agent-poc (#2010) * Update `fetchBeatsBinary` to be reused in elastic-agent-poc (#1984) * update func * fix path * work on download * small fix * remove test * add sha to google * fix typo * add comment (cherry picked from commit d3365c9) # Conflicts: # pkg/downloads/versions.go * fix: resolve conflicts * fix: update tests after missing backport Co-authored-by: Mariana Dima <[email protected]> Co-authored-by: Manuel de la Peña <[email protected]>

* main: (45 commits) feat: add CentOS 8 support (elastic#2034) fix: set default region for AWS cli (elastic#2053) chore: use Ansible's built-in replace instead of sed (elastic#2048) chore: split stack configuration and start into two tasks (elastic#2044) feat: enable SSH access to users for debugging cloud instances (elastic#2001) fix: use the right branch for 7.17 backports (elastic#2025) SLES15 enablement (elastic#2007) chore: bump stale agent for main (elastic#2014) Update `fetchBeatsBinary` to be reused in elastic-agent-poc (elastic#1984) chore: add resiliency when provisioning the stack (elastic#1990) chore: bump elastic-package to v0.32.1 (elastic#1959) feat: export Fetch&Download methods in the /pkg directory (elastic#1943) bump stack version 8.1.0-dbc834fd (elastic#1948) bump stack version 8.1.0-76902d39 (elastic#1946) chore: retire 7.15 adding 7.17 (elastic#1938) ci: use withAPMEnv (elastic#1917) Update main branch (elastic#1928) bump stack version 8.1.0-befff95a (elastic#1929) chore: properly evaluate how tests are skipped on CI when checking modified files (elastic#1924) bump stack version 8.1.0-60bffc32 (elastic#1921) ...

…1984) * update func * fix path * work on download * small fix * remove test * add sha to google * fix typo * add comment

* ci: use withAPMEnv (#1917) * feat: partial backport for (#1628) * chore: define variable for BeatsLocalPath * fix: update path initialisation in unit tests * chore: pass inline env vars to commands Co-authored-by: Victor Martinez <[email protected]> Co-authored-by: Victor Martinez <[email protected]> * feat: export Fetch&Download methods in the /pkg directory (#1943) * chore: copy internal's version files into the exported pkg public package * chore: run unit tests from top-level goal * chore: use public package instead of the internal one * chore: remove old internal files for habdling downloads * fix: update missing references * Update `fetchBeatsBinary` to be reused in elastic-agent-poc (#1984) * update func * fix path * work on download * small fix * remove test * add sha to google * fix typo * add comment * ci: increase log rotation (#2138) * chore: increase build timeout to 90 minutes (#2139) * feat: support downloading project artifacts for the new bucket layout (#2172) * feat: support for the new bucket structure * feat: calculate bucket URLs using resolvers * chore: support for legacy Beats Old PRs needs to be tested and the binaries could not exist in the new layout * chore: support looking up the bucket in up-to 3 locations 1. new project layout 2. new beats layout 3. legacy beats layout * chore: simplify boolean logic * fix: pass worker environment for versions to the VMs * fix: predefine kibana version * fix: remove extra char * fix: remove extra char * chore: log resolvers * chore: use new project method when fetching artifacts * fix: pass stack version to stack deployment * fix: use artifact to check for ubi8 variant * chore: improve logs in resolvers * fix: apply variants to project resolver Ubi8 is a valid variant for elastic-agent * chore: use Stack version for the dependant beats In the running-on-top-of-beats feature file, the code installs some Beats and we do not want to use the given SHA, because elastic-agent now lives in a separate repo, therefore they do not share commit SHAs * fix: use fleet-ci bucket for elastic-agent and fleet-server * chore: look up both CI buckets * chore: make useCISnapshots configurable on downloads * chore: background processes should not use CI snapshots * feat: separate elastic-agent version from beats version * chore: extract useCISnapshots to a function * chore: use Github_Check_Repo to identify where the Git commit lives * fix: k8s-autodiscover lives in both worlds * fix: missing package * fix: use snapshot version in k8s-autodiscover * fix: missing import Co-authored-by: Victor Martinez <[email protected]> Co-authored-by: Mariana Dima <[email protected]>

update func

34d8a97

mergify bot added the backport-skip Skip notification from the automated backport with mergify label Jan 10, 2022

narph added 2 commits January 10, 2022 15:33

fix path

4b938d4

Merge branch 'main' into download-path

ff34138

narph requested a review from mdelapenya January 10, 2022 15:16

narph self-assigned this Jan 10, 2022