Skip to content

Mount Go build cache into crossbuild container #9094

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Mount Go build cache into crossbuild container #9094

wants to merge 4 commits into from
+23 −26

Conversation

swiatekm
Copy link
Contributor

@swiatekm swiatekm commented Jul 21, 2025 •
edited
Loading

What does this PR do?

Mounts the Go build cache into the golang-crossbuild container. To facilitate this and allow the container to generate the build cache, it also changes the crossbuild container to run as the host user.

This has been tested locally on both Mac and Linux, and the CI passes for this PR. It probably requires somewhat more careful testing, as permission changes in the host build cache could potentially result in breakage for unified releases.

Why is it important?

This reduces the build time of agent during packaging by around 75%. On my machine, the same-architecture build goes from 2 minutes to 25 seconds.

Checklist

  • I have read and understood the pull request guidelines of this project.
  • 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/fragments using the changelog tool
  • [ ] I have added an integration test or an E2E test

How to test this PR locally

Run mage crossbuild and time it.

Related issues

@swiatekm swiatekm added skip-changelog chore Tasks that just need to be done, they are neither bug, nor enhancements backport-active-all Automated backport with mergify to all the active branches labels Jul 21, 2025
@swiatekm swiatekm marked this pull request as ready for review July 28, 2025 13:05
@swiatekm swiatekm requested a review from a team as a code owner July 28, 2025 13:05
@swiatekm swiatekm requested review from pchila, straistaru, pkoutsovasilis and a team July 28, 2025 13:05
swiatekm
swiatekm commented Jul 28, 2025
View reviewed changes
dev-tools/mage/build.go
Comment on lines -169 to -179
defer DockerChown(filepath.Join(params.OutputDir, params.Name+binaryExtension(GOOS)))
defer DockerChown(filepath.Join(params.OutputDir))

mountPoint, err := ElasticBeatsDir()
if err != nil {
return err
}
if err := sh.Run("git", "config", "--global", "--add", "safe.directory", mountPoint); err != nil {
return err
}

Copy link
Contributor Author

@swiatekm swiatekm Jul 28, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All of this was necessary because files written in the container were owned by root. They're now owned by the host user to begin with.

swiatekm
swiatekm commented Jul 28, 2025
View reviewed changes
dev-tools/mage/crossbuild.go
Comment on lines -172 to -176
if CrossBuildMountModcache {
// Make sure the module dependencies are downloaded on the host,
// as they will be mounted into the container read-only.
mg.Deps(func() error { return gotool.Mod.Download() })
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The CrossBuildMountModCache variable is always true, so I simply removed it.

v1v
v1v reviewed Jul 28, 2025
View reviewed changes
Copy link
Member

@v1v v1v left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we need to test these set of changes in some of the other BK pipelines:

Can you please run them and copy the link to those builds in this PR please?

@swiatekm
Copy link
Contributor Author

@v1v both of them succeeded at the build steps, but failed at the publishing step, even though I set them to dry-run. Not sure if that's in some way an effect of this change or if I made a mistake.

See:

@v1v
Copy link
Member

v1v commented Jul 29, 2025 •
edited
Loading

That's an expected error, I think it does not support feature branches:

The specified project directory '/release/project-configs/chore/crossbuild-build-cache' does not exist.

https://buildkite.com/elastic/elastic-agent-package/builds/7168#01985144-a389-4509-af44-f1717edd59b2/282-310
https://buildkite.com/elastic/elastic-agent-binary-dra/builds/4211#0198518a-dc7c-4bb0-ad62-f7d65c48a0b9/145-173

As far as I see, you can override them with DRA_BRANCH=main:

I'm gonna run them again:

using the env variables:

DRA_BRANCH="main"
DRA_DRY_RUN="--dry-run"
DRA_PROJECT_ARTIFACT_ID="agent-core"
DRA_PROJECT_ID="elastic-agent-core"
DRA_VERSION=9.2.0
DRA_WORKFLOW="snapshot"

@pchila
Copy link
Member

pchila commented Jul 29, 2025

both of them succeeded at the build steps, but failed at the publishing step, even though I set them to dry-run. Not sure if that's in some way an effect of this change or if I made a mistake.

You would need to define DRA_BRANCH="main" (or another release branch) and DRA_DRY_RUN="--dry-run" like in this build https://buildkite.com/elastic/elastic-agent-package/builds/7181

Just be careful because without the appropriate DRA_DRY_RUN value you will end up overwriting DRAs built from the release branches with one from your PR

@swiatekm swiatekm requested a review from v1v July 29, 2025 17:32
v1v
v1v approved these changes Jul 29, 2025
View reviewed changes
Copy link
Member

@v1v v1v left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. I've just left a comment; however, I'm not an expert in the build system. Please take my +1 as only an acknowledgment.

v1v
v1v previously approved these changes Jul 29, 2025
View reviewed changes
Copy link
Member

@v1v v1v left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. I've just left a comment; however, I'm not an expert in the build system. Please take my +1 as only an acknowledgment.

@elastic-sonarqube Elastic SonarQube
Copy link

Quality Gate passed Quality Gate passed

Issues
0 New issues
0 Fixed issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarQube

@elasticmachine
Copy link
Collaborator

💛 Build succeeded, but was flaky

Failed CI Steps

History

cc @swiatekm

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@v1v v1v v1v left review comments

@pchila pchila Awaiting requested review from pchila pchila is a code owner automatically assigned from elastic/elastic-agent-control-plane

@straistaru straistaru Awaiting requested review from straistaru straistaru is a code owner automatically assigned from elastic/elastic-agent-control-plane

@pkoutsovasilis pkoutsovasilis Awaiting requested review from pkoutsovasilis

At least 1 approving review is required to merge this pull request.

Assignees

@swiatekm swiatekm

Labels
backport-active-all Automated backport with mergify to all the active branches chore Tasks that just need to be done, they are neither bug, nor enhancements skip-changelog
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@swiatekm @v1v @pchila @elasticmachine