Feature/fix compose multi stage builds#1530
Conversation
janekbaraniewski
left a comment
There was a problem hiding this comment.
Awesome, just few nitpicks 😄 really like what you did with including more comments in code handling compose configuration!
I was thinking maybe we could also extract some of those pieces of code to smaller functions (like readOriginalDockerfile etc) so that this buildAndExtendDockerCompose function becomes a bit smaller and easier to grasp and also self-documents better. wdyt?
yes for sure that sounds good, I'll try that now |
Fixes - #1433
The issue was that during our build pipeline we assumed an Image was used instead of a dockerfile, resulting in a
FROM empty string AS finalbeing added. The empty string came from the compose's devcontainer image name, which is nil since we are building the devcontainer.The fix was to assign the dockerFileContents to the read docker file if no modifications were made. Looking at
pkg/devcontainer/parse.goline 223 we perform a check on the target and potentially return an empty string, as the dockerfile does not need to be modified. In this case we need to simply assign dockerFileContents to the variable containing the original dockerfile (line 450 inpkg/devcontainer/compose.go).I also added 2 examples, one for multi stage builds and a compose. You can use these to validate the fix by seeing the OP's error running
devpod up examples/composethen running the same with this PR's build.