-
Notifications
You must be signed in to change notification settings - Fork 9.5k
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
provider/aws: Error when unable to find a Root Block Device name #2646
Conversation
cc @phinze to double check that erring there is reasonable |
@@ -760,6 +760,10 @@ func fetchRootDeviceName(ami string, conn *ec2.EC2) (*string, error) { | |||
rootDeviceName = image.BlockDeviceMappings[0].DeviceName | |||
} | |||
|
|||
if rootDeviceName == nil { | |||
return nil, fmt.Errorf("[WARN] Error finding Root Device Name for AMI (%s)", ami) |
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.
In the linked issue the problem was a bad AMI ID, which means we would have hit the early nil return up on L735.
We specifically added that "no error when AMI not found" thingy in #1250 to fix unrecoverable error states because it was being hit in Read
.
So we need to figure out how we can maintain the "allow users to correct bad AMIs" while still getting a specific error message when they specify one.
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.
In the linked issue the problem was a bad AMI ID, which means we would have hit the early nil return up on L735.
Let's sync up to discuss. The issue I was debugging (and the only way I could repo the source issue) did not trigger L735; there was an image, but there was no root device matching in the lines above this. I couldn't use an AMI that didn't exist; TF returned an error before this point. I'll double check though...
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.
For example, when I use a bad AMI in this context (Launch Configuration):
* InvalidAMIID.Malformed: Invalid id: "ami-nopenope"
status code: 400, request id: []
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.
@phinze did you see my updates here? Hopefully they made sense, let's sync up and see what we need to do
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.
@catsby ah okay. so if we can verify that these steps still work:
- create ami
- create instance from AMI
- delete ami
- terraform refresh should not error
If that still is the case, then we're good to go.
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.
terraform refresh should not error
It does not error... but a Launch Configuration will show a -/+
, since the AMI no longer exists ...
Commented inline - I think we'll need to dig into this. |
468f9ad
to
99f9b93
Compare
Pulling this in |
provider/aws: Error when unable to find a Root Block Device name
This change makes it impossible to use instance-store backed Amazon Linux images since they don't have a root device name on them. Please consider rolling this change back and finding another solution for #2633. |
Hey @ikalinin – sorry for the trouble here! Do you have an example configuration for me to test out? I think we can avoid this error by checking the I was able to reproduce the error using AMI |
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 have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further. |
Fixes #2633 by returning an error if we can't find a Root Block Device
With a config like this (using an ami that is Instance backed):
Now produces error:
Instead of the error shown in #2633