Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve support for virtual circuits on shared connections #363

Closed
ctreatma opened this issue Aug 22, 2023 · 5 comments
Closed

Improve support for virtual circuits on shared connections #363

ctreatma opened this issue Aug 22, 2023 · 5 comments
Labels
area/resources/metal Issues related to Metal APIs

Comments

@ctreatma
Copy link
Contributor

ctreatma commented Aug 22, 2023

Community Note

  • Please vote on this issue by adding a 👍 reaction to the original issue to help the community and maintainers prioritize this request
  • Please do not leave "+1" or "me too" comments, they generate extra noise for issue followers and do not help prioritize the request
  • If you are interested in working on this issue or have submitted a pull request, please leave a comment.

Description

An issue we run up against for managing certain attributes of a shared metal_connection is that the API creates hidden virtual circuits (or hidden ports & virtual circuits?) for shared connections. In the past, we've worked around this limitation by having the metal_connection directly manage some virtual circuit settings (in particular, vlans) for those hidden virtual circuits.

As we add more features to those hidden virtual circuits, it becomes more complicated to hide logic inside the metal_connection resource. For example, we'd like to add support for VRF on shared connections (#360); if we follow the existing model, we would add a vrfs attribute that only works for shared connections, and would hide the logic for attaching those to the connection's hidden virtual circuits inside the metal_connection resource.

Instead of managing shared metal_connection virtual circuits via the metal_connection resource, we could enable customers to manage those circuits with a separate metal_shared_virtual_circuit resource, so that network configuration is more consistent between shared & dedicated connections, and so that we are not hiding some virtual circuit management inside our metal_connection resource.

The below HCL example demonstrates a metal_shared_virtual_circuit resource that expects the id attribute to be provided. On create, it makes a VCUpdateRequest instead of VCCreateRequest, passing the vlan or vrf specified in the configuration. On delete, it makes a VCUpdateRequest instead of VCDeleteRequest, which detaches the vrf or vlan from the virtual circuit.

While this is more verbose than hiding the shared virtual circuit logic in the metal_connection resource, it is also more explicit/intentional, and it more closely matches the reality of how these hidden virtual circuits are managed. In the web UI, for example, you can only change shared connection VLANs one at a time, and swapping VLANs requires at least 3 button clicks.

Another alternative would be to add a new metal_shared_virtual_circuit to implement the logic for virtual circuits that are attached to shared connections and can't be created or deleted.

New or Affected Resource(s)

  • metal_shared_virtual_circuit (maybe?)

Potential Terraform Configuration

# Attaching VLANs to the circuits of a shared metal_connection
resource "metal_connection" "shared_with_vlan" {
  # some attributes
  type = "shared"
}

resource "metal_shared_virtual_circuit" "primary_vlan" {
  id = metal_connection.shared_with_vlan.ports[0].virtual_circuits[0].id
  vlan_id = <a_vlan_ref1>
}

resource "metal_virtual_circuit" "shared_secondary_vlan" {
  id = metal_connection.shared_with_vlan.ports[1].virtual_circuits[0].id
  vlan_id = <a_vlan_ref2>
}

# Attaching a VRF to the circuits of a shared metal_connection
resource "metal_connection" "shared_with_vrf" {
  # some attributes
  type = "shared"
}

resource "metal_shared_virtual_circuit" "primary_vrf" {
  id = metal_connection.shared_with_vrf.ports[0].virtual_circuits[0].id
  vlan_id = <a_vrf_id>
}

resource "metal_virtual_circuit" "shared_secondary_vrf" {
  id = metal_connection.shared_with_vrf.ports[1].virtual_circuits[0].id
  vrf = <a_vrf_id>
}

References

@displague
Copy link
Member

displague commented Aug 22, 2023

#263 and the issues linked there, were created in a similar effort, wrt vlans vs vrfs.

@ctreatma
Copy link
Contributor Author

It turns out that, for shared connections, you must specify the VRFs when the connection is created and changing the VRFs requires deleting and recreating the connection. Since that is more solidly hidden inside the shared connection resource within the API, it would be harder to support VRFs on shared connections using the new resource described in this issue than to hide the VRF logic inside the connection resource.

It's possible we'd benefit from splitting dedicated vs. shared connections into separate resources (deprecate metal_connection and replace it with metal_shared_connection and metal_dedicated_connection), but even that might not be worth the effort.

@displague
Copy link
Member

displague commented Oct 26, 2023

VLANs are currently optional for equinix_metal_connection:

Only used with shared connection. Vlans to attach. Pass one vlan for Primary/Single connection and two vlans for Redundant connection.

service_token_type is also optional for shared connection.

VRFs could be added and made optional (only for shared, since dedicated would take a virtual-circuit approact) and force new.

perhaps it would be clearer if equinix_metal_connection vrf and vlan parameters were all contained within a shared_ prefix or shared {} subsection. The current optional approach matches the upstream API, and both the API and TF implementation have to be smoothed over with documentation.

@ctreatma
Copy link
Contributor Author

ctreatma commented Mar 28, 2024

#629 relates to this; without improved support for virtual circuits on shared connections, it is not straightforward to update the BGP settings on a virtual circuit belonging to a shared VRF connection. Updating those settings at the moment would require creating the shared VRF connection, importing the API-managed circuits into terraform state, and then updating terraform config to define values for required properties that match the API-managed values in order to be able to terraform plan/apply the BGP setting changes.

Since the last activity on this issue ~6 months ago, the API has also been updated to limit what can be done with virtual circuits on shared interconnections:

  • You can no longer change the vlan or vrf for an existing virtual circuit
  • You must provide vlans or vrfs at creation time for a shared interconnection

It's possible the only update you could make to a virtual circuit on a shared interconnection is to change the BGP settings, in which case it might make more sense to have the interconnection resource handle the BGP settings rather than making changes to the virtual circuit resource or adding a separate resource just for this case.

If it were possible, in the API, to specify these BGP settings when a shared VRF connection is created, would users still need to update them later?

@ctreatma
Copy link
Contributor Author

Closed by #717, which updated the existing metal_virtual_circuit resource to support virtual circuits on shared interconnections and also added support for IPv4 and IPv6 BGP settings for VRF virtual circuits. That change was released in v2.1.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/resources/metal Issues related to Metal APIs
Projects
None yet
Development

No branches or pull requests

2 participants