⚠ This page is served via a proxy. Original site: https://github.com
This service does not collect credentials or authentication data.
Skip to content

Conversation

@dimamo5
Copy link

@dimamo5 dimamo5 commented Sep 21, 2025

This PR proposes to introduce a new AMI for Amazon Linux 2023 in the ARM 64 architecture. I took a similar approach to what was done for Ubuntu Jammy where the code is duplicated into a new Packer directory. I'm also open to parameterize the runner architecture and have a single Packer configuration.

@dimamo5 dimamo5 requested a review from a team as a code owner September 21, 2025 13:34
@dimamo5 dimamo5 changed the title Introduce Amazon Linux 2023 ARM image feat: Introduce Amazon Linux 2023 ARM image Sep 21, 2025
@dimamo5 dimamo5 force-pushed the amazon-linux-2023-arm branch from c9ca410 to 3d30fc0 Compare September 21, 2025 13:36
@npalm
Copy link
Member

npalm commented Sep 23, 2025

To me the example looks similar to al2023 for x64. Images are just an example and not tested at all. I have no issue with adding another example. But would it not be possible to make the al2023 more generic and just inject the architecture?

Any thoughts?

@dimamo5
Copy link
Author

dimamo5 commented Sep 26, 2025

But would it not be possible to make the al2023 more generic and just inject the architecture?

Made these changes in the latest commits. Seems to be working well in both architectures.

@dimamo5
Copy link
Author

dimamo5 commented Sep 30, 2025

@npalm when you have some free time, could you take a look?

## install the runner

s3_location=${S3_LOCATION_RUNNER_DISTRIBUTION}
architecture=${RUNNER_ARCHITECTURE}
Copy link
Member

Choose a reason for hiding this comment

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

What is the reason of removing this variable? Is it not used?

Copy link
Contributor

@guicaulada guicaulada Jan 16, 2026

Choose a reason for hiding this comment

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

Seems like it's not used, it was introduced in #1624 which added a condition:

if [[ "$architecture" == "arm64" ]]; then
  yum install -y libicu60
fi

But that was eventually removed by #3437 because libcu became needed in all linux architectures:

if [[ ! "$os_id" =~ ^ubuntu.* ]]; then
  dnf install -y libicu
fi

@npalm
Copy link
Member

npalm commented Oct 8, 2025

Thx for the PR, will not be able to able to get the PR merged in the next weeks. I will catch up end of the month. Sorry for tthe delay.

@github-actions
Copy link
Contributor

github-actions bot commented Jan 7, 2026

This pull request has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed if no further activity occurs. Thank you for your contributions.

@github-actions github-actions bot added the Stale label Jan 7, 2026
@npalm npalm removed the Stale label Jan 13, 2026
Copy link
Contributor

@guicaulada guicaulada left a comment

Choose a reason for hiding this comment

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

Both images built successfuly.

Since they are just examples I haven't tested them extensively, but I was able to configure and run the actions runner to connect to GitHub and listen for jobs.

@guicaulada guicaulada requested a review from npalm January 16, 2026 21:13
guicaulada
guicaulada previously approved these changes Jan 16, 2026
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 introduces ARM64 support for Amazon Linux 2023 by parameterizing the architecture in the Packer configuration. Unlike the Ubuntu Jammy implementation which uses separate directories for x64 and arm64, this approach uses a single Packer configuration with an architecture variable to support both architectures.

Changes:

  • Added an architecture variable to the AL2023 Packer configuration with validation for x64 and arm64 values
  • Updated AMI naming, instance type selection, source AMI filtering, and runner tarball URL to use the architecture variable
  • Removed unused RUNNER_ARCHITECTURE template parameter from install-runner.sh template instantiation

Reviewed changes

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

File Description
modules/runners/templates/install-runner.sh Removed unused architecture variable declaration
images/linux-al2023/github_agent.linux.pkr.hcl Added architecture parameterization for ARM64 support

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

source_ami_filter {
filters = {
name = "al2023-ami-2023.*-kernel-6.*-x86_64"
name = "al2023-ami-2023.*-kernel-6.*-${var.architecture == "x64" ? "x86_64" : var.architecture}"
Copy link

Copilot AI Jan 16, 2026

Choose a reason for hiding this comment

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

The ternary expression could be clearer. Consider extracting the architecture mapping to a local variable for better readability, e.g., locals { ami_architecture = var.architecture == \"x64\" ? \"x86_64\" : var.architecture }.

Copilot uses AI. Check for mistakes.
Copy link
Contributor

Choose a reason for hiding this comment

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

Good recommendation, but not necessary, local.ami_architecture and var.architecture could be confusing, if you decide to implement this suggestion I would use a clearer name for the local variable such as ami_name_architecture to make it clear that is the only place where x86_64 is being used.

Copy link
Contributor

@guicaulada guicaulada left a comment

Choose a reason for hiding this comment

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

Tested both images, they built successfully and registered as runners without issues.

The unused variable removal is a bit out of scope, but no harm done.

LGTM! Thanks for the contribution 🙌

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.

3 participants