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 functionality to pass a custom name for the vpe gateway #362

Merged
merged 10 commits into from
Aug 15, 2023

Conversation

MatthewLemmond
Copy link
Member

Description

Add a new variable to allow for passing a map containing keys which match the service name for which a VPE Gateway will be created corresponding to a value which is the name you want that gateway to have. Example and test for this functionality also created.

Release required?

Identify the type of release. For information about the changes in a semantic versioning release, see Release versioning.

  • No release
  • Patch release (x.x.X)
  • Minor release (x.X.x)
  • Major release (X.x.x)
Release notes content

Add the ability to customize the names of the Gateways being created by the module, in order to pass a name assign a map to var.vpe_names whose keys are the services which you want to have a custom gateway name for, and the corresponding values are the names you want to assign to those gateways.

Run the pipeline

If the CI pipeline doesn't run when you create the PR, the PR requires a user with GitHub collaborators access to run the pipeline.

Run the CI pipeline when the PR is ready for review and you expect tests to pass. Add a comment to the PR with the following text:

/run pipeline

Checklist for reviewers

  • If relevant, a test for the change is included or updated with this PR.
  • If relevant, documentation for the change is included or updated with this PR.

Merge actions for mergers

  • Use a relevant conventional commit message that is based on the PR contents and any release notes provided by the PR author. The commit message determines whether a new version of the module is needed, and if so, which semver increment to use (major, minor, or patch).
  • Merge by using "Squash and merge".

@MatthewLemmond
Copy link
Member Author

/run pipeline

@MatthewLemmond
Copy link
Member Author

/run pipeline

Copy link
Contributor

@shemau shemau left a comment

Choose a reason for hiding this comment

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

The implementation in the main module looks fine.

I would question the whole second example.
During CI tests both others_tests and pr_tests are going to run. Resulting in a second example test that is identical other the names on 3 out of 18 VPE gateways.

The new example code sets and passes the new variable and other_test.pr sends a value. I wonder if a better solution would be to put that code in the existing example and have it called from pr_test with the new map?
My thought is that the coverage is the same, both the old (default name, on 15 of 18 gateways) and the new (passed name, on 3 of 18 gateways) would be tested covering both code paths. This is achieved with only one VPC and ICD instance. Resource consumption is lower and the opportunity to fail is lower (VPC tear down has been a bit flaky at times).

Co-authored-by: Renovate Bot <renovate@whitesourcesoftware.com>
Co-authored-by: Renovate Bot <renovate@whitesourcesoftware.com>
Co-authored-by: Renovate Bot <renovate@whitesourcesoftware.com>
…terratest-wrapper to v1.10.17 (#365)

Co-authored-by: Renovate Bot <renovate@whitesourcesoftware.com>
@MatthewLemmond
Copy link
Member Author

Given the above feedback, I've removed the non-default example folder and moved the 3 custom names into the setup options the default test uses. Given that this is passing a new argument to the test, the upgrade test is going to fail. I'm including here the output of running terraform plan on both the main branch and this branch to show there are no changes in the output from the plan. I'm going to let the upgrade test run and fail then push a commit to skip it just to verify.

Output from main terraform plan:

Changes to Outputs:
  ~ vpe_ips = {
      ~ tmp-ml-vpc-instance-cloud-object-storage = [
          + {
              + address       = "10.20.10.5"
              + id            = "0727-a35c3b9f-67e2-4edb-a073-37df69068871"
              + name          = "excluded-saved-agility-duct"
              + resource_type = "subnet_reserved_ip"
              + subnet        = ""
            },
          + {
              + address       = "10.30.10.4"
              + id            = "0737-6eb41720-7a3b-4490-9e5d-dc5413c79db8"
              + name          = "obstacle-uncross-wiry-essential"
              + resource_type = "subnet_reserved_ip"
              + subnet        = ""
            },
          + {
              + address       = "10.10.10.4"
              + id            = "0717-9d954cf7-5e0c-4c41-96c8-ce5b30ffbc38"
              + name          = "cope-unfrosted-neurotic-eagle"
              + resource_type = "subnet_reserved_ip"
              + subnet        = ""
            },
        ]
      ~ tmp-ml-vpc-instance-kms                  = [
          + {
              + address       = "10.10.10.5"
              + id            = "0717-fe104ec7-7ef8-4a5a-8a2c-984644a3780e"
              + name          = "pupil-guise-sling-septet"
              + resource_type = "subnet_reserved_ip"
              + subnet        = ""
            },
          + {
              + address       = "10.20.10.4"
              + id            = "0727-f4c18626-fb21-4aaf-a96b-86509af23d16"
              + name          = "kerchief-detergent-glimmer-spotted"
              + resource_type = "subnet_reserved_ip"
              + subnet        = ""
            },
          + {
              + address       = "10.30.10.5"
              + id            = "0737-a1bc3a48-fb79-4187-995f-51462768c6a5"
              + name          = "stoppage-freehand-crinkly-lagged"
              + resource_type = "subnet_reserved_ip"
              + subnet        = ""
            },
        ]
      ~ tmp-ml-vpc-instance-postgresql           = [
          + {
              + address       = "10.10.10.6"
              + id            = "0717-77a5ced5-b9ed-4dc8-8265-6cbed95c28ab"
              + name          = "unshattered-overall-matchbook-frogman"
              + resource_type = "subnet_reserved_ip"
              + subnet        = ""
            },
          + {
              + address       = "10.20.10.6"
              + id            = "0727-9b0a2e81-8e14-4792-8be6-cf42d24442df"
              + name          = "ferment-disagree-suffixed-obstinate"
              + resource_type = "subnet_reserved_ip"
              + subnet        = ""
            },
          + {
              + address       = "10.30.10.6"
              + id            = "0737-5e990900-e20b-46da-b1f5-2f722bd5bf45"
              + name          = "far-flashy-enchilada-until"
              + resource_type = "subnet_reserved_ip"
              + subnet        = ""
            },
        ]
    }

Output from vpe-name terraform plan:

Changes to Outputs:
  ~ vpe_ips = {
      ~ tmp-ml-vpc-instance-cloud-object-storage = [
          + {
              + address       = "10.10.10.4"
              + id            = "0717-9d954cf7-5e0c-4c41-96c8-ce5b30ffbc38"
              + name          = "cope-unfrosted-neurotic-eagle"
              + resource_type = "subnet_reserved_ip"
              + subnet        = ""
            },
          + {
              + address       = "10.20.10.5"
              + id            = "0727-a35c3b9f-67e2-4edb-a073-37df69068871"
              + name          = "excluded-saved-agility-duct"
              + resource_type = "subnet_reserved_ip"
              + subnet        = ""
            },
          + {
              + address       = "10.30.10.4"
              + id            = "0737-6eb41720-7a3b-4490-9e5d-dc5413c79db8"
              + name          = "obstacle-uncross-wiry-essential"
              + resource_type = "subnet_reserved_ip"
              + subnet        = ""
            },
        ]
      ~ tmp-ml-vpc-instance-kms                  = [
          + {
              + address       = "10.10.10.5"
              + id            = "0717-fe104ec7-7ef8-4a5a-8a2c-984644a3780e"
              + name          = "pupil-guise-sling-septet"
              + resource_type = "subnet_reserved_ip"
              + subnet        = ""
            },
          + {
              + address       = "10.20.10.4"
              + id            = "0727-f4c18626-fb21-4aaf-a96b-86509af23d16"
              + name          = "kerchief-detergent-glimmer-spotted"
              + resource_type = "subnet_reserved_ip"
              + subnet        = ""
            },
          + {
              + address       = "10.30.10.5"
              + id            = "0737-a1bc3a48-fb79-4187-995f-51462768c6a5"
              + name          = "stoppage-freehand-crinkly-lagged"
              + resource_type = "subnet_reserved_ip"
              + subnet        = ""
            },
        ]
      ~ tmp-ml-vpc-instance-postgresql           = [
          + {
              + address       = "10.20.10.6"
              + id            = "0727-9b0a2e81-8e14-4792-8be6-cf42d24442df"
              + name          = "ferment-disagree-suffixed-obstinate"
              + resource_type = "subnet_reserved_ip"
              + subnet        = ""
            },
          + {
              + address       = "10.30.10.6"
              + id            = "0737-5e990900-e20b-46da-b1f5-2f722bd5bf45"
              + name          = "far-flashy-enchilada-until"
              + resource_type = "subnet_reserved_ip"
              + subnet        = ""
            },
          + {
              + address       = "10.10.10.6"
              + id            = "0717-77a5ced5-b9ed-4dc8-8265-6cbed95c28ab"
              + name          = "unshattered-overall-matchbook-frogman"
              + resource_type = "subnet_reserved_ip"
              + subnet        = ""
            },
        ]
    }

@MatthewLemmond
Copy link
Member Author

/run pipeline

SKIP UPGRADE TEST upgrade test fails due to new variable being passed to the test, skipping
@MatthewLemmond
Copy link
Member Author

/run pipeline

Copy link
Contributor

@shemau shemau left a comment

Choose a reason for hiding this comment

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

LGTM

@ocofaigh ocofaigh merged commit 3bf013b into main Aug 15, 2023
2 checks passed
@ocofaigh ocofaigh deleted the vpe-name branch August 15, 2023 16:29
@terraform-ibm-modules-ops
Copy link
Contributor

🎉 This PR is included in version 2.7.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants