feat: use metal-go for device reads (data source and resource) #291

ctreatma · 2023-01-30T21:24:41Z

This updates the device data source to use metal-go instead of
packngo and also updates the device resource to use metal-go
for read operations. As part of this migration, the device data source
and resource gain the new sos_hostname attribute which enables
customers to reduce their reliance on the deprecated facility attribute,
which was previously necessary in order to compute the SOS hostname
for a device.

codecov-commenter · 2023-01-30T22:34:00Z

Codecov Report

Attention: 9 lines in your changes are missing coverage. Please review.

Comparison is base (26b2f3d) 60.11% compared to head (e378823) 60.04%.
Report is 7 commits behind head on main.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #291      +/-   ##
==========================================
- Coverage   60.11%   60.04%   -0.08%     
==========================================
  Files          99       99              
  Lines       20045    20033      -12     
==========================================
- Hits        12051    12028      -23     
- Misses       7691     7696       +5     
- Partials      303      309       +6

Files	Coverage Δ
equinix/errors.go	`86.95% <100.00%> (+0.59%)`	⬆️
equinix/helpers_device.go	`82.26% <97.14%> (-2.65%)`	⬇️
equinix/data_source_metal_device.go	`88.33% <89.47%> (-0.71%)`	⬇️
equinix/resource_metal_device.go	`75.72% <87.50%> (-3.36%)`	⬇️

... and 7 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

equinix/data_source_metal_device.go

displague · 2023-03-06T15:22:06Z

equinix/data_source_metal_device.go

 	d.Set("state", device.State)
 	d.Set("billing_cycle", device.BillingCycle)
-	d.Set("ipxe_script_url", device.IPXEScriptURL)
-	d.Set("always_pxe", device.AlwaysPXE)
+	d.Set("ipxe_script_url", device.IpxeScriptUrl)


Little changes like these in properties between Packngo and MetalGo make me wonder if we have the 'better' pattern in one project or the other.

If "ID", "PXE", "IP", "BGP", "SSH", "MAC", "CIDR", or "URL" is preferable, the codegen in metal-go could take that route with initialisms support (OpenAPITools/openapi-generator#4833 has an associated stale/open PR). (ID isn't capitalized for the same reason, but perhaps we prefer it that way)

If/when the generator has broad support for defining acronyms/initialisms, it may be worth looking into proper capitalization of those. At the moment, the camel-casing is consistent across metal-go, metal-java, and metal-python, so I'd be hesitant to add a bunch of post-patches in one of those (or, even worse, maintain equivalent post-patches for all of them) to change that.

Casing decisions for model names are driven by the spec, and I think that's where we could use more consistency; here's an example where BGP is capitalized differently because of the spec:

https://github.com/equinix-labs/metal-go/blob/7e28dc6f080125526593528336c15de56e0e99bf/metal/v1/model_bgp_session.go

https://github.com/equinix-labs/metal-go/blob/7e28dc6f080125526593528336c15de56e0e99bf/metal/v1/model_bgp_session_input.go

At this point, I'd probably lean towards camel-casing acronyms & initialisms in model names in the spec since that's what the generator would do for property names that contain the same acronyms & initialisms.

Looks like initialism may be fixed.
OpenAPITools/openapi-generator#15083

I added equinix-labs/metal-go#112 to investigate the reservedWordsMapping and see if it supports initialisms without introducing unintended side effects. Given that casing for initialisms is a stylistic issue rather than a functional one, I think it would be ok to adopt metal-go in this project without waiting for the results of that SDK investigation.

equinix/data_source_metal_device.go

displague · 2023-05-25T13:42:38Z

#312

Equinix Metal's use of Deprecation and Sunset headers should be surfaced through Terraform output or logs

It's worth exploring wether we have good access to response headers and a generic method of adding any response Sunset and Deprecation headers to the TF log as warnings. https://developer.hashicorp.com/terraform/tutorials/providers/provider-debug#update-error-messages

equinix/data_source_metal_device.go

ctreatma · 2023-06-27T20:10:49Z

#312

Equinix Metal's use of Deprecation and Sunset headers should be surfaced through Terraform output or logs

It's worth exploring wether we have good access to response headers and a generic method of adding any response Sunset and Deprecation headers to the TF log as warnings. https://developer.hashicorp.com/terraform/tutorials/providers/provider-debug#update-error-messages

This PR has been upgraded to v0.10.0 of metal-go, which replicates packngo behavior of dumping Deprecation and Sunset headers & related links to stdout. It's probably still worth looking into exposing that information in a different, possibly tool-specific way, but I think doing that here would be adding unnecessary scope to the conversion.

This adds a metal-go client to the terraform provider. The metal-go client can be gradually adopted by Metal resources and data sources in this provider to enable the eventual deprecation of packngo. An example of using this client to interact with the Metal API can be found in #291.

displague · 2023-08-16T14:36:46Z

This should be rebased now that #353 is merged.

ctreatma · 2023-08-30T18:05:11Z

BGP data source changes were split out to #368

This could benefit from the helper methods introduced in #366

This adds a metal-go client to the terraform provider. The metal-go client can be gradually adopted by Metal resources and data sources in this provider to enable the eventual deprecation of packngo. An example of using this client to interact with the Metal API can be found in #291.

This adds a metal-go client to the terraform provider. The metal-go client can be gradually adopted by Metal resources and data sources in this provider to enable the eventual deprecation of packngo. An example of using this client to interact with the Metal API can be found in equinix#291.

ctreatma · 2023-09-29T20:21:54Z

equinix/helpers_device.go

-					ni.Host = *ip.Address
-					ni.IPv4SubnetSize = int(*ip.Cidr)
-					ni.PublicIPv4 = *ip.Address
+		if ip.GetManagement() {


This is replacing pointer dereferences with Get...() calls for better protection against nil pointer errors.

ctreatma · 2023-09-29T20:25:34Z

equinix/helpers_device.go

-				ni.PublicIPv6 = ip.Address
-			}
+func getNetworkType(device *metalv1.Device) (*string, error) {
+	pgDevice := packngo.Device{}


The NetworkType device attribute exposed by packngo is not an API-provided attribute; this serves as a shim so we can use the packngo logic to calculate NetworkType. We could instead either duplicate the complete packngo logic here or copy it into metal-go, but my understanding is that NetworkType is not something we want to keep around anyway, so this shim is designed to get us through the SDK transition and then be eliminated when packngo is no longer used in the provider.

ctreatma · 2023-09-29T20:27:18Z

equinix/helpers_device.go

-			"type":   p.Type,
-			"mac":    p.Data.MAC,
-			"bonded": p.Data.Bonded,
+			"name":   p.GetName(),


As with getNetworkInfo, this change replaces the innards of getPorts with those of getPortsMetalGo while also updating the function to use Get...() helpers, since those helpers more closely match the behavior of the packngo implementation.

displague · 2023-10-05T19:09:46Z

🐛 in E2E:

panic: interface conversion: interface {} is int32, not *int32

goroutine 61190 [running]:
github.com/equinix/terraform-provider-equinix/equinix.getDeviceMap.func1(0x1, 0x0)
	/home/runner/work/terraform-provider-equinix/terraform-provider-equinix/equinix/helpers_device.go:244 +0x348
sort.insertionSort_func({0xc000568d10?, 0xc00250b260?}, 0x0, 0x3)
	/opt/hostedtoolcache/go/1.20.8/x64/src/sort/zsortfunc.go:12 +0xb1
sort.stable_func({0xc000568d10?, 0xc00250b260?}, 0x3)
	/opt/hostedtoolcache/go/1.20.8/x64/src/sort/zsortfunc.go:343 +0x7a
sort.SliceStable({0x11f8900?, 0xc0028b5290?}, 0x4?)
	/opt/hostedtoolcache/go/1.20.8/x64/src/sort/slice.go:38 +0xe6
github.com/equinix/terraform-provider-equinix/equinix.getDeviceMap({0xc001c4f0ec, 0xc002ce0820, 0xc001c4f0e8, 0xc003012090, 0xc000c76690, 0xc000673080, 0x0, 0xc0009f73e0, 0x0, 0xc002ce0810, ...})
	/home/runner/work/terraform-provider-equinix/terraform-provider-equinix/equinix/helpers_device.go:243 +0x14a
github.com/equinix/terraform-provider-equinix/equinix.flattenDevice({0x14027a0?, 0xc00106f4a0?}, {0x141359a?, 0x6?}, 0xc000fbbc80?)
	/home/runner/work/terraform-provider-equinix/terraform-provider-equinix/equinix/data_source_metal_devices.go:96 +0x159
github.com/equinix/terraform-provider-equinix/equinix/internal/datalist.dataListResourceRead.func1({0x160af40?, 0xc000a7dec0?}, 0x0?, {0x1371b20, 0xc00230db00})
	/home/runner/work/terraform-provider-equinix/terraform-provider-equinix/equinix/internal/datalist/schema.go:99 +0x2ca
github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema.(*Resource).read(0xc0003ce7e0, {0x160af40, 0xc000a7dec0}, 0xd?, {0x1371b20, 0xc00230db00})
	/home/runner/go/pkg/mod/github.com/hashicorp/terraform-plugin-sdk/[email protected]/helper/schema/resource.go:724 +0x12e
github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema.(*Resource).ReadDataApply(0xc0003ce7e0, {0x160af40, 0xc000a7dec0}, 0xc003010200, {0x1371b20, 0xc00230db00})
	/home/runner/go/pkg/mod/github.com/hashicorp/terraform-plugin-sdk/[email protected]/helper/schema/resource.go:943 +0x145
github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema.(*GRPCProviderServer).ReadDataSource(0xc0028e49c0, {0x160af40?, 0xc000a7de00?}, 0xc0022cd280)
	/home/runner/go/pkg/mod/github.com/hashicorp/terraform-plugin-sdk/[email protected]/helper/schema/grpc_provider.go:1195 +0x38f
github.com/hashicorp/terraform-plugin-go/tfprotov5/tf5server.(*server).ReadDataSource(0xc003d54aa0, {0x160af40?, 0xc000a7d320?}, 0xc0039c0e60)
	/home/runner/go/pkg/mod/github.com/hashicorp/[email protected]/tfprotov5/tf5server/server.go:658 +0x403
github.com/hashicorp/terraform-plugin-go/tfprotov5/internal/tfplugin5._Provider_ReadDataSource_Handler({0x13aa6a0?, 0xc003d54aa0}, {0x160af40, 0xc000a7d320}, 0xc003bb4a10, 0x0)
	/home/runner/go/pkg/mod/github.com/hashicorp/[email protected]/tfprotov5/internal/tfplugin5/tfplugin5_grpc.pb.go:421 +0x170
google.golang.org/grpc.(*Server).processUnaryRPC(0xc0031ac3c0, {0x160f838, 0xc000930340}, 0xc000ccc7e0, 0xc0018f8720, 0x1e53710, 0x0)
	/home/runner/go/pkg/mod/google.golang.org/[email protected]/server.go:1337 +0xdf3
google.golang.org/grpc.(*Server).handleStream(0xc0031ac3c0, {0x160f838, 0xc000930340}, 0xc000ccc7e0, 0x0)
	/home/runner/go/pkg/mod/google.golang.org/[email protected]/server.go:1714 +0xa36
google.golang.org/grpc.(*Server).serveStreams.func1.1()
	/home/runner/go/pkg/mod/google.golang.org/[email protected]/server.go:959 +0x98
created by google.golang.org/grpc.(*Server).serveStreams.func1
	/home/runner/go/pkg/mod/google.golang.org/[email protected]/server.go:957 +0x18c

ctreatma · 2023-10-10T14:32:40Z

🐛 in E2E:

Missed a conversion from pointer derefs to values when collapsing the duplicate helpers; it has been fixed.

This updates the device data source to use `metal-go` instead of `packngo`. As part of this migration, the device data source gains the new `sos_hostname` attribute which enables customers to reduce their reliance on the deprecated `facility` attribute, which was previously necessary in order to compute the SOS hostname for a device.

This reverts commit a26f1b945d5f089dd9cac7b086e5ca75e579abe2.

…esource PR

displague · 2023-10-25T15:44:52Z

equinix/data_source_metal_device.go

+func findDeviceByHostname(devices *metalv1.DeviceList, hostname string) (*metalv1.Device, error) {
+	results := make([]metalv1.Device, 0)
+	for _, d := range devices.GetDevices() {
+		if *d.Hostname == hostname {


nit: presumably this will never be null in a returned devices list, so this should be fine. GetHostname() is an option.

ctreatma force-pushed the metal-go branch from 3bf75f5 to 69d885f Compare February 23, 2023 21:15

displague reviewed Mar 6, 2023

View reviewed changes

equinix/data_source_metal_device.go Outdated Show resolved Hide resolved

displague reviewed Mar 6, 2023

View reviewed changes

equinix/data_source_metal_device.go Outdated Show resolved Hide resolved

ctreatma force-pushed the metal-go branch from 69d885f to de56386 Compare May 9, 2023 21:26

This was referenced May 25, 2023

Adopt metal-go equinix/metal-cli#270

Closed

Deprecate Packngo equinix-labs/metal-go#99

Closed

ctreatma force-pushed the metal-go branch 2 times, most recently from 893394f to 43e0318 Compare June 5, 2023 14:53

ctreatma force-pushed the metal-go branch from 43e0318 to 04d4c2d Compare June 12, 2023 19:02

ctreatma force-pushed the metal-go branch from 04d4c2d to 718317a Compare June 23, 2023 19:02

ctreatma marked this pull request as ready for review June 23, 2023 19:02

ctreatma changed the title ~~WIP start using metal-go~~ Use metal-go for the device and BGP neighbors data sources Jun 23, 2023

ctreatma changed the title ~~Use metal-go for the device and BGP neighbors data sources~~ Use metal-go for the device and BGP neighbors data sources Jun 23, 2023

ctreatma commented Jun 23, 2023

View reviewed changes

equinix/data_source_metal_device.go Show resolved Hide resolved

ctreatma force-pushed the metal-go branch from 718317a to c521c51 Compare June 27, 2023 20:05

ctreatma force-pushed the metal-go branch from c521c51 to dc617c1 Compare July 24, 2023 21:00

ctreatma mentioned this pull request Aug 7, 2023

feat: introduce metal-go client #353

Merged

ctreatma force-pushed the metal-go branch from dc617c1 to f4fb9ea Compare August 21, 2023 20:09

ctreatma force-pushed the metal-go branch from f4fb9ea to 07dc327 Compare August 30, 2023 18:03

ctreatma force-pushed the metal-go branch from 07dc327 to 120ef2e Compare September 7, 2023 18:54

ctreatma commented Sep 29, 2023

View reviewed changes

ctreatma force-pushed the metal-go branch from c092059 to 613016b Compare October 5, 2023 17:21

ctreatma temporarily deployed to internal October 5, 2023 17:21 — with GitHub Actions Inactive

ctreatma temporarily deployed to internal October 5, 2023 23:43 — with GitHub Actions Inactive

ctreatma force-pushed the metal-go branch from f3abe2e to 9a912ca Compare October 5, 2023 23:47

ctreatma temporarily deployed to internal October 5, 2023 23:47 — with GitHub Actions Inactive

ctreatma force-pushed the metal-go branch from 9a912ca to 209143e Compare October 17, 2023 19:39

ctreatma temporarily deployed to internal October 17, 2023 19:39 — with GitHub Actions Inactive

ctreatma requested a deployment to internal October 17, 2023 20:38 — with GitHub Actions Abandoned

ctreatma added 11 commits October 24, 2023 10:33

Revert "Revert "[WIP] Use metal-go for device resource""

4a6d54f

This reverts commit a26f1b945d5f089dd9cac7b086e5ca75e579abe2.

Replicate friendlyError for metal-go responses

eb0f974

clean up pointer derefs and rename helpers to use a common suffix

243f372

Document the sos_hostname attribute

7b79a23

update device data source helper references to match names from the r…

8eb1424

…esource PR

convert time to RFC3339 to match what packngo did

ff04f9c

fix error check in networktype shim

743119e

consolidate helper functions that were duplicated across PRs

f80eb68

remove no-longer-needed MetalGo suffix from device helpers

cfb80a7

clean up some lingering pointer operations

e378823

ctreatma force-pushed the metal-go branch from 209143e to e378823 Compare October 24, 2023 15:33

ctreatma temporarily deployed to internal October 24, 2023 15:34 — with GitHub Actions Inactive

displague reviewed Oct 25, 2023

View reviewed changes

displague approved these changes Oct 25, 2023

View reviewed changes

displague merged commit 7f62d83 into main Oct 25, 2023
4 of 5 checks passed

displague deleted the metal-go branch October 25, 2023 15:50

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: use metal-go for device reads (data source and resource) #291

feat: use metal-go for device reads (data source and resource) #291

ctreatma commented Jan 30, 2023 •

edited

Loading

codecov-commenter commented Jan 30, 2023 •

edited

Loading

displague Mar 6, 2023 •

edited

Loading

ctreatma Mar 6, 2023

displague May 31, 2023

ctreatma Jun 30, 2023

displague commented May 25, 2023

ctreatma commented Jun 27, 2023

displague commented Aug 16, 2023

ctreatma commented Aug 30, 2023

ctreatma Sep 29, 2023

ctreatma Sep 29, 2023

ctreatma Sep 29, 2023

displague commented Oct 5, 2023 •

edited

Loading

ctreatma commented Oct 10, 2023

displague Oct 25, 2023

feat: use metal-go for device reads (data source and resource) #291

feat: use metal-go for device reads (data source and resource) #291

Conversation

ctreatma commented Jan 30, 2023 • edited Loading

codecov-commenter commented Jan 30, 2023 • edited Loading

Codecov Report

displague Mar 6, 2023 • edited Loading

Choose a reason for hiding this comment

ctreatma Mar 6, 2023

Choose a reason for hiding this comment

displague May 31, 2023

Choose a reason for hiding this comment

ctreatma Jun 30, 2023

Choose a reason for hiding this comment

displague commented May 25, 2023

ctreatma commented Jun 27, 2023

displague commented Aug 16, 2023

ctreatma commented Aug 30, 2023

ctreatma Sep 29, 2023

Choose a reason for hiding this comment

ctreatma Sep 29, 2023

Choose a reason for hiding this comment

ctreatma Sep 29, 2023

Choose a reason for hiding this comment

displague commented Oct 5, 2023 • edited Loading

ctreatma commented Oct 10, 2023

displague Oct 25, 2023

Choose a reason for hiding this comment

ctreatma commented Jan 30, 2023 •

edited

Loading

codecov-commenter commented Jan 30, 2023 •

edited

Loading

displague Mar 6, 2023 •

edited

Loading

displague commented Oct 5, 2023 •

edited

Loading