cli icon indicating copy to clipboard operation
cli copied to clipboard

fix: default to legacy builder instead of BuildKit with podman

Open eqladios opened this issue 1 year ago • 4 comments

Description

Podman doesn't support buildkit and defaults to "DOCKER_BUILDKIT=0", See https://github.com/containers/podman/blob/f7be7a365ad3e90db5f96f269a555f6f380f9275/cmd/podman/compose.go#L157-L170

This results in Podman failing builds when BUILDKIT_INLINE_CACHE or additional_contexts is used, as for example in https://github.com/devcontainers/cli/blob/9ba1fdaa11dee087b142d33e4ac13c5788392e34/src/spec-node/dockerCompose.ts#L230-L245

A simple fix would to make cli ignore --buildkit parameter, default to never and log a warning when Podman is used.

Fixes #863

Testing

A failing example would be

Dockerfile

ARG RUBY_VERSION=3.2.5
FROM ghcr.io/rails/devcontainer/images/ruby:$RUBY_VERSION

devcontainer.json

{
  "name": "app",
  "dockerComposeFile": "compose.yaml",
  "service": "rails-app",
  "workspaceFolder": "/workspaces/${localWorkspaceFolderBasename}",
  "features": {
    "ghcr.io/devcontainers/features/github-cli:1": {},
    "ghcr.io/rails/devcontainer/features/sqlite3": {}
  },
  "containerUser": "vscode",
  "updateRemoteUserUID": true,
  "containerEnv": {
    "REDIS_URL": "redis://redis:6379/1",
    "HOME": "/home/vscode"
  },
  "forwardPorts": [3000, 6379],
  "postCreateCommand": "bin/setup"
}

compose.yaml

name: "app"

services:
  rails-app:
    build:
      context: ..
      dockerfile: .devcontainer/Dockerfile
    volumes:
    - ../..:/workspaces:cached
    command: sleep infinity
    depends_on:
    - redis

  redis:
    image: redis:7.2
    restart: unless-stopped
    volumes:
    - redis-data:/data

volumes:
  redis-data:

when features is used in devcontainer.json this would always fail with log the classic builder doesn't support additional contexts, set DOCKER_BUILDKIT=1 to use BuildKit

I have compiled this on my machine and used it with --buildkit auto and --buildkit none and it works correctly and tests are passing normally, maybe an additional testing can be added for this.

eqladios avatar Sep 20 '24 16:09 eqladios

@microsoft-github-policy-service agree

eqladios avatar Sep 20 '24 16:09 eqladios

Should this only target the compose case?

I believe this would still fail with devcontainer build --buildkit auto while using podman

https://github.com/devcontainers/cli/blob/9ba1fdaa11dee087b142d33e4ac13c5788392e34/src/spec-node/singleContainer.ts#L190-L211

I guess I can add the check for podman in these lines and the compose lines instead, thought to short-circuit it from the --buildkit argument, not sure.

also maybe I'm missing it but is there a way to define Docker Path to be podman or docker before running the tests?

eqladios avatar Sep 27 '24 11:09 eqladios

With Podman 4.5.1 I see buildah 1.30 with podman buildx version and that supports --build-context which we use in containerFeatures.ts. Due to that I think single containers might still benefit from podman buildx.

chrmarti avatar Nov 04 '24 08:11 chrmarti

With Podman 4.5.1 I see buildah 1.30 with podman buildx version and that supports --build-context which we use in containerFeatures.ts. Due to that I think single containers might still benefit from podman buildx.

@chrmarti sorry didn't get to check this again, I believe I was testing with podman 5+ and it was failing, anyway I'll try to check it again soon with podman and update this accordingly

eqladios avatar Nov 25 '24 22:11 eqladios