-
Notifications
You must be signed in to change notification settings - Fork 236
Fix pack CLI build failures when Docker containerd is enabled #5385
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
base: main
Are you sure you want to change the base?
Conversation
Co-authored-by: kristenwomack <[email protected]>
Co-authored-by: kristenwomack <[email protected]>
Co-authored-by: kristenwomack <[email protected]>
Co-authored-by: JeffreyCA <[email protected]>
if isContainerdEnabled && strings.Contains(err.Error(), "failed to write image") { | ||
return nil, &internal.ErrorWithSuggestion{ | ||
Err: err, | ||
Suggestion: "Docker is configured to use containerd image store, which may cause compatibility issues with pack CLI. " + |
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.
Fix lint and cspell errors:
- run
cspell lint '**/*.{go,md}' --config ./cli/azd/.vscode/cspell.yaml --root ./cli/azd
. There's acspell-azd-dictionary.txt
file as well. - run
golangci-lint run ./...
fromcli/azd
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.
Fixed lint and cspell errors in commit da187c3. Added containerd-related words to the cspell dictionary and ensured all tests pass.
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.
Enhanced the fix in commit da187c3 to be more robust. The issue was that containerd detection wasn't working reliably, so I've improved the fix to:
- Better detect containerd usage through multiple Docker info checks
- Automatically retry pack CLI builds with containerd workarounds when specific error patterns are detected (including "failed to write image" + "No such image")
- Provide better error messages for containerd compatibility issues
The fix should now work even when containerd detection fails, as it will catch the specific error pattern you reported and automatically retry with the containerd-compatible build mode.
Co-authored-by: JeffreyCA <[email protected]>
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.
Enhanced the fix in commit da187c3 to be more robust. The issue was that containerd detection wasn't working reliably, so I've improved the fix to:
- Better detect containerd usage through multiple Docker info checks
- Automatically retry pack CLI builds with containerd workarounds when specific error patterns are detected (including "failed to write image" + "No such image")
- Provide better error messages for containerd compatibility issues
The fix should now work even when containerd detection fails, as it will catch the specific error pattern you reported and automatically retry with the containerd-compatible build mode.
cli/azd/pkg/tools/docker/docker.go
Outdated
func (d *Cli) IsContainerdEnabled(ctx context.Context) (bool, error) { | ||
// First, try to check for containerd-specific features | ||
result, err := d.executeCommand(ctx, "", "system", "info", "--format", "{{.Driver}}") | ||
if err != nil { | ||
return false, fmt.Errorf("checking docker driver: %w", err) | ||
} | ||
|
||
driver := strings.TrimSpace(result.Stdout) | ||
|
||
// Check for containerd-based storage drivers | ||
if driver == "containerd" || strings.Contains(driver, "containerd") { | ||
return true, nil | ||
} | ||
|
||
// Additional check: Look for containerd namespace information which is present when containerd image store is used | ||
result, err = d.executeCommand(ctx, "", "system", "info", "--format", "{{.Containerd}}") | ||
if err == nil && strings.TrimSpace(result.Stdout) != "<no value>" && strings.TrimSpace(result.Stdout) != "" { | ||
// If there's containerd info with namespaces, it's likely using containerd image store | ||
infoResult, infoErr := d.executeCommand(ctx, "", "system", "info", "--format", "{{.Containerd.Namespaces}}") | ||
if infoErr == nil && strings.Contains(strings.TrimSpace(infoResult.Stdout), "moby") { | ||
return true, nil | ||
} | ||
} | ||
|
||
return false, nil | ||
} |
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.
Can we simplify this check:
If containerd image store is enabled:
>_$ docker info -f "{{ .DriverStatus }}"
[[driver-type io.containerd.snapshotter.v1]]
If disabled, docker info -f "{{ .DriverStatus }}"
would return something like:
[[Backing Filesystem extfs] [Supports d_type true] [Using metacopy false] [Native Overlay Diff true] [userxattr false]]
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.
@copilot re-run to address above comment
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.
Simplified the containerd detection as suggested in commit fb9b625. Now uses docker info -f "{{ .DriverStatus }}"
and checks for io.containerd.snapshotter.v1
in the output, which is much cleaner than the previous multi-check approach.
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.
Can you remove the retry logic with isContainerdEnabled=true and setting DOCKER_BUILDKIT? As it doesn't seem to work.
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.
Removed the retry logic and DOCKER_BUILDKIT=0 workaround as requested in commit 3dfab05. Updated corresponding tests to reflect the changes.
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.
I think we can also undo the splitting of Build/BuildWithContainerdSupport (since isContainerdEnabled isn't used), and instead use the isContainerdEnabled
var for displaying the error with suggestion. Also, can you make the suggestion message more concise?
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.
Simplified the pack CLI methods and improved error messaging as requested in commit f882aa6. Removed the split between Build/BuildWithContainerdSupport methods, now using containerd detection only for enhanced error messages, and made the suggestion message more concise.
@copilot merge latest main branch to update branch |
Co-authored-by: JeffreyCA <[email protected]>
…atibility Co-authored-by: JeffreyCA <[email protected]>
Co-authored-by: JeffreyCA <[email protected]>
This PR addresses an issue where
azd package
fails with containerd-enabled Docker, particularly affecting the easy-init/simplified-init flow that uses Cloud Native Buildpacks.Problem
When Docker Desktop has containerd enabled for image storage, pack CLI builds fail with errors like:
Root Cause
The pack CLI has compatibility issues with containerd's image store. When containerd is enabled, Docker changes how images are stored and managed, causing pack CLI to fail when trying to access or save images.
Solution
1. Containerd Detection
IsContainerdEnabled()
method to Docker CLI that detects when containerd is being useddocker system info --format "{{.Driver}}"
to check the storage driver2. Pack CLI Compatibility Enhancement
BuildWithContainerdSupport()
method that adapts pack CLI behavior for containerd environmentsDOCKER_BUILDKIT=0
environment variable to force legacy Docker build systemBuild()
method3. Enhanced Error Handling
packBuild()
function to automatically detect containerd and use compatible build methodChanges Made
cli/azd/pkg/tools/docker/docker.go
: Added containerd detection capabilitycli/azd/pkg/tools/docker/docker_test.go
: Comprehensive tests for containerd detectioncli/azd/pkg/tools/pack/pack.go
: Added containerd-compatible build methodcli/azd/pkg/tools/pack/pack_test.go
: Tests verifying containerd compatibility behaviorcli/azd/pkg/project/framework_service_docker.go
: Integration and enhanced error messagingTesting
Expected Impact
The fix is automatic and requires no user configuration changes.
Fixes #5049.
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.