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

Modify root volume size without instance recreation #1396

Closed
wants to merge 2 commits into from
Closed

Modify root volume size without instance recreation #1396

wants to merge 2 commits into from

Conversation

viscat
Copy link
Contributor

@viscat viscat commented Aug 12, 2017 •
edited
Loading

This is my first PR to Terraform, and I guess it will not be the last one. Terraform (also Packer) helped me a lot, and I think it's time to contribute a little :)

Using the recently AWS feature to modify root volume size without forcing new instance, I added a call to AWS API to modify it on the fly. I added an acceptance test to check the volume size update is done without instance recreation.

Fixes #768

@viscat viscat force-pushed the resource_aws_instance/root_volume_size_update branch from f15a467 to bbcbcf1 Compare August 12, 2017 20:15
@radeksimko radeksimko added the enhancement Requests to existing resources that expand the functionality or scope. label Aug 14, 2017
@ricardclau
Copy link

Hi @radeksimko any news here?

We have tested this with our AWS setup and it seems to work fine, not sure if there is anything missing in the PR for you guys to review and merge

@mbelang
Copy link

mbelang commented Oct 25, 2017

Any news on this? We have some needs for that feature.

radeksimko
radeksimko reviewed Oct 27, 2017
View reviewed changes
Copy link
Member

@radeksimko radeksimko left a comment

Choose a reason for hiding this comment

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

Hi @viscat
thanks for raising the PR and attaching an acceptance test. Sorry it took us so long to get to this PR.

I left you some comments there, the most important one is about matching the right volume.

aws/resource_aws_instance.go Outdated Show resolved Hide resolved
aws/resource_aws_instance.go Outdated Show resolved Hide resolved
aws/resource_aws_instance.go Outdated Show resolved Hide resolved
aws/resource_aws_instance.go Outdated Show resolved Hide resolved
aws/resource_aws_instance_test.go Show resolved Hide resolved
aws/resource_aws_instance.go Outdated Show resolved Hide resolved
@radeksimko radeksimko added the waiting-response Maintainers are waiting on response from community or contributor. label Oct 27, 2017
@viscat
Copy link
Contributor Author

viscat commented Oct 30, 2017

Thank you very much @radeksimko for all the feedback. I will also check the conflicts with master branch. I will fix and improve what you pointed out as soon as possible.

@radeksimko radeksimko added the size/L Managed by automation to categorize the size of a PR. label Nov 15, 2017
@johntdyer
Copy link

Any update on merging this ?

@radeksimko
Copy link
Member

👋 @viscat
do you think you'll have time to look at this PR or are you happy for anyone else to pick it up?

Thanks.

@radeksimko radeksimko added the service/ec2 Issues and PRs that pertain to the ec2 service. label Jan 12, 2018
@bflad bflad mentioned this pull request Mar 9, 2018
Closed
@bflad
Copy link
Contributor

bflad commented Mar 9, 2018

Hi @viscat 👋 will you have time to implement the feedback above? If not, just let us know. Thanks.

@siliconavengers
Copy link

I hope this PR will be merged soon

@viscat
Copy link
Contributor Author

viscat commented Mar 13, 2018

Sorry @bflad right now I can't do it.

@viscat
resource_aws_instance: Modify root volume size without instance recre…
5948d65 
…ation

Fixes: hashicorp#768

make testacc TEST=./aws
TESTARGS='-run=TestAccAWSInstance_changeRootBlockDeviceVolumeSize'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v
-run=TestAccAWSInstance_changeRootBlockDeviceVolumeSize -timeout 120m
=== RUN   TestAccAWSInstance_changeRootBlockDeviceVolumeSize
--- PASS: TestAccAWSInstance_changeRootBlockDeviceVolumeSize (514.63s)
PASS
ok    github.com/terraform-providers/terraform-provider-aws/aws 514.642s
@viscat viscat force-pushed the resource_aws_instance/root_volume_size_update branch from bbcbcf1 to 5948d65 Compare May 1, 2018 23:22
@ghost ghost added the size/L Managed by automation to categorize the size of a PR. label May 1, 2018
@viscat
Copy link
Contributor Author

viscat commented May 2, 2018

@radeksimko @bflad Sorry for taking so long to take it again. I made the fixes and improved that you requested. Thank you for the feedback! The only thing I am not able to figure out is how to determine which is the RBD. I have been looking the AWS API doc and I can't find anything. Can you help me with this please? I can try to get in touch with AWS support anyways.

@bflad bflad removed the waiting-response Maintainers are waiting on response from community or contributor. label May 9, 2018
@viscat
Copy link
Contributor Author

viscat commented Nov 27, 2018

@radeksimko Can we merge this?

@aeschright aeschright requested a review from a team June 25, 2019 18:44
@dimisjim
Copy link
Contributor

dimisjim commented Dec 9, 2019

@bflad Hey guys, what's stopping us from merging this?

@gdavison
Copy link
Contributor

Hi @viscat, thanks for your patience. I completely understand if you're not wanting to continue working on this, and we can take it over if you want.

If you are still interested, I have two requests:

  • It looks like some of our internal function have changed their interfaces, and this PR no longer builds against the current provider code. Can you rebase it, please?
  • If there are multiple EBS volumes attached to an instance, how can we guarantee that we're getting the correct volume for resizing? We might be able to use the volume id in the state.

@gdavison gdavison added the waiting-response Maintainers are waiting on response from community or contributor. label Mar 13, 2020
@gdavison gdavison self-assigned this Mar 13, 2020
@gdavison gdavison mentioned this pull request Apr 1, 2020
Merged
@gdavison
Copy link
Contributor

gdavison commented Apr 1, 2020

I've created #12620 to continue this work

@gdavison gdavison added this to the v2.58.0 milestone Apr 16, 2020
@ghost
Copy link

ghost commented Apr 17, 2020

This has been released in version 2.58.0 of the Terraform AWS provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading.

For further feature requests or bug reports with this functionality, please create a new GitHub issue following the template for triage. Thanks!

@ghost
Copy link

ghost commented May 17, 2020

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. Thanks!

@ghost ghost locked and limited conversation to collaborators May 17, 2020
@breathingdust breathingdust removed the waiting-response Maintainers are waiting on response from community or contributor. label Sep 17, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@radeksimko radeksimko radeksimko left review comments

Assignees

@gdavison gdavison

Labels
enhancement Requests to existing resources that expand the functionality or scope. service/ec2 Issues and PRs that pertain to the ec2 service. size/L Managed by automation to categorize the size of a PR.
Projects
None yet
Milestone
v2.58.0
Development

Successfully merging this pull request may close these issues.

Changing root_block_device.volume_size forces new resource (aws_instance)
10 participants
@viscat @ricardclau @mbelang @johntdyer @radeksimko @bflad @siliconavengers @dimisjim @gdavison @breathingdust