core: fix expand bug

[svanharmelen]

After fixing this part, there also needs to be made a change to the Atlas example or the schema of aws_elb.

[svanharmelen]

@mitchellh please have a quick look at this one... I think the Expand() func should be updated according to this PR as currently this seems to cause weird results. Take for example the config provided in #639 when applying the config with count = 1 all is good and everything is created as expected. But if you then update the count to 2, you get this plan:

~ aws_elb.web
    availability_zones.#: "1" => "<computed>"
    instances.#:          "1" => "<computed>"

- aws_instance.web

+ aws_instance.web.1
    ami:               "" => "ami-4fccb37f"
    availability_zone: "" => "<computed>"
    instance_type:     "" => "m1.small"
    key_name:          "" => "<computed>"
    private_dns:       "" => "<computed>"
    private_ip:        "" => "<computed>"
    public_dns:        "" => "<computed>"
    public_ip:         "" => "<computed>"
    security_groups.#: "" => "1"
    security_groups.0: "" => "allow_all"
    subnet_id:         "" => "<computed>"
    tenancy:           "" => "<computed>"

Instead of this (expected) plan:

~ aws_elb.web
    availability_zones.#: "1" => "<computed>"
    instances.#:          "1" => "<computed>"

+ aws_instance.web.1
    ami:               "" => "ami-4fccb37f"
    availability_zone: "" => "<computed>"
    instance_type:     "" => "m1.small"
    key_name:          "" => "<computed>"
    private_dns:       "" => "<computed>"
    private_ip:        "" => "<computed>"
    public_dns:        "" => "<computed>"
    public_ip:         "" => "<computed>"
    security_groups.#: "" => "1"
    security_groups.0: "" => "allow_all"
    subnet_id:         "" => "<computed>"
    tenancy:           "" => "<computed>"

I do notice that after this fix you still get an error, but that is because of line 15 which changes the availability_zones which has a ForceNew set to true in it's schema. When I change either line 15 of the config or remove the ForceNew, things now work as expected...

[svanharmelen]

Ow, and I do also noticed that the aws_instance.web (so the original one without the .0 or .1 created in the first apply run) remains in the state file. It doesn't show in terraform show/refresh/plan so it doesn't cause any unexpected behaviour, but it's still in the file itself.

Doesn't hurt, but I would say it shouldn't remain in there right?

[mitchellh]

Should this be within the if below? Do we have tests covering this?

[svanharmelen]

No, it's correct as is... Will have an additional look at the existing test and if we need additional tests, but let's first see what @armon thinks of this...

[mitchellh]

@svanharmelen I'd like @armon to look at this as well since graph.go is hairy. But on the service looks reasonable. I'm going to pull this down and take a closer look as well.

Additionally, yes, it should get removed from the state. I'd say that is a separate issue but we should look into that (outside this PR).

[armon]

LGTM!

[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.