-
Notifications
You must be signed in to change notification settings - Fork 106
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
Dockerfiles: run /bin/detect and /bin/build for extensions #786
Conversation
Signed-off-by: Natalie Arellano <[email protected]>
Signed-off-by: Natalie Arellano <[email protected]>
Signed-off-by: Natalie Arellano <[email protected]>
Signed-off-by: Natalie Arellano <[email protected]>
Signed-off-by: Natalie Arellano <[email protected]>
Signed-off-by: Natalie Arellano <[email protected]>
Signed-off-by: Natalie Arellano <[email protected]>
Notes for RFC/spec
Code things to be done
Code style questions
It’s a pretty big diff because I renamed some variables to be more generic to a thing that could be a buildpack OR an extension. The most interesting files are:
|
In short and, if I can still read go code, what you did within this PR has been to:
Questions:
|
Thanks @cmoulliard for your comments. Some thoughts:
This "glue code" is yet to be written. I think we'll want to align on what format the "extender" binary is expecting to find this information.
I think that is the job of the extension to decide whether to detect and/or output a Dockerfile or not, based on the source code and the build plan it is given.
In principle I think so, but we haven't given it too much thought yet. This is probably one of the next things to tackle.
Do you mean pass env vars to the extension /bin/detect and/or /bin/build? Or to the "extender" binary that applies the Dockerfiles? I think both are possible.
I think that's up to the RFC and/or spec to decide - we should note it as an open question. (cc @jkutner)
Since extensions and buildpacks collaborate on the build plan to pass or fail detect, we need to run /bin/build for extensions after the entire detect phase has finished. But there's really no need to spin up an entire separate container to run /bin/build for extensions, because no additional privileges are required. We could probably create a "wrapper" binary - similar to the creator - that runs detect and then runs build (for extensions only). |
This point must be clarified as currently I do not see/understand how steps will take place. We need a sequence diagram ! |
Passing ENV vars to the component parsing the Dockerfiles and generating the layer |
Again, this is something which should be clarified to describe which component (Lifecycle, ...) will be responsible to perform which work, the boundaries of the component, how they communicate ... |
Why are the path different within the following
Why do you generate the content of the extensions under
Will the
How can the extension know when they must use hereafter
See test case to engage the discussion: https://github.com/redhat-buildpacks/poc/blob/9e0a5f9f5fb5576b2c5b71314e0f8c7d8cbb2aed/buildah/code/parse/toml_test.go#L11
Is it for debugging purpose that you read the files lifecycle/buildpack/extension.go Lines 72 to 73 in 8f9a532
|
@cmoulliard thanks for taking another look! Let me see if I can address your comments -
Total laziness... the schema for
I think only the extender binary needs to care about Dockerfile build args.
I think that was my mistake in putting together this file. You are right, the extender binary needs some way to know which build arg is applicable when extending the build or run image. Maybe we could have something like
|
POC supports to read a toml file - redhat-buildpacks/poc@1ee100a#diff-443c22fd43332d23d4bc2d3f895875af24669e7053bf1fa2e41b4465ec834adaR186. I will now pass the parameters to the dockerfile. Should we pass them as ARG or ENV to the process building the Dockerfile ? @natalieparellano |
+1 |
How will the
|
@cmoulliard I think it would be easiest if the process that reads the TOML file and the process that applies the Dockerfile are one and the same. Is that possible with your POC? |
Yes. The poc now is processing a metadata.toml file under $WORKSPACE + |
Is it an issue if we have to pass such a docker parameter using pack and buildah extension |
Do you agree that we use your PR to merge the code of the POC able to parse/build dockerfile(s) ? @natalieparellano |
…pply to both build and run will be repeated in the list. Signed-off-by: Natalie Arellano <[email protected]>
Are the buildpack samples (branch dockerfile-poc) compatible with this commit |
I did a test to build lifecycle using last commit pushed (= 151d752) but that fails. A module reference is hard coded at this line and should be fixed !
|
@cmoulliard Thanks for giving it a try. That path points to a relative path on the file-system and is how I can reference your poc code on this branch. I'd be happier referencing the dependency more conventionally, when things are a little "cleaner". If your poc repo can be copied to |
Why dont we include the Kaniko's POC code and that we continue to improve it here (= this PR) ? |
Signed-off-by: Anthony Emengo <[email protected]>
Signed-off-by: Anthony Emengo <[email protected]>
@cmoulliard apologies for breaking you! I'll push up an intermediate fix that should make it compilable and then we'll work on pushing the changes upstream. |
Signed-off-by: Natalie Arellano <[email protected]>
151d752
to
d5fb974
Compare
Signed-off-by: Natalie Arellano <[email protected]>
Thanks to the work of @aemengo and @cmoulliard we can now see the lifecycle running detect for buildpacks and extensions, running build for extensions, extending the build image, and running build for buildpacks in the context of the extended image! To 🎉 see this in practice 🎉 , ensure that the buildpacks/samples repo is checked out at the The output here indicates that
|
|
I'm going to close this as all the work is captured in #802 |
This PR updates the lifecycle to:
Note that this PR does NOT tackle actually applying Dockerfiles to the build and/or run image - for the work being done there, see #709
Testing
Update for ease of use, please use the
test-extender.sh
script. Example invocation:LIFECYCLE_REPO_PATH=~/workspace/lifecycle SAMPLES_REPO_PATH=~/workspace/samples/ ./test-extender.sh
The following assumes that github.com/buildpacks/samples is checked out to the
dockerfiles-poc
branchYou should see:
./layers/group.toml
is created withsamples/curl
andsamples/rebasable
; empty./layers/plan.toml
is created./layers/config/metadata.toml
is created with the "buildpacks" (really extensions) that participated in the build and a list of Dockerfiles that should be applied by the build and/or run extender