Fix port mismatch for bait-and-switch resources in Kubernetes publisher#14590
Fix port mismatch for bait-and-switch resources in Kubernetes publisher#14590mitchdenny merged 31 commits intodotnet:release/13.2from
Conversation
* Add startup perf collection script * Analyze trace more efficiently * Increase pause between iterations * Fix TraceAnalyzer * Add startup-perf skill
…se/13.2 to main (dotnet#14453) * Add backmerge release workflow to automate merging changes from release/13.2 to main * Apply suggestions from code review Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Apply more fixes and use dotnet's action --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
* Initial plan * Bump Aspire branding from 13.2 to 13.3 Co-authored-by: joperezr <13854455+joperezr@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: joperezr <13854455+joperezr@users.noreply.github.com>
…o latest (dotnet#14361) * Update to Azure.Core 1.51.1 Use latest versions for all dotnet/runtime nuget packages. This simplifies our dependency management. Remove ForceLatestDotnetVersions property from multiple project files * Update AzureDeployerTests to use WaitForShutdown instead of StopAsync There is a timing issue when using Start/Stop since the background pipeline might still be running and it cancels the pipeline before it can complete. * Fix AuxiliaryBackchannelTests by adding a Task that completes when the AuxiliaryBackchannelService is listening and ready for connections. * Remove double registration of AuxiliaryBackchannelService as an IHostedService. * Fix ResourceLoggerForwarderServiceTests to ensure the ResourceLoggerForwarderService has started before signalling the stopping token.
…13556) Co-authored-by: Jose Perez Rodriguez <joperezr@microsoft.com>
…ease-13.2-to-main
…cies (dotnet#14478) * Initial plan * Fix transitive Azure role assignments through WaitFor dependencies Remove CollectAnnotationDependencies calls from CollectDependenciesFromValue to prevent WaitFor/parent/connection-string-redirect annotations from referenced resources being included as direct dependencies of the caller. Add tests verifying: - DirectOnly mode excludes WaitFor deps from referenced resources - WaitFor doesn't create transitive role assignments in Azure publish Co-authored-by: eerhardt <8291187+eerhardt@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: eerhardt <8291187+eerhardt@users.noreply.github.com>
…-main [Automated] Backmerge release/13.2 to main
* Remove auto-merge step from backmerge workflow * Update PR body to request merge commit instead of auto-merge
…ease-13.2-to-main
…-main [Automated] Backmerge release/13.2 to main
…ease-13.2-to-main
* Manually update DCP to 0.22.5 (dotnet#14501) * Fix Spectre markup rendering in CLI selection prompts (dotnet#14497) * Fix Spectre markup rendering in CLI selection prompts The blanket EscapeMarkup() added to the UseConverter in PromptForSelectionAsync/PromptForSelectionsAsync (PR dotnet#14422) escaped all markup including the intentional [bold]...[/] used by AddCommand.PackageNameWithFriendlyNameIfAvailable, causing literal '[bold]postgresql[/]' to appear in 'aspire add' output. Fix: remove blanket escaping from the generic converter and instead escape dynamic values at each caller site that formats user-controlled or file-system content (file paths, package names, channel details, publish prompt options), while preserving intentional Spectre markup. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Update tests/Aspire.Cli.Tests/Interaction/ConsoleInteractionServiceTests.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: Mitch Denny <midenn@microsoft.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Update DCP to version 0.22.6 (dotnet#14517) --------- Co-authored-by: David Negstad <50252651+danegsta@users.noreply.github.com> Co-authored-by: Jose Perez Rodriguez <joperezr@microsoft.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: Mitch Denny <midenn@microsoft.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Co-authored-by: Karol Zadora-Przylecki <karolz@microsoft.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
* Fix Windows pipeline image to use windows.vs2022.amd64.open (dotnet#14492) * Fix Windows pipeline image to use windows.vs2022.amd64.open * Use windows.vs2026preview.scout.amd64 for public pipeline Windows pool Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Add Azure portal link for Resource Group in deploy pipeline summary (dotnet#14434) * Add Azure portal link for Resource Group in pipeline summary When printing the Resource Group in the pipeline summary of `aspire deploy`, include a clickable link to the Azure portal resource group page. The link uses the format: https://portal.azure.com/#@{tenantId}/resource/subscriptions/{subscriptionId}/resourceGroups/{resourceGroupName}/overview Changes: - AzureEnvironmentResource.AddToPipelineSummary: construct markdown link for resource group - ConsoleActivityLogger.FormatPipelineSummaryKvp: convert markdown to Spectre markup for clickable links - Add ConsoleActivityLoggerTests for the new markdown rendering behavior Co-authored-by: eerhardt <8291187+eerhardt@users.noreply.github.com> * Clean up the code * Fix tests * More test fixups * Refactor code * Update src/Aspire.Cli/Utils/MarkdownToSpectreConverter.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Add test for color-enabled non-interactive rendering path Co-authored-by: eerhardt <8291187+eerhardt@users.noreply.github.com> * fix test --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: eerhardt <8291187+eerhardt@users.noreply.github.com> Co-authored-by: Eric Erhardt <eric.erhardt@microsoft.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --------- Co-authored-by: Jose Perez Rodriguez <joperezr@microsoft.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com> Co-authored-by: eerhardt <8291187+eerhardt@users.noreply.github.com> Co-authored-by: Eric Erhardt <eric.erhardt@microsoft.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: sebastienros <sebastienros@users.noreply.github.com>
Check default installation directories (~/.aspire/bin, ~/.dotnet/tools) when the Aspire CLI is not found on the system PATH. If found at a default location, the VS Code setting is auto-updated. If later found on PATH, the setting is cleared. Resolution order: configured custom path > system PATH > default install paths. Fixes dotnet#14235
…tnet#14531) * Initial plan * [automated] Unquarantine stable tests - Unquarantined: DeployCommandIncludesDeployFlagInArguments - Unquarantined: GetAppHostsCommand_WithMultipleProjects_ReturnsSuccessWithAllCandidates - Unquarantined: GetAppHostsCommand_WithSingleProject_ReturnsSuccessWithValidJson - Unquarantined: PushImageToRegistry_WithRemoteRegistry_PushesImage - Unquarantined: ProcessParametersStep_ValidatesBehavior - Unquarantined: WithHttpCommand_EnablesCommandOnceResourceIsRunning These tests are being unquarantined as they have had 25+ days of quarantined run data with zero failures. Co-authored-by: radical <1472+radical@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: radical <1472+radical@users.noreply.github.com>
…_WithMultipleComputeEnvironments_Works (dotnet#14551) * Initial plan * Update snapshot for DeployAsync_WithMultipleComputeEnvironments_Works test Co-authored-by: radical <1472+radical@users.noreply.github.com> * Remove quarantine attribute from DeployAsync_WithMultipleComputeEnvironments_Works test Co-authored-by: radical <1472+radical@users.noreply.github.com> * Restore quarantine attribute - step="deploy" case still fails Co-authored-by: radical <1472+radical@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: radical <1472+radical@users.noreply.github.com>
…ease-13.2-to-main
…-main [Automated] Backmerge release/13.2 to main
* Update daily report to 13.2 milestone burndown Refocus the daily-repo-status agentic workflow to serve as a 13.2 release burndown report: - Track 13.2 milestone issues closed/opened in the last 24 hours - Highlight new bugs added to the milestone - Summarize PRs merged to release/13.2 branch - List PRs targeting release/13.2 awaiting review - Surface relevant 13.2 discussions - Generate a Mermaid xychart burndown using cache-memory snapshots - Keep general triage queue as a brief secondary section - Schedule daily around 9am, auto-close older report issues Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Address review feedback: clarify cache schema and queries - Exclude PRs from milestone counts (issues-only filter) - Specify exact JSON schema for cache-memory burndown snapshots - Add dedup, sort, and trim-to-7 logic for cache entries - Simplify 'new issues' query to opened-in-last-24h with milestone Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ease-13.2-to-main
…-main [Automated] Backmerge release/13.2 to main
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/dotnet/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 14590Or
iex "& { $(irm https://raw.githubusercontent.com/dotnet/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 14590" |
There was a problem hiding this comment.
Pull request overview
This PR fixes a port allocation mismatch in the Kubernetes publisher caused by the "bait-and-switch" pattern used by PublishAsDockerFile(). When an ExecutableResource is replaced with an ExecutableContainerResource, the Kubernetes publisher's cache dictionary failed to recognize them as the same logical resource, causing duplicate KubernetesResource objects with different ports to be created.
Changes:
- Modified
KubernetesEnvironmentContextto useResourceNameComparerfor its resource cache dictionary - Added
ResourceNameComparer.csas a shared file to the Kubernetes project - Added a test case to verify port mapping works correctly for bait-and-switch resources
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/Aspire.Hosting.Kubernetes/KubernetesEnvironmentContext.cs | Updated dictionary to use ResourceNameComparer for name-based equality instead of reference equality |
| src/Aspire.Hosting.Kubernetes/Aspire.Hosting.Kubernetes.csproj | Added ResourceNameComparer.cs as a shared compile include |
| tests/Aspire.Hosting.Kubernetes.Tests/KubernetesPublisherTests.cs | Added test case for bait-and-switch pattern with port mapping validation |
|
@mitchdenny lets do this in 13.2 |
@mitchdenny could this be added to the 13.2 milestone so it isn't forgotten? |
mitchdenny
left a comment
There was a problem hiding this comment.
Deployment tests passed on the rebased branch (PR #14627 targeting release/13.2).
Results: 25/26 deployment jobs passed ✅. Both AKS-specific tests (AksStarterDeploymentTests and AksStarterWithRedisDeploymentTests) that exercise AddKubernetesEnvironment passed successfully. The single failure (AcaCompactNamingDeploymentTests) is an Azure Container Apps test unrelated to the Kubernetes publisher changes.
Local unit test results: All 13 tests in Aspire.Hosting.Kubernetes.Tests pass, including the new KubernetesMapsPortsForBaitAndSwitchResources test.
Code review: The fix correctly applies the ResourceNameComparer pattern (already used by Docker Compose, Azure App Service, and Azure Container Apps publishers) to the Kubernetes publisher dictionary. The change is minimal, correct, and well-tested.
|
I created a PR with these changes on it so we could run it against release/13.2 and our end to end deployment tests. There was an unrelated issue but this change worked out - so I'm approving and merging. Thanks @bbartels! |
Fixes #9226
Description
The Kubernetes publisher uses a
Dictionary<IResource, KubernetesResource>cache inKubernetesEnvironmentContextkeyed by object identity. WhenPublishAsDockerFile()replaces anExecutableResourcewith anExecutableContainerResource(the "bait and switch" pattern), other resources still holdEndpointReferenceobjects pointing to the original. The cache misses on the original vs replacement, creating twoKubernetesResourceobjects for the same logical resource, each allocating a different port from the sharedPortAllocator. This causes the referencing resource to get a different port than the one in the service/deployment YAML.The fix applies the same
ResourceNameComparerpattern already used by the Docker Compose, Azure App Service, and Azure Container Apps publishers.Checklist