equinix_metal_connection field vlans should be forcenew

[displague]

The vlans field of equinix_metal_connection can not be updated. If a user changes this field the resource must be recreated. What happens today is the change is silently updated in state without making remote changes.

https://github.com/equinix/terraform-provider-equinix/blob/master/equinix/resource_metal_connection.go#L141

[ctreatma]

@ocobleseqx and @displague we discussed this issue earlier today, and I believe we concluded that it is possible to update VLANs for a shared connection, so we don't have to force recreate a connection when the vlans attribute changes. My understanding is that, when a customer creates a shared metal connection without specifying vlans, some other resources are created implicitly, and in order to update VLANs, a customer would have to import those "hidden" resources into terraform state & update them before updating the connection vlans. Does that sound like an accurate summary of the conversation? Are there any additional details or edge cases that are important to call out? One detail I'm not clear on is what kind of resources are created implicitly; is it a VLAN and a virtual circuit? One or the other?

One alternative I suggested, which could allow us to avoid recreating a shared metal connection when vlans is changed and also avoid customers importing a separate resource if/when they want to change vlans, is to make that a required attribute rather than implicitly creating other resources when a user creates a shared metal connection and does not specify vlans, but I think there were questions as to whether we can even specify vlans at creation time for a shared connection?

[ocobles]

VLAN must remain optional since they can just be defined for dedicated connection(DC) and this resource supports the creation of both shared and dedicated connections. Each shared connection (SC) comes with a single Virtual Circuit (VC) (one virtual circuit for each port if you are creating a redundant connection) and you cannot have more than 1 VC and you cannot delete/add a different VC to the connection . For DC you are able to add multiple VCs, using equinix_metal_virtual_circuit, and there's no pre-created VC. Therefore, there's no terraform resource to update the VC of a SC unless you import the pre-created VC into an equinix_metal_virtual_circuit. Moreover, The creation of a SC is completed and ready to use only once the connection is also configured in the Fabric side, in terraform you will need to create an equinix_ecx_l2_connection using a one-time generated token. So, forcing a new equinix_metal_connection will imply that users also re-create the equinix_ecx_l2_connection.

This conversation took place because in the API there are different endpoints to create the SC and to update a VC, and we try to avoid hidden magic in the behavior of the terraform provider, and each resource must be a representation of an object/API endpoint. In this case, I think that recreating the connection instead of just updating the VLAN in the pre-created VC can cause several difficulties for the end user, for example, there are customers where the Metal resources and Fabric resources are managed by different departments or even different companies. However, that can backfire for users coming from the API and will also work against our journey of achieving an auto-generated terraform provider, where all resources will be just a representation of the API specs.

To sum up, in my opinion, I'd go ahead with this change and add the force_new, since there's a bug that must be fixed (...the change is silently updated in state without making remote changes). However, it will be nice to improve the documentation and explain users the alternatives they have if they want to update the VLANs and they cannot simply recreate the connection resources.

In addition to that, we don't have enough information today about the limitations and inconveniences that can cause a user forcing the recreation of the connection resources (on both Metal and Fabric sides). That is something we can change and work on in the future once we receive feedback or any special requests.

[ctreatma]

The more I think about this, the more I'm concerned about adopting ForceNew for all changes to the vlans attribute. It sounds like adopting that would introduce new problems for dedicated connections, even if it eliminates the state mismatch. Having poked at this a little bit more, I see 2 problems to fix here:

1. The vlans attribute is not refreshed when the state is refreshed; this means that when I apply a change to vlans, the state updates without the infrastructure updating, and then the next plan shows no changes, and it also means that if I change the vlans manually in the web console, terraform won't detect that.
2. ForceNew on all changes to the vlans attribute sounds too broad, based on your description above? It sounds like it would be preferable to keep the current behavior for dedicated connections, but force recreation if vlans changes for a shared connection (we can do that by customizing the diff).

[ocobles]

VLANS field is only used with shared connection, not dedicated

[ctreatma]

Superseded by #272