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

fix(ec2_instance): adding placement_group_id to aws_instance resource for shared placement groups from different account #38527

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

jracollins
Copy link

@jracollins jracollins commented Jul 24, 2024

Recently when sharing a placement group to another AWS account, and trying to launch EC2s via aws_instance or an ASG with a launch template with the placement group name provided, I ran into the following error:

InvalidPlacementGroup.Unkown: The placement group 'xxxx' is unknown

I could create manually in the UI using the same shared placement group causing the error, and after opening a support request it was confirmed from AWS that referencing the placement group by name is only possible when used in the account that created the placement group.

A simple cut down example of how I encountered it is below:

resource "aws_placement_group" "local" {
  name     = "test"
  strategy = "cluster"
}

resource "aws_instance" "test" {
  ami           = var.ami
  instance_type = var.instance_type

  placement_group = "shared_placement_group_name"
  # placement_group = aws_placement_group.local.name # works  

  launch_template {
    id      = var.launch_template_id
    version = var.launch_template_id.version
  }
}

I have looked through the provider code and can see it only references by name, and have made this PR that should resolve it for the aws_instance resource use case.

However the AWS Go SDK does not actually support the placement group ID field in all places where it would be needed (ie launch templates, spot requests) so it is not as simple or easy as the small changes I've made here.

I have left a commented section in this PR where the error happens in a spot request within this single resource.

It would be great if it could be merged with just this small change and adding instance_market_options to the ConflictsWith to protect against any errors until the AWS Go SDK version gets bumped/supports it for all the other resources.

Closes #38908

@jracollins jracollins requested a review from a team as a code owner July 24, 2024 23:17
Copy link

Community Note

Voting for Prioritization

  • Please vote on this pull request by adding a 👍 reaction to the original post to help the community and maintainers prioritize this pull request.
  • Please see our prioritization guide for information on how we prioritize.
  • Please do not leave "+1" or other comments that do not add relevant new information or questions, they generate extra noise for issue followers and do not help prioritize the request.

For Submitters

  • Review the contribution guide relating to the type of change you are making to ensure all of the necessary steps have been taken.
  • For new resources and data sources, use skaff to generate scaffolding with comments detailing common expectations.
  • Whether or not the branch has been rebased will not impact prioritization, but doing so is always a welcome surprise.

@terraform-aws-provider terraform-aws-provider bot added tests PRs: expanded test coverage. Issues: expanded coverage, enhancements to test infrastructure. service/ec2 Issues and PRs that pertain to the ec2 service. size/M Managed by automation to categorize the size of a PR. labels Jul 24, 2024
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Welcome @jracollins 👋

It looks like this is your first Pull Request submission to the Terraform AWS Provider! If you haven’t already done so please make sure you have checked out our CONTRIBUTOR guide and FAQ to make sure your contribution is adhering to best practice and has all the necessary elements in place for a successful approval.

Also take a look at our FAQ which details how we prioritize Pull Requests for inclusion.

Thanks again, and welcome to the community! 😃

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
service/ec2 Issues and PRs that pertain to the ec2 service. size/M Managed by automation to categorize the size of a PR. tests PRs: expanded test coverage. Issues: expanded coverage, enhancements to test infrastructure.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Enhancement]: EC2 placement groups can now be shared across multiple AWS accounts
1 participant