fix: use declarative style for complete variant of the elastic-agent #28526

mdelapenya · 2021-10-19T10:33:07Z

What does this PR do?

This PR does a few things:

Reverts two commits from @andrewvc, that were added to support a programmatic manner to create Variants:
- 467f58f - Revert "[elastic-agent] Use -complete in docker image name, not tag ([elastic-agent] Use -complete in docker image name, not tag #27399)" (54 minutes ago) <Manuel de la Peña>
- 08c6c20 - Revert "[elastic-agent] Fix docker tar.gz generation for complete image ([elastic-agent] Fix docker tar.gz generation for complete image #27621)" (55 minutes ago) <Manuel de la Peña>
Moves the logic to create a variant to the declarative way, which means using the packages.yml descriptor file. AS we already had an ubi8 variant, and we recently added a cloud one, adding a complete variant means replicating the building blocks to form the Docker image.
Partially reverts the logic to set and retrieve the Variants of a PackageSpec: instead of looping through the variants, we simply delegate to the new variant declared in the YML file.
Changes the logic in the Dockerfile template to, instead of using the Variant, using the image name field, so that the NodeJS runtime is properly added for that specific variant.

Why is it important?

We realised that CI builds were generating incorrect binaries for the agent, as the elastic-agent docker image (TAR.GZ binary) was pushed to the internal GCP bucket only once, as described in #27608. Then, any consumer of those binaries was always receiving a +500MB file when downloading it from the bucket. An example of a consumer is the e2e-testing framework, which gets a binary for a merge commit or a PR in Beats project, and runs the E2E tests against those non-released delta binaries. We noticed the download times for the default artifact increased up to the double, also missing the complete TAR.GZ binary in the GCP bucket.

With this fix we expect the CI artifacts, which are super useful for bisecting errors, will be generated properly again.

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 added an entry in CHANGELOG.next.asciidoc or CHANGELOG-developer.next.asciidoc.

Author's Checklist

[ ]

How to test this PR locally

cd x-pack/elastic-agent
PLATFORMS="linux/amd64" mage package

After a while:

ls -l x-pack/elastic-agent/build/distributions | grep docker
-rw-r--r--  1 mdelapenya  staff  244359238 26 Aug 13:51 elastic-agent-8.0.0-linux-amd64.docker.tar.gz
-rw-r--r--  1 mdelapenya  staff        175 26 Aug 13:51 elastic-agent-8.0.0-linux-amd64.docker.tar.gz.sha512
-rw-r--r--  1 mdelapenya  staff  614539725 26 Aug 13:55 elastic-agent-complete-8.0.0-linux-amd64.docker.tar.gz
-rw-r--r--  1 mdelapenya  staff        184 26 Aug 13:56 elastic-agent-complete-8.0.0-linux-amd64.docker.tar.gz.sha512
-rw-r--r--  1 mdelapenya  staff  163226167 26 Aug 13:50 elastic-agent-ubi8-8.0.0-linux-amd64.docker.tar.gz
-rw-r--r--  1 mdelapenya  staff        180 26 Aug 13:50 elastic-agent-ubi8-8.0.0-linux-amd64.docker.tar.gz.sha512

$ docker images | grep "elastic-agent"
docker.elastic.co/beats/elastic-agent-complete            8.0.0                                      45386a23dba6   11 minutes ago   1.57GB
docker.elastic.co/beats-ci/elastic-agent-cloud            8.0.0                                      822ae902e8d5   13 minutes ago   849MB
docker.elastic.co/beats/elastic-agent                     8.0.0                                      3d87257176fa   13 minutes ago   526MB
docker.elastic.co/beats/elastic-agent-ubi8                8.0.0                                      5fa582646d69   14 minutes ago   298MB

Related issues

…ge (elastic#27621)" This reverts commit 89e415d.

…lastic#27399)" This reverts commit 954a250.

As we prefer using the declarative way over the programmatic way, we do not need the variable. Instead, we'll use the YAML file for declaring the new variants

mergify · 2021-10-19T10:33:29Z

This pull request does not have a backport label. Could you fix it @mdelapenya? 🙏
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.

mdelapenya · 2021-10-19T10:37:35Z

dev-tools/packaging/templates/docker/Dockerfile.elastic-agent.tmpl

@@ -37,9 +37,6 @@ RUN readlink -f {{ $beatBinary }} | xargs setcap {{ .linux_capabilities }}

 FROM {{ .from }}

-# Contains the elastic agent image variant, an empty string for the standard variant
-# or "complete" for the bigger one.
-ENV ELASTIC_AGENT_IMAGE_VARIANT={{.Variant}}


If we still need this environment variable for the complete variant, we can enclose it in an if/else block:

{{- if (and (contains .image_name "-complete")) }} ENV ELASTIC_AGENT_IMAGE_VARIANT=complete

See #27052 (comment)

I wonder if adding a metadata could be an option, though the docker tag contains that value anywawy.

I was not able to find any references for ELASTIC_AGENT_IMAGE_VARIANT

beats/dev-tools/packaging/templates/docker/Dockerfile.elastic-agent.tmpl

Line 102 in 7a0d9bc

description="{{ .BeatDescription }}"

maybe?

image_name: {{ .image_name }}

I agree that labels or metadata would be better to consider

BTW, my comment was just a comment, so therefore it's not blocking and I don't see any reasons why ELASTIC_AGENT_IMAGE_VARIANT should be kept

cc @andrewvc , can you please help to understand if removing is harmless?

It's currently unused, so can be deleted.

elasticmachine · 2021-10-19T11:27:40Z

Pinging @elastic/uptime (Team:Uptime)

elasticmachine · 2021-10-19T11:27:41Z

Pinging @elastic/elastic-agent (Team:Elastic-Agent)

v1v

neat!!!

elasticmachine · 2021-10-19T15:53:32Z

💚 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

Start Time: 2021-10-20T08:01:05.986+0000
Duration: 275 min 12 sec
Commit: c74f6ae

Test stats 🧪

Test	Results
Failed	0
Passed	53982
Skipped	5344
Total	59326

💚 Flaky test report

Tests succeeded.

🤖 GitHub comments

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

/test : Re-trigger the build.
/package : Generate the packages and run the E2E tests.
/beats-tester : Run the installation tests with beats-tester.

jsoriano

This is looking good 👍

jsoriano · 2021-10-19T18:56:49Z

dev-tools/mage/pkgtypes.go

@@ -48,7 +48,7 @@ const (
 	packageStagingDir = "build/package"

 	// defaultBinaryName specifies the output file for zip and tar.gz.
-	defaultBinaryName = "{{.Name}}-{{if .Variant}}{{.Variant}}-{{end}}{{.Version}}{{if .Snapshot}}-SNAPSHOT{{end}}{{if .OS}}-{{.OS}}{{end}}{{if .Arch}}-{{.Arch}}{{end}}"


Does this need to be represented in the packages.yml definition? If this is needed, maybe output_file (used here) can be set in agent_docker_complete_spec.

(I see output_file is not defined anywhere, maybe you find surprises if you try to use it 😬)

Not sure about it, this line is part of the revert of the initial commits. Maybe @andrewvc can help us out here

@jsoriano if you agree, I'd like to merge this one, to avoid more "variants" to appear/be merged (see #28597)

dev-tools/packaging/packages.yml

Co-authored-by: Jaime Soriano Pastor <[email protected]>

…28526) * Revert "[elastic-agent] Fix docker tar.gz generation for complete image (#27621)" This reverts commit 89e415d. * Revert "[elastic-agent] Use -complete in docker image name, not tag (#27399)" This reverts commit 954a250. * chore: use declarative variant for complete * fix: pass agent dockerfile to complete spec * fix: do not append variant to the artifact name * docs: update reference to complete variant * chore: remove unused param * chore: remove Variants from PacakgeSpec struct As we prefer using the declarative way over the programmatic way, we do not need the variable. Instead, we'll use the YAML file for declaring the new variants * fix: align comment Co-authored-by: Jaime Soriano Pastor <[email protected]> Co-authored-by: Jaime Soriano Pastor <[email protected]> (cherry picked from commit 554399a) # Conflicts: # dev-tools/packaging/packages.yml # dev-tools/packaging/templates/docker/Dockerfile.elastic-agent.tmpl

…28526) * Revert "[elastic-agent] Fix docker tar.gz generation for complete image (#27621)" This reverts commit 89e415d. * Revert "[elastic-agent] Use -complete in docker image name, not tag (#27399)" This reverts commit 954a250. * chore: use declarative variant for complete * fix: pass agent dockerfile to complete spec * fix: do not append variant to the artifact name * docs: update reference to complete variant * chore: remove unused param * chore: remove Variants from PacakgeSpec struct As we prefer using the declarative way over the programmatic way, we do not need the variable. Instead, we'll use the YAML file for declaring the new variants * fix: align comment Co-authored-by: Jaime Soriano Pastor <[email protected]> Co-authored-by: Jaime Soriano Pastor <[email protected]> (cherry picked from commit 554399a) # Conflicts: # dev-tools/packaging/packages.yml

…urnalbeat-ci * upstream/master: (49 commits) [CI]: use the downstream packaging pipeline for branches/tags (elastic#28589) fix: use declarative style for complete variant of the elastic-agent (elastic#28526) x-pack/auditbeat/tracing: fix regexp for kprobe description line (elastic#28609) docs: Update `api_key` example on elasticsearch output (elastic#28606) chore: add build scripts to CODEOWNERS (elastic#28615) Osquerybeat: Fix host_processes missing cmdline arguments (elastic#28622) Add note about changes to regexp package in Golang (elastic#28616) CI: nightly/weekly builds for 7.x targeting 7.16 instead (elastic#28612) Osquerybeat: Fix extenstion unable to start on windows (elastic#28598) Osquerybeat: Return the query result count with the action response (elastic#28576) Agent: Allow custom response properties in the action response (elastic#28575) [Heartbeat] Only setuid in elastic-agent image (elastic#28577) Fix formatting of `mapStateJSON` and `layerListJSON` in dashboard assets (elastic#28530) CI: refactor the run e2e build (elastic#28502) Use fsnotify with long windows name-safe changes (elastic#28517) Remove unneeded mergify config backport: Add 7.16 branch (elastic#28560) Add proxy_url support to threatintel module's malwarebazaar fileset (elastic#28533) Osquerybeat: Implement host_users, host_groups, host_processes tables as a part of our osquery_extension. (elastic#28434) [Heartbeat] Make run_once syntax a boolean (elastic#28548) ...

…lastic#28526) * Revert "[elastic-agent] Fix docker tar.gz generation for complete image (elastic#27621)" This reverts commit 89e415d. * Revert "[elastic-agent] Use -complete in docker image name, not tag (elastic#27399)" This reverts commit 954a250. * chore: use declarative variant for complete * fix: pass agent dockerfile to complete spec * fix: do not append variant to the artifact name * docs: update reference to complete variant * chore: remove unused param * chore: remove Variants from PacakgeSpec struct As we prefer using the declarative way over the programmatic way, we do not need the variable. Instead, we'll use the YAML file for declaring the new variants * fix: align comment Co-authored-by: Jaime Soriano Pastor <[email protected]> Co-authored-by: Jaime Soriano Pastor <[email protected]>

…nt of the elastic-agent (#28635) * fix: use declarative style for complete variant of the elastic-agent (#28526) * Revert "[elastic-agent] Fix docker tar.gz generation for complete image (#27621)" This reverts commit 89e415d. * Revert "[elastic-agent] Use -complete in docker image name, not tag (#27399)" This reverts commit 954a250. * chore: use declarative variant for complete * fix: pass agent dockerfile to complete spec * fix: do not append variant to the artifact name * docs: update reference to complete variant * chore: remove unused param * chore: remove Variants from PacakgeSpec struct As we prefer using the declarative way over the programmatic way, we do not need the variable. Instead, we'll use the YAML file for declaring the new variants * fix: align comment Co-authored-by: Jaime Soriano Pastor <[email protected]> Co-authored-by: Jaime Soriano Pastor <[email protected]> (cherry picked from commit 554399a) # Conflicts: # dev-tools/packaging/packages.yml * fix: resolve conflicts in packages.yml file Co-authored-by: Manuel de la Peña <[email protected]>

mdelapenya added 8 commits October 19, 2021 11:29

Revert "[elastic-agent] Fix docker tar.gz generation for complete ima…

08c6c20

…ge (elastic#27621)" This reverts commit 89e415d.

Revert "[elastic-agent] Use -complete in docker image name, not tag (e…

467f58f

…lastic#27399)" This reverts commit 954a250.

chore: use declarative variant for complete

c8f542f

fix: pass agent dockerfile to complete spec

5a5e107

fix: do not append variant to the artifact name

4b2ad3a

docs: update reference to complete variant

84f3271

chore: remove unused param

8bb2079

chore: remove Variants from PacakgeSpec struct

7a0d9bc

As we prefer using the declarative way over the programmatic way, we do not need the variable. Instead, we'll use the YAML file for declaring the new variants

mdelapenya self-assigned this Oct 19, 2021

mdelapenya added the Team:Automation Label for the Observability productivity team label Oct 19, 2021

mdelapenya requested a review from a team October 19, 2021 10:33

botelastic bot added needs_team Indicates that the issue/PR needs a Team:* label and removed needs_team Indicates that the issue/PR needs a Team:* label labels Oct 19, 2021

mergify bot added the backport-skip Skip notification from the automated backport with mergify label Oct 19, 2021

mdelapenya added bug build-system Issue or change affecting Mage, Make, or build scripts. Team:Elastic-Agent Label for the Agent team Team:obs-ds-hosted-services Label for the Observability Hosted Services team labels Oct 19, 2021

mdelapenya requested review from andrewvc and jsoriano October 19, 2021 10:35

mdelapenya commented Oct 19, 2021

View reviewed changes

mdelapenya mentioned this pull request Oct 19, 2021

fix: generate complete variant binaries properly #27609

Closed

9 tasks

mdelapenya marked this pull request as ready for review October 19, 2021 11:27

v1v approved these changes Oct 19, 2021

View reviewed changes

jsoriano reviewed Oct 19, 2021

View reviewed changes

fix: align comment

c74f6ae

Co-authored-by: Jaime Soriano Pastor <[email protected]>

cachedout approved these changes Oct 20, 2021

View reviewed changes

mdelapenya mentioned this pull request Oct 22, 2021

[heartbeat] Add support for ubi8-complete image #28597

Closed

6 tasks

jsoriano approved these changes Oct 25, 2021

View reviewed changes

mdelapenya added backport-v7.15.0 Automated backport with mergify backport-v7.16.0 Automated backport with mergify and removed backport-skip Skip notification from the automated backport with mergify labels Oct 26, 2021

mdelapenya merged commit 554399a into elastic:master Oct 26, 2021

mergify bot mentioned this pull request Oct 26, 2021

[7.15](backport #28526) fix: use declarative style for complete variant of the elastic-agent #28634

Closed

mergify bot mentioned this pull request Oct 26, 2021

[7.16](backport #28526) fix: use declarative style for complete variant of the elastic-agent #28635

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: use declarative style for complete variant of the elastic-agent #28526

fix: use declarative style for complete variant of the elastic-agent #28526

mdelapenya commented Oct 19, 2021

mergify bot commented Oct 19, 2021

mdelapenya Oct 19, 2021

v1v Oct 19, 2021

v1v Oct 19, 2021

mdelapenya Oct 19, 2021

v1v Oct 19, 2021

andrewvc Oct 25, 2021

elasticmachine commented Oct 19, 2021

elasticmachine commented Oct 19, 2021

v1v left a comment

elasticmachine commented Oct 19, 2021 •

edited by jenkins-beats-ci bot

Loading

Build stats

Test stats 🧪

jsoriano left a comment

jsoriano Oct 19, 2021

mdelapenya Oct 19, 2021

mdelapenya Oct 25, 2021

fix: use declarative style for complete variant of the elastic-agent #28526

fix: use declarative style for complete variant of the elastic-agent #28526

Conversation

mdelapenya commented Oct 19, 2021

What does this PR do?

Why is it important?

Checklist

Author's Checklist

How to test this PR locally

Related issues

mergify bot commented Oct 19, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

elasticmachine commented Oct 19, 2021

elasticmachine commented Oct 19, 2021

v1v left a comment

Choose a reason for hiding this comment

elasticmachine commented Oct 19, 2021 • edited by jenkins-beats-ci bot Loading

💚 Build Succeeded

Build stats

Test stats 🧪

💚 Flaky test report

🤖 GitHub comments

jsoriano left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

elasticmachine commented Oct 19, 2021 •

edited by jenkins-beats-ci bot

Loading