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

feat: Add FCR 2 Metal Redundant Connection Example #108

Merged
merged 10 commits into from
Aug 2, 2024

Conversation

d-bhola
Copy link
Collaborator

@d-bhola d-bhola commented Aug 1, 2024

  • The PR contains a new example for FCR 2 Metal Redundant Connection.
  • This includes one FCR, one Metal Network and two connections - Primary and Secondary (Redundant)
  • The changes are tested in prod and redundant connection is successfully created.

direct_rp_name = var.secondary_direct_rp_name
direct_equinix_ipv4_ip = var.secondary_direct_equinix_ipv4_ip
}
resource "time_sleep" "wait_dl_connection" {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Remove time_sleep and metal connection data source, It's only required when we are adding example to Sanity suite.

@@ -0,0 +1,27 @@
equinix_client_id = "<MyEquinixClientId>"
Copy link
Collaborator

Choose a reason for hiding this comment

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

format terraform.tfvars.example file

@@ -0,0 +1,80 @@
provider "equinix" {
Copy link
Collaborator

@srushti-patl srushti-patl Aug 1, 2024

Choose a reason for hiding this comment

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

Add extra spaces between all code blocks

@d-bhola
Copy link
Collaborator Author

d-bhola commented Aug 1, 2024

Addressed all PR comments 👍

Copy link
Collaborator

@srushti-patl srushti-patl left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Collaborator

@thogarty thogarty left a comment

Choose a reason for hiding this comment

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

Changes requested.


See example usage below for details on how to use this example.

<!-- Begin Example Usage (Do not edit contents) -->
Copy link
Collaborator

Choose a reason for hiding this comment

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

These don't belong. We're not leveraging them anymore. Please remove.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Clarity on comment: we don't need these boundary declarations anymore <!-- Begin Example Usage (Do not edit contents) -->. They were for the previous shell script methods but now that we just use terraform-docs then we won't have any need for these. Hope that clears it up.

Copy link
Collaborator Author

@d-bhola d-bhola Aug 2, 2024

Choose a reason for hiding this comment

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

Addressed with commit

primary_direct_rp_name = "pri_direct_rp"
secondary_direct_rp_name = "sec_direct_rp"
primary_direct_equinix_ipv4_ip = "190.1.1.25/30"
secondary_direct_equinix_ipv4_ip = "190.1.1.9/30"
Copy link
Collaborator

Choose a reason for hiding this comment

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

I would reorder the example variables to match the ordering present in variables.tf files.

Copy link
Collaborator Author

@d-bhola d-bhola Aug 2, 2024

Choose a reason for hiding this comment

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

Addressed with commit

Copy link
Collaborator

@thogarty thogarty left a comment

Choose a reason for hiding this comment

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

LGTM

@thogarty thogarty merged commit 291f1a2 into main Aug 2, 2024
0 of 2 checks passed
@thogarty thogarty deleted the CXF-95866-fcr2metal-redundant branch August 2, 2024 23:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants