Fix gateway and vc delete waiting

CHANGELOG.md

@@ -39,7 +39,6 @@ This repository is now Deprecated meaning that this software is only supported o
 * packngo version bumped to 0.25.0
 * CI build cleanup (go bumped to 1.17, vendor directory removed) ([#213](https://github.com/equinix/terraform-provider-metal/pull/213))
 * E2E tests sped up with parallelization and fixing spot market tests
-* E2E tests check plans/facilities to ensure hardware availability ([#239](https://github.com/equinix/terraform-provider-metal/pull/239))
 * E2E tests sweepers will ensure VLAN/projects clean up ([#234](https://github.com/equinix/terraform-provider-metal/pull/234))
 * `organization_id` is now optional in the `metal_connection` resource ([#223](https://github.com/equinix/terraform-provider-metal/pull/223))
 * `ports` attribute in the `metal_connection` data source and resource are sorted by role (primary/secondary) ([#223](https://github.com/equinix/terraform-provider-metal/pull/223))

docs/data-sources/virtual_circuit.md

@@ -38,6 +38,7 @@ In addition to all arguments above, the following attributes are exported:
 
 * `name` - Name of the virtual circuit resource.
 * `status` - Status of the virtal circuit.
+* `port_id` - UUID of the Connection Port where the VC is scoped to.
 * `project_id` - ID of project to which the VC belongs.
 * `vnid`, `nni_vlan`, `nni_nvid` - VLAN parameters, see the [documentation for Equinix Fabric](https://metal.equinix.com/developers/docs/networking/fabric/).
 * `description` - Description for the Virtual Circuit resource.
@@ -50,6 +51,6 @@ In addition to all arguments above, the following attributes are exported:
   * For a /31 block, it will only have two IP addresses, which will be used for
   the metal_ip and customer_ip.
   * For a /30 block, it will have four IP addresses, but the first and last IP addresses are not usable. We will default to the first usable IP address for the metal_ip.
-* `metal_ip` - The IP address that’s set as “our” IP that is configured on the rack_local_vlan SVI. Will default to the first usable IP in the subnet.
-* `customer_ip` - The IP address set as the customer IP which the CSR switch will peer with. Will default to the other usable IP in the subnet.
+* `metal_ip` - The Metal IP address for the SVI (Switch Virtual Interface) of the VirtualCircuit. Will default to the first usable IP in the subnet.
+* `customer_ip` - The Customer IP address which the CSR switch will peer with. Will default to the other usable IP in the subnet.
 * `md5` - The password that can be set for the VRF BGP peer

docs/resources/virtual_circuit.md

@@ -62,8 +62,8 @@ The following arguments are supported:
   * For a /31 block, it will only have two IP addresses, which will be used for
   the metal_ip and customer_ip.
   * For a /30 block, it will have four IP addresses, but the first and last IP addresses are not usable. We will default to the first usable IP address for the metal_ip.
-* `metal_ip` - (Optional, required with `vrf_id`) The IP address that’s set as “our” IP that is configured on the rack_local_vlan SVI (Switch Virtual Interface). Will default to the first usable IP in the subnet.
-* `customer_ip` - (Optional, required with `vrf_id`) The IP address set as the customer IP which the CSR switch will peer with. Will default to the other usable IP in the subnet.
+* `metal_ip` - (Optional, required with `vrf_id`) The Metal IP address for the SVI (Switch Virtual Interface) of the VirtualCircuit. Will default to the first usable IP in the subnet.
+* `customer_ip` - (Optional, required with `vrf_id`) The Customer IP address which the CSR switch will peer with. Will default to the other usable IP in the subnet.
 * `md5` - (Optional, only valid with `vrf_id`) The password that can be set for the VRF BGP peer
 
 ## Attributes Reference

metal/datasource_metal_virtual_circuit.go

@@ -2,7 +2,6 @@ package metal
 
 import (
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
-	"github.com/packethost/packngo"
 )
 
 func dataSourceMetalVirtualCircuit() *schema.Resource {
@@ -56,34 +55,61 @@ func dataSourceMetalVirtualCircuit() *schema.Resource {
 				Computed:    true,
 				Description: "ID of the projct to which the virtual circuit belongs",
 			},
+			"port_id": {
+				Type:        schema.TypeString,
+				Computed:    true,
+				Description: "UUID of the Connection Port where the VC is scoped to",
+			},
+			"speed": {
+				Type:        schema.TypeString,
+				Description: "Description of the Virtual Circuit speed. This is for information purposes and is computed when the connection type is shared.",
+				Computed:    true,
+			},
+			"vlan_id": {
+				Type:        schema.TypeString,
+				Computed:    true,
+				Description: "UUID of the associated VLAN",
+			},
+			"vrf_id": {
+				Type:        schema.TypeString,
+				Computed:    true,
+				Description: "UUID of the associated VRF",
+			},
+			"peer_asn": {
+				Type:        schema.TypeInt,
+				Computed:    true,
+				Description: "The BGP ASN of the peer. The same ASN may be the used across several VCs, but it cannot be the same as the local_asn of the VRF.",
+			},
+			"subnet": {
+				Type:     schema.TypeString,
+				Computed: true,
+				Description: `A subnet from one of the IP blocks associated with the VRF that we will help create an IP reservation for. Can only be either a /30 or /31.
+				 * For a /31 block, it will only have two IP addresses, which will be used for the metal_ip and customer_ip.
+				 * For a /30 block, it will have four IP addresses, but the first and last IP addresses are not usable. We will default to the first usable IP address for the metal_ip.`,
+			},
+			"metal_ip": {
+				Type:        schema.TypeString,
+				Computed:    true,
+				Description: "The Metal IP address for the SVI (Switch Virtual Interface) of the VirtualCircuit. Will default to the first usable IP in the subnet.",
+			},
+			"customer_ip": {
+				Type:        schema.TypeString,
+				Computed:    true,
+				Description: "The Customer IP address which the CSR switch will peer with. Will default to the other usable IP in the subnet.",
+			},
+			"md5": {
+				Type:        schema.TypeString,
+				Computed:    true,
+				Sensitive:   true,
+				Description: "The password that can be set for the VRF BGP peer",
+			},
 		},
 	}
 }
 
 func dataSourceMetalVirtualCircuitRead(d *schema.ResourceData, meta interface{}) error {
-	client := meta.(*packngo.Client)
 	vcId := d.Get("virtual_circuit_id").(string)
 
-	vc, _, err := client.VirtualCircuits.Get(
-		vcId,
-		&packngo.GetOptions{Includes: []string{"project"}})
-	if err != nil {
-		return err
-	}
-
-	d.Set("virtual_circuit_id", vc.ID)
-	d.Set("name", vc.Name)
-	d.Set("status", vc.Status)
-	d.Set("vnid", vc.VNID)
-	d.Set("nni_vnid", vc.NniVNID)
-	d.Set("nni_vlan", vc.NniVLAN)
-	d.Set("project_id", vc.Project.ID)
-	d.Set("description", vc.Description)
-	tags := d.Get("tags.#").(int)
-	if tags > 0 {
-		vc.Tags = convertStringArr(d.Get("tags").([]interface{}))
-	}
-	d.SetId(vc.ID)
-
-	return nil
+	d.SetId(vcId)
+	return resourceMetalVirtualCircuitRead(d, meta)
 }

metal/resource_metal_gateway.go

@@ -2,7 +2,9 @@ package metal
 
 import (
 	"fmt"
+	"time"
 
+	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
 	"github.com/packethost/packngo"
 )
@@ -157,5 +159,38 @@ func resourceMetalGatewayDelete(d *schema.ResourceData, meta interface{}) error
 	if ignoreResponseErrors(httpForbidden, httpNotFound)(resp, err) != nil {
 		return friendlyError(err)
 	}
+	deleteWaiter := getGatewayStateWaiter(
+		client,
+		d.Id(),
+		d.Timeout(schema.TimeoutDelete),
+		[]string{string(packngo.MetalGatewayDeleting)},
+		[]string{},
+	)
+
+	_, err = deleteWaiter.WaitForState()
+	if ignoreResponseErrors(httpForbidden, httpNotFound)(nil, err) != nil {
+		return fmt.Errorf("Error deleting Metal Gateway %s: %s", d.Id(), err)
+	}
+
+	d.SetId("")
 	return nil
 }
+
+func getGatewayStateWaiter(client *packngo.Client, id string, timeout time.Duration, pending, target []string) *resource.StateChangeConf {
+	return &resource.StateChangeConf{
+		Pending: pending,
+		Target:  target,
+		Refresh: func() (interface{}, string, error) {
+			getOpts := &packngo.GetOptions{Includes: []string{"project", "ip_reservation", "virtual_network", "vrf"}}
+
+			gw, _, err := client.MetalGateways.Get(id, getOpts) // TODO: we are not using the returned VRF. Remove the includes?
+			if err != nil {
+				return 0, "", err
+			}
+			return gw, string(gw.State), nil
+		},
+		Timeout:    timeout,
+		Delay:      10 * time.Second,
+		MinTimeout: 5 * time.Second,
+	}
+}

metal/resource_metal_virtual_circuit.go

@@ -82,7 +82,7 @@ func resourceMetalVirtualCircuit() *schema.Resource {
 			"vrf_id": {
 				Type:         schema.TypeString,
 				Optional:     true,
-				Description:  "UUID of the VLAN to associate",
+				Description:  "UUID of the VRF to associate",
 				ExactlyOneOf: []string{"vlan_id", "vrf_id"},
 				ForceNew:     true,
 			},
@@ -105,17 +105,18 @@ func resourceMetalVirtualCircuit() *schema.Resource {
 				Type:         schema.TypeString,
 				Optional:     true,
 				RequiredWith: []string{"vrf_id"},
-				Description:  "The IP address that’s set as “our” IP that is configured on the rack_local_vlan SVI. Will default to the first usable IP in the subnet.",
+				Description:  "The Metal IP address for the SVI (Switch Virtual Interface) of the VirtualCircuit. Will default to the first usable IP in the subnet.",
 			},
 			"customer_ip": {
 				Type:         schema.TypeString,
 				Optional:     true,
 				RequiredWith: []string{"vrf_id"},
-				Description:  "The IP address set as the customer IP which the CSR switch will peer with. Will default to the other usable IP in the subnet.",
+				Description:  "The Customer IP address which the CSR switch will peer with. Will default to the other usable IP in the subnet.",
 			},
 			"md5": {
 				Type:        schema.TypeString,
 				Optional:    true,
+				Sensitive:   true,
 				Description: "The password that can be set for the VRF BGP peer",
 			},
 
@@ -343,7 +344,7 @@ func resourceMetalVirtualCircuitDelete(d *schema.ResourceData, meta interface{})
 		return err
 	}
 
-	// then we delete the VC. VRF VCs will be in the "active" state.
+	// We wait until vc status is not deactivating. VRF VCs will be in the "active" state.
 	detachWaiter := getVCStateWaiter(
 		client,
 		d.Id(),
@@ -354,9 +355,10 @@ func resourceMetalVirtualCircuitDelete(d *schema.ResourceData, meta interface{})
 
 	_, err = detachWaiter.WaitForState()
 	if err != nil {
-		return fmt.Errorf("Error deleting virtual circuit %s: %s", d.Id(), err)
+		return fmt.Errorf("Error waiting for virtual circuit %s status is not deactivating before deleting it: %s", d.Id(), err)
 	}
 
+	// then we delete the VC. VRF VCs will be in the "active" state.
 	resp, err := client.VirtualCircuits.Delete(d.Id())
 	if ignoreResponseErrors(httpForbidden, httpNotFound)(resp, err) != nil {
 		return friendlyError(err)
@@ -371,9 +373,10 @@ func resourceMetalVirtualCircuitDelete(d *schema.ResourceData, meta interface{})
 	)
 
 	_, err = deleteWaiter.WaitForState()
-	if ignoreResponseErrors(httpForbidden, httpNotFound)(resp, err) != nil {
-		return nil
+	if ignoreResponseErrors(httpForbidden, httpNotFound)(nil, err) != nil {
+		return fmt.Errorf("Error deleting virtual circuit %s: %s", d.Id(), err)
 	}
 
-	return fmt.Errorf("Error deleting virtual circuit %s: %s", d.Id(), err)
+	d.SetId("")
+	return nil
 }

metal/resource_metal_virtual_circuit_test.go

@@ -74,39 +74,47 @@ func testAccCheckMetalVirtualCircuitDestroy(s *terraform.State) error {
 	return nil
 }
 
-func testAccMetalVirtualCircuitConfig_Dedicated(randstr string, randint int) string {
+func testAccMetalConnectionConfig_vc(randint int) string {
 	// Dedicated connection in DA metro
 	testConnection := os.Getenv(metalDedicatedConnIDEnvVar)
 
 	return fmt.Sprintf(`
-data "metal_connection" "test" {
-	connection_id = "%[1]s"
+        locals {
+                conn_id = "%s"
+        }
+        data "metal_connection" test {
+            connection_id = local.conn_id
+        }
+        resource "metal_project" "test" {
+            name = "tfacc-conn-pro-%[2]d"
+        }
+        resource "metal_vlan" "test" {
+            project_id = metal_project.test.id
+            metro      = data.metal_connection.test.metro
+			description = "tfacc-vlan test"
+        }
+        resource "metal_virtual_circuit" "test" {
+            name = "tfacc-vc-%[2]d"
+            description = "tfacc-vc-%[2]d"
+            connection_id = data.metal_connection.test.connection_id
+            project_id = metal_project.test.id
+            port_id = data.metal_connection.test.ports[0].id
+            vlan_id = metal_vlan.test.id
+            nni_vlan = %[2]d
+        }
+        `,
+		testConnection, randint)
 }
 
-resource "metal_project" "test" {
-	name = "%[4]s-pro-vc-%[2]s"
-}
-
-resource "metal_vlan" "test" {
-	project_id  = metal_project.test.id
-	metro       = "da"
-	description = "%[4]s-vlan test"
-}
-
-resource "metal_virtual_circuit" "test" {
-	name = "%[4]s-vc-%[2]s"
-	description = "%[4]s-vc-%[2]s"
-	connection_id = data.metal_connection.test.id
-	project_id = metal_project.test.id
-	port_id = data.metal_connection.test.ports[0].id
-	vlan_id = metal_vlan.test.id
-	nni_vlan = %[3]d
-}
-`, testConnection, randstr, randint, tstResourcePrefix)
+func testAccMetalConnectionConfig_vcds(randint int) string {
+	return testAccMetalConnectionConfig_vc(randint) + `
+	data "metal_virtual_circuit" "test" {
+		virtual_circuit_id = metal_virtual_circuit.test.id
+	}
+	`
 }
 
 func TestAccMetalVirtualCircuit_Dedicated(t *testing.T) {
-	rs := acctest.RandString(10)
 	ri := acctest.RandIntRange(1024, 1093)
 
 	resource.ParallelTest(t, resource.TestCase{
@@ -115,7 +123,7 @@ func TestAccMetalVirtualCircuit_Dedicated(t *testing.T) {
 		CheckDestroy: testAccCheckMetalVirtualCircuitDestroy,
 		Steps: []resource.TestStep{
 			{
-				Config: testAccMetalVirtualCircuitConfig_Dedicated(rs, ri),
+				Config: testAccMetalConnectionConfig_vc(ri),
 				Check: resource.ComposeTestCheckFunc(
 					resource.TestCheckResourceAttrPair(
 						"metal_virtual_circuit.test", "vlan_id",
@@ -129,6 +137,32 @@ func TestAccMetalVirtualCircuit_Dedicated(t *testing.T) {
 				ImportStateVerify:       true,
 				ImportStateVerifyIgnore: []string{"connection_id"},
 			},
+			{
+				Config: testAccMetalConnectionConfig_vcds(ri),
+				Check: resource.ComposeTestCheckFunc(
+					resource.TestCheckResourceAttrPair(
+						"metal_virtual_circuit.test", "id",
+						"data.metal_virtual_circuit.test", "virtual_circuit_id",
+					),
+					resource.TestCheckResourceAttrPair(
+						"metal_virtual_circuit.test", "speed",
+						"data.metal_virtual_circuit.test", "speed",
+					),
+
+					resource.TestCheckResourceAttrPair(
+						"metal_virtual_circuit.test", "port_id",
+						"data.metal_virtual_circuit.test", "port_id",
+					),
+					resource.TestCheckResourceAttrPair(
+						"metal_virtual_circuit.test", "vlan_id",
+						"data.metal_virtual_circuit.test", "vlan_id",
+					),
+					resource.TestCheckResourceAttrPair(
+						"metal_virtual_circuit.test", "nni_vlan",
+						"data.metal_virtual_circuit.test", "nni_vlan",
+					),
+				),
+			},
 		},
 	})
 }