-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
Add fine-grained network endpoint to compute NEGs #1846
Add fine-grained network endpoint to compute NEGs #1846
Conversation
Hi! I'm the modular magician, I work on Magic Modules. Pull request statusesNo diff detected in terraform-google-conversion. New Pull RequestsI built this PR into one or more new PRs on other repositories, and when those are closed, this PR will also be merged and closed. |
Hi! I'm the modular magician, I work on Magic Modules. Pull request statusesterraform-provider-google-beta already has an open PR. New Pull RequestsI didn't open any new pull requests because of this PR. |
f76f8e6
to
1b646d8
Compare
Hi! I'm the modular magician, I work on Magic Modules. Pull request statusesterraform-provider-google-beta already has an open PR. New Pull RequestsI didn't open any new pull requests because of this PR. |
Hi! I'm the modular magician, I work on Magic Modules. Pull request statusesterraform-provider-google-beta already has an open PR. New Pull RequestsI didn't open any new pull requests because of this PR. |
d37697f
to
6ebcbee
Compare
Hi! I'm the modular magician, I work on Magic Modules. Pull request statusesterraform-provider-google-beta already has an open PR. New Pull RequestsI didn't open any new pull requests because of this PR. |
6ebcbee
to
f60a7e0
Compare
Hi! I'm the modular magician, I work on Magic Modules. Pull request statusesterraform-provider-google-beta already has an open PR. New Pull RequestsI didn't open any new pull requests because of this PR. |
Hi! I'm the modular magician, I work on Magic Modules. Pull request statusesterraform-provider-google-beta already has an open PR. New Pull RequestsI didn't open any new pull requests because of this PR. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Generally makes sense, couple nitpicks and a question. Also running the test in CI rn before 👍, I'm just curious about the API.
Providers: testAccProvidersOiCS, | ||
Steps: []resource.TestStep{ | ||
{ | ||
Config: testAccComputeNetworkEndpoint_networkEndpointsBasic(context), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you add a case before this with multiple NEGs, so we test the mutex & test deleting a single NEG when we move to this step?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nope, not until we get to GA provider tests! If you delete any of them, the next step automatically assumes the resource that was removed from config needs provider "google".
I can add multiple to test the mutex, but honestly I was just gonna add more tests that are actually good once I switch this over to GA provider.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(both NEG and endpoints are in GA API already)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since we probably want to add a multi-stage test for this regardless of version, should we use the normal acceptance test provider set here so that google
is whichever version of the provider we're using?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh, i misunderstood what testAccProviders/OiCS does - so if I just remove the google-beta blocks it will work and still use the google-beta provider in the test?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep! Using the OiCS provider set tests that the google-beta
provider is specified on the resources explicitly to make sure the OiCS examples work.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 fixed test then
Hi! I'm the modular magician, I work on Magic Modules. Pull request statusesterraform-provider-google-beta already has an open PR. New Pull RequestsI didn't open any new pull requests because of this PR. |
Hi! I'm the modular magician, I work on Magic Modules. Pull request statusesterraform-provider-google-beta already has an open PR. New Pull RequestsI didn't open any new pull requests because of this PR. |
Hi! I'm the modular magician, I work on Magic Modules. Pull request statusesterraform-provider-google-beta already has an open PR. New Pull RequestsI didn't open any new pull requests because of this PR. |
Hi! I'm the modular magician, I work on Magic Modules. Pull request statusesterraform-provider-google-beta already has an open PR. New Pull RequestsI didn't open any new pull requests because of this PR. |
4e5751d
to
d15836f
Compare
Hi! I'm the modular magician, I work on Magic Modules. Pull request statusesterraform-provider-google-beta already has an open PR. New Pull RequestsI didn't open any new pull requests because of this PR. |
Removing myself since it looks like Riley already did the review, but feel free to add me back if you'd like an additional opinion |
Tracked submodules are build/terraform-beta build/terraform-mapper build/terraform build/ansible build/inspec.
d15836f
to
1026a35
Compare
Relies on #1845
[all]
[terraform]
Add fine-grained network endpoint to compute NEGs
[terraform-beta]
Add fine-grained network endpoint to compute NEGs
[ansible]
[inspec]