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

fix delete and import for redundant l2 connections #103

Merged
merged 4 commits into from
Mar 2, 2022

Conversation

ocobles
Copy link
Contributor

@ocobles ocobles commented Feb 28, 2022

Description:

Field RedundantUUID was used to check if there was a secondary connection. This field is not been returned for new connections anymore. This field can be deleted from the schema but then it will be required a full scan for all connections in the read function to ensure the terraform import command works also for redundant connections.

From terraform import function docs:

This function requires the Read function to be able to refresh the entire resource with d.Id() ONLY

Without redundantUUID field returned from the API there is no way to know if it is a redundant connection just with the primary connection ID, since all connections both single and redundant have a redundancyGroup. Therefore it will be needed a full scan in the read function to search for a connection with same RedundancyGroup.

To handle that without a full scan I propose to keep RedundantUUID but without using the value returned from the API. Instead value will be set in:

  • Create function
    if ecx.StringValue(secondaryID) != "" {
    d.Set(ecxL2ConnectionSchemaNames["RedundantUUID"], secondaryID)
    waitConfigs = append(waitConfigs,
    createConnectionStatusProvisioningWaitConfiguration(conf.ecx.GetL2Connection, ecx.StringValue(secondaryID), 2*time.Second, d.Timeout(schema.TimeoutCreate)),
    )
    }
  • Importer will require users to specify both primary and secondary ID
    Importer: &schema.ResourceImporter{
    StateContext: func(ctx context.Context, d *schema.ResourceData, meta interface{}) ([]*schema.ResourceData, error) {
    // The expected ID to import redundant connections is '(primaryID):(secondaryID)', e.g.,
    // terraform import equinix_ecx_l2_connection.example 1111-11-11-1111:2222-22-22-2222
    ids := strings.Split(d.Id(), ":")
    d.SetId(ids[0])
    if len(ids) > 1 {
    d.Set(ecxL2ConnectionSchemaNames["RedundantUUID"], ids[1])
    }
    return []*schema.ResourceData{d}, nil
    },
    },

Depends on:

Tasks:

Copy link
Member

@displague displague left a comment

Choose a reason for hiding this comment

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

Without redundantUUID field returned from the API there is no way to know if it is a redundant connection just with the primary connection ID, since all connections both single and redundant have a redundancyGroup.

Should we add a data source for querying a connection based on any combination of the following filters:

  • connection id
  • redundancy group
  • primary/secondary
  • others?

Is this the only way users would be able to get Terraform details for the redundant connection?

@ocobles
Copy link
Contributor Author

ocobles commented Feb 28, 2022

Is this the only way users would be able to get Terraform details for the redundant connection?

Since secondary connection has its own UUID/name it could be retrieved independently:

data "equinix_ecx_l2_connection" "primary" {
  name = var.primary_conn_name
  redundancy_group_id = var.redundancy_group_id
}

data "equinix_ecx_l2_connection" "secondary" {
  name = var.secondary_conn_name
  redundancy_group_id = var.redundancy_group_id
}

@ocobles ocobles marked this pull request as ready for review February 28, 2022 18:40
@codecov-commenter
Copy link

codecov-commenter commented Mar 1, 2022

Codecov Report

Merging #103 (0f6c971) into master (4c6eea2) will decrease coverage by 0.55%.
The diff coverage is 7.07%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #103      +/-   ##
==========================================
- Coverage   61.70%   61.15%   -0.56%     
==========================================
  Files          65       65              
  Lines       11134    11061      -73     
==========================================
- Hits         6870     6764     -106     
- Misses       4013     4044      +31     
- Partials      251      253       +2     
Impacted Files Coverage Δ
cmd/migration-tool/main.go 0.00% <ø> (ø)
equinix/resource_ecx_l2_connection.go 43.78% <7.07%> (-7.73%) ⬇️
equinix/utils.go 72.72% <0.00%> (-15.16%) ⬇️
equinix/port_helpers.go 66.03% <0.00%> (-1.15%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4c6eea2...0f6c971. Read the comment docs.

return nil, "", err
restErr, ok := err.(rest.Error)
if ok {
// IC-LAYER2-4021 = Connection already deleted
Copy link
Member

Choose a reason for hiding this comment

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

nit: this could be ok && ...

@ocobles ocobles merged commit 0d976af into master Mar 2, 2022
@ocobles ocobles deleted the fix-r-ecx_l2_connection-secondary branch March 2, 2022 10:40
thogarty pushed a commit that referenced this pull request Sep 5, 2023
fix delete and import for redundant l2 connections
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants