Skip to content

Commit

Permalink
providers/aws: fix source_dest_check on instance creation
Browse files Browse the repository at this point in the history
The `SourceDestCheck` attribute can only be changed via
`ModifyInstance`, so the AWS instance resource's `Create` function calls
out to `Update` before it returns to take care of applying
`source_dest_check` properly.

The `Update` function originally guarded against unnecessary API calls
with `GetOk`, which worked fine until hashicorp#993 when we changed the `GetOk`
semantics to no longer distinguish between "configured and zero-value"
and "not configured".

I attempted in hashicorp#1003 to fix this by switching to `HasChange` for the
guard, but this does not work in the `Create` case.

I played around with a few different ideas, none of which worked:

(a) Setting `Default: true` on `source_dest_check' has no effect

(b) Setting `Computed: true` on `source_dest_check' and adding a `d.Set`
    call in the `Read` function (which will initially set the value to `true`
    after instance creation). I really thought I could get this to work,
    but it results in the following:

```go
d.Get('source_dest_check')       // true
d.HasChange('source_dest_check') // false
d.GetChange('source_dest_check') // old: false, new: false
```

I couldn't figure out a way of coherently dealing with that result, so I
ended up throwing up my hands and giving up on the guard altogether.
We'll call `ModifyInstance` more than we have to, but this at least
yields expected behavior for both Creates and Updates.

Fixes hashicorp#1020
  • Loading branch information
phinze authored and Yahya Poonawala committed Mar 13, 2015
1 parent 5a3ed4f commit 9be2a7c
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 21 deletions.
23 changes: 8 additions & 15 deletions builtin/providers/aws/resource_aws_instance.go
Original file line number Diff line number Diff line change
Expand Up @@ -487,26 +487,19 @@ func resourceAwsInstanceRead(d *schema.ResourceData, meta interface{}) error {

func resourceAwsInstanceUpdate(d *schema.ResourceData, meta interface{}) error {
ec2conn := meta.(*AWSClient).ec2conn

modify := false
opts := new(ec2.ModifyInstance)

if d.HasChange("source_dest_check") {
opts.SetSourceDestCheck = true
opts.SourceDestCheck = d.Get("source_dest_check").(bool)
modify = true
}
opts.SetSourceDestCheck = true
opts.SourceDestCheck = d.Get("source_dest_check").(bool)

if modify {
log.Printf("[INFO] Modifying instance %s: %#v", d.Id(), opts)
if _, err := ec2conn.ModifyInstance(d.Id(), opts); err != nil {
return err
}

// TODO(mitchellh): wait for the attributes we modified to
// persist the change...
log.Printf("[INFO] Modifying instance %s: %#v", d.Id(), opts)
if _, err := ec2conn.ModifyInstance(d.Id(), opts); err != nil {
return err
}

// TODO(mitchellh): wait for the attributes we modified to
// persist the change...

if err := setTags(ec2conn, d); err != nil {
return err
} else {
Expand Down
18 changes: 12 additions & 6 deletions builtin/providers/aws/resource_aws_instance_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -145,19 +145,25 @@ func TestAccAWSInstance_sourceDestCheck(t *testing.T) {
CheckDestroy: testAccCheckInstanceDestroy,
Steps: []resource.TestStep{
resource.TestStep{
Config: testAccInstanceConfigSourceDest,
Config: testAccInstanceConfigSourceDestDisable,
Check: resource.ComposeTestCheckFunc(
testAccCheckInstanceExists(
"aws_instance.foo", &v),
testAccCheckInstanceExists("aws_instance.foo", &v),
testCheck(false),
),
},

resource.TestStep{
Config: testAccInstanceConfigSourceDestEnable,
Check: resource.ComposeTestCheckFunc(
testAccCheckInstanceExists("aws_instance.foo", &v),
testCheck(true),
),
},

resource.TestStep{
Config: testAccInstanceConfigSourceDestDisable,
Check: resource.ComposeTestCheckFunc(
testAccCheckInstanceExists(
"aws_instance.foo", &v),
testAccCheckInstanceExists("aws_instance.foo", &v),
testCheck(false),
),
},
Expand Down Expand Up @@ -388,7 +394,7 @@ resource "aws_instance" "foo" {
}
`

const testAccInstanceConfigSourceDest = `
const testAccInstanceConfigSourceDestEnable = `
resource "aws_vpc" "foo" {
cidr_block = "10.1.0.0/16"
}
Expand Down

0 comments on commit 9be2a7c

Please sign in to comment.