Skip to content

feat(runtime): add ContainerRuntime abstraction interface#281

Merged
skevetter merged 4 commits into
mainfrom
skevetter/devsy-098-container-runtime-abstraction
May 14, 2026
Merged

feat(runtime): add ContainerRuntime abstraction interface#281
skevetter merged 4 commits into
mainfrom
skevetter/devsy-098-container-runtime-abstraction

Conversation

@skevetter

@skevetter skevetter commented May 14, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Introduces a ContainerRuntime interface (pkg/docker/runtime.go) that replaces scattered IsPodman()/IsNerdctl() detection calls with capability-based queries (SupportsInternalBuildKit(), SupportsSignalProxy(), SupportsMountConsistency(), NeedsUserNamespaceArgs(), GPUAvailable())
  • Implements three runtime types: dockerRuntime, podmanRuntime, and nerdctlRuntime with correct capability flags
  • Adds Runtime config field to ProviderDockerDriverConfig for explicit runtime declaration; auto-detects from binary when not set, with per-binary caching
  • Refactors all 5 runtime detection call sites in pkg/docker/helper.go, pkg/driver/docker/docker.go, and pkg/driver/docker/build.go to use the new abstraction
  • Updates Podman provider with explicit runtime: podman config
  • Adds 14 unit tests covering runtime capabilities, name resolution, caching, and DockerHelper integration

Summary by CodeRabbit

Release Notes

  • New Features

    • Added explicit container runtime configuration option for Docker, Podman, and nerdctl
    • Improved GPU detection capabilities across all supported container runtimes
  • Improvements

    • Enhanced automatic runtime detection with caching for better performance
    • Better handling of runtime-specific features based on detected or configured runtime

Review Change Stack

Replace scattered IsPodman()/IsNerdctl() runtime detection with a
ContainerRuntime interface that expresses capabilities directly.
Runtimes can now be explicitly configured via provider config or
auto-detected from the binary with caching.
@netlify

netlify Bot commented May 14, 2026

Copy link
Copy Markdown

Deploy Preview for devsydev canceled.

Name Link
🔨 Latest commit 18afe35
🔍 Latest deploy log https://app.netlify.com/projects/devsydev/deploys/6a05291fa1d322000887380a

@coderabbitai

coderabbitai Bot commented May 14, 2026

Copy link
Copy Markdown

Warning

Rate limit exceeded

@skevetter has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 41 minutes and 7 seconds before requesting another review.

You’ve run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 3ccdcd6c-bbb6-4a13-839e-13c845025c9b

📥 Commits

Reviewing files that changed from the base of the PR and between 2c354b2 and 18afe35.

📒 Files selected for processing (4)
  • pkg/docker/helper.go
  • pkg/docker/runtime.go
  • pkg/docker/runtime_test.go
  • pkg/driver/docker/docker.go
📝 Walkthrough

Walkthrough

This PR refactors container runtime detection and capability queries from scattered if-statements into a reusable ContainerRuntime abstraction. Docker, Podman, and nerdctl are now concrete implementations with capability flags (internal BuildKit, signal proxy, mount consistency, user namespaces) and per-runtime GPU detection, replacing runtime-specific helper methods.

Changes

Container Runtime Abstraction

Layer / File(s) Summary
ContainerRuntime interface and concrete implementations
pkg/docker/runtime.go
RuntimeName constants and ContainerRuntime interface define capability queries (BuildKit, signal proxy, mount consistency, user namespaces) and GPU detection. Three concrete implementations (dockerRuntime, podmanRuntime, nerdctlRuntime) encode per-runtime capability flags and GPU detection via subprocess inspection.
Runtime detection and factory functions
pkg/docker/runtime.go
DetectRuntime() probes <command> --version with per-binary caching; RuntimeFromName() maps explicit names to implementations; detect() classifies output and defaults to Docker on failure.
DockerHelper runtime integration
pkg/docker/helper.go
DockerHelper gains Runtime field and GetRuntime() to return configured or auto-detected runtime. GPUSupportEnabled() delegates to runtime, and IsPodman/IsNerdctl check runtime name instead of probing versions.
Configuration and provider setup
pkg/provider/provider.go, providers/podman/provider.yaml
ProviderDockerDriverConfig adds Runtime field for explicit selection or auto-detection; Podman provider sets runtime: podman.
Docker driver runtime capability usage
pkg/driver/docker/docker.go, pkg/driver/docker/build.go
NewDockerDriver selects and stores runtime on helper. Driver calls SupportsSignalProxy(), SupportsMountConsistency(), NeedsUserNamespaceArgs(), and SupportsInternalBuildKit() instead of hardcoded Podman/nerdctl checks across multiple code paths.
Runtime abstraction test coverage
pkg/docker/runtime_test.go
Tests verify RuntimeFromName mapping, per-runtime capability flags, DockerHelper runtime selection with fallback and explicit config, and DetectRuntime caching behavior.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

  • devsy-org/devsy#270: Both PRs modify buildOrchestrator.selectStrategy to avoid internal BuildKit when the container runtime cannot support it; this PR generalizes via the SupportsInternalBuildKit() capability abstraction.

Suggested labels

size/l

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 15.38% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly describes the main feature being added: a ContainerRuntime abstraction interface, which aligns with the primary changes introducing the interface and refactoring code to use capability-based queries instead of scattered runtime checks.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@pkg/docker/helper.go`:
- Around line 62-63: The GPUSupportEnabled method currently calls
GetRuntime().GPUAvailable with context.TODO(), so add a cancellable context with
a sensible timeout and pass it instead: create a context via context.WithTimeout
(e.g., 5s or configurable), defer cancel(), then call
r.GetRuntime().GPUAvailable(ctx, r) and return its result; ensure
GPUSupportEnabled no longer uses context.TODO() so the runtime call cannot hang
indefinitely.

In `@pkg/docker/runtime_test.go`:
- Around line 24-28: The subtest is being created with an empty name when
tt.input == "", producing unclear test output; update the t.Run call in the loop
that exercises RuntimeFromName so it uses a non-empty descriptive name (e.g.,
compute name := tt.input; if name == "" set name = "empty" or name =
fmt.Sprintf("input=%q", tt.input)) and call t.Run(name, func(t *testing.T) { ...
}) instead of t.Run(tt.input, ...).

In `@pkg/docker/runtime.go`:
- Around line 87-95: RuntimeFromName currently defaults any unknown non-empty
name to docker, which hides typos; change RuntimeFromName(name string) to return
(ContainerRuntime, error) and implement logic: if strings.TrimSpace(name) == ""
return dockerRuntime{}, nil; switch on RuntimeName(strings.ToLower(name)) { case
RuntimePodman: return podmanRuntime{}, nil; case RuntimeNerdctl: return
nerdctlRuntime{}, nil; default: return nil, fmt.Errorf("unknown container
runtime: %q", name) } and update all callers to handle the error; reference
RuntimeFromName, RuntimeName, RuntimePodman, RuntimeNerdctl, and dockerRuntime
when making these edits.
- Around line 118-120: The detect() probe must not block indefinitely and must
not run while holding the runtimeDetectionCache mutex; change the exec call to
use exec.CommandContext with a short timeout (e.g., a couple seconds) so the
--version call is bounded, and restructure runtimeDetectionCache.get usage so
you only hold the lock to read/update the cached value — if cache is empty,
release the lock, run detect() (with the CommandContext timeout), then
re-acquire the lock to store the result (or handle concurrent in-progress
detection accordingly). Ensure you reference the detect function and the
runtimeDetectionCache get/set paths when making the change.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 4638a232-b5e2-4095-8f69-bc0bbf1f6a64

📥 Commits

Reviewing files that changed from the base of the PR and between 7a31acc and 2c354b2.

📒 Files selected for processing (7)
  • pkg/docker/helper.go
  • pkg/docker/runtime.go
  • pkg/docker/runtime_test.go
  • pkg/driver/docker/build.go
  • pkg/driver/docker/docker.go
  • pkg/provider/provider.go
  • providers/podman/provider.yaml

Comment thread pkg/docker/helper.go Outdated
Comment thread pkg/docker/runtime_test.go
Comment thread pkg/docker/runtime.go Outdated
Comment thread pkg/docker/runtime.go
skevetter added 3 commits May 13, 2026 20:33
- Add 5s timeout to GPUSupportEnabled() instead of context.TODO()
- Add 5s timeout to detect() runtime probe via exec.CommandContext
- Restructure runtimeDetectionCache to release mutex during exec
- Make RuntimeFromName return error for unknown runtime names
- Fix empty subtest name in RuntimeFromName test table
- Add test for unknown runtime name error case
@skevetter
skevetter merged commit d74b492 into main May 14, 2026
53 of 57 checks passed
@skevetter
skevetter deleted the skevetter/devsy-098-container-runtime-abstraction branch May 14, 2026 02:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant