Fix issue when using custom BuildKit frontend

[AbdullahWali]

resolves microsoft/vscode-remote-release#6848

Changes buildAndExtendDockerCompose to build the user specified Dockerfile and extend the image instead of extending the Dockerfile directly

more details in microsoft/vscode-remote-release#6848 (comment)

CC @chrmarti

[AbdullahWali]

@microsoft-github-policy-service agree company="Bloomberg"

[chrmarti]

Thanks for the PR! I have left a comment regarding the original reason we merged the two image builds.

[chrmarti]

Using two builds had the issues mentioned in microsoft/vscode-remote-release#6848 (comment).

[AbdullahWali]

Thank you @chrmarti , are multi-platform builds only supported with the single container workflow? This seems to be the case according to

cli/src/spec-node/devContainersSpecCLI.ts

Lines 409 to 413 in 38c025e

	} else if ('dockerComposeFile' in config) {
	if (buildxPlatform || buildxPush) {
	throw new ContainerError({ description: '--platform or --push not supported.' });
	}

We are only blocked on docker-compose workflows, I can revert the changes to singleContainer.ts and limit the changes to dockerCompose.ts if that addresses the multi-platform builds issue?

[chrmarti]

I would prefer to keep the two code paths (single container and docker-compose) in sync to avoid having subtly different behaviors.

What about the following options:

• Have a flag in the devcontainer.json to enable using an extra Dockerfile instead of the merged one.
• Similarly: Have a directive at the start of the Dockerfile to enable using an extra Dockerfile instead of the merged one.
• Have a way for your custom builder to pick up the addition to the Dockerfile and handle it. I guess your custom builder would need access to the Docker builder to delegate to it.

[AbdullahWali]

Have a flag in the devcontainer.json to enable using an extra Dockerfile instead of the merged one.

I've moved the changes behind a flag prebuildDockerfile (can change the name if needed).

is that in line with what you had in mind?

[AbdullahWali]

Hi @chrmarti, following up on this thread, have you had a chance to review the changes? :)

[AbdullahWali]

Hi @chrmarti , can i please get an update on this?

[chrmarti]

@AbdullahWali Sorry for the delay, I will do a review in next days. Thanks for your patience.

[Bilalh]

@AbdullahWali Sorry for the delay, I will do a review in next days. Thanks for your patience.

It been nearly a month, could this be reviewed? This would unblock using custom buildkit dockerfiles with devcontainers

[AbdullahWali]

Hi @chrmarti, have you had a chance to review this? We are blocked on enabling docker-compose v2 due to this issue

[AbdullahWali]

Hi @chrmarti, following up on this. Our company utilises custom frontends heavily for our local development workflows, and thus are stuck on version 0.234.0 of Devcontainers. This version breaks when enabling docker-compose V2 [1], so we are also unable to use docker-compose V2 until this is resolved. Starting from April we won't be able to disable docker-compose V2 [2], so we'll be forced to move away from Devcontainers for our local development if we can't resolve this issue.

We'd like to keep using VSCode + Devcontainers ideally, so we'd appreciate your support on progressing with resolving this PR/issue. If there are any further changes to this PR which you think are needed then please let me know. I'd like to help in any way I can.

[1] microsoft/vscode-remote-release#7047
[2] https://www.docker.com/blog/new-docker-compose-v2-and-v1-deprecation/

[chrmarti]

Thanks for the PR, left a few comments for discussion.

[chrmarti]

I wonder if we should indeed make this a flag on the devcontainer.json. The way we build the image seems to be an implementation detail that shouldn't necessarily surface there. Two alternatives:

• Check if the "syntax" image is a docker/dockerfile image (we already have code doing this), if not: prebuild the Dockerfile.
• Come up with our own custom builder inside of which we can run the two build steps (user's Dockerfile and container features' Dockerfile) making it look from the outside as one single build step. (Not sure if this is possible and how hard it would be to implement.)

Another reason to avoid the flag in the devcontainer.json is that we can avoid the new code path when we don't have anything to add with a separate Dockerfile.

[chrmarti]

--target from the devcontainer.json needs to apply here like below.

Should we add --platform, --push, --output, --cache-from, --no-cache, --pull, --build-arg and BUILDKIT_INLINE_CACHE=1 depending on parameters like below?

Makes we wonder if we should instead of "prebuild the user's Dockerfile" turn the change around and "add features later".

[AbdullahWali]

Noted on the build args

Makes we wonder if we should instead of "prebuild the user's Dockerfile" turn the change around and "add features later".

I don't quite get what you mean here, do you mind elaborating how the features would be added later?

[AbdullahWali]

Thank you for looking into this @chrmarti ! Will implement the changes requested and update the PR

[AbdullahWali]

Hi @chrmarti , I've added the requested changes.

• I've removed prebuildDockerfile, and instead will prebuild when supportsBuildContexts returns unknown
• I've included the params that are used in the main build.
  • In order to do that while reducing code duplication, I've moved some of these parts into separate helper functions (eg, getCacheFromParams, getBuildArgParams in singleContainer.ts)
  • In dockerCompose.ts, I've had to refactor the part where the cacheFrom is added to decouple that from the buildOverrideContent. So now instead there are up to 2 files in additionalComposeOverrideFiles, one for cache from overrides, and one for buildOverrideContent (which seems to be used for features?)

Please let me know if you're happy with the change, then i can proceed to resolve any merge conflicts and proceed with any next steps suggested

[AbdullahWali]

Hi @chrmarti , hope you've had a nice weekend, have you had a chance to review this?

[AbdullahWali]

any updates on this?

[sstriker]

@chrmarti any chance we can get this over the line soon? It's been 6 months on this PR and a year since microsoft/vscode-remote-release#6848. I am confident we agree that seeing that issue addressed would be ideal. Thanks in advance!

[mohkale]

Hi @chrmarti it's been a little over a year since this pr last had activity. Is there anything we can do to help re-prioritize work on it. I've spoken to Abdullah and am happy to take over the PR and contribute to any ongoing change requests but I'm not sure this is actively being tracked atm.

[chrmarti]

Apologies for the delayed response. I'm a bit worried this will increase the complexity of the build code. Also note that using two Dockerfiles had the issues mentioned in microsoft/vscode-remote-release#6848 (comment).