Skip to content

Conversation

danegsta
Copy link
Member

A recent orchestrator update changed how we autodetect container runtimes, so the workaround of forcing the runtime should no longer be necessary.

@Copilot Copilot AI review requested due to automatic review settings May 15, 2025 20:41
@danegsta danegsta requested review from radical and eerhardt as code owners May 15, 2025 20:41
@github-actions github-actions bot added the area-engineering-systems infrastructure helix infra engineering repo stuff label May 15, 2025
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR removes an outdated workaround for explicitly setting the container runtime in test environments, as the orchestrator update now supports autodetection.

  • Removed the forced container runtime command from the Helix test project.
  • Removed the environment variable overrides from the BuildAndTest pipeline and GitHub workflows.

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
tests/helix/send-to-helix-inner.proj Removed the explicit container runtime setting and its comment
eng/pipelines/templates/BuildAndTest.yml Removed the container runtime environment variable override
.github/workflows/run-tests.yml Removed the container runtime environment variable override from multiple job definitions
Comments suppressed due to low confidence (4)

tests/helix/send-to-helix-inner.proj:155

  • Removal of the explicit container runtime setting is correct given the orchestrator update. Please verify that tests in environments relying on autodetection behave as expected.
<!-- Force container runtime to Docker since that's what is installed on Helix -->

eng/pipelines/templates/BuildAndTest.yml:108

  • The removal of the forced container runtime in the pipeline environment variables is aligned with the updated orchestrator behavior. Consider confirming that the build and test scripts work correctly across all supported platforms.
DOTNET_ASPIRE_CONTAINER_RUNTIME: docker

.github/workflows/run-tests.yml:215

  • Consistent removal of the container runtime override from the workflow is appropriate with the orchestrator update. Ensure that the workflow tests remain stable without this variable.
DOTNET_ASPIRE_CONTAINER_RUNTIME: docker

.github/workflows/run-tests.yml:237

  • The removal of the runtime override in this job is consistent with the intended changes. Confirm that no dependency on the explicit variable exists in any downstream steps.
DOTNET_ASPIRE_CONTAINER_RUNTIME: docker

@karolz-ms karolz-ms merged commit c9f02da into dotnet:main May 15, 2025
255 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Jun 15, 2025
@danegsta danegsta deleted the dev/danegsta/removeRuntimeWorkaround branch July 17, 2025 17:43
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-engineering-systems infrastructure helix infra engineering repo stuff
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants