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

refactor: move device resource and datasource to separate package #574

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
120 changes: 120 additions & 0 deletions equinix/common_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,120 @@
package equinix

import (
"fmt"
"strings"
"time"
)

// list of plans and metros and os used as filter criteria to find available hardware to run tests
var (
preferable_plans = []string{"x1.small.x86", "t1.small.x86", "c2.medium.x86", "c3.small.x86", "c3.medium.x86", "m3.small.x86"}
preferable_metros = []string{"ch", "ny", "sv", "ty", "am"}
preferable_os = []string{"ubuntu_20_04"}
)

// Deprecated: use the identical TestDeviceTerminationTime from internal/acceptance instead
func testDeviceTerminationTime() string {
return time.Now().UTC().Add(60 * time.Minute).Format(time.RFC3339)
}

// This function should be used to find available plans in all test where a metal_device resource is needed.
//
// TODO consider adding a datasource for equinix_metal_operating_system and making the local.os conditional
//
// https://github.com/equinix/terraform-provider-equinix/pull/220#discussion_r915418418equinix_metal_operating_system
// https://github.com/equinix/terraform-provider-equinix/discussions/221
func confAccMetalDevice_base(plans, metros, os []string) string {
return fmt.Sprintf(`
data "equinix_metal_plans" "test" {
sort {
attribute = "id"
direction = "asc"
}

filter {
attribute = "name"
values = [%s]
}
filter {
attribute = "available_in_metros"
values = [%s]
}
filter {
attribute = "deployment_types"
values = ["on_demand", "spot_market"]
}
}

// Select a metal plan randomly and lock it in
// so that we don't pick a different one for
// every subsequent terraform plan
resource "random_integer" "plan_idx" {
min = 0
max = length(data.equinix_metal_plans.test.plans) - 1
}

resource "terraform_data" "plan" {
input = data.equinix_metal_plans.test.plans[random_integer.plan_idx.result]

lifecycle {
ignore_changes = ["input"]
}
}

resource "terraform_data" "facilities" {
input = sort(tolist(setsubtract(terraform_data.plan.output.available_in, ["nrt1", "dfw2", "ewr1", "ams1", "sjc1", "ld7", "sy4", "ny6"])))

lifecycle {
ignore_changes = ["input"]
}
}

// Select a metal facility randomly and lock it in
// so that we don't pick a different one for
// every subsequent terraform plan
resource "random_integer" "facility_idx" {
min = 0
max = length(local.facilities) - 1
}

resource "terraform_data" "facility" {
input = local.facilities[random_integer.facility_idx.result]

lifecycle {
ignore_changes = ["input"]
}
}

// Select a metal metro randomly and lock it in
// so that we don't pick a different one for
// every subsequent terraform plan
resource "random_integer" "metro_idx" {
min = 0
max = length(local.metros) - 1
}

resource "terraform_data" "metro" {
input = local.metros[random_integer.metro_idx.result]

lifecycle {
ignore_changes = ["input"]
}
}

locals {
// Select a random plan
plan = terraform_data.plan.output.slug

// Select a random facility from the facilities in which the selected plan is available, excluding decommed facilities
facilities = terraform_data.facilities.output
facility = terraform_data.facility.output

// Select a random metro from the metros in which the selected plan is available
metros = sort(tolist(terraform_data.plan.output.available_in_metros))
metro = terraform_data.metro.output

os = [%s][0]
}
`, fmt.Sprintf("\"%s\"", strings.Join(plans[:], `","`)), fmt.Sprintf("\"%s\"", strings.Join(metros[:], `","`)), fmt.Sprintf("\"%s\"", strings.Join(os[:], `","`)))
}
28 changes: 23 additions & 5 deletions equinix/data_source_metal_device_bgp_neighbors_acc_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,15 +9,15 @@ import (
)

func TestAccDataSourceMetalDeviceBgpNeighbors(t *testing.T) {
projectName := fmt.Sprintf("ds-device-%s", acctest.RandString(10))
projSuffix := fmt.Sprintf("ds-device-%s", acctest.RandString(10))

resource.ParallelTest(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
ExternalProviders: testExternalProviders,
ProtoV5ProviderFactories: testAccProtoV5ProviderFactories,
Steps: []resource.TestStep{
{
Config: testAccDataSourceMetalDeviceBgpNeighborsConfig(projectName),
Config: testAccDataSourceMetalDeviceBgpNeighborsConfig(projSuffix),
Check: resource.ComposeTestCheckFunc(
resource.TestCheckResourceAttrSet(
"data.equinix_metal_device_bgp_neighbors.test", "bgp_neighbors.#"),
Expand All @@ -27,16 +27,34 @@ func TestAccDataSourceMetalDeviceBgpNeighbors(t *testing.T) {
})
}

func testAccDataSourceMetalDeviceBgpNeighborsConfig(projectName string) string {
func testAccDataSourceMetalDeviceBgpNeighborsConfig(projSuffix string) string {
return fmt.Sprintf(`
%s

resource "equinix_metal_project" "test" {
name = "tfacc-project-%s"
}

resource "equinix_metal_device" "test" {
hostname = "tfacc-test-device"
plan = local.plan
metro = local.metro
operating_system = local.os
billing_cycle = "hourly"
project_id = "${equinix_metal_project.test.id}"
termination_time = "%s"
}

data "equinix_metal_device" "test" {
project_id = equinix_metal_project.test.id
hostname = equinix_metal_device.test.hostname
}

data "equinix_metal_device_bgp_neighbors" "test" {
device_id = equinix_metal_device.test.id
}

output "bgp_neighbors_listing" {
value = data.equinix_metal_device_bgp_neighbors.test.bgp_neighbors
}
`, testDataSourceMetalDeviceConfig_basic(projectName))
}`, confAccMetalDevice_base(preferable_plans, preferable_metros, preferable_os), projSuffix, testDeviceTerminationTime())
}
7 changes: 4 additions & 3 deletions equinix/provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"time"

"github.com/equinix/terraform-provider-equinix/internal/config"
metal_device "github.com/equinix/terraform-provider-equinix/internal/resources/metal/device"
metal_project "github.com/equinix/terraform-provider-equinix/internal/resources/metal/project"
"github.com/equinix/terraform-provider-equinix/internal/resources/metal/vrf"

Expand Down Expand Up @@ -102,8 +103,8 @@ func Provider() *schema.Provider {
"equinix_metal_operating_system": dataSourceOperatingSystem(),
"equinix_metal_organization": dataSourceMetalOrganization(),
"equinix_metal_spot_market_price": dataSourceSpotMarketPrice(),
"equinix_metal_device": dataSourceMetalDevice(),
"equinix_metal_devices": dataSourceMetalDevices(),
"equinix_metal_device": metal_device.DataSource(),
"equinix_metal_devices": metal_device.ListDataSource(),
"equinix_metal_device_bgp_neighbors": dataSourceMetalDeviceBGPNeighbors(),
"equinix_metal_plans": dataSourceMetalPlans(),
"equinix_metal_port": dataSourceMetalPort(),
Expand Down Expand Up @@ -132,7 +133,7 @@ func Provider() *schema.Provider {
"equinix_network_file": resourceNetworkFile(),
"equinix_metal_user_api_key": resourceMetalUserAPIKey(),
"equinix_metal_project_api_key": resourceMetalProjectAPIKey(),
"equinix_metal_device": resourceMetalDevice(),
"equinix_metal_device": metal_device.Resource(),
"equinix_metal_device_network_type": resourceMetalDeviceNetworkType(),
"equinix_metal_organization_member": resourceMetalOrganizationMember(),
"equinix_metal_port": resourceMetalPort(),
Expand Down
5 changes: 5 additions & 0 deletions equinix/resource_metal_spot_market_request.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"fmt"
"log"
"reflect"
"regexp"
"sort"
"strconv"
"time"
Expand All @@ -22,6 +23,10 @@ import (
"github.com/packethost/packngo"
)

var (
matchIPXEScript = regexp.MustCompile(`(?i)^#![i]?pxe`)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Chose to duplicate this matcher instead of finding a place to put it. IMO we can clean that up after framework and equinix-sdk-go migrations are done.

Copy link
Member

@displague displague Feb 22, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think the validation this enforces is necessary in the Terraform provider.

if matchIPXEScript.MatchString(params.UserData) {
return diag.Errorf("\"user_data\" should not be an iPXE " +
"script when \"ipxe_script_url\" is also provided.")
}

Perhaps this block would be better as a warning or left to the API to enforce. It's well enough to leave this validation alone since there are no known edge cases where the combination of an IPXE URL and a Custom IPXE Userdata are necessary.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps the regexp could be moved closer to where it is used since it is not used elsewhere.

)

func resourceMetalSpotMarketRequest() *schema.Resource {
return &schema.Resource{
CreateContext: resourceMetalSpotMarketRequestCreate,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package equinix
package device

import (
"context"
Expand All @@ -17,7 +17,7 @@ import (
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/structure"
)

func dataSourceMetalDevice() *schema.Resource {
func DataSource() *schema.Resource {
return &schema.Resource{
ReadWithoutTimeout: dataSourceMetalDeviceRead,
Schema: map[string]*schema.Schema{
Expand Down
Original file line number Diff line number Diff line change
@@ -1,24 +1,25 @@
package equinix
package device_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"
)

func TestAccDataSourceMetalDevice_basic(t *testing.T) {
projectName := fmt.Sprintf("ds-device-%s", acctest.RandString(10))
projSuffix := fmt.Sprintf("ds-device-%s", 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: testAccMetalDeviceCheckDestroyed,
Steps: []resource.TestStep{
{
Config: testDataSourceMetalDeviceConfig_basic(projectName),
Config: testDataSourceMetalDeviceConfig_basic(projSuffix),
Check: resource.ComposeTestCheckFunc(
resource.TestCheckResourceAttr(
"data.equinix_metal_device.test", "hostname", "tfacc-test-device"),
Expand Down Expand Up @@ -59,20 +60,20 @@ resource "equinix_metal_device" "test" {
data "equinix_metal_device" "test" {
project_id = equinix_metal_project.test.id
hostname = equinix_metal_device.test.hostname
}`, confAccMetalDevice_base(preferable_plans, preferable_metros, preferable_os), projSuffix, testDeviceTerminationTime())
}`, acceptance.ConfAccMetalDevice_base(acceptance.Preferable_plans, acceptance.Preferable_metros, acceptance.Preferable_os), projSuffix, acceptance.TestDeviceTerminationTime())
}

func TestAccDataSourceMetalDevice_byID(t *testing.T) {
projectName := fmt.Sprintf("ds-device-by-id-%s", acctest.RandString(10))
projSuffix := fmt.Sprintf("ds-device-by-id-%s", 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: testAccMetalDeviceCheckDestroyed,
Steps: []resource.TestStep{
{
Config: testDataSourceMetalDeviceConfig_byID(projectName),
Config: testDataSourceMetalDeviceConfig_byID(projSuffix),
Check: resource.ComposeTestCheckFunc(
resource.TestCheckResourceAttr(
"data.equinix_metal_device.test", "hostname", "tfacc-test-device"),
Expand Down Expand Up @@ -112,5 +113,5 @@ resource "equinix_metal_device" "test" {

data "equinix_metal_device" "test" {
device_id = equinix_metal_device.test.id
}`, confAccMetalDevice_base(preferable_plans, preferable_metros, preferable_os), projSuffix, testDeviceTerminationTime())
}`, acceptance.ConfAccMetalDevice_base(acceptance.Preferable_plans, acceptance.Preferable_metros, acceptance.Preferable_os), projSuffix, acceptance.TestDeviceTerminationTime())
}
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package equinix
package device

import (
"context"
Expand Down Expand Up @@ -131,7 +131,7 @@ func hwReservationStateRefreshFunc(ctx context.Context, client *metalv1.APIClien
}
}

func waitUntilReservationProvisionable(ctx context.Context, client *metalv1.APIClient, reservationId, instanceId string, delay, timeout, minTimeout time.Duration) error {
func WaitUntilReservationProvisionable(ctx context.Context, client *metalv1.APIClient, reservationId, instanceId string, delay, timeout, minTimeout time.Duration) error {
stateConf := &retry.StateChangeConf{
Pending: []string{deprovisioning},
Target: []string{provisionable, reprovisioned},
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package equinix
package device_test

import (
"context"
Expand All @@ -10,9 +10,10 @@ import (

"github.com/equinix/equinix-sdk-go/services/metalv1"
"github.com/equinix/terraform-provider-equinix/internal/config"
"github.com/equinix/terraform-provider-equinix/internal/resources/metal/device"
)

func Test_waitUntilReservationProvisionable(t *testing.T) {
func Test_WaitUntilReservationProvisionable(t *testing.T) {
type args struct {
reservationId string
instanceId string
Expand Down Expand Up @@ -162,7 +163,7 @@ func Test_waitUntilReservationProvisionable(t *testing.T) {
meta.Load(ctx)

client := meta.NewMetalClientForTesting()
if err := waitUntilReservationProvisionable(ctx, client, tt.args.reservationId, tt.args.instanceId, 50*time.Millisecond, 1*time.Second, 50*time.Millisecond); (err != nil) != tt.wantErr {
if err := device.WaitUntilReservationProvisionable(ctx, client, tt.args.reservationId, tt.args.instanceId, 50*time.Millisecond, 1*time.Second, 50*time.Millisecond); (err != nil) != tt.wantErr {
t.Errorf("waitUntilReservationProvisionable() error = %v, wantErr %v", err, tt.wantErr)
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package equinix
package device

import (
"context"
Expand All @@ -11,8 +11,8 @@ import (
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
)

func dataSourceMetalDevices() *schema.Resource {
dsmd := dataSourceMetalDevice()
func ListDataSource() *schema.Resource {
dsmd := DataSource()
sch := dsmd.Schema
Comment on lines -14 to +15
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I opted to move the metal_devices data source into the same directory as the metal_device resource and datasource, because I think that's better than putting it in a separate internal/resources/metal/devices directory.

If that is agreed to be a good approach, then that raises the question: should metal_device_bgp_neighbors also go in this directory, or does it still go in a separate internal/resources/metal/device_bgp_neighbors directory?

Copy link
Member

@displague displague Feb 28, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The pluralization of the resource names, as in multi-response datasources, was a subtle point that I missed on first glance of this comment.

The AWS provider followed this pattern. Seems fine to me.
https://github.com/hashicorp/terraform-provider-aws/blob/main/internal/service/ec2/ec2_instances_data_source.go

for _, v := range sch {
if v.Optional {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
package equinix
package device_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"
)
Expand All @@ -12,9 +13,9 @@ func TestAccDataSourceMetalDevices(t *testing.T) {
projectName := fmt.Sprintf("ds-device-%s", 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: testAccMetalDeviceCheckDestroyed,
Steps: []resource.TestStep{
{
Expand Down Expand Up @@ -78,5 +79,5 @@ data "equinix_metal_devices" "test_search" {
project_id = equinix_metal_project.test.id
search = "unlikelystring"
depends_on = [equinix_metal_device.dev_search]
}`, confAccMetalDevice_base(preferable_plans, preferable_metros, preferable_os), projSuffix, testDeviceTerminationTime())
}`, acceptance.ConfAccMetalDevice_base(acceptance.Preferable_plans, acceptance.Preferable_metros, acceptance.Preferable_os), projSuffix, acceptance.TestDeviceTerminationTime())
}
Loading
Loading