Skip to content
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

Add support for arm64_mac architecture #487

Merged

Conversation

jaysoffian
Copy link
Contributor

No description provided.

@jaysoffian jaysoffian requested a review from a team as a code owner June 11, 2024 08:10
Copy link

hashicorp-cla-app bot commented Jun 11, 2024

CLA assistant check
All committers have signed the CLA.

Copy link
Contributor

@lbajolet-hashicorp lbajolet-hashicorp left a comment

Choose a reason for hiding this comment

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

Hi @jaysoffian,

Thanks for opening this PR!

The approach looks good to me, I left some questions/remarks on the code for now, mostly because the Architecture was not updated for all the builders/post-processors (not sure if they all support it though), but aside from that, the code looks good to me.

I'll let you circle back later and we can do another pass on that.

builder/chroot/builder.go Show resolved Hide resolved
builder/chroot/builder.go Show resolved Hide resolved
@jaysoffian
Copy link
Contributor Author

The approach looks good to me, I left some questions/remarks on the code for now, mostly because the Architecture was not updated for all the builders/post-processors (not sure if they all support it though), but aside from that, the code looks good to me.

As far as the builders go, this PR updates chroot and ebssurrogate which are the only builders where you'd potentially mount an AMI from a different architecture than the host, so they are the only ones that have an architecture configuration item. As far as the other builders:

  • ebs: customizes the running AMI,so the architecture stays the same as the host.
  • instance: not compatible with macOS EC2 instances which can only boot from an EBS volume.
  • ebsvolume: doesn't build an AMI.

With respect to post-processors, this plugin has only the import plugin:

The Packer Amazon Import post-processor takes an OVA artifact from various builders and imports it to an AMI available to Amazon Web Services EC2.

Amazon requires EC2 Mac instances to boot from one of their AMIs (or a customized version thereof). I don't think you can start with an arbitrary OVA artifact. So I don't think supporting the two Mac architectures in the import post-processor makes sense.

Copy link
Contributor

@lbajolet-hashicorp lbajolet-hashicorp left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for this @jaysoffian!

@lbajolet-hashicorp lbajolet-hashicorp merged commit 3c9034f into hashicorp:main Jun 20, 2024
12 checks passed
@jaysoffian jaysoffian deleted the add-arm64_mac-architecture branch June 20, 2024 16:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants