-
Notifications
You must be signed in to change notification settings - Fork 455
fix: ARC/DinD pre-create mount dirs, install docker compose, and pull build-tools image #42906
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
84ed43e
293c001
581be88
29948fb
0d6ad63
c67239f
18d8420
65f31c8
d628540
5f50ed0
30da62a
4e9079e
53fed35
ca8c762
8c82fdf
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -131,6 +131,19 @@ func collectDockerImages(tools map[string]any, workflowData *WorkflowData, actio | |
| dockerLog.Printf("Added AWF cli-proxy sidecar container: %s", cliProxyImage) | ||
| } | ||
| } | ||
|
|
||
| // Add build-tools sysroot image for ARC/DinD topology. | ||
| // AWF uses this as an init container to populate the sysroot volume with | ||
| // system binaries (gcc, make, libraries) that are invisible on the DinD daemon's FS. | ||
| if isArcDindTopology(workflowData) { | ||
| buildToolsImage := constants.DefaultFirewallRegistry + "/build-tools:" + awfImageTag | ||
| if !setutil.Contains(imageSet, buildToolsImage) { | ||
| images = append(images, buildToolsImage) | ||
| imageSet[buildToolsImage] = struct { | ||
| }{} | ||
| dockerLog.Printf("Added AWF build-tools sysroot container for arc-dind: %s", buildToolsImage) | ||
| } | ||
| } | ||
|
Comment on lines
+135
to
+146
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added |
||
| } | ||
|
|
||
| // Collect sandbox.mcp container (MCP gateway) | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,91 @@ | ||
| //go:build !integration | ||
|
|
||
| package workflow | ||
|
|
||
| import ( | ||
| "slices" | ||
| "strings" | ||
| "testing" | ||
|
|
||
| "github.com/github/gh-aw/pkg/constants" | ||
| ) | ||
|
|
||
| func TestCollectDockerImages_BuildToolsForArcDind(t *testing.T) { | ||
| // Use a version without "v" prefix — getAWFImageTag strips it | ||
| awfImageTag := "0.27.13" | ||
|
|
||
| t.Run("includes build-tools image when topology is arc-dind and firewall enabled", func(t *testing.T) { | ||
| workflowData := &WorkflowData{ | ||
| AI: "claude", | ||
| NetworkPermissions: &NetworkPermissions{ | ||
| Firewall: &FirewallConfig{ | ||
| Enabled: true, | ||
| Version: awfImageTag, | ||
| }, | ||
| }, | ||
| RunnerConfig: &RunnerConfig{Topology: RunnerTopologyArcDind}, | ||
| } | ||
|
|
||
| images := collectDockerImages(nil, workflowData, ActionModeRelease) | ||
|
|
||
| buildToolsImage := constants.DefaultFirewallRegistry + "/build-tools:" + awfImageTag | ||
| if !slices.Contains(images, buildToolsImage) { | ||
| t.Errorf("Expected build-tools image %q in collected images for arc-dind, got: %v", buildToolsImage, images) | ||
| } | ||
| }) | ||
|
|
||
| t.Run("excludes build-tools image when topology is not arc-dind", func(t *testing.T) { | ||
| workflowData := &WorkflowData{ | ||
| AI: "claude", | ||
| NetworkPermissions: &NetworkPermissions{ | ||
| Firewall: &FirewallConfig{ | ||
| Enabled: true, | ||
| Version: awfImageTag, | ||
| }, | ||
| }, | ||
| } | ||
|
|
||
| images := collectDockerImages(nil, workflowData, ActionModeRelease) | ||
|
|
||
| buildToolsImage := constants.DefaultFirewallRegistry + "/build-tools:" + awfImageTag | ||
| if slices.Contains(images, buildToolsImage) { | ||
| t.Errorf("Did not expect build-tools image %q in collected images without arc-dind topology, got: %v", buildToolsImage, images) | ||
| } | ||
| }) | ||
|
|
||
| t.Run("excludes build-tools image when firewall is disabled even with arc-dind topology", func(t *testing.T) { | ||
| workflowData := &WorkflowData{ | ||
| AI: "claude", | ||
| RunnerConfig: &RunnerConfig{Topology: RunnerTopologyArcDind}, | ||
| } | ||
|
|
||
| images := collectDockerImages(nil, workflowData, ActionModeRelease) | ||
|
|
||
| for _, img := range images { | ||
| if strings.Contains(img, "/build-tools:") { | ||
| t.Errorf("Did not expect any build-tools image when firewall is disabled, got: %v", images) | ||
| } | ||
| } | ||
| }) | ||
|
|
||
| t.Run("build-tools image uses the correct AWF image tag", func(t *testing.T) { | ||
| customTag := "0.27.5" | ||
| workflowData := &WorkflowData{ | ||
| AI: "copilot", | ||
| NetworkPermissions: &NetworkPermissions{ | ||
| Firewall: &FirewallConfig{ | ||
| Enabled: true, | ||
| Version: customTag, | ||
| }, | ||
| }, | ||
| RunnerConfig: &RunnerConfig{Topology: RunnerTopologyArcDind}, | ||
| } | ||
|
|
||
| images := collectDockerImages(nil, workflowData, ActionModeRelease) | ||
|
|
||
| expectedImage := constants.DefaultFirewallRegistry + "/build-tools:" + customTag | ||
| if !slices.Contains(images, expectedImage) { | ||
| t.Errorf("Expected build-tools image %q with custom tag, got: %v", expectedImage, images) | ||
| } | ||
| }) | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Valid observation. The embedded
action_pins.jsonis synced from.github/aw/actions-lock.jsonviamake sync-action-pins, which is populated during thegh aw update/ release cycle. Thebuild-toolsimage has never been included in that cycle because it's a new image type — the existing pinned versions only coveragent,squid,api-proxy,cli-proxy, andagent-act. The release workflow (release.md) validates that all container images referenced by compiled workflows have SHA pins before releasing, so the first release that includes this will require the pins to be present inactions-lock.json. Adding the digest pins requires registry access to fetch them — that needs to happen as part of thegh aw updatepin-refresh process, which is the appropriate path for all firewall images. Flagging this as a prerequisite for the first release that ships arc-dind support.