From 66ca917594dc0ac1ad31bbb24b6c56fb55a1f910 Mon Sep 17 00:00:00 2001 From: Charles Treatman Date: Mon, 24 Jun 2024 16:08:25 -0500 Subject: [PATCH 1/6] move port resource/datasource files --- .../resources/metal/port/datasource.go | 0 .../resources/metal/port/datasource_test.go | 0 .../port_helpers.go => internal/resources/metal/port/helpers.go | 0 .../resources/metal/port/resource.go | 0 .../resources/metal/port/resource_test.go | 0 5 files changed, 0 insertions(+), 0 deletions(-) rename equinix/data_source_metal_port.go => internal/resources/metal/port/datasource.go (100%) rename equinix/data_source_metal_port_acc_test.go => internal/resources/metal/port/datasource_test.go (100%) rename equinix/port_helpers.go => internal/resources/metal/port/helpers.go (100%) rename equinix/resource_metal_port.go => internal/resources/metal/port/resource.go (100%) rename equinix/resource_metal_port_acc_test.go => internal/resources/metal/port/resource_test.go (100%) diff --git a/equinix/data_source_metal_port.go b/internal/resources/metal/port/datasource.go similarity index 100% rename from equinix/data_source_metal_port.go rename to internal/resources/metal/port/datasource.go diff --git a/equinix/data_source_metal_port_acc_test.go b/internal/resources/metal/port/datasource_test.go similarity index 100% rename from equinix/data_source_metal_port_acc_test.go rename to internal/resources/metal/port/datasource_test.go diff --git a/equinix/port_helpers.go b/internal/resources/metal/port/helpers.go similarity index 100% rename from equinix/port_helpers.go rename to internal/resources/metal/port/helpers.go diff --git a/equinix/resource_metal_port.go b/internal/resources/metal/port/resource.go similarity index 100% rename from equinix/resource_metal_port.go rename to internal/resources/metal/port/resource.go diff --git a/equinix/resource_metal_port_acc_test.go b/internal/resources/metal/port/resource_test.go similarity index 100% rename from equinix/resource_metal_port_acc_test.go rename to internal/resources/metal/port/resource_test.go From 8012af120b2da9bd56572a6c74801dab1e4ded5d Mon Sep 17 00:00:00 2001 From: Charles Treatman Date: Mon, 24 Jun 2024 16:09:01 -0500 Subject: [PATCH 2/6] fix function references, package names, etc. after move --- equinix/provider.go | 5 ++- internal/resources/metal/port/datasource.go | 4 +- .../resources/metal/port/datasource_test.go | 19 +++++---- internal/resources/metal/port/helpers.go | 23 +--------- internal/resources/metal/port/resource.go | 31 ++++++++++++-- .../resources/metal/port/resource_test.go | 42 ++++++++++--------- 6 files changed, 65 insertions(+), 59 deletions(-) diff --git a/equinix/provider.go b/equinix/provider.go index 7fbb03d74..46275dc5d 100644 --- a/equinix/provider.go +++ b/equinix/provider.go @@ -9,6 +9,7 @@ import ( "github.com/equinix/terraform-provider-equinix/internal/config" fabric_connection "github.com/equinix/terraform-provider-equinix/internal/resources/fabric/connection" fabric_network "github.com/equinix/terraform-provider-equinix/internal/resources/fabric/network" + metal_port "github.com/equinix/terraform-provider-equinix/internal/resources/metal/port" "github.com/equinix/terraform-provider-equinix/internal/resources/metal/virtual_circuit" "github.com/equinix/terraform-provider-equinix/internal/resources/metal/vrf" "github.com/hashicorp/terraform-plugin-sdk/v2/diag" @@ -105,7 +106,7 @@ func Provider() *schema.Provider { "equinix_metal_devices": dataSourceMetalDevices(), "equinix_metal_device_bgp_neighbors": dataSourceMetalDeviceBGPNeighbors(), "equinix_metal_plans": dataSourceMetalPlans(), - "equinix_metal_port": dataSourceMetalPort(), + "equinix_metal_port": metal_port.DataSource(), "equinix_metal_reserved_ip_block": dataSourceMetalReservedIPBlock(), "equinix_metal_spot_market_request": dataSourceMetalSpotMarketRequest(), "equinix_metal_virtual_circuit": virtual_circuit.DataSource(), @@ -128,7 +129,7 @@ func Provider() *schema.Provider { "equinix_metal_project_api_key": resourceMetalProjectAPIKey(), "equinix_metal_device": resourceMetalDevice(), "equinix_metal_device_network_type": resourceMetalDeviceNetworkType(), - "equinix_metal_port": resourceMetalPort(), + "equinix_metal_port": metal_port.Resource(), "equinix_metal_reserved_ip_block": resourceMetalReservedIPBlock(), "equinix_metal_ip_attachment": resourceMetalIPAttachment(), "equinix_metal_spot_market_request": resourceMetalSpotMarketRequest(), diff --git a/internal/resources/metal/port/datasource.go b/internal/resources/metal/port/datasource.go index 0c31b2ec8..aeae95b6a 100644 --- a/internal/resources/metal/port/datasource.go +++ b/internal/resources/metal/port/datasource.go @@ -1,11 +1,11 @@ -package equinix +package port import ( "github.com/equinix/terraform-provider-equinix/internal/network" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" ) -func dataSourceMetalPort() *schema.Resource { +func DataSource() *schema.Resource { return &schema.Resource{ ReadWithoutTimeout: resourceMetalPortRead, diff --git a/internal/resources/metal/port/datasource_test.go b/internal/resources/metal/port/datasource_test.go index 070564893..9747ef13c 100644 --- a/internal/resources/metal/port/datasource_test.go +++ b/internal/resources/metal/port/datasource_test.go @@ -1,9 +1,10 @@ -package equinix +package port_test import ( "fmt" "testing" + "github.com/equinix/terraform-provider-equinix/internal/acceptance" "github.com/hashicorp/terraform-plugin-testing/helper/acctest" "github.com/hashicorp/terraform-plugin-testing/helper/resource" ) @@ -12,9 +13,9 @@ func TestAccDataSourceMetalPort_byName(t *testing.T) { rs := acctest.RandString(10) resource.ParallelTest(t, resource.TestCase{ - PreCheck: func() { testAccPreCheck(t) }, - ExternalProviders: testExternalProviders, - ProtoV5ProviderFactories: testAccProtoV5ProviderFactories, + PreCheck: func() { acceptance.TestAccPreCheck(t) }, + ExternalProviders: acceptance.TestExternalProviders, + ProtoV5ProviderFactories: acceptance.ProtoV5ProviderFactories, Steps: []resource.TestStep{ { Config: testAccDataSourceMetalPortConfig_byName(rs), @@ -50,16 +51,16 @@ data "equinix_metal_port" "test" { name = "eth0" } -`, confAccMetalDevice_base(preferable_plans, preferable_metros, preferable_os), name, testDeviceTerminationTime()) +`, acceptance.ConfAccMetalDevice_base(acceptance.Preferable_plans, acceptance.Preferable_metros, acceptance.Preferable_os), name, acceptance.TestDeviceTerminationTime()) } func TestAccDataSourceMetalPort_byId(t *testing.T) { rs := acctest.RandString(10) resource.ParallelTest(t, resource.TestCase{ - PreCheck: func() { testAccPreCheck(t) }, - ExternalProviders: testExternalProviders, - ProtoV5ProviderFactories: testAccProtoV5ProviderFactories, + PreCheck: func() { acceptance.TestAccPreCheck(t) }, + ExternalProviders: acceptance.TestExternalProviders, + ProtoV5ProviderFactories: acceptance.ProtoV5ProviderFactories, Steps: []resource.TestStep{ { Config: testAccDataSourceMetalPortConfig_byId(rs), @@ -93,5 +94,5 @@ resource "equinix_metal_device" "test" { data "equinix_metal_port" "test" { port_id = equinix_metal_device.test.ports[0].id } -`, confAccMetalDevice_base(preferable_plans, preferable_metros, preferable_os), name, testDeviceTerminationTime()) +`, acceptance.ConfAccMetalDevice_base(acceptance.Preferable_plans, acceptance.Preferable_metros, acceptance.Preferable_os), name, acceptance.TestDeviceTerminationTime()) } diff --git a/internal/resources/metal/port/helpers.go b/internal/resources/metal/port/helpers.go index 407ed8e14..e16c48d92 100644 --- a/internal/resources/metal/port/helpers.go +++ b/internal/resources/metal/port/helpers.go @@ -1,4 +1,4 @@ -package equinix +package port import ( "context" @@ -340,24 +340,3 @@ func portSanityChecks(cpr *ClientPortResource) error { return nil } - -func portProperlyDestroyed(port *packngo.Port) error { - var errs []string - if !port.Data.Bonded { - errs = append(errs, fmt.Sprintf("port %s wasn't bonded after equinix_metal_port destroy;", port.ID)) - } - if port.Type == "NetworkBondPort" && port.NetworkType != "layer3" { - errs = append(errs, "bond port should be in layer3 type after destroy;") - } - if port.NativeVirtualNetwork != nil { - errs = append(errs, "port should not have native VLAN assigned after destroy;") - } - if len(port.AttachedVirtualNetworks) != 0 { - errs = append(errs, "port should not have VLANs attached after destroy") - } - if len(errs) > 0 { - return fmt.Errorf("%s", errs) - } - - return nil -} diff --git a/internal/resources/metal/port/resource.go b/internal/resources/metal/port/resource.go index f78323393..df233b145 100644 --- a/internal/resources/metal/port/resource.go +++ b/internal/resources/metal/port/resource.go @@ -1,7 +1,8 @@ -package equinix +package port import ( "context" + "fmt" "log" "time" @@ -9,6 +10,7 @@ import ( equinix_errors "github.com/equinix/terraform-provider-equinix/internal/errors" equinix_schema "github.com/equinix/terraform-provider-equinix/internal/schema" + "github.com/packethost/packngo" "github.com/equinix/terraform-provider-equinix/internal/config" @@ -27,7 +29,7 @@ var ( l3Types = []string{"layer3", "hybrid", "hybrid-bonded"} ) -func resourceMetalPort() *schema.Resource { +func Resource() *schema.Resource { return &schema.Resource{ Timeouts: &schema.ResourceTimeout{ Create: schema.DefaultTimeout(30 * time.Minute), @@ -217,7 +219,7 @@ func resourceMetalPortDelete(ctx context.Context, d *schema.ResourceData, meta i // to reset the port to defaults we iterate through helpers (used in // create/update), some of which rely on resource state. reuse those helpers by // setting ephemeral state. - port := resourceMetalPort() + port := Resource() copy := port.Data(d.State()) cpr.Resource = copy if err = equinix_schema.SetMap(cpr.Resource, map[string]interface{}{ @@ -239,9 +241,30 @@ func resourceMetalPortDelete(ctx context.Context, d *schema.ResourceData, meta i } } // TODO(displague) error or warn? - if warn := portProperlyDestroyed(cpr.Port); warn != nil { + if warn := ProperlyDestroyed(cpr.Port); warn != nil { log.Printf("[WARN] %s\n", warn) } } return nil } + +func ProperlyDestroyed(port *packngo.Port) error { + var errs []string + if !port.Data.Bonded { + errs = append(errs, fmt.Sprintf("port %s wasn't bonded after equinix_metal_port destroy;", port.ID)) + } + if port.Type == "NetworkBondPort" && port.NetworkType != "layer3" { + errs = append(errs, "bond port should be in layer3 type after destroy;") + } + if port.NativeVirtualNetwork != nil { + errs = append(errs, "port should not have native VLAN assigned after destroy;") + } + if len(port.AttachedVirtualNetworks) != 0 { + errs = append(errs, "port should not have VLANs attached after destroy") + } + if len(errs) > 0 { + return fmt.Errorf("%s", errs) + } + + return nil +} diff --git a/internal/resources/metal/port/resource_test.go b/internal/resources/metal/port/resource_test.go index 8a7ca65d6..a003ac94b 100644 --- a/internal/resources/metal/port/resource_test.go +++ b/internal/resources/metal/port/resource_test.go @@ -1,11 +1,13 @@ -package equinix +package port_test import ( "fmt" "regexp" "testing" + "github.com/equinix/terraform-provider-equinix/internal/acceptance" "github.com/equinix/terraform-provider-equinix/internal/config" + "github.com/equinix/terraform-provider-equinix/internal/resources/metal/port" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" "github.com/hashicorp/terraform-plugin-testing/helper/acctest" @@ -42,7 +44,7 @@ locals { eth0_id = [for p in equinix_metal_device.test.ports: p.id if p.name == "eth0"][0] } -`, confAccMetalDevice_base(preferable_plans, preferable_metros, preferable_os), name, testDeviceTerminationTime()) +`, acceptance.ConfAccMetalDevice_base(acceptance.Preferable_plans, acceptance.Preferable_metros, acceptance.Preferable_os), name, acceptance.TestDeviceTerminationTime()) } func confAccMetalPort_L3(name string) string { @@ -237,9 +239,9 @@ resource "equinix_metal_vlan" "test" { func TestAccMetalPort_hybridBondedVxlan(t *testing.T) { rs := acctest.RandString(10) resource.ParallelTest(t, resource.TestCase{ - PreCheck: func() { testAccPreCheck(t) }, - ExternalProviders: testExternalProviders, - ProtoV5ProviderFactories: testAccProtoV5ProviderFactories, + PreCheck: func() { acceptance.TestAccPreCheck(t) }, + ExternalProviders: acceptance.TestExternalProviders, + ProtoV5ProviderFactories: acceptance.ProtoV5ProviderFactories, CheckDestroy: testAccMetalPortDestroyed, Steps: []resource.TestStep{ { @@ -266,9 +268,9 @@ func TestAccMetalPort_hybridBondedVxlan(t *testing.T) { func TestAccMetalPort_L2IndividualNativeVlan(t *testing.T) { rs := acctest.RandString(10) resource.ParallelTest(t, resource.TestCase{ - PreCheck: func() { testAccPreCheck(t) }, - ExternalProviders: testExternalProviders, - ProtoV5ProviderFactories: testAccProtoV5ProviderFactories, + PreCheck: func() { acceptance.TestAccPreCheck(t) }, + ExternalProviders: acceptance.TestExternalProviders, + ProtoV5ProviderFactories: acceptance.ProtoV5ProviderFactories, CheckDestroy: testAccMetalPortDestroyed, Steps: []resource.TestStep{ { @@ -299,9 +301,9 @@ func TestAccMetalPort_L2IndividualNativeVlan(t *testing.T) { func testAccMetalPortTemplate(t *testing.T, conf func(string) string, expectedType string) { rs := acctest.RandString(10) resource.ParallelTest(t, resource.TestCase{ - PreCheck: func() { testAccPreCheck(t) }, - ExternalProviders: testExternalProviders, - ProtoV5ProviderFactories: testAccProtoV5ProviderFactories, + PreCheck: func() { acceptance.TestAccPreCheck(t) }, + ExternalProviders: acceptance.TestExternalProviders, + ProtoV5ProviderFactories: acceptance.ProtoV5ProviderFactories, CheckDestroy: testAccMetalPortDestroyed, Steps: []resource.TestStep{ { @@ -356,7 +358,7 @@ func TestAccMetalPort_hybridBonded(t *testing.T) { } func testAccMetalPortDestroyed(s *terraform.State) error { - client := testAccProvider.Meta().(*config.Config).Metal + client := acceptance.TestAccProvider.Meta().(*config.Config).Metal port_ids := []string{} @@ -373,7 +375,7 @@ func testAccMetalPortDestroyed(s *terraform.State) error { if err != nil { return fmt.Errorf("Error getting port %s during destroy check", pid) } - err = portProperlyDestroyed(p) + err = port.ProperlyDestroyed(p) if err != nil { return err } @@ -391,7 +393,7 @@ func testAccWaitForPortActive(deviceName, portName string) resource.ImportStateI return "", fmt.Errorf("No Record ID is set") } - meta := testAccProvider.Meta() + meta := acceptance.TestAccProvider.Meta() rd := new(schema.ResourceData) meta.(*config.Config).AddModuleToMetalUserAgent(rd) client := meta.(*config.Config).Metal @@ -422,9 +424,9 @@ func TestAccMetalPortCreate_hybridBonded_timeout(t *testing.T) { deviceName := "equinix_metal_device.test" resource.ParallelTest(t, resource.TestCase{ - PreCheck: func() { testAccPreCheck(t) }, - ExternalProviders: testExternalProviders, - ProtoV5ProviderFactories: testAccProtoV5ProviderFactories, + PreCheck: func() { acceptance.TestAccPreCheck(t) }, + ExternalProviders: acceptance.TestExternalProviders, + ProtoV5ProviderFactories: acceptance.ProtoV5ProviderFactories, CheckDestroy: testAccMetalPortDestroyed, Steps: []resource.TestStep{ { @@ -463,9 +465,9 @@ func TestAccMetalPortUpdate_hybridBonded_timeout(t *testing.T) { rInt := acctest.RandInt() resource.ParallelTest(t, resource.TestCase{ - PreCheck: func() { testAccPreCheck(t) }, - ExternalProviders: testExternalProviders, - ProtoV5ProviderFactories: testAccProtoV5ProviderFactories, + PreCheck: func() { acceptance.TestAccPreCheck(t) }, + ExternalProviders: acceptance.TestExternalProviders, + ProtoV5ProviderFactories: acceptance.ProtoV5ProviderFactories, CheckDestroy: testAccMetalPortDestroyed, Steps: []resource.TestStep{ { From ac9722bd8a776181802e963e16e4666e02adb0c1 Mon Sep 17 00:00:00 2001 From: Charles Treatman Date: Mon, 24 Jun 2024 17:16:48 -0500 Subject: [PATCH 3/6] woof...convert to equinix-sdk-go --- internal/resources/metal/port/helpers.go | 180 ++++++++++-------- internal/resources/metal/port/resource.go | 80 ++++---- .../resources/metal/port/resource_test.go | 5 +- 3 files changed, 146 insertions(+), 119 deletions(-) diff --git a/internal/resources/metal/port/helpers.go b/internal/resources/metal/port/helpers.go index e16c48d92..ded05ded2 100644 --- a/internal/resources/metal/port/helpers.go +++ b/internal/resources/metal/port/helpers.go @@ -3,10 +3,12 @@ package port import ( "context" "fmt" + "net/http" "slices" "strings" "time" + "github.com/equinix/equinix-sdk-go/services/metalv1" "github.com/equinix/terraform-provider-equinix/internal/converters" "github.com/equinix/terraform-provider-equinix/internal/config" @@ -17,23 +19,28 @@ import ( "github.com/pkg/errors" ) +var ( + // Deprecated: empty port assignment input that is required + // for some endpoints; probably indicates a bug in the API spec + dummy = metalv1.PortAssignInput{} +) + type ClientPortResource struct { - Client *packngo.Client - Port *packngo.Port + Client *metalv1.APIClient + Port *metalv1.Port Resource *schema.ResourceData } -func getClientPortResource(d *schema.ResourceData, meta interface{}) (*ClientPortResource, *packngo.Response, error) { - meta.(*config.Config).AddModuleToMetalUserAgent(d) - client := meta.(*config.Config).Metal +func getClientPortResource(ctx context.Context, d *schema.ResourceData, meta interface{}) (*ClientPortResource, *http.Response, error) { + client := meta.(*config.Config).NewMetalClientForSDK(d) port_id := d.Get("port_id").(string) - getOpts := &packngo.GetOptions{Includes: []string{ + getOpts := []string{ "native_virtual_network", "virtual_networks", - }} - port, resp, err := client.Ports.Get(port_id, getOpts) + } + port, resp, err := client.PortsApi.FindPortById(ctx, port_id).Include(getOpts).Execute() if err != nil { return nil, resp, err } @@ -46,7 +53,7 @@ func getClientPortResource(d *schema.ResourceData, meta interface{}) (*ClientPor return cpr, resp, nil } -func getPortByResourceData(d *schema.ResourceData, client *packngo.Client) (*packngo.Port, error) { +func getPortByResourceData(ctx context.Context, d *schema.ResourceData, client *metalv1.APIClient) (*metalv1.Port, *http.Response, error) { portId, portIdOk := d.GetOk("port_id") resourceId := d.Id() @@ -63,34 +70,37 @@ func getPortByResourceData(d *schema.ResourceData, client *packngo.Client) (*pac // check parameter sanity only for a new (not-yet-created) resource if resourceId == "" { if portIdOk && (deviceIdOk || portNameOk) { - return nil, fmt.Errorf("you must specify either id or (device_id and name)") + return nil, nil, fmt.Errorf("you must specify either id or (device_id and name)") } } - var port *packngo.Port + var port *metalv1.Port + var resp *http.Response + var err error - getOpts := &packngo.GetOptions{Includes: []string{ + getOpts := []string{ "native_virtual_network", "virtual_networks", - }} + } if portIdOk { - var err error - port, _, err = client.Ports.Get(portId.(string), getOpts) + port, resp, err = client.PortsApi.FindPortById(ctx, portId.(string)).Include(getOpts).Execute() if err != nil { - return nil, err + return nil, resp, err } } else { if !(deviceIdOk && portNameOk) { - return nil, fmt.Errorf("If you don't use port_id, you must supply both device_id and name") + return nil, nil, fmt.Errorf("If you don't use port_id, you must supply both device_id and name") } - device, _, err := client.Devices.Get(deviceId.(string), getOpts) + var device *metalv1.Device + device, resp, err = client.DevicesApi.FindDeviceById(ctx, deviceId.(string)).Include(getOpts).Execute() if err != nil { - return nil, err + return nil, resp, err } - return device.GetPortByName(portName.(string)) + port, err = GetPortByName(device, portName.(string)) + return port, nil, err } - return port, nil + return port, resp, nil } func getSpecifiedNative(d *schema.ResourceData) string { @@ -102,18 +112,18 @@ func getSpecifiedNative(d *schema.ResourceData) string { return specifiedNative } -func getCurrentNative(p *packngo.Port) string { +func getCurrentNative(p *metalv1.Port) string { currentNative := "" if p.NativeVirtualNetwork != nil { - currentNative = p.NativeVirtualNetwork.ID + currentNative = p.NativeVirtualNetwork.GetId() } return currentNative } -func attachedVlanIds(p *packngo.Port) []string { +func attachedVlanIds(p *metalv1.Port) []string { attached := []string{} - for _, v := range p.AttachedVirtualNetworks { - attached = append(attached, v.ID) + for _, v := range p.VirtualNetworks { + attached = append(attached, v.GetId()) } return attached } @@ -132,8 +142,8 @@ func specifiedVlanIds(d *schema.ResourceData) []string { return []string{} } -func batchVlans(ctx context.Context, start time.Time, removeOnly bool) func(*ClientPortResource) error { - return func(cpr *ClientPortResource) error { +func batchVlans(start time.Time, removeOnly bool) func(context.Context, *ClientPortResource) error { + return func(ctx context.Context, cpr *ClientPortResource) error { var vlansToAssign []string var currentNative string vlansToRemove := converters.Difference( @@ -148,19 +158,18 @@ func batchVlans(ctx context.Context, start time.Time, removeOnly bool) func(*Cli attachedVlanIds(cpr.Port), ) } - vacr := &packngo.VLANAssignmentBatchCreateRequest{} + vacr := metalv1.PortVlanAssignmentBatchCreateInput{} for _, v := range vlansToRemove { - vacr.VLANAssignments = append(vacr.VLANAssignments, packngo.VLANAssignmentCreateRequest{ - VLAN: v, - State: packngo.VLANAssignmentUnassigned, + vacr.VlanAssignments = append(vacr.VlanAssignments, metalv1.PortVlanAssignmentBatchCreateInputVlanAssignmentsInner{ + Vlan: &v, + State: metalv1.PORTVLANASSIGNMENTBATCHVLANASSIGNMENTSINNERSTATE_UNASSIGNED.Ptr(), }) } - for _, v := range vlansToAssign { native := currentNative == v - vacr.VLANAssignments = append(vacr.VLANAssignments, packngo.VLANAssignmentCreateRequest{ - VLAN: v, - State: packngo.VLANAssignmentAssigned, + vacr.VlanAssignments = append(vacr.VlanAssignments, metalv1.PortVlanAssignmentBatchCreateInputVlanAssignmentsInner{ + Vlan: &v, + State: metalv1.PORTVLANASSIGNMENTBATCHVLANASSIGNMENTSINNERSTATE_ASSIGNED.Ptr(), Native: &native, }) } @@ -168,15 +177,15 @@ func batchVlans(ctx context.Context, start time.Time, removeOnly bool) func(*Cli } } -func createAndWaitForBatch(ctx context.Context, start time.Time, cpr *ClientPortResource, vacr *packngo.VLANAssignmentBatchCreateRequest) error { - if len(vacr.VLANAssignments) == 0 { +func createAndWaitForBatch(ctx context.Context, start time.Time, cpr *ClientPortResource, vacr metalv1.PortVlanAssignmentBatchCreateInput) error { + if len(vacr.VlanAssignments) == 0 { return nil } - portID := cpr.Port.ID + portID := cpr.Port.GetId() c := cpr.Client - b, _, err := c.VLANAssignments.CreateBatch(portID, vacr, nil) + b, _, err := c.PortsApi.CreatePortVlanAssignmentBatch(ctx, portID).PortVlanAssignmentBatchCreateInput(vacr).Execute() if err != nil { return fmt.Errorf("vlan assignment batch could not be created: %w", err) } @@ -192,38 +201,38 @@ func createAndWaitForBatch(ctx context.Context, start time.Time, cpr *ClientPort MinTimeout: 5 * time.Second, Timeout: ctxTimeout - time.Since(start) - 30*time.Second, Refresh: func() (result interface{}, state string, err error) { - b, _, err := c.VLANAssignments.GetBatch(portID, b.ID, nil) - switch b.State { - case packngo.VLANAssignmentBatchFailed: - return b, string(packngo.VLANAssignmentBatchFailed), - fmt.Errorf("vlan assignment batch %s provisioning failed: %s", b.ID, strings.Join(b.ErrorMessages, "; ")) - case packngo.VLANAssignmentBatchCompleted: - return b, string(packngo.VLANAssignmentBatchCompleted), nil + b, _, err := c.PortsApi.FindPortVlanAssignmentBatchByPortIdAndBatchId(ctx, portID, b.GetId()).Execute() + switch b.GetState() { + case metalv1.PORTVLANASSIGNMENTBATCHSTATE_FAILED: + return b, string(metalv1.PORTVLANASSIGNMENTBATCHSTATE_FAILED), + fmt.Errorf("vlan assignment batch %s provisioning failed: %s", b.GetId(), strings.Join(b.ErrorMessages, "; ")) + case metalv1.PORTVLANASSIGNMENTBATCHSTATE_COMPLETED: + return b, string(metalv1.PORTVLANASSIGNMENTBATCHSTATE_COMPLETED), nil default: if err != nil { - return b, "", fmt.Errorf("vlan assignment batch %s could not be polled: %w", b.ID, err) + return b, "", fmt.Errorf("vlan assignment batch %s could not be polled: %w", b.GetId(), err) } - return b, string(b.State), err + return b, string(b.GetState()), err } }, } if _, err = stateChangeConf.WaitForStateContext(ctx); err != nil { - return errors.Wrapf(err, "vlan assignment batch %s is not complete after timeout", b.ID) + return errors.Wrapf(err, "vlan assignment batch %s is not complete after timeout", b.GetId()) } return nil } -func updateNativeVlan(cpr *ClientPortResource) error { +func updateNativeVlan(ctx context.Context, cpr *ClientPortResource) error { currentNative := getCurrentNative(cpr.Port) specifiedNative := getSpecifiedNative(cpr.Resource) if currentNative != specifiedNative { - var port *packngo.Port + var port *metalv1.Port var err error if specifiedNative == "" && currentNative != "" { - port, _, err = cpr.Client.Ports.UnassignNative(cpr.Port.ID) + port, _, err = cpr.Client.PortsApi.DeleteNativeVlan(ctx, cpr.Port.GetId()).Execute() } else { - port, _, err = cpr.Client.Ports.AssignNative(cpr.Port.ID, specifiedNative) + port, _, err = cpr.Client.PortsApi.AssignNativeVlan(ctx, cpr.Port.GetId()).Vnid(specifiedNative).Execute() } if err != nil { return err @@ -233,27 +242,30 @@ func updateNativeVlan(cpr *ClientPortResource) error { return nil } -func processBondAction(cpr *ClientPortResource, actionIsBond bool) error { +func processBondAction(ctx context.Context, cpr *ClientPortResource, actionIsBond bool) error { wantsBondedRaw, wantsBondedOk := cpr.Resource.GetOkExists("bonded") wantsBonded := wantsBondedRaw.(bool) // only act if the necessary action is the one specified in doBond if wantsBondedOk && (wantsBonded == actionIsBond) { // act if the current Bond state of the port is different than the spcified - if wantsBonded != cpr.Port.Data.Bonded { - action := cpr.Client.DevicePorts.Disbond + if wantsBonded != cpr.Port.Data.GetBonded() { + var port *metalv1.Port + var err error if wantsBonded { - action = cpr.Client.DevicePorts.Bond + port, _, err = cpr.Client.PortsApi.BondPort(ctx, cpr.Port.GetId()).Execute() + } else { + + port, _, err = cpr.Client.PortsApi.DisbondPort(ctx, cpr.Port.GetId()).Execute() } - port, _, err := action(cpr.Port, false) if err != nil { return err } - getOpts := &packngo.GetOptions{Includes: []string{ + getOpts := []string{ "native_virtual_network", "virtual_networks", - }} - port, _, err = cpr.Client.Ports.Get(port.ID, getOpts) + } + port, _, err = cpr.Client.PortsApi.FindPortById(ctx, port.GetId()).Include(getOpts).Execute() if err != nil { return err } @@ -264,20 +276,20 @@ func processBondAction(cpr *ClientPortResource, actionIsBond bool) error { return nil } -func makeBond(cpr *ClientPortResource) error { - return processBondAction(cpr, true) +func makeBond(ctx context.Context, cpr *ClientPortResource) error { + return processBondAction(ctx, cpr, true) } -func makeDisbond(cpr *ClientPortResource) error { - return processBondAction(cpr, false) +func makeDisbond(ctx context.Context, cpr *ClientPortResource) error { + return processBondAction(ctx, cpr, false) } -func convertToL2(cpr *ClientPortResource) error { +func convertToL2(ctx context.Context, cpr *ClientPortResource) error { l2, l2Ok := cpr.Resource.GetOkExists("layer2") - isLayer2 := slices.Contains(l2Types, cpr.Port.NetworkType) + isLayer2 := slices.Contains(l2Types, cpr.Port.GetNetworkType()) if l2Ok && l2.(bool) && !isLayer2 { - port, _, err := cpr.Client.Ports.ConvertToLayerTwo(cpr.Port.ID) + port, _, err := cpr.Client.PortsApi.ConvertLayer2(ctx, cpr.Port.GetId()).PortAssignInput(dummy).Execute() if err != nil { return err } @@ -286,17 +298,20 @@ func convertToL2(cpr *ClientPortResource) error { return nil } -func convertToL3(cpr *ClientPortResource) error { +func convertToL3(ctx context.Context, cpr *ClientPortResource) error { l2, l2Ok := cpr.Resource.GetOkExists("layer2") - isLayer2 := slices.Contains(l2Types, cpr.Port.NetworkType) + isLayer2 := slices.Contains(l2Types, cpr.Port.GetNetworkType()) if l2Ok && !l2.(bool) && isLayer2 { - ips := []packngo.AddressRequest{ - {AddressFamily: 4, Public: true}, - {AddressFamily: 4, Public: false}, - {AddressFamily: 6, Public: true}, + ips := metalv1.PortConvertLayer3Input{ + RequestIps: []metalv1.PortConvertLayer3InputRequestIpsInner{ + {AddressFamily: metalv1.PtrInt32(4), Public: metalv1.PtrBool(true)}, + {AddressFamily: metalv1.PtrInt32(4), Public: metalv1.PtrBool(false)}, + {AddressFamily: metalv1.PtrInt32(6), Public: metalv1.PtrBool(true)}, + }, } - port, _, err := cpr.Client.Ports.ConvertToLayerThree(cpr.Port.ID, ips) + + port, _, err := cpr.Client.PortsApi.ConvertLayer3(ctx, cpr.Port.GetId()).PortConvertLayer3Input(ips).Execute() if err != nil { return err } @@ -305,8 +320,8 @@ func convertToL3(cpr *ClientPortResource) error { return nil } -func portSanityChecks(cpr *ClientPortResource) error { - isBondPort := cpr.Port.Type == "NetworkBondPort" +func portSanityChecks(_ context.Context, cpr *ClientPortResource) error { + isBondPort := cpr.Port.GetType() == "NetworkBondPort" // Constraint: Only bond ports have layer2 mode l2Raw, l2Ok := cpr.Resource.GetOkExists("layer2") @@ -340,3 +355,12 @@ func portSanityChecks(cpr *ClientPortResource) error { return nil } + +func GetPortByName(d *metalv1.Device, name string) (*metalv1.Port, error) { + for _, port := range d.NetworkPorts { + if port.GetName() == name { + return &port, nil + } + } + return nil, fmt.Errorf("Port %s not found in device %s", name, d.GetId()) +} diff --git a/internal/resources/metal/port/resource.go b/internal/resources/metal/port/resource.go index df233b145..fbcb01d69 100644 --- a/internal/resources/metal/port/resource.go +++ b/internal/resources/metal/port/resource.go @@ -4,13 +4,14 @@ import ( "context" "fmt" "log" + "net/http" "time" "slices" + "github.com/equinix/equinix-sdk-go/services/metalv1" equinix_errors "github.com/equinix/terraform-provider-equinix/internal/errors" equinix_schema "github.com/equinix/terraform-provider-equinix/internal/schema" - "github.com/packethost/packngo" "github.com/equinix/terraform-provider-equinix/internal/config" @@ -25,8 +26,8 @@ Race conditions: */ var ( - l2Types = []string{"layer2-individual", "layer2-bonded"} - l3Types = []string{"layer3", "hybrid", "hybrid-bonded"} + l2Types = []metalv1.PortNetworkType{"layer2-individual", "layer2-bonded"} + l3Types = []metalv1.PortNetworkType{"layer3", "hybrid", "hybrid-bonded"} ) func Resource() *schema.Resource { @@ -129,22 +130,22 @@ func Resource() *schema.Resource { func resourceMetalPortUpdate(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics { start := time.Now() - cpr, _, err := getClientPortResource(d, meta) + cpr, _, err := getClientPortResource(ctx, d, meta) if err != nil { return diag.FromErr(equinix_errors.FriendlyError(err)) } - for _, f := range [](func(*ClientPortResource) error){ + for _, f := range [](func(context.Context, *ClientPortResource) error){ portSanityChecks, - batchVlans(ctx, start, true), + batchVlans(start, true), makeDisbond, convertToL2, makeBond, convertToL3, - batchVlans(ctx, start, false), + batchVlans(start, false), updateNativeVlan, } { - if err := f(cpr); err != nil { + if err := f(ctx, cpr); err != nil { return diag.FromErr(equinix_errors.FriendlyError(err)) } } @@ -153,12 +154,11 @@ func resourceMetalPortUpdate(ctx context.Context, d *schema.ResourceData, meta i } func resourceMetalPortRead(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics { - meta.(*config.Config).AddModuleToMetalUserAgent(d) - client := meta.(*config.Config).Metal + client := meta.(*config.Config).NewMetalClientForSDK(d) - port, err := getPortByResourceData(d, client) + port, resp, err := getPortByResourceData(ctx, d, client) if err != nil { - if equinix_errors.IsNotFound(err) || equinix_errors.IsForbidden(err) { + if resp != nil && slices.Contains([]int{http.StatusNotFound, http.StatusForbidden}, resp.StatusCode) { log.Printf("[WARN] Port (%s) not accessible, removing from state", d.Id()) d.SetId("") @@ -167,16 +167,16 @@ func resourceMetalPortRead(ctx context.Context, d *schema.ResourceData, meta int return diag.FromErr(err) } m := map[string]interface{}{ - "port_id": port.ID, - "type": port.Type, - "name": port.Name, - "network_type": port.NetworkType, - "mac": port.Data.MAC, - "bonded": port.Data.Bonded, - "disbond_supported": port.DisbondOperationSupported, + "port_id": port.GetId(), + "type": port.GetType(), + "name": port.GetName(), + "network_type": port.GetNetworkType(), + "mac": port.Data.GetMac(), + "bonded": port.Data.GetBonded(), + "disbond_supported": port.GetDisbondOperationSupported(), } - l2 := slices.Contains(l2Types, port.NetworkType) - l3 := slices.Contains(l3Types, port.NetworkType) + l2 := slices.Contains(l2Types, port.GetNetworkType()) + l3 := slices.Contains(l3Types, port.GetNetworkType()) if l2 { m["layer2"] = true @@ -186,24 +186,24 @@ func resourceMetalPortRead(ctx context.Context, d *schema.ResourceData, meta int } if port.NativeVirtualNetwork != nil { - m["native_vlan_id"] = port.NativeVirtualNetwork.ID + m["native_vlan_id"] = port.NativeVirtualNetwork.GetId() } vlans := []string{} vxlans := []int{} - for _, n := range port.AttachedVirtualNetworks { - vlans = append(vlans, n.ID) - vxlans = append(vxlans, n.VXLAN) + for _, n := range port.VirtualNetworks { + vlans = append(vlans, n.GetId()) + vxlans = append(vxlans, int(n.GetVxlan())) } m["vlan_ids"] = vlans m["vxlan_ids"] = vxlans if port.Bond != nil { - m["bond_id"] = port.Bond.ID - m["bond_name"] = port.Bond.Name + m["bond_id"] = port.Bond.GetId() + m["bond_name"] = port.Bond.GetName() } - d.SetId(port.ID) + d.SetId(port.GetId()) return diag.FromErr(equinix_schema.SetMap(d, m)) } @@ -211,9 +211,11 @@ func resourceMetalPortDelete(ctx context.Context, d *schema.ResourceData, meta i resetRaw, resetOk := d.GetOk("reset_on_delete") if resetOk && resetRaw.(bool) { start := time.Now() - cpr, resp, err := getClientPortResource(d, meta) - if equinix_errors.IgnoreResponseErrors(equinix_errors.HttpForbidden, equinix_errors.HttpNotFound)(resp, err) != nil { - return diag.FromErr(err) + cpr, resp, err := getClientPortResource(ctx, d, meta) + if err != nil { + if resp != nil && !slices.Contains([]int{http.StatusForbidden, http.StatusNotFound}, resp.StatusCode) { + return diag.FromErr(err) + } } // to reset the port to defaults we iterate through helpers (used in @@ -231,12 +233,12 @@ func resourceMetalPortDelete(ctx context.Context, d *schema.ResourceData, meta i }); err != nil { return diag.FromErr(err) } - for _, f := range [](func(*ClientPortResource) error){ - batchVlans(ctx, start, true), + for _, f := range [](func(context.Context, *ClientPortResource) error){ + batchVlans(start, true), makeBond, convertToL3, } { - if err := f(cpr); err != nil { + if err := f(ctx, cpr); err != nil { return diag.FromErr(err) } } @@ -248,18 +250,18 @@ func resourceMetalPortDelete(ctx context.Context, d *schema.ResourceData, meta i return nil } -func ProperlyDestroyed(port *packngo.Port) error { +func ProperlyDestroyed(port *metalv1.Port) error { var errs []string - if !port.Data.Bonded { - errs = append(errs, fmt.Sprintf("port %s wasn't bonded after equinix_metal_port destroy;", port.ID)) + if !port.Data.GetBonded() { + errs = append(errs, fmt.Sprintf("port %s wasn't bonded after equinix_metal_port destroy;", port.GetId())) } - if port.Type == "NetworkBondPort" && port.NetworkType != "layer3" { + if port.GetType() == "NetworkBondPort" && port.GetNetworkType() != "layer3" { errs = append(errs, "bond port should be in layer3 type after destroy;") } if port.NativeVirtualNetwork != nil { errs = append(errs, "port should not have native VLAN assigned after destroy;") } - if len(port.AttachedVirtualNetworks) != 0 { + if len(port.VirtualNetworks) != 0 { errs = append(errs, "port should not have VLANs attached after destroy") } if len(errs) > 0 { diff --git a/internal/resources/metal/port/resource_test.go b/internal/resources/metal/port/resource_test.go index a003ac94b..bbb4b8ae3 100644 --- a/internal/resources/metal/port/resource_test.go +++ b/internal/resources/metal/port/resource_test.go @@ -1,6 +1,7 @@ package port_test import ( + "context" "fmt" "regexp" "testing" @@ -358,7 +359,7 @@ func TestAccMetalPort_hybridBonded(t *testing.T) { } func testAccMetalPortDestroyed(s *terraform.State) error { - client := acceptance.TestAccProvider.Meta().(*config.Config).Metal + client := acceptance.TestAccProvider.Meta().(*config.Config).NewMetalClientForTesting() port_ids := []string{} @@ -371,7 +372,7 @@ func testAccMetalPortDestroyed(s *terraform.State) error { } } for _, pid := range port_ids { - p, _, err := client.Ports.Get(pid, nil) + p, _, err := client.PortsApi.FindPortById(context.Background(), pid).Execute() if err != nil { return fmt.Errorf("Error getting port %s during destroy check", pid) } From 017de288a03f76e3d9e9be6f699a747052cbc912 Mon Sep 17 00:00:00 2001 From: Charles Treatman Date: Tue, 2 Jul 2024 12:14:04 -0500 Subject: [PATCH 4/6] include error when port delete check fails during testing --- internal/resources/metal/port/resource_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/resources/metal/port/resource_test.go b/internal/resources/metal/port/resource_test.go index bbb4b8ae3..ef2f76904 100644 --- a/internal/resources/metal/port/resource_test.go +++ b/internal/resources/metal/port/resource_test.go @@ -374,7 +374,7 @@ func testAccMetalPortDestroyed(s *terraform.State) error { for _, pid := range port_ids { p, _, err := client.PortsApi.FindPortById(context.Background(), pid).Execute() if err != nil { - return fmt.Errorf("Error getting port %s during destroy check", pid) + return fmt.Errorf("Error getting port %s during destroy check: %v", pid, err) } err = port.ProperlyDestroyed(p) if err != nil { From deeae93f9ece1eb0c7ab09e61aa58d2911c210e2 Mon Sep 17 00:00:00 2001 From: Charles Treatman Date: Tue, 2 Jul 2024 13:09:04 -0500 Subject: [PATCH 5/6] fix error check when validating that a port was deleted --- internal/resources/metal/port/resource_test.go | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/internal/resources/metal/port/resource_test.go b/internal/resources/metal/port/resource_test.go index ef2f76904..1766a8c74 100644 --- a/internal/resources/metal/port/resource_test.go +++ b/internal/resources/metal/port/resource_test.go @@ -3,7 +3,9 @@ package port_test import ( "context" "fmt" + "net/http" "regexp" + "slices" "testing" "github.com/equinix/terraform-provider-equinix/internal/acceptance" @@ -372,8 +374,11 @@ func testAccMetalPortDestroyed(s *terraform.State) error { } } for _, pid := range port_ids { - p, _, err := client.PortsApi.FindPortById(context.Background(), pid).Execute() + p, resp, err := client.PortsApi.FindPortById(context.Background(), pid).Execute() if err != nil { + if resp != nil && slices.Contains([]int{http.StatusNotFound, http.StatusForbidden}, resp.StatusCode) { + continue + } return fmt.Errorf("Error getting port %s during destroy check: %v", pid, err) } err = port.ProperlyDestroyed(p) From f057498313ff6b6eaad17521ca05bf4a24b29e8e Mon Sep 17 00:00:00 2001 From: Charles Treatman Date: Fri, 5 Jul 2024 15:19:53 -0500 Subject: [PATCH 6/6] include virtual_networks in VLAN delete so the VLAN IDs are available in the response --- internal/resources/metal/vlan/resource.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/resources/metal/vlan/resource.go b/internal/resources/metal/vlan/resource.go index 9a95d8bd9..80d13d0c1 100644 --- a/internal/resources/metal/vlan/resource.go +++ b/internal/resources/metal/vlan/resource.go @@ -164,7 +164,7 @@ func (r *Resource) Delete(ctx context.Context, request resource.DeleteRequest, r vlan, resp, err := client.ProjectVirtualNetworks.Get( data.ID.ValueString(), - &packngo.GetOptions{Includes: []string{"instances", "meta_gateway"}}, + &packngo.GetOptions{Includes: []string{"instances", "virtual_networks", "meta_gateway"}}, ) if err != nil { if equinix_errors.IgnoreResponseErrors(equinix_errors.HttpForbidden, equinix_errors.HttpNotFound)(resp, err) != nil {