chore: refactor build artifacts methods #689

mdelapenya · 2021-02-02T11:42:13Z

Sorry for this massive PR, I started small but you know.. yak shaving

What does this PR do?

This PR simplifies the generation of artifact names, adding unit tests and refactoring the code. More in detail:

we are adding a helper method to generate build artifact names, removing duplicated code and isolation that functionality in one place, allowing to unit test it. It will support calculating the name for both local paths, GCP buckets and official repositories.
we are moving the code around to always pass the artifact name, generated with above method, to avoid passing all tokens everywhere: artifact, arch, os, extension, etc.
we are updating the artifact name for those artifacts downloaded from Elastic's official repository, because the names are different: they use a -docker-image token that comes right after the version, but in Beats build it uses a .docker token that comes right before the extension.
on the same hand, we are considering that the artifact name could contain "-ubi8" as suffix, which only affects the artifacts to be downloaded.
we are adding a new installer for Docker images, which is able to load the TAR file representing the image into the local docker engine at test time. It will use base TAR installer, and will be used in the standalone mode.
we are consuming local docker images for Fleet's standalone mode, allowing running the standalone tests with local Beats repo (we include unit tests for it)
we are consuming local docker images for Metricbeat test suite, loading the local images into the docker engine before metricbeat container is run. See Implement local mode for metricbeat #695
as a consequence of the above item, we are extracting the common logic to load a docker image to a helper method, so that metricbeat and the agent are able to load images when needed.
we are also adding support for metricbeat to consume docker images from GCP snapshots, affecting Beats' PRs and branches. It always used the official artifacts, and now it's possible to consume a HASH, a pull request, or a CI snapshot, as we already do for Fleet. This change could have come in another PR, but is was so easy to add after this PR's refactors.
we are extracting the download artifact to a helper method, so both metricbeat and the agent are able to download artifacts from GCP, local file system, or official repositories. This is a big change, in which we noticed that there is no need to pass the stale argument to the method calls: we set the version when creating the stale installer and that's all. We are checking that the version used is a PR at installer build time,
we removed an old log entry that was misleading
we removed a wrong field in a log entry, which was causing logrus to complain.

Why is it important?

Code health, avoiding redundant code and reusability.

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 E2E, 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

Run the tests for PRs, snapshots and releases
Run the tests for stale agents (Fleet's upgrade test)

How to test this PR locally

Let's use the existing goals:

make -C e2e fleet-fleet                   # Fleet test suite for fleet mode
make -C e2e fleet-fleet-ci-snapshot       # Fleet test suite for fleet mode for a SHA1
make -C e2e fleet-fleet-pr-ci-snapshot    # Fleet test suite for fleet mode for a PR

Related issues

Built on top of feat: support using local Beats repository in the tests #688
Closes Implement local mode for metricbeat #695

Follow-ups

…hin the method

elasticmachine · 2021-02-02T12:21:18Z

💚 Build Succeeded

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

Expand to view the summary

Build stats

Build Cause: Pull request #689 updated
Start Time: 2021-02-08T16:07:30.039+0000
Duration: 25 min 37 sec
Commit: b4af14a

Test stats 🧪

Test	Results
Failed	0
Passed	104
Skipped	9
Total	113

Trends 🧪

💚 Flaky test report

Tests succeeded.

Expand to view the summary

Test stats 🧪

Test	Results
Failed	0
Passed	104
Skipped	9
Total	113

e2e/utils.go

Local builds uses the same as the CI

In the case we are using a PR , where the version is "pr-12345", then we need to use the base version of the product

mdelapenya · 2021-02-05T16:19:55Z

e2e/utils.go

+// i.e. GetElasticArtifactURL("elastic-agent-$VERSION-amd64.deb", "elastic-agent", "$VERSION")
+// i.e. GetElasticArtifactURL("elastic-agent-$VERSION-x86_64.rpm", "elastic-agent","$VERSION")
+// i.e. GetElasticArtifactURL("elastic-agent-$VERSION-linux-amd64.tar.gz", "elastic-agent","$VERSION")


This change will simplify maintenance in backports

…binary

mdelapenya · 2021-02-08T16:05:09Z

e2e/_suites/fleet/ingest-manager_test.go

@@ -231,21 +233,6 @@ func (imts *IngestManagerTestSuite) processStateOnTheHost(process string, state
 	return checkProcessStateOnTheHost(containerName, process, state)
 }

-// checkElasticAgentVersion returns a fallback version (agentVersionBase) if the version set by the environment is empty
-func checkElasticAgentVersion(version string) string {


Moved (and abstracted) to utils

e2e/utils_test.go

mdelapenya · 2021-02-08T16:31:56Z

This is ready. I'm going to merge it right after the CI passes.

* feat: use local beats directory for elastic-agent installers * docs: document BEATS_LOCAL_PATH variable * chore: check that the local file exists * chore: add unit tests for the BEATS_LOCAL_PATH variable * chore: simplify logic after test coverage * chore: enrich test name * chore: rename test * chore: move default values to the end of the descriptions * chore: add a helper method to build artifact names * chore: build artifact name only once * chore: pass the entire filename to calculate the GCP bucket coordinates * chore: simplify method signature, as the file name is not changed within the method * chore: use buildArtifactName method * chore: pass filename to helper methods to avoid unnecessary calculations * chore: rename variable for consistency * chore: add license headers * chore: use lowercase comparison for artifact name * chore: build artifact changes from Beats CI to Elastic's artifactory * fix: support building artifact names from local Local builds uses the same as the CI * WIP * fix: rename docker-ubi8 installer key * chore: pass a fallback version when building artifact name In the case we are using a PR , where the version is "pr-12345", then we need to use the base version of the product * chore: remove log * fix: do not override artifact name in Docker installer * feat: support consuming the docker images from snapshots or local in standalone mode * chore: add unit tests for fetching docker images from local Beats repo * chore: apply version to Docker installer * chore: support loading metricbeat image from local repository * chore: exrtact loadImage method to docker helper * feat: support consuming CI artifacts in metricbeat * fix: remove non-existent field from logrus log * chore: extract download logic to helper methods We are discarding the stale parameter, because the stale version is set when the installer is required, so we simply download what is requested * chore: fix typo * chore: check agent version before setting it * chore: use the docker client to load the image instead of the docker binary * chore: add unit tests for check method * chore: pass fallback version to check method * fix: properly calculate version for PRs * chore: move PR check logic to a method * chore: make test independent of maintenance branch # Conflicts: # e2e/_suites/fleet/services.go # e2e/utils.go

* feat: use local beats directory for elastic-agent installers * docs: document BEATS_LOCAL_PATH variable * chore: check that the local file exists * chore: add unit tests for the BEATS_LOCAL_PATH variable * chore: simplify logic after test coverage * chore: enrich test name * chore: rename test * chore: move default values to the end of the descriptions * chore: add a helper method to build artifact names * chore: build artifact name only once * chore: pass the entire filename to calculate the GCP bucket coordinates * chore: simplify method signature, as the file name is not changed within the method * chore: use buildArtifactName method * chore: pass filename to helper methods to avoid unnecessary calculations * chore: rename variable for consistency * chore: add license headers * chore: use lowercase comparison for artifact name * chore: build artifact changes from Beats CI to Elastic's artifactory * fix: support building artifact names from local Local builds uses the same as the CI * WIP * fix: rename docker-ubi8 installer key * chore: pass a fallback version when building artifact name In the case we are using a PR , where the version is "pr-12345", then we need to use the base version of the product * chore: remove log * fix: do not override artifact name in Docker installer * feat: support consuming the docker images from snapshots or local in standalone mode * chore: add unit tests for fetching docker images from local Beats repo * chore: apply version to Docker installer * chore: support loading metricbeat image from local repository * chore: exrtact loadImage method to docker helper * feat: support consuming CI artifacts in metricbeat * fix: remove non-existent field from logrus log * chore: extract download logic to helper methods We are discarding the stale parameter, because the stale version is set when the installer is required, so we simply download what is requested * chore: fix typo * chore: check agent version before setting it * chore: use the docker client to load the image instead of the docker binary * chore: add unit tests for check method * chore: pass fallback version to check method * fix: properly calculate version for PRs * chore: move PR check logic to a method * chore: make test independent of maintenance branch # Conflicts: # e2e/_suites/fleet/fleet.go # e2e/_suites/fleet/ingest-manager_test.go # e2e/_suites/fleet/installers.go # e2e/_suites/fleet/services.go # e2e/_suites/fleet/services_test.go

* feat: use local beats directory for elastic-agent installers * docs: document BEATS_LOCAL_PATH variable * chore: check that the local file exists * chore: add unit tests for the BEATS_LOCAL_PATH variable * chore: simplify logic after test coverage * chore: enrich test name * chore: rename test * chore: move default values to the end of the descriptions * chore: add a helper method to build artifact names * chore: build artifact name only once * chore: pass the entire filename to calculate the GCP bucket coordinates * chore: simplify method signature, as the file name is not changed within the method * chore: use buildArtifactName method * chore: pass filename to helper methods to avoid unnecessary calculations * chore: rename variable for consistency * chore: add license headers * chore: use lowercase comparison for artifact name * chore: build artifact changes from Beats CI to Elastic's artifactory * fix: support building artifact names from local Local builds uses the same as the CI * WIP * fix: rename docker-ubi8 installer key * chore: pass a fallback version when building artifact name In the case we are using a PR , where the version is "pr-12345", then we need to use the base version of the product * chore: remove log * fix: do not override artifact name in Docker installer * feat: support consuming the docker images from snapshots or local in standalone mode * chore: add unit tests for fetching docker images from local Beats repo * chore: apply version to Docker installer * chore: support loading metricbeat image from local repository * chore: exrtact loadImage method to docker helper * feat: support consuming CI artifacts in metricbeat * fix: remove non-existent field from logrus log * chore: extract download logic to helper methods We are discarding the stale parameter, because the stale version is set when the installer is required, so we simply download what is requested * chore: fix typo * chore: check agent version before setting it * chore: use the docker client to load the image instead of the docker binary * chore: add unit tests for check method * chore: pass fallback version to check method * fix: properly calculate version for PRs * chore: move PR check logic to a method * chore: make test independent of maintenance branch # Conflicts: # e2e/_suites/fleet/services_test.go

* feat: use local beats directory for elastic-agent installers * docs: document BEATS_LOCAL_PATH variable * chore: check that the local file exists * chore: add unit tests for the BEATS_LOCAL_PATH variable * chore: simplify logic after test coverage * chore: enrich test name * chore: rename test * chore: move default values to the end of the descriptions * chore: add a helper method to build artifact names * chore: build artifact name only once * chore: pass the entire filename to calculate the GCP bucket coordinates * chore: simplify method signature, as the file name is not changed within the method * chore: use buildArtifactName method * chore: pass filename to helper methods to avoid unnecessary calculations * chore: rename variable for consistency * chore: add license headers * chore: use lowercase comparison for artifact name * chore: build artifact changes from Beats CI to Elastic's artifactory * fix: support building artifact names from local Local builds uses the same as the CI * WIP * fix: rename docker-ubi8 installer key * chore: pass a fallback version when building artifact name In the case we are using a PR , where the version is "pr-12345", then we need to use the base version of the product * chore: remove log * fix: do not override artifact name in Docker installer * feat: support consuming the docker images from snapshots or local in standalone mode * chore: add unit tests for fetching docker images from local Beats repo * chore: apply version to Docker installer * chore: support loading metricbeat image from local repository * chore: exrtact loadImage method to docker helper * feat: support consuming CI artifacts in metricbeat * fix: remove non-existent field from logrus log * chore: extract download logic to helper methods We are discarding the stale parameter, because the stale version is set when the installer is required, so we simply download what is requested * chore: fix typo * chore: check agent version before setting it * chore: use the docker client to load the image instead of the docker binary * chore: add unit tests for check method * chore: pass fallback version to check method * fix: properly calculate version for PRs * chore: move PR check logic to a method * chore: make test independent of maintenance branch # Conflicts: # e2e/_suites/fleet/services.go # e2e/utils.go

…tifacts methods (#689) backport for 6.8.x (#713) * chore: refactor Fleet upgrade tests (#671) * chore: use nightly annotation for the Upgrade tests * chore: add two make goals for the nightly use cases - e2e-fleet-nightly: run the nightly tests not using CI snapshots. It should download the binaries from the official artifactory. - e2e-fleet-nightly-ci-snapshots: run the nightly tests using the CI snapshots for a specific SHA commit from Beats, downloading them from a GCP bucket * chore: bump elastic-agent stale version * chore: pass version and state state when creating an installer This will allow selecting the proper binary, depending if we are using a stale agent or a regular one. * fix: append snapshot to the stale version when using CI snapshots * fix: check for version aliases with non-stale versions * chore: store current agent version in the test suite struct * chore: make sure the layout is properly created for TAR installer * chore: add make goals for testing fleet use cases * chore: move Make goals to the e2e Makefile * chore: refactor build artifacts methods (#689) * feat: use local beats directory for elastic-agent installers * docs: document BEATS_LOCAL_PATH variable * chore: check that the local file exists * chore: add unit tests for the BEATS_LOCAL_PATH variable * chore: simplify logic after test coverage * chore: enrich test name * chore: rename test * chore: move default values to the end of the descriptions * chore: add a helper method to build artifact names * chore: build artifact name only once * chore: pass the entire filename to calculate the GCP bucket coordinates * chore: simplify method signature, as the file name is not changed within the method * chore: use buildArtifactName method * chore: pass filename to helper methods to avoid unnecessary calculations * chore: rename variable for consistency * chore: add license headers * chore: use lowercase comparison for artifact name * chore: build artifact changes from Beats CI to Elastic's artifactory * fix: support building artifact names from local Local builds uses the same as the CI * WIP * fix: rename docker-ubi8 installer key * chore: pass a fallback version when building artifact name In the case we are using a PR , where the version is "pr-12345", then we need to use the base version of the product * chore: remove log * fix: do not override artifact name in Docker installer * feat: support consuming the docker images from snapshots or local in standalone mode * chore: add unit tests for fetching docker images from local Beats repo * chore: apply version to Docker installer * chore: support loading metricbeat image from local repository * chore: exrtact loadImage method to docker helper * feat: support consuming CI artifacts in metricbeat * fix: remove non-existent field from logrus log * chore: extract download logic to helper methods We are discarding the stale parameter, because the stale version is set when the installer is required, so we simply download what is requested * chore: fix typo * chore: check agent version before setting it * chore: use the docker client to load the image instead of the docker binary * chore: add unit tests for check method * chore: pass fallback version to check method * fix: properly calculate version for PRs * chore: move PR check logic to a method * chore: make test independent of maintenance branch # Conflicts: # e2e/_suites/fleet/services_test.go

* feat: use local beats directory for elastic-agent installers * docs: document BEATS_LOCAL_PATH variable * chore: check that the local file exists * chore: add unit tests for the BEATS_LOCAL_PATH variable * chore: simplify logic after test coverage * chore: enrich test name * chore: rename test * chore: move default values to the end of the descriptions * chore: add a helper method to build artifact names * chore: build artifact name only once * chore: pass the entire filename to calculate the GCP bucket coordinates * chore: simplify method signature, as the file name is not changed within the method * chore: use buildArtifactName method * chore: pass filename to helper methods to avoid unnecessary calculations * chore: rename variable for consistency * chore: add license headers * chore: use lowercase comparison for artifact name * chore: build artifact changes from Beats CI to Elastic's artifactory * fix: support building artifact names from local Local builds uses the same as the CI * WIP * fix: rename docker-ubi8 installer key * chore: pass a fallback version when building artifact name In the case we are using a PR , where the version is "pr-12345", then we need to use the base version of the product * chore: remove log * fix: do not override artifact name in Docker installer * feat: support consuming the docker images from snapshots or local in standalone mode * chore: add unit tests for fetching docker images from local Beats repo * chore: apply version to Docker installer * chore: support loading metricbeat image from local repository * chore: exrtact loadImage method to docker helper * feat: support consuming CI artifacts in metricbeat * fix: remove non-existent field from logrus log * chore: extract download logic to helper methods We are discarding the stale parameter, because the stale version is set when the installer is required, so we simply download what is requested * chore: fix typo * chore: check agent version before setting it * chore: use the docker client to load the image instead of the docker binary * chore: add unit tests for check method * chore: pass fallback version to check method * fix: properly calculate version for PRs * chore: move PR check logic to a method * chore: make test independent of maintenance branch # Conflicts: # e2e/_suites/fleet/services.go # e2e/utils.go

mdelapenya and others added 14 commits February 1, 2021 18:02

feat: use local beats directory for elastic-agent installers

dd1b287

docs: document BEATS_LOCAL_PATH variable

465ac0d

chore: check that the local file exists

ea13640

chore: add unit tests for the BEATS_LOCAL_PATH variable

505ac24

chore: simplify logic after test coverage

8facf2e

chore: enrich test name

f45c453

chore: rename test

08e180d

chore: move default values to the end of the descriptions

702af00

chore: add a helper method to build artifact names

c647285

chore: build artifact name only once

bf9f341

chore: pass the entire filename to calculate the GCP bucket coordinates

2cf6da7

chore: simplify method signature, as the file name is not changed wit…

29e9dcf

…hin the method

chore: use buildArtifactName method

7a36bdc

chore: pass filename to helper methods to avoid unnecessary calculations

7496b5b

mdelapenya self-assigned this Feb 2, 2021

mdelapenya requested a review from a team February 2, 2021 11:42

kuisathaverat approved these changes Feb 2, 2021

View reviewed changes

cachedout approved these changes Feb 2, 2021

View reviewed changes

e2e/utils.go Outdated Show resolved Hide resolved

mdelapenya added 3 commits February 2, 2021 17:31

chore: rename variable for consistency

a9f3710

chore: add license headers

198e044

chore: use lowercase comparison for artifact name

4bee7c1

cachedout approved these changes Feb 2, 2021

View reviewed changes

mdelapenya added 7 commits February 3, 2021 09:48

chore: build artifact changes from Beats CI to Elastic's artifactory

8e9921e

fix: support building artifact names from local

fe9db23

Local builds uses the same as the CI

WIP

d881e72

Merge branch 'master' into 668-refactor-build-artifact-methods

39341c9

fix: rename docker-ubi8 installer key

b258393

chore: pass a fallback version when building artifact name

983cc42

In the case we are using a PR , where the version is "pr-12345", then we need to use the base version of the product

chore: remove log

e7040b8

mdelapenya commented Feb 5, 2021

View reviewed changes

mdelapenya added 2 commits February 5, 2021 17:20

chore: fix typo

406e764

chore: check agent version before setting it

27b4dbb

mdelapenya requested a review from michalpristas February 5, 2021 16:22

mdelapenya added 5 commits February 8, 2021 10:52

chore: use the docker client to load the image instead of the docker …

a49db02

…binary

chore: add unit tests for check method

abf0c0f

chore: pass fallback version to check method

586c60b

fix: properly calculate version for PRs

f2a03ca

chore: move PR check logic to a method

c12ee5a

mdelapenya commented Feb 8, 2021

View reviewed changes

e2e/utils_test.go Outdated Show resolved Hide resolved

chore: make test independent of maintenance branch

b4af14a

mdelapenya merged commit f4ee90e into elastic:master Feb 8, 2021

mdelapenya mentioned this pull request Feb 8, 2021

chore: refactor build artifacts methods (#689) backport for 7.x #709

Merged

mdelapenya mentioned this pull request Feb 8, 2021

chore: refactor build artifacts methods (#689) backport for 7.11.x #710

Merged

mdelapenya mentioned this pull request Feb 8, 2021

chore: refactor build artifacts methods (#689) backport for 7.10.x #711

Merged

mdelapenya mentioned this pull request Feb 8, 2021

chore: refactor build artifacts methods (#689) backport for 6.8.x #712

Closed

mdelapenya mentioned this pull request Feb 8, 2021

chore: refactor Fleet upgrade tests (#671) | chore: refactor build artifacts methods (#689) backport for 6.8.x #713

Merged

mdelapenya deleted the 668-refactor-build-artifact-methods branch February 9, 2021 18:33

mdelapenya mentioned this pull request Oct 26, 2021

chore: bump elastic-package to v0.25.0 #1721

Merged

8 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore: refactor build artifacts methods #689

chore: refactor build artifacts methods #689

mdelapenya commented Feb 2, 2021 •

edited

Loading

elasticmachine commented Feb 2, 2021 •

edited

Loading

Build stats

Test stats 🧪

Trends 🧪

Test stats 🧪

mdelapenya Feb 5, 2021

mdelapenya Feb 8, 2021

mdelapenya commented Feb 8, 2021

chore: refactor build artifacts methods #689

chore: refactor build artifacts methods #689

Conversation

mdelapenya commented Feb 2, 2021 • edited Loading

What does this PR do?

Why is it important?

Checklist

Author's Checklist

How to test this PR locally

Related issues

Follow-ups

elasticmachine commented Feb 2, 2021 • edited Loading

💚 Build Succeeded

Build stats

Test stats 🧪

Trends 🧪

💚 Flaky test report

Test stats 🧪

mdelapenya Feb 5, 2021

Choose a reason for hiding this comment

mdelapenya Feb 8, 2021

Choose a reason for hiding this comment

mdelapenya commented Feb 8, 2021

mdelapenya commented Feb 2, 2021 •

edited

Loading

elasticmachine commented Feb 2, 2021 •

edited

Loading