Repository navigation
feat: native container steps for workflows and custom commands - #2626
Merged
Merged
Conversation
Add a native `type: container` step (build/push/run) to the shared step library used by workflows and custom commands, plus a formal step-outputs contract so one step can build an image and later steps push/run the exact produced artifact. Built on the existing pkg/container Docker/Podman runtime (ephemeral one-shot runner, image build/tag/push/inspect helpers) with per-step identity for registry auth and Docker Buildx Bake support. Includes the examples/container-step example, a hermetic GitHub Actions job that exercises build -> push -> run against a registry:2 service on localhost:5000 (plus failure-propagation), workflow step-type docs, a changelog blog post, and a roadmap update marking container steps and step outputs shipped. Also lands the design PRDs for the follow-on primitives (container components, compose components, and membership-based compositions) and trims the container-step PRD to cover only the procedural step. The earlier targets-based composition scaffolding is removed in favor of those PRDs. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Erik Osterman (Cloud Posse) (osterman)
requested review from
a team
as code owners
June 18, 2026 00:58
Contributor
|
Tip Atmos Pro
No affected stacks workflow was detected for this pull request. 
|
Erik Osterman (Cloud Posse) (osterman)
added
the
minor
Dependency Review✅ No vulnerabilities or license issues found.Scanned FilesNone |
Contributor
|
Important Cloud Posse Engineering Team Review RequiredThis pull request modifies files that require Cloud Posse's review. Please be patient, and a core maintainer will review your changes. To expedite this process, reach out to us on Slack in the |
lychee resolves /core-concepts/* as repo-relative file paths, which do not exist; use plain text in the PRD intro instead of website-route links. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
✅ Files skipped from review due to trivial changes (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds native container execution, workflow container sandboxes, step output propagation, and the supporting schema, runtime, example, CI, and documentation updates. ChangesContainer step and workflow container feature
Sequence Diagram(s)sequenceDiagram
participant Executor
participant StepRegistry
participant ContainerHandler
participant ContainerSession
Executor->>StepRegistry: resolve extended step type
StepRegistry-->>Executor: handler
Executor->>ContainerHandler: Validate and Execute
ContainerHandler->>ContainerSession: run or inspect inside workflow container
ContainerHandler-->>Executor: StepResult with outputs
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (3)
examples/container-step/Dockerfile (1)
1-1: ⚡ Quick winPin the base image instead of
alpine:latest.Line 1 is using a mutable tag, so builds can drift unexpectedly over time. Pin to a fixed version (ideally with digest) for reproducibility.
Suggested change
-FROM alpine:latest +FROM alpine:3.22.1🤖 Prompt for 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. In `@examples/container-step/Dockerfile` at line 1, The Dockerfile is using the mutable tag alpine:latest for the FROM instruction, which causes builds to be non-reproducible as the base image can change over time. Replace alpine:latest with a pinned version tag (such as alpine:3.18 or the latest stable version number) to ensure consistent and reproducible builds. For maximum reproducibility, also include the image digest (the sha256 hash) after the version tag..github/workflows/test.yml (1)
684-697: ⚡ Quick winUse an immutable registry image reference for the service container.
Lines 684-687 describe this job as hermetic, but Line 695 uses mutable
registry:2. Pinning by digest keeps runs reproducible and avoids silent upstream drift.Suggested change
- image: registry:2 + image: registry:2@sha256:<pinned-digest>🤖 Prompt for 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. In @.github/workflows/test.yml around lines 684 - 697, The registry service container in the container-step job uses a mutable tag `registry:2` which contradicts the hermetic nature described in the comments and can cause silent upstream drift. Replace the mutable `registry:2` tag in the image field of the registry service with an immutable digest reference by pinning it to a specific SHA256 digest (e.g., registry:2@sha256:...). This ensures the workflow remains reproducible across runs.pkg/container/podman_test.go (1)
793-801: ⚡ Quick winBroaden this unsupported-path test to assert error type and both guard branches.
Right now this covers only
Bake != niland checks message text. Add a case forEngine: "buildx"and assert the wrapped sentinel (ErrContainerRuntimeOperation) so error-contract regressions are caught early.💡 Suggested update
+import errUtils "github.com/cloudposse/atmos/errors" + func TestPodmanRuntime_BuildBakeUnsupported(t *testing.T) { runtime := NewPodmanRuntime() - err := runtime.Build(context.Background(), &BuildConfig{ - Bake: &BakeConfig{File: "docker-bake.hcl"}, - }) - - require.Error(t, err) - assert.Contains(t, err.Error(), "Docker Buildx requires Docker") + + t.Run("bake config", func(t *testing.T) { + err := runtime.Build(context.Background(), &BuildConfig{ + Bake: &BakeConfig{File: "docker-bake.hcl"}, + }) + require.Error(t, err) + assert.ErrorIs(t, err, errUtils.ErrContainerRuntimeOperation) + assert.Contains(t, err.Error(), "Docker Buildx requires Docker") + }) + + t.Run("buildx engine", func(t *testing.T) { + err := runtime.Build(context.Background(), &BuildConfig{ + Engine: "buildx", + }) + require.Error(t, err) + assert.ErrorIs(t, err, errUtils.ErrContainerRuntimeOperation) + assert.Contains(t, err.Error(), "Docker Buildx requires Docker") + }) }As per coding guidelines: “All errors MUST be wrapped using static errors… and use
errors.Is()for error checking,” and “All features need tests.”🤖 Prompt for 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. In `@pkg/container/podman_test.go` around lines 793 - 801, The test TestPodmanRuntime_BuildBakeUnsupported currently only covers the Bake field branch and uses message text assertion instead of error type checking. Extend this test to also cover the Engine: "buildx" branch by adding another BuildConfig case with just Engine set to "buildx", and replace the assert.Contains check with errors.Is to verify that both branches return the wrapped ErrContainerRuntimeOperation sentinel error rather than relying on error message text.Source: Coding guidelines
🤖 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 @.github/workflows/test.yml:
- Around line 726-733: The current test for the failing-check workflow only
validates that it returns a non-zero exit code, but does not verify the actual
reason for failure, which could allow false positives if the workflow breaks for
unrelated reasons. Capture the output from the atmos workflow failing-check
command execution and add an assertion that checks for expected marker text
within that output to confirm the workflow failed for the intended reason, not
due to incidental breakage.
In `@docs/prd/compose-components.md`:
- Around line 165-168: Remove the stale placeholder text `Plugins` `",remain"`
from the first bullet point where it registers compose in schema.Components.
This phrase appears corrupted and unclear; simply delete the parenthetical
phrase that contains it (starting with "may begin via") and end the line cleanly
after listing the sibling components
(terraform/helmfile/packer/ansible/container) to improve clarity and readability
of the implementation note.
In `@docs/prd/compositions.md`:
- Around line 219-221: The documentation in the compositions.md file contains a
stale and incorrect example path reference. Find the mention of
`examples/containers/storefront` in the text and replace it with the correct
path `examples/container-step`, which is the actual runnable example that exists
in the repository and is consistently referenced throughout the PR materials.
In `@pkg/container/detector.go`:
- Around line 107-117: The inner condition in
DetectRuntimeWithPreferenceAndRecovery checks Docker availability instead of
Podman availability. When preferred is set to Podman via environment variable,
the recovery should trigger only if Podman is not available, not Docker. Change
the condition from checking !isAvailable(ctx, TypeDocker) to !isAvailable(ctx,
TypePodman) so that TryRecoverPodmanRuntime is invoked when Podman is actually
unavailable and needs to be recovered, regardless of Docker's availability
status.
In `@pkg/container/ephemeral_test.go`:
- Around line 57-81: Add a new test function that serves as the negative-path
counterpart to TestRunEphemeralContainer_PullMissingRetriesCreate. This test
should verify that the pull recovery mechanism does NOT trigger when the
condition is absent. Set up a similar mock environment using gomock.InOrder, but
configure the EphemeralConfig with PullPolicy set to PullNever. Expect the
runtime.Create call to fail with an error, and crucially, do NOT include an
expectation for runtime.Pull in the gomock.InOrder chain. Verify that
RunEphemeralContainer returns the error without attempting to pull the image,
confirming that recovery is correctly skipped when the pull policy forbids
pulling.
In `@pkg/container/ephemeral.go`:
- Around line 77-85: Raw runtime errors from runtime.Pull() and
createEphemeralContainer() are being returned directly without wrapping, which
breaks the project's typed-error contract. Wrap all error returns in the
ephemeral container creation flow (including errors from runtime.Pull,
createEphemeralContainer, and other runtime operations) using the appropriate
container sentinel error defined in errors/errors.go before returning them, and
ensure all upstream error checking uses errors.Is() for proper error type
validation.
- Around line 90-93: The deferred cleanup block calls runtime.Remove with the
original ctx parameter, which may be cancelled or have an expired deadline by
the time cleanup executes, causing the removal to fail and leak containers.
Replace the ctx argument in the runtime.Remove call within the defer function
with a fresh, non-cancelled context created from context.Background() or a new
context derived from Background, ensuring the cleanup operation is not affected
by the original context's cancellation or deadline.
---
Nitpick comments:
In @.github/workflows/test.yml:
- Around line 684-697: The registry service container in the container-step job
uses a mutable tag `registry:2` which contradicts the hermetic nature described
in the comments and can cause silent upstream drift. Replace the mutable
`registry:2` tag in the image field of the registry service with an immutable
digest reference by pinning it to a specific SHA256 digest (e.g.,
registry:2@sha256:...). This ensures the workflow remains reproducible across
runs.
In `@examples/container-step/Dockerfile`:
- Line 1: The Dockerfile is using the mutable tag alpine:latest for the FROM
instruction, which causes builds to be non-reproducible as the base image can
change over time. Replace alpine:latest with a pinned version tag (such as
alpine:3.18 or the latest stable version number) to ensure consistent and
reproducible builds. For maximum reproducibility, also include the image digest
(the sha256 hash) after the version tag.
In `@pkg/container/podman_test.go`:
- Around line 793-801: The test TestPodmanRuntime_BuildBakeUnsupported currently
only covers the Bake field branch and uses message text assertion instead of
error type checking. Extend this test to also cover the Engine: "buildx" branch
by adding another BuildConfig case with just Engine set to "buildx", and replace
the assert.Contains check with errors.Is to verify that both branches return the
wrapped ErrContainerRuntimeOperation sentinel error rather than relying on error
message text.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: ce993397-1338-4a3b-ba3c-19e5b6413037
📒 Files selected for processing (45)
.github/workflows/test.ymlcmd/cmd_utils.godocs/prd/compose-components.mddocs/prd/compositions.mddocs/prd/container-actions-and-step-outputs.mddocs/prd/container-components.mdexamples/container-step/Dockerfileexamples/container-step/README.mdexamples/container-step/atmos.yamlexamples/container-step/docker-bake.hclexamples/container-step/workflows/container-step.yamlinternal/exec/workflow_utils.gopkg/container/common.gopkg/container/common_test.gopkg/container/detector.gopkg/container/docker.gopkg/container/ephemeral.gopkg/container/ephemeral_test.gopkg/container/image.gopkg/container/mock_runtime_test.gopkg/container/podman.gopkg/container/podman_test.gopkg/container/runtime.gopkg/devcontainer/mock_runtime_test.gopkg/devcontainer/runtime.gopkg/runner/runner.gopkg/runner/step/command_handlers_test.gopkg/runner/step/container.gopkg/runner/step/container_build.gopkg/runner/step/container_push.gopkg/runner/step/container_run.gopkg/runner/step/container_test.gopkg/runner/step/executor.gopkg/runner/step/types.gopkg/runner/step/variables.gopkg/runner/step/variables_test.gopkg/runner/task_test.gopkg/schema/task.gopkg/schema/workflow.gopkg/workflow/executor.gopkg/workflow/executor_test.gowebsite/blog/2026-06-17-native-container-steps.mdxwebsite/docs/workflows/_partials/_step-types.mdxwebsite/docs/workflows/workflows.mdxwebsite/src/data/roadmap.js
…tput DockerRuntime.Create() returned the full CombinedOutput of `docker create` as the container ID. When the image was missing locally, `docker create` pulled it inline and the pull progress was captured alongside the ID, so the multi-line blob was passed to `docker start`, producing "Error response from daemon: page not found" (the container-step example job). Extract the container ID as the last non-empty line of output via a shared extractContainerID() helper, used by both the Docker and Podman runtimes (de-duplicating Podman's inline loop). Add an empty-ID guard and a regression test covering the exact inline-pull output from CI. Also bundles related container refinements already in the worktree: runtime resolution/recovery ordering in detector.go, error wrapping and cancellation-safe cleanup in ephemeral.go, and PRD doc link fixes. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
pkg/container/ephemeral.go (1)
250-259:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winQuote preview args before joining.
The preview is not equivalent when an arg contains spaces:
[]string{"/bin/sh", "-lc", "echo ok"}renders as... -lc echo ok. Route all preview builders through one small quote-aware join helper.Preview join helper
+func joinPreviewArgs(args []string) string { + quoted := make([]string, len(args)) + for i, arg := range args { + quoted[i] = quotePreviewArg(arg) + } + return strings.Join(quoted, spaceSeparator) +} + +func quotePreviewArg(arg string) string { + if arg == "" { + return "''" + } + if !strings.ContainsAny(arg, " \t\n'\"\\$`!&;()<>|*?[]{}") { + return arg + } + return "'" + strings.ReplaceAll(arg, "'", "'\"'\"'") + "'" +} + func BuildEphemeralPreview(runtimeName string, config *EphemeralConfig) string { @@ - return strings.Join(args, spaceSeparator) + return joinPreviewArgs(args) } @@ - return strings.Join(args, spaceSeparator) + return joinPreviewArgs(args) } @@ - return strings.Join(args, spaceSeparator) + return joinPreviewArgs(args) } @@ - return strings.Join(args, spaceSeparator) + return joinPreviewArgs(args) }Also applies to: 307-308, 318-319, 329-330
🤖 Prompt for 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. In `@pkg/container/ephemeral.go` around lines 250 - 259, The current preview output uses strings.Join with a simple space separator, which fails to properly quote arguments containing spaces (e.g., an argument "echo ok" renders as "echo ok" instead of being treated as a single argument). Create a new quote-aware join helper function that properly quotes arguments when joining them, and replace the strings.Join(args, spaceSeparator) call in the return statement of this function with the new helper. Apply the same helper function to all other preview builders mentioned (at lines 307-308, 318-319, and 329-330) to ensure consistent quote-aware argument handling across all preview outputs.
🧹 Nitpick comments (1)
pkg/container/docker.go (1)
63-68: Add validation for container ID format and test the warning-after-ID edge case.The backward scan is already implemented and handles inline pull output correctly. However, the function lacks ID format validation—it returns any non-empty line without confirming it's actually a hex container ID. Add a regex check (docker IDs are 64-char hex) to guard against edge cases where trailing output isn't an ID, then add a test case:
<valid-id>\nWARNING: ...\nto ensure the function rejects warnings that might appear after the ID.🤖 Prompt for 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. In `@pkg/container/docker.go` around lines 63 - 68, The extractContainerID function currently accepts any non-empty line as a valid container ID without validating its format. Add a regex validation check to ensure the extracted containerID is a 64-character hexadecimal string before accepting it as valid—this guards against edge cases where trailing output (like warnings) might be incorrectly identified as a container ID. Additionally, add a test case for the extractContainerID function that covers the edge case where a valid container ID is followed by a WARNING line to verify the function correctly identifies and validates the true container ID while rejecting any non-ID output.
🤖 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/container/ephemeral.go`:
- Around line 157-159: The fmt.Errorf call in the error handling block uses two
%w verbs in a single format string, which is not the recommended pattern for
combining multiple errors. Replace this with errors.Join to explicitly combine
the create error (err) and pull error (pullErr). Wrap each individual error with
its context label using fmt.Errorf, then join them together, and finally wrap
the joined result with the overall error message "failed to create container and
pull image" to provide the full context.
---
Outside diff comments:
In `@pkg/container/ephemeral.go`:
- Around line 250-259: The current preview output uses strings.Join with a
simple space separator, which fails to properly quote arguments containing
spaces (e.g., an argument "echo ok" renders as "echo ok" instead of being
treated as a single argument). Create a new quote-aware join helper function
that properly quotes arguments when joining them, and replace the
strings.Join(args, spaceSeparator) call in the return statement of this function
with the new helper. Apply the same helper function to all other preview
builders mentioned (at lines 307-308, 318-319, and 329-330) to ensure consistent
quote-aware argument handling across all preview outputs.
---
Nitpick comments:
In `@pkg/container/docker.go`:
- Around line 63-68: The extractContainerID function currently accepts any
non-empty line as a valid container ID without validating its format. Add a
regex validation check to ensure the extracted containerID is a 64-character
hexadecimal string before accepting it as valid—this guards against edge cases
where trailing output (like warnings) might be incorrectly identified as a
container ID. Additionally, add a test case for the extractContainerID function
that covers the edge case where a valid container ID is followed by a WARNING
line to verify the function correctly identifies and validates the true
container ID while rejecting any non-ID output.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: f301098d-6b4f-4494-a196-a47ba25e1da5
📒 Files selected for processing (9)
docs/prd/compose-components.mddocs/prd/compositions.mdpkg/container/common.gopkg/container/common_test.gopkg/container/detector.gopkg/container/docker.gopkg/container/ephemeral.gopkg/container/ephemeral_test.gopkg/container/podman.go
✅ Files skipped from review due to trivial changes (2)
- docs/prd/compose-components.md
- docs/prd/compositions.md
🚧 Files skipped from review as they are similar to previous changes (3)
- pkg/container/detector.go
- pkg/container/podman.go
- pkg/container/common.go
Adds an EnvSetter seam so container runtimes (Docker/Podman) run their CLI subprocesses with a materialized environment (e.g. DOCKER_CONFIG for ECR login) instead of always inheriting os.Environ(). Wires the env through the runner container steps (build/push/run) and updates the container-step example, step-type docs, and the native-container-steps blog post. Includes new env_test.go and container_env_test.go coverage. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
pkg/container/podman.go (1)
357-362:⚠️ Potential issue | 🟠 Major | ⚡ Quick winWrap
PodmanRuntime.Logsfailures with the static runtime error.Line 361 returns
cmd.Run()directly, which makes this method inconsistent with the runtime error-wrapping contract used elsewhere.Proposed fix.
func (p *PodmanRuntime) Logs(ctx context.Context, containerID string, follow bool, tail string, stdout, stderr io.Writer) error { defer perf.Track(nil, "container.PodmanRuntime.Logs")() @@ cmd := p.command(ctx, args...) cmd.Stdout = stdout cmd.Stderr = stderr - return cmd.Run() + if err := cmd.Run(); err != nil { + return fmt.Errorf("%w: podman logs failed: %w", errUtils.ErrContainerRuntimeOperation, err) + } + return nil }As per coding guidelines, “All errors MUST be wrapped using static errors defined in errors/errors.go.”
🤖 Prompt for 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. In `@pkg/container/podman.go` around lines 357 - 362, The Logs method in PodmanRuntime returns the error from cmd.Run() directly without wrapping it, which violates the coding guideline that all errors must be wrapped using static errors defined in errors/errors.go. Instead of returning cmd.Run() directly, capture the error result from cmd.Run(), wrap it using an appropriate static error wrapper from the errors package (likely following the pattern used elsewhere in the codebase for similar runtime failures), and return the wrapped error.Source: Coding guidelines
pkg/container/docker.go (1)
395-399:⚠️ Potential issue | 🟠 Major | ⚡ Quick winWrap
DockerRuntime.Logsfailures with the static runtime error.Line 399 returns
cmd.Run()directly, so this path breaks the runtime error contract and cannot be matched consistently witherrors.Is(..., errUtils.ErrContainerRuntimeOperation).Proposed fix.
func (d *DockerRuntime) Logs(ctx context.Context, containerID string, follow bool, tail string, stdout, stderr io.Writer) error { defer perf.Track(nil, "container.DockerRuntime.Logs")() @@ cmd := d.command(ctx, args...) cmd.Stdout = stdout cmd.Stderr = stderr - return cmd.Run() + if err := cmd.Run(); err != nil { + return fmt.Errorf("%w: docker logs failed: %w", errUtils.ErrContainerRuntimeOperation, err) + } + return nil }As per coding guidelines, “All errors MUST be wrapped using static errors defined in errors/errors.go.”
🤖 Prompt for 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. In `@pkg/container/docker.go` around lines 395 - 399, The code returns cmd.Run() directly without wrapping the error, violating the runtime error contract required by the coding guidelines. Instead of directly returning cmd.Run(), assign its result to an error variable, then wrap it with errUtils.ErrContainerRuntimeOperation before returning. This ensures all errors from the DockerRuntime.Logs method are consistently wrapped with the static runtime error and can be matched using errors.Is().Source: Coding guidelines
🤖 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.
Outside diff comments:
In `@pkg/container/docker.go`:
- Around line 395-399: The code returns cmd.Run() directly without wrapping the
error, violating the runtime error contract required by the coding guidelines.
Instead of directly returning cmd.Run(), assign its result to an error variable,
then wrap it with errUtils.ErrContainerRuntimeOperation before returning. This
ensures all errors from the DockerRuntime.Logs method are consistently wrapped
with the static runtime error and can be matched using errors.Is().
In `@pkg/container/podman.go`:
- Around line 357-362: The Logs method in PodmanRuntime returns the error from
cmd.Run() directly without wrapping it, which violates the coding guideline that
all errors must be wrapped using static errors defined in errors/errors.go.
Instead of returning cmd.Run() directly, capture the error result from
cmd.Run(), wrap it using an appropriate static error wrapper from the errors
package (likely following the pattern used elsewhere in the codebase for similar
runtime failures), and return the wrapped error.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: cfbad11d-202e-42a7-af71-239962a1abfb
📒 Files selected for processing (16)
examples/container-step/README.mdexamples/container-step/atmos.yamlexamples/container-step/workflows/container-step.yamlpkg/container/common.gopkg/container/docker.gopkg/container/env_test.gopkg/container/podman.gopkg/container/runtime.gopkg/runner/step/container.gopkg/runner/step/container_build.gopkg/runner/step/container_env_test.gopkg/runner/step/container_push.gopkg/runner/step/container_run.gopkg/runner/step/variables.gowebsite/blog/2026-06-17-native-container-steps.mdxwebsite/docs/workflows/_partials/_step-types.mdx
✅ Files skipped from review due to trivial changes (3)
- examples/container-step/README.md
- examples/container-step/atmos.yaml
- website/docs/workflows/_partials/_step-types.mdx
🚧 Files skipped from review as they are similar to previous changes (8)
- examples/container-step/workflows/container-step.yaml
- website/blog/2026-06-17-native-container-steps.mdx
- pkg/runner/step/container_push.go
- pkg/container/runtime.go
- pkg/runner/step/container_build.go
- pkg/runner/step/variables.go
- pkg/runner/step/container.go
- pkg/runner/step/container_run.go
The linux Acceptance Tests job runs `make testacc-cover` with `-coverpkg=./...` across the whole repo, which is memory-heavy. It ran on the `terraform` runner whose RunsOn config has an 8 GiB RAM floor (`ram: [8, 64]`), so spot instances as small as 8 GiB were provisioned. The coverage build was killed (exit 137, "runner received a shutdown signal") at the coverage step on consecutive runs — every test package passed; only the coverage collection OOMed. Switch the linux matrix entry to `runner=large` (`ram: [16, 128]`, `disk: large`), the same runner the `release` job already uses. macos/windows run plain `testacc` (no coverage instrumentation) and are unaffected. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
createEphemeralContainer used a single fmt.Errorf with two %w verbs to wrap the create and pull errors. Per the project error-handling guidelines, combine multiple error causes with errors.Join: wrap each cause with its context label and join them under the outer message. Preserves both error chains for errors.Is. Addresses CodeRabbit review on PR #2626. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… OOM" This reverts commit b13f9e9.
…step handlers
Custom-command step dispatch had a broad pre-check that sent every *registered*
step type (except `exec`) — including `shell` and `atmos` — through the new
pkg/runner/step handlers, making the legacy switch cases dead code. That
regressed two ways:
- Windows: the shell handler hardcodes `exec.Command("sh", "-c", ...)`, so
shell-based custom commands (greet/echo/deploy/show) failed with
`exit status 126`, breaking their golden snapshots.
- Linux: the handler path spawned `atmos` grandchildren without process-group
cleanup, leaking orphaned `atmos` processes (32 in one acceptance run vs 0 on
main). Accumulated coverage-instrumented `atmos` processes exhausted memory and
the on-demand runner was torn down (exit 137 / "runner received a shutdown
signal") — which looked like an OOM/spot flake but was a process leak.
Gate on stepPkg.IsExtendedStepType instead (matching internal/exec/workflow_utils.go
and pkg/workflow/executor.go): shell/atmos/exec keep the legacy, cross-platform,
child-reaping paths; only genuinely-extended types (container, input, …) route
through the registered handlers via the default case, which now also carries the
resolved step env. Removes the now-dead runRegisteredStepHandler helper.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #2626 +/- ##
==========================================
+ Coverage 80.05% 80.20% +0.14%
==========================================
Files 1361 1371 +10
Lines 127594 129506 +1912
==========================================
+ Hits 102150 103873 +1723
- Misses 19783 19877 +94
- Partials 5661 5756 +95
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
Contributor
|
💥 This pull request now has conflicts. Could you fix it Erik Osterman (Cloud Posse) (@osterman)? 🙏 |
…ep-prd # Conflicts: # website/docs/cli/configuration/commands.mdx # website/plugins/file-browser/index.js
The "Check Markdown Links" job failed with a network connection error to https://no-color.org/ (referenced from docs/prd/help-system-architecture.md and docs/prd/io-handling-strategy.md). The URL is the canonical NO_COLOR spec site and resolves in a real browser; it just intermittently refuses connections from CI runners. Add it to the lychee exclude list, consistent with the existing entries for gnu.org, tldp.org, kubernetes.io, nx.dev, and other valid-but-flaky external links. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
|
💥 This pull request now has conflicts. Could you fix it Erik Osterman (Cloud Posse) (@osterman)? 🙏 |
…ep-prd # Conflicts: # lychee.toml # pkg/schema/schema.go
Andriy Knysh (aknysh)
approved these changes
Jun 23, 2026
Contributor
|
Tip Atmos Pro
No affected stacks workflow was detected for this pull request.
|
|
Warning Release Documentation RequiredThis PR is labeled
|
|
These changes were released in v1.222.0-rc.8. |
This was referenced Jul 2, 2026
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
what
type: containerstep (build/push/run) to the shared step library used by both workflows and custom commands, built on the existingpkg/containerDocker/Podman runtime (new ephemeral one-shot runner plus image build/tag/push/inspect helpers;ImageInspectadded to theRuntimeinterface, mocks regenerated).value/values/metadata/outputs/skipped/error(command-like steps addstdout/stderr/exit_code), so a build step can publish an image reference consumed by later push/run steps via{{ .steps.<name>.outputs.<key> }}.identityfor registry auth and Docker Buildx + Buildx Bake builds; Podman uses the nativepodman buildpath.examples/container-stepexample and a hermetic GitHub Actions job ([container-step]) that exercises build → push → run against aregistry:2service onlocalhost:5000, including failure-propagation.website/docs/workflows), add a changelog blog post, and update the roadmap (container steps + step outputs marked shipped).container-components.md,compose-components.md, and a rewritten membership-basedcompositions.md— and trimcontainer-actions-and-step-outputs.mdto cover only the procedural step. Remove the earliertargets:-based composition scaffolding (pkg/composition,cmd/composition, the composition step, andschema.Composition*) in favor of those PRDs.why
references
docs/prd/container-actions-and-step-outputs.md,docs/prd/container-components.md,docs/prd/compose-components.md,docs/prd/compositions.mdwebsite/blog/2026-06-17-native-container-steps.mdxwebsite/src/data/roadmap.js