Skip to content

Fix regression of ARM instance startup#60

Merged
ethanholz merged 2 commits intoomsf:mainfrom
ethanholz:fix/regression-arm
Jul 9, 2025
Merged

Fix regression of ARM instance startup#60
ethanholz merged 2 commits intoomsf:mainfrom
ethanholz:fix/regression-arm

Conversation

@ethanholz
Copy link
Contributor

Closes #59
This pull request introduces a minor enhancement to support configurable runner architecture and updates the corresponding test cases to reflect this change. The most important changes include adding support for dynamic architecture selection in the clouddeployment.py file and ensuring the test cases validate this new functionality.

Enhancements to runner architecture configuration:

  • src/gha_runner/clouddeployment.py: Modified the __post_init__ method to dynamically determine the runner architecture from cloud_params (defaulting to "x64") and pass it to the get_latest_runner_release method.

Updates to test cases:

  • tests/test_clouddeployment.py:
    • Updated test_deploy_instance_creation to verify that get_latest_runner_release is called with the correct architecture parameter.
    • Fixed formatting in test_teardown_instance_failure to align the expected output list for readability.
    • Added a blank line for readability in the get_instance_mapping method.

@ethanholz ethanholz requested a review from Copilot July 9, 2025 15:37
@ethanholz ethanholz self-assigned this Jul 9, 2025
Copy link
Contributor

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 fixes a regression by enabling dynamic runner architecture selection and updates tests to validate the change.

  • Dynamically reads an arch key from cloud_params (defaulting to "x64") in __post_init__
  • Passes the chosen architecture to get_latest_runner_release
  • Updates tests to assert the new parameter and refactors formatting in teardown tests

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
src/gha_runner/clouddeployment.py Added dynamic architecture lookup in __post_init__
tests/test_clouddeployment.py Enhanced tests to assert architecture arg and improved formatting
Comments suppressed due to low confidence (3)

src/gha_runner/clouddeployment.py:150

  • [nitpick] Consider renaming the cloud_params key from 'arch' to 'architecture' for consistency with the parameter name and improve clarity.
        architecture = self.cloud_params.get("arch", "x64")

src/gha_runner/clouddeployment.py:140

  • Update the docstring of post_init to document the new cloud_params key 'arch' and explain how it controls runner architecture selection.
    def __post_init__(self):

tests/test_clouddeployment.py:77

  • Add a test case for a non-default architecture value to ensure custom 'arch' values in cloud_params are passed correctly to get_latest_runner_release.
    gh_mock.get_latest_runner_release.assert_called_once_with(

@ethanholz ethanholz requested a review from dwhswenson July 9, 2025 15:40
Copy link
Member

@dwhswenson dwhswenson left a comment

Choose a reason for hiding this comment

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

LGTM. Only question is whether it might be worth adding a test with non-default OS/arch, to make sure we can explicitly set it. But I don't see that as a necessary blocker, just a potential improvement.

@ethanholz ethanholz merged commit e169fe5 into omsf:main Jul 9, 2025
4 checks passed
@ethanholz ethanholz deleted the fix/regression-arm branch July 9, 2025 17:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Regression with using ARM instance types

3 participants