Skip to content
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.

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

transparently use docker targets when building #2519

Merged
merged 1 commit into from
Oct 30, 2023

Conversation

jaxesn
Copy link
Member

@jaxesn jaxesn commented Sep 28, 2023

Issue #, if available:

Description of changes:

We have traditionally relied on building our components on the host, which requires a fair amount of setup and dependencies. A while back we did introduce the run-in-docker target(s) to make it easier to run parts of the build locally and match checksums/attribution files by using a docker container running the builder-base image which is what we run in prow/coudebuild. Recently we started using EKS-D golang builds which are only produced for linux so building on mac and matching checksums (which was possible before) is no longer possible. Using the builder-base docker approach has worked pretty well.

This PR extends our use of these docker container by converting certain targets, go build/go licenses/attribution and some project specific targets, to use docker automatically if the required toolset is missing. In the case of golang, we just assume its missing since the likelihood of someone having install EKS-D go is not very high. This can be overridden if the developer wants to build on the host instead of the container.

The approach used in Common.mk to run the docker targets transparently is a bit hacky.... Its basically the same approach documented and taken in #2546. The good new is that for project Makefiles where we want to use docker if, say npm is missing in the case of the prometheus build, its a pretty simple change.

I did go through and update the building-locally doc, in which I created and recreated AL2 and AL23 instances to confirm I had as much documented as I could find.

I did add a build-all target to the root Makefile, which I do not intend to be used anywhere but it is a nice helper when trying to validate that an instance has all the dependencies needed to build pretty much the entire stack.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@eks-distro-bot
Copy link
Collaborator

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@eks-distro-bot eks-distro-bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Sep 28, 2023
@eks-distro-bot eks-distro-bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Oct 2, 2023
@jaxesn jaxesn force-pushed the jgw/better-docker-usage branch 2 times, most recently from 77b6c4c to 2b8f18d Compare October 2, 2023 15:44
@eks-distro-bot eks-distro-bot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Oct 3, 2023
@jaxesn jaxesn force-pushed the jgw/better-docker-usage branch 9 times, most recently from dc0c4d5 to 1dc551a Compare October 12, 2023 02:41
@eks-distro-bot eks-distro-bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Oct 13, 2023
@jaxesn jaxesn force-pushed the jgw/better-docker-usage branch 6 times, most recently from 25243e5 to c684842 Compare October 16, 2023 18:37
@jaxesn jaxesn force-pushed the jgw/better-docker-usage branch 3 times, most recently from 09fa217 to e973c9f Compare October 19, 2023 20:24
@jaxesn
Copy link
Member Author

jaxesn commented Oct 19, 2023

/hold

@jaxesn jaxesn force-pushed the jgw/better-docker-usage branch 12 times, most recently from 4b71b67 to 2a8a8c2 Compare October 24, 2023 19:03
@jaxesn jaxesn force-pushed the jgw/better-docker-usage branch 2 times, most recently from c6a755e to e0f7ab0 Compare October 28, 2023 05:11
@abhay-krishna
Copy link
Member

/lgtm
/approve

@eks-distro-bot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: abhay-krishna

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@jaxesn
Copy link
Member Author

jaxesn commented Oct 30, 2023

/test all

@jaxesn
Copy link
Member Author

jaxesn commented Oct 30, 2023

/unhold

@eks-distro-bot eks-distro-bot merged commit a956c77 into aws:main Oct 30, 2023
83 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved lgtm size/XL Denotes a PR that changes 500-999 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants