From 5b9b1eb5af77f842e0b5cd321da14296f46da914 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Ros?= <1165805+sebastienros@users.noreply.github.com> Date: Tue, 11 Aug 2026 11:36:31 -0700 Subject: [PATCH 1/9] Fix AKS Helm destroy ordering Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- ...bernetesEnvironmentResource.AksPipeline.cs | 24 +++++ .../AzureKubernetesEnvironmentResource.cs | 36 ++++++- .../CertManagerExtensions.cs | 3 +- .../Deployment/HelmDeploymentEngine.cs | 3 + .../KubernetesHelmChartExtensions.cs | 3 +- .../AksWithHelmChartDeploymentTests.cs | 20 ++-- .../AzureKubernetesInfrastructureTests.cs | 99 +++++++++++++++++++ 7 files changed, 174 insertions(+), 14 deletions(-) diff --git a/src/Aspire.Hosting.Azure.Kubernetes/AzureKubernetesEnvironmentResource.AksPipeline.cs b/src/Aspire.Hosting.Azure.Kubernetes/AzureKubernetesEnvironmentResource.AksPipeline.cs index 9e5b3f38302..d0ae7487297 100644 --- a/src/Aspire.Hosting.Azure.Kubernetes/AzureKubernetesEnvironmentResource.AksPipeline.cs +++ b/src/Aspire.Hosting.Azure.Kubernetes/AzureKubernetesEnvironmentResource.AksPipeline.cs @@ -2,6 +2,7 @@ // The .NET Foundation licenses this file to you under the MIT license. #pragma warning disable ASPIREPIPELINES001 // Pipeline step types used for push/deploy dependency wiring +#pragma warning disable ASPIREPIPELINES002 // Deployment state is used to skip destroy credentials when no Helm deployment exists #pragma warning disable ASPIREAZURE001 // AzureEnvironmentResource.ProvisionInfrastructureStepName for pipeline ordering #pragma warning disable ASPIREFILESYSTEM001 // IFileSystemService/TempDirectory are experimental @@ -288,6 +289,29 @@ await getCredsTask.FailAsync( } } + private async Task GetAksCredentialsForDestroyAsync(PipelineStepContext context) + { + var deploymentStateManager = context.Services.GetRequiredService(); + var stateSection = await deploymentStateManager + .AcquireSectionAsync("Azure", context.CancellationToken) + .ConfigureAwait(false); + + // Azure state remains until the entire destroy pipeline succeeds. Use it rather than the + // main Helm release state, which may already be gone when retrying a partial teardown that + // still needs to uninstall an external chart or delete another cluster-scoped resource. + var resourceGroupName = stateSection.Data["ResourceGroup"]?.ToString(); + var subscriptionId = stateSection.Data["SubscriptionId"]?.ToString(); + if (string.IsNullOrEmpty(resourceGroupName) || string.IsNullOrEmpty(subscriptionId)) + { + context.Logger.LogInformation( + "No Azure deployment state found for AKS environment '{EnvironmentName}'. Skipping credential acquisition.", + Name); + return; + } + + await GetAksCredentialsAsync(context).ConfigureAwait(false); + } + /// /// Applies the AGC ApplicationLoadBalancer custom resource for the supplied /// into the cluster. Polls the diff --git a/src/Aspire.Hosting.Azure.Kubernetes/AzureKubernetesEnvironmentResource.cs b/src/Aspire.Hosting.Azure.Kubernetes/AzureKubernetesEnvironmentResource.cs index 6b0e3de146e..f33ba9f046d 100644 --- a/src/Aspire.Hosting.Azure.Kubernetes/AzureKubernetesEnvironmentResource.cs +++ b/src/Aspire.Hosting.Azure.Kubernetes/AzureKubernetesEnvironmentResource.cs @@ -36,6 +36,8 @@ public AzureKubernetesEnvironmentResource( // can observe the annotations. // - aks-get-credentials-{name}: fetches AKS credentials into an isolated // kubeconfig file after AKS is provisioned, before Helm prepare runs. + // - aks-get-credentials-for-destroy-{name}: fetches credentials from saved + // deployment state before cluster-scoped destroy steps run. Annotations.Add(new PipelineStepAnnotation(_ => { var k8sEnv = KubernetesEnvironment; @@ -67,7 +69,39 @@ public AzureKubernetesEnvironmentResource( RequiredBySteps = [$"prepare-{k8sEnv.Name}"] }; - return Task.FromResult>([prepareStep, getCredentialsStep]); + var getDestroyCredentialsStep = new PipelineStep + { + Name = $"aks-get-credentials-for-destroy-{Name}", + Description = $"Fetches AKS credentials for destroying {Name}", + Action = ctx => GetAksCredentialsForDestroyAsync(ctx), + // Keep this separate from the deploy credential step: depending on Azure + // provisioning here would pull provisioning into the destroy graph. + DependsOnSteps = [WellKnownPipelineSteps.DestroyPrereq] + }; + + return Task.FromResult>([prepareStep, getCredentialsStep, getDestroyCredentialsStep]); + })); + + Annotations.Add(new PipelineConfigurationAnnotation(context => + { + var k8sEnv = KubernetesEnvironment; + var getDestroyCredentialsStep = context.GetSteps(this) + .Single(step => step.Name == $"aks-get-credentials-for-destroy-{Name}"); + var kubernetesDestroySteps = context + .GetSteps(HelmDeploymentEngine.GetKubernetesDestroyTag(k8sEnv.Name)) + .ToList(); + + var azureEnvironment = context.Model.Resources.OfType().Single(); + var destroyAzureStep = context.GetSteps(azureEnvironment) + .Single(step => step.Name == $"destroy-azure-{azureEnvironment.Name}"); + + foreach (var kubernetesDestroyStep in kubernetesDestroySteps) + { + kubernetesDestroyStep.DependsOn(getDestroyCredentialsStep); + destroyAzureStep.DependsOn(kubernetesDestroyStep); + } + + return Task.CompletedTask; })); } diff --git a/src/Aspire.Hosting.Kubernetes/CertManagerExtensions.cs b/src/Aspire.Hosting.Kubernetes/CertManagerExtensions.cs index 52a30bff98d..579089eb1ef 100644 --- a/src/Aspire.Hosting.Kubernetes/CertManagerExtensions.cs +++ b/src/Aspire.Hosting.Kubernetes/CertManagerExtensions.cs @@ -398,7 +398,8 @@ private static Task> BuildIssuerDeleteSteps( Name = $"cm-issuer-delete-{captured.Name}", Description = $"Deletes cert-manager ClusterIssuer '{captured.Name}'", Action = ctx => DeleteClusterIssuerAsync(ctx, certManager, captured), - DependsOnSteps = [WellKnownPipelineSteps.DestroyPrereq] + DependsOnSteps = [WellKnownPipelineSteps.DestroyPrereq], + Tags = [HelmDeploymentEngine.GetKubernetesDestroyTag(certManager.Parent.Name)] }; // Run before the cert-manager helm chart is uninstalled. Once the chart goes, diff --git a/src/Aspire.Hosting.Kubernetes/Deployment/HelmDeploymentEngine.cs b/src/Aspire.Hosting.Kubernetes/Deployment/HelmDeploymentEngine.cs index c0e602dcf90..a73b0d1da11 100644 --- a/src/Aspire.Hosting.Kubernetes/Deployment/HelmDeploymentEngine.cs +++ b/src/Aspire.Hosting.Kubernetes/Deployment/HelmDeploymentEngine.cs @@ -30,6 +30,8 @@ internal static partial class HelmDeploymentEngine private const string HelmUninstallTag = "helm-uninstall"; internal const string PrintSummaryTag = "print-summary"; + internal static string GetKubernetesDestroyTag(string environmentName) => $"kubernetes-destroy-{environmentName}"; + /// /// Gets the environment-specific values file name, mirroring Docker Compose's .env.{envName} pattern. /// @@ -183,6 +185,7 @@ internal static Task> CreateStepsAsync( { Name = $"destroy-helm-{environment.Name}", Description = $"Confirms and destroys the Helm deployment for {environment.Name}.", + Tags = [GetKubernetesDestroyTag(environment.Name)], Action = async ctx => { // Check deployment state to verify this environment was actually deployed diff --git a/src/Aspire.Hosting.Kubernetes/KubernetesHelmChartExtensions.cs b/src/Aspire.Hosting.Kubernetes/KubernetesHelmChartExtensions.cs index 7607f2ebcd6..2dbeeb28b81 100644 --- a/src/Aspire.Hosting.Kubernetes/KubernetesHelmChartExtensions.cs +++ b/src/Aspire.Hosting.Kubernetes/KubernetesHelmChartExtensions.cs @@ -112,7 +112,8 @@ public static IResourceBuilder AddHelmChart( Name = $"helm-uninstall-{name}", Description = $"Uninstalls Helm chart '{name}' from namespace '{@namespace}'", Action = ctx => UninstallHelmChartAsync(ctx, environment, resource, releaseName, @namespace), - DependsOnSteps = [WellKnownPipelineSteps.DestroyPrereq] + DependsOnSteps = [WellKnownPipelineSteps.DestroyPrereq], + Tags = [HelmDeploymentEngine.GetKubernetesDestroyTag(environment.Name)] }; // The uninstall path shells out to `helm uninstall`, so it must observe the same // Helm CLI / version preflight as the deploy path. Without this dep, a missing or diff --git a/tests/Aspire.Deployment.EndToEnd.Tests/AksWithHelmChartDeploymentTests.cs b/tests/Aspire.Deployment.EndToEnd.Tests/AksWithHelmChartDeploymentTests.cs index d92f4fced95..6d397173d5d 100644 --- a/tests/Aspire.Deployment.EndToEnd.Tests/AksWithHelmChartDeploymentTests.cs +++ b/tests/Aspire.Deployment.EndToEnd.Tests/AksWithHelmChartDeploymentTests.cs @@ -215,20 +215,18 @@ await auto.TypeAsync( await auto.EnterAsync(); await auto.WaitForSuccessPromptAsync(counter, TimeSpan.FromSeconds(10)); - // Step 15: Destroy and verify the external Helm chart was uninstalled too. - output.WriteLine("Step 15: Destroying deployment..."); - await auto.AspireDestroyAsync(counter); - - // Step 16: Verify the podinfo release is gone (this is the WithDestroy() contract). - output.WriteLine("Step 16: Verifying podinfo Helm release was uninstalled by aspire destroy..."); - await auto.TypeAsync( - "RELEASES=$(helm list -n podinfo -q 2>/dev/null); " + - "if [ -z \"$RELEASES\" ]; then echo 'VERIFY_OK: podinfo release was uninstalled'; " + - "else echo \"FAIL: podinfo release still exists: $RELEASES\"; exit 1; fi"); + // Step 15: Replace the ambient kubeconfig so destroy proves it acquires AKS + // credentials itself instead of reusing the context configured in Step 10. + output.WriteLine("Step 15: Clearing ambient Kubernetes credentials..."); + await auto.TypeAsync("export KUBECONFIG=$(mktemp)"); await auto.EnterAsync(); - await auto.WaitUntilTextAsync("VERIFY_OK", timeout: TimeSpan.FromMinutes(2)); await auto.WaitForSuccessPromptAsync(counter, TimeSpan.FromSeconds(10)); + // Step 16: Destroy the application and opted-in external Helm chart before + // deleting the AKS resource group. + output.WriteLine("Step 16: Destroying deployment..."); + await auto.AspireDestroyAsync(counter); + // Step 17: Exit terminal await auto.TypeAsync("exit"); await auto.EnterAsync(); diff --git a/tests/Aspire.Hosting.Azure.Kubernetes.Tests/AzureKubernetesInfrastructureTests.cs b/tests/Aspire.Hosting.Azure.Kubernetes.Tests/AzureKubernetesInfrastructureTests.cs index d88065be275..d75f901407a 100644 --- a/tests/Aspire.Hosting.Azure.Kubernetes.Tests/AzureKubernetesInfrastructureTests.cs +++ b/tests/Aspire.Hosting.Azure.Kubernetes.Tests/AzureKubernetesInfrastructureTests.cs @@ -191,4 +191,103 @@ public async Task KubernetesPipelineStepsFlowThroughAksEnvironment() Assert.Contains(logs, msg => msg.Contains("aks-get-credentials-aks")); Assert.DoesNotContain(logs, msg => msg.Contains("aks-k8s")); } + + [Fact] + public async Task DestroyPipelineFetchesCredentialsBeforeClusterCleanupAndDeletesAzureLast() + { + using var workspace = TemporaryWorkspace.Create(output); + using var builder = TestDistributedApplicationBuilder.Create( + DistributedApplicationOperation.Publish, + workspace.Path, + step: WellKnownPipelineSteps.Diagnostics); + + var reporter = new TestPipelineActivityReporter(output); + builder.Services.AddSingleton(); + builder.Services.AddSingleton(reporter); + + var aks = builder.AddAzureKubernetesEnvironment("aks"); + aks.AddHelmChart("podinfo", "oci://ghcr.io/stefanprodan/charts/podinfo", "6.7.1") + .WithDestroy(); + aks.AddCertManager("cert-manager") + .AddIssuer("letsencrypt"); + builder.AddContainer("api", "myimage") + .WithHttpEndpoint(targetPort: 8080); + + await using var app = builder.Build(); + await app.RunAsync(); + + var diagnosticLines = reporter.LoggedMessages + .Where(s => s.StepTitle == "diagnostics") + .Select(s => s.Message) + .SelectMany(message => message.Split('\n')) + .Select(line => line.Trim()) + .ToList(); + + Assert.Equal( + "Direct dependencies: destroy-prereq", + GetDirectDependencies(diagnosticLines, "aks-get-credentials-for-destroy-aks")); + Assert.Equal( + "Direct dependencies: aks-get-credentials-for-destroy-aks, destroy-prereq", + GetDirectDependencies(diagnosticLines, "destroy-helm-aks")); + Assert.Equal( + "Direct dependencies: aks-get-credentials-for-destroy-aks, check-helm-prereqs-aks, cm-issuer-delete-letsencrypt, destroy-prereq", + GetDirectDependencies(diagnosticLines, "helm-uninstall-cert-manager-chart")); + Assert.Equal( + "Direct dependencies: aks-get-credentials-for-destroy-aks, check-helm-prereqs-aks, destroy-prereq", + GetDirectDependencies(diagnosticLines, "helm-uninstall-podinfo")); + Assert.Equal( + "Direct dependencies: aks-get-credentials-for-destroy-aks, destroy-prereq", + GetDirectDependencies(diagnosticLines, "cm-issuer-delete-letsencrypt")); + Assert.Equal( + "Direct dependencies: cm-issuer-delete-letsencrypt, destroy-helm-aks, destroy-prereq, helm-uninstall-cert-manager-chart, helm-uninstall-podinfo", + GetDirectDependencies(diagnosticLines, "destroy-azure-azure-environment")); + } + + [Fact] + public async Task DestroyPipelineUsesMatchingCredentialsForEachAksEnvironment() + { + using var workspace = TemporaryWorkspace.Create(output); + using var builder = TestDistributedApplicationBuilder.Create( + DistributedApplicationOperation.Publish, + workspace.Path, + step: WellKnownPipelineSteps.Diagnostics); + + var reporter = new TestPipelineActivityReporter(output); + builder.Services.AddSingleton(); + builder.Services.AddSingleton(reporter); + + var east = builder.AddAzureKubernetesEnvironment("east"); + var west = builder.AddAzureKubernetesEnvironment("west"); + builder.AddContainer("east-api", "myimage") + .WithComputeEnvironment(east); + builder.AddContainer("west-api", "myimage") + .WithComputeEnvironment(west); + + await using var app = builder.Build(); + await app.RunAsync(); + + var diagnosticLines = reporter.LoggedMessages + .Where(s => s.StepTitle == "diagnostics") + .Select(s => s.Message) + .SelectMany(message => message.Split('\n')) + .Select(line => line.Trim()) + .ToList(); + + Assert.Equal( + "Direct dependencies: aks-get-credentials-for-destroy-east, destroy-prereq", + GetDirectDependencies(diagnosticLines, "destroy-helm-east")); + Assert.Equal( + "Direct dependencies: aks-get-credentials-for-destroy-west, destroy-prereq", + GetDirectDependencies(diagnosticLines, "destroy-helm-west")); + Assert.Equal( + "Direct dependencies: destroy-helm-east, destroy-helm-west, destroy-prereq", + GetDirectDependencies(diagnosticLines, "destroy-azure-azure-environment")); + } + + private static string GetDirectDependencies(List diagnosticLines, string stepName) + { + var targetLine = diagnosticLines.IndexOf($"If targeting '{stepName}':"); + Assert.InRange(targetLine, 0, diagnosticLines.Count - 2); + return diagnosticLines[targetLine + 1]; + } } From cbddfdd6a053d805ef12eb943edd4c8bc4a62a74 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Ros?= <1165805+sebastienros@users.noreply.github.com> Date: Tue, 11 Aug 2026 15:04:16 -0700 Subject: [PATCH 2/9] Use persisted Azure target for AKS destroy Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: a5fbe5fe-6726-431d-af32-692b54aa1c2b --- ...bernetesEnvironmentResource.AksPipeline.cs | 46 +++++++++++---- ...pire.Hosting.Azure.Kubernetes.Tests.csproj | 1 + .../AzureKubernetesInfrastructureTests.cs | 57 +++++++++++++++++++ 3 files changed, 92 insertions(+), 12 deletions(-) diff --git a/src/Aspire.Hosting.Azure.Kubernetes/AzureKubernetesEnvironmentResource.AksPipeline.cs b/src/Aspire.Hosting.Azure.Kubernetes/AzureKubernetesEnvironmentResource.AksPipeline.cs index d0ae7487297..73ca6047324 100644 --- a/src/Aspire.Hosting.Azure.Kubernetes/AzureKubernetesEnvironmentResource.AksPipeline.cs +++ b/src/Aspire.Hosting.Azure.Kubernetes/AzureKubernetesEnvironmentResource.AksPipeline.cs @@ -24,6 +24,11 @@ namespace Aspire.Hosting.Azure.Kubernetes; /// public partial class AzureKubernetesEnvironmentResource { + /// + /// Overrides Azure CLI execution for tests that validate generated commands. + /// + internal Func>? AzCommandRunner { get; set; } + /// /// Per-environment AKS preparation work invoked by the prepare-aks-{name} pipeline /// step. Ensures a default user node pool exists and applies node-pool affinity and @@ -201,6 +206,14 @@ private static AksNodePoolResource FindNodePoolResource( /// subsequent Helm and kubectl commands target the AKS cluster. /// private async Task GetAksCredentialsAsync(PipelineStepContext context) + { + await GetAksCredentialsAsync(context, resourceGroup: null, subscriptionId: null).ConfigureAwait(false); + } + + private async Task GetAksCredentialsAsync( + PipelineStepContext context, + string? resourceGroup, + string? subscriptionId) { var getCredsTask = await context.ReportingStep.CreateTaskAsync( $"Fetching AKS credentials for {Name}", @@ -216,16 +229,18 @@ private async Task GetAksCredentialsAsync(PipelineStepContext context) var clusterName = await NameOutputReference.GetValueAsync(context.CancellationToken).ConfigureAwait(false) ?? Name; - var azPath = FindAzCli(); - // Defense-in-depth: validate that values used as CLI arguments // contain only expected characters (alphanumeric, hyphens, underscores, dots). ValidateAzureResourceName(clusterName, "cluster name"); - var resourceGroup = await GetResourceGroupAsync(azPath, clusterName, context) + resourceGroup ??= await GetResourceGroupAsync(clusterName, context) .ConfigureAwait(false); ValidateAzureResourceName(resourceGroup, "resource group"); + if (subscriptionId is not null) + { + ValidateAzureResourceName(subscriptionId, "subscription ID"); + } // Fetch kubeconfig content to stdout using --file - to avoid az CLI // writing credentials with potentially permissive file permissions. @@ -239,8 +254,8 @@ private async Task GetAksCredentialsAsync(PipelineStepContext context) clusterName, resourceGroup); var result = await RunAzCommandAsync( - azPath, - $"aks get-credentials --resource-group \"{resourceGroup}\" --name \"{clusterName}\" --file -", + $"aks get-credentials --resource-group \"{resourceGroup}\" --name \"{clusterName}\" --file -" + + (subscriptionId is null ? string.Empty : $" --subscription \"{subscriptionId}\""), context.Logger).ConfigureAwait(false); if (result.ExitCode != 0) @@ -273,7 +288,10 @@ private async Task GetAksCredentialsAsync(PipelineStepContext context) context.Summary.Add( "🔑 Connect to cluster", - new MarkdownString($"`az aks get-credentials --resource-group {resourceGroup} --name {clusterName}`")); + new MarkdownString( + $"`az aks get-credentials --resource-group {resourceGroup} --name {clusterName}" + + (subscriptionId is null ? string.Empty : $" --subscription {subscriptionId}") + + "`")); await getCredsTask.SucceedAsync( $"AKS credentials fetched for cluster {clusterName}", @@ -309,7 +327,7 @@ private async Task GetAksCredentialsForDestroyAsync(PipelineStepContext context) return; } - await GetAksCredentialsAsync(context).ConfigureAwait(false); + await GetAksCredentialsAsync(context, resourceGroupName, subscriptionId).ConfigureAwait(false); } /// @@ -567,8 +585,7 @@ private static string FindAzCli() /// On first deploy, the deployment state may not be loaded into IConfiguration yet /// because it's written during the pipeline run (after create-provisioning-context). /// - private static async Task GetResourceGroupAsync( - string azPath, + private async Task GetResourceGroupAsync( string clusterName, PipelineStepContext context) { @@ -587,7 +604,6 @@ private static async Task GetResourceGroupAsync( clusterName); var result = await RunAzCommandAsync( - azPath, $"resource list --resource-type Microsoft.ContainerService/managedClusters --name \"{clusterName}\" --query [0].resourceGroup -o tsv", context.Logger).ConfigureAwait(false); @@ -613,13 +629,19 @@ private static async Task GetResourceGroupAsync( /// Runs an az CLI command using the shared ProcessSpec/ProcessUtil infrastructure. /// Returns the captured stdout, stderr, and exit code. /// - private static async Task RunAzCommandAsync( - string azPath, + private async Task RunAzCommandAsync( string arguments, ILogger logger) { + if (AzCommandRunner is not null) + { + var result = await AzCommandRunner(arguments).ConfigureAwait(false); + return new AzCommandResult(result.ExitCode, result.StandardOutput, result.StandardError); + } + var stdout = new StringBuilder(); var stderr = new StringBuilder(); + var azPath = FindAzCli(); var spec = new ProcessSpec(azPath) { diff --git a/tests/Aspire.Hosting.Azure.Kubernetes.Tests/Aspire.Hosting.Azure.Kubernetes.Tests.csproj b/tests/Aspire.Hosting.Azure.Kubernetes.Tests/Aspire.Hosting.Azure.Kubernetes.Tests.csproj index c5760c0421c..8831dc60110 100644 --- a/tests/Aspire.Hosting.Azure.Kubernetes.Tests/Aspire.Hosting.Azure.Kubernetes.Tests.csproj +++ b/tests/Aspire.Hosting.Azure.Kubernetes.Tests/Aspire.Hosting.Azure.Kubernetes.Tests.csproj @@ -16,6 +16,7 @@ + diff --git a/tests/Aspire.Hosting.Azure.Kubernetes.Tests/AzureKubernetesInfrastructureTests.cs b/tests/Aspire.Hosting.Azure.Kubernetes.Tests/AzureKubernetesInfrastructureTests.cs index d75f901407a..8808292288e 100644 --- a/tests/Aspire.Hosting.Azure.Kubernetes.Tests/AzureKubernetesInfrastructureTests.cs +++ b/tests/Aspire.Hosting.Azure.Kubernetes.Tests/AzureKubernetesInfrastructureTests.cs @@ -3,9 +3,11 @@ #pragma warning disable ASPIREAZURE003 #pragma warning disable ASPIREPIPELINES001 +#pragma warning disable ASPIREPIPELINES002 #pragma warning disable ASPIREPIPELINES003 using System.Runtime.CompilerServices; +using System.Text.Json.Nodes; using Aspire.Hosting.ApplicationModel; using Aspire.Hosting.Azure.Kubernetes; using Aspire.Hosting.Kubernetes; @@ -14,6 +16,7 @@ using Aspire.Hosting.Tests; using Aspire.Hosting.Utils; using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Logging.Abstractions; namespace Aspire.Hosting.Azure.Tests; @@ -284,6 +287,60 @@ public async Task DestroyPipelineUsesMatchingCredentialsForEachAksEnvironment() GetDirectDependencies(diagnosticLines, "destroy-azure-azure-environment")); } + [Fact] + public async Task DestroyCredentialAcquisitionUsesPersistedAzureTarget() + { + using var workspace = TemporaryWorkspace.Create(output); + var stateManager = new InMemoryDeploymentStateManager(); + stateManager.SetSection("Azure", new JsonObject + { + ["ResourceGroup"] = "persisted-resource-group", + ["SubscriptionId"] = "00000000-1111-2222-3333-444444444444" + }); + string? azArguments = null; + var azCommandCount = 0; + + using var builder = TestDistributedApplicationBuilder.Create( + DistributedApplicationOperation.Publish, + workspace.Path); + builder.Services.AddSingleton(stateManager); + var aks = builder.AddAzureKubernetesEnvironment("aks"); + aks.Resource.Outputs["name"] = "aks"; + aks.Resource.AzCommandRunner = arguments => + { + azCommandCount++; + azArguments = arguments; + return Task.FromResult((0, "apiVersion: v1", string.Empty)); + }; + + await using var app = builder.Build(); + var model = app.Services.GetRequiredService(); + var pipelineContext = new PipelineContext( + model, + app.Services.GetRequiredService(), + app.Services, + NullLogger.Instance, + CancellationToken.None); + await using var reportingStep = await new NullPublishingActivityReporter().CreateStepAsync("test"); + + await GetAksCredentialsForDestroyAsync(aks.Resource, new PipelineStepContext + { + PipelineContext = pipelineContext, + ReportingStep = reportingStep + }); + + Assert.Equal(1, azCommandCount); + Assert.Equal( + "aks get-credentials --resource-group \"persisted-resource-group\" --name \"aks\" --file - " + + "--subscription \"00000000-1111-2222-3333-444444444444\"", + azArguments); + } + + [UnsafeAccessor(UnsafeAccessorKind.Method, Name = "GetAksCredentialsForDestroyAsync")] + private static extern Task GetAksCredentialsForDestroyAsync( + AzureKubernetesEnvironmentResource resource, + PipelineStepContext context); + private static string GetDirectDependencies(List diagnosticLines, string stepName) { var targetLine = diagnosticLines.IndexOf($"If targeting '{stepName}':"); From 3bb56705094ca5c84409a8047f6d46cda7741b91 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Ros?= <1165805+sebastienros@users.noreply.github.com> Date: Tue, 11 Aug 2026 16:14:46 -0700 Subject: [PATCH 3/9] Read persisted AKS name during destroy Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: a5fbe5fe-6726-431d-af32-692b54aa1c2b --- ...bernetesEnvironmentResource.AksPipeline.cs | 37 +++++++++---- .../AzureKubernetesInfrastructureTests.cs | 55 +++++++++++-------- 2 files changed, 58 insertions(+), 34 deletions(-) diff --git a/src/Aspire.Hosting.Azure.Kubernetes/AzureKubernetesEnvironmentResource.AksPipeline.cs b/src/Aspire.Hosting.Azure.Kubernetes/AzureKubernetesEnvironmentResource.AksPipeline.cs index 73ca6047324..0e50ea90e2d 100644 --- a/src/Aspire.Hosting.Azure.Kubernetes/AzureKubernetesEnvironmentResource.AksPipeline.cs +++ b/src/Aspire.Hosting.Azure.Kubernetes/AzureKubernetesEnvironmentResource.AksPipeline.cs @@ -7,6 +7,7 @@ #pragma warning disable ASPIREFILESYSTEM001 // IFileSystemService/TempDirectory are experimental using System.Text; +using System.Text.Json.Nodes; using System.Text.RegularExpressions; using Aspire.Hosting.ApplicationModel; using Aspire.Hosting.Dcp.Process; @@ -207,11 +208,18 @@ private static AksNodePoolResource FindNodePoolResource( /// private async Task GetAksCredentialsAsync(PipelineStepContext context) { - await GetAksCredentialsAsync(context, resourceGroup: null, subscriptionId: null).ConfigureAwait(false); + // Get the actual provisioned cluster name from the Bicep output. + // The Azure.Provisioning SDK may add a unique suffix to the name + // (e.g., take('aks-${uniqueString(resourceGroup().id)}', 63)). + var clusterName = await NameOutputReference.GetValueAsync(context.CancellationToken).ConfigureAwait(false) + ?? Name; + + await GetAksCredentialsAsync(context, clusterName, resourceGroup: null, subscriptionId: null).ConfigureAwait(false); } private async Task GetAksCredentialsAsync( PipelineStepContext context, + string clusterName, string? resourceGroup, string? subscriptionId) { @@ -223,12 +231,6 @@ private async Task GetAksCredentialsAsync( { try { - // Get the actual provisioned cluster name from the Bicep output. - // The Azure.Provisioning SDK may add a unique suffix to the name - // (e.g., take('aks-${uniqueString(resourceGroup().id)}', 63)). - var clusterName = await NameOutputReference.GetValueAsync(context.CancellationToken).ConfigureAwait(false) - ?? Name; - // Defense-in-depth: validate that values used as CLI arguments // contain only expected characters (alphanumeric, hyphens, underscores, dots). ValidateAzureResourceName(clusterName, "cluster name"); @@ -319,15 +321,30 @@ private async Task GetAksCredentialsForDestroyAsync(PipelineStepContext context) // still needs to uninstall an external chart or delete another cluster-scoped resource. var resourceGroupName = stateSection.Data["ResourceGroup"]?.ToString(); var subscriptionId = stateSection.Data["SubscriptionId"]?.ToString(); - if (string.IsNullOrEmpty(resourceGroupName) || string.IsNullOrEmpty(subscriptionId)) + var deploymentStateSection = await deploymentStateManager + .AcquireSectionAsync($"Azure:Deployments:{Name}", context.CancellationToken) + .ConfigureAwait(false); + + // Azure deployment outputs are persisted as a JSON string with the ARM output shape: + // { "name": { "type": "String", "value": "aks-abc123" } } + // Read it directly because the provisioning step that normally populates Outputs is not + // part of a fresh destroy process. + var outputsJson = deploymentStateSection.Data["Outputs"]?.GetValue(); + var clusterName = string.IsNullOrEmpty(outputsJson) + ? null + : JsonNode.Parse(outputsJson)?["name"]?["value"]?.GetValue(); + + if (string.IsNullOrEmpty(resourceGroupName) || + string.IsNullOrEmpty(subscriptionId) || + string.IsNullOrEmpty(clusterName)) { context.Logger.LogInformation( - "No Azure deployment state found for AKS environment '{EnvironmentName}'. Skipping credential acquisition.", + "No complete Azure deployment state found for AKS environment '{EnvironmentName}'. Skipping credential acquisition.", Name); return; } - await GetAksCredentialsAsync(context, resourceGroupName, subscriptionId).ConfigureAwait(false); + await GetAksCredentialsAsync(context, clusterName, resourceGroupName, subscriptionId).ConfigureAwait(false); } /// diff --git a/tests/Aspire.Hosting.Azure.Kubernetes.Tests/AzureKubernetesInfrastructureTests.cs b/tests/Aspire.Hosting.Azure.Kubernetes.Tests/AzureKubernetesInfrastructureTests.cs index 8808292288e..12b22238490 100644 --- a/tests/Aspire.Hosting.Azure.Kubernetes.Tests/AzureKubernetesInfrastructureTests.cs +++ b/tests/Aspire.Hosting.Azure.Kubernetes.Tests/AzureKubernetesInfrastructureTests.cs @@ -1,6 +1,7 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. +#pragma warning disable ASPIREAZURE001 #pragma warning disable ASPIREAZURE003 #pragma warning disable ASPIREPIPELINES001 #pragma warning disable ASPIREPIPELINES002 @@ -16,7 +17,6 @@ using Aspire.Hosting.Tests; using Aspire.Hosting.Utils; using Microsoft.Extensions.DependencyInjection; -using Microsoft.Extensions.Logging.Abstractions; namespace Aspire.Hosting.Azure.Tests; @@ -288,7 +288,7 @@ public async Task DestroyPipelineUsesMatchingCredentialsForEachAksEnvironment() } [Fact] - public async Task DestroyCredentialAcquisitionUsesPersistedAzureTarget() + public async Task DestroyPipelineUsesPersistedAksOutputWithoutProvisioning() { using var workspace = TemporaryWorkspace.Create(output); var stateManager = new InMemoryDeploymentStateManager(); @@ -297,15 +297,28 @@ public async Task DestroyCredentialAcquisitionUsesPersistedAzureTarget() ["ResourceGroup"] = "persisted-resource-group", ["SubscriptionId"] = "00000000-1111-2222-3333-444444444444" }); + stateManager.SetSection("Azure:Deployments:aks", new JsonObject + { + ["Outputs"] = new JsonObject + { + ["name"] = new JsonObject + { + ["type"] = "String", + ["value"] = "aks-physical-name" + } + }.ToJsonString() + }); string? azArguments = null; var azCommandCount = 0; using var builder = TestDistributedApplicationBuilder.Create( DistributedApplicationOperation.Publish, - workspace.Path); + workspace.Path, + step: WellKnownPipelineSteps.Destroy); builder.Services.AddSingleton(stateManager); + builder.Services.AddSingleton(); + builder.Services.Configure(o => o.SkipConfirmation = true); var aks = builder.AddAzureKubernetesEnvironment("aks"); - aks.Resource.Outputs["name"] = "aks"; aks.Resource.AzCommandRunner = arguments => { azCommandCount++; @@ -313,34 +326,28 @@ public async Task DestroyCredentialAcquisitionUsesPersistedAzureTarget() return Task.FromResult((0, "apiVersion: v1", string.Empty)); }; - await using var app = builder.Build(); - var model = app.Services.GetRequiredService(); - var pipelineContext = new PipelineContext( - model, - app.Services.GetRequiredService(), - app.Services, - NullLogger.Instance, - CancellationToken.None); - await using var reportingStep = await new NullPublishingActivityReporter().CreateStepAsync("test"); - - await GetAksCredentialsForDestroyAsync(aks.Resource, new PipelineStepContext + // Keep the test on the real destroy target while excluding the final ARM deletion, + // which has separate coverage and would require Azure credentials. + var azureEnvironment = builder.Resources.OfType().Single(); + azureEnvironment.Annotations.Add(new PipelineConfigurationAnnotation(context => { - PipelineContext = pipelineContext, - ReportingStep = reportingStep - }); + var destroyAzureStep = context.GetSteps(azureEnvironment) + .Single(step => step.Name == $"destroy-azure-{azureEnvironment.Name}"); + destroyAzureStep.RequiredBySteps.Remove(WellKnownPipelineSteps.Destroy); + return Task.CompletedTask; + })); + + await using var app = builder.Build(); + await app.RunAsync().WaitAsync(TimeSpan.FromSeconds(10)); Assert.Equal(1, azCommandCount); Assert.Equal( - "aks get-credentials --resource-group \"persisted-resource-group\" --name \"aks\" --file - " + + "aks get-credentials --resource-group \"persisted-resource-group\" --name \"aks-physical-name\" --file - " + "--subscription \"00000000-1111-2222-3333-444444444444\"", azArguments); + Assert.Empty(aks.Resource.Outputs); } - [UnsafeAccessor(UnsafeAccessorKind.Method, Name = "GetAksCredentialsForDestroyAsync")] - private static extern Task GetAksCredentialsForDestroyAsync( - AzureKubernetesEnvironmentResource resource, - PipelineStepContext context); - private static string GetDirectDependencies(List diagnosticLines, string stepName) { var targetLine = diagnosticLines.IndexOf($"If targeting '{stepName}':"); From 8f6fbe484d4c12141e93f4d0c740e8d37d2239e4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Ros?= <1165805+sebastienros@users.noreply.github.com> Date: Wed, 12 Aug 2026 11:45:00 -0700 Subject: [PATCH 4/9] Harden AKS destroy credential acquisition Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: a5fbe5fe-6726-431d-af32-692b54aa1c2b --- ...bernetesEnvironmentResource.AksPipeline.cs | 77 +++++++++++---- .../AzureKubernetesEnvironmentResource.cs | 29 +++++- .../AksWithHelmChartDeploymentTests.cs | 2 +- .../AzureKubernetesInfrastructureTests.cs | 98 ++++++++++++++++++- 4 files changed, 177 insertions(+), 29 deletions(-) diff --git a/src/Aspire.Hosting.Azure.Kubernetes/AzureKubernetesEnvironmentResource.AksPipeline.cs b/src/Aspire.Hosting.Azure.Kubernetes/AzureKubernetesEnvironmentResource.AksPipeline.cs index 64e0621a4b8..02338857489 100644 --- a/src/Aspire.Hosting.Azure.Kubernetes/AzureKubernetesEnvironmentResource.AksPipeline.cs +++ b/src/Aspire.Hosting.Azure.Kubernetes/AzureKubernetesEnvironmentResource.AksPipeline.cs @@ -334,38 +334,73 @@ await getCredsTask.FailAsync( private async Task GetAksCredentialsForDestroyAsync(PipelineStepContext context) { var deploymentStateManager = context.Services.GetRequiredService(); - var stateSection = await deploymentStateManager - .AcquireSectionAsync("Azure", context.CancellationToken) - .ConfigureAwait(false); - - // Azure state remains until the entire destroy pipeline succeeds. Use it rather than the - // main Helm release state, which may already be gone when retrying a partial teardown that - // still needs to uninstall an external chart or delete another cluster-scoped resource. - var resourceGroupName = stateSection.Data["ResourceGroup"]?.ToString(); - var subscriptionId = stateSection.Data["SubscriptionId"]?.ToString(); var deploymentStateSection = await deploymentStateManager .AcquireSectionAsync($"Azure:Deployments:{Name}", context.CancellationToken) .ConfigureAwait(false); + if (deploymentStateSection.Data.Count == 0) + { + throw new InvalidOperationException( + $"No Azure deployment state was found for AKS environment '{Name}'. " + + "Cluster cleanup cannot run without an isolated kubeconfig."); + } + // Azure deployment outputs are persisted as a JSON string with the ARM output shape: // { "name": { "type": "String", "value": "aks-abc123" } } // Read it directly because the provisioning step that normally populates Outputs is not // part of a fresh destroy process. - var outputsJson = deploymentStateSection.Data["Outputs"]?.GetValue(); - var clusterName = string.IsNullOrEmpty(outputsJson) - ? null - : JsonNode.Parse(outputsJson)?["name"]?["value"]?.GetValue(); - - if (string.IsNullOrEmpty(resourceGroupName) || - string.IsNullOrEmpty(subscriptionId) || - string.IsNullOrEmpty(clusterName)) + string? clusterName; + try { - context.Logger.LogInformation( - "No complete Azure deployment state found for AKS environment '{EnvironmentName}'. Skipping credential acquisition.", - Name); - return; + var outputsJson = deploymentStateSection.Data["Outputs"]?.GetValue(); + clusterName = string.IsNullOrEmpty(outputsJson) + ? null + : JsonNode.Parse(outputsJson)?["name"]?["value"]?.GetValue(); + } + catch (Exception ex) when (ex is not OperationCanceledException) + { + throw new InvalidOperationException( + $"The Azure deployment state for AKS environment '{Name}' contains invalid outputs.", + ex); } + if (string.IsNullOrEmpty(clusterName)) + { + throw new InvalidOperationException( + $"The Azure deployment state for AKS environment '{Name}' does not contain the deployed cluster name."); + } + + // Scope is persisted as a JSON string using the same shape produced by + // BicepUtilities.SetScopeAsync: + // { "resourceGroup": "shared-rg", "subscription": "00000000-..." } + // A missing property means the resource did not pin that scope value, so only that value + // falls back to global Azure deployment state. Older state without Scope falls back to the + // resource's current explicit scope before consulting global state. + var (scopedSubscription, scopedResourceGroup) = GetExplicitScopeValues(); + if (deploymentStateSection.Data["Scope"] is not null) + { + try + { + var scopeJson = deploymentStateSection.Data["Scope"]!.GetValue(); + var scope = JsonNode.Parse(scopeJson)?.AsObject() + ?? throw new InvalidOperationException("The persisted scope is not a JSON object."); + scopedSubscription = scope["subscription"]?.GetValue(); + scopedResourceGroup = scope["resourceGroup"]?.GetValue(); + } + catch (Exception ex) when (ex is not OperationCanceledException) + { + throw new InvalidOperationException( + $"The Azure deployment state for AKS environment '{Name}' contains an invalid scope.", + ex); + } + } + + var (subscriptionId, resourceGroupName) = await ResolveDeploymentScopeAsync( + scopedSubscription, + scopedResourceGroup, + context.Services, + context.CancellationToken).ConfigureAwait(false); + await GetAksCredentialsAsync(context, clusterName, resourceGroupName, subscriptionId).ConfigureAwait(false); } diff --git a/src/Aspire.Hosting.Azure.Kubernetes/AzureKubernetesEnvironmentResource.cs b/src/Aspire.Hosting.Azure.Kubernetes/AzureKubernetesEnvironmentResource.cs index f33ba9f046d..495c77abcad 100644 --- a/src/Aspire.Hosting.Azure.Kubernetes/AzureKubernetesEnvironmentResource.cs +++ b/src/Aspire.Hosting.Azure.Kubernetes/AzureKubernetesEnvironmentResource.cs @@ -3,10 +3,13 @@ #pragma warning disable ASPIREAZURE003 // Type is for evaluation purposes only and is subject to change or removal in future updates. Suppress this diagnostic to proceed. #pragma warning disable ASPIREPIPELINES001 +#pragma warning disable ASPIREPIPELINES002 #pragma warning disable ASPIREAZURE001 using Aspire.Hosting.Kubernetes; using Aspire.Hosting.Pipelines; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Options; namespace Aspire.Hosting.Azure.Kubernetes; @@ -82,7 +85,7 @@ public AzureKubernetesEnvironmentResource( return Task.FromResult>([prepareStep, getCredentialsStep, getDestroyCredentialsStep]); })); - Annotations.Add(new PipelineConfigurationAnnotation(context => + Annotations.Add(new PipelineConfigurationAnnotation(async context => { var k8sEnv = KubernetesEnvironment; var getDestroyCredentialsStep = context.GetSteps(this) @@ -91,6 +94,28 @@ public AzureKubernetesEnvironmentResource( .GetSteps(HelmDeploymentEngine.GetKubernetesDestroyTag(k8sEnv.Name)) .ToList(); + var deploymentStateManager = context.Services.GetRequiredService(); + var deploymentStateSection = await deploymentStateManager + .AcquireSectionAsync($"Azure:Deployments:{Name}") + .ConfigureAwait(false); + + // A never-deployed AKS environment has no isolated kubeconfig to acquire. Remove all of + // its tagged cluster cleanup from the aggregate destroy target rather than allowing those + // commands to fall back to the caller's ambient Kubernetes context. Explicitly targeting + // one of those cleanup steps still runs through the credential prerequisite and fails. + var targetStep = context.Services.GetRequiredService>().Value.Step; + if (deploymentStateSection.Data.Count == 0 && + string.Equals(targetStep, WellKnownPipelineSteps.Destroy, StringComparison.Ordinal)) + { + foreach (var kubernetesDestroyStep in kubernetesDestroySteps) + { + kubernetesDestroyStep.RequiredBySteps.RemoveAll( + static stepName => string.Equals(stepName, WellKnownPipelineSteps.Destroy, StringComparison.Ordinal)); + } + + return; + } + var azureEnvironment = context.Model.Resources.OfType().Single(); var destroyAzureStep = context.GetSteps(azureEnvironment) .Single(step => step.Name == $"destroy-azure-{azureEnvironment.Name}"); @@ -100,8 +125,6 @@ public AzureKubernetesEnvironmentResource( kubernetesDestroyStep.DependsOn(getDestroyCredentialsStep); destroyAzureStep.DependsOn(kubernetesDestroyStep); } - - return Task.CompletedTask; })); } diff --git a/tests/Aspire.Deployment.EndToEnd.Tests/AksWithHelmChartDeploymentTests.cs b/tests/Aspire.Deployment.EndToEnd.Tests/AksWithHelmChartDeploymentTests.cs index 6d397173d5d..13f6a696681 100644 --- a/tests/Aspire.Deployment.EndToEnd.Tests/AksWithHelmChartDeploymentTests.cs +++ b/tests/Aspire.Deployment.EndToEnd.Tests/AksWithHelmChartDeploymentTests.cs @@ -151,7 +151,7 @@ private async Task DeployAksWithHelmChartCore(CancellationToken cancellationToke // Step 9: Deploy to AKS output.WriteLine("Step 9: Starting AKS deployment with external Helm chart..."); - await auto.TypeAsync("aspire deploy --clear-cache"); + await auto.TypeAsync("aspire deploy"); await auto.EnterAsync(); await auto.WaitForPipelineSuccessAsync(timeout: TimeSpan.FromMinutes(30)); await auto.WaitForSuccessPromptAsync(counter, TimeSpan.FromMinutes(2)); diff --git a/tests/Aspire.Hosting.Azure.Kubernetes.Tests/AzureKubernetesInfrastructureTests.cs b/tests/Aspire.Hosting.Azure.Kubernetes.Tests/AzureKubernetesInfrastructureTests.cs index 8c358d6d5d6..5aed06f162c 100644 --- a/tests/Aspire.Hosting.Azure.Kubernetes.Tests/AzureKubernetesInfrastructureTests.cs +++ b/tests/Aspire.Hosting.Azure.Kubernetes.Tests/AzureKubernetesInfrastructureTests.cs @@ -289,13 +289,13 @@ public async Task DestroyPipelineUsesMatchingCredentialsForEachAksEnvironment() } [Fact] - public async Task DestroyPipelineUsesPersistedAksOutputWithoutProvisioning() + public async Task DestroyPipelineUsesPersistedAksOutputAndScopeWithoutProvisioning() { using var workspace = TemporaryWorkspace.Create(output); var stateManager = new InMemoryDeploymentStateManager(); stateManager.SetSection("Azure", new JsonObject { - ["ResourceGroup"] = "persisted-resource-group", + ["ResourceGroup"] = "app-resource-group", ["SubscriptionId"] = "00000000-1111-2222-3333-444444444444" }); stateManager.SetSection("Azure:Deployments:aks", new JsonObject @@ -307,6 +307,11 @@ public async Task DestroyPipelineUsesPersistedAksOutputWithoutProvisioning() ["type"] = "String", ["value"] = "aks-physical-name" } + }.ToJsonString(), + ["Scope"] = new JsonObject + { + ["resourceGroup"] = "cluster-resource-group", + ["subscription"] = "00000000-5555-6666-7777-888888888888" }.ToJsonString() }); string? azArguments = null; @@ -347,12 +352,97 @@ public async Task DestroyPipelineUsesPersistedAksOutputWithoutProvisioning() Assert.Equal(1, azCommandCount); Assert.Equal( - "aks get-credentials --resource-group \"persisted-resource-group\" --name \"aks-physical-name\" --file - " + - "--subscription \"00000000-1111-2222-3333-444444444444\"", + "aks get-credentials --resource-group \"cluster-resource-group\" --name \"aks-physical-name\" --file - " + + "--subscription \"00000000-5555-6666-7777-888888888888\"", azArguments); Assert.Empty(aks.Resource.Outputs); } + [Fact] + public async Task DestroyPipelineFailsBeforeClusterCleanupWhenAksDeploymentStateIsIncomplete() + { + using var workspace = TemporaryWorkspace.Create(output); + var stateManager = new InMemoryDeploymentStateManager(); + stateManager.SetSection("Azure", new JsonObject + { + ["ResourceGroup"] = "app-resource-group", + ["SubscriptionId"] = "00000000-1111-2222-3333-444444444444" + }); + stateManager.SetSection("Azure:Deployments:aks", new JsonObject + { + ["Outputs"] = "{}" + }); + stateManager.SetSection("Helm:aks", new JsonObject + { + ["ReleaseName"] = "same-name-as-ambient-release", + ["Namespace"] = "default" + }); + + var reporter = new TestPipelineActivityReporter(output); + using var builder = TestDistributedApplicationBuilder.Create( + DistributedApplicationOperation.Publish, + workspace.Path, + step: WellKnownPipelineSteps.Destroy); + builder.Services.AddSingleton(stateManager); + builder.Services.AddSingleton(); + builder.Services.AddSingleton(reporter); + builder.Services.Configure(o => o.SkipConfirmation = true); + var aks = builder.AddAzureKubernetesEnvironment("aks"); + aks.Resource.AzCliPathResolverForTesting = () => + throw new InvalidOperationException("The Azure CLI must not run for incomplete deployment state."); + + await using var app = builder.Build(); + await app.RunAsync().WaitAsync(TimeSpan.FromSeconds(10)); + + Assert.Equal(CompletionState.CompletedWithError, reporter.ResultCompletionState); + Assert.Equal( + "Step 'aks-get-credentials-for-destroy-aks' failed: " + + "The Azure deployment state for AKS environment 'aks' does not contain the deployed cluster name.", + reporter.CompletionMessage); + Assert.Equal( + ["aks-get-credentials-for-destroy-aks", "destroy-prereq"], + reporter.CreatedSteps + .Where(step => step.Contains("destroy", StringComparison.Ordinal)) + .Order(StringComparer.Ordinal)); + Assert.Null(aks.Resource.KubernetesEnvironment.KubeConfigPath); + } + + [Fact] + public async Task DestroyPipelineSkipsClusterCleanupForNeverDeployedAksEnvironment() + { + using var workspace = TemporaryWorkspace.Create(output); + var reporter = new TestPipelineActivityReporter(output); + var stateManager = new InMemoryDeploymentStateManager(); + stateManager.SetSection("Sentinel", new JsonObject { ["Value"] = "cleared-by-destroy" }); + using var builder = TestDistributedApplicationBuilder.Create( + DistributedApplicationOperation.Publish, + workspace.Path, + step: WellKnownPipelineSteps.Destroy); + builder.Services.AddSingleton(stateManager); + builder.Services.AddSingleton(); + builder.Services.AddSingleton(reporter); + builder.Services.Configure(o => o.SkipConfirmation = true); + + var aks = builder.AddAzureKubernetesEnvironment("aks"); + aks.AddHelmChart("same-name-as-ambient-release", "oci://example.com/chart", "1.0.0") + .WithDestroy(); + + await using var app = builder.Build(); + await app.RunAsync().WaitAsync(TimeSpan.FromSeconds(10)); + + Assert.Equal( + ["destroy", "destroy-azure-azure-environment", "destroy-prereq"], + reporter.CreatedSteps + .Where(step => step.Contains("destroy", StringComparison.Ordinal) || + step.StartsWith("helm-uninstall-", StringComparison.Ordinal)) + .Order(StringComparer.Ordinal)); + var sentinelState = await stateManager.AcquireSectionAsync( + "Sentinel", + TestContext.Current.CancellationToken); + Assert.Empty(sentinelState.Data); + Assert.Null(aks.Resource.KubernetesEnvironment.KubeConfigPath); + } + private static string GetDirectDependencies(List diagnosticLines, string stepName) { var targetLine = diagnosticLines.IndexOf($"If targeting '{stepName}':"); From fde40dc8e4fc955bc57c524c80178f37f97c2f69 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Ros?= <1165805+sebastienros@users.noreply.github.com> Date: Mon, 17 Aug 2026 19:10:49 -0700 Subject: [PATCH 5/9] Allow direct Azure cleanup without AKS state Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: a5fbe5fe-6726-431d-af32-692b54aa1c2b --- .../AzureKubernetesEnvironmentResource.cs | 34 +++++---- .../AzureKubernetesInfrastructureTests.cs | 70 +++++++++++++++++++ 2 files changed, 91 insertions(+), 13 deletions(-) diff --git a/src/Aspire.Hosting.Azure.Kubernetes/AzureKubernetesEnvironmentResource.cs b/src/Aspire.Hosting.Azure.Kubernetes/AzureKubernetesEnvironmentResource.cs index 495c77abcad..1f3e2278d30 100644 --- a/src/Aspire.Hosting.Azure.Kubernetes/AzureKubernetesEnvironmentResource.cs +++ b/src/Aspire.Hosting.Azure.Kubernetes/AzureKubernetesEnvironmentResource.cs @@ -99,27 +99,35 @@ public AzureKubernetesEnvironmentResource( .AcquireSectionAsync($"Azure:Deployments:{Name}") .ConfigureAwait(false); + var azureEnvironment = context.Model.Resources.OfType().Single(); + var destroyAzureStep = context.GetSteps(azureEnvironment) + .Single(step => step.Name == $"destroy-azure-{azureEnvironment.Name}"); + // A never-deployed AKS environment has no isolated kubeconfig to acquire. Remove all of - // its tagged cluster cleanup from the aggregate destroy target rather than allowing those - // commands to fall back to the caller's ambient Kubernetes context. Explicitly targeting - // one of those cleanup steps still runs through the credential prerequisite and fails. + // its tagged cluster cleanup from the aggregate destroy target, and do not add it as a + // prerequisite when targeting Azure cleanup directly. Explicitly targeting one of those + // Kubernetes cleanup steps still runs through the credential prerequisite and fails rather + // than allowing the command to fall back to the caller's ambient Kubernetes context. var targetStep = context.Services.GetRequiredService>().Value.Step; - if (deploymentStateSection.Data.Count == 0 && - string.Equals(targetStep, WellKnownPipelineSteps.Destroy, StringComparison.Ordinal)) + if (deploymentStateSection.Data.Count == 0) { - foreach (var kubernetesDestroyStep in kubernetesDestroySteps) + if (string.Equals(targetStep, WellKnownPipelineSteps.Destroy, StringComparison.Ordinal)) { - kubernetesDestroyStep.RequiredBySteps.RemoveAll( - static stepName => string.Equals(stepName, WellKnownPipelineSteps.Destroy, StringComparison.Ordinal)); + foreach (var kubernetesDestroyStep in kubernetesDestroySteps) + { + kubernetesDestroyStep.RequiredBySteps.RemoveAll( + static stepName => string.Equals(stepName, WellKnownPipelineSteps.Destroy, StringComparison.Ordinal)); + } + + return; } - return; + if (string.Equals(targetStep, destroyAzureStep.Name, StringComparison.Ordinal)) + { + return; + } } - var azureEnvironment = context.Model.Resources.OfType().Single(); - var destroyAzureStep = context.GetSteps(azureEnvironment) - .Single(step => step.Name == $"destroy-azure-{azureEnvironment.Name}"); - foreach (var kubernetesDestroyStep in kubernetesDestroySteps) { kubernetesDestroyStep.DependsOn(getDestroyCredentialsStep); diff --git a/tests/Aspire.Hosting.Azure.Kubernetes.Tests/AzureKubernetesInfrastructureTests.cs b/tests/Aspire.Hosting.Azure.Kubernetes.Tests/AzureKubernetesInfrastructureTests.cs index 5aed06f162c..be36ab06c6c 100644 --- a/tests/Aspire.Hosting.Azure.Kubernetes.Tests/AzureKubernetesInfrastructureTests.cs +++ b/tests/Aspire.Hosting.Azure.Kubernetes.Tests/AzureKubernetesInfrastructureTests.cs @@ -443,6 +443,76 @@ public async Task DestroyPipelineSkipsClusterCleanupForNeverDeployedAksEnvironme Assert.Null(aks.Resource.KubernetesEnvironment.KubeConfigPath); } + [Fact] + public async Task DirectAzureDestroySkipsClusterCleanupForNeverDeployedAksEnvironment() + { + using var workspace = TemporaryWorkspace.Create(output); + var reporter = new TestPipelineActivityReporter(output); + var stateManager = new InMemoryDeploymentStateManager(); + using var builder = TestDistributedApplicationBuilder.Create( + DistributedApplicationOperation.Publish, + workspace.Path, + step: "destroy-azure-azure-environment"); + builder.Services.AddSingleton(stateManager); + builder.Services.AddSingleton(); + builder.Services.AddSingleton(reporter); + builder.Services.Configure(o => o.SkipConfirmation = true); + + var aks = builder.AddAzureKubernetesEnvironment("aks"); + aks.AddHelmChart("same-name-as-ambient-release", "oci://example.com/chart", "1.0.0") + .WithDestroy(); + + await using var app = builder.Build(); + await app.RunAsync().WaitAsync(TimeSpan.FromSeconds(10)); + + Assert.Equal( + ["destroy-azure-azure-environment", "destroy-prereq"], + reporter.CreatedSteps + .Where(step => step.Contains("destroy", StringComparison.Ordinal) || + step.StartsWith("helm-uninstall-", StringComparison.Ordinal)) + .Order(StringComparer.Ordinal)); + Assert.Contains( + reporter.CompletedSteps, + step => step is ("destroy-azure-azure-environment", _, CompletionState.Completed)); + Assert.Null(aks.Resource.KubernetesEnvironment.KubeConfigPath); + } + + [Fact] + public async Task DirectKubernetesCleanupFailsForNeverDeployedAksEnvironment() + { + using var workspace = TemporaryWorkspace.Create(output); + var reporter = new TestPipelineActivityReporter(output); + var stateManager = new InMemoryDeploymentStateManager(); + using var builder = TestDistributedApplicationBuilder.Create( + DistributedApplicationOperation.Publish, + workspace.Path, + step: "helm-uninstall-same-name-as-ambient-release"); + builder.Services.AddSingleton(stateManager); + builder.Services.AddSingleton(); + builder.Services.AddSingleton(reporter); + + var aks = builder.AddAzureKubernetesEnvironment("aks"); + aks.AddHelmChart("same-name-as-ambient-release", "oci://example.com/chart", "1.0.0") + .WithDestroy(); + + await using var app = builder.Build(); + await app.RunAsync().WaitAsync(TimeSpan.FromSeconds(10)); + + Assert.Equal(CompletionState.CompletedWithError, reporter.ResultCompletionState); + Assert.Equal( + "Step 'aks-get-credentials-for-destroy-aks' failed: " + + "No Azure deployment state was found for AKS environment 'aks'. " + + "Cluster cleanup cannot run without an isolated kubeconfig.", + reporter.CompletionMessage); + Assert.Equal( + ["aks-get-credentials-for-destroy-aks", "destroy-prereq"], + reporter.CreatedSteps + .Where(step => step.Contains("destroy", StringComparison.Ordinal) || + step.StartsWith("helm-uninstall-", StringComparison.Ordinal)) + .Order(StringComparer.Ordinal)); + Assert.Null(aks.Resource.KubernetesEnvironment.KubeConfigPath); + } + private static string GetDirectDependencies(List diagnosticLines, string stepName) { var targetLine = diagnosticLines.IndexOf($"If targeting '{stepName}':"); From 1035e3612880a1591a750681f570364ea4f601ef Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Ros?= <1165805+sebastienros@users.noreply.github.com> Date: Tue, 18 Aug 2026 13:49:37 -0700 Subject: [PATCH 6/9] Harden AKS destroy retries and state recovery Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: a5fbe5fe-6726-431d-af32-692b54aa1c2b --- ...bernetesEnvironmentResource.AksPipeline.cs | 85 ++++++-- .../AzureKubernetesEnvironmentResource.cs | 2 +- .../KubernetesHelmChartExtensions.cs | 6 + ...pire.Hosting.Azure.Kubernetes.Tests.csproj | 1 + .../AzureKubernetesInfrastructureTests.cs | 205 +++++++++++++++++- .../Aspire.Hosting.Kubernetes.Tests.csproj | 1 + .../HelmVersionValidatorTests.cs | 2 + .../KubernetesDeployTests.cs | 50 +++++ .../FakeHelmRunner.cs | 18 +- 9 files changed, 340 insertions(+), 30 deletions(-) rename tests/{Aspire.Hosting.Kubernetes.Tests => Shared}/FakeHelmRunner.cs (80%) diff --git a/src/Aspire.Hosting.Azure.Kubernetes/AzureKubernetesEnvironmentResource.AksPipeline.cs b/src/Aspire.Hosting.Azure.Kubernetes/AzureKubernetesEnvironmentResource.AksPipeline.cs index 02338857489..d52bdf08d6a 100644 --- a/src/Aspire.Hosting.Azure.Kubernetes/AzureKubernetesEnvironmentResource.AksPipeline.cs +++ b/src/Aspire.Hosting.Azure.Kubernetes/AzureKubernetesEnvironmentResource.AksPipeline.cs @@ -7,6 +7,7 @@ #pragma warning disable ASPIREFILESYSTEM001 // IFileSystemService/TempDirectory are experimental using System.Text; +using System.Text.Json; using System.Text.Json.Nodes; using System.Text.RegularExpressions; using Aspire.Hosting.ApplicationModel; @@ -14,6 +15,7 @@ using Aspire.Hosting.Kubernetes; using Aspire.Hosting.Kubernetes.Resources; using Aspire.Hosting.Pipelines; +using Azure.Core; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging; @@ -346,16 +348,16 @@ private async Task GetAksCredentialsForDestroyAsync(PipelineStepContext context) } // Azure deployment outputs are persisted as a JSON string with the ARM output shape: - // { "name": { "type": "String", "value": "aks-abc123" } } + // { + // "id": { "type": "String", "value": "/subscriptions/.../managedClusters/aks-abc123" }, + // "name": { "type": "String", "value": "aks-abc123" } + // } // Read it directly because the provisioning step that normally populates Outputs is not // part of a fresh destroy process. - string? clusterName; + ResourceIdentifier? clusterResourceId; try { - var outputsJson = deploymentStateSection.Data["Outputs"]?.GetValue(); - clusterName = string.IsNullOrEmpty(outputsJson) - ? null - : JsonNode.Parse(outputsJson)?["name"]?["value"]?.GetValue(); + clusterResourceId = GetPersistedAksResourceId(deploymentStateSection.Data); } catch (Exception ex) when (ex is not OperationCanceledException) { @@ -364,21 +366,25 @@ private async Task GetAksCredentialsForDestroyAsync(PipelineStepContext context) ex); } - if (string.IsNullOrEmpty(clusterName)) + if (clusterResourceId is null) { throw new InvalidOperationException( - $"The Azure deployment state for AKS environment '{Name}' does not contain the deployed cluster name."); + $"The Azure deployment state for AKS environment '{Name}' does not contain the deployed cluster identity."); } // Scope is persisted as a JSON string using the same shape produced by // BicepUtilities.SetScopeAsync: // { "resourceGroup": "shared-rg", "subscription": "00000000-..." } // A missing property means the resource did not pin that scope value, so only that value - // falls back to global Azure deployment state. Older state without Scope falls back to the - // resource's current explicit scope before consulting global state. - var (scopedSubscription, scopedResourceGroup) = GetExplicitScopeValues(); + // falls back to global Azure deployment state. Older state without Scope uses the persisted + // resource ID so a changed AppHost scope cannot redirect cleanup to another cluster or wait + // on provisioning that is not part of the destroy graph. + string subscriptionId; + string? resourceGroupName; if (deploymentStateSection.Data["Scope"] is not null) { + string? scopedSubscription; + string? scopedResourceGroup; try { var scopeJson = deploymentStateSection.Data["Scope"]!.GetValue(); @@ -393,15 +399,60 @@ private async Task GetAksCredentialsForDestroyAsync(PipelineStepContext context) $"The Azure deployment state for AKS environment '{Name}' contains an invalid scope.", ex); } + + (subscriptionId, resourceGroupName) = await ResolveDeploymentScopeAsync( + scopedSubscription, + scopedResourceGroup, + context.Services, + context.CancellationToken).ConfigureAwait(false); + } + else + { + subscriptionId = clusterResourceId.SubscriptionId!; + resourceGroupName = clusterResourceId.ResourceGroupName!; } - var (subscriptionId, resourceGroupName) = await ResolveDeploymentScopeAsync( - scopedSubscription, - scopedResourceGroup, - context.Services, - context.CancellationToken).ConfigureAwait(false); + await GetAksCredentialsAsync(context, clusterResourceId.Name, resourceGroupName, subscriptionId).ConfigureAwait(false); + } + + private static bool HasPersistedAksIdentity(JsonObject deploymentState) + { + try + { + return GetPersistedAksResourceId(deploymentState) is not null; + } + catch (Exception ex) when (ex is JsonException or InvalidOperationException or FormatException) + { + // Malformed or partial state must not prevent the aggregate Azure destroy path from + // deleting the containing resource group. A directly targeted Kubernetes cleanup still + // invokes the credential step, which reports the invalid state rather than using ambient + // Kubernetes credentials. + return false; + } + } + + private static ResourceIdentifier? GetPersistedAksResourceId(JsonObject deploymentState) + { + var outputsJson = deploymentState["Outputs"]?.GetValue(); + if (string.IsNullOrEmpty(outputsJson)) + { + return null; + } + + var resourceId = JsonNode.Parse(outputsJson)?["id"]?["value"]?.GetValue(); + if (!ResourceIdentifier.TryParse(resourceId, out var parsedResourceId) || + parsedResourceId is null || + string.IsNullOrEmpty(parsedResourceId.SubscriptionId) || + string.IsNullOrEmpty(parsedResourceId.ResourceGroupName) || + !string.Equals( + parsedResourceId.ResourceType.ToString(), + "Microsoft.ContainerService/managedClusters", + StringComparison.OrdinalIgnoreCase)) + { + return null; + } - await GetAksCredentialsAsync(context, clusterName, resourceGroupName, subscriptionId).ConfigureAwait(false); + return parsedResourceId; } /// diff --git a/src/Aspire.Hosting.Azure.Kubernetes/AzureKubernetesEnvironmentResource.cs b/src/Aspire.Hosting.Azure.Kubernetes/AzureKubernetesEnvironmentResource.cs index 1f3e2278d30..c141a14097a 100644 --- a/src/Aspire.Hosting.Azure.Kubernetes/AzureKubernetesEnvironmentResource.cs +++ b/src/Aspire.Hosting.Azure.Kubernetes/AzureKubernetesEnvironmentResource.cs @@ -109,7 +109,7 @@ public AzureKubernetesEnvironmentResource( // Kubernetes cleanup steps still runs through the credential prerequisite and fails rather // than allowing the command to fall back to the caller's ambient Kubernetes context. var targetStep = context.Services.GetRequiredService>().Value.Step; - if (deploymentStateSection.Data.Count == 0) + if (!HasPersistedAksIdentity(deploymentStateSection.Data)) { if (string.Equals(targetStep, WellKnownPipelineSteps.Destroy, StringComparison.Ordinal)) { diff --git a/src/Aspire.Hosting.Kubernetes/KubernetesHelmChartExtensions.cs b/src/Aspire.Hosting.Kubernetes/KubernetesHelmChartExtensions.cs index 2dbeeb28b81..f6a74e0f3ff 100644 --- a/src/Aspire.Hosting.Kubernetes/KubernetesHelmChartExtensions.cs +++ b/src/Aspire.Hosting.Kubernetes/KubernetesHelmChartExtensions.cs @@ -414,6 +414,12 @@ private static async Task UninstallHelmChartAsync( var arguments = new StringBuilder(); arguments.Append(CultureInfo.InvariantCulture, $"uninstall {releaseName} --namespace {@namespace}"); + // The chart state is deleted before later destroy steps run, so a retry can reach this + // command after Helm already removed the release. Helm's --ignore-not-found only converts + // that missing-release case to success; authentication, connectivity, and other failures + // still return a nonzero exit code. + // See https://helm.sh/docs/helm/helm_uninstall/. + arguments.Append(" --ignore-not-found"); if (environment.KubeConfigPath is not null) { diff --git a/tests/Aspire.Hosting.Azure.Kubernetes.Tests/Aspire.Hosting.Azure.Kubernetes.Tests.csproj b/tests/Aspire.Hosting.Azure.Kubernetes.Tests/Aspire.Hosting.Azure.Kubernetes.Tests.csproj index 8831dc60110..c218dc9c093 100644 --- a/tests/Aspire.Hosting.Azure.Kubernetes.Tests/Aspire.Hosting.Azure.Kubernetes.Tests.csproj +++ b/tests/Aspire.Hosting.Azure.Kubernetes.Tests/Aspire.Hosting.Azure.Kubernetes.Tests.csproj @@ -17,6 +17,7 @@ + diff --git a/tests/Aspire.Hosting.Azure.Kubernetes.Tests/AzureKubernetesInfrastructureTests.cs b/tests/Aspire.Hosting.Azure.Kubernetes.Tests/AzureKubernetesInfrastructureTests.cs index be36ab06c6c..295216eab5b 100644 --- a/tests/Aspire.Hosting.Azure.Kubernetes.Tests/AzureKubernetesInfrastructureTests.cs +++ b/tests/Aspire.Hosting.Azure.Kubernetes.Tests/AzureKubernetesInfrastructureTests.cs @@ -17,6 +17,7 @@ using Aspire.Hosting.Tests; using Aspire.Hosting.Utils; using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.DependencyInjection.Extensions; using Microsoft.Extensions.Logging.Abstractions; namespace Aspire.Hosting.Azure.Tests; @@ -247,6 +248,118 @@ public async Task DestroyPipelineFetchesCredentialsBeforeClusterCleanupAndDelete GetDirectDependencies(diagnosticLines, "destroy-azure-azure-environment")); } + [Fact] + public async Task DestroyRetryAfterExternalChartCleanupStillReachesAzureDeletion() + { + using var workspace = TemporaryWorkspace.Create(output); + var stateManager = new InMemoryDeploymentStateManager(); + stateManager.SetSection("Azure", new JsonObject + { + ["ResourceGroup"] = "app-resource-group", + ["SubscriptionId"] = "00000000-1111-2222-3333-444444444444" + }); + stateManager.SetSection("Azure:Deployments:aks", new JsonObject + { + ["Outputs"] = new JsonObject + { + ["id"] = new JsonObject + { + ["type"] = "String", + ["value"] = "/subscriptions/00000000-5555-6666-7777-888888888888/" + + "resourceGroups/cluster-resource-group/providers/Microsoft.ContainerService/" + + "managedClusters/aks-physical-name" + }, + ["name"] = new JsonObject + { + ["type"] = "String", + ["value"] = "aks-physical-name" + } + }.ToJsonString(), + ["Scope"] = new JsonObject + { + ["resourceGroup"] = "cluster-resource-group", + ["subscription"] = "00000000-5555-6666-7777-888888888888" + }.ToJsonString() + }); + stateManager.SetSection("HelmChart:aks:podinfo", new JsonObject + { + ["ReleaseName"] = "podinfo", + ["Namespace"] = "podinfo" + }); + + var uninstallCount = 0; + var fakeHelm = new FakeHelmRunner + { + CommandResultFactory = arguments => + { + if (!arguments.StartsWith("uninstall", StringComparison.OrdinalIgnoreCase)) + { + return (0, null); + } + + if (Interlocked.Increment(ref uninstallCount) == 1 || + arguments.Contains(" --ignore-not-found", StringComparison.Ordinal)) + { + return (0, null); + } + + // Helm reports a missing release in this form after the first destroy removed it. + return (1, "Error: uninstall: Release not loaded: podinfo: release: not found"); + } + }; + + async Task RunDestroyAsync() + { + var reporter = new TestPipelineActivityReporter(output); + using var builder = TestDistributedApplicationBuilder.Create( + DistributedApplicationOperation.Publish, + workspace.Path, + step: WellKnownPipelineSteps.Destroy); + builder.Services.AddSingleton(stateManager); + builder.Services.AddSingleton(); + builder.Services.AddSingleton(reporter); + builder.Services.AddSingleton(fakeHelm); + builder.Services.Configure(o => o.SkipConfirmation = true); + + var aks = builder.AddAzureKubernetesEnvironment("aks"); + aks.Resource.AzCliPathResolverForTesting = () => "/fake/az"; + aks.Resource.AzCommandRunnerForTesting = (_, _, _) => Task.FromResult( + new AzureKubernetesEnvironmentResource.AzCommandResult(0, "apiVersion: v1", string.Empty)); + aks.AddHelmChart("podinfo", "oci://example.com/chart", "1.0.0") + .WithDestroy(); + + // Force the real Azure destroy step to fail after its Kubernetes prerequisites. + // The retry must reach this same failure instead of being blocked by the now-missing release. + builder.Services.RemoveAll(); + + await using var app = builder.Build(); + await app.RunAsync().WaitAsync(TimeSpan.FromSeconds(10)); + return reporter; + } + + var firstReporter = await RunDestroyAsync(); + var chartStateAfterFirstDestroy = await stateManager.AcquireSectionAsync( + "HelmChart:aks:podinfo", + TestContext.Current.CancellationToken); + + Assert.Empty(chartStateAfterFirstDestroy.Data); + Assert.StartsWith( + "Step 'destroy-azure-azure-environment' failed:", + firstReporter.CompletionMessage, + StringComparison.Ordinal); + + var retryReporter = await RunDestroyAsync(); + + Assert.Equal(2, uninstallCount); + Assert.All( + fakeHelm.Arguments.Where(arguments => arguments.StartsWith("uninstall", StringComparison.OrdinalIgnoreCase)), + arguments => Assert.Contains(" --ignore-not-found", arguments, StringComparison.Ordinal)); + Assert.StartsWith( + "Step 'destroy-azure-azure-environment' failed:", + retryReporter.CompletionMessage, + StringComparison.Ordinal); + } + [Fact] public async Task DestroyPipelineUsesMatchingCredentialsForEachAksEnvironment() { @@ -302,6 +415,11 @@ public async Task DestroyPipelineUsesPersistedAksOutputAndScopeWithoutProvisioni { ["Outputs"] = new JsonObject { + ["id"] = new JsonObject + { + ["type"] = "String", + ["value"] = "/subscriptions/00000000-5555-6666-7777-888888888888/resourceGroups/cluster-resource-group/providers/Microsoft.ContainerService/managedClusters/aks-physical-name" + }, ["name"] = new JsonObject { ["type"] = "String", @@ -359,18 +477,82 @@ public async Task DestroyPipelineUsesPersistedAksOutputAndScopeWithoutProvisioni } [Fact] - public async Task DestroyPipelineFailsBeforeClusterCleanupWhenAksDeploymentStateIsIncomplete() + public async Task DestroyPipelineUsesPersistedAksResourceIdWhenScopeIsAbsent() { + const string clusterSubscriptionId = "00000000-5555-6666-7777-888888888888"; + const string clusterResourceGroup = "cluster-resource-group"; + const string clusterName = "aks-physical-name"; using var workspace = TemporaryWorkspace.Create(output); var stateManager = new InMemoryDeploymentStateManager(); stateManager.SetSection("Azure", new JsonObject { - ["ResourceGroup"] = "app-resource-group", + ["ResourceGroup"] = "current-app-resource-group", ["SubscriptionId"] = "00000000-1111-2222-3333-444444444444" }); stateManager.SetSection("Azure:Deployments:aks", new JsonObject { - ["Outputs"] = "{}" + ["Outputs"] = new JsonObject + { + ["id"] = new JsonObject + { + ["type"] = "String", + ["value"] = $"/subscriptions/{clusterSubscriptionId}/resourceGroups/{clusterResourceGroup}/providers/Microsoft.ContainerService/managedClusters/{clusterName}" + } + }.ToJsonString() + }); + string? azArguments = null; + + using var builder = TestDistributedApplicationBuilder.Create( + DistributedApplicationOperation.Publish, + workspace.Path, + step: WellKnownPipelineSteps.Destroy); + builder.Services.AddSingleton(stateManager); + builder.Services.AddSingleton(); + builder.Services.Configure(o => o.SkipConfirmation = true); + var aks = builder.AddAzureKubernetesEnvironment("aks"); + + // A fresh destroy does not run provisioning, so resolving either reference would wait + // indefinitely. Compatibility state without Scope must use the persisted resource ID. + aks.Resource.Scope = new AzureBicepResourceScope( + aks.Resource.NameOutputReference, + aks.Resource.Id); + aks.Resource.AzCliPathResolverForTesting = () => "/fake/az"; + aks.Resource.AzCommandRunnerForTesting = (_, arguments, _) => + { + azArguments = arguments; + return Task.FromResult(new AzureKubernetesEnvironmentResource.AzCommandResult( + 0, + "apiVersion: v1", + string.Empty)); + }; + + var azureEnvironment = builder.Resources.OfType().Single(); + azureEnvironment.Annotations.Add(new PipelineConfigurationAnnotation(context => + { + var destroyAzureStep = context.GetSteps(azureEnvironment) + .Single(step => step.Name == $"destroy-azure-{azureEnvironment.Name}"); + destroyAzureStep.RequiredBySteps.Remove(WellKnownPipelineSteps.Destroy); + return Task.CompletedTask; + })); + + await using var app = builder.Build(); + await app.RunAsync().WaitAsync(TimeSpan.FromSeconds(10)); + + Assert.Equal( + $"aks get-credentials --resource-group \"{clusterResourceGroup}\" --name \"{clusterName}\" --file - " + + $"--subscription \"{clusterSubscriptionId}\"", + azArguments); + Assert.Empty(aks.Resource.Outputs); + } + + [Fact] + public async Task DestroyPipelineSkipsClusterCleanupWhenAksDeploymentStateHasNoIdentity() + { + using var workspace = TemporaryWorkspace.Create(output); + var stateManager = new InMemoryDeploymentStateManager(); + stateManager.SetSection("Azure:Deployments:aks", new JsonObject + { + ["Location"] = "westus2" }); stateManager.SetSection("Helm:aks", new JsonObject { @@ -394,16 +576,14 @@ public async Task DestroyPipelineFailsBeforeClusterCleanupWhenAksDeploymentState await using var app = builder.Build(); await app.RunAsync().WaitAsync(TimeSpan.FromSeconds(10)); - Assert.Equal(CompletionState.CompletedWithError, reporter.ResultCompletionState); Assert.Equal( - "Step 'aks-get-credentials-for-destroy-aks' failed: " + - "The Azure deployment state for AKS environment 'aks' does not contain the deployed cluster name.", - reporter.CompletionMessage); - Assert.Equal( - ["aks-get-credentials-for-destroy-aks", "destroy-prereq"], + ["destroy", "destroy-azure-azure-environment", "destroy-prereq"], reporter.CreatedSteps .Where(step => step.Contains("destroy", StringComparison.Ordinal)) .Order(StringComparer.Ordinal)); + Assert.Contains( + reporter.CompletedSteps, + step => step is ("destroy-azure-azure-environment", _, CompletionState.Completed)); Assert.Null(aks.Resource.KubernetesEnvironment.KubeConfigPath); } @@ -444,11 +624,15 @@ public async Task DestroyPipelineSkipsClusterCleanupForNeverDeployedAksEnvironme } [Fact] - public async Task DirectAzureDestroySkipsClusterCleanupForNeverDeployedAksEnvironment() + public async Task DirectAzureDestroySkipsClusterCleanupWithoutPersistedAksIdentity() { using var workspace = TemporaryWorkspace.Create(output); var reporter = new TestPipelineActivityReporter(output); var stateManager = new InMemoryDeploymentStateManager(); + stateManager.SetSection("Azure:Deployments:aks", new JsonObject + { + ["Location"] = "westus2" + }); using var builder = TestDistributedApplicationBuilder.Create( DistributedApplicationOperation.Publish, workspace.Path, @@ -490,6 +674,7 @@ public async Task DirectKubernetesCleanupFailsForNeverDeployedAksEnvironment() builder.Services.AddSingleton(stateManager); builder.Services.AddSingleton(); builder.Services.AddSingleton(reporter); + builder.Services.AddSingleton(); var aks = builder.AddAzureKubernetesEnvironment("aks"); aks.AddHelmChart("same-name-as-ambient-release", "oci://example.com/chart", "1.0.0") diff --git a/tests/Aspire.Hosting.Kubernetes.Tests/Aspire.Hosting.Kubernetes.Tests.csproj b/tests/Aspire.Hosting.Kubernetes.Tests/Aspire.Hosting.Kubernetes.Tests.csproj index d20646ecbcd..634bf320726 100644 --- a/tests/Aspire.Hosting.Kubernetes.Tests/Aspire.Hosting.Kubernetes.Tests.csproj +++ b/tests/Aspire.Hosting.Kubernetes.Tests/Aspire.Hosting.Kubernetes.Tests.csproj @@ -29,6 +29,7 @@ + diff --git a/tests/Aspire.Hosting.Kubernetes.Tests/HelmVersionValidatorTests.cs b/tests/Aspire.Hosting.Kubernetes.Tests/HelmVersionValidatorTests.cs index 134498cfc38..73e124c2475 100644 --- a/tests/Aspire.Hosting.Kubernetes.Tests/HelmVersionValidatorTests.cs +++ b/tests/Aspire.Hosting.Kubernetes.Tests/HelmVersionValidatorTests.cs @@ -1,6 +1,8 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. +using Aspire.Hosting.Tests; + namespace Aspire.Hosting.Kubernetes.Tests; public class HelmVersionValidatorTests diff --git a/tests/Aspire.Hosting.Kubernetes.Tests/KubernetesDeployTests.cs b/tests/Aspire.Hosting.Kubernetes.Tests/KubernetesDeployTests.cs index 877570b8647..a7f2dbf9f56 100644 --- a/tests/Aspire.Hosting.Kubernetes.Tests/KubernetesDeployTests.cs +++ b/tests/Aspire.Hosting.Kubernetes.Tests/KubernetesDeployTests.cs @@ -1806,4 +1806,54 @@ public async Task DestroyHelm_WhenUninstallFails_PreservesState() var stateSection = await stateManager.AcquireSectionAsync("Helm:env"); Assert.Equal("my-release", stateSection.Data["ReleaseName"]?.ToString()); } + + [Fact] + public async Task DestroyExternalHelmChart_DoesNotIgnoreUnrelatedFailure() + { + using var workspace = TemporaryWorkspace.Create(outputHelper); + + var fakeHelm = new FakeHelmRunner + { + CommandResultFactory = arguments => arguments.StartsWith("uninstall", StringComparison.OrdinalIgnoreCase) + ? (1, "Error: Kubernetes cluster unreachable") + : (0, null) + }; + var stateManager = new InMemoryDeploymentStateManager(); + stateManager.SetSection("HelmChart:env:podinfo", new JsonObject + { + ["ReleaseName"] = "podinfo", + ["Namespace"] = "podinfo" + }); + + var reporter = new TestPipelineActivityReporter(outputHelper); + var builder = TestDistributedApplicationBuilder.Create( + DistributedApplicationOperation.Publish, + workspace.Path, + step: WellKnownPipelineSteps.Destroy); + + builder.Services.AddSingleton(); + builder.Services.AddSingleton(reporter); + builder.Services.AddSingleton(stateManager); + builder.Services.AddSingleton(fakeHelm); + builder.Services.Configure(o => o.SkipConfirmation = true); + + builder.AddKubernetesEnvironment("env") + .AddHelmChart("podinfo", "oci://example.com/chart", "1.0.0") + .WithDestroy(); + + using var app = builder.Build(); + await app.RunAsync(); + + var uninstallArguments = Assert.Single( + fakeHelm.Arguments, + arguments => arguments.StartsWith("uninstall", StringComparison.OrdinalIgnoreCase)); + Assert.Contains(" --ignore-not-found", uninstallArguments, StringComparison.Ordinal); + Assert.Equal( + "Step 'helm-uninstall-podinfo' failed: helm uninstall for chart 'podinfo' failed: " + + "Error: Kubernetes cluster unreachable", + reporter.CompletionMessage); + + var stateSection = await stateManager.AcquireSectionAsync("HelmChart:env:podinfo"); + Assert.Equal("podinfo", stateSection.Data["ReleaseName"]?.ToString()); + } } \ No newline at end of file diff --git a/tests/Aspire.Hosting.Kubernetes.Tests/FakeHelmRunner.cs b/tests/Shared/FakeHelmRunner.cs similarity index 80% rename from tests/Aspire.Hosting.Kubernetes.Tests/FakeHelmRunner.cs rename to tests/Shared/FakeHelmRunner.cs index 721f025d98c..dd69f0765a8 100644 --- a/tests/Aspire.Hosting.Kubernetes.Tests/FakeHelmRunner.cs +++ b/tests/Shared/FakeHelmRunner.cs @@ -1,7 +1,10 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. -namespace Aspire.Hosting.Kubernetes.Tests; +using System.Collections.Concurrent; +using Aspire.Hosting.Kubernetes; + +namespace Aspire.Hosting.Tests; /// /// In-memory fake of for tests. Records arguments, @@ -19,8 +22,12 @@ internal sealed class FakeHelmRunner : IHelmRunner public string? LastArguments { get; private set; } + public ConcurrentQueue Arguments { get; } = []; + public int ExitCode { get; set; } + public Func? CommandResultFactory { get; set; } + /// /// Output emitted to onOutputData when arguments start with /// "version". Defaults to a recent stable Helm 4.x release so the @@ -43,6 +50,7 @@ public Task RunAsync( CancellationToken cancellationToken = default) { LastArguments = arguments; + Arguments.Enqueue(arguments); // Match any `helm version ...` probe (the validator passes // `version --short`). @@ -68,6 +76,12 @@ public Task RunAsync( WasUninstallCalled = true; } - return Task.FromResult(ExitCode); + var result = CommandResultFactory?.Invoke(arguments) ?? (ExitCode, null); + if (!string.IsNullOrEmpty(result.StandardError)) + { + onErrorData?.Invoke(result.StandardError); + } + + return Task.FromResult(result.ExitCode); } } From 327e4dc6a7266e7c9996e78a0bec37f932ff6c49 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Ros?= <1165805+sebastienros@users.noreply.github.com> Date: Tue, 18 Aug 2026 14:44:49 -0700 Subject: [PATCH 7/9] Harden AKS destroy retries Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: a5fbe5fe-6726-431d-af32-692b54aa1c2b --- ...bernetesEnvironmentResource.AksPipeline.cs | 67 +++++- .../AzureKubernetesEnvironmentResource.cs | 12 +- .../CertManagerExtensions.cs | 8 + .../Deployment/HelmDeploymentEngine.cs | 37 +++- .../KubernetesEnvironmentResource.cs | 4 + .../KubernetesHelmChartExtensions.cs | 20 +- .../AzureKubernetesInfrastructureTests.cs | 193 ++++++++++++++++-- .../KubernetesDeployTests.cs | 116 ++++++----- 8 files changed, 374 insertions(+), 83 deletions(-) diff --git a/src/Aspire.Hosting.Azure.Kubernetes/AzureKubernetesEnvironmentResource.AksPipeline.cs b/src/Aspire.Hosting.Azure.Kubernetes/AzureKubernetesEnvironmentResource.AksPipeline.cs index d52bdf08d6a..05d6f7af4e9 100644 --- a/src/Aspire.Hosting.Azure.Kubernetes/AzureKubernetesEnvironmentResource.AksPipeline.cs +++ b/src/Aspire.Hosting.Azure.Kubernetes/AzureKubernetesEnvironmentResource.AksPipeline.cs @@ -218,14 +218,20 @@ private async Task GetAksCredentialsAsync(PipelineStepContext context) var clusterName = await NameOutputReference.GetValueAsync(context.CancellationToken).ConfigureAwait(false) ?? Name; - await GetAksCredentialsAsync(context, clusterName, savedResourceGroup: null, subscriptionId: null).ConfigureAwait(false); + await GetAksCredentialsAsync( + context, + clusterName, + savedResourceGroup: null, + subscriptionId: null, + verifyClusterExists: false).ConfigureAwait(false); } private async Task GetAksCredentialsAsync( PipelineStepContext context, string clusterName, string? savedResourceGroup, - string? subscriptionId) + string? subscriptionId, + bool verifyClusterExists) { var getCredsTask = await context.ReportingStep.CreateTaskAsync( $"Fetching AKS credentials for {Name}", @@ -270,6 +276,28 @@ Task RunAzAsync(string path, string arguments) ValidateAzureResourceName(resourceGroup, "resource group"); + KubernetesEnvironment.SkipDestroyCleanup = false; + if (verifyClusterExists && + !await AksResourceExistsAsync( + azPath, + subscriptionId, + resourceGroup, + clusterName, + RunAzAsync).ConfigureAwait(false)) + { + KubernetesEnvironment.KubeConfigPath = null; + KubernetesEnvironment.SkipDestroyCleanup = true; + + context.Logger.LogInformation( + "AKS cluster '{ClusterName}' no longer exists in resource group '{ResourceGroup}'. Skipping cluster cleanup.", + clusterName, + resourceGroup); + await getCredsTask.SucceedAsync( + $"AKS cluster {clusterName} no longer exists; cluster cleanup will be skipped", + context.CancellationToken).ConfigureAwait(false); + return; + } + // Fetch kubeconfig content to stdout using --file - to avoid az CLI // writing credentials with potentially permissive file permissions. // We then write the content ourselves to a temp file with controlled access. @@ -412,7 +440,12 @@ private async Task GetAksCredentialsForDestroyAsync(PipelineStepContext context) resourceGroupName = clusterResourceId.ResourceGroupName!; } - await GetAksCredentialsAsync(context, clusterResourceId.Name, resourceGroupName, subscriptionId).ConfigureAwait(false); + await GetAksCredentialsAsync( + context, + clusterResourceId.Name, + resourceGroupName, + subscriptionId, + verifyClusterExists: true).ConfigureAwait(false); } private static bool HasPersistedAksIdentity(JsonObject deploymentState) @@ -931,6 +964,27 @@ internal static async Task FetchKubeConfigAsync( return result.StandardOutput; } + internal static async Task AksResourceExistsAsync( + string azPath, + string subscriptionId, + string resourceGroup, + string clusterName, + Func> runAzCommandAsync) + { + var result = await runAzCommandAsync( + azPath, + BuildAksResourceExistsArguments(subscriptionId, resourceGroup, clusterName)).ConfigureAwait(false); + + if (result.ExitCode != 0) + { + throw new InvalidOperationException( + $"az resource list failed while checking AKS cluster existence " + + $"(exit code {result.ExitCode}): {result.StandardError}"); + } + + return !string.IsNullOrWhiteSpace(result.StandardOutput); + } + internal static string BuildGetCredentialsArguments( string subscriptionId, string resourceGroup, @@ -940,6 +994,13 @@ internal static string BuildGetCredentialsArguments( internal static string BuildResourceGroupQueryArguments(string subscriptionId, string clusterName) => $"resource list --resource-type Microsoft.ContainerService/managedClusters --name \"{clusterName}\" --query [].resourceGroup -o tsv --subscription \"{subscriptionId}\""; + internal static string BuildAksResourceExistsArguments( + string subscriptionId, + string resourceGroup, + string clusterName) + => $"resource list --resource-group \"{resourceGroup}\" --resource-type Microsoft.ContainerService/managedClusters " + + $"--name \"{clusterName}\" --query [0].id -o tsv --subscription \"{subscriptionId}\""; + /// /// Runs an az CLI command using the shared ProcessSpec/ProcessUtil infrastructure. /// Returns the captured stdout, stderr, and exit code. diff --git a/src/Aspire.Hosting.Azure.Kubernetes/AzureKubernetesEnvironmentResource.cs b/src/Aspire.Hosting.Azure.Kubernetes/AzureKubernetesEnvironmentResource.cs index c141a14097a..fdd586779e8 100644 --- a/src/Aspire.Hosting.Azure.Kubernetes/AzureKubernetesEnvironmentResource.cs +++ b/src/Aspire.Hosting.Azure.Kubernetes/AzureKubernetesEnvironmentResource.cs @@ -131,7 +131,17 @@ public AzureKubernetesEnvironmentResource( foreach (var kubernetesDestroyStep in kubernetesDestroySteps) { kubernetesDestroyStep.DependsOn(getDestroyCredentialsStep); - destroyAzureStep.DependsOn(kubernetesDestroyStep); + + // The direct Helm uninstall step is an explicit, no-confirmation alternative to the + // aggregate destroy step. It needs isolated credentials when targeted directly, but + // Azure cleanup must not schedule both alternatives and uninstall the release twice. + if (!string.Equals( + kubernetesDestroyStep.Name, + HelmDeploymentEngine.GetHelmUninstallStepName(k8sEnv.Name), + StringComparison.Ordinal)) + { + destroyAzureStep.DependsOn(kubernetesDestroyStep); + } } })); } diff --git a/src/Aspire.Hosting.Kubernetes/CertManagerExtensions.cs b/src/Aspire.Hosting.Kubernetes/CertManagerExtensions.cs index 8bb360e9bb0..2e557dd09f7 100644 --- a/src/Aspire.Hosting.Kubernetes/CertManagerExtensions.cs +++ b/src/Aspire.Hosting.Kubernetes/CertManagerExtensions.cs @@ -500,6 +500,14 @@ private static async Task DeleteClusterIssuerAsync( CertManagerIssuerResource issuer) { var environment = certManager.Parent; + if (environment.SkipDestroyCleanup) + { + context.Logger.LogInformation( + "Skipping cert-manager cleanup for Kubernetes environment '{EnvironmentName}' because the cluster no longer exists.", + environment.Name); + return; + } + // Match the lowercase normalization used at apply time so we target the same object. var k8sIssuerName = issuer.Name.ToKubernetesResourceName(); diff --git a/src/Aspire.Hosting.Kubernetes/Deployment/HelmDeploymentEngine.cs b/src/Aspire.Hosting.Kubernetes/Deployment/HelmDeploymentEngine.cs index a73b0d1da11..65cd34a250e 100644 --- a/src/Aspire.Hosting.Kubernetes/Deployment/HelmDeploymentEngine.cs +++ b/src/Aspire.Hosting.Kubernetes/Deployment/HelmDeploymentEngine.cs @@ -31,6 +31,7 @@ internal static partial class HelmDeploymentEngine internal const string PrintSummaryTag = "print-summary"; internal static string GetKubernetesDestroyTag(string environmentName) => $"kubernetes-destroy-{environmentName}"; + internal static string GetHelmUninstallStepName(string environmentName) => $"helm-uninstall-{environmentName}"; /// /// Gets the environment-specific values file name, mirroring Docker Compose's .env.{envName} pattern. @@ -188,6 +189,14 @@ internal static Task> CreateStepsAsync( Tags = [GetKubernetesDestroyTag(environment.Name)], Action = async ctx => { + if (environment.SkipDestroyCleanup) + { + ctx.Logger.LogInformation( + "Skipping Helm cleanup for Kubernetes environment '{EnvironmentName}' because the cluster no longer exists.", + environment.Name); + return; + } + // Check deployment state to verify this environment was actually deployed var deploymentStateManager = ctx.Services.GetRequiredService(); var stateSection = await deploymentStateManager.AcquireSectionAsync($"Helm:{environment.Name}", ctx.CancellationToken).ConfigureAwait(false); @@ -207,11 +216,6 @@ await ctx.ReportingStep.CompleteAsync( var @namespace = savedNamespace ?? "default"; await ConfirmDestroyAsync(ctx, $"Uninstall Helm release '{savedReleaseName}' from namespace '{@namespace}'? This action cannot be undone.").ConfigureAwait(false); - var helmRunner = ctx.Services.GetRequiredService(); - // Defer the prereq check until state exists so `aspire destroy` against a - // never-deployed environment can still report "Nothing to destroy" without - // requiring Helm on PATH. - await HelmVersionValidator.EnsureMinimumVersionAsync(helmRunner, ctx.CancellationToken).ConfigureAwait(false); await HelmUninstallAsync(ctx, environment, savedReleaseName, @namespace).ConfigureAwait(false); ctx.Summary.Add("🗑️ Helm Release", savedReleaseName); @@ -228,12 +232,11 @@ await ctx.ReportingStep.CompleteAsync( // Step 5: Helm uninstall (teardown, callable directly via aspire do without confirmation) var helmUninstallStep = new PipelineStep { - Name = $"helm-uninstall-{environment.Name}", + Name = GetHelmUninstallStepName(environment.Name), Description = $"Uninstalls the Helm release for {environment.Name}.", - Tags = [HelmUninstallTag], + Tags = [HelmUninstallTag, GetKubernetesDestroyTag(environment.Name)], Action = ctx => HelmUninstallAsync(ctx, environment) }; - helmUninstallStep.DependsOn($"check-helm-prereqs-{environment.Name}"); steps.Add(helmUninstallStep); return Task.FromResult>(steps); @@ -583,6 +586,14 @@ private static async Task HelmUninstallAsync(PipelineStepContext context, Kubern private static async Task HelmUninstallAsync(PipelineStepContext context, KubernetesEnvironmentResource environment, string releaseName, string @namespace) { + if (environment.SkipDestroyCleanup) + { + context.Logger.LogInformation( + "Skipping Helm cleanup for Kubernetes environment '{EnvironmentName}' because the cluster no longer exists.", + environment.Name); + return; + } + var uninstallTask = await context.ReportingStep.CreateTaskAsync( new MarkdownString($"Uninstalling Helm release **{releaseName}** from namespace **{@namespace}**"), context.CancellationToken).ConfigureAwait(false); @@ -592,7 +603,15 @@ private static async Task HelmUninstallAsync(PipelineStepContext context, Kubern try { var helmRunner = context.Services.GetRequiredService(); - var arguments = $"uninstall {releaseName} --namespace {@namespace}"; + // Keep the preflight inside the action so AKS destroy can skip cleanup for a cluster + // that no longer exists without requiring Helm on the machine. + await HelmVersionValidator.EnsureMinimumVersionAsync( + helmRunner, + context.CancellationToken).ConfigureAwait(false); + + // The release can already be absent after a direct uninstall or a prior destroy whose + // state cleanup failed. Keep retries idempotent without masking unrelated Helm errors. + var arguments = $"uninstall {releaseName} --namespace {@namespace} --ignore-not-found"; if (environment.KubeConfigPath is not null) { diff --git a/src/Aspire.Hosting.Kubernetes/KubernetesEnvironmentResource.cs b/src/Aspire.Hosting.Kubernetes/KubernetesEnvironmentResource.cs index efa85832fea..3ff8b85b9d2 100644 --- a/src/Aspire.Hosting.Kubernetes/KubernetesEnvironmentResource.cs +++ b/src/Aspire.Hosting.Kubernetes/KubernetesEnvironmentResource.cs @@ -129,6 +129,10 @@ public sealed class KubernetesEnvironmentResource : Resource, IComputeEnvironmen /// public string? KubeConfigPath { get; set; } + // AKS destroy sets this when persisted state points to a cluster that no longer exists. + // Cluster-scoped cleanup must then no-op instead of falling back to ambient credentials. + internal bool SkipDestroyCleanup { get; set; } + /// /// Gets or sets the parent compute environment resource that owns this Kubernetes environment. /// When set, resources with WithComputeEnvironment targeting the parent will also diff --git a/src/Aspire.Hosting.Kubernetes/KubernetesHelmChartExtensions.cs b/src/Aspire.Hosting.Kubernetes/KubernetesHelmChartExtensions.cs index f6a74e0f3ff..32f457f90de 100644 --- a/src/Aspire.Hosting.Kubernetes/KubernetesHelmChartExtensions.cs +++ b/src/Aspire.Hosting.Kubernetes/KubernetesHelmChartExtensions.cs @@ -115,12 +115,6 @@ public static IResourceBuilder AddHelmChart( DependsOnSteps = [WellKnownPipelineSteps.DestroyPrereq], Tags = [HelmDeploymentEngine.GetKubernetesDestroyTag(environment.Name)] }; - // The uninstall path shells out to `helm uninstall`, so it must observe the same - // Helm CLI / version preflight as the deploy path. Without this dep, a missing or - // too-old Helm during teardown would surface as the raw spawn / unknown-flag error - // the env-wide `check-helm-prereqs-{env}` step exists to convert into an actionable - // message. Install is already covered transitively via `helm-deploy-{env}`. - uninstallStep.DependsOn($"check-helm-prereqs-{environment.Name}"); uninstallStep.RequiredBy(WellKnownPipelineSteps.Destroy); steps.Add(uninstallStep); } @@ -393,6 +387,14 @@ private static async Task UninstallHelmChartAsync( string defaultReleaseName, string defaultNamespace) { + if (environment.SkipDestroyCleanup) + { + context.Logger.LogInformation( + "Skipping Helm chart cleanup for Kubernetes environment '{EnvironmentName}' because the cluster no longer exists.", + environment.Name); + return; + } + var logger = context.Services.GetRequiredService>(); var helmRunner = context.Services.GetRequiredService(); var deploymentStateManager = context.Services.GetRequiredService(); @@ -408,6 +410,12 @@ private static async Task UninstallHelmChartAsync( var releaseName = !string.IsNullOrEmpty(savedReleaseName) ? savedReleaseName : defaultReleaseName; var @namespace = !string.IsNullOrEmpty(savedNamespace) ? savedNamespace : defaultNamespace; + // Keep the preflight inside the action so AKS destroy can skip cleanup for a cluster + // that no longer exists without requiring Helm on the machine. + await HelmVersionValidator.EnsureMinimumVersionAsync( + helmRunner, + context.CancellationToken).ConfigureAwait(false); + logger.LogInformation( "Uninstalling Helm release '{ReleaseName}' for chart '{ChartName}' from namespace '{Namespace}'.", releaseName, chart.Name, @namespace); diff --git a/tests/Aspire.Hosting.Azure.Kubernetes.Tests/AzureKubernetesInfrastructureTests.cs b/tests/Aspire.Hosting.Azure.Kubernetes.Tests/AzureKubernetesInfrastructureTests.cs index 295216eab5b..cb875195c4e 100644 --- a/tests/Aspire.Hosting.Azure.Kubernetes.Tests/AzureKubernetesInfrastructureTests.cs +++ b/tests/Aspire.Hosting.Azure.Kubernetes.Tests/AzureKubernetesInfrastructureTests.cs @@ -235,10 +235,13 @@ public async Task DestroyPipelineFetchesCredentialsBeforeClusterCleanupAndDelete "Direct dependencies: aks-get-credentials-for-destroy-aks, destroy-prereq", GetDirectDependencies(diagnosticLines, "destroy-helm-aks")); Assert.Equal( - "Direct dependencies: aks-get-credentials-for-destroy-aks, check-helm-prereqs-aks, cm-issuer-delete-letsencrypt, destroy-prereq", + "Direct dependencies: aks-get-credentials-for-destroy-aks", + GetDirectDependencies(diagnosticLines, "helm-uninstall-aks")); + Assert.Equal( + "Direct dependencies: aks-get-credentials-for-destroy-aks, cm-issuer-delete-letsencrypt, destroy-prereq", GetDirectDependencies(diagnosticLines, "helm-uninstall-cert-manager-chart")); Assert.Equal( - "Direct dependencies: aks-get-credentials-for-destroy-aks, check-helm-prereqs-aks, destroy-prereq", + "Direct dependencies: aks-get-credentials-for-destroy-aks, destroy-prereq", GetDirectDependencies(diagnosticLines, "helm-uninstall-podinfo")); Assert.Equal( "Direct dependencies: aks-get-credentials-for-destroy-aks, destroy-prereq", @@ -292,7 +295,7 @@ public async Task DestroyRetryAfterExternalChartCleanupStillReachesAzureDeletion { CommandResultFactory = arguments => { - if (!arguments.StartsWith("uninstall", StringComparison.OrdinalIgnoreCase)) + if (!arguments.StartsWith("uninstall podinfo ", StringComparison.OrdinalIgnoreCase)) { return (0, null); } @@ -396,6 +399,12 @@ public async Task DestroyPipelineUsesMatchingCredentialsForEachAksEnvironment() Assert.Equal( "Direct dependencies: aks-get-credentials-for-destroy-west, destroy-prereq", GetDirectDependencies(diagnosticLines, "destroy-helm-west")); + Assert.Equal( + "Direct dependencies: aks-get-credentials-for-destroy-east", + GetDirectDependencies(diagnosticLines, "helm-uninstall-east")); + Assert.Equal( + "Direct dependencies: aks-get-credentials-for-destroy-west", + GetDirectDependencies(diagnosticLines, "helm-uninstall-west")); Assert.Equal( "Direct dependencies: destroy-helm-east, destroy-helm-west, destroy-prereq", GetDirectDependencies(diagnosticLines, "destroy-azure-azure-environment")); @@ -432,8 +441,7 @@ public async Task DestroyPipelineUsesPersistedAksOutputAndScopeWithoutProvisioni ["subscription"] = "00000000-5555-6666-7777-888888888888" }.ToJsonString() }); - string? azArguments = null; - var azCommandCount = 0; + var azArguments = new List(); using var builder = TestDistributedApplicationBuilder.Create( DistributedApplicationOperation.Publish, @@ -446,8 +454,7 @@ public async Task DestroyPipelineUsesPersistedAksOutputAndScopeWithoutProvisioni aks.Resource.AzCliPathResolverForTesting = () => "/fake/az"; aks.Resource.AzCommandRunnerForTesting = (_, arguments, _) => { - azCommandCount++; - azArguments = arguments; + azArguments.Add(arguments); return Task.FromResult(new AzureKubernetesEnvironmentResource.AzCommandResult( 0, "apiVersion: v1", @@ -468,10 +475,13 @@ public async Task DestroyPipelineUsesPersistedAksOutputAndScopeWithoutProvisioni await using var app = builder.Build(); await app.RunAsync().WaitAsync(TimeSpan.FromSeconds(10)); - Assert.Equal(1, azCommandCount); Assert.Equal( - "aks get-credentials --resource-group \"cluster-resource-group\" --name \"aks-physical-name\" --file - " + - "--subscription \"00000000-5555-6666-7777-888888888888\"", + [ + "resource list --resource-group \"cluster-resource-group\" --resource-type Microsoft.ContainerService/managedClusters " + + "--name \"aks-physical-name\" --query [0].id -o tsv --subscription \"00000000-5555-6666-7777-888888888888\"", + "aks get-credentials --resource-group \"cluster-resource-group\" --name \"aks-physical-name\" --file - " + + "--subscription \"00000000-5555-6666-7777-888888888888\"" + ], azArguments); Assert.Empty(aks.Resource.Outputs); } @@ -500,7 +510,7 @@ public async Task DestroyPipelineUsesPersistedAksResourceIdWhenScopeIsAbsent() } }.ToJsonString() }); - string? azArguments = null; + var azArguments = new List(); using var builder = TestDistributedApplicationBuilder.Create( DistributedApplicationOperation.Publish, @@ -519,7 +529,7 @@ public async Task DestroyPipelineUsesPersistedAksResourceIdWhenScopeIsAbsent() aks.Resource.AzCliPathResolverForTesting = () => "/fake/az"; aks.Resource.AzCommandRunnerForTesting = (_, arguments, _) => { - azArguments = arguments; + azArguments.Add(arguments); return Task.FromResult(new AzureKubernetesEnvironmentResource.AzCommandResult( 0, "apiVersion: v1", @@ -539,8 +549,12 @@ public async Task DestroyPipelineUsesPersistedAksResourceIdWhenScopeIsAbsent() await app.RunAsync().WaitAsync(TimeSpan.FromSeconds(10)); Assert.Equal( - $"aks get-credentials --resource-group \"{clusterResourceGroup}\" --name \"{clusterName}\" --file - " + - $"--subscription \"{clusterSubscriptionId}\"", + [ + $"resource list --resource-group \"{clusterResourceGroup}\" --resource-type Microsoft.ContainerService/managedClusters " + + $"--name \"{clusterName}\" --query [0].id -o tsv --subscription \"{clusterSubscriptionId}\"", + $"aks get-credentials --resource-group \"{clusterResourceGroup}\" --name \"{clusterName}\" --file - " + + $"--subscription \"{clusterSubscriptionId}\"" + ], azArguments); Assert.Empty(aks.Resource.Outputs); } @@ -661,6 +675,140 @@ public async Task DirectAzureDestroySkipsClusterCleanupWithoutPersistedAksIdenti Assert.Null(aks.Resource.KubernetesEnvironment.KubeConfigPath); } + [Fact] + public async Task DirectAzureDestroySkipsClusterCleanupWhenPersistedAksNoLongerExists() + { + const string subscriptionId = "00000000-5555-6666-7777-888888888888"; + const string resourceGroup = "cluster-resource-group"; + const string clusterName = "deleted-aks"; + using var workspace = TemporaryWorkspace.Create(output); + var reporter = new TestPipelineActivityReporter(output); + var stateManager = new InMemoryDeploymentStateManager(); + stateManager.SetSection("Azure:Deployments:aks", new JsonObject + { + ["Outputs"] = new JsonObject + { + ["id"] = new JsonObject + { + ["type"] = "String", + ["value"] = $"/subscriptions/{subscriptionId}/resourceGroups/{resourceGroup}/providers/Microsoft.ContainerService/managedClusters/{clusterName}" + } + }.ToJsonString() + }); + stateManager.SetSection("Helm:aks", new JsonObject + { + ["ReleaseName"] = "same-name-as-ambient-release", + ["Namespace"] = "default" + }); + var fakeHelm = new FakeHelmRunner { ThrowOnVersion = true }; + var azArguments = new List(); + + using var builder = TestDistributedApplicationBuilder.Create( + DistributedApplicationOperation.Publish, + workspace.Path, + step: "destroy-azure-azure-environment"); + builder.Services.AddSingleton(stateManager); + builder.Services.AddSingleton(); + builder.Services.AddSingleton(reporter); + builder.Services.AddSingleton(fakeHelm); + builder.Services.Configure(o => o.SkipConfirmation = true); + + var aks = builder.AddAzureKubernetesEnvironment("aks"); + aks.Resource.AzCliPathResolverForTesting = () => "/fake/az"; + aks.Resource.AzCommandRunnerForTesting = (_, arguments, _) => + { + azArguments.Add(arguments); + return Task.FromResult(new AzureKubernetesEnvironmentResource.AzCommandResult( + 0, + string.Empty, + string.Empty)); + }; + aks.AddHelmChart("same-name-as-ambient-release", "oci://example.com/chart", "1.0.0") + .WithDestroy(); + + await using var app = builder.Build(); + await app.RunAsync().WaitAsync(TimeSpan.FromSeconds(10)); + + Assert.Equal( + [ + $"resource list --resource-group \"{resourceGroup}\" --resource-type Microsoft.ContainerService/managedClusters " + + $"--name \"{clusterName}\" --query [0].id -o tsv --subscription \"{subscriptionId}\"" + ], + azArguments); + Assert.False(fakeHelm.WasUninstallCalled); + Assert.False(fakeHelm.WasVersionCalled); + Assert.Contains( + reporter.CompletedSteps, + step => step is ("destroy-azure-azure-environment", _, CompletionState.Completed)); + Assert.Null(aks.Resource.KubernetesEnvironment.KubeConfigPath); + Assert.True(aks.Resource.KubernetesEnvironment.SkipDestroyCleanup); + } + + [Fact] + public async Task DirectMainHelmUninstallUsesPersistedAksCredentials() + { + const string subscriptionId = "00000000-5555-6666-7777-888888888888"; + const string resourceGroup = "cluster-resource-group"; + const string clusterName = "aks-physical-name"; + using var workspace = TemporaryWorkspace.Create(output); + var stateManager = new InMemoryDeploymentStateManager(); + stateManager.SetSection("Azure:Deployments:aks", new JsonObject + { + ["Outputs"] = new JsonObject + { + ["id"] = new JsonObject + { + ["type"] = "String", + ["value"] = $"/subscriptions/{subscriptionId}/resourceGroups/{resourceGroup}/providers/Microsoft.ContainerService/managedClusters/{clusterName}" + } + }.ToJsonString() + }); + var fakeHelm = new FakeHelmRunner(); + var azArguments = new List(); + + using var builder = TestDistributedApplicationBuilder.Create( + DistributedApplicationOperation.Publish, + workspace.Path, + step: "helm-uninstall-aks"); + builder.Services.AddSingleton(stateManager); + builder.Services.AddSingleton(); + builder.Services.AddSingleton(fakeHelm); + var aks = builder.AddAzureKubernetesEnvironment("aks"); + aks.Resource.KubernetesEnvironment.Annotations.Add( + new HelmReleaseNameAnnotation(ReferenceExpression.Create($"main-release"))); + aks.Resource.KubernetesEnvironment.Annotations.Add( + new KubernetesNamespaceAnnotation(ReferenceExpression.Create($"main-namespace"))); + aks.Resource.AzCliPathResolverForTesting = () => "/fake/az"; + aks.Resource.AzCommandRunnerForTesting = (_, arguments, _) => + { + azArguments.Add(arguments); + return Task.FromResult(new AzureKubernetesEnvironmentResource.AzCommandResult( + 0, + arguments.StartsWith("resource list", StringComparison.Ordinal) + ? $"/subscriptions/{subscriptionId}/resourceGroups/{resourceGroup}/providers/Microsoft.ContainerService/managedClusters/{clusterName}" + : "apiVersion: v1", + string.Empty)); + }; + + await using var app = builder.Build(); + await app.RunAsync().WaitAsync(TimeSpan.FromSeconds(10)); + + Assert.Equal( + [ + $"resource list --resource-group \"{resourceGroup}\" --resource-type Microsoft.ContainerService/managedClusters " + + $"--name \"{clusterName}\" --query [0].id -o tsv --subscription \"{subscriptionId}\"", + $"aks get-credentials --resource-group \"{resourceGroup}\" --name \"{clusterName}\" --file - " + + $"--subscription \"{subscriptionId}\"" + ], + azArguments); + var kubeConfigPath = Assert.IsType(aks.Resource.KubernetesEnvironment.KubeConfigPath); + Assert.Equal( + $"uninstall main-release --namespace main-namespace --ignore-not-found --kubeconfig \"{kubeConfigPath}\"", + Assert.Single( + fakeHelm.Arguments, + arguments => arguments.StartsWith("uninstall", StringComparison.OrdinalIgnoreCase))); + } + [Fact] public async Task DirectKubernetesCleanupFailsForNeverDeployedAksEnvironment() { @@ -883,6 +1031,23 @@ public async Task FetchKubeConfigThrowsWhenAzureCliFails() exception.Message); } + [Fact] + public async Task AksResourceExistsThrowsWhenAzureCliQueryFails() + { + var exception = await Assert.ThrowsAsync( + () => AzureKubernetesEnvironmentResource.AksResourceExistsAsync( + "/usr/bin/az", + "00000000-0000-0000-0000-000000000001", + "deployment-rg", + "deployment-aks", + (path, arguments) => Task.FromResult( + new AzureKubernetesEnvironmentResource.AzCommandResult(1, "", "authentication failed")))); + + Assert.Equal( + "az resource list failed while checking AKS cluster existence (exit code 1): authentication failed", + exception.Message); + } + [Fact] public async Task GetCredentialsStepScopesEveryAzureCliCallToDeploymentSubscription() { diff --git a/tests/Aspire.Hosting.Kubernetes.Tests/KubernetesDeployTests.cs b/tests/Aspire.Hosting.Kubernetes.Tests/KubernetesDeployTests.cs index a7f2dbf9f56..0488ebf5246 100644 --- a/tests/Aspire.Hosting.Kubernetes.Tests/KubernetesDeployTests.cs +++ b/tests/Aspire.Hosting.Kubernetes.Tests/KubernetesDeployTests.cs @@ -533,22 +533,17 @@ public async Task HelmUninstallStep_RequiredByDestroy() } [Fact] - public async Task HelmUninstallStep_DependsOnCheckHelmPrereqs() + public async Task HelmUninstallStep_ValidatesHelmVersionBeforeUninstall() { - // Regression coverage for PR #17491 review feedback: direct uninstall - // invokes `helm`, so it must gate on the same prereq check as deploy. - // `destroy-helm-{env}` defers the check until saved state exists so the - // no-state path can still report "Nothing to destroy" without Helm. using var workspace = TemporaryWorkspace.Create(outputHelper); - + var fakeHelm = new FakeHelmRunner { VersionExitCode = 1 }; var builder = TestDistributedApplicationBuilder.Create( DistributedApplicationOperation.Publish, workspace.Path, - step: WellKnownPipelineSteps.Diagnostics); - var mockActivityReporter = new TestPipelineActivityReporter(outputHelper); + step: "helm-uninstall-env"); builder.Services.AddSingleton(); - builder.Services.AddSingleton(mockActivityReporter); + builder.Services.AddSingleton(fakeHelm); builder.AddKubernetesEnvironment("env"); builder.AddContainer("api", "myimage"); @@ -556,45 +551,23 @@ public async Task HelmUninstallStep_DependsOnCheckHelmPrereqs() using var app = builder.Build(); await app.RunAsync(); - var logs = mockActivityReporter.LoggedMessages - .Where(s => s.StepTitle == "diagnostics") - .Select(s => s.Message) - .ToList(); - - var diagnosticLines = string.Join('\n', logs) - .Split('\n') - .Select(l => l.Trim()) - .ToList(); - - var destroyTargetLine = diagnosticLines.IndexOf("If targeting 'destroy-helm-env':"); - Assert.InRange(destroyTargetLine, 0, diagnosticLines.Count - 2); - Assert.Equal("Direct dependencies: destroy-prereq", diagnosticLines[destroyTargetLine + 1]); - - var uninstallTargetLine = diagnosticLines.IndexOf("If targeting 'helm-uninstall-env':"); - Assert.InRange(uninstallTargetLine, 0, diagnosticLines.Count - 2); - Assert.Equal("Direct dependencies: check-helm-prereqs-env", diagnosticLines[uninstallTargetLine + 1]); + Assert.Equal(["version --short"], fakeHelm.Arguments); + Assert.True(fakeHelm.WasVersionCalled); + Assert.False(fakeHelm.WasUninstallCalled); } [Fact] - public async Task PerChartHelmUninstallStep_DependsOnCheckHelmPrereqs() + public async Task PerChartHelmUninstallStep_ValidatesHelmVersionBeforeUninstall() { - // Regression coverage for PR #17491 review feedback: per-chart - // `helm-uninstall-{name}` steps created by `AddHelmChart(...).WithDestroy()` - // must depend on `check-helm-prereqs-{env}`. The install side is covered - // transitively (via `helm-deploy-{env}`), but the uninstall side previously - // only set `DependsOnSteps = [DestroyPrereq]`, so a missing or too-old - // Helm during chart teardown would bypass the validator and surface as - // the cryptic spawn / unknown-flag error this PR exists to prevent. using var workspace = TemporaryWorkspace.Create(outputHelper); - + var fakeHelm = new FakeHelmRunner { VersionExitCode = 1 }; var builder = TestDistributedApplicationBuilder.Create( DistributedApplicationOperation.Publish, workspace.Path, - step: WellKnownPipelineSteps.Diagnostics); - var mockActivityReporter = new TestPipelineActivityReporter(outputHelper); + step: "helm-uninstall-podinfo"); builder.Services.AddSingleton(); - builder.Services.AddSingleton(mockActivityReporter); + builder.Services.AddSingleton(fakeHelm); var k8s = builder.AddKubernetesEnvironment("env"); k8s.AddHelmChart("podinfo", "oci://ghcr.io/stefanprodan/charts/podinfo", "6.7.1") @@ -603,14 +576,9 @@ public async Task PerChartHelmUninstallStep_DependsOnCheckHelmPrereqs() using var app = builder.Build(); await app.RunAsync(); - var logs = mockActivityReporter.LoggedMessages - .Where(s => s.StepTitle == "diagnostics") - .Select(s => s.Message) - .ToList(); - - var chartUninstallLines = logs.Where(l => l.Contains("helm-uninstall-podinfo")).ToList(); - Assert.NotEmpty(chartUninstallLines); - Assert.Contains(chartUninstallLines, msg => msg.Contains("check-helm-prereqs-env")); + Assert.Equal(["version --short"], fakeHelm.Arguments); + Assert.True(fakeHelm.WasVersionCalled); + Assert.False(fakeHelm.WasUninstallCalled); } [Fact] @@ -1727,10 +1695,9 @@ public async Task DestroyHelm_WithState_RunsHelmUninstall() using var app = builder.Build(); await app.RunAsync(); - // Verify helm uninstall was called with saved state values - Assert.True(fakeHelm.WasUninstallCalled); - Assert.Contains("my-release", fakeHelm.LastArguments!); - Assert.Contains("my-namespace", fakeHelm.LastArguments!); + Assert.Equal( + "uninstall my-release --namespace my-namespace --ignore-not-found", + fakeHelm.LastArguments); } [Fact] @@ -1807,6 +1774,55 @@ public async Task DestroyHelm_WhenUninstallFails_PreservesState() Assert.Equal("my-release", stateSection.Data["ReleaseName"]?.ToString()); } + [Fact] + public async Task DestroyHelm_WhenReleaseWasAlreadyRemoved_CompletesRetry() + { + using var workspace = TemporaryWorkspace.Create(outputHelper); + + var fakeHelm = new FakeHelmRunner + { + CommandResultFactory = arguments => + arguments.Contains(" --ignore-not-found", StringComparison.Ordinal) + ? (0, null) + : (1, "Error: uninstall: Release not loaded: my-release: release: not found") + }; + var stateManager = new InMemoryDeploymentStateManager(); + stateManager.SetSection("Helm:env", new JsonObject + { + ["ReleaseName"] = "my-release", + ["Namespace"] = "my-namespace" + }); + + var reporter = new TestPipelineActivityReporter(outputHelper); + var builder = TestDistributedApplicationBuilder.Create( + DistributedApplicationOperation.Publish, + workspace.Path, + step: WellKnownPipelineSteps.Destroy); + + builder.Services.AddSingleton(); + builder.Services.AddSingleton(reporter); + builder.Services.AddSingleton(stateManager); + builder.Services.AddSingleton(fakeHelm); + builder.Services.Configure(o => o.SkipConfirmation = true); + + builder.AddKubernetesEnvironment("env"); + builder.AddContainer("api", "myimage"); + + using var app = builder.Build(); + await app.RunAsync(); + + Assert.Equal( + "uninstall my-release --namespace my-namespace --ignore-not-found", + Assert.Single( + fakeHelm.Arguments, + arguments => arguments.StartsWith("uninstall", StringComparison.OrdinalIgnoreCase))); + var stateSection = await stateManager.AcquireSectionAsync("Helm:env"); + Assert.Empty(stateSection.Data); + Assert.Contains( + reporter.CompletedSteps, + step => step is ("pipeline-execution", "Completed successfully", CompletionState.Completed)); + } + [Fact] public async Task DestroyExternalHelmChart_DoesNotIgnoreUnrelatedFailure() { From a4ea3c3bbaadacb01cae0e03ada66dd74304e2a6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Ros?= <1165805+sebastienros@users.noreply.github.com> Date: Fri, 21 Aug 2026 22:33:29 -0700 Subject: [PATCH 8/9] Handle partial AKS destroy state Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: a5fbe5fe-6726-431d-af32-692b54aa1c2b --- .../AzureKubernetesEnvironmentResource.cs | 68 ++++- .../Deployment/HelmDeploymentEngine.cs | 23 +- .../AzureKubernetesInfrastructureTests.cs | 266 +++++++++++++++++- 3 files changed, 346 insertions(+), 11 deletions(-) diff --git a/src/Aspire.Hosting.Azure.Kubernetes/AzureKubernetesEnvironmentResource.cs b/src/Aspire.Hosting.Azure.Kubernetes/AzureKubernetesEnvironmentResource.cs index fdd586779e8..55979f448d7 100644 --- a/src/Aspire.Hosting.Azure.Kubernetes/AzureKubernetesEnvironmentResource.cs +++ b/src/Aspire.Hosting.Azure.Kubernetes/AzureKubernetesEnvironmentResource.cs @@ -6,6 +6,8 @@ #pragma warning disable ASPIREPIPELINES002 #pragma warning disable ASPIREAZURE001 +using System.Text.Json.Nodes; +using Aspire.Hosting.ApplicationModel; using Aspire.Hosting.Kubernetes; using Aspire.Hosting.Pipelines; using Microsoft.Extensions.DependencyInjection; @@ -103,13 +105,22 @@ public AzureKubernetesEnvironmentResource( var destroyAzureStep = context.GetSteps(azureEnvironment) .Single(step => step.Name == $"destroy-azure-{azureEnvironment.Name}"); - // A never-deployed AKS environment has no isolated kubeconfig to acquire. Remove all of - // its tagged cluster cleanup from the aggregate destroy target, and do not add it as a - // prerequisite when targeting Azure cleanup directly. Explicitly targeting one of those - // Kubernetes cleanup steps still runs through the credential prerequisite and fails rather - // than allowing the command to fall back to the caller's ambient Kubernetes context. + // A never-deployed AKS environment has no isolated kubeconfig to acquire. Likewise, a + // partially deployed environment can persist the cluster ID before any Helm release saves + // destroy state. In either case, aggregate Azure cleanup must skip cluster-scoped destroy + // steps rather than block on reacquiring credentials when there is nothing known to clean + // up. Explicitly targeting one of those Kubernetes cleanup steps still runs through the + // credential prerequisite and fails rather than allowing the command to fall back to the + // caller's ambient Kubernetes context. var targetStep = context.Services.GetRequiredService>().Value.Step; - if (!HasPersistedAksIdentity(deploymentStateSection.Data)) + var hasPersistedAksIdentity = HasPersistedAksIdentity(deploymentStateSection.Data); + var hasPersistedKubernetesCleanupState = hasPersistedAksIdentity && + await HasPersistedKubernetesCleanupStateAsync( + deploymentStateManager, + context.Model, + k8sEnv).ConfigureAwait(false); + + if (!hasPersistedAksIdentity || !hasPersistedKubernetesCleanupState) { if (string.Equals(targetStep, WellKnownPipelineSteps.Destroy, StringComparison.Ordinal)) { @@ -146,6 +157,51 @@ public AzureKubernetesEnvironmentResource( })); } + private static async Task HasPersistedKubernetesCleanupStateAsync( + IDeploymentStateManager deploymentStateManager, + DistributedApplicationModel model, + KubernetesEnvironmentResource environment) + { + var environmentState = await deploymentStateManager + .AcquireSectionAsync($"Helm:{environment.Name}") + .ConfigureAwait(false); + if (HasPersistedHelmReleaseState(environmentState.Data)) + { + return true; + } + + foreach (var chart in model.Resources.OfType()) + { + if (!chart.DestroyOnUninstall || + !string.Equals(chart.Parent.Name, environment.Name, StringComparison.Ordinal)) + { + continue; + } + + var chartState = await deploymentStateManager + .AcquireSectionAsync($"HelmChart:{environment.Name}:{chart.Name}") + .ConfigureAwait(false); + if (HasPersistedHelmReleaseState(chartState.Data)) + { + return true; + } + } + + return false; + } + + private static bool HasPersistedHelmReleaseState(JsonObject deploymentState) + { + try + { + return !string.IsNullOrEmpty(deploymentState["ReleaseName"]?.GetValue()); + } + catch (Exception ex) when (ex is InvalidOperationException or FormatException) + { + return false; + } + } + /// /// Gets the underlying Kubernetes environment resource used for Helm-based deployment. /// diff --git a/src/Aspire.Hosting.Kubernetes/Deployment/HelmDeploymentEngine.cs b/src/Aspire.Hosting.Kubernetes/Deployment/HelmDeploymentEngine.cs index 65cd34a250e..e6d7bb783cc 100644 --- a/src/Aspire.Hosting.Kubernetes/Deployment/HelmDeploymentEngine.cs +++ b/src/Aspire.Hosting.Kubernetes/Deployment/HelmDeploymentEngine.cs @@ -579,6 +579,11 @@ private static async Task PrintDeploymentInstructionsAsync( private static async Task HelmUninstallAsync(PipelineStepContext context, KubernetesEnvironmentResource environment) { + if (TrySkipDestroyCleanup(context, environment)) + { + return; + } + var @namespace = await ResolveNamespaceAsync(context, environment).ConfigureAwait(false); var releaseName = await ResolveReleaseNameAsync(context, environment).ConfigureAwait(false); await HelmUninstallAsync(context, environment, releaseName, @namespace).ConfigureAwait(false); @@ -586,11 +591,8 @@ private static async Task HelmUninstallAsync(PipelineStepContext context, Kubern private static async Task HelmUninstallAsync(PipelineStepContext context, KubernetesEnvironmentResource environment, string releaseName, string @namespace) { - if (environment.SkipDestroyCleanup) + if (TrySkipDestroyCleanup(context, environment)) { - context.Logger.LogInformation( - "Skipping Helm cleanup for Kubernetes environment '{EnvironmentName}' because the cluster no longer exists.", - environment.Name); return; } @@ -649,6 +651,19 @@ await uninstallTask.CompleteAsync( } } + private static bool TrySkipDestroyCleanup(PipelineStepContext context, KubernetesEnvironmentResource environment) + { + if (!environment.SkipDestroyCleanup) + { + return false; + } + + context.Logger.LogInformation( + "Skipping Helm cleanup for Kubernetes environment '{EnvironmentName}' because the cluster no longer exists.", + environment.Name); + return true; + } + private static async Task ConfirmDestroyAsync(PipelineStepContext context, string message) { var options = context.Services.GetRequiredService>(); diff --git a/tests/Aspire.Hosting.Azure.Kubernetes.Tests/AzureKubernetesInfrastructureTests.cs b/tests/Aspire.Hosting.Azure.Kubernetes.Tests/AzureKubernetesInfrastructureTests.cs index cb875195c4e..f7918428bec 100644 --- a/tests/Aspire.Hosting.Azure.Kubernetes.Tests/AzureKubernetesInfrastructureTests.cs +++ b/tests/Aspire.Hosting.Azure.Kubernetes.Tests/AzureKubernetesInfrastructureTests.cs @@ -353,10 +353,16 @@ async Task RunDestroyAsync() var retryReporter = await RunDestroyAsync(); - Assert.Equal(2, uninstallCount); + Assert.Equal(1, uninstallCount); Assert.All( fakeHelm.Arguments.Where(arguments => arguments.StartsWith("uninstall", StringComparison.OrdinalIgnoreCase)), arguments => Assert.Contains(" --ignore-not-found", arguments, StringComparison.Ordinal)); + Assert.Equal( + ["destroy-azure-azure-environment", "destroy-prereq"], + retryReporter.CreatedSteps + .Where(step => step.Contains("destroy", StringComparison.Ordinal) || + step.StartsWith("helm-uninstall-", StringComparison.Ordinal)) + .Order(StringComparer.Ordinal)); Assert.StartsWith( "Step 'destroy-azure-azure-environment' failed:", retryReporter.CompletionMessage, @@ -441,6 +447,11 @@ public async Task DestroyPipelineUsesPersistedAksOutputAndScopeWithoutProvisioni ["subscription"] = "00000000-5555-6666-7777-888888888888" }.ToJsonString() }); + stateManager.SetSection("Helm:aks", new JsonObject + { + ["ReleaseName"] = "aks", + ["Namespace"] = "default" + }); var azArguments = new List(); using var builder = TestDistributedApplicationBuilder.Create( @@ -510,6 +521,11 @@ public async Task DestroyPipelineUsesPersistedAksResourceIdWhenScopeIsAbsent() } }.ToJsonString() }); + stateManager.SetSection("Helm:aks", new JsonObject + { + ["ReleaseName"] = "aks", + ["Namespace"] = "default" + }); var azArguments = new List(); using var builder = TestDistributedApplicationBuilder.Create( @@ -601,6 +617,58 @@ public async Task DestroyPipelineSkipsClusterCleanupWhenAksDeploymentStateHasNoI Assert.Null(aks.Resource.KubernetesEnvironment.KubeConfigPath); } + [Fact] + public async Task DestroyPipelineSkipsClusterCleanupWhenPersistedAksIdentityHasNoKubernetesCleanupState() + { + const string subscriptionId = "00000000-5555-6666-7777-888888888888"; + const string resourceGroup = "cluster-resource-group"; + const string clusterName = "aks-physical-name"; + + using var workspace = TemporaryWorkspace.Create(output); + var stateManager = new InMemoryDeploymentStateManager(); + stateManager.SetSection("Azure:Deployments:aks", new JsonObject + { + ["Outputs"] = new JsonObject + { + ["id"] = new JsonObject + { + ["type"] = "String", + ["value"] = $"/subscriptions/{subscriptionId}/resourceGroups/{resourceGroup}/providers/Microsoft.ContainerService/managedClusters/{clusterName}" + } + }.ToJsonString() + }); + + var reporter = new TestPipelineActivityReporter(output); + using var builder = TestDistributedApplicationBuilder.Create( + DistributedApplicationOperation.Publish, + workspace.Path, + step: WellKnownPipelineSteps.Destroy); + builder.Services.AddSingleton(stateManager); + builder.Services.AddSingleton(); + builder.Services.AddSingleton(reporter); + builder.Services.Configure(o => o.SkipConfirmation = true); + + var aks = builder.AddAzureKubernetesEnvironment("aks"); + aks.Resource.AzCliPathResolverForTesting = () => + throw new InvalidOperationException("The Azure CLI must not run when no Kubernetes cleanup state was persisted."); + aks.AddHelmChart("same-name-as-ambient-release", "oci://example.com/chart", "1.0.0") + .WithDestroy(); + + await using var app = builder.Build(); + await app.RunAsync().WaitAsync(TimeSpan.FromSeconds(10)); + + Assert.Equal( + ["destroy", "destroy-azure-azure-environment", "destroy-prereq"], + reporter.CreatedSteps + .Where(step => step.Contains("destroy", StringComparison.Ordinal) || + step.StartsWith("helm-uninstall-", StringComparison.Ordinal)) + .Order(StringComparer.Ordinal)); + Assert.Contains( + reporter.CompletedSteps, + step => step is ("destroy-azure-azure-environment", _, CompletionState.Completed)); + Assert.Null(aks.Resource.KubernetesEnvironment.KubeConfigPath); + } + [Fact] public async Task DestroyPipelineSkipsClusterCleanupForNeverDeployedAksEnvironment() { @@ -675,6 +743,58 @@ public async Task DirectAzureDestroySkipsClusterCleanupWithoutPersistedAksIdenti Assert.Null(aks.Resource.KubernetesEnvironment.KubeConfigPath); } + [Fact] + public async Task DirectAzureDestroySkipsClusterCleanupWhenPersistedAksIdentityHasNoKubernetesCleanupState() + { + const string subscriptionId = "00000000-5555-6666-7777-888888888888"; + const string resourceGroup = "cluster-resource-group"; + const string clusterName = "aks-physical-name"; + + using var workspace = TemporaryWorkspace.Create(output); + var reporter = new TestPipelineActivityReporter(output); + var stateManager = new InMemoryDeploymentStateManager(); + stateManager.SetSection("Azure:Deployments:aks", new JsonObject + { + ["Outputs"] = new JsonObject + { + ["id"] = new JsonObject + { + ["type"] = "String", + ["value"] = $"/subscriptions/{subscriptionId}/resourceGroups/{resourceGroup}/providers/Microsoft.ContainerService/managedClusters/{clusterName}" + } + }.ToJsonString() + }); + + using var builder = TestDistributedApplicationBuilder.Create( + DistributedApplicationOperation.Publish, + workspace.Path, + step: "destroy-azure-azure-environment"); + builder.Services.AddSingleton(stateManager); + builder.Services.AddSingleton(); + builder.Services.AddSingleton(reporter); + builder.Services.Configure(o => o.SkipConfirmation = true); + + var aks = builder.AddAzureKubernetesEnvironment("aks"); + aks.Resource.AzCliPathResolverForTesting = () => + throw new InvalidOperationException("The Azure CLI must not run when no Kubernetes cleanup state was persisted."); + aks.AddHelmChart("same-name-as-ambient-release", "oci://example.com/chart", "1.0.0") + .WithDestroy(); + + await using var app = builder.Build(); + await app.RunAsync().WaitAsync(TimeSpan.FromSeconds(10)); + + Assert.Equal( + ["destroy-azure-azure-environment", "destroy-prereq"], + reporter.CreatedSteps + .Where(step => step.Contains("destroy", StringComparison.Ordinal) || + step.StartsWith("helm-uninstall-", StringComparison.Ordinal)) + .Order(StringComparer.Ordinal)); + Assert.Contains( + reporter.CompletedSteps, + step => step is ("destroy-azure-azure-environment", _, CompletionState.Completed)); + Assert.Null(aks.Resource.KubernetesEnvironment.KubeConfigPath); + } + [Fact] public async Task DirectAzureDestroySkipsClusterCleanupWhenPersistedAksNoLongerExists() { @@ -744,6 +864,150 @@ public async Task DirectAzureDestroySkipsClusterCleanupWhenPersistedAksNoLongerE Assert.True(aks.Resource.KubernetesEnvironment.SkipDestroyCleanup); } + [Fact] + public async Task DirectMainHelmUninstallSkipsAbsentClusterWithoutResolvingParameterBackedAnnotations() + { + const string subscriptionId = "00000000-5555-6666-7777-888888888888"; + const string resourceGroup = "cluster-resource-group"; + const string clusterName = "deleted-aks"; + using var workspace = TemporaryWorkspace.Create(output); + var reporter = new TestPipelineActivityReporter(output); + var stateManager = new InMemoryDeploymentStateManager(); + stateManager.SetSection("Azure:Deployments:aks", new JsonObject + { + ["Outputs"] = new JsonObject + { + ["id"] = new JsonObject + { + ["type"] = "String", + ["value"] = $"/subscriptions/{subscriptionId}/resourceGroups/{resourceGroup}/providers/Microsoft.ContainerService/managedClusters/{clusterName}" + } + }.ToJsonString() + }); + var fakeHelm = new FakeHelmRunner { ThrowOnVersion = true }; + var azArguments = new List(); + + using var builder = TestDistributedApplicationBuilder.Create( + DistributedApplicationOperation.Publish, + workspace.Path, + step: "helm-uninstall-aks"); + builder.Services.AddSingleton(stateManager); + builder.Services.AddSingleton(); + builder.Services.AddSingleton(reporter); + builder.Services.AddSingleton(fakeHelm); + + var releaseParameter = builder.AddParameter("helm-release"); + var namespaceParameter = builder.AddParameter("helm-namespace"); + var aks = builder.AddAzureKubernetesEnvironment("aks"); + aks.Resource.KubernetesEnvironment.Annotations.Add( + new HelmReleaseNameAnnotation(ReferenceExpression.Create($"{releaseParameter.Resource}"))); + aks.Resource.KubernetesEnvironment.Annotations.Add( + new KubernetesNamespaceAnnotation(ReferenceExpression.Create($"{namespaceParameter.Resource}"))); + aks.Resource.AzCliPathResolverForTesting = () => "/fake/az"; + aks.Resource.AzCommandRunnerForTesting = (_, arguments, _) => + { + azArguments.Add(arguments); + return Task.FromResult(new AzureKubernetesEnvironmentResource.AzCommandResult( + 0, + string.Empty, + string.Empty)); + }; + + await using var app = builder.Build(); + await app.RunAsync().WaitAsync(TimeSpan.FromSeconds(10)); + + Assert.Equal( + [ + $"resource list --resource-group \"{resourceGroup}\" --resource-type Microsoft.ContainerService/managedClusters " + + $"--name \"{clusterName}\" --query [0].id -o tsv --subscription \"{subscriptionId}\"" + ], + azArguments); + Assert.Equal( + ["aks-get-credentials-for-destroy-aks", "destroy-prereq", "helm-uninstall-aks"], + reporter.CreatedSteps + .Where(step => step.Contains("destroy", StringComparison.Ordinal) || + step.StartsWith("helm-uninstall-", StringComparison.Ordinal)) + .Order(StringComparer.Ordinal)); + Assert.False(fakeHelm.WasUninstallCalled); + Assert.False(fakeHelm.WasVersionCalled); + Assert.Contains( + reporter.CompletedSteps, + step => step is ("pipeline-execution", "Completed successfully", CompletionState.Completed)); + Assert.Null(aks.Resource.KubernetesEnvironment.KubeConfigPath); + Assert.True(aks.Resource.KubernetesEnvironment.SkipDestroyCleanup); + } + + [Fact] + public async Task DirectMainHelmUninstallSkipsAbsentClusterWithoutResolvingProvisioningBackedAnnotations() + { + const string subscriptionId = "00000000-5555-6666-7777-888888888888"; + const string resourceGroup = "cluster-resource-group"; + const string clusterName = "deleted-aks"; + using var workspace = TemporaryWorkspace.Create(output); + var reporter = new TestPipelineActivityReporter(output); + var stateManager = new InMemoryDeploymentStateManager(); + stateManager.SetSection("Azure:Deployments:aks", new JsonObject + { + ["Outputs"] = new JsonObject + { + ["id"] = new JsonObject + { + ["type"] = "String", + ["value"] = $"/subscriptions/{subscriptionId}/resourceGroups/{resourceGroup}/providers/Microsoft.ContainerService/managedClusters/{clusterName}" + } + }.ToJsonString() + }); + var fakeHelm = new FakeHelmRunner { ThrowOnVersion = true }; + var azArguments = new List(); + + using var builder = TestDistributedApplicationBuilder.Create( + DistributedApplicationOperation.Publish, + workspace.Path, + step: "helm-uninstall-aks"); + builder.Services.AddSingleton(stateManager); + builder.Services.AddSingleton(); + builder.Services.AddSingleton(reporter); + builder.Services.AddSingleton(fakeHelm); + + var aks = builder.AddAzureKubernetesEnvironment("aks"); + // A direct cleanup never provisions the AKS resource, so resolving this output would wait + // indefinitely on ProvisioningTaskCompletionSource if the absent-cluster no-op ran too late. + aks.Resource.KubernetesEnvironment.Annotations.Add( + new HelmReleaseNameAnnotation(ReferenceExpression.Create($"{aks.Resource.NameOutputReference}"))); + aks.Resource.AzCliPathResolverForTesting = () => "/fake/az"; + aks.Resource.AzCommandRunnerForTesting = (_, arguments, _) => + { + azArguments.Add(arguments); + return Task.FromResult(new AzureKubernetesEnvironmentResource.AzCommandResult( + 0, + string.Empty, + string.Empty)); + }; + + await using var app = builder.Build(); + await app.RunAsync().WaitAsync(TimeSpan.FromSeconds(10)); + + Assert.Equal( + [ + $"resource list --resource-group \"{resourceGroup}\" --resource-type Microsoft.ContainerService/managedClusters " + + $"--name \"{clusterName}\" --query [0].id -o tsv --subscription \"{subscriptionId}\"" + ], + azArguments); + Assert.Equal( + ["aks-get-credentials-for-destroy-aks", "destroy-prereq", "helm-uninstall-aks"], + reporter.CreatedSteps + .Where(step => step.Contains("destroy", StringComparison.Ordinal) || + step.StartsWith("helm-uninstall-", StringComparison.Ordinal)) + .Order(StringComparer.Ordinal)); + Assert.False(fakeHelm.WasUninstallCalled); + Assert.False(fakeHelm.WasVersionCalled); + Assert.Contains( + reporter.CompletedSteps, + step => step is ("pipeline-execution", "Completed successfully", CompletionState.Completed)); + Assert.Null(aks.Resource.KubernetesEnvironment.KubeConfigPath); + Assert.True(aks.Resource.KubernetesEnvironment.SkipDestroyCleanup); + } + [Fact] public async Task DirectMainHelmUninstallUsesPersistedAksCredentials() { From c303691c9de7ec5e0a36b776feab7292fa7cb12b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Ros?= <1165805+sebastienros@users.noreply.github.com> Date: Fri, 4 Sep 2026 08:15:07 -0700 Subject: [PATCH 9/9] Address AKS destroy follow-ups Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: a5fbe5fe-6726-431d-af32-692b54aa1c2b --- ...bernetesEnvironmentResource.AksPipeline.cs | 8 +++++ .../AzureKubernetesEnvironmentResource.cs | 8 +++-- .../AksWithHelmChartDeploymentTests.cs | 6 +++- .../AzureKubernetesInfrastructureTests.cs | 32 ++++++++++++++++--- 4 files changed, 46 insertions(+), 8 deletions(-) diff --git a/src/Aspire.Hosting.Azure.Kubernetes/AzureKubernetesEnvironmentResource.AksPipeline.cs b/src/Aspire.Hosting.Azure.Kubernetes/AzureKubernetesEnvironmentResource.AksPipeline.cs index 05d6f7af4e9..3e6db1a9474 100644 --- a/src/Aspire.Hosting.Azure.Kubernetes/AzureKubernetesEnvironmentResource.AksPipeline.cs +++ b/src/Aspire.Hosting.Azure.Kubernetes/AzureKubernetesEnvironmentResource.AksPipeline.cs @@ -977,6 +977,14 @@ internal static async Task AksResourceExistsAsync( if (result.ExitCode != 0) { + // Azure CLI reports an out-of-band deleted resource group as: + // (ResourceGroupNotFound) Resource group 'deployment-rg' could not be found. + // This proves the persisted AKS resource is absent, so cluster cleanup can be skipped. + if (result.StandardError.Contains("(ResourceGroupNotFound)", StringComparison.OrdinalIgnoreCase)) + { + return false; + } + throw new InvalidOperationException( $"az resource list failed while checking AKS cluster existence " + $"(exit code {result.ExitCode}): {result.StandardError}"); diff --git a/src/Aspire.Hosting.Azure.Kubernetes/AzureKubernetesEnvironmentResource.cs b/src/Aspire.Hosting.Azure.Kubernetes/AzureKubernetesEnvironmentResource.cs index 33f312e9bf9..ad2029fc09f 100644 --- a/src/Aspire.Hosting.Azure.Kubernetes/AzureKubernetesEnvironmentResource.cs +++ b/src/Aspire.Hosting.Azure.Kubernetes/AzureKubernetesEnvironmentResource.cs @@ -102,9 +102,13 @@ public AzureKubernetesEnvironmentResource( .AcquireSectionAsync($"Azure:Deployments:{Name}") .ConfigureAwait(false); - var azureEnvironment = context.Model.Resources.OfType().Single(); + var azureEnvironment = context.Model.Resources.OfType().SingleOrDefault() + ?? throw new InvalidOperationException( + $"Azure environment resource required by AKS environment '{Name}' was not found."); var destroyAzureStep = context.GetSteps(azureEnvironment) - .Single(step => step.Name == $"destroy-azure-{azureEnvironment.Name}"); + .SingleOrDefault(step => step.Name == $"destroy-azure-{azureEnvironment.Name}") + ?? throw new InvalidOperationException( + $"Azure destroy step for environment '{azureEnvironment.Name}' was not found."); // A never-deployed AKS environment has no isolated kubeconfig to acquire. Likewise, a // partially deployed environment can persist the cluster ID before any Helm release saves diff --git a/tests/Aspire.Deployment.EndToEnd.Tests/AksWithHelmChartDeploymentTests.cs b/tests/Aspire.Deployment.EndToEnd.Tests/AksWithHelmChartDeploymentTests.cs index 13f6a696681..374e52dd870 100644 --- a/tests/Aspire.Deployment.EndToEnd.Tests/AksWithHelmChartDeploymentTests.cs +++ b/tests/Aspire.Deployment.EndToEnd.Tests/AksWithHelmChartDeploymentTests.cs @@ -225,7 +225,11 @@ await auto.TypeAsync( // Step 16: Destroy the application and opted-in external Helm chart before // deleting the AKS resource group. output.WriteLine("Step 16: Destroying deployment..."); - await auto.AspireDestroyAsync(counter); + await auto.TypeAsync("aspire destroy --yes"); + await auto.EnterAsync(); + await auto.WaitUntilTextAsync("helm-uninstall-podinfo", timeout: TimeSpan.FromMinutes(10)); + await auto.WaitForPipelineSuccessAsync(timeout: TimeSpan.FromMinutes(20)); + await auto.WaitForSuccessPromptAsync(counter, TimeSpan.FromMinutes(1)); // Step 17: Exit terminal await auto.TypeAsync("exit"); diff --git a/tests/Aspire.Hosting.Azure.Kubernetes.Tests/AzureKubernetesInfrastructureTests.cs b/tests/Aspire.Hosting.Azure.Kubernetes.Tests/AzureKubernetesInfrastructureTests.cs index f7918428bec..21d5bd317d2 100644 --- a/tests/Aspire.Hosting.Azure.Kubernetes.Tests/AzureKubernetesInfrastructureTests.cs +++ b/tests/Aspire.Hosting.Azure.Kubernetes.Tests/AzureKubernetesInfrastructureTests.cs @@ -795,8 +795,13 @@ public async Task DirectAzureDestroySkipsClusterCleanupWhenPersistedAksIdentityH Assert.Null(aks.Resource.KubernetesEnvironment.KubeConfigPath); } - [Fact] - public async Task DirectAzureDestroySkipsClusterCleanupWhenPersistedAksNoLongerExists() + [Theory] + [InlineData(0, "", "")] + [InlineData(3, "", "(ResourceGroupNotFound) Resource group 'cluster-resource-group' could not be found.")] + public async Task DirectAzureDestroySkipsClusterCleanupWhenPersistedAksNoLongerExists( + int resourceQueryExitCode, + string resourceQueryOutput, + string resourceQueryError) { const string subscriptionId = "00000000-5555-6666-7777-888888888888"; const string resourceGroup = "cluster-resource-group"; @@ -839,9 +844,9 @@ public async Task DirectAzureDestroySkipsClusterCleanupWhenPersistedAksNoLongerE { azArguments.Add(arguments); return Task.FromResult(new AzureKubernetesEnvironmentResource.AzCommandResult( - 0, - string.Empty, - string.Empty)); + resourceQueryExitCode, + resourceQueryOutput, + resourceQueryError)); }; aks.AddHelmChart("same-name-as-ambient-release", "oci://example.com/chart", "1.0.0") .WithDestroy(); @@ -1312,6 +1317,23 @@ public async Task AksResourceExistsThrowsWhenAzureCliQueryFails() exception.Message); } + [Fact] + public async Task AksResourceDoesNotExistWhenResourceGroupWasDeleted() + { + var exists = await AzureKubernetesEnvironmentResource.AksResourceExistsAsync( + "/usr/bin/az", + "00000000-0000-0000-0000-000000000001", + "deleted-rg", + "deployment-aks", + (path, arguments) => Task.FromResult( + new AzureKubernetesEnvironmentResource.AzCommandResult( + 3, + "", + "(ResourceGroupNotFound) Resource group 'deleted-rg' could not be found."))); + + Assert.False(exists); + } + [Fact] public async Task GetCredentialsStepScopesEveryAzureCliCallToDeploymentSubscription() {