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

Add support for INTERNAL_SELF_MANAGED backend service #1833

Conversation

slevenick
Copy link
Contributor

Add support for INTERNAL_SELF_MANAGED backend service and global forwarding rules


[all]

[terraform]

[terraform-beta]

Add support for internal self managed load balancing

[ansible]

[inspec]

@modular-magician
Copy link
Collaborator

Hi! I'm the modular magician, I work on Magic Modules.
This PR seems not to have generated downstream PRs before, as of 707c88c.

Pull request statuses

New Pull Requests

I 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.
depends: hashicorp/terraform-provider-google-beta#772
depends: GoogleCloudPlatform/terraform-google-conversion#88
depends: hashicorp/terraform-provider-google#3719
depends: modular-magician/ansible#265
depends: modular-magician/inspec-gcp#158

Copy link
Contributor

@danawillow danawillow left a comment

Choose a reason for hiding this comment

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

Ooh I think this means that allowing internal load balancing with global backend services. Can you take a look at #1689 and see if we need to update docs that were changed then?

Also @rileykarson fyi

@slevenick
Copy link
Contributor Author

Ooh I think this means that allowing internal load balancing with global backend services. Can you take a look at #1689 and see if we need to update docs that were changed then?

Also @rileykarson fyi

I don't think we need to change any of those back. I've been testing settings like balancing_mode = "RATE" and other settings that previously were disallowed for internal balancers. From what I understand the INTERNAL_SELF_MANAGED balancers allow HTTP(s) and all of the settings that were previously only allowed on global external backend services

@modular-magician
Copy link
Collaborator

Hi! I'm the modular magician, I work on Magic Modules.
I see that this PR has already had some downstream PRs generated. Any open downstreams are already updated to your most recent commit, c337d1a.

Pull request statuses

terraform-provider-google-beta already has an open PR.
terraform-google-conversion already has an open PR.
terraform-provider-google already has an open PR.
Ansible already has an open PR.
InSpec already has an open PR.

New Pull Requests

I didn't open any new pull requests because of this PR.

@slevenick slevenick requested a review from danawillow May 28, 2019 22:38
@@ -0,0 +1,100 @@
resource "google_compute_global_forwarding_rule" "default" {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Does it make sense for this file to be part of the examples? I can also make it a separate test file.

What is best practice on this sort of thing?

Copy link
Contributor

Choose a reason for hiding this comment

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

Unfortunately it's a bit of a best-judgment thing right now. I'd say for this one since global forwarding rules need backend services but not the other way around, that this example belongs with global forwarding rule and not backend service. My inclination is that the docs page should have realistic-ish examples, but not too many of them. So since this reads as a pretty typical way to set up load balancing, I'd say putting it in the docs is perfectly reasonable, but adding 5 more examples with different tweaks of the configuration wouldn't be.

One thing to be aware of with beta fields are that the generated tests don't have the import step, so it's a good idea to verify those manually as well.

Also it looks like the GFR docs didn't get generated for some reason, not really sure what's going on there.

@@ -0,0 +1,59 @@
resource "google_compute_global_forwarding_rule" "default" {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Same as above

@modular-magician
Copy link
Collaborator

Hi! I'm the modular magician, I work on Magic Modules.
I see that this PR has already had some downstream PRs generated. Any open downstreams are already updated to your most recent commit, add1834.

Pull request statuses

terraform-provider-google-beta already has an open PR.
terraform-google-conversion already has an open PR.
terraform-provider-google already has an open PR.
Ansible already has an open PR.
InSpec already has an open PR.

New Pull Requests

I didn't open any new pull requests because of this PR.

@modular-magician
Copy link
Collaborator

Hi! I'm the modular magician, I work on Magic Modules.
I see that this PR has already had some downstream PRs generated. Any open downstreams are already updated to your most recent commit, b874de2.

Pull request statuses

terraform-provider-google-beta already has an open PR.
terraform-google-conversion already has an open PR.
terraform-provider-google already has an open PR.
Ansible already has an open PR.
InSpec already has an open PR.

New Pull Requests

I didn't open any new pull requests because of this PR.

@@ -637,6 +637,34 @@ func TestAccComputeBackendService_withCustomHeaders(t *testing.T) {
}
<% end -%>

<% unless version == 'ga' -%>
Copy link
Contributor

Choose a reason for hiding this comment

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

This hand-written test is only necessary in order to do the import part, right? If so, can you add a comment to remove it once the feature goes GA (since then we'd get import for the example test)?

@modular-magician
Copy link
Collaborator

Hi! I'm the modular magician, I work on Magic Modules.
I see that this PR has already had some downstream PRs generated. Any open downstreams are already updated to your most recent commit, a16bad3.

Pull request statuses

terraform-provider-google-beta already has an open PR.
terraform-google-conversion already has an open PR.
terraform-provider-google already has an open PR.
Ansible already has an open PR.
InSpec already has an open PR.

New Pull Requests

I didn't open any new pull requests because of this PR.

@slevenick slevenick force-pushed the internal-self-managed-backend branch from d5a736e to 9d100ae Compare May 31, 2019 20:07
@modular-magician
Copy link
Collaborator

Hi! I'm the modular magician, I work on Magic Modules.
I see that this PR has already had some downstream PRs generated. Any open downstreams are already updated to your most recent commit, 9d100ae.

Pull request statuses

terraform-provider-google-beta already has an open PR.
terraform-google-conversion already has an open PR.
terraform-provider-google already has an open PR.
Ansible already has an open PR.
InSpec already has an open PR.

New Pull Requests

I didn't open any new pull requests because of this PR.

@modular-magician modular-magician force-pushed the internal-self-managed-backend branch from 9d100ae to 8914332 Compare May 31, 2019 21:28
@modular-magician modular-magician merged commit c9ece49 into GoogleCloudPlatform:master May 31, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants