Makefile: use linuxkit env vars for org and builder config#5669
Makefile: use linuxkit env vars for org and builder config#5669rucoder wants to merge 1 commit intolf-edge:masterfrom
Conversation
5ed85e1 to
78c02a6
Compare
78c02a6 to
e31be55
Compare
| endif | ||
|
|
||
| # Check for a custom registry (used for development purposes) | ||
| ifdef REGISTRY |
There was a problem hiding this comment.
I personally like the REGISTRY variable, it's intuitive and easy to use (no need to provide the org). But if you are insist to remove it, then the documentation must be updated: https://github.com/lf-edge/eve/blob/master/docs/BUILD.md#using-a-local-container-registry
There was a problem hiding this comment.
@rene i know you love it but I just wanted to make the experience consistent , I do not insist, we can keep it if you want but IMO it maybe introduce more confusion in the future
There was a problem hiding this comment.
ok, so update the documentation....
linuxkit v1.8.3 introduces environment variable support for flags previously threaded through every linuxkit call site in this Makefile. Remove LINUXKIT_ORG_TARGET and BUILDKIT_CONFIG_OPTS workarounds: - Drop BUILDKIT_CONFIG_FILE / BUILDKIT_CONFIG_OPTS variables and the manifest-mode guard; replace with LINUXKIT_BUILDER_CONFIG env var which defaults to /etc/buildkit/buildkitd.toml if the file exists - Drop LINUXKIT_ORG_TARGET from all linuxkit call sites (9 locations) - Drop REGISTRY make variable; use LINUXKIT_PKG_ORG directly Migration: make REGISTRY=myregistry -> LINUXKIT_PKG_ORG=myregistry/lfedge make For buildkit registry mirrors, LINUXKIT_BUILDER_CONFIG auto-detects /etc/buildkit/buildkitd.toml as before; override by setting LINUXKIT_BUILDER_CONFIG explicitly in the environment. For pull-side mirrors, set LINUXKIT_MIRROR in the runner environment. Bump LINUXKIT_VERSION to v1.8.3. Signed-off-by: Mikhail Malyshev <mike.malyshev@gmail.com>
e31be55 to
d522fbc
Compare
europaul
left a comment
There was a problem hiding this comment.
LGTM, I only wish LINUXKIT_BUILDER_CONFIG would be called BUILDKIT_CONFIG_PATH but I think it's too late for that :)
@europaul no, not to late. but it must be exactly |
It's just an arbitrary name but I think |
Description
NOTES:
linuxkit v1.8.3 introduces native environment variable support for flags previously threaded through every
linuxkitcall site in this Makefile viaLINUXKIT_ORG_TARGETandBUILDKIT_CONFIG_OPTS. This PR removes those Makefile-level workarounds and lets linuxkit pick up configuration directly from the runner environment.Removed:
BUILDKIT_CONFIG_FILE/BUILDKIT_CONFIG_OPTSvariables and themanifest-mode guardLINUXKIT_ORG_TARGETvariable and its injection at 9 call sitesREGISTRYmake variable (was only used to feedLINUXKIT_ORG_TARGET)New interface — set in the runner environment or on the make command line:
make REGISTRY=myregistryLINUXKIT_PKG_ORG=myregistry/lfedge makeBUILDKIT_CONFIG_FILE=/etc/buildkit/buildkitd.toml(auto-detected)LINUXKIT_BUILDER_CONFIG=/path/to/buildkitd.toml(explicit)LINUXKIT_MIRROR=docker.io=http://my-mirror.localA
$(warning ...)is emitted if/etc/buildkit/buildkitd.tomlexists on the local machine butLINUXKIT_BUILDER_CONFIGis not set, to avoid silently losing a previously auto-detected config.Bumps
LINUXKIT_VERSIONtov1.8.3.PR dependencies
Requires linuxkit v1.8.3 which includes:
linuxkit/linuxkit#4205
How to test and validate this PR
makewithout any overrides should behave identically to before.LINUXKIT_PKG_ORG=myregistry/lfedge makeshould tag/push packages tomyregistry/lfedge/*(replacesmake REGISTRY=myregistry).LINUXKIT_BUILDER_CONFIG=/etc/buildkit/buildkitd.tomlon a runner that has the file; verify buildkit uses the mirror config./etc/buildkit/buildkitd.tomllocally without settingLINUXKIT_BUILDER_CONFIG; confirm the warning is printed.Changelog notes
No user-facing changes. Build tooling simplification: linuxkit registry mirror and org configuration is now passed via environment variables (
LINUXKIT_BUILDER_CONFIG,LINUXKIT_PKG_ORG,LINUXKIT_MIRROR) instead of make variables, making CI runner configuration more transparent.PR Backports
Checklist