Skip to content
This repository was archived by the owner on Sep 17, 2024. It is now read-only.

fix: rename downloaded file using artifact name#1149

Merged
adam-stokes merged 1 commit intoelastic:masterfrom
mdelapenya:fix-download-url-from-gcp
May 6, 2021
Merged

fix: rename downloaded file using artifact name#1149
adam-stokes merged 1 commit intoelastic:masterfrom
mdelapenya:fix-download-url-from-gcp

Conversation

@mdelapenya
Copy link
Copy Markdown
Contributor

What does this PR do?

It renames the Beats binary downloaded from a GCP bucket, using the artifact name (i.e. elastic-agent-8.0.0-SNAPSHOT-x86_64.rpm) that was calculated before that.

I added the following test, but removed it as it's very dependendant on the network and the availability of GCP:

func TestFetchBeatsBinary(t *testing.T) {
	os.Setenv("BEATS_USE_CI_SNAPSHOTS", "true")
	os.Setenv("GITHUB_CHECK_SHA1", "b989478a3003c1b41722b7245586d58d832748a4")
	f, err := FetchBeatsBinary("elastic-agent-8.0.0-SNAPSHOT-x86_64.rpm", "elastic-agent", "8.0.0-SNAPSHOT", "8.0.0-SNAPSHOT", 3, true)
	assert.Nil(t, err)
	defer os.Remove(filepath.Dir(f))
	defer os.Unsetenv("BEATS_USE_CI_SNAPSHOTS")
	defer os.Unsetenv("GITHUB_CHECK_SHA1")

	// name has been sanitised
	assert.True(t, strings.HasSuffix(f, "elastic-agent-8.0.0-SNAPSHOT-x86_64.rpm"))
}

If you agree, we can keep it. Please let me know your thoughts

Why is it important?

In #1136 we added support for Docker-copying files to the running containers, but it was incomplete, as it only handled the name of regular files (with regular I mean those files downloaded from the Elastic repository). But when downloading from GCP buckets, the files contains many HTTP params in the file name, causing the Docker copy AND the execution of commands in the container against that copied file to fail.

Therefore, any PR or merge commit from Beats repo will fail the build because of that.

Checklist

  • My code follows the style guidelines of this project
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have made corresponding change to the default configuration files
  • I have added tests that prove my fix is effective or that my feature works
  • I have run the Unit tests for the CLI, and they are passing locally
  • I have run the End-2-End tests for the suite I'm working on, and they are passing locally
  • I have noticed new Go dependencies (run make notice in the proper directory)

Author's Checklist

  • [ ]

How to test this PR locally

Running a test using a binary for a commit:

SUITE="fleet" TAGS="backend_processes && centos && enroll" TIMEOUT_FACTOR=3 GITHUB_CHECK_SHA1=b989478a3003c1b41722b7245586d58d832748a4 LOG_LEVEL=TRACE BEATS_USE_CI_SNAPSHOTS=true DEVELOPER_MODE=true SKIP_PULL=true make -C e2e functional-test

Related issues

Backports

Needs to be backported to 7.x and 7.13.x

@mdelapenya mdelapenya self-assigned this May 6, 2021
@mdelapenya mdelapenya requested a review from a team May 6, 2021 11:11
@mdelapenya mdelapenya marked this pull request as ready for review May 6, 2021 11:13
@elasticmachine
Copy link
Copy Markdown
Contributor

elasticmachine commented May 6, 2021

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS
Pipeline View Test View Changes Artifacts preview

Expand to view the summary

Build stats

  • Build Cause: Started by user Manuel de la Peña

  • Start Time: 2021-05-06T11:23:17.103+0000

  • Duration: 20 min 51 sec

  • Commit: 4f52d53

Test stats 🧪

Test Results
Failed 0
Passed 159
Skipped 0
Total 159

Trends 🧪

Image of Build Times

Image of Tests

💚 Flaky test report

Tests succeeded.

Expand to view the summary

Test stats 🧪

Test Results
Failed 0
Passed 159
Skipped 0
Total 159

@adam-stokes adam-stokes merged commit fd419f4 into elastic:master May 6, 2021
mergify Bot pushed a commit that referenced this pull request May 6, 2021
(cherry picked from commit fd419f4)
mergify Bot pushed a commit that referenced this pull request May 6, 2021
(cherry picked from commit fd419f4)
adam-stokes pushed a commit that referenced this pull request May 6, 2021
(cherry picked from commit fd419f4)

Co-authored-by: Manuel de la Peña <mdelapenya@gmail.com>
adam-stokes pushed a commit that referenced this pull request May 6, 2021
(cherry picked from commit fd419f4)

Co-authored-by: Manuel de la Peña <mdelapenya@gmail.com>
mdelapenya added a commit to mdelapenya/e2e-testing that referenced this pull request May 6, 2021
* master:
  chore: rename downloaded file (elastic#1149)
  fix: bump stale agent to 7.13-SNAPSHOT (elastic#1146)
@mdelapenya mdelapenya deleted the fix-download-url-from-gcp branch May 19, 2021 05:49
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants