fix: generate complete variant binaries properly #27609
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mdelapenya
added
the
Team:Automation
botelastic
bot
added
needs_team
mdelapenya
added
Team:Elastic-Agent
|
Pinging @elastic/uptime (Team:Uptime) |
|
Pinging @elastic/agent (Team:Agent) |
|
BTW, |
❕ Build Aborted
Expand to view the summary
Build stats
Test stats 🧪
Trends 🧪Steps errors
Expand to view the steps failures
|
|
What is up with this PR? Is it still required? Personally, I like this approach because it is consistent with the current packaging. |
Do you think of any incompatibility with #27621? As you suggest, I'd like to see more robustness in the build process too |
|
No, I don't see any incompatibility. |
* master: Forward port 7.14.1 changelog to master (elastic#27687) Addressing multiple dashboard issues: deps loading once, field conversion, etc. (elastic#27669) Remove adaptive queue sizes from agent's spec files (elastic#27653) Osquerybeat: Improve testability and unit test coverage (elastic#27591) Osquerybeat: lockdown flagsfile, prevent global defaults (elastic#27611) Import the references of dashboard assets using the Saved Objects API (elastic#27647) Fix bug with override path in cgroups (elastic#27620) Allow Kibana client to authorize with Elasticsearch API key (elastic#27540) Filebeat auditd: Fix Top Exec Commands dashboard visualization (elastic#27638) [elastic-agent] Fix docker tar.gz generation for complete image (elastic#27621) Follow up changes in dashboards in mage check && fix minor issue (elastic#27553) [Heartbeat] Fix bug where `enabled: false` is ignored. (elastic#27615) Support kube_state_metrics v2.0.0 (elastic#27552)
|
jenkins run the tests please |
kvch
approved these changes
Sep 6, 2021
jsoriano
requested changes
Sep 7, 2021
There was a problem hiding this comment.
With these changes the following images are generated, one of them containing two suffixes, and another one with a -complete suffix but seemingly having the same content as the one without this suffix. Is this expected? 🤔
REPOSITORY TAG IMAGE ID CREATED SIZE
docker.elastic.co/beats/elastic-agent-complete-complete 8.0.0 f5bc5bc3e5fd 34 seconds ago 1.55GB
docker.elastic.co/beats/elastic-agent 8.0.0 4069689b3330 3 minutes ago 523MB
docker.elastic.co/beats/elastic-agent-complete 8.0.0 61d02e44734e 3 minutes ago 523MB
docker.elastic.co/beats/elastic-agent-ubi8 8.0.0 51cecc7df1fb 3 minutes ago 318MB
I think because #27621 was merged before this one, there are conflicts in resolving the issue described here. As @kvch said, these changes are affecting configuration, while that PR is updating the build system to cover this use case. I tested my changes before #27621, I can give it a try again, with and without that commit. |
* master: (39 commits) [Heartbeat] Move JSON tests from python->go (elastic#27816) docs: simplify permissions for Dockerfile COPY (elastic#27754) Osquerybeat: Fix osquery logger plugin severy levels mapping (elastic#27789) [Filebeat] Update compatibility function to remove processor description on ES < 7.9.0 (elastic#27774) warn log entry and no validation failure when both queue_url and buck… (elastic#27612) libbeat/cmd/instance: ensure test config file has appropriate permissions (elastic#27178) [Heartbeat] Add httpcommon options to ZipURL (elastic#27699) Add a header round tripper option to httpcommon (elastic#27509) [Elastic Agent] Add validation to ensure certificate paths are absolute. (elastic#27779) Rename dashboards according to module.yml files for master (elastic#27749) Refactor vagrantfile, add scripts for provisioning with docker/kind (elastic#27726) Accept syslog dates with leading 0 (elastic#27775) [Filebeat] Add timezone config option to decode_cef and syslog input (elastic#27727) [Filebeat] Threatintel compatibility updates (elastic#27323) Add support for ephemeral containers in elastic agent dynamic provider (elastic#27707) [Filebeat] Integration tests in CI for AWS-S3 input (elastic#27491) Fix flakyness of TestFilestreamEmptyLine (elastic#27705) [Filebeat] kafka v2 using parsers (elastic#27335) Update Kafka version parsing / supported range (elastic#27720) Update Sarama to 1.29.1 (elastic#27717) ...
* master: [Auditbeat] scanner honor include_files (elastic#27722) chore(ci): remove not used param when triggering e2e tests (elastic#27823) LineReader: Reuse temporary buffer to reduce per-line allocation (elastic#27782)
|
@jsoriano @kvch @andrewkroh @andrewvc I updated this PR and verified that: cd x-pack/elastic-agent
PLATFORMS="linux/amd64" mage package
$ ls -l x-pack/elastic-agent/build/distributions/
total 2791688
-rw-r--r--@ 1 mdelapenya staff 119939138 9 Sep 17:18 elastic-agent-8.0.0-amd64.deb
-rw-r--r-- 1 mdelapenya staff 159 9 Sep 17:18 elastic-agent-8.0.0-amd64.deb.sha512
-rw-r--r-- 1 mdelapenya staff 244353172 9 Sep 17:18 elastic-agent-8.0.0-linux-amd64.docker.tar.gz
-rw-r--r-- 1 mdelapenya staff 175 9 Sep 17:18 elastic-agent-8.0.0-linux-amd64.docker.tar.gz.sha512
-rw-r--r-- 1 mdelapenya staff 120003980 9 Sep 17:17 elastic-agent-8.0.0-linux-x86_64.tar.gz
-rw-r--r-- 1 mdelapenya staff 169 9 Sep 17:17 elastic-agent-8.0.0-linux-x86_64.tar.gz.sha512
-rw-r--r-- 1 mdelapenya staff 119745407 9 Sep 17:18 elastic-agent-8.0.0-x86_64.rpm
-rw-r--r-- 1 mdelapenya staff 160 9 Sep 17:18 elastic-agent-8.0.0-x86_64.rpm.sha512
-rw-r--r-- 1 mdelapenya staff 615426504 9 Sep 17:23 elastic-agent-complete-8.0.0-linux-amd64.docker.tar.gz
-rw-r--r-- 1 mdelapenya staff 184 9 Sep 17:24 elastic-agent-complete-8.0.0-linux-amd64.docker.tar.gz.sha512
-rw-r--r-- 1 mdelapenya staff 163243411 9 Sep 17:18 elastic-agent-ubi8-8.0.0-linux-amd64.docker.tar.gz
-rw-r--r-- 1 mdelapenya staff 180 9 Sep 17:18 elastic-agent-ubi8-8.0.0-linux-amd64.docker.tar.gz.sha512
$ docker images | grep agent | grep -v SNAP
docker.elastic.co/beats/elastic-agent-complete 8.0.0 7cc90f716192 40 minutes ago 1.55GB
docker.elastic.co/beats/elastic-agent-ubi8 8.0.0 58ed21edadcd 44 minutes ago 297MB
docker.elastic.co/beats/elastic-agent 8.0.0 a9c9517dcace 44 minutes ago 521MBTBH, I'm pretty sure I'm acting as a surgeon in their first to-open-heart surgery with this PR affecting the build system. I'd appreciate your review. Thanks! |
mdelapenya
commented
Sep 9, 2021
jsoriano
requested changes
Sep 10, 2021
There was a problem hiding this comment.
Looks good, I like that variants are more just another image in the yml file and less a special case in dockerbuilder.go.
Added some comments about removing unneeded code and fully reverting #27621. I wonder if we can even remove more code for variants in the docker builder.
| @@ -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}}" | |||
| defaultBinaryName = "{{.Name}}-{{.Version}}{{if .Snapshot}}-SNAPSHOT{{end}}{{if .OS}}-{{.OS}}{{end}}{{if .Arch}}-{{.Arch}}{{end}}" | |||
There was a problem hiding this comment.
If Variant is not needed anymore here, I guess that the rest of #27621 can be reverted too?
This reverts commit bbc386d.
|
This pull request does not have a backport label. Could you fix it @mdelapenya? 🙏
NOTE: |
mergify
bot
added
the
backport-skip
|
Once #28134 is merged, we should update this PR and possibly resolve conflicts |
cachedout
approved these changes
Sep 30, 2021
|
This pull request is now in conflicts. Could you fix it? 🙏 |
kuisathaverat
approved these changes
Oct 6, 2021
6 tasks
|
Overridden by #28526 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What does this PR do?
This PR is enriching the image name for the "complete" variant of the Elastic Agent when building it using
mage package. It simply adds an extra spec that must be added to the complete variant. We are adding an additional build package for the Elastic Agent, exactly the same as for the UBI8 variant.Given I'm not a Beats build expert, I'm copying what the ubi8 variant does, because it seems the
variantskeyword used in the past is not doing its job. Maybe @jsoriano or @kvch knows more about it.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
completeTAR.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
CHANGELOG.next.asciidocorCHANGELOG-developer.next.asciidoc.Author's Checklist
variantskey in the packages.yml file? How is it used, because without this fix it seemed not to work as expected?How to test this PR locally
After around 10 minutes... check for the binaries produced by the build system:
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.sha512Related issues