From a39762eddff7eeee4e72c628a333b598a6d47a0b Mon Sep 17 00:00:00 2001 From: Jeffrey Chen Date: Wed, 9 Sep 2026 22:08:59 +0000 Subject: [PATCH] fix: repair remote container build fallback Build and package local images after an ACR Tasks scheduling refusal, preserve failure diagnostics, and synchronize shared runtime selection. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 6211a2e6-c280-4468-a804-4b019c435e9b --- cli/azd/docs/concurrency-model.md | 23 + .../grpcserver/container_service_test.go | 40 ++ cli/azd/pkg/containerregistry/remote_build.go | 20 + .../containerregistry/remote_build_test.go | 86 +++- cli/azd/pkg/project/container_helper.go | 119 ++++- .../container_helper_remote_build_test.go | 485 ++++++++++++++++++ cli/azd/pkg/project/container_helper_test.go | 96 ---- cli/azd/pkg/tools/docker/docker.go | 71 ++- .../tools/docker/docker_concurrency_test.go | 196 +++++++ docs/reference/azure-yaml-schema.md | 6 +- schemas/alpha/azure.yaml.json | 2 +- schemas/v1.0/azure.yaml.json | 2 +- 12 files changed, 984 insertions(+), 162 deletions(-) create mode 100644 cli/azd/pkg/project/container_helper_remote_build_test.go create mode 100644 cli/azd/pkg/tools/docker/docker_concurrency_test.go diff --git a/cli/azd/docs/concurrency-model.md b/cli/azd/docs/concurrency-model.md index 8be3b8dc942..416906965f2 100644 --- a/cli/azd/docs/concurrency-model.md +++ b/cli/azd/docs/concurrency-model.md @@ -185,6 +185,29 @@ race on `env` (one writing `KUBECONFIG=…`, the other reading it for an --- +## `pkg/tools/docker.Cli` + +| Lock | Protects | Acquired by | +|------|----------|-------------| +| `engineMu sync.Mutex` | `containerEngine` and runtime selection | `ContainerEngine`, `selectContainerEngine`, `getContainerEngine` | + +**Contract**: Every read and write of `containerEngine` holds `engineMu`. +`ContainerEngine` holds it across the cache check, environment/PATH detection, +and publication. `detectContainerEngineLocked` requires the caller to hold it. +`CheckInstalled` uses `selectContainerEngine` to select and snapshot under the +lock, then validates that snapshot, including error names, without the lock. +Each call repeats selection and readiness checks; failures are not cached. + +`Name`, `InstallUrl`, and container operations read through +`getContainerEngine`, which snapshots under the lock without detection and +defaults to Docker before selection. No engine lock is held during version +checks, daemon checks, builds, or other container subprocesses. + +**Why it matters**: Parallel services and remote-build fallbacks share the +singleton `docker.Cli`. + +--- + ## `pkg/project.containerAppTarget` and `pkg/project.aksTarget` These targets no longer carry package-level `envMu` / `aksEnvMu` mutexes diff --git a/cli/azd/internal/grpcserver/container_service_test.go b/cli/azd/internal/grpcserver/container_service_test.go index 3686def7de4..612382734b5 100644 --- a/cli/azd/internal/grpcserver/container_service_test.go +++ b/cli/azd/internal/grpcserver/container_service_test.go @@ -7,11 +7,13 @@ import ( "context" "errors" "fmt" + "net/http" osexec "os/exec" "testing" "github.com/Azure/azure-sdk-for-go/sdk/azcore" "github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/containerregistry/armcontainerregistry" + "github.com/azure/azure-dev/cli/azd/internal" "github.com/azure/azure-dev/cli/azd/pkg/azdext" "github.com/azure/azure-dev/cli/azd/pkg/containerregistry" "github.com/azure/azure-dev/cli/azd/pkg/environment" @@ -378,6 +380,44 @@ func TestMapContainerPublishError_PreservesResponseError(t *testing.T) { require.Same(t, err, mapContainerPublishError(err)) } +func TestMapContainerPublishError_LocalFallbackFailure(t *testing.T) { + t.Parallel() + + buildErr := &azdexec.ExitError{Cmd: "docker", ExitCode: 1} + tests := []struct { + name string + err error + }{ + {"RuntimeUnavailable", errors.New("local container runtime unavailable")}, + {"BuildFailed", buildErr}, + {"PushFailed", &internal.ErrorWithSuggestion{Err: buildErr, Suggestion: "Check registry authentication."}}, + {"Canceled", context.Canceled}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + responseErr := &azcore.ResponseError{ + StatusCode: http.StatusForbidden, + ErrorCode: "TasksOperationsNotAllowed", + } + remoteErr := &containerregistry.RemoteBuildUnavailableError{Err: responseErr} + err := fmt.Errorf("remote build failed: %w\n\nLocal fallback failed: %w", remoteErr, tt.err) + mapped := mapContainerToolError(mapContainerPublishError(err)) + require.Same(t, err, mapped) + require.ErrorIs(t, mapped, responseErr) + require.ErrorIs(t, mapped, tt.err) + + st, ok := status.FromError(mapHostError(mapped)) + require.True(t, ok) + require.Equal(t, err.Error(), st.Message()) + detail := requireServiceErrorDetail(t, st) + require.Equal(t, "TasksOperationsNotAllowed", detail.GetErrorCode()) + require.Equal(t, int32(http.StatusForbidden), detail.GetStatusCode()) + require.Empty(t, relayedExtensionErrorDetails(st)) + }) + } +} + func TestMapContainerToolError(t *testing.T) { t.Parallel() diff --git a/cli/azd/pkg/containerregistry/remote_build.go b/cli/azd/pkg/containerregistry/remote_build.go index fa4e814205b..4e04997a7b5 100644 --- a/cli/azd/pkg/containerregistry/remote_build.go +++ b/cli/azd/pkg/containerregistry/remote_build.go @@ -126,6 +126,22 @@ func (r *RemoteBuildManager) UploadBuildSource( return sourceUploadRes.SourceUploadDefinition, nil } +// RemoteBuildUnavailableError indicates that ACR explicitly refused to schedule a build. +// Callers may attempt a local build instead. Errors after submission do not use this type. +type RemoteBuildUnavailableError struct { + Err error +} + +// Error returns the original scheduling error. +func (e *RemoteBuildUnavailableError) Error() string { + return e.Err.Error() +} + +// Unwrap preserves the original Azure service error. +func (e *RemoteBuildUnavailableError) Unwrap() error { + return e.Err +} + // RemoteBuildRunError represents a terminal failure reported by an Azure Container Registry remote build. type RemoteBuildRunError struct { Status armcontainerregistry.RunStatus @@ -195,6 +211,10 @@ func (r *RemoteBuildManager) RunDockerBuildRequestWithLogs( runPoller, err := regClient.BeginScheduleRun(ctx, resourceGroupName, registryName, buildRequest, nil) if err != nil { + if responseErr, ok := errors.AsType[*azcore.ResponseError](err); ok && + responseErr.ErrorCode == "TasksOperationsNotAllowed" { + return &RemoteBuildUnavailableError{Err: err} + } return err } diff --git a/cli/azd/pkg/containerregistry/remote_build_test.go b/cli/azd/pkg/containerregistry/remote_build_test.go index d892df7c47e..460048649df 100644 --- a/cli/azd/pkg/containerregistry/remote_build_test.go +++ b/cli/azd/pkg/containerregistry/remote_build_test.go @@ -227,33 +227,65 @@ func TestRunDockerBuildRequestWithLogs_CredentialError(t *testing.T) { func TestRunDockerBuildRequestWithLogs_ScheduleRunError(t *testing.T) { t.Parallel() - mockCtx := mocks.NewMockContext(t.Context()) - mockCtx.HttpClient.When(func(request *http.Request) bool { - return strings.Contains(request.URL.Path, "scheduleRun") - }).RespondFn(func(request *http.Request) (*http.Response, error) { - return mocks.CreateHttpResponseWithBody( - request, http.StatusBadRequest, map[string]any{ - "error": map[string]string{ - "code": "BadRequest", - "message": "invalid build request", - }, - }, - ) - }) - - mgr := NewRemoteBuildManager( - mockCtx.SubscriptionCredentialProvider, - armOptionsNoRetry(mockCtx.HttpClient), - ) - - err := mgr.RunDockerBuildRequestWithLogs( - t.Context(), testSubscriptionID, testResourceGroup, testRegistryName, - &armcontainerregistry.DockerBuildRequest{}, io.Discard, - ) - require.Error(t, err) - require.Contains(t, err.Error(), "BadRequest") - var responseErr *azcore.ResponseError - require.ErrorAs(t, err, &responseErr) + tests := []struct { + name string + code string + status int + afterSubmission bool + wantUnavailable bool + }{ + {"TasksRefused", "TasksOperationsNotAllowed", http.StatusForbidden, false, true}, + {"InvalidRequest", "BadRequest", http.StatusBadRequest, false, false}, + {"AuthorizationFailed", "AuthorizationFailed", http.StatusForbidden, false, false}, + {"ServerError", "InternalServerError", http.StatusInternalServerError, false, false}, + {"RefusalAfterSubmission", "TasksOperationsNotAllowed", http.StatusForbidden, true, false}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + mockCtx := mocks.NewMockContext(t.Context()) + errorResponse := func(request *http.Request) (*http.Response, error) { + return mocks.CreateHttpResponseWithBody(request, tt.status, map[string]any{ + "error": map[string]string{ + "code": tt.code, + "message": "request refused", + }, + }) + } + mockCtx.HttpClient.When(func(request *http.Request) bool { + return strings.Contains(request.URL.Path, "scheduleRun") + }).RespondFn(func(request *http.Request) (*http.Response, error) { + if !tt.afterSubmission { + return errorResponse(request) + } + return mocks.CreateHttpResponseWithBody(request, http.StatusOK, armcontainerregistry.Run{ + Properties: &armcontainerregistry.RunProperties{RunID: new("run-id")}, + }) + }) + mockCtx.HttpClient.When(func(request *http.Request) bool { + return strings.Contains(request.URL.Path, "listLogSasUrl") + }).RespondFn(errorResponse) + + mgr := NewRemoteBuildManager( + mockCtx.SubscriptionCredentialProvider, + armOptionsNoRetry(mockCtx.HttpClient), + ) + err := mgr.RunDockerBuildRequestWithLogs( + t.Context(), testSubscriptionID, testResourceGroup, testRegistryName, + &armcontainerregistry.DockerBuildRequest{}, io.Discard, + ) + require.Error(t, err) + responseErr, ok := errors.AsType[*azcore.ResponseError](err) + require.True(t, ok) + require.Equal(t, tt.code, responseErr.ErrorCode) + unavailableErr, ok := errors.AsType[*RemoteBuildUnavailableError](err) + require.Equal(t, tt.wantUnavailable, ok) + if ok { + require.Same(t, responseErr, unavailableErr.Unwrap()) + require.Equal(t, responseErr.Error(), unavailableErr.Error()) + } + }) + } } func TestRunDockerBuildRequestWithLogs_TerminalStatus(t *testing.T) { diff --git a/cli/azd/pkg/project/container_helper.go b/cli/azd/pkg/project/container_helper.go index dad5dddb4f6..eda18fab2de 100644 --- a/cli/azd/pkg/project/container_helper.go +++ b/cli/azd/pkg/project/container_helper.go @@ -404,6 +404,15 @@ func (ch *ContainerHelper) Build( return &ServiceBuildResult{}, nil } + return ch.buildLocalImage(ctx, serviceConfig, env, progress) +} + +func (ch *ContainerHelper) buildLocalImage( + ctx context.Context, + serviceConfig *ServiceConfig, + env *environment.Environment, + progress *async.Progress[ServiceProgress], +) (*ServiceBuildResult, error) { dockerOptions := getDockerOptionsWithDefaults(serviceConfig.Docker) resolveDockerPaths(serviceConfig, &dockerOptions) @@ -601,6 +610,16 @@ func (ch *ContainerHelper) Package( return &ServicePackageResult{}, nil } + return ch.packageLocalImage(ctx, serviceConfig, serviceContext, env, progress) +} + +func (ch *ContainerHelper) packageLocalImage( + ctx context.Context, + serviceConfig *ServiceConfig, + serviceContext *ServiceContext, + env *environment.Environment, + progress *async.Progress[ServiceProgress], +) (*ServicePackageResult, error) { var imageId string var sourceImage string var imageHash string @@ -779,6 +798,10 @@ func (ch *ContainerHelper) Publish( fields.ContainerRemoteBuildKey.Bool(serviceConfig.Docker.RemoteBuild), ) + if err := ctx.Err(); err != nil { + return nil, err + } + var remoteImage string if err := validatePublishOptions(serviceConfig, options); err != nil { @@ -807,20 +830,29 @@ func (ch *ContainerHelper) Publish( } else if serviceConfig.Docker.RemoteBuild { remoteImage, err = ch.runRemoteBuild(ctx, serviceConfig, targetResource, env, progress, imageOverride) if err != nil { - // Check if a local container runtime (Docker/Podman) is available before falling back - if dockerErr := ch.docker.CheckInstalled(ctx); dockerErr != nil { - return nil, fmt.Errorf( - "remote build failed: %w\n\nLocal fallback unavailable: %w", - err, dockerErr) + remoteErr := err + if errors.Is(remoteErr, context.Canceled) || errors.Is(remoteErr, context.DeadlineExceeded) { + return nil, remoteErr + } + if ctxErr := ctx.Err(); ctxErr != nil { + return nil, fmt.Errorf("remote build failed: %w\n\nPublish canceled: %w", remoteErr, ctxErr) + } + if _, ok := errors.AsType[*containerregistry.RemoteBuildUnavailableError](remoteErr); !ok { + return nil, remoteErr } - ch.console.MessageUxItem(ctx, &ux.WarningMessage{ - Description: fmt.Sprintf( - "Remote build failed: %s\nFalling back to local Docker build.", err), - HidePrefix: false, - }) - remoteImage, err = ch.publishLocalImage( + remoteImage, err = ch.publishLocalFallback( ctx, serviceConfig, serviceContext, env, progress, imageOverride) + if err != nil { + err = fmt.Errorf("remote build failed: %w\n\nLocal fallback failed: %w", remoteErr, err) + if suggestion, ok := errors.AsType[*internal.ErrorWithSuggestion](err); ok { + // Rich CLI output renders suggestion.Err rather than its outer wrappers. + combined := *suggestion + combined.Err = err + return nil, &combined + } + return nil, err + } } } else if useDotnetPublishForDockerBuild(serviceConfig) { remoteImage, err = ch.runDotnetPublish(ctx, serviceConfig, targetResource, env, progress) @@ -848,7 +880,70 @@ func (ch *ContainerHelper) Publish( }, nil } -// publishLocalImage builds the image locally and pushes it to the remote registry, it returns the full remote image name. +func (ch *ContainerHelper) publishLocalFallback( + ctx context.Context, + serviceConfig *ServiceConfig, + serviceContext *ServiceContext, + env *environment.Environment, + progress *async.Progress[ServiceProgress], + imageOverride *imageOverride, +) (string, error) { + if err := ctx.Err(); err != nil { + return "", err + } + if err := ch.docker.CheckInstalled(ctx); err != nil { + return "", fmt.Errorf("local container runtime unavailable: %w", err) + } + if err := ctx.Err(); err != nil { + return "", err + } + + var hasPackage bool + if serviceContext != nil { + artifact, found := serviceContext.Package.FindFirst(WithKind(ArtifactKindContainer)) + hasPackage = found + if found && artifact.LocationKind == LocationKindRemote { + return "", errors.New("local fallback requires a local container package") + } + } + + action := fmt.Sprintf("Building locally with %s.", ch.docker.Name()) + if hasPackage { + action = fmt.Sprintf("Publishing the existing local image with %s.", ch.docker.Name()) + } + ch.console.MessageUxItem(ctx, &ux.WarningMessage{ + Description: fmt.Sprintf("ACR refused the build request with TasksOperationsNotAllowed. %s", action), + }) + + if !hasPackage { + // Remote mode skips local build/package. Keep fallback artifacts separate from completed lifecycle state. + serviceContext = NewServiceContext() + buildResult, err := ch.buildLocalImage(ctx, serviceConfig, env, progress) + if err != nil { + return "", fmt.Errorf("building local image: %w", err) + } + if err := serviceContext.Build.Add(buildResult.Artifacts...); err != nil { + return "", fmt.Errorf("adding local build artifacts: %w", err) + } + if err := ctx.Err(); err != nil { + return "", err + } + packageResult, err := ch.packageLocalImage(ctx, serviceConfig, serviceContext, env, progress) + if err != nil { + return "", fmt.Errorf("packaging local image: %w", err) + } + if err := serviceContext.Package.Add(packageResult.Artifacts...); err != nil { + return "", fmt.Errorf("adding local package artifacts: %w", err) + } + } + + if err := ctx.Err(); err != nil { + return "", err + } + return ch.publishLocalImage(ctx, serviceConfig, serviceContext, env, progress, imageOverride) +} + +// publishLocalImage publishes a prepared container package and returns the full remote image name. func (ch *ContainerHelper) publishLocalImage( ctx context.Context, serviceConfig *ServiceConfig, diff --git a/cli/azd/pkg/project/container_helper_remote_build_test.go b/cli/azd/pkg/project/container_helper_remote_build_test.go new file mode 100644 index 00000000000..26c3f15b4a0 --- /dev/null +++ b/cli/azd/pkg/project/container_helper_remote_build_test.go @@ -0,0 +1,485 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +package project + +import ( + "context" + "encoding/json" + "errors" + "io" + "net/http" + "os" + "path/filepath" + "slices" + "strconv" + "strings" + "sync" + "testing" + "time" + + "github.com/Azure/azure-sdk-for-go/sdk/azcore" + "github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/containerregistry/armcontainerregistry" + "github.com/azure/azure-dev/cli/azd/internal" + "github.com/azure/azure-dev/cli/azd/pkg/async" + "github.com/azure/azure-dev/cli/azd/pkg/cloud" + "github.com/azure/azure-dev/cli/azd/pkg/containerregistry" + "github.com/azure/azure-dev/cli/azd/pkg/environment" + "github.com/azure/azure-dev/cli/azd/pkg/exec" + "github.com/azure/azure-dev/cli/azd/pkg/osutil" + "github.com/azure/azure-dev/cli/azd/pkg/output/ux" + "github.com/azure/azure-dev/cli/azd/pkg/tools/docker" + "github.com/azure/azure-dev/cli/azd/pkg/tools/dotnet" + "github.com/azure/azure-dev/cli/azd/test/mocks" + "github.com/benbjohnson/clock" + "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/require" +) + +func TestContainerHelperRemoteBuildFallback(t *testing.T) { + tests := []struct { + name string + runtime string + packageImage string + metadataImage bool + emptyPackage bool + imageOverride string + failure string + cancelAt string + wantError string + wantOps []string + }{ + { + name: "BuildFromSource", + wantOps: []string{"schedule", "--version", "ps", "build", "package", "tag", "login", "push"}, + }, + { + name: "PodmanFromSource", runtime: "podman", + wantOps: []string{"schedule", "--version", "ps", "build", "package", "tag", "login", "push"}, + }, + { + name: "ImageOverride", imageOverride: "custom/repository:release", + wantOps: []string{"schedule", "--version", "ps", "build", "package", "tag", "login", "push"}, + }, + { + name: "SuppliedPackage", packageImage: "existing/image:tested", + wantOps: []string{"schedule", "--version", "ps", "tag", "login", "push"}, + }, + { + name: "LegacyPackageMetadata", packageImage: "existing/image:tested", metadataImage: true, + wantOps: []string{"schedule", "--version", "ps", "tag", "login", "push"}, + }, + { + name: "MalformedPackage", emptyPackage: true, + wantError: "failed retrieving package result details", + wantOps: []string{"schedule", "--version", "ps"}, + }, + { + name: "RuntimeUnavailable", failure: "--version", wantError: "local container runtime unavailable", + wantOps: []string{"schedule", "--version"}, + }, + { + name: "DaemonUnavailable", failure: "ps", wantError: "local container runtime unavailable", + wantOps: []string{"schedule", "--version", "ps"}, + }, + { + name: "BuildFailure", failure: "build", wantError: "building local image", + wantOps: []string{"schedule", "--version", "ps", "build"}, + }, + { + name: "PackageFailure", failure: "package", wantError: "packaging local image", + wantOps: []string{"schedule", "--version", "ps", "build", "package"}, + }, + { + name: "LoginFailure", failure: "login", wantError: "Local fallback failed", + wantOps: []string{"schedule", "--version", "ps", "build", "package", "tag", "login"}, + }, + { + name: "PushFailure", failure: "push", wantError: "Local fallback failed", + wantOps: []string{"schedule", "--version", "ps", "build", "package", "tag", "login", "push"}, + }, + { + name: "CanceledDuringScheduling", cancelAt: "schedule", wantError: "context canceled", + wantOps: []string{"schedule"}, + }, + { + name: "CanceledAfterReadiness", cancelAt: "ps", wantError: "Local fallback failed", + wantOps: []string{"schedule", "--version", "ps"}, + }, + { + name: "CanceledAfterBuild", cancelAt: "build", wantError: "Local fallback failed", + wantOps: []string{"schedule", "--version", "ps", "build"}, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + runtime := tt.runtime + if runtime == "" { + runtime = "docker" + } + t.Setenv("AZD_CONTAINER_RUNTIME", runtime) + t.Setenv("NO_COLOR", "1") + f := newRemoteBuildFixture(t) + f.scheduleCode = "TasksOperationsNotAllowed" + f.failure = tt.failure + if tt.failure == "login" { + f.loginCall.Return(f.localError) + } + f.cancelAt = tt.cancelAt + ctx, cancel := context.WithCancel(*f.mocks.Context) + defer cancel() + f.cancel = cancel + f.options.Image = tt.imageOverride + + progress := async.NewNoopProgress[ServiceProgress]() + defer progress.Done() + build, err := f.helper.Build(ctx, f.config, f.serviceContext, f.env, progress) + require.NoError(t, err) + require.Empty(t, build.Artifacts) + require.NoError(t, f.serviceContext.Build.Add(build.Artifacts...)) + pkg, err := f.helper.Package(ctx, f.config, f.serviceContext, f.env, progress) + require.NoError(t, err) + require.Empty(t, pkg.Artifacts) + require.NoError(t, f.serviceContext.Package.Add(pkg.Artifacts...)) + require.Empty(t, f.operations) + + if tt.packageImage != "" || tt.emptyPackage { + artifact := &Artifact{ + Kind: ArtifactKindContainer, LocationKind: LocationKindLocal, Location: tt.packageImage, + } + if tt.metadataImage { + artifact.Location = "" + artifact.Metadata = map[string]string{"targetImage": tt.packageImage} + } + f.serviceContext.Package = append(f.serviceContext.Package, artifact) + } else { + require.NoError(t, f.serviceContext.Package.Add(&Artifact{ + Kind: ArtifactKindConfig, LocationKind: LocationKindLocal, Location: "config.json", + })) + } + before, err := json.Marshal(f.serviceContext) + require.NoError(t, err) + dockerConfig := f.config.Docker + + result, err := f.publish(ctx) + require.Equal(t, tt.wantOps, f.operations) + after, marshalErr := json.Marshal(f.serviceContext) + require.NoError(t, marshalErr) + require.JSONEq(t, string(before), string(after)) + require.Equal(t, dockerConfig, f.config.Docker) + + if tt.wantError != "" { + require.ErrorContains(t, err, tt.wantError) + require.Nil(t, result) + if tt.failure == "--version" || tt.failure == "ps" || tt.cancelAt == "schedule" || tt.cancelAt == "ps" { + require.Empty(t, f.mocks.Console.Output()) + } + if tt.cancelAt == "schedule" { + require.ErrorIs(t, err, context.Canceled) + return + } + responseErr, ok := errors.AsType[*azcore.ResponseError](err) + require.True(t, ok) + require.Equal(t, "TasksOperationsNotAllowed", responseErr.ErrorCode) + if tt.failure != "" { + require.ErrorIs(t, err, f.localError) + } + if tt.failure == "push" { + suggestion, ok := errors.AsType[*internal.ErrorWithSuggestion](err) + require.True(t, ok) + display := &ux.ErrorWithSuggestion{ + Err: suggestion.Err, Message: suggestion.Message, + Suggestion: suggestion.Suggestion, Links: suggestion.Links, + } + rendered := display.ToString("") + require.Contains(t, rendered, "TasksOperationsNotAllowed") + require.Contains(t, rendered, f.localError.Error()) + require.Contains(t, rendered, "docker login") + } + if tt.cancelAt != "" { + require.ErrorIs(t, err, context.Canceled) + } + return + } + require.NoError(t, err) + expectedImage := "contoso.azurecr.io/project/app-dev:azd-deploy-0" + if tt.packageImage != "" { + expectedImage = "contoso.azurecr.io/" + tt.packageImage + } + if tt.imageOverride != "" { + expectedImage = "contoso.azurecr.io/" + tt.imageOverride + } + require.Equal(t, ArtifactCollection{{ + Kind: ArtifactKindContainer, LocationKind: LocationKindRemote, Location: expectedImage, + Metadata: map[string]string{"remoteImage": expectedImage}, + }}, result.Artifacts) + require.Equal(t, expectedImage, f.pushedImage) + require.Len(t, f.mocks.Console.Output(), 1) + warning := f.mocks.Console.Output()[0] + require.Contains(t, warning, "TasksOperationsNotAllowed") + if tt.packageImage == "" { + require.Contains(t, warning, "Building locally with "+f.helper.docker.Name()) + } else { + require.Contains(t, warning, "Publishing the existing local image") + } + }) + } +} + +func TestContainerHelperRemoteBuildNoFallback(t *testing.T) { + tests := []struct { + name string + status armcontainerregistry.RunStatus + scheduleCode string + logError bool + platform string + contextError error + }{ + {name: "Success", status: armcontainerregistry.RunStatusSucceeded}, + {name: "Failed", status: armcontainerregistry.RunStatusFailed}, + {name: "Error", status: armcontainerregistry.RunStatusError}, + {name: "Timeout", status: armcontainerregistry.RunStatusTimeout}, + {name: "Canceled", status: armcontainerregistry.RunStatusCanceled}, + {name: "BadRequest", scheduleCode: "BadRequest"}, + {name: "AuthorizationFailed", scheduleCode: "AuthorizationFailed"}, + {name: "UnknownOutcome", logError: true}, + {name: "UnsupportedPlatform", platform: "linux/arm64"}, + {name: "ContextCanceled", contextError: context.Canceled}, + {name: "DeadlineExceeded", contextError: context.DeadlineExceeded}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + f := newRemoteBuildFixture(t) + f.runStatus = tt.status + f.scheduleCode = tt.scheduleCode + f.logError = tt.logError + f.config.Docker.Platform = tt.platform + ctx := *f.mocks.Context + if errors.Is(tt.contextError, context.Canceled) { + canceled, cancel := context.WithCancel(ctx) + cancel() + ctx = canceled + } else if errors.Is(tt.contextError, context.DeadlineExceeded) { + expired, cancel := context.WithDeadline(ctx, time.Now().Add(-time.Second)) + defer cancel() + ctx = expired + } + + result, err := f.publish(ctx) + if tt.status == armcontainerregistry.RunStatusSucceeded { + require.NoError(t, err) + require.Len(t, result.Artifacts, 1) + } else { + require.Error(t, err) + require.Nil(t, result) + if tt.contextError != nil { + require.ErrorIs(t, err, tt.contextError) + } + if tt.status != "" { + runErr, ok := errors.AsType[*containerregistry.RemoteBuildRunError](err) + require.True(t, ok) + require.Equal(t, tt.status, runErr.Status) + require.ErrorContains(t, err, "build log") + } + _, eligible := errors.AsType[*containerregistry.RemoteBuildUnavailableError](err) + require.False(t, eligible) + } + if tt.platform != "" || tt.contextError != nil { + require.Empty(t, f.operations) + } else { + require.Equal(t, []string{"schedule"}, f.operations) + } + require.Empty(t, f.mocks.Console.Output()) + }) + } +} + +func TestContainerHelperRemoteBuildParallelFallback(t *testing.T) { + t.Setenv("AZD_CONTAINER_RUNTIME", "docker") + fixtures := []*remoteBuildFixture{newRemoteBuildFixture(t), newRemoteBuildFixture(t)} + var wg sync.WaitGroup + results := make([]*ServicePublishResult, len(fixtures)) + errs := make([]error, len(fixtures)) + for i, f := range fixtures { + f.scheduleCode = "TasksOperationsNotAllowed" + f.helper.docker = fixtures[0].helper.docker + wg.Go(func() { + results[i], errs[i] = f.publish(*f.mocks.Context) + }) + } + wg.Wait() + for i := range fixtures { + require.NoError(t, errs[i]) + require.Len(t, results[i].Artifacts, 1) + require.Empty(t, fixtures[i].serviceContext.Build) + require.Empty(t, fixtures[i].serviceContext.Package) + } +} + +type remoteBuildFixture struct { + mocks *mocks.MockContext + helper *ContainerHelper + config *ServiceConfig + serviceContext *ServiceContext + env *environment.Environment + target *environment.TargetResource + options *PublishOptions + scheduleCode string + runStatus armcontainerregistry.RunStatus + logError bool + failure string + cancelAt string + cancel context.CancelFunc + localError error + loginCall *mock.Call + mu sync.Mutex + operations []string + pushedImage string +} + +func (f *remoteBuildFixture) record(operation string) error { + f.mu.Lock() + defer f.mu.Unlock() + f.operations = append(f.operations, operation) + if f.cancelAt == operation { + f.cancel() + } + if f.failure == operation { + return f.localError + } + return nil +} + +func (f *remoteBuildFixture) publish(ctx context.Context) (*ServicePublishResult, error) { + progress := async.NewNoopProgress[ServiceProgress]() + defer progress.Done() + return f.helper.Publish(ctx, f.config, f.serviceContext, f.target, f.env, progress, f.options) +} + +func newRemoteBuildFixture(t *testing.T) *remoteBuildFixture { + t.Helper() + m := mocks.NewMockContext(t.Context()) + m.ArmClientOptions.Retry.MaxRetries = -1 + f := &remoteBuildFixture{ + mocks: m, config: createTestServiceConfig("./src/api", ContainerAppTarget, ServiceLanguageTypeScript), + serviceContext: NewServiceContext(), env: environment.NewWithValues("dev", map[string]string{}), + target: environment.NewTargetResource("SUBSCRIPTION_ID", "RESOURCE_GROUP", "app", "Microsoft.App/containerApps"), + options: &PublishOptions{}, runStatus: armcontainerregistry.RunStatusSucceeded, + localError: errors.New("local operation failed"), + } + f.config.Project.Path = t.TempDir() + f.config.Project.Name = "project" + f.config.Name = "app" + f.config.Docker.Registry = osutil.NewExpandableString("contoso.azurecr.io") + f.config.Docker.RemoteBuild = true + require.NoError(t, os.MkdirAll(f.config.Path(), 0700)) + require.NoError(t, os.WriteFile(filepath.Join(f.config.Path(), "Dockerfile"), []byte("FROM scratch"), 0600)) + m.CommandRunner.MockToolInPath("docker", nil) + m.CommandRunner.MockToolInPath("podman", nil) + m.CommandRunner.When(func(args exec.RunArgs, _ string) bool { + return args.Cmd == "docker" || args.Cmd == "podman" + }).RespondFn(func(args exec.RunArgs) (exec.RunResult, error) { + operation := args.Args[0] + if operation == "tag" && args.Args[1] == "IMAGE_ID" { + operation = "package" + } + if err := f.record(operation); err != nil { + return exec.RunResult{}, err + } + switch operation { + case "--version": + version := "Docker version 20.10.17, build 100c701" + if args.Cmd == "podman" { + version = "podman version 4.9.0" + } + return exec.NewRunResult(0, version, ""), nil + case "build": + index := slices.Index(args.Args, "--iidfile") + require.GreaterOrEqual(t, index, 0) + return exec.RunResult{}, os.WriteFile(args.Args[index+1], []byte("IMAGE_ID"), 0600) + case "push": + f.mu.Lock() + f.pushedImage = args.Args[1] + f.mu.Unlock() + case "ps", "package", "tag": + default: + return exec.RunResult{}, errors.New("unexpected container operation: " + operation) + } + return exec.RunResult{}, nil + }) + registry := &mockContainerRegistryService{} + registry.On("FindContainerRegistryResourceGroup", mock.Anything, "SUBSCRIPTION_ID", "contoso"). + Return("REGISTRY_RG", nil) + f.loginCall = registry.On("Login", mock.Anything, mock.Anything, "contoso.azurecr.io"). + Return(nil).Run(func(mock.Arguments) { + _ = f.record("login") + }) + f.helper = NewContainerHelper( + clock.NewMock(), registry, + containerregistry.NewRemoteBuildManager(m.SubscriptionCredentialProvider, m.ArmClientOptions), + m.CommandRunner, docker.NewCli(m.CommandRunner), dotnet.NewCli(m.CommandRunner), m.Console, cloud.AzurePublic(), + ) + m.HttpClient.When(func(*http.Request) bool { return true }).RespondFn(f.respond) + return f +} + +func (f *remoteBuildFixture) respond(request *http.Request) (*http.Response, error) { + const buildLog = "build log\n" + errorResponse := func(code string) (*http.Response, error) { + return mocks.CreateHttpResponseWithBody(request, http.StatusForbidden, map[string]any{ + "error": map[string]string{"code": code, "message": "ACR request refused"}, + }) + } + switch { + case strings.Contains(request.URL.Path, "listBuildSourceUploadUrl"): + return mocks.CreateHttpResponseWithBody(request, http.StatusOK, armcontainerregistry.SourceUploadDefinition{ + UploadURL: new("https://upload.example.com/source.tar.gz"), RelativePath: new("source.tar.gz"), + }) + case strings.HasSuffix(request.URL.Path, "/source.tar.gz"): + _, err := io.Copy(io.Discard, request.Body) + if err != nil { + return nil, err + } + response, err := mocks.CreateEmptyHttpResponse(request, http.StatusCreated) + if err == nil { + response.Header.Set("ETag", `"etag"`) + } + return response, err + case strings.Contains(request.URL.Path, "scheduleRun"): + if err := f.record("schedule"); err != nil { + return nil, err + } + if f.scheduleCode != "" { + return errorResponse(f.scheduleCode) + } + return mocks.CreateHttpResponseWithBody(request, http.StatusOK, armcontainerregistry.Run{ + Properties: &armcontainerregistry.RunProperties{RunID: new("run-id")}, + }) + case strings.Contains(request.URL.Path, "listLogSasUrl"): + if f.logError { + return errorResponse("TasksOperationsNotAllowed") + } + return mocks.CreateHttpResponseWithBody(request, http.StatusOK, armcontainerregistry.RunGetLogResult{ + LogLink: new("https://logs.example.com/run.log"), + }) + case strings.HasSuffix(request.URL.Path, "/run.log"): + response, err := mocks.CreateEmptyHttpResponse(request, http.StatusOK) + if err != nil { + return nil, err + } + response.Header.Set("Content-Length", strconv.Itoa(len(buildLog))) + response.Header.Set("x-ms-meta-complete", "true") + if request.Method == http.MethodGet { + response.StatusCode = http.StatusPartialContent + response.Body = io.NopCloser(strings.NewReader(buildLog)) + } + return response, nil + case strings.Contains(request.URL.Path, "/runs/run-id"): + return mocks.CreateHttpResponseWithBody(request, http.StatusOK, armcontainerregistry.Run{ + Properties: &armcontainerregistry.RunProperties{Status: new(f.runStatus)}, + }) + default: + return nil, errors.New("unexpected remote build request: " + request.URL.Path) + } +} diff --git a/cli/azd/pkg/project/container_helper_test.go b/cli/azd/pkg/project/container_helper_test.go index 5a5b51b8344..34f57dfa87c 100644 --- a/cli/azd/pkg/project/container_helper_test.go +++ b/cli/azd/pkg/project/container_helper_test.go @@ -1853,102 +1853,6 @@ func Test_ContainerHelper_Publish(t *testing.T) { } } -func Test_ContainerHelper_Publish_RemoteBuildLocalFallback(t *testing.T) { - mockContext := mocks.NewMockContext(t.Context()) - mockResults := setupDockerMocks(mockContext) - env := environment.NewWithValues("dev", map[string]string{}) - dockerCli := docker.NewCli(mockContext.CommandRunner) - dotnetCli := dotnet.NewCli(mockContext.CommandRunner) - - // Mock Docker availability checks for local fallback - mockContext.CommandRunner.MockToolInPath("docker", nil) - - mockContext.CommandRunner.When(func(args exec.RunArgs, command string) bool { - return strings.Contains(command, "docker --version") - }).RespondFn(func(args exec.RunArgs) (exec.RunResult, error) { - return exec.RunResult{ - Stdout: "Docker version 20.10.17, build 100c701", - ExitCode: 0, - }, nil - }) - - mockContext.CommandRunner.When(func(args exec.RunArgs, command string) bool { - return strings.Contains(command, "docker ps") - }).RespondFn(func(args exec.RunArgs) (exec.RunResult, error) { - return exec.RunResult{ - Stdout: "CONTAINER ID IMAGE COMMAND CREATED STATUS PORTS NAMES", - ExitCode: 0, - }, nil - }) - - mockContainerRegistryService := &mockContainerRegistryService{} - setupContainerRegistryMocks(mockContext, &mockContainerRegistryService.Mock) - - containerHelper := NewContainerHelper( - clock.NewMock(), - mockContainerRegistryService, - nil, - mockContext.CommandRunner, - dockerCli, - dotnetCli, - mockContext.Console, - cloud.AzurePublic(), - ) - - serviceConfig := createTestServiceConfig("./src/api", ContainerAppTarget, ServiceLanguageTypeScript) - serviceConfig.Docker.Registry = osutil.NewExpandableString("contoso.azurecr.io") - serviceConfig.Docker.RemoteBuild = true - serviceConfig.Docker.Platform = "linux/arm64" - - dockerArtifact := &Artifact{ - Kind: ArtifactKindContainer, - Location: "my-project/my-service:azd-deploy-0", - LocationKind: LocationKindLocal, - Metadata: map[string]string{ - "imageHash": "IMAGE_ID", - "sourceImage": "", - "targetImage": "my-project/my-service:azd-deploy-0", - }, - } - - serviceContext := &ServiceContext{ - Package: ArtifactCollection{dockerArtifact}, - } - - targetResource := environment.NewTargetResource( - "SUBSCRIPTION_ID", - "RESOURCE_GROUP", - "CONTAINER_APP", - "Microsoft.App/containerApps", - ) - - publishResult, err := logProgress( - t, func(progress *async.Progress[ServiceProgress]) (*ServicePublishResult, error) { - return containerHelper.Publish( - *mockContext.Context, serviceConfig, serviceContext, targetResource, env, progress, &PublishOptions{}) - }, - ) - - require.NoError(t, err) - require.Len(t, publishResult.Artifacts, 1) - expectedImage := "contoso.azurecr.io/my-project/my-service:azd-deploy-0" - require.Equal(t, expectedImage, - publishResult.Artifacts[0].Metadata["remoteImage"]) - - _, dockerPushCalled := mockResults["docker-push"] - require.True(t, dockerPushCalled) - - warningFound := false - for _, line := range mockContext.Console.Output() { - if strings.Contains(line, "Remote build failed:") && - strings.Contains(line, "Falling back to local Docker build.") { - warningFound = true - break - } - } - require.True(t, warningFound) -} - func Test_ContainerHelper_DockerfileBuilder(t *testing.T) { ch := &ContainerHelper{} builder := ch.DockerfileBuilder() diff --git a/cli/azd/pkg/tools/docker/docker.go b/cli/azd/pkg/tools/docker/docker.go index 93c9def1908..565a7881bc3 100644 --- a/cli/azd/pkg/tools/docker/docker.go +++ b/cli/azd/pkg/tools/docker/docker.go @@ -13,6 +13,7 @@ import ( "regexp" "strconv" "strings" + "sync" "github.com/azure/azure-dev/cli/azd/pkg/exec" "github.com/azure/azure-dev/cli/azd/pkg/tools" @@ -31,8 +32,10 @@ func NewCli(commandRunner exec.CommandRunner) *Cli { } type Cli struct { - commandRunner exec.CommandRunner - containerEngine string // "docker" or "podman", detected during CheckInstalled + commandRunner exec.CommandRunner + + engineMu sync.Mutex // Protects containerEngine and runtime selection, not subprocess execution. + containerEngine string } // ContainerEngine returns the detected container engine name ("docker" or "podman"). @@ -42,16 +45,18 @@ type Cli struct { // engine name (e.g., dotnet publish with -p:ContainerEngine) get the correct // value even when docker.Cli is not in RequiredExternalTools. func (d *Cli) ContainerEngine() string { + d.engineMu.Lock() + defer d.engineMu.Unlock() + if d.containerEngine == "" { - d.detectContainerEngine() + d.detectContainerEngineLocked() } return d.containerEngine } -// detectContainerEngine performs a lightweight detection of the container engine -// by checking AZD_CONTAINER_RUNTIME and PATH. Unlike CheckInstalled(), it does -// not validate versions or daemon readiness. -func (d *Cli) detectContainerEngine() { +// detectContainerEngineLocked checks AZD_CONTAINER_RUNTIME and PATH, not readiness. +// The caller must hold engineMu. +func (d *Cli) detectContainerEngineLocked() { if runtime := os.Getenv("AZD_CONTAINER_RUNTIME"); runtime == "docker" || runtime == "podman" { d.containerEngine = runtime return @@ -72,6 +77,9 @@ func (d *Cli) detectContainerEngine() { // CheckInstalled() should be called first to detect and set the container engine. // If not set, defaults to "docker" for backward compatibility. func (d *Cli) getContainerEngine() string { + d.engineMu.Lock() + defer d.engineMu.Unlock() + if d.containerEngine == "" { // Default to "docker" for backward compatibility with existing code // that may not call CheckInstalled() first @@ -81,8 +89,9 @@ func (d *Cli) getContainerEngine() string { } func (d *Cli) Login(ctx context.Context, loginServer string, username string, password string) error { + engineName := d.getContainerEngine() runArgs := exec.NewRunArgs( - d.getContainerEngine(), "login", + engineName, "login", "--username", username, "--password-stdin", loginServer, @@ -90,7 +99,7 @@ func (d *Cli) Login(ctx context.Context, loginServer string, username string, pa _, err := d.commandRunner.Run(ctx, runArgs) if err != nil { - return fmt.Errorf("failed logging into %s: %w", d.Name(), err) + return fmt.Errorf("failed logging into %s: %w", containerEngineDisplayName(engineName), err) } return nil @@ -353,13 +362,26 @@ func isSupportedPodmanVersion(cliOutput string) (bool, error) { return version.GTE(minVersion), nil } func (d *Cli) CheckInstalled(ctx context.Context) error { + engineName, err := d.selectContainerEngine() + if err != nil { + return err + } + + return d.validateContainerEngine(ctx, engineName) +} + +// selectContainerEngine refreshes the selection and returns a snapshot for readiness checks. +func (d *Cli) selectContainerEngine() (string, error) { + d.engineMu.Lock() + defer d.engineMu.Unlock() + // Check for environment variable override first containerRuntime := os.Getenv("AZD_CONTAINER_RUNTIME") if containerRuntime != "" { // Validate the specified runtime if containerRuntime != "docker" && containerRuntime != "podman" { - return fmt.Errorf( + return "", fmt.Errorf( "unsupported container runtime '%s' specified in AZD_CONTAINER_RUNTIME. "+ "Supported values: docker, podman", containerRuntime) @@ -373,23 +395,18 @@ func (d *Cli) CheckInstalled(ctx context.Context) error { d.containerEngine = "podman" } else { // Neither tool is installed - return fmt.Errorf( + return "", fmt.Errorf( "neither docker nor podman is installed. " + "Please install Docker: https://aka.ms/azure-dev/docker-install " + "or Podman: https://aka.ms/azure-dev/podman-install") } } - // Now validate the selected engine (version check and daemon/service running) - return d.validateContainerEngine(ctx) + return d.containerEngine, nil } -// validateContainerEngine validates that the selected container engine (docker or podman) meets version -// and readiness requirements. -// The engine must have been selected first via CheckInstalled (stored in d.containerEngine). -func (d *Cli) validateContainerEngine(ctx context.Context) error { - engineName := d.getContainerEngine() - +// validateContainerEngine checks version and readiness using an engine snapshot without holding engineMu. +func (d *Cli) validateContainerEngine(ctx context.Context, engineName string) error { // Check version versionOutput, err := tools.ExecuteCommand(ctx, d.commandRunner, engineName, "--version") if err != nil { @@ -413,7 +430,7 @@ func (d *Cli) validateContainerEngine(ctx context.Context) error { return err } if !supported { - return &tools.ErrSemver{ToolName: d.Name(), VersionInfo: versionInfo} + return &tools.ErrSemver{ToolName: containerEngineDisplayName(engineName), VersionInfo: versionInfo} } // Check if daemon/service is running @@ -425,14 +442,18 @@ func (d *Cli) validateContainerEngine(ctx context.Context) error { } func (d *Cli) InstallUrl() string { - if d.containerEngine == "podman" { + if d.getContainerEngine() == "podman" { return "https://aka.ms/azure-dev/podman-install" } return "https://aka.ms/azure-dev/docker-install" } func (d *Cli) Name() string { - if d.containerEngine == "podman" { + return containerEngineDisplayName(d.getContainerEngine()) +} + +func containerEngineDisplayName(engineName string) string { + if engineName == "podman" { return "Podman" } return "Docker" @@ -440,12 +461,14 @@ func (d *Cli) Name() string { // IsContainerdEnabled checks if Docker is using containerd as the image store func (d *Cli) IsContainerdEnabled(ctx context.Context) (bool, error) { + engineName := d.getContainerEngine() // Containerd image store is only applicable to Docker, not Podman - if d.getContainerEngine() == "podman" { + if engineName == "podman" { return false, nil } - result, err := d.executeCommand(ctx, "", "system", "info", "--format", "{{.DriverStatus}}") + result, err := d.commandRunner.Run(ctx, + exec.NewRunArgs(engineName, "system", "info", "--format", "{{.DriverStatus}}")) if err != nil { return false, fmt.Errorf("checking docker driver status: %w", err) } diff --git a/cli/azd/pkg/tools/docker/docker_concurrency_test.go b/cli/azd/pkg/tools/docker/docker_concurrency_test.go new file mode 100644 index 00000000000..05c8478938f --- /dev/null +++ b/cli/azd/pkg/tools/docker/docker_concurrency_test.go @@ -0,0 +1,196 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +package docker + +import ( + "context" + "errors" + "sync" + "sync/atomic" + "testing" + "time" + + "github.com/azure/azure-dev/cli/azd/pkg/exec" + "github.com/azure/azure-dev/cli/azd/test/mocks/mockexec" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestContainerEngineConcurrent(t *testing.T) { + tests := []struct { + name string + override string + dockerErr error + engine string + engineName string + version string + }{ + { + name: "docker first", engine: "docker", engineName: "Docker", + version: "Docker version 20.10.17, build 100c701", + }, + { + name: "podman discovery", dockerErr: errors.New("docker not found"), + engine: "podman", engineName: "Podman", version: "podman version 4.3.1", + }, + { + name: "podman override", override: "podman", + engine: "podman", engineName: "Podman", version: "podman version 4.3.1", + }, + { + name: "docker override", override: "docker", + engine: "docker", engineName: "Docker", version: "Docker version 20.10.17, build 100c701", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Setenv("AZD_CONTAINER_RUNTIME", tt.override) + runner := mockexec.NewMockCommandRunner() + runner.MockToolInPath("docker", tt.dockerErr) + runner.MockToolInPath("podman", nil) + runner.When(func(args exec.RunArgs, command string) bool { + return command == tt.engine+" --version" + }).Respond(exec.RunResult{Stdout: tt.version}) + runner.When(func(args exec.RunArgs, command string) bool { + return command == tt.engine+" ps" || command == tt.engine+" pull image" + }).Respond(exec.RunResult{}) + + cli := NewCli(runner) + require.Equal(t, "Docker", cli.Name()) + require.Equal(t, "https://aka.ms/azure-dev/docker-install", cli.InstallUrl()) + + // Register all mocks before starting concurrent readers. + start := make(chan struct{}) + var wg sync.WaitGroup + for range 8 { + wg.Go(func() { + <-start + for range 10 { + assert.Equal(t, tt.engine, cli.ContainerEngine()) + assert.NoError(t, cli.CheckInstalled(t.Context())) + assert.Equal(t, tt.engineName, cli.Name()) + assert.Equal(t, "https://aka.ms/azure-dev/"+tt.engine+"-install", cli.InstallUrl()) + assert.NoError(t, cli.Pull(t.Context(), "image")) + } + }) + } + close(start) + wg.Wait() + }) + } +} + +func TestCheckInstalledSnapshotAndRetry(t *testing.T) { + tests := []struct { + name string + blockedCommand string + firstVersion string + firstErr error + wantError string + }{ + { + name: "ready", blockedCommand: "--version", firstVersion: "podman version 4.3.1", + }, + { + name: "version failure", blockedCommand: "--version", firstErr: errors.New("version unavailable"), + wantError: "checking podman version: version unavailable", + }, + { + name: "unsupported version", blockedCommand: "--version", firstVersion: "podman version 2.9.0", + wantError: "need at least version 3.0.0 or later of Podman installed", + }, + { + name: "daemon failure", blockedCommand: "ps", firstErr: errors.New("daemon unavailable"), + wantError: "the podman service is not running, please start it: daemon unavailable", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Setenv("AZD_CONTAINER_RUNTIME", "podman") + ctx, cancel := context.WithTimeout(t.Context(), 5*time.Second) + defer cancel() + + started := make(chan struct{}) + release := make(chan struct{}) + defer close(release) + var calls atomic.Int32 + runner := mockexec.NewMockCommandRunner() + cli := NewCli(runner) + runner.When(func(args exec.RunArgs, command string) bool { + return command == "docker --version" + }).Respond(exec.RunResult{Stdout: "Docker version 20.10.17, build 100c701"}) + runner.When(func(args exec.RunArgs, command string) bool { + return command == "podman --version" + }).Respond(exec.RunResult{Stdout: "podman version 4.3.1"}) + runner.When(func(args exec.RunArgs, command string) bool { + return command == "docker ps" || command == "podman ps" || command == "docker pull image" + }).Respond(exec.RunResult{}) + runner.When(func(args exec.RunArgs, command string) bool { + return command == "podman "+tt.blockedCommand + }).RespondFn(func(args exec.RunArgs) (exec.RunResult, error) { + if calls.Add(1) == 1 { + close(started) + select { + case <-release: + return exec.RunResult{Stdout: tt.firstVersion}, tt.firstErr + case <-ctx.Done(): + return exec.RunResult{}, ctx.Err() + } + } + return exec.RunResult{Stdout: "podman version 4.3.1"}, nil + }) + runner.When(func(args exec.RunArgs, command string) bool { + return args.Cmd == "docker" && len(args.Args) > 0 && args.Args[0] == "build" + }).RespondFn(func(args exec.RunArgs) (exec.RunResult, error) { + if !cli.engineMu.TryLock() { + return exec.RunResult{}, errors.New("engine lock held during build") + } + cli.engineMu.Unlock() + return exec.RunResult{}, errors.New("build stopped") + }) + + done := make(chan error, 1) + go func() { + done <- cli.CheckInstalled(ctx) + }() + select { + case <-started: + case <-ctx.Done(): + t.Fatal("readiness check did not reach the subprocess") + } + + // Reselect while the first readiness subprocess is blocked. Neither + // another check nor container operations may wait for that subprocess. + t.Setenv("AZD_CONTAINER_RUNTIME", "docker") + require.NoError(t, cli.CheckInstalled(ctx)) + require.Equal(t, "docker", cli.ContainerEngine()) + require.Equal(t, "Docker", cli.Name()) + require.Equal(t, "https://aka.ms/azure-dev/docker-install", cli.InstallUrl()) + require.NoError(t, cli.Pull(ctx, "image")) + _, err := cli.Build(ctx, ".", "Dockerfile", "", "", ".", "", nil, nil, nil, "", nil) + require.EqualError(t, err, "building image: build stopped") + require.NoError(t, ctx.Err(), "runtime operations waited for the blocked readiness subprocess") + + select { + case release <- struct{}{}: + case <-ctx.Done(): + t.Fatal("readiness check did not resume") + } + err = <-done + if tt.wantError == "" { + require.NoError(t, err) + } else { + require.ErrorContains(t, err, tt.wantError) + } + require.Equal(t, "docker", cli.ContainerEngine()) + + t.Setenv("AZD_CONTAINER_RUNTIME", "podman") + require.NoError(t, cli.CheckInstalled(ctx)) + require.Equal(t, int32(2), calls.Load(), "readiness checks must run again after success or failure") + require.Equal(t, "podman", cli.ContainerEngine()) + }) + } +} diff --git a/docs/reference/azure-yaml-schema.md b/docs/reference/azure-yaml-schema.md index 235ca7eb04f..5bcdce918d1 100644 --- a/docs/reference/azure-yaml-schema.md +++ b/docs/reference/azure-yaml-schema.md @@ -71,9 +71,13 @@ services: | `tag` | string | Tag applied to a built container image | | `buildArgs` | list | Arguments passed to the container build | | `network` | string | Networking mode for Dockerfile `RUN` instructions | -| `remoteBuild` | boolean | Build and push with Azure Container Registry remote build instead of building locally | +| `remoteBuild` | boolean | Prefer building and pushing with Azure Container Registry; fall back locally only when ACR refuses scheduling with `TasksOperationsNotAllowed` | | `imagePassthrough` | boolean | Reuse an existing remote service `image` without building or publishing it; `azd deploy --from-package` can override the image for one deployment | +If ACR refuses scheduling with `TasksOperationsNotAllowed`, azd warns and falls back automatically, including in non-interactive runs. Docker or Podman must be installed and running. Fallback publishes a supplied local package or builds and pushes from source. + +Other errors, including build failures, cancellation, and failures reading remote logs or status, do not trigger fallback. If fallback also fails, azd preserves both errors. Set `docker.remoteBuild: false` to build locally. + `docker.imagePassthrough` declares that azd does not own the container image lifecycle. It requires the service-level `image` property to contain a fully qualified remote image and cannot be combined with `docker.remoteBuild`. During package, publish, and deploy operations, azd uses the configured image as the existing remote image without building, pulling, tagging, copying, or publishing it: diff --git a/schemas/alpha/azure.yaml.json b/schemas/alpha/azure.yaml.json index 35c8de50023..e6f77ff8600 100644 --- a/schemas/alpha/azure.yaml.json +++ b/schemas/alpha/azure.yaml.json @@ -1333,7 +1333,7 @@ "remoteBuild": { "type": "boolean", "title": "Optional. Whether to build the image remotely", - "description": "If set to true, the image will be built remotely using the Azure Container Registry remote build feature. If set to false, the image will be built locally using Docker." + "description": "If set to true, azd builds the image remotely using Azure Container Registry. Automatic local fallback uses Docker or Podman only when scheduling returns TasksOperationsNotAllowed. If set to false, azd builds locally." }, "imagePassthrough": { "type": "boolean", diff --git a/schemas/v1.0/azure.yaml.json b/schemas/v1.0/azure.yaml.json index 826969d3ed3..aa3fd3c8c45 100644 --- a/schemas/v1.0/azure.yaml.json +++ b/schemas/v1.0/azure.yaml.json @@ -1293,7 +1293,7 @@ "remoteBuild": { "type": "boolean", "title": "Optional. Whether to build the image remotely", - "description": "If set to true, the image will be built remotely using the Azure Container Registry remote build feature. If the remote build fails, azd automatically falls back to building locally using Docker or Podman if available. If set to false, the image will be built locally." + "description": "If set to true, azd builds the image remotely using Azure Container Registry. Automatic local fallback uses Docker or Podman only when scheduling returns TasksOperationsNotAllowed. If set to false, azd builds locally." }, "imagePassthrough": { "type": "boolean",