-
Notifications
You must be signed in to change notification settings - Fork 701
feat: Introduce Amazon Linux 2023 ARM image #4780
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
c9ca410 to
3d30fc0
Compare
|
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? |
Made these changes in the latest commits. Seems to be working well in both architectures. |
|
@npalm when you have some free time, could you take a look? |
| ## install the runner | ||
|
|
||
| s3_location=${S3_LOCATION_RUNNER_DISTRIBUTION} | ||
| architecture=${RUNNER_ARCHITECTURE} |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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
|
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. |
|
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. |
guicaulada
left a comment
There was a problem hiding this 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.
There was a problem hiding this 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
architecturevariable to the AL2023 Packer configuration with validation forx64andarm64values - Updated AMI naming, instance type selection, source AMI filtering, and runner tarball URL to use the architecture variable
- Removed unused
RUNNER_ARCHITECTUREtemplate 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}" |
Copilot
AI
Jan 16, 2026
There was a problem hiding this comment.
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 }.
There was a problem hiding this comment.
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.
There was a problem hiding this 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 🙌
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.