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

Cannot seem to import existing VXCs properly #139

Open
hollow opened this issue Aug 26, 2024 · 4 comments
Open

Cannot seem to import existing VXCs properly #139

hollow opened this issue Aug 26, 2024 · 4 comments
Labels
bug Something isn't working

Comments

@hollow
Copy link

hollow commented Aug 26, 2024

While upgrading from v0.4.1 to the latest v1.1.1 I had to remove VXC resources due to backwards incompatible changes only to realize that importing and updating resources does not work afterwards. I'm facing the following two similar issues:

│ Error: Provider produced inconsistent result after apply
│ 
│ When applying changes to megaport_vxc.google["mp-europe-west3-a"], provider "provider[\"registry.terraform.io/megaport/megaport\"]" produced an unexpected new value: .a_end.ordered_vlan:
│ was cty.NumberIntVal(531), but now null.
│ 
│ This is a bug in the provider, which should be reported in the provider's own issue tracker.
╵
╷
│ Error: Provider produced inconsistent result after apply
│ 
│ When applying changes to megaport_vxc.google["mp-europe-west3-a"], provider "provider[\"registry.terraform.io/megaport/megaport\"]" produced an unexpected new value: .b_end_partner_config:
│ was cty.ObjectVal(map[string]cty.Value{"aws_config":cty.NullVal(cty.Object(map[string]cty.Type{"amazon_asn":cty.Number, "amazon_ip_address":cty.String, "asn":cty.Number,
│ "auth_key":cty.String, "connect_type":cty.String, "customer_ip_address":cty.String, "name":cty.String, "owner_account":cty.String, "prefixes":cty.String, "type":cty.String})),
│ "azure_config":cty.NullVal(cty.Object(map[string]cty.Type{"peers":cty.List(cty.Object(map[string]cty.Type{"peer_asn":cty.String, "prefixes":cty.String, "primary_subnet":cty.String,
│ "secondary_subnet":cty.String, "shared_key":cty.String, "type":cty.String, "vlan":cty.Number})), "port_choice":cty.String, "service_key":cty.String})),
│ "google_config":cty.ObjectVal(map[string]cty.Value{"pairing_key":cty.StringVal("41d00e54-2d12-494a-abed-c7baa39bee3d/europe-west3/1")}),
│ "oracle_config":cty.NullVal(cty.Object(map[string]cty.Type{"virtual_circuit_id":cty.String})), "partner":cty.StringVal("google"),
│ "partner_a_end_config":cty.NullVal(cty.Object(map[string]cty.Type{"interfaces":cty.List(cty.Object(map[string]cty.Type{"bfd":cty.Object(map[string]cty.Type{"multiplier":cty.Number,
│ "rx_interval":cty.Number, "tx_interval":cty.Number}), "bgp_connections":cty.List(cty.Object(map[string]cty.Type{"as_path_prepend_count":cty.Number, "bfd_enabled":cty.Bool,
│ "deny_export_to":cty.List(cty.String), "description":cty.String, "export_blacklist":cty.String, "export_policy":cty.String, "export_whitelist":cty.String, "import_blacklist":cty.String,
│ "import_whitelist":cty.String, "local_ip_address":cty.String, "med_in":cty.Number, "med_out":cty.Number, "password":cty.String, "peer_asn":cty.Number, "peer_ip_address":cty.String,
│ "permit_export_to":cty.List(cty.String), "shutdown":cty.Bool})), "ip_addresses":cty.List(cty.String), "ip_routes":cty.List(cty.Object(map[string]cty.Type{"description":cty.String,
│ "next_hop":cty.String, "prefix":cty.String})), "nat_ip_addresses":cty.List(cty.String)}))})),
│ "vrouter_config":cty.NullVal(cty.Object(map[string]cty.Type{"interfaces":cty.List(cty.Object(map[string]cty.Type{"bfd":cty.Object(map[string]cty.Type{"multiplier":cty.Number,
│ "rx_interval":cty.Number, "tx_interval":cty.Number}), "bgp_connections":cty.List(cty.Object(map[string]cty.Type{"as_path_prepend_count":cty.Number, "bfd_enabled":cty.Bool,
│ "deny_export_to":cty.List(cty.String), "description":cty.String, "export_blacklist":cty.String, "export_policy":cty.String, "export_whitelist":cty.String, "import_blacklist":cty.String,
│ "import_whitelist":cty.String, "local_ip_address":cty.String, "med_in":cty.Number, "med_out":cty.Number, "password":cty.String, "peer_asn":cty.Number, "peer_ip_address":cty.String,
│ "peer_type":cty.String, "permit_export_to":cty.List(cty.String), "shutdown":cty.Bool})), "ip_addresses":cty.List(cty.String),
│ "ip_routes":cty.List(cty.Object(map[string]cty.Type{"description":cty.String, "next_hop":cty.String, "prefix":cty.String})), "nat_ip_addresses":cty.List(cty.String),
│ "vlan":cty.Number}))}))}), but now null.
│ 
│ This is a bug in the provider, which should be reported in the provider's own issue tracker.

This is the Terraform resource code:

resource "megaport_vxc" "google" {
  for_each = local.google_interconnect_attachments

  product_name = "${each.value.name}-v${each.value.vlan}"
  rate_limit   = each.value.rate

  contract_term_months = 1

  a_end = {
    requested_product_uid = megaport_port.leaseweb["${each.value.site}-${each.value.port}"].product_uid
    ordered_vlan          = each.value.vlan
  }

  b_end = {
    requested_product_uid = data.megaport_partner.google[each.key].product_uid
  }

  b_end_partner_config = {
    partner = "google"
    google_config = {
      pairing_key = google_compute_interconnect_attachment.megaport[each.key].pairing_key
    }
  }
}
@hollow hollow added the bug Something isn't working label Aug 26, 2024
@efettero
Copy link

efettero commented Aug 29, 2024

I am experiencing challenges importing VXCs as well. Overall, this resource is confusing in how it operates and how you are supposed to import it.

When you order a VXC using 1.x.x you specify ordered_vlan which sets vlan (which seems to be read-only as if you try changing it the provider crashes). When you import it ordered_vlan is an empty string while vlan is set correctly, but when you attempt to set ordered_vlan (required by our VXC child module) the provider crashes.

Similar behavior can be seen too around the requested_product_uid and current_product_uid attributes. You set it with requested_product_uid, it sets current_product_uid, and when you import it it only gives you current_product_uid yet you need requested_product_uid

@mega-alex
Copy link
Collaborator

Hey there, we've got this on our radar and we are working on improving the workflow for importing now, but I'll add some color as to why it is structured this way.

Currently our API will pick a different vlan if the one provided by the customer isn't available which would cause the provider to error because of inconsistent state, we chose to address this by having those two attributes, however maybe it's worth another look to see if we can make that into one attribute with some more error handling.

The requested_product_uid has a similar reason, when connecting to certain partner ports our API will actually choose a different port under the hood for capacity / optimization reasons. This means that the actual product UID can be different than what the customer provided, which will cause a provider error. This flow works great for a portal and regular API users, but isn't the best for terraform unfortunately. We're working on addressing some of these API quirks, but will take some time and we wanted to get an updated provider out for customers with all the other improvements.

@mega-alex
Copy link
Collaborator

In the meantime you can omit the attributes ordered_vlan and b_end_partner_config during import, then after it is imported add back the ordered_vlan which will allow you to edit the vlan again. For b_end_partner_config that is going to require a fix on our end, I'll update this ticket when that is in. You shouldn't need to set the attribute as it is only set when the VXC is created and requires the VXC to be rebuilt when it's changed, but we definitely still want to let users track that on imported VXCs.

@efettero
Copy link

Hey @mega-alex!

I appreciate the responses and you providing more context around the current implementation, that is helpful!

Currently our API will pick a different vlan if the one provided by the customer isn't available which would cause the provider to error because of inconsistent state, we chose to address this by having those two attributes, however maybe it's worth another look to see if we can make that into one attribute with some more error handling.

I would expect this to behave similarly to the portal where if you enter a VLAN already in use, it warns you and you have to change it. It does not accept it and deploy with a random unused VLAN. I understand there is not the same back-and-forth when ordering with the API or Terraform, but I would prefer the API to error and tell us the VLAN is used, rather than continuing with a random VLAN.

In the meantime you can omit the attributes ordered_vlan and b_end_partner_config during import, then after it is imported add back the ordered_vlan which will allow you to edit the vlan again.

Thank you, I will give this a try!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants
@hollow @efettero @mega-alex and others