From 34c7bbcf4da63a6e5a1a611d89d7c642d76e9859 Mon Sep 17 00:00:00 2001 From: Paul Hinze Date: Tue, 7 Apr 2015 17:05:00 -0500 Subject: [PATCH] providers/aws: reduce scope of block_device set hashcodes Fixes #1409 Resource set hash calculation is a bit of a devil's bargain when it comes to optional, computed attributes. If you omit the optional, computed attribute from the hash function, changing it in an existing config is not properly detected. If you include the optional, computed attribute in the hash and do not specify a value for it in the config, then you'll end up with a perpetual, unresolvable diff. We'll need to think about how to get the best of both worlds, here, but for now I'm switching us to the latter and documenting the fact that changing these attributes requires manual `terraform taint` to apply. --- builtin/providers/aws/resource_aws_instance.go | 18 ++---------------- .../aws/resource_aws_instance_test.go | 18 +++++++++--------- .../aws/resource_aws_launch_configuration.go | 17 ++--------------- .../providers/aws/r/instance.html.markdown | 13 ++++--------- .../aws/r/launch_config.html.markdown | 5 +++++ 5 files changed, 22 insertions(+), 49 deletions(-) diff --git a/builtin/providers/aws/resource_aws_instance.go b/builtin/providers/aws/resource_aws_instance.go index 69c627862..001247005 100644 --- a/builtin/providers/aws/resource_aws_instance.go +++ b/builtin/providers/aws/resource_aws_instance.go @@ -204,16 +204,8 @@ func resourceAwsInstance() *schema.Resource { Set: func(v interface{}) int { var buf bytes.Buffer m := v.(map[string]interface{}) - buf.WriteString(fmt.Sprintf("%t-", m["delete_on_termination"].(bool))) buf.WriteString(fmt.Sprintf("%s-", m["device_name"].(string))) - buf.WriteString(fmt.Sprintf("%t-", m["encrypted"].(bool))) - // NOTE: Not considering IOPS in hash; when using gp2, IOPS can come - // back set to something like "33", which throws off the set - // calculation and generates an unresolvable diff. - // buf.WriteString(fmt.Sprintf("%d-", m["iops"].(int))) buf.WriteString(fmt.Sprintf("%s-", m["snapshot_id"].(string))) - buf.WriteString(fmt.Sprintf("%d-", m["volume_size"].(int))) - buf.WriteString(fmt.Sprintf("%s-", m["volume_type"].(string))) return hashcode.String(buf.String()) }, }, @@ -288,14 +280,8 @@ func resourceAwsInstance() *schema.Resource { }, }, Set: func(v interface{}) int { - var buf bytes.Buffer - m := v.(map[string]interface{}) - buf.WriteString(fmt.Sprintf("%t-", m["delete_on_termination"].(bool))) - // See the NOTE in "ebs_block_device" for why we skip iops here. - // buf.WriteString(fmt.Sprintf("%d-", m["iops"].(int))) - buf.WriteString(fmt.Sprintf("%d-", m["volume_size"].(int))) - buf.WriteString(fmt.Sprintf("%s-", m["volume_type"].(string))) - return hashcode.String(buf.String()) + // there can be only one root device; no need to hash anything + return 0 }, }, }, diff --git a/builtin/providers/aws/resource_aws_instance_test.go b/builtin/providers/aws/resource_aws_instance_test.go index fab8aeed1..ed2a3c508 100644 --- a/builtin/providers/aws/resource_aws_instance_test.go +++ b/builtin/providers/aws/resource_aws_instance_test.go @@ -140,25 +140,25 @@ func TestAccAWSInstance_blockDevices(t *testing.T) { resource.TestCheckResourceAttr( "aws_instance.foo", "root_block_device.#", "1"), resource.TestCheckResourceAttr( - "aws_instance.foo", "root_block_device.1023169747.volume_size", "11"), + "aws_instance.foo", "root_block_device.0.volume_size", "11"), resource.TestCheckResourceAttr( - "aws_instance.foo", "root_block_device.1023169747.volume_type", "gp2"), + "aws_instance.foo", "root_block_device.0.volume_type", "gp2"), resource.TestCheckResourceAttr( "aws_instance.foo", "ebs_block_device.#", "2"), resource.TestCheckResourceAttr( - "aws_instance.foo", "ebs_block_device.2225977507.device_name", "/dev/sdb"), + "aws_instance.foo", "ebs_block_device.2576023345.device_name", "/dev/sdb"), resource.TestCheckResourceAttr( - "aws_instance.foo", "ebs_block_device.2225977507.volume_size", "9"), + "aws_instance.foo", "ebs_block_device.2576023345.volume_size", "9"), resource.TestCheckResourceAttr( - "aws_instance.foo", "ebs_block_device.2225977507.volume_type", "standard"), + "aws_instance.foo", "ebs_block_device.2576023345.volume_type", "standard"), resource.TestCheckResourceAttr( - "aws_instance.foo", "ebs_block_device.1977224956.device_name", "/dev/sdc"), + "aws_instance.foo", "ebs_block_device.2554893574.device_name", "/dev/sdc"), resource.TestCheckResourceAttr( - "aws_instance.foo", "ebs_block_device.1977224956.volume_size", "10"), + "aws_instance.foo", "ebs_block_device.2554893574.volume_size", "10"), resource.TestCheckResourceAttr( - "aws_instance.foo", "ebs_block_device.1977224956.volume_type", "io1"), + "aws_instance.foo", "ebs_block_device.2554893574.volume_type", "io1"), resource.TestCheckResourceAttr( - "aws_instance.foo", "ebs_block_device.1977224956.iops", "100"), + "aws_instance.foo", "ebs_block_device.2554893574.iops", "100"), resource.TestCheckResourceAttr( "aws_instance.foo", "ephemeral_block_device.#", "1"), resource.TestCheckResourceAttr( diff --git a/builtin/providers/aws/resource_aws_launch_configuration.go b/builtin/providers/aws/resource_aws_launch_configuration.go index dc5d3e8e4..4ee258fca 100644 --- a/builtin/providers/aws/resource_aws_launch_configuration.go +++ b/builtin/providers/aws/resource_aws_launch_configuration.go @@ -158,15 +158,8 @@ func resourceAwsLaunchConfiguration() *schema.Resource { Set: func(v interface{}) int { var buf bytes.Buffer m := v.(map[string]interface{}) - buf.WriteString(fmt.Sprintf("%t-", m["delete_on_termination"].(bool))) buf.WriteString(fmt.Sprintf("%s-", m["device_name"].(string))) - // NOTE: Not considering IOPS in hash; when using gp2, IOPS can come - // back set to something like "33", which throws off the set - // calculation and generates an unresolvable diff. - // buf.WriteString(fmt.Sprintf("%d-", m["iops"].(int))) buf.WriteString(fmt.Sprintf("%s-", m["snapshot_id"].(string))) - buf.WriteString(fmt.Sprintf("%d-", m["volume_size"].(int))) - buf.WriteString(fmt.Sprintf("%s-", m["volume_type"].(string))) return hashcode.String(buf.String()) }, }, @@ -240,14 +233,8 @@ func resourceAwsLaunchConfiguration() *schema.Resource { }, }, Set: func(v interface{}) int { - var buf bytes.Buffer - m := v.(map[string]interface{}) - buf.WriteString(fmt.Sprintf("%t-", m["delete_on_termination"].(bool))) - // See the NOTE in "ebs_block_device" for why we skip iops here. - // buf.WriteString(fmt.Sprintf("%d-", m["iops"].(int))) - buf.WriteString(fmt.Sprintf("%d-", m["volume_size"].(int))) - buf.WriteString(fmt.Sprintf("%s-", m["volume_type"].(string))) - return hashcode.String(buf.String()) + // there can be only one root device; no need to hash anything + return 0 }, }, }, diff --git a/website/source/docs/providers/aws/r/instance.html.markdown b/website/source/docs/providers/aws/r/instance.html.markdown index 07189565a..672042419 100644 --- a/website/source/docs/providers/aws/r/instance.html.markdown +++ b/website/source/docs/providers/aws/r/instance.html.markdown @@ -109,15 +109,10 @@ list](http://docs.aws.amazon.com/AWSEC2/latest/UserGuide/InstanceStorage.html#St of which ephemeral devices are available on each type. The devices are always identified by the `virtual_name` in the format `"ephemeral{0..N}"`. - -~> **NOTE:** Because AWS [does not expose Instance Store mapping -details](http://docs.aws.amazon.com/AWSEC2/latest/UserGuide/block-device-mapping-concepts.html#bdm-instance-metadata) -via an externally accessible API, `ephemeral_block_device` configuration may -only be applied at instance creation time, and changes to configuration of -existing resources cannot be detected by Terraform. Updates to Instance Store -block device configuration can be manually triggered by using the [`taint` -command](/docs/commands/taint.html). - +~> **NOTE:** Currently, changes to `*_block_device` configuration of _existing_ +resources cannot be automatically detected by Terraform. After making updates +to block device configuration, resource recreation can be manually triggered by +using the [`taint` command](/docs/commands/taint.html). ## Attributes Reference diff --git a/website/source/docs/providers/aws/r/launch_config.html.markdown b/website/source/docs/providers/aws/r/launch_config.html.markdown index 5a80a97b8..29c9fbdbc 100644 --- a/website/source/docs/providers/aws/r/launch_config.html.markdown +++ b/website/source/docs/providers/aws/r/launch_config.html.markdown @@ -85,6 +85,11 @@ list](http://docs.aws.amazon.com/AWSEC2/latest/UserGuide/InstanceStorage.html#St of which ephemeral devices are available on each type. The devices are always identified by the `virtual_name` in the format `"ephemeral{0..N}"`. +~> **NOTE:** Changes to `*_block_device` configuration of _existing_ resources +cannot currently be detected by Terraform. After updating to block device +configuration, resource recreation can be manually triggered by using the +[`taint` command](/docs/commands/taint.html). + ## Attributes Reference The following attributes are exported: