-
Notifications
You must be signed in to change notification settings - Fork 9k
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
resource/aws_launch_configuration: Added support for no_device #2132
Conversation
Any estimate on a pull? Should I contact someone? Could really use this in the next release. |
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.
Hi @maxfortun
thanks for the PR.
Besides my inline question do you mind adding this new field to any acceptance test? Feel free to create a new one if it's easiest.
ebs := &autoscaling.Ebs{} | ||
|
||
if v, ok := bd["no_device"].(bool); !ok && v { | ||
ebs.DeleteOnTermination = aws.Bool(bd["delete_on_termination"].(bool)) |
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.
I'm not sure I follow what's happening here. Was this supposed to be a fix of existing code? i.e.
if v, ok := bd["delete_on_termination"].(bool); ok {
ebs.DeleteOnTermination = aws.Bool(v)
if not then can you please explain why can't we just leave the validation of potentially conflicting fields to the API?
Hi @maxfortun It's totally 👌 if not - in which case would you be happy for someone else to pick it up? Thanks. |
Sorry, don't have the time to finish this right now. And no, I do not mind at all if someone takes over and finishes this enhancement. Found a bug in this too. The state does not retain the change to no_device. This needs to be fixed as well. |
This has been released in version 1.14.0 of the AWS provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading. |
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! |
http://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-properties-ec2-blockdev-mapping.html
Launch configs need ability to pass NoDevice for AMIs that come preconfigured with partitions.