security group change of instance in VPC shouldn't require new resource

[lgtml]

Security group membership changes on VPC nodes should not be destructive.

To reproduce:

1. Launch a node with default security groups than update the instance.
2. Run Apply

note: will open another issue in regards to the security group ordering.

    security_groups.0: "sg-aaaaaaa" => "sg-bbbbbbb" (forces new resource)
    security_groups.1: "sg-bbbbbbb" => "sg-aaaaaaa" (forces new resource)

[tmtk75]

FYI: I also ran into this problem and sent a pull request to fix this.
#185
It works well on my local.

[mitchellh]

@tmtk75 I think you addressed ordering, but this is another issue which is that with VPCs, security group updates shouldn't require a new resource.

[lgtml]

@tmtk75 Yes your ordering fix does work, but I this is a different issue where nodes in VPC should not be destroyed if you change their security group membership.

I should have clarified further. Thank you

[tmtk75]

Hi, guys.
Thanks for your explanations! I understood.

[mitchellh]

I'm working on a branch on converting the aws_isntance resource to the new helper/schema framework, which will more easily enable both of these fixes. I'm in a branch now.

[mitchellh]

Ordering has been fixed, now we have the update vs recreate issue.

[ashmere]

any update as this destroying instances on every security group change or addition isn't really viable for a lot of use cases

[armon]

@ashmere I think this is being caused by the set handling which may be resolved via #616

[davedash]

I don't think it's being caused by a set issue. Or rather that issue is solved, this issue remains:

    security_groups.#:          "3" => "2" (forces new resource)
    security_groups.3052868674: "sg-e95bb28d" => "sg-e95bb28d" (forces new resource)
    security_groups.3312015480: "sg-a22dc2c6" => "sg-a22dc2c6" (forces new resource)

This should just remove the spurious SG since we're in VPC.

[ggiamarchi]

@davedash It seems to be the same as #873

[catsby]

This may be fixed with #1153 which looks promising but needs to be rebased on master

[mitchellh]

It is fixed by #1153 (or at least, the trail starting there)

[ghost]

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.