feat: (CXF 90011) Updating Metal & Fabric Provider with NIMF functionality #626

srushti-patl · 2024-03-22T22:35:17Z

Added authorization_code to metal_interconnection schema.
Added buildSharedPortVCVLANCreateRequest() function for the connection type shared_port_vlan request.
Added DataSource and Resource Acc Tests for Vlan Port Share functionality.
Added "METAL_NETWORK" to Fabric connection Access Point Type schema
Updated Fabric Go SDK Version to 0.9.0

…ource

…ource with sharedPort request

codecov-commenter · 2024-03-22T22:37:53Z

Codecov Report

Attention: Patch coverage is 79.24528% with 11 lines in your changes are missing coverage. Please review.

Project coverage is 60.96%. Comparing base (4d0b305) to head (b225209).

Files	Patch %	Lines
internal/resources/metal/connection/resource.go	71.79%	9 Missing and 2 partials ⚠️

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

Additional details and impacted files

@@             Coverage Diff             @@
##             main     #626       +/-   ##
===========================================
+ Coverage   38.00%   60.96%   +22.95%     
===========================================
  Files         120      120               
  Lines       19428    19476       +48     
===========================================
+ Hits         7384    11873     +4489     
+ Misses      11836     7034     -4802     
- Partials      208      569      +361

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

internal/resources/metal/connection/resource_schema.go

ctreatma · 2024-03-25T14:20:29Z

IMO this should have the feat tag instead of fix, since it's adding a previously unsupported feature, not fixing a feature that was already supported in the provider.

internal/resources/metal/connection/resource.go

internal/resources/metal/connection/resource_test.go

…o CXF-90011-Metal-NIMF-Update

adammohammed · 2024-03-26T21:42:28Z

internal/resources/metal/connection/resource_test.go

+					resource.TestCheckResourceAttr(
+						"equinix_metal_connection.test", "metro", "sv"),
+					resource.TestCheckResourceAttr(
+						"equinix_metal_connection.test", "contact_email", "tfacc@example.com"),


Should we test that the authorization_code is populated here?

I have added authorization_code parameter check in acc tests

adammohammed

Will there be a related PR for the Metal2SpAws create body on the Fabric /connections API?

…tests

srushti-patl · 2024-03-28T21:56:47Z

Will there be a related PR for the Metal2SpAws create body on the Fabric /connections API?

We have included the required Fabric Provider changes in this PR only. There is only one change required in the Fabric Provider to support NIMF functionality.
This is the PR for Metal 2 AWS example: https://github.com/equinix/terraform-equinix-fabric/pull/34/files

thogarty

LGTM for Fabric changes. Metal side looks good as well but leaving that for those who have more familiarity with the product side of things.

0xch4z · 2024-03-29T00:30:26Z

might need a rebase, but LGTM

go.mod

ctreatma

I had one question about the fabric-go upgrade, and I don't see the changes Adam suggested for testing that the authorization_code attribute has a value.

…o CXF-90011-Metal-NIMF-Update

srushti-patl · 2024-03-29T22:09:00Z

I had one question about the fabric-go upgrade, and I don't see the changes Adam suggested for testing that the authorization_code attribute has a value.

I have upgraded fabric-go-sdk version to 0.9.0 and I have already added authorization_code parameter check in metal connection resource and data source acc tests with TestCheckResourceAttrSet() function as authorization_code's value is dynamic.

adammohammed · 2024-04-01T14:13:29Z

@ctreatma I see it now, the addition of METAL_NETWORK is all that was required on the Fabric side, since it already had the authentication_key field from other request bodies.

adammohammed

LGTM, though there are some failing acceptance tests

srushti-patl · 2024-04-01T17:56:59Z

Screenshots of resource and data source Acceptance tests passing locally.
The Metal Acceptance test case workflow failing for these two test cases because the user is not whitelisted for NIMF functionality.

ctreatma · 2024-04-01T18:32:56Z

@adammohammed how can we add the CI user to the allow list for NIMF?

adammohammed · 2024-04-01T19:35:59Z

@ctreatma on it. @srushti-patl the CI organization should have this feature available to it now. We should be able to re-run the tests.

ctreatma

I see the new tests passing in run #2 of the Metal acceptance tests. Test failures are all in unrelated tests due to platform capacity ("no available servers in metro"), or tests using recently-disabled features ("facility is not supported"), or tests that are known to flake (spot market tests).

srushti-patl added 2 commits March 21, 2024 15:40

fix: Adding SharedPortVCVlanCreateInput method to interconnection res…

3e8ec06

…ource

fix: Updating acc tests for Metal InterConnection resource and data s…

fdcdfa0

…ource with sharedPort request

srushti-patl requested a review from a team as a code owner March 22, 2024 22:35

srushti-patl temporarily deployed to internal March 22, 2024 22:35 — with GitHub Actions Inactive

srushti-patl changed the title ~~CXF 90011: Updating Metal Provider with NIMF functionality~~ fix: (CXF 90011) Updating Metal Provider with NIMF functionality Mar 22, 2024

Merge branch 'main' into CXF-90011-Metal-NIMF-Update

f34c2e7

srushti-patl temporarily deployed to internal March 22, 2024 22:36 — with GitHub Actions Inactive

ctreatma reviewed Mar 25, 2024

View reviewed changes

internal/resources/metal/connection/resource_schema.go Show resolved Hide resolved

ctreatma reviewed Mar 25, 2024

View reviewed changes

internal/resources/metal/connection/resource_schema.go Show resolved Hide resolved

srushti-patl requested a review from 0xch4z March 26, 2024 16:55

srushti-patl changed the title ~~fix: (CXF 90011) Updating Metal Provider with NIMF functionality~~ feat: (CXF 90011) Updating Metal Provider with NIMF functionality Mar 26, 2024

0xch4z reviewed Mar 26, 2024

View reviewed changes

internal/resources/metal/connection/resource.go Outdated Show resolved Hide resolved

internal/resources/metal/connection/resource_test.go Outdated Show resolved Hide resolved

srushti-patl added 3 commits March 26, 2024 14:06

fix: Adding metal_network type to Fabric Access Point

9ad1db3

nit:Updating formatting

98e9a61

Merge remote-tracking branch 'origin/CXF-90011-Metal-NIMF-Update' int…

3bac0cc

…o CXF-90011-Metal-NIMF-Update

srushti-patl requested review from a team as code owners March 26, 2024 21:08

srushti-patl temporarily deployed to internal March 26, 2024 21:08 — with GitHub Actions Inactive

srushti-patl changed the title ~~feat: (CXF 90011) Updating Metal Provider with NIMF functionality~~ feat: (CXF 90011) Updating Metal & Fabric Provider with NIMF functionality Mar 26, 2024

srushti-patl requested review from ctreatma, 0xch4z and thogarty March 26, 2024 21:14

adammohammed reviewed Mar 28, 2024

View reviewed changes

fix: removing additional property parameter from shared port request

279b07e

srushti-patl temporarily deployed to internal March 28, 2024 21:47 — with GitHub Actions Inactive

fix: Adding authorization_code and redundancy parameter check in acc …

5a65da5

…tests

srushti-patl temporarily deployed to internal March 28, 2024 21:53 — with GitHub Actions Inactive

srushti-patl requested a review from adammohammed March 28, 2024 21:56

thogarty approved these changes Mar 28, 2024

View reviewed changes

0xch4z approved these changes Mar 29, 2024

View reviewed changes

Merge branch 'main' into CXF-90011-Metal-NIMF-Update

8868f87

srushti-patl temporarily deployed to internal March 29, 2024 16:55 — with GitHub Actions Inactive

ctreatma reviewed Mar 29, 2024

View reviewed changes

go.mod Outdated Show resolved Hide resolved

ctreatma reviewed Mar 29, 2024

View reviewed changes

srushti-patl added 2 commits March 29, 2024 15:00

fix: Upgrading fabric-go-sdk version

99f0b2b

Merge remote-tracking branch 'origin/CXF-90011-Metal-NIMF-Update' int…

b225209

…o CXF-90011-Metal-NIMF-Update

srushti-patl temporarily deployed to internal March 29, 2024 22:01 — with GitHub Actions Inactive

srushti-patl requested review from ctreatma and 0xch4z March 29, 2024 22:09

adammohammed approved these changes Apr 1, 2024

View reviewed changes

srushti-patl requested a deployment to internal April 1, 2024 19:44 — with GitHub Actions Abandoned

srushti-patl temporarily deployed to internal April 1, 2024 21:19 — with GitHub Actions Inactive

ctreatma approved these changes Apr 1, 2024

View reviewed changes

srushti-patl merged commit d63009a into main Apr 1, 2024
7 of 10 checks passed

srushti-patl deleted the CXF-90011-Metal-NIMF-Update branch April 1, 2024 21:58

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: (CXF 90011) Updating Metal & Fabric Provider with NIMF functionality #626

feat: (CXF 90011) Updating Metal & Fabric Provider with NIMF functionality #626

srushti-patl commented Mar 22, 2024 •

edited

Loading

codecov-commenter commented Mar 22, 2024 •

edited

Loading

ctreatma commented Mar 25, 2024

adammohammed Mar 26, 2024

srushti-patl Mar 28, 2024

adammohammed left a comment

srushti-patl commented Mar 28, 2024

thogarty left a comment

0xch4z commented Mar 29, 2024

ctreatma left a comment

srushti-patl commented Mar 29, 2024

adammohammed commented Apr 1, 2024

adammohammed left a comment

srushti-patl commented Apr 1, 2024

ctreatma commented Apr 1, 2024 •

edited

Loading

adammohammed commented Apr 1, 2024 •

edited

Loading

ctreatma left a comment

feat: (CXF 90011) Updating Metal & Fabric Provider with NIMF functionality #626

feat: (CXF 90011) Updating Metal & Fabric Provider with NIMF functionality #626

Conversation

srushti-patl commented Mar 22, 2024 • edited Loading

codecov-commenter commented Mar 22, 2024 • edited Loading

Codecov Report

ctreatma commented Mar 25, 2024

adammohammed Mar 26, 2024

Choose a reason for hiding this comment

srushti-patl Mar 28, 2024

Choose a reason for hiding this comment

adammohammed left a comment

Choose a reason for hiding this comment

srushti-patl commented Mar 28, 2024

thogarty left a comment

Choose a reason for hiding this comment

0xch4z commented Mar 29, 2024

ctreatma left a comment

Choose a reason for hiding this comment

srushti-patl commented Mar 29, 2024

adammohammed commented Apr 1, 2024

adammohammed left a comment

Choose a reason for hiding this comment

srushti-patl commented Apr 1, 2024

ctreatma commented Apr 1, 2024 • edited Loading

adammohammed commented Apr 1, 2024 • edited Loading

ctreatma left a comment

Choose a reason for hiding this comment

srushti-patl commented Mar 22, 2024 •

edited

Loading

codecov-commenter commented Mar 22, 2024 •

edited

Loading

ctreatma commented Apr 1, 2024 •

edited

Loading

adammohammed commented Apr 1, 2024 •

edited

Loading