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.

Sign up for GitHub

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

Jump to bottom

AWS EC2 partition number support #7777

Closed
wants to merge 4 commits into from
Closed

AWS EC2 partition number support #7777

wants to merge 4 commits into from

Conversation

david-cako
Copy link
Contributor

@david-cako david-cako commented Mar 1, 2019 •
edited
Loading

Changes proposed in this pull request:

  • Allow specifying placement_partition_number for AWS EC2 instances.
  • Includes @ys56's implementation of partition_count for aws_placement_group
  • Fixes TestAccAWSInstance_placementGroup (was not in functional state in forked commit -- instance no longer available and AMI incompatible)

Output from acceptance testing:

$ make testacc TESTARGS='-run=TestAccAWSInstance_placementGroup'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go1.11.5 test ./... -v -parallel 20 -run=TestAccAWSInstance_placementGroup -timeout 120m
?       github.com/terraform-providers/terraform-provider-aws   [no test files]
=== RUN   TestAccAWSInstance_placementGroup
=== PAUSE TestAccAWSInstance_placementGroup
=== CONT  TestAccAWSInstance_placementGroup
--- PASS: TestAccAWSInstance_placementGroup (141.24s)
PASS
ok      github.com/terraform-providers/terraform-provider-aws/aws       142.236s

...

ysfj and others added 4 commits February 21, 2019 21:48
add partition_count argument to aws_placement_group
9d4a689 
AWS added partition placement group feature which uses partition_count argument.
So I added partiton_count argument to set the number.
add partition number support
c211349
@david-cako
changelog
9b300a1
@david-cako
fix unit test
60d942f
@ghost ghost added size/M Managed by automation to categorize the size of a PR. documentation Introduces or discusses updates to documentation. service/ec2 Issues and PRs that pertain to the ec2 service. tests PRs: expanded test coverage. Issues: expanded coverage, enhancements to test infrastructure. labels Mar 1, 2019
@bflad bflad added the enhancement Requests to existing resources that expand the functionality or scope. label Mar 4, 2019
@bflad bflad mentioned this pull request Mar 4, 2019
Closed
@david-cako david-cako closed this Mar 6, 2019
@david-cako david-cako reopened this Mar 6, 2019
@spion06
Copy link

spion06 commented May 9, 2019

currently using this locally without any issues. +1

@aeschright aeschright requested a review from a team June 26, 2019 00:47
ziggythehamster
ziggythehamster reviewed Jul 17, 2019
View reviewed changes
aws/resource_aws_placement_group.go
partition_count := d.Get("partition_count").(int)

if strategy != "partition" && partition_count > 1 {
log.Printf("[WARN] partition_count is only valid for storategy partition for PlacementGroup")
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
log.Printf("[WARN] partition_count is only valid for storategy partition for PlacementGroup")
log.Printf("[WARN] partition_count is only valid for strategy partition for PlacementGroup")

@ziggythehamster
Copy link
Contributor

I updated this for the latest provider if you want to merge my changes (and also make sure I have the tests done correctly... I'm not a Go programmer): https://github.com/art19/terraform-provider-aws/tree/art19-v2.53.0

I wonder how we get Hashicorp to notice this PR?

@alanbrent alanbrent mentioned this pull request Sep 17, 2020
Closed
@sean-
Copy link
Contributor

sean- commented Sep 28, 2020

FYI, there is another implementation of this feature that is tightly scoped to just the placement groups in #15360

Base automatically changed from master to main January 23, 2021 00:55
@breathingdust breathingdust requested a review from a team as a code owner January 23, 2021 00:55
@ewbankkit
Copy link
Contributor

@david-cako Thanks for the contribution 🎉 👏.
I have incorporated these changes into #15360.

@ewbankkit ewbankkit closed this Oct 13, 2021
@github-actions
Copy link

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 16, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@ziggythehamster ziggythehamster ziggythehamster left review comments

Assignees
No one assigned
Labels
documentation Introduces or discusses updates to documentation. enhancement Requests to existing resources that expand the functionality or scope. 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
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@david-cako @spion06 @ziggythehamster @sean- @ewbankkit @bflad