-
Notifications
You must be signed in to change notification settings - Fork 9.1k
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_ebs_volume: Always set all attributes in Terraform state, suppress difference for iops = 0 with non-io1 type #8343
Conversation
…, suppress difference for iops with non-io1 type The Terraform 0.12 Provider SDK acceptance testing framework was picking up the lack of the `iops` attribute being set during `ImportStateVerify` testing. Here we prefer to always set all attributes into the Terraform state and ignore the difference for the `iops` attribute as necessary. Previous output from Terraform 0.12 Provider SDK acceptance testing: ``` --- FAIL: TestAccAWSEBSVolume_basic (15.45s) testing.go:568: Step 0 error: Check failed: Check 4/8 error: aws_ebs_volume.test: Attribute 'iops' found when not expected --- FAIL: TestAccAWSEBSVolume_kmsKey (38.24s) testing.go:568: Step 1 error: ImportStateVerify attributes not equivalent. Difference is shown below. Top is actual, bottom is expected. (map[string]string) { } (map[string]string) (len=1) { (string) (len=4) "iops": (string) (len=1) "0" } --- FAIL: TestAccAWSEBSVolume_updateAttachedEbsVolume (127.89s) testing.go:568: Step 1 error: ImportStateVerify attributes not equivalent. Difference is shown below. Top is actual, bottom is expected. (map[string]string) { } (map[string]string) (len=1) { (string) (len=4) "iops": (string) (len=1) "0" } --- FAIL: TestAccAWSEBSVolume_updateSize (19.08s) testing.go:568: Step 1 error: ImportStateVerify attributes not equivalent. Difference is shown below. Top is actual, bottom is expected. (map[string]string) { } (map[string]string) (len=1) { (string) (len=4) "iops": (string) (len=1) "0" } --- FAIL: TestAccAWSEBSVolume_updateType (19.01s) testing.go:568: Step 1 error: ImportStateVerify attributes not equivalent. Difference is shown below. Top is actual, bottom is expected. (map[string]string) { } (map[string]string) (len=1) { (string) (len=4) "iops": (string) (len=1) "0" } --- FAIL: TestAccAWSEBSVolume_withTags (19.11s) testing.go:568: Step 1 error: ImportStateVerify attributes not equivalent. Difference is shown below. Top is actual, bottom is expected. (map[string]string) { } (map[string]string) (len=1) { (string) (len=4) "iops": (string) (len=1) "0" } ``` Output from Terraform 0.12 Provider SDK acceptance testing: ``` --- PASS: TestAccAWSEBSVolume_NoIops (24.99s) --- PASS: TestAccAWSEBSVolume_basic (25.08s) --- PASS: TestAccAWSEBSVolume_withTags (25.21s) --- PASS: TestAccAWSEBSVolume_updateIops (44.99s) --- PASS: TestAccAWSEBSVolume_updateSize (45.46s) --- PASS: TestAccAWSEBSVolume_updateType (45.51s) --- PASS: TestAccAWSEBSVolume_kmsKey (55.31s) --- PASS: TestAccAWSEBSVolume_updateAttachedEbsVolume (193.59s) ```
@@ -44,6 +44,9 @@ func resourceAwsEbsVolume() *schema.Resource { | |||
Type: schema.TypeInt, | |||
Optional: true, | |||
Computed: true, | |||
DiffSuppressFunc: func(k, old, new string, d *schema.ResourceData) 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.
This is to address the comment below pertaining to iops
values for non Io1 volume types, correct?
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.
This allows operators to continue using syntax like the below in Terraform 0.11 with modules 👍
variable "iops" {
default = "0"
}
variable "type" {
default = "standard"
}
resource "aws_ebs_volume" "example" {
# ... other configuration ...
iops = "${var.iops}"
type = "${var.type}"
}
module "example1" {
# ...
iops = "1000"
type = "io1"
}
module "example2" {
# ...
type = "gp2"
}
Selectively choosing when to save the information into the Terraform state was a workaround to suppressing the difference in certain cases, although the selectiveness itself can cause issues after updates (e.g. not refreshing attributes appropriately).
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.
LGTM 👍 👍
I left a question more for my understanding of the situation.
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! |
Community Note
The Terraform 0.12 Provider SDK acceptance testing framework was picking up the lack of the
iops
attribute being set duringImportStateVerify
testing. Here we prefer to always set all attributes into the Terraform state and ignore the difference for theiops
attribute as necessary.Previous output from Terraform 0.12 Provider SDK acceptance testing:
Output from Terraform 0.12 Provider SDK acceptance testing: