same type transitions should not trigger an error

OK!

This function builds to the list of helpers in #230

I expect we would refactor ConvertDevice to call this function for each Bond.

So you would prefer to still convert network type per device? Instead of to convert bonds?

I just noticed that it's not poss to put bond1 (in n2.xlarge) to L3 and hybrid type, and it reminded of a message you sent me a week ago:

I don’t think we are able to do L3 for bond1. And we were discussing to limit bond1 capabilities to only L2 bonded and unbonded.
Only bond0 should be used for L3 capabilities.

Since the configuration for n2.xlarge is so inflexible (or special), we could treat it as a special case and create a separate function for it. Also we could create a separate tf resource.

If we treat n2 as a special case, maybe we can rollback to use network type for a device (instead of for a bond as this PR suggests). I'm sorry I pushed for the network-type per bond, I thought it's the most versatile way to control the network type.

I don't understand why the bonds are now displayed and controlled separately in the web console, when the 2 bonds in n2.xlarge are so inflexible. Anyway, I figured what network types are possible for bonds of n2.xlarge.

• bond0: L3, L2-bonded, L2-unbonded (no hybrid or hybrid-bonded)
• bond1: L2-bonded, L2-unbonded (no L3, hybrid, hybrid-bonded)

Considering all this, I think we should define possible (useful) configurations for the n2.xlarge, and just treat it differently. I don't think that we should map the n2.xlarge confs to the one-bond types.

So you would prefer to still convert network type per device? Instead of to convert bonds?

The thought here is that users (Terraform) will prefer ConvertDeviceBond. For backward compatibility, ConvertDevice can leverage ConvertDeviceBond to do the work. We can deprecate it independently of the refactor.

If we keep ConvertDevice it would have to result in the bonds and ports being in the same state they used to.
It may be easier to deprecate it (or remove it now).

What do you think?

Since the configuration for n2.xlarge is so inflexible (or special), we could treat it as a special case and create a separate function for it. Also we could create a separate tf resource.

I don't think users should have to choose different resource type based on device plan. I do think that they should consult documentation and use/choose attribute values that apply to the bond that they are configuring.

We run into problems when we try to bake logic into Terraform and Packngo to do more than what the API strictly offers.

If we remove the magic of device port name lookups and network type translation, a user could have a more powerful interface that shouldn't need much revisiting in the future.

resource "metal_port" "server-bond0" {
  port_id = metal_device.foo.network_ports.bond0.id
  bonded = true
  layer2 = false // this is default and represents either layer2 or layer3, leads to /port/id/convert-layer-X call on diff
  // network_type = "hybrid-bonded" // computed, read-only
  virtual_networks = [metal_vlan.foo.id]
  native_virtual_network = metal_vlan.native.id
  ip_address {
    type = "private_ipv4"
    cidr = 30
    // reservation_ids
    // cidr_notation from metal_ip_attachment
  }
}

resource "metal_port" "server-bond1" {
  port_id = metal_device.foo.network_ports.bond1.id
  bonded = true
  layer2 = true // representing the desired /port/id/convert-layer-X state
  // network_type = "layer2-bonded" // computed, read-only
  virtual_networks = [metal_vlan.foo.id]
}

The default of layer2=false would mean that layer3 is applied, which is the API default and is the most permissive (allowing for both IP addresses and VLANs in IBX facilities, thanks to hybrid-bonded).

The API endpoint has a hyphen (.../convert/layer-2, .../convert/layer-3), the facility Layer2 feature uses underscore (layer_2), and the network_type returned by bond devices uses no hyphens (layer3). I think we can follow the network_type naming pattern for this field without much confusion.

If the user sets invalid combinations, they will receive an API error and can adjust accordingly. Terraform doesn't have to validate the user input and does not need to make assumptions about what values can be used.

We can try adding validation, but I fear we will run into more problems than we avoid by doing so.

Considering all this, I think we should define possible (useful) configurations for the n2.xlarge, and just treat it differently. I don't think that we should map the n2.xlarge confs to the one-bond types.

I can add that in the future we may have 8 port resources, for example, that are neither "metal_device" nor "metal_hardware_reservation" resources. We'll benefit from treating ports as independent resources.

The thought here is that users (Terraform) will prefer ConvertDeviceBond. For backward compatibility, ConvertDevice can leverage ConvertDeviceBond to do the work. We can deprecate it independently of the refactor.

sound good

If we remove the magic of device port name lookups and network type translation, a user could have a more powerful interface that shouldn't need much revisiting in the future.

powerful indeed, but not simple. A lot of docs will be needed to map it to the states from the UI. But it's probably a way to go if there are more resources with ports coming.

The API endpoint has a hyphen (.../convert/layer-2, .../convert/layer-3), the facility Layer2 feature uses underscore (layer_2), and the network_type returned by bond devices uses no hyphens (layer3). I think we can follow the network_type naming pattern for this field without much confusion.

Yes, "layer{2,3}" for naming is ok. Just it's not straightforward to find out the current l2/l3 state (for the sake of the /convert/ calls) from the network_type attr, but it can be done. I think it's

• layer-3: layer3, hybrid, hybrid-bonded
• layer-2: network type can be layer2-individual, layer2-bonded

This is sth that's not in the UI.